-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve speed of SQL queries & admin dashboard load #1719
Conversation
@@ -497,13 +499,13 @@ def init_curation_status | |||
|
|||
# Date on which the user first submitted this resource | |||
def submitted_date | |||
curation_activities.order(:id).where("status = 'submitted' OR status = 'peer_review'")&.first&.created_at | |||
process_date.submitted || process_date.peer_review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this changing the meaning of the date? It might make sense to reverse the order, since peer_review dates will typically be before submitted dates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is changing it a bit. On the admin dashboard the submission date helps curators decide what order to do things in based on how long they have been waiting in the queue, in which case using an earlier peer review date over the submitted date makes it seem like things have been there for months and something has gone wrong. I think it makes sense to prefer the date the status actually goes to 'submitted'. Should we discuss more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check with Molly? She has the best understanding of who is using each type of date.
@@ -72,7 +72,7 @@ | |||
<td class="c-lined-table__digits"><%= formatted_datetime(dataset.last_curation_activity.updated_at) %></span></td> | |||
<% end %> | |||
<% if @fields.include?('submit_date') %> | |||
<td class="c-lined-table__digits"><%= formatted_datetime(dataset.submit_date) %></td> | |||
<td class="c-lined-table__digits"><%= formatted_datetime(dataset.process_date.submitted || dataset.process_date.peer_review) %></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, it might make sense to switch the order here, so any peer_review date is preferred. (Depending on how the submitted date is calculated)
dpc = dataset.identifier.payment_id.split("funder:").last.split("|").first if dataset.identifier.payment_id.starts_with?('funder') | ||
row << dpc | ||
end | ||
row << dataset.last_curation_activity.updated_at if @fields.include?('updated_at') | ||
row << dataset.submit_date if @fields.include?('submit_date') | ||
row << (dataset.process_date.submitted || dataset.process_date.peer_review) if @fields.include?('submit_date') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here too
This is ready to merge after the above comments are resolved. |
Through careful changing of how and when (and what kind of) joins are loaded, I've been able to increase the speed of queries for this page dramatically (on dev).
In addition to the changes directly to the page, I've created migrations & changes for new tables which should expand our ability to use associations instead of complicated query calculations or methods generally—the dates table should make it much easier to calculate curation stats, though I am waiting until the reporting and stats decisions are made to make those changes.