Add a Parking Lot

media/commitfest/js/commitfest.js

@@ -18,6 +18,11 @@ function verify_next() {
         '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?',
     );
 }
+function verify_parked() {
+    return confirm(
+        '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?',
+    );
+}
 function findLatestThreads() {
     $("#attachThreadListWrap").addClass("loading");
     $("#attachThreadSearchButton").addClass("disabled");

pgcommitfest/commitfest/feeds.py

@@ -30,12 +30,7 @@ def item_description(self, item):
         )
 
     def item_link(self, item):
-        if self.cfid:
-            return "https://commitfest.postgresql.org/{0}/{1}/".format(
-                self.cfid, item["patchid"]
-            )
-        else:
-            return "https://commitfest.postgresql.org/{cfid}/{patchid}/".format(**item)
+        return "https://commitfest.postgresql.org/patch/{0}/".format(item["patchid"])
 
     def item_pubdate(self, item):
         return item["date"]

pgcommitfest/commitfest/migrations/0011_patch_status_parked.py

@@ -0,0 +1,46 @@
+# Generated by Django 4.2.19 on 2025-03-07 23:55
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("commitfest", "0010_add_failing_since_column"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="patchoncommitfest",
+            name="status",
+            field=models.IntegerField(
+                choices=[
+                    (1, "Needs review"),
+                    (2, "Waiting on Author"),
+                    (3, "Ready for Committer"),
+                    (4, "Committed"),
+                    (5, "Moved to next CF"),
+                    (6, "Rejected"),
+                    (7, "Returned with feedback"),
+                    (8, "Withdrawn"),
+                    (9, "Moved to Parking Lot"),
+                ],
+                default=1,
+            ),
+        ),
+        migrations.RunSQL("""
+INSERT INTO commitfest_patchstatus (status, statusstring, sortkey) VALUES
+(1,'Needs review',10),
+(2,'Waiting on Author',15),
+(3,'Ready for Committer',20),
+(4,'Committed',25),
+(5,'Moved to next CF',30),
+(6,'Rejected',50),
+(7,'Returned with Feedback',50),
+(8,'Withdrawn', 50),
+(9,'Moved to Parking Lot', 30)
+ON CONFLICT (status) DO UPDATE SET statusstring=excluded.statusstring, sortkey=excluded.sortkey;
+"""),
+        migrations.RunSQL(
+            "DELETE FROM commitfest_patchstatus WHERE status < 1 OR status > 9"
+        ),
+    ]

pgcommitfest/commitfest/migrations/0012_commitfest_status_parking.py

@@ -0,0 +1,26 @@
+# Generated by Django 4.2.19 on 2025-03-08 03:56
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("commitfest", "0011_patch_status_parked"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="commitfest",
+            name="status",
+            field=models.IntegerField(
+                choices=[
+                    (1, "Future"),
+                    (2, "Open"),
+                    (3, "In Progress"),
+                    (4, "Closed"),
+                    (5, "Parking"),
+                ],
+                default=1,
+            ),
+        ),
+    ]

pgcommitfest/commitfest/migrations/0013_add_parking_lot.py

@@ -0,0 +1,16 @@
+# Generated by Django 4.2.19 on 2025-03-10 16:28
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("commitfest", "0012_commitfest_status_parking"),
+    ]
+
+    operations = [
+        migrations.RunSQL("""
+            INSERT INTO commitfest_commitfest (id, name, status)
+                                       VALUES (0, 'Parking Lot', 5);
+        """),
+    ]

pgcommitfest/commitfest/models.py

@@ -38,17 +38,20 @@ class CommitFest(models.Model):
     STATUS_OPEN = 2
     STATUS_INPROGRESS = 3
     STATUS_CLOSED = 4
+    STATUS_PARKING = 5
     _STATUS_CHOICES = (
         (STATUS_FUTURE, "Future"),
         (STATUS_OPEN, "Open"),
         (STATUS_INPROGRESS, "In Progress"),
         (STATUS_CLOSED, "Closed"),
+        (STATUS_PARKING, "Parking"),
     )
     _STATUS_LABELS = (
         (STATUS_FUTURE, "default"),
         (STATUS_OPEN, "info"),
         (STATUS_INPROGRESS, "success"),
         (STATUS_CLOSED, "danger"),
+        (STATUS_PARKING, "info"),
     )
     name = models.CharField(max_length=100, blank=False, null=False, unique=True)
     status = models.IntegerField(
@@ -159,7 +162,7 @@ class Patch(models.Model, DiffableModel):
     }
 
     def current_commitfest(self):
