Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add next button to navigate within a patch series #574

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add next button to navigate within a patch series #574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions patchwork/templates/patchwork/submission.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@
<h1>{{ submission.name }}</h1>
</div>

{% if next_submission %}
<div class="btn-group pull-right">
<a class="btn btn-default"
href="{% url 'patch-detail' project_id=project.linkname msgid=next_submission.encoded_msgid %}">
next
</a>
</div>
{% endif %}

<table id="patch-meta" class="patch-meta" data-submission-type={{submission|verbose_name_plural|lower}} data-submission-id={{submission.id}}>
<tr>
<th>Message ID</th>
Expand Down
7 changes: 7 additions & 0 deletions patchwork/views/patch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

image

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.

for idx in range(num_patches - 1):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

if patch.id == patch_series[idx].id:
context['next_submission'] = patch_series[idx + 1]

context['comments'] = comments
context['checks'] = Patch.filter_unique_checks(
patch.check_set.all().select_related('user'),
Expand Down
Loading