Add next button to navigate within a patch series #574
Conversation
|
Could you rebase on |
|
Could you also provide a partial screenshot of what this looks like, just to avoid me spinning up a dev environment to test this? 😅 |
miguelsaura
force-pushed
the
add-next-button
branch
from
0fe3f5c
to
5a395a7
Compare
@stephenfin Sure, the button is hidden when viewing the last patch in a series.
|
| @@ -136,6 +136,13 @@ def patch_detail(request, project_id, msgid): | |||
| related_same_project = [] | |||
| related_different_project = [] | |||
|
|
|||
| context['next_submission'] = None | |||
| patch_series = patch.series.patches.all() | |||
| num_patches = patch.series.patches.count() | |||
There was a problem hiding this comment.
This is a very expensive query. We're not currently joining on .series or .series.patches, which means we're lazy loading a lot of this. I see our total queries on the page go from 20 to 49 (you can see this yourself using django-debug-toolbar, which is available on the top right of the viewport).
This needs to be addressed. Given we're already loading some series information to populate the hidden Series pane, I think this should be relatively easy to address.
| context['next_submission'] = None | ||
| patch_series = patch.series.patches.all() | ||
| num_patches = patch.series.patches.count() | ||
| for idx in range(num_patches - 1): |
There was a problem hiding this comment.
To the best of my knowledge, the sort order for this is based on Patch.Meta.ordering, which is date. Patch.date is typically taken from the send date of the email, not the received date. I think it's fair to say patch 1/N will always have an earlier/the same date as patch 2/N, as so on. Might be worth double checking though.
(fwiw, I think this is also an issue for the Series pane)
Closes #510