Skip to content

Commit 6c6e1af

Browse files
jchampiojchampio
committed
WIP: add a Parking Lot
The Parking Lot is a special always-open commitfest for the purpose of holding and testing draft patch submissions. Having such a holding area should make it easier for people to keep track of patchsets they're not quite ready to submit for review. Internally, the Parking Lot is assigned a static ID of zero. This is chosen because a) it does not overlap with Django's default AutoField sequence, which begins at one, and b) it requires no updates to the current URL patterns, which match nonnegative integers. The Parking Lot entry has the special status STATUS_PARKING so that it does not conflict with pre-existing coded assumptions on what "open", "future", etc. mean. STATUS_PARKING CFs are excluded from the "num_cfs" count for a patch. The new /close/parked handler is added to swap patches into the Parking Lot. Patches are then removed by moving them to the next open CF, or by closing as usual. Prior to this patch: - CFs with IDs less than the current in-progress CF could safely be assumed closed, - patches only ever moved forward through increasing CF IDs, and - the latest CF start date determined a patch's "current" CF. These assumptions all break under the current model. They have been modified: - use STATUS_CLOSED specifically when deciding whether a CF is closed - when moving a patch between CFs, allow for the possibility of an existing entry in the junction table - a patch's "current" CF is determined by its latest entry date TODO: - ensure all prior assumptions on CF ID are cleaned up - should the default filter be changed for the Parking Lot?
1 parent b611f21 commit 6c6e1af

File tree

8 files changed

+94
-10
lines changed

8 files changed

+94
-10
lines changed

‎media/commitfest/js/commitfest.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ function verify_next() {
1818
'Are you sure you want to move this patch to the next commitfest?\n\nThis means the patch will be marked as closed in this commitfest, but will automatically be moved to the next one. If no further work is expected on this patch, it should be closed with "Rejected" or "Returned with Feedback" instead.\n\nSo - are you sure?',
1919
);
2020
}
21+
function verify_parked() {
22+
return confirm(
23+
'Are you sure you want to park this patch?\n\nThis means the patch will be marked as closed in this commitfest, and automatically moved to the Parking Lot. Its status will be reset to Waiting on Author, and it will remain there until it is closed or moved to the next open commitfest.\n\nSo - are you sure?',
24+
);
25+
}
2126
function findLatestThreads() {
2227
$("#attachThreadListWrap").addClass("loading");
2328
$("#attachThreadSearchButton").addClass("disabled");

‎pgcommitfest/commitfest/migrations/0013_add_parking_lot.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Generated by Django 4.2.19 on 2025-03-10 16:28
2+
3+
from django.db import migrations
4+
5+
6+
class Migration(migrations.Migration):
7+
dependencies = [
8+
("commitfest", "0012_commitfest_status_parking"),
9+
]
10+
11+
operations = [
12+
migrations.RunSQL("""
13+
INSERT INTO commitfest_commitfest (id, name, status)
14+
VALUES (0, 'Parking Lot', 5);
15+
"""),
16+
]

‎pgcommitfest/commitfest/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ class Patch(models.Model, DiffableModel):
162162
}
163163

164164
def current_commitfest(self):
165-
return self.commitfests.order_by("-startdate").first()
165+
return self.commitfests.order_by("-patchoncommitfest__enterdate").first()
166166

167167
def current_patch_on_commitfest(self):
168168
cf = self.current_commitfest()

‎pgcommitfest/commitfest/templates/home.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ <h3>Commands</h3>
2424
</form>
2525
<h3>List of commitfests</h3>
2626
<ul>
27+
<li><a href="/0/">Parking Lot</a> (Drafts)</li>
2728
{%for c in commitfests%}
2829
<li><a href="/{{c.id}}/">{{c}}</a> ({{c.statusstring}}{%if c.startdate%} - {{c.periodstring}}{%endif%})</li>
2930
{%endfor%}

‎pgcommitfest/commitfest/templates/patch_commands.inc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
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>
2424
<li role="presentation"><a href="close/next/?cfid={{cf.id}}" onclick="return verify_next()">Move to next CF</a></li>
25+
<li role="presentation"><a href="close/parked/" onclick="return verify_parked()">Move to Parking Lot</a></li>
2526
<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>
2627
</ul>
2728
</div>

‎pgcommitfest/commitfest/views.py

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@
4646

4747

