diff --git a/media/commitfest/js/commitfest.js b/media/commitfest/js/commitfest.js index 158e572c..8b32be0f 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_draft() { + return confirm( + 'Are you sure you want to move this patch to Drafts?\n\nThis means the patch will be marked as closed in this commitfest. Its status will be reset to Waiting on Author, and it will remain in Drafts 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/forms.py b/pgcommitfest/commitfest/forms.py index c3b9a18d..cdadbd1c 100644 --- a/pgcommitfest/commitfest/forms.py +++ b/pgcommitfest/commitfest/forms.py @@ -27,7 +27,7 @@ def __init__(self, data, *args, **kwargs): self.fields["sortkey"].widget = forms.HiddenInput() - c = [(-1, "* All")] + list(PatchOnCommitFest._STATUS_CHOICES) + c = [(-1, "* All"), (-2, "* Open")] + list(PatchOnCommitFest._STATUS_CHOICES) self.fields["status"] = forms.ChoiceField(choices=c, required=False) userchoices = [(-1, "* All"), (-2, "* None"), (-3, "* Yourself")] diff --git a/pgcommitfest/commitfest/migrations/0011_patch_status_draft.py b/pgcommitfest/commitfest/migrations/0011_patch_status_draft.py new file mode 100644 index 00000000..86f4ad76 --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0011_patch_status_draft.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 Drafts"), + ], + 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 Drafts', 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_draft.py b/pgcommitfest/commitfest/migrations/0012_commitfest_status_draft.py new file mode 100644 index 00000000..a651518c --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0012_commitfest_status_draft.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_draft"), + ] + + operations = [ + migrations.AlterField( + model_name="commitfest", + name="status", + field=models.IntegerField( + choices=[ + (1, "Future"), + (2, "Open"), + (3, "In Progress"), + (4, "Closed"), + (5, "Drafts"), + ], + default=1, + ), + ), + ] diff --git a/pgcommitfest/commitfest/migrations/0013_add_drafts_cf.py b/pgcommitfest/commitfest/migrations/0013_add_drafts_cf.py new file mode 100644 index 00000000..2065bc09 --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0013_add_drafts_cf.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_draft"), + ] + + operations = [ + migrations.RunSQL(""" + INSERT INTO commitfest_commitfest (id, name, status) + VALUES (0, 'Drafts', 5); + """), + ] diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index fcd9edb9..231f726f 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_DRAFT = 5 _STATUS_CHOICES = ( (STATUS_FUTURE, "Future"), (STATUS_OPEN, "Open"), (STATUS_INPROGRESS, "In Progress"), (STATUS_CLOSED, "Closed"), + (STATUS_DRAFT, "Drafts"), ) _STATUS_LABELS = ( (STATUS_FUTURE, "default"), (STATUS_OPEN, "info"), (STATUS_INPROGRESS, "success"), (STATUS_CLOSED, "danger"), + (STATUS_DRAFT, "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_DRAFT = 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_DRAFT, "Moved to Drafts"), ) _STATUS_LABELS = ( (STATUS_REVIEW, "default"), @@ -247,6 +252,7 @@ class PatchOnCommitFest(models.Model): (STATUS_REJECTED, "danger"), (STATUS_RETURNED, "danger"), (STATUS_WITHDRAWN, "danger"), + (STATUS_DRAFT, "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..d824b84e 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 7b6dd63d..43d4de6d 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 @@ -168,7 +170,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 ) @@ -243,13 +245,26 @@ def patchlist(request, cf, personalized=False): whereclauses = [] whereparams = {} + in_drafts = cf.status == CommitFest.STATUS_DRAFT + if in_drafts and not request.GET: + # Special case: apply the Open patch filter to Drafts by default + return PatchList( + patches=[], + has_filter=False, + sortkey=0, + redirect=HttpResponseRedirect("%s?status=-2" % (request.path)), + ) + if request.GET.get("status", "-1") != "-1": - try: - whereparams["status"] = int(request.GET["status"]) - whereclauses.append("poc.status=%(status)s") - except ValueError: - # int() failed -- so just ignore this filter - pass + if request.GET["status"] == "-2": + whereclauses.append("poc.status=ANY(%(openstatuses)s)") + else: + try: + whereparams["status"] = int(request.GET["status"]) + whereclauses.append("poc.status=%(status)s") + except ValueError: + # int() failed -- so just ignore this filter + pass if request.GET.get("targetversion", "-1") != "-1": if request.GET["targetversion"] == "-2": @@ -350,7 +365,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 @@ -376,6 +391,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)) @@ -457,7 +473,7 @@ def patchlist(request, cf, personalized=False): orderby_str = "topic, created" sortkey = 0 - if not has_filter and sortkey == 0 and request.GET: + if not has_filter and not in_drafts and sortkey == 0 and request.GET: # Redirect to get rid of the ugly url return PatchList( patches=[], @@ -473,6 +489,7 @@ def patchlist(request, cf, personalized=False): params = { "openstatuses": PatchOnCommitFest.OPEN_STATUSES, "cid": cf.id, + "draft": CommitFest.STATUS_DRAFT, } params.update(whereparams) @@ -484,7 +501,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 != %(draft)s) AS num_cfs, branch.needs_rebase_since, branch.failing_since, @@ -542,7 +561,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, }, @@ -658,7 +683,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 @@ -1054,6 +1079,7 @@ def close(request, patchid, status): PatchOnCommitFest.STATUS_REVIEW, PatchOnCommitFest.STATUS_AUTHOR, PatchOnCommitFest.STATUS_COMMITTER, + PatchOnCommitFest.STATUS_DRAFT, ): # This one can be moved pass @@ -1106,14 +1132,70 @@ 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 draft, + # 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 == "draft": + # The Drafts CF 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_DRAFT, + PatchOnCommitFest.STATUS_NEXT, + PatchOnCommitFest.STATUS_REJECTED, + ): + # Can't be moved! + messages.error( + request, + "A patch in status {0} cannot be moved to Drafts.".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_DRAFT + + # Map this patch directly to the Drafts CF. + try: + drafts = CommitFest.objects.get(pk=0) + except CommitFest.DoesNotExist: + messages.error(request, "No draft commitfest exists!") + return HttpResponseRedirect("/%s/%s/" % (poc.commitfest.id, poc.patch.id)) + + if poc.commitfest == drafts: + messages.error(request, "Patch is already in Drafts.") + return HttpResponseRedirect("/%s/%s/" % (poc.commitfest.id, poc.patch.id)) + + # Patches may be bounced back and forth from draft, so we have + # to handle a potential previous entry for this patch. + PatchOnCommitFest.objects.update_or_create( + patch=poc.patch, + commitfest=drafts, + 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..b477018e 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|draft)/$", + views.close, ), re_path(r"^patch/(\d+)/reviewer/(become|remove)/$", views.reviewer), re_path(r"^patch/(\d+)/committer/(become|remove)/$", views.committer),