diff --git a/media/commitfest/js/commitfest.js b/media/commitfest/js/commitfest.js index 77d49fc2..eb45abf8 100644 --- a/media/commitfest/js/commitfest.js +++ b/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"); diff --git a/pgcommitfest/commitfest/feeds.py b/pgcommitfest/commitfest/feeds.py index 9aff9025..38aee7a0 100644 --- a/pgcommitfest/commitfest/feeds.py +++ b/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"] diff --git a/pgcommitfest/commitfest/migrations/0011_patch_status_parked.py b/pgcommitfest/commitfest/migrations/0011_patch_status_parked.py new file mode 100644 index 00000000..a66ca1c4 --- /dev/null +++ b/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" + ), + ] diff --git a/pgcommitfest/commitfest/migrations/0012_commitfest_status_parking.py b/pgcommitfest/commitfest/migrations/0012_commitfest_status_parking.py new file mode 100644 index 00000000..9af3c813 --- /dev/null +++ b/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, + ), + ), + ] diff --git a/pgcommitfest/commitfest/migrations/0013_add_parking_lot.py b/pgcommitfest/commitfest/migrations/0013_add_parking_lot.py new file mode 100644 index 00000000..89e74bfd --- /dev/null +++ b/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); + """), + ] diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index fcd9edb9..389cb8e4 100644 --- a/pgcommitfest/commitfest/models.py +++ b/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] diff --git a/pgcommitfest/commitfest/templates/activity.html b/pgcommitfest/commitfest/templates/activity.html index 621155ee..13a9d428 100644 --- a/pgcommitfest/commitfest/templates/activity.html +++ b/pgcommitfest/commitfest/templates/activity.html @@ -16,7 +16,7 @@ {{a.date}} {{a.by}} - {{a.name}} + {{a.name}} {{a.what}} {%endfor%} diff --git a/pgcommitfest/commitfest/templates/home.html b/pgcommitfest/commitfest/templates/home.html index a3f26da0..0cb27c9a 100644 --- a/pgcommitfest/commitfest/templates/home.html +++ b/pgcommitfest/commitfest/templates/home.html @@ -24,6 +24,7 @@

Commands

List of commitfests

diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 471f3225..ad570a76 100644 --- a/pgcommitfest/commitfest/views.py +++ b/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: diff --git a/pgcommitfest/settings.py b/pgcommitfest/settings.py index 9b867b71..99b1b3c0 100644 --- a/pgcommitfest/settings.py +++ b/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: diff --git a/pgcommitfest/urls.py b/pgcommitfest/urls.py index a67f55fc..5f8c6e61 100644 --- a/pgcommitfest/urls.py +++ b/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),