4848
def home(request):
49-
commitfests = list(CommitFest.objects.all())
49+
# Skip "special" commitfest holding areas (primary keys <= 0); they're
50+
# handled separately.
51+
commitfests = list(CommitFest.objects.filter(pk__gt=0))
5052
opencf = next((c for c in commitfests if c.status == CommitFest.STATUS_OPEN), None)
5153
inprogresscf = next(
5254
(c for c in commitfests if c.status == CommitFest.STATUS_INPROGRESS), None
@@ -447,6 +449,7 @@ def patchlist(request, cf, personalized=False):
447449
params = {
448450
"openstatuses": PatchOnCommitFest.OPEN_STATUSES,
449451
"cid": cf.id,
452+
"parking": CommitFest.STATUS_PARKING,
450453
}
451454
params.update(whereparams)
452455

@@ -458,7 +461,9 @@ def patchlist(request, cf, personalized=False):
458461
(poc.status=ANY(%(openstatuses)s)) AS is_open,
459462
(SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_authors cpa ON cpa.user_id=auth_user.id WHERE cpa.patch_id=p.id) AS author_names,
460463
(SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_reviewers cpr ON cpr.user_id=auth_user.id WHERE cpr.patch_id=p.id) AS reviewer_names,
461-
(SELECT count(1) FROM commitfest_patchoncommitfest pcf WHERE pcf.patch_id=p.id) AS num_cfs,
464+
(SELECT count(1) FROM commitfest_patchoncommitfest pcf
465+
INNER JOIN commitfest_commitfest cf ON cf.id = pcf.commitfest_id
466+
WHERE pcf.patch_id=p.id AND cf.status != %(parking)s) AS num_cfs,
462467
463468
branch.needs_rebase_since,
464469
branch.failing_since,
@@ -638,7 +643,7 @@ def patch(request, patchid):
638643
patch_commitfests = (
639644
PatchOnCommitFest.objects.select_related("commitfest")
640645
.filter(patch=patch)
641-
.order_by("-commitfest__startdate")
646+
.order_by("-enterdate")
642647
.all()
643648
)
644649
cf = patch_commitfests[0].commitfest
@@ -1034,6 +1039,7 @@ def close(request, patchid, status):
10341039
PatchOnCommitFest.STATUS_REVIEW,
10351040
PatchOnCommitFest.STATUS_AUTHOR,
10361041
PatchOnCommitFest.STATUS_COMMITTER,
1042+
PatchOnCommitFest.STATUS_PARKED,
10371043
):
10381044
# This one can be moved
10391045
pass
@@ -1086,14 +1092,66 @@ def close(request, patchid, status):
10861092
"/%s/%s/" % (poc.commitfest.id, poc.patch.id)
10871093
)
10881094
# Create a mapping to the new commitfest that we are bouncing
1089-
# this patch to.
1090-
newpoc = PatchOnCommitFest(
1095+
# this patch to. Patches may be bounced back and forth from the parking
1096+
# lot, so we have to handle a potential previous entry for this patch.
1097+
PatchOnCommitFest.objects.update_or_create(
10911098
patch=poc.patch,
10921099
commitfest=newcf[0],
1093-
status=oldstatus,
1094-
enterdate=datetime.now(),
1100+
defaults=dict(
1101+
status=oldstatus,
1102+
enterdate=datetime.now(),
1103+
leavedate=None,
1104+
),
1105+
)
1106+
elif status == "parked":
1107+
# Parking has similar considerations to "next", but we're more lenient
1108+
# about what can be moved in.
1109+
if poc.status in (
1110+
PatchOnCommitFest.STATUS_COMMITTED,
1111+
PatchOnCommitFest.STATUS_NEXT,
1112+
PatchOnCommitFest.STATUS_PARKED,
1113+
PatchOnCommitFest.STATUS_REJECTED,
1114+
):
1115+
# Can't be moved!
1116+
messages.error(
1117+
request,
1118+
"A patch in status {0} cannot be moved to the parking lot.".format(
1119+
poc.statusstring
1120+
),
1121+
)
1122+
return HttpResponseRedirect("/%s/%s/" % (poc.commitfest.id, poc.patch.id))
1123+
elif poc.status in (
1124+
PatchOnCommitFest.STATUS_AUTHOR,
1125+
PatchOnCommitFest.STATUS_COMMITTER,
1126+
PatchOnCommitFest.STATUS_RETURNED,
1127+
PatchOnCommitFest.STATUS_REVIEW,
1128+
PatchOnCommitFest.STATUS_WITHDRAWN,
1129+
):
1130+
# This one can be moved
1131+
pass
1132+
else:
1133+
messages.error(request, "Invalid existing patch status")
1134+
1135+
poc.status = PatchOnCommitFest.STATUS_PARKED
1136+
1137+
# Map this patch directly to the parking lot.
1138+
try:
1139+
parking_lot = CommitFest.objects.get(pk=0)
1140+
except CommitFest.DoesNotExist:
1141+
messages.error(request, "No parking lot exists!")
1142+
return HttpResponseRedirect("/%s/%s/" % (poc.commitfest.id, poc.patch.id))
1143+
1144+
# Patches may be bounced back and forth from the parking lot, so we have
1145+
# to handle a potential previous entry for this patch.
1146+
PatchOnCommitFest.objects.update_or_create(
1147+
patch=poc.patch,
1148+
commitfest=parking_lot,
1149+
defaults=dict(
1150+
status=PatchOnCommitFest.STATUS_AUTHOR,
1151+
enterdate=datetime.now(),
1152+
leavedate=None,
1153+
),
10951154
)
1096-
newpoc.save()
10971155
elif status == "committed":
10981156
committer = get_object_or_404(Committer, user__username=request.GET["c"])
10991157
if committer != poc.patch.committer:

‎pgcommitfest/settings.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
}
2020
}
2121

22+
# "Static" commitfests use primary key values <= 0, so this must be an unsigned
23+
# field type.
2224
DEFAULT_AUTO_FIELD = "django.db.models.AutoField"
2325

2426
# Local time zone for this installation. Choices can be found here:

‎pgcommitfest/urls.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@
2727
re_path(r"^(\d+)/new/$", views.newpatch),
2828
re_path(r"^patch/(\d+)/status/(review|author|committer)/$", views.status),
2929
re_path(
30-
r"^patch/(\d+)/close/(reject|withdrawn|feedback|committed|next)/$", views.close
30+
r"^patch/(\d+)/close/(reject|withdrawn|feedback|committed|next|parked)/$",
31+
views.close,
3132
),
3233
re_path(r"^patch/(\d+)/reviewer/(become|remove)/$", views.reviewer),
3334
re_path(r"^patch/(\d+)/committer/(become|remove)/$", views.committer),

0 commit comments

Comments
 (0)