Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 0957e96

Browse filesBrowse files
JelteFpolkerty
JelteF
authored and
polkerty
committed
Change the direction of the dropdowns at the bottom of the patch page.
1 parent 5a2ff44 commit 0957e96
Copy full SHA for 0957e96

File tree

7 files changed

+67
-57
lines changed
Filter options

7 files changed

+67
-57
lines changed

‎README.md

Copy file name to clipboardExpand all lines: README.md
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ A commitfest is a collection of patches and reviews for a project and is part of
66

77
## The Application
88

9-
This is a Django 3.2 application backed by PostgreSQL and running on Python 3.x.
9+
This is a Django 4.2 application backed by PostgreSQL and running on Python 3.x.
1010

1111
## Getting Started
1212

‎media/commitfest/css/commitfest.css

Copy file name to clipboardExpand all lines: media/commitfest/css/commitfest.css
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ div.form-group div.controls input.threadpick-input {
3737
display: inline;
3838
}
3939

40+
.dropdown-menu--up {
41+
top: initial;
42+
bottom: 100%;
43+
}
4044

4145

4246
/*

‎pgcommitfest/commitfest/models.py

Copy file name to clipboardExpand all lines: pgcommitfest/commitfest/models.py
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ class Patch(models.Model, DiffableModel):
122122
'reviewers': 'reviewers_string',
123123
}
124124

125+
def current_commitfest(self):
126+
return self.commitfests.order_by('-startdate').first()
127+
125128
# Some accessors
126129
@property
127130
def authors_string(self):

‎pgcommitfest/commitfest/templates/patch.html

Copy file name to clipboardExpand all lines: pgcommitfest/commitfest/templates/patch.html
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,9 @@ <h4>Annotations</h4>
183183
</tbody>
184184
</table>
185185

186+
{% with dropdown_mode="dropdown-menu--up" %}
186187
{%include "patch_commands.inc"%}
188+
{% endwith %}
187189

188190
{%comment%}commit dialog{%endcomment%}
189191
<div class="modal fade" id="commitModal" role="dialog">

‎pgcommitfest/commitfest/templates/patch_commands.inc

Copy file name to clipboardExpand all lines: pgcommitfest/commitfest/templates/patch_commands.inc
+5-5Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33

44
<div class="btn-group">
55
<a class="btn btn-default dropdown-toggle" data-toggle="dropdown" href="#">Comment/Review <span class="caret"></span></a>
6-
<ul class="dropdown-menu">
6+
<ul class="dropdown-menu {{ dropdown_mode }}">
77
<li><a href="comment/">Comment</a>
88
<li><a href="review/">Review</a>
99
</ul>
1010
</div>
1111

1212
<div class="btn-group">
1313
<a class="btn btn-default dropdown-toggle" data-toggle="dropdown" href="#">Change Status <span class="caret"></span></a>
14-
<ul class="dropdown-menu" role="menu">
14+
<ul class="dropdown-menu {{ dropdown_mode }}" role="menu">
1515
<li role="presentation" class="dropdown-header">Open statuses</li>
1616
<li role="presentation"><a href="status/review/">Needs review</a></li>
1717
<li role="presentation"><a href="status/author/">Waiting on Author</a></li>
@@ -21,20 +21,20 @@
2121
<li role="presentation"><a href="close/reject/" onclick="return verify_reject()">Rejected</a></li>
2222
<li role="presentation"><a href="close/withdrawn/" onclick="return verify_withdrawn()">Withdrawn</a></li>
2323
<li role="presentation"><a href="close/feedback/" onclick="return verify_returned()">Returned with feedback</a></li>
24-
<li role="presentation"><a href="close/next/" onclick="return verify_next()">Move to next CF</a></li>
24+
<li role="presentation"><a href="close/next/?cfid={{cf.id}}" onclick="return verify_next()">Move to next CF</a></li>
2525
<li role="presentation"><a href="close/committed/" onclick="return flagCommitted({%if patch.committer%}'{{patch.committer}}'{%elif is_committer%}'{{user.username}}'{%else%}null{%endif%})">Committed</a></li>
2626
</ul>
2727
</div>
2828

2929
{%if request.user.is_staff%}
3030
<div class="btn-group">
3131
<a class="btn btn-default dropdown-toggle" data-toggle="dropdown" href="#">Send private mail <span class="caret"></span></a>
32-
<ul class="dropdown-menu">
32+
<ul class="dropdown-menu {{ dropdown_mode }}">
3333
<li><a href="send_email/?authors={{patch.id}}">Send mail to authors</a></li>
3434
<li><a href="send_email/?reviewers={{patch.id}}">Send mail to reviewers</a></li>
3535
<li><a href="send_email/?authors={{patch.id}}&reviewers={{patch.id}}">Send mail to authors and reviewers</a></li>
3636
</ul>
3737
</div>
3838
{%endif%}
3939

40-
</div>
40+
</div>

‎pgcommitfest/commitfest/views.py

Copy file name to clipboardExpand all lines: pgcommitfest/commitfest/views.py
+36-42Lines changed: 36 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,9 @@ def patch(request, patchid):
374374

375375
@login_required
376376
@transaction.atomic
377-
def patchform(request, cfid, patchid):
378-
cf = get_object_or_404(CommitFest, pk=cfid)
379-
patch = get_object_or_404(Patch, pk=patchid, commitfests=cf)
377+
def patchform(request, patchid):
378+
patch = get_object_or_404(Patch, pk=patchid)
379+
cf = patch.current_commitfest()
380380

381381
prevreviewers = list(patch.reviewers.all())
382382
prevauthors = list(patch.authors.all())
@@ -466,21 +466,12 @@ def _review_status_string(reviewstatus):
466466

467467
@login_required
468468
@transaction.atomic
469-
def comment(request, cfid, patchid, what):
470-
cf = get_object_or_404(CommitFest, pk=cfid)
469+
def comment(request, patchid, what):
471470
patch = get_object_or_404(Patch, pk=patchid)
471+
cf = patch.current_commitfest()
472472
poc = get_object_or_404(PatchOnCommitFest, patch=patch, commitfest=cf)
473473
is_review = (what == 'review')
474474

475-
if poc.is_closed:
476-
# We allow modification of patches in closed CFs *only* if it's the
477-
# last CF that the patch is part of. If it's part of another CF, that
478-
# is later than this one, tell the user to go there instead.
479-
lastcf = PatchOnCommitFest.objects.filter(patch=patch).order_by('-commitfest__startdate')[0]
480-
if poc != lastcf:
481-
messages.add_message(request, messages.INFO, "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!")
482-
return HttpResponseRedirect('..')
483-
484475
if request.method == 'POST':
485476
try:
486477
form = CommentForm(patch, poc, is_review, data=request.POST)
@@ -563,17 +554,10 @@ def comment(request, cfid, patchid, what):
563554

564555
@login_required
565556
@transaction.atomic
566-
def status(request, cfid, patchid, status):
567-
poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cfid, patch__id=patchid)
568-
569-
if poc.is_closed:
570-
# We allow modification of patches in closed CFs *only* if it's the
571-
# last CF that the patch is part of. If it's part of another CF, that
572-
# is later than this one, tell the user to go there instead.
573-
lastcf = PatchOnCommitFest.objects.filter(patch__id=patchid).order_by('-commitfest__startdate')[0]
574-
if poc != lastcf:
575-
messages.add_message(request, messages.INFO, "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!")
576-
return HttpResponseRedirect('/%s/%s/' % (poc.commitfest.id, poc.patch.id))
557+
def status(request, patchid, status):
558+
patch = get_object_or_404(Patch.objects.select_related(), pk=patchid)
559+
cf = patch.current_commitfest()
560+
poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cf.id, patch__id=patchid)
577561

578562
if status == 'review':
579563
newstatus = PatchOnCommitFest.STATUS_REVIEW
@@ -593,22 +577,29 @@ def status(request, cfid, patchid, status):
593577

594578
PatchHistory(patch=poc.patch, by=request.user, what='New status: %s' % poc.statusstring).save_and_notify()
595579

596-
return HttpResponseRedirect('/%s/%s/' % (poc.commitfest.id, poc.patch.id))
580+
return HttpResponseRedirect('/patch/%s/' % (poc.patch.id))
597581

598582

599583
@login_required
600584
@transaction.atomic
601-
def close(request, cfid, patchid, status):
602-
poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cfid, patch__id=patchid)
603-
604-
if poc.is_closed:
605-
# We allow modification of patches in closed CFs *only* if it's the
606-
# last CF that the patch is part of. If it's part of another CF, that
607-
# is later than this one, tell the user to go there instead.
608-
lastcf = PatchOnCommitFest.objects.filter(patch__id=patchid).order_by('-commitfest__startdate')[0]
609-
if poc != lastcf:
610-
messages.add_message(request, messages.INFO, "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!")
611-
return HttpResponseRedirect('/%s/%s/' % (poc.commitfest.id, poc.patch.id))
585+
def close(request, patchid, status):
586+
patch = get_object_or_404(Patch.objects.select_related(), pk=patchid)
587+
cf = patch.current_commitfest()
588+
589+
try:
590+
request_cfid = int(request.GET.get('cfid', ''))
591+
except ValueError:
592+
# int() failed, ignore
593+
request_cfid = None
594+
595+
if request_cfid is not None and request_cfid != cf.id:
596+
# The cfid parameter is only added to the /next/ link. That's the only
597+
# close operation where two people pressing the button at the same time
598+
# can have unintended effects.
599+
messages.error(request, "The patch was moved to a new commitfest by someone else. Please double check if you still want to retry this operation.")
600+
return HttpResponseRedirect('/%s/%s/' % (cf.id, patch.id))
601+
602+
poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cf.id, patch__id=patchid)
612603

613604
poc.leavedate = datetime.now()
614605

@@ -696,8 +687,7 @@ def close(request, cfid, patchid, status):
696687

697688
@login_required
698689
@transaction.atomic
699-
def reviewer(request, cfid, patchid, status):
700-
get_object_or_404(CommitFest, pk=cfid)
690+
def reviewer(request, patchid, status):
701691
patch = get_object_or_404(Patch, pk=patchid)
702692

703693
is_reviewer = request.user in patch.reviewers.all()
@@ -716,7 +706,6 @@ def reviewer(request, cfid, patchid, status):
716706
@login_required
717707
@transaction.atomic
718708
def committer(request, cfid, patchid, status):
719-
get_object_or_404(CommitFest, pk=cfid)
720709
patch = get_object_or_404(Patch, pk=patchid)
721710

722711
committer = list(Committer.objects.filter(user=request.user, active=True))
@@ -741,8 +730,7 @@ def committer(request, cfid, patchid, status):
741730

742731
@login_required
743732
@transaction.atomic
744-
def subscribe(request, cfid, patchid, sub):
745-
get_object_or_404(CommitFest, pk=cfid)
733+
def subscribe(request, patchid, sub):
746734
patch = get_object_or_404(Patch, pk=patchid)
747735

748736
if sub == 'un':
@@ -755,6 +743,12 @@ def subscribe(request, cfid, patchid, sub):
755743
return HttpResponseRedirect("../")
756744

757745

746+
def send_patch_email(request, patchid):
747+
patch = get_object_or_404(Patch, pk=patchid)
748+
cf = patch.current_commitfest()
749+
return send_email(request, cf.id)
750+
751+
758752
@login_required
759753
@transaction.atomic
760754
def send_email(request, cfid):

‎pgcommitfest/urls.py

Copy file name to clipboardExpand all lines: pgcommitfest/urls.py
+16-9Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,25 +19,32 @@
1919
re_path(r'^(\d+)/$', views.commitfest),
2020
re_path(r'^(open|inprogress|current)/(.*)$', views.redir),
2121
re_path(r'^(?P<cfid>\d+)/activity(?P<rss>\.rss)?/$', views.activity),
22-
re_path(r'^patch/(\d+)/$', views.patch),
2322
re_path(r'^(\d+)/(\d+)/$', views.patch_legacy_redirect),
24-
re_path(r'^(\d+)/(\d+)/edit/$', views.patchform),
23+
re_path(r'^patch/(\d+)/$', views.patch),
24+
re_path(r'^patch/(\d+)/edit/$', views.patchform),
2525
re_path(r'^(\d+)/new/$', views.newpatch),
26-
re_path(r'^(\d+)/(\d+)/status/(review|author|committer)/$', views.status),
27-
re_path(r'^(\d+)/(\d+)/close/(reject|withdrawn|feedback|committed|next)/$', views.close),
28-
re_path(r'^(\d+)/(\d+)/reviewer/(become|remove)/$', views.reviewer),
29-
re_path(r'^(\d+)/(\d+)/committer/(become|remove)/$', views.committer),
30-
re_path(r'^(\d+)/(\d+)/(un)?subscribe/$', views.subscribe),
31-
re_path(r'^(\d+)/(\d+)/(comment|review)/', views.comment),
26+
re_path(r'^patch/(\d+)/status/(review|author|committer)/$', views.status),
27+
re_path(r'^patch/(\d+)/close/(reject|withdrawn|feedback|committed|next)/$', views.close),
28+
re_path(r'^patch/(\d+)/reviewer/(become|remove)/$', views.reviewer),
29+
re_path(r'^patch/(\d+)/committer/(become|remove)/$', views.committer),
30+
re_path(r'^patch/(\d+)/(un)?subscribe/$', views.subscribe),
31+
re_path(r'^patch/(\d+)/(comment|review)/', views.comment),
3232
re_path(r'^(\d+)/send_email/$', views.send_email),
33-
re_path(r'^(\d+)/\d+/send_email/$', views.send_email),
33+
re_path(r'^patch/(\d+)/send_email/$', views.send_patch_email),
3434
re_path(r'^(\d+)/reports/authorstats/$', reports.authorstats),
3535
re_path(r'^search/$', views.global_search),
3636
re_path(r'^ajax/(\w+)/$', ajax.main),
3737
re_path(r'^lookups/user/$', lookups.userlookup),
3838
re_path(r'^thread_notify/$', views.thread_notify),
3939
re_path(r'^cfbot_notify/$', views.cfbot_notify),
4040

41+
# Legacy email POST route. This can be safely removed in a few days from
42+
# the first time this is deployed. It's only puprose is not breaking
43+
# submissions from a previous page lood, during the deploy of the new
44+
# /patch/(\d+) routes. It would be a shame if someone lost their well
45+
# written email because of this.
46+
re_path(r'^\d+/(\d+)/send_email/$', views.send_patch_email),
47+
4148
# Auth system integration
4249
re_path(r'^(?:account/)?login/?$', pgcommitfest.auth.login),
4350
re_path(r'^(?:account/)?logout/?$', pgcommitfest.auth.logout),

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.