Skip to content

Commit d33830d

Browse files
committed
Add support for removing all reviewers from PGConf.dev tagged patches
1 parent 1ef7a40 commit d33830d

6 files changed

Lines changed: 103 additions & 1 deletion

File tree

pgcommitfest/commitfest/fixtures/commitfest_data.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,15 @@
353353
"description": "Improves security"
354354
}
355355
},
356+
{
357+
"model": "commitfest.tag",
358+
"pk": 30,
359+
"fields": {
360+
"name": "PGConf.dev",
361+
"color": "#336791",
362+
"description": "part of the PGConf.dev in-person commitfest — see https://wiki.postgresql.org/wiki/PGConf.dev_2026_In-Person_Commitfest"
363+
}
364+
},
356365
{
357366
"model": "commitfest.patch",
358367
"pk": 1,

pgcommitfest/commitfest/models.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,10 @@ def current_patch_on_commitfest(self):
536536
PatchOnCommitFest, Q(patch=self) & ~Q(status=PatchOnCommitFest.STATUS_MOVED)
537537
)
538538

539+
@property
540+
def can_remove_all_reviewers(self):
541+
return self.tags.filter(name="PGConf.dev").exists()
542+
539543
# Some accessors
540544
@property
541545
def authors_string(self):

pgcommitfest/commitfest/templates/patch.html

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,11 @@
119119
</tr>
120120
<tr>
121121
<th>Reviewers</th>
122-
<td>{{patch.reviewers_string}}<a href="reviewer/{{is_reviewer|yesno:"remove,become"}}/" class="btn btn-secondary float-end">{{is_reviewer|yesno:"Remove from reviewers,Become reviewer"}}</a></td>
122+
<td>
123+
{{patch.reviewers_string}}
124+
{%if is_reviewer%}<a href="reviewer/remove/" class="btn btn-secondary float-end">Remove from reviewers</a>{%else%}<a href="reviewer/become/" class="btn btn-secondary float-end">Become reviewer</a>{%endif%}
125+
{%if patch.can_remove_all_reviewers and patch.reviewers.all%}<a href="reviewer/remove_all/" class="btn btn-secondary float-end me-1">Remove all reviewers</a>{%endif%}
126+
</td>
123127
</tr>
124128
<tr>
125129
<th>Committer</th>
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
from datetime import datetime
2+
3+
import pytest
4+
5+
from pgcommitfest.commitfest.models import (
6+
Patch,
7+
PatchHistory,
8+
PatchOnCommitFest,
9+
PendingNotification,
10+
Tag,
11+
)
12+
from pgcommitfest.userprofile.models import UserProfile
13+
14+
pytestmark = pytest.mark.django_db
15+
16+
17+
def test_remove_all_reviewers(client, open_cf, alice, bob, charlie):
18+
UserProfile.objects.create(user=bob, notify_all_reviewer=True)
19+
20+
pgconfdev_tag = Tag.objects.create(
21+
name="PGConf.dev", color="#000000", description="PGConf.dev"
22+
)
23+
patch = Patch.objects.create(name="Test patch")
24+
patch.authors.add(alice)
25+
patch.reviewers.add(bob, charlie)
26+
patch.tags.add(pgconfdev_tag)
27+
PatchOnCommitFest.objects.create(
28+
patch=patch,
29+
commitfest=open_cf,
30+
enterdate=datetime.now(),
31+
status=PatchOnCommitFest.STATUS_REVIEW,
32+
)
33+
34+
client.force_login(alice)
35+
response = client.get(f"/patch/{patch.id}/reviewer/remove_all/")
36+
assert response.status_code == 302
37+
assert list(patch.reviewers.all()) == []
38+
assert (
39+
"Removed all reviewers"
40+
in PatchHistory.objects.filter(patch=patch).latest("date").what
41+
)
42+
assert PendingNotification.objects.filter(user=bob).exists()
43+
44+
45+
def test_remove_all_reviewers_forbidden_without_tag(client, open_cf, alice, bob):
46+
patch = Patch.objects.create(name="Test patch")
47+
patch.authors.add(alice)
48+
patch.reviewers.add(bob)
49+
PatchOnCommitFest.objects.create(
50+
patch=patch,
51+
commitfest=open_cf,
52+
enterdate=datetime.now(),
53+
status=PatchOnCommitFest.STATUS_REVIEW,
54+
)
55+
56+
client.force_login(alice)
57+
response = client.get(f"/patch/{patch.id}/reviewer/remove_all/")
58+
assert response.status_code == 403
59+
assert bob in patch.reviewers.all()

pgcommitfest/commitfest/views.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,31 @@ def reviewer(request, patchid, status):
13531353
return HttpResponseRedirect("../../")
13541354

13551355

1356+
@login_required
1357+
@transaction.atomic
1358+
def remove_all_reviewers(request, patchid):
1359+
patch = get_object_or_404(Patch, pk=patchid)
1360+
1361+
if not patch.can_remove_all_reviewers:
1362+
return HttpResponseForbidden(
1363+
"Removing reviewers is only allowed for PGConf.dev patches."
1364+
)
1365+
1366+
prevreviewers = list(patch.reviewers.all())
1367+
if not prevreviewers:
1368+
return HttpResponseRedirect(f"/patch/{patchid}/")
1369+
1370+
patch.reviewers.clear()
1371+
patch.set_modified()
1372+
patch.save()
1373+
PatchHistory(
1374+
patch=patch,
1375+
by=request.user,
1376+
what="Removed all reviewers",
1377+
).save_and_notify(prevreviewers=prevreviewers)
1378+
return HttpResponseRedirect(f"/patch/{patchid}/")
1379+
1380+
13561381
@login_required
13571382
@transaction.atomic
13581383
def committer(request, patchid, status):

pgcommitfest/urls.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
re_path(r"^patch/(\d+)/close/(reject|withdrawn|feedback|committed)/$", views.close),
3636
re_path(r"^patch/(\d+)/move/$", views.move),
3737
re_path(r"^patch/(\d+)/reviewer/(become|remove)/$", views.reviewer),
38+
re_path(r"^patch/(\d+)/reviewer/remove_all/$", views.remove_all_reviewers),
3839
re_path(r"^patch/(\d+)/committer/(become|remove)/$", views.committer),
3940
re_path(r"^patch/(\d+)/(un)?subscribe/$", views.subscribe),
4041
re_path(r"^patch/(\d+)/(comment|review)/", views.comment),

0 commit comments

Comments
 (0)