-        return self.commitfests.order_by("-startdate").first()
+        return self.commitfests.order_by("-patchoncommitfest__enterdate").first()
 
     def current_patch_on_commitfest(self):
         cf = self.current_commitfest()
@@ -228,6 +231,7 @@ class PatchOnCommitFest(models.Model):
     STATUS_REJECTED = 6
     STATUS_RETURNED = 7
     STATUS_WITHDRAWN = 8
+    STATUS_PARKED = 9
     _STATUS_CHOICES = (
         (STATUS_REVIEW, "Needs review"),
         (STATUS_AUTHOR, "Waiting on Author"),
@@ -237,6 +241,7 @@ class PatchOnCommitFest(models.Model):
         (STATUS_REJECTED, "Rejected"),
         (STATUS_RETURNED, "Returned with feedback"),
         (STATUS_WITHDRAWN, "Withdrawn"),
+        (STATUS_PARKED, "Moved to Parking Lot"),
     )
     _STATUS_LABELS = (
         (STATUS_REVIEW, "default"),
@@ -247,6 +252,7 @@ class PatchOnCommitFest(models.Model):
         (STATUS_REJECTED, "danger"),
         (STATUS_RETURNED, "danger"),
         (STATUS_WITHDRAWN, "danger"),
+        (STATUS_PARKED, "warning"),
     )
     OPEN_STATUSES = [STATUS_REVIEW, STATUS_AUTHOR, STATUS_COMMITTER]
 

pgcommitfest/commitfest/templates/activity.html

@@ -16,7 +16,7 @@
     <tr>
      <td style="white-space: nowrap;">{{a.date}}</td>
      <td>{{a.by}}</td>
-     <td><a href="/{%if commitfest%}{{commitfest.id}}{%else%}{{a.cfid}}{%endif%}/{{a.patchid}}/">{{a.name}}</a></td>
+     <td><a href="/patch/{{a.patchid}}/">{{a.name}}</a></td>
      <td>{{a.what}}</td>
     </tr>
    {%endfor%}

pgcommitfest/commitfest/templates/home.html

@@ -24,6 +24,7 @@ <h3>Commands</h3>
  </form>
  <h3>List of commitfests</h3>
  <ul>
+   <li><a href="/0/">Parking Lot</a> (Drafts)</li>
   {%for c in commitfests%}
    <li><a href="/{{c.id}}/">{{c}}</a> ({{c.statusstring}}{%if c.startdate%} - {{c.periodstring}}{%endif%})</li>
   {%endfor%}

pgcommitfest/commitfest/templates/patch_commands.inc

@@ -22,6 +22,7 @@
      <li role="presentation"><a href="close/withdrawn/" onclick="return verify_withdrawn()">Withdrawn</a></li>
      <li role="presentation"><a href="close/feedback/" onclick="return verify_returned()">Returned with feedback</a></li>
      <li role="presentation"><a href="close/next/?cfid={{cf.id}}" onclick="return verify_next()">Move to next CF</a></li>
+     <li role="presentation"><a href="close/parked/" onclick="return verify_parked()">Move to Parking Lot</a></li>
      <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>
     </ul>
    </div>

pgcommitfest/commitfest/views.py

@@ -46,7 +46,9 @@
 
 
 def home(request):
-    commitfests = list(CommitFest.objects.all())
+    # Skip "special" commitfest holding areas (primary keys <= 0); they're
+    # handled separately.
+    commitfests = list(CommitFest.objects.filter(pk__gt=0))
     opencf = next((c for c in commitfests if c.status == CommitFest.STATUS_OPEN), None)
     inprogresscf = next(
         (c for c in commitfests if c.status == CommitFest.STATUS_INPROGRESS), None
@@ -141,7 +143,7 @@ def activity(request, cfid=None, rss=None):
         cf = None
         where = ""
 
-    sql = "SELECT ph.date, auth_user.username AS by, ph.what, p.id AS patchid, p.name, (SELECT max(commitfest_id) FROM commitfest_patchoncommitfest poc WHERE poc.patch_id=p.id) AS cfid FROM commitfest_patchhistory ph INNER JOIN commitfest_patch p ON ph.patch_id=p.id INNER JOIN auth_user on auth_user.id=ph.by_id {0} ORDER BY ph.date DESC LIMIT {1}".format(
+    sql = "SELECT ph.date, auth_user.username AS by, ph.what, p.id AS patchid, p.name FROM commitfest_patchhistory ph INNER JOIN commitfest_patch p ON ph.patch_id=p.id INNER JOIN auth_user on auth_user.id=ph.by_id {0} ORDER BY ph.date DESC LIMIT {1}".format(
         where, num
     )
 
@@ -323,7 +325,7 @@ def patchlist(request, cf, personalized=False):
                 EXISTS (
                     SELECT 1 FROM commitfest_patch_authors cpa WHERE cpa.patch_id=p.id AND cpa.user_id=%(self)s
                 ) AND (
-                    poc.commitfest_id < %(cid)s
+                    cf.status = %(status_closed)s
                 )
             THEN 'Your still open patches in a closed commitfest (you should move or close these)'
             WHEN
@@ -349,6 +351,7 @@ def patchlist(request, cf, personalized=False):
             cf.name AS cf_name,
             cf.status AS cf_status,
         """
+        whereparams["status_closed"] = CommitFest.STATUS_CLOSED
         whereparams["needs_author"] = PatchOnCommitFest.STATUS_AUTHOR
         whereparams["needs_committer"] = PatchOnCommitFest.STATUS_COMMITTER
         is_committer = bool(Committer.objects.filter(user=request.user, active=True))
@@ -446,6 +449,7 @@ def patchlist(request, cf, personalized=False):
     params = {
         "openstatuses": PatchOnCommitFest.OPEN_STATUSES,
         "cid": cf.id,
+        "parking": CommitFest.STATUS_PARKING,
     }
     params.update(whereparams)
 
@@ -457,7 +461,9 @@ def patchlist(request, cf, personalized=False):
 (poc.status=ANY(%(openstatuses)s)) AS is_open,
 (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,
 (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,
-(SELECT count(1) FROM commitfest_patchoncommitfest pcf WHERE pcf.patch_id=p.id) AS num_cfs,
+(SELECT count(1) FROM commitfest_patchoncommitfest pcf
+                 INNER JOIN commitfest_commitfest cf ON cf.id = pcf.commitfest_id
+                 WHERE pcf.patch_id=p.id AND cf.status != %(parking)s) AS num_cfs,
 
 branch.needs_rebase_since,
 branch.failing_since,
@@ -515,7 +521,13 @@ def commitfest(request, cfid):
     # Generate patch status summary.
     curs = connection.cursor()
     curs.execute(
-        "SELECT ps.status, ps.statusstring, count(*) FROM commitfest_patchoncommitfest poc INNER JOIN commitfest_patchstatus ps ON ps.status=poc.status WHERE commitfest_id=%(id)s GROUP BY ps.status ORDER BY ps.sortkey",
+        """SELECT ps.status, ps.statusstring, count(*)
+             FROM commitfest_patchoncommitfest poc
+             INNER JOIN commitfest_patchstatus ps ON ps.status=poc.status
+             INNER JOIN commitfest_commitfest cf ON cf.id=poc.commitfest_id
+             WHERE commitfest_id=%(id)s
+             GROUP BY ps.status
+             ORDER BY ps.sortkey""",
         {
             "id": cf.id,
         },
@@ -631,7 +643,7 @@ def patch(request, patchid):
     patch_commitfests = (
         PatchOnCommitFest.objects.select_related("commitfest")
         .filter(patch=patch)
-        .order_by("-commitfest__startdate")
+        .order_by("-enterdate")
         .all()
     )
     cf = patch_commitfests[0].commitfest
@@ -1027,6 +1039,7 @@ def close(request, patchid, status):
             PatchOnCommitFest.STATUS_REVIEW,
             PatchOnCommitFest.STATUS_AUTHOR,
             PatchOnCommitFest.STATUS_COMMITTER,
+            PatchOnCommitFest.STATUS_PARKED,
         ):
             # This one can be moved
             pass
@@ -1079,14 +1092,66 @@ def close(request, patchid, status):
                     "/%s/%s/" % (poc.commitfest.id, poc.patch.id)
                 )
         # Create a mapping to the new commitfest that we are bouncing
-        # this patch to.
-        newpoc = PatchOnCommitFest(
+        # this patch to. Patches may be bounced back and forth from the parking
+        # lot, so we have to handle a potential previous entry for this patch.
+        PatchOnCommitFest.objects.update_or_create(
             patch=poc.patch,
             commitfest=newcf[0],
-            status=oldstatus,
-            enterdate=datetime.now(),
+            defaults=dict(
+                status=oldstatus,
+                enterdate=datetime.now(),
+                leavedate=None,
+            ),
+        )
+    elif status == "parked":
+        # Parking has similar considerations to "next", but we're more lenient
+        # about what can be moved in.
+        if poc.status in (
+            PatchOnCommitFest.STATUS_COMMITTED,
+            PatchOnCommitFest.STATUS_NEXT,
+            PatchOnCommitFest.STATUS_PARKED,
+            PatchOnCommitFest.STATUS_REJECTED,
+        ):
+            # Can't be moved!
+            messages.error(
+                request,
+                "A patch in status {0} cannot be moved to the parking lot.".format(
+                    poc.statusstring
+                ),
+            )
+            return HttpResponseRedirect("/%s/%s/" % (poc.commitfest.id, poc.patch.id))
+        elif poc.status in (
+            PatchOnCommitFest.STATUS_AUTHOR,
+            PatchOnCommitFest.STATUS_COMMITTER,
+            PatchOnCommitFest.STATUS_RETURNED,
+            PatchOnCommitFest.STATUS_REVIEW,
+            PatchOnCommitFest.STATUS_WITHDRAWN,
+        ):
+            # This one can be moved
+            pass
+        else:
+            messages.error(request, "Invalid existing patch status")
+
+        poc.status = PatchOnCommitFest.STATUS_PARKED
+
+        # Map this patch directly to the parking lot.
+        try:
+            parking_lot = CommitFest.objects.get(pk=0)
+        except CommitFest.DoesNotExist:
+            messages.error(request, "No parking lot exists!")
+            return HttpResponseRedirect("/%s/%s/" % (poc.commitfest.id, poc.patch.id))
+
+        # Patches may be bounced back and forth from the parking lot, so we have
+        # to handle a potential previous entry for this patch.
+        PatchOnCommitFest.objects.update_or_create(
+            patch=poc.patch,
+            commitfest=parking_lot,
+            defaults=dict(
+                status=PatchOnCommitFest.STATUS_AUTHOR,
+                enterdate=datetime.now(),
+                leavedate=None,
+            ),
         )
-        newpoc.save()
     elif status == "committed":
         committer = get_object_or_404(Committer, user__username=request.GET["c"])
         if committer != poc.patch.committer:

pgcommitfest/settings.py

@@ -19,6 +19,8 @@
     }
 }
 
+# "Static" commitfests use primary key values <= 0, so this must be an unsigned
+# field type.
 DEFAULT_AUTO_FIELD = "django.db.models.AutoField"
 
 # Local time zone for this installation. Choices can be found here:

pgcommitfest/urls.py

@@ -27,7 +27,8 @@
     re_path(r"^(\d+)/new/$", views.newpatch),
     re_path(r"^patch/(\d+)/status/(review|author|committer)/$", views.status),
     re_path(
-        r"^patch/(\d+)/close/(reject|withdrawn|feedback|committed|next)/$", views.close
+        r"^patch/(\d+)/close/(reject|withdrawn|feedback|committed|next|parked)/$",
+        views.close,
     ),
     re_path(r"^patch/(\d+)/reviewer/(become|remove)/$", views.reviewer),
     re_path(r"^patch/(\d+)/committer/(become|remove)/$", views.committer),