From 90792d1bb4a8da73eaa277f9ceaef9e4054dcc23 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 9 Jun 2025 12:04:04 +0200 Subject: [PATCH 1/8] Introduce Drafts page and automatic CF creation This introduce a new type of CommitFest a "Draft" CommitFest. This CommitFest is never "In Progress", but it can be open. It exists for a year. It opens when the final regular CommitFest of the year becomes "In Progress" and stays open for exactly a year. It never becomes "In Progress" itself. Adding a second type of CommitFest also needed a redesign of quite a few things, like the homepage. Also management of the CommitFests needed to be made a bit easier, so admins don't forget to close/create Draft CommitFests. So now, closing/opening/creating CommitFests is done automatically when the time is right for that. A help page is also introduced to explain the CommitFest app. The naming of CommitFests has been changed too. Since we now have a Draft CF every year that needs a name, it seemed reasonable to align the names of the other CFs with that too. So each PG release cycle now has 5 regular commitfests that are called: - PG18-1 - PG18-2 - PG18-3 - PG18-4 - PG18-Final And a single Draft CommitFest, called: - PG18-Draft Finally, it also adds a small initial API endpoint for the CFBot, to request the commitfests that need CI runs. Future PRs will extend this API surface to also include/allow requesting the actual patches that CI should run on. Co-Authored-By: David G. Johnston --- pgcommitfest/commitfest/apiv1.py | 46 +++ .../commitfest/fixtures/auth_data.json | 36 +++ .../commitfest/fixtures/commitfest_data.json | 91 +++++- .../0011_add_draft_remove_future.py | 51 ++++ .../0012_add_status_related_constraints.py | 68 +++++ pgcommitfest/commitfest/models.py | 285 ++++++++++++++++-- .../commitfest/templates/all_commitfests.html | 10 + pgcommitfest/commitfest/templates/help.html | 51 ++++ pgcommitfest/commitfest/templates/home.html | 89 +++++- pgcommitfest/commitfest/templates/me.html | 9 +- .../commitfest/templates/patch_commands.inc | 22 +- .../commitfest/templatetags/commitfest.py | 1 + pgcommitfest/commitfest/views.py | 213 ++++++------- pgcommitfest/urls.py | 9 +- 14 files changed, 809 insertions(+), 172 deletions(-) create mode 100644 pgcommitfest/commitfest/apiv1.py create mode 100644 pgcommitfest/commitfest/migrations/0011_add_draft_remove_future.py create mode 100644 pgcommitfest/commitfest/migrations/0012_add_status_related_constraints.py create mode 100644 pgcommitfest/commitfest/templates/all_commitfests.html create mode 100644 pgcommitfest/commitfest/templates/help.html diff --git a/pgcommitfest/commitfest/apiv1.py b/pgcommitfest/commitfest/apiv1.py new file mode 100644 index 00000000..a90ba4d3 --- /dev/null +++ b/pgcommitfest/commitfest/apiv1.py @@ -0,0 +1,46 @@ +from django.http import ( + HttpResponse, +) + +import json +from datetime import date, datetime, timedelta, timezone + +from .models import ( + CommitFest, +) + + +def datetime_serializer(obj): + if isinstance(obj, date): + return obj.isoformat() + + if isinstance(obj, datetime): + return obj.replace(tzinfo=timezone.utc).isoformat() + + if hasattr(obj, "to_json"): + return obj.to_json() + + raise TypeError(f"Type {type(obj)} not serializable to JSON") + + +def api_response(payload, status=200, content_type="application/json"): + response = HttpResponse( + json.dumps(payload, default=datetime_serializer), status=status + ) + response["Content-Type"] = content_type + response["Access-Control-Allow-Origin"] = "*" + return response + + +def commitfestst_that_need_ci(request): + cfs = CommitFest.relevant_commitfests() + + # We continue to run CI on the previous commitfest for a week after it ends + # to give people some time to move patches over to the next one. + if cfs["previous"].enddate <= datetime.now(timezone.utc).date() - timedelta(days=7): + del cfs["previous"] + + del cfs["next_open"] + del cfs["final"] + + return api_response({"commitfests": cfs}) diff --git a/pgcommitfest/commitfest/fixtures/auth_data.json b/pgcommitfest/commitfest/fixtures/auth_data.json index 88d8c708..9a6e2f03 100644 --- a/pgcommitfest/commitfest/fixtures/auth_data.json +++ b/pgcommitfest/commitfest/fixtures/auth_data.json @@ -88,5 +88,41 @@ "groups": [], "user_permissions": [] } +}, +{ + "model": "auth.user", + "pk": 6, + "fields": { + "password": "", + "last_login": null, + "is_superuser": false, + "username": "prolific-author", + "first_name": "Prolific", + "last_name": "Author", + "email": "", + "is_staff": false, + "is_active": true, + "date_joined": "2025-01-01T00:00:00", + "groups": [], + "user_permissions": [] + } +}, +{ + "model": "auth.user", + "pk": 7, + "fields": { + "password": "", + "last_login": null, + "is_superuser": false, + "username": "prolific-reviewer", + "first_name": "Prolific", + "last_name": "Reviewer", + "email": "", + "is_staff": false, + "is_active": true, + "date_joined": "2025-01-01T00:00:00", + "groups": [], + "user_permissions": [] + } } ] diff --git a/pgcommitfest/commitfest/fixtures/commitfest_data.json b/pgcommitfest/commitfest/fixtures/commitfest_data.json index 6e5b32ff..d51214dc 100644 --- a/pgcommitfest/commitfest/fixtures/commitfest_data.json +++ b/pgcommitfest/commitfest/fixtures/commitfest_data.json @@ -24,40 +24,44 @@ "model": "commitfest.commitfest", "pk": 1, "fields": { - "name": "Sample Old Commitfest", + "name": "PG18-3", "status": 4, - "startdate": "2024-05-01", - "enddate": "2024-05-31" + "startdate": "2024-11-01", + "enddate": "2024-11-30", + "draft": false } }, { "model": "commitfest.commitfest", "pk": 2, "fields": { - "name": "Sample In Progress Commitfest", + "name": "PG18-4", "status": 3, "startdate": "2025-01-01", - "enddate": "2025-02-28" + "enddate": "2025-01-31", + "draft": false } }, { "model": "commitfest.commitfest", "pk": 3, "fields": { - "name": "Sample Open Commitfest", + "name": "PG18-Final", "status": 2, "startdate": "2025-03-01", - "enddate": "2025-03-31" + "enddate": "2025-03-31", + "draft": false } }, { "model": "commitfest.commitfest", "pk": 4, "fields": { - "name": "Sample Future Commitfest", - "status": 1, - "startdate": "2025-05-01", - "enddate": "2025-05-31" + "name": "PG18-Drafts", + "status": 2, + "startdate": "2024-03-01", + "enddate": "2025-02-28", + "draft": true } }, { @@ -237,6 +241,33 @@ ] } }, +{ + "model": "commitfest.patch", + "pk": 8, + "fields": { + "name": "Test DGJ Multi-Author and Reviewer", + "topic": 3, + "wikilink": "", + "gitlink": "", + "targetversion": 1, + "committer": 4, + "created": "2025-02-01T00:00:00", + "modified": "2025-02-01T00:00:00", + "lastmail": "2025-02-01T00:00:00", + "authors": [ + 3, + 6 + ], + "reviewers": [ + 1, + 7 + ], + "subscribers": [], + "mailthread_set": [ + 8 + ] + } +}, { "model": "commitfest.patchoncommitfest", "pk": 1, @@ -325,6 +356,17 @@ "status": 1 } }, +{ + "model": "commitfest.patchoncommitfest", + "pk": 9, + "fields": { + "patch": 8, + "commitfest": 4, + "enterdate": "2025-02-01T00:00:00", + "leavedate": null, + "status": 1 + } +}, { "model": "commitfest.patchhistory", "pk": 1, @@ -632,6 +674,33 @@ "latestmsgid": "example@message-31" } }, +{ + "model": "commitfest.mailthread", + "pk": 8, + "fields": { + "messageid": "dgj-example@message-08", + "subject": "Test DGJ Multi-Author and Reviewer", + "firstmessage": "2025-02-01T00:00:00", + "firstauthor": "test@test.com", + "latestmessage": "2025-02-01T00:00:00", + "latestauthor": "test@test.com", + "latestsubject": "Test DGJ Multi-Author and Reviewer", + "latestmsgid": "dgj-example@message-08" + } +}, +{ + "model": "commitfest.mailthreadattachment", + "pk": 8, + "fields": { + "mailthread": 8, + "messageid": "dgj-example@message-08", + "attachmentid": 1, + "filename": "v1-0001-content.patch", + "date": "2025-02-01T00:00:00", + "author": "test@test.com", + "ispatch": true + } +}, { "model": "commitfest.patchstatus", "pk": 1, diff --git a/pgcommitfest/commitfest/migrations/0011_add_draft_remove_future.py b/pgcommitfest/commitfest/migrations/0011_add_draft_remove_future.py new file mode 100644 index 00000000..740aa175 --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0011_add_draft_remove_future.py @@ -0,0 +1,51 @@ +# Generated by Django 4.2.19 on 2025-06-08 10:47 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("commitfest", "0010_add_failing_since_column"), + ] + + operations = [ + migrations.AddField( + model_name="commitfest", + name="draft", + field=models.BooleanField(default=False), + ), + migrations.AlterField( + model_name="commitfest", + name="status", + field=models.IntegerField( + choices=[(2, "Open"), (3, "In Progress"), (4, "Closed")], default=2 + ), + ), + migrations.AlterField( + model_name="commitfest", + name="enddate", + field=models.DateField(), + ), + migrations.AlterField( + model_name="commitfest", + name="startdate", + field=models.DateField(), + ), + 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 different CF"), + (6, "Rejected"), + (7, "Returned with feedback"), + (8, "Withdrawn"), + ], + default=1, + ), + ), + ] diff --git a/pgcommitfest/commitfest/migrations/0012_add_status_related_constraints.py b/pgcommitfest/commitfest/migrations/0012_add_status_related_constraints.py new file mode 100644 index 00000000..4340c5a1 --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0012_add_status_related_constraints.py @@ -0,0 +1,68 @@ +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("commitfest", "0011_add_draft_remove_future"), + ] + operations = [ + migrations.RunSQL( + """ +CREATE UNIQUE INDEX cf_enforce_maxoneopen_idx +ON commitfest_commitfest (status, draft) +WHERE status not in (1,4); +""", + reverse_sql=""" +DROP INDEX IF EXISTS cf_enforce_maxoneopen_idx; +""", + ), + migrations.RunSQL( + """ +CREATE UNIQUE INDEX poc_enforce_maxoneoutcome_idx +ON commitfest_patchoncommitfest (patch_id) +WHERE status not in (5); +""", + reverse_sql=""" +DROP INDEX IF EXISTS poc_enforce_maxoneoutcome_idx; +""", + ), + migrations.RunSQL( + """ +ALTER TABLE commitfest_patchoncommitfest +ADD CONSTRAINT status_and_leavedate_correlation +CHECK ((status IN (4,5,6,7,8)) = (leavedate IS NOT NULL)); +""", + reverse_sql=""" +ALTER TABLE commitfest_patchoncommitfest +DROP CONSTRAINT IF EXISTS status_and_leavedate_correlation; +""", + ), + migrations.RunSQL( + """ +COMMENT ON COLUMN commitfest_patchoncommitfest.leavedate IS +$$A leave date is recorded in two situations, both of which +means this particular patch-cf combination became inactive +on the corresponding date. For status 5 the patch was moved +to some other cf. For 4,6,7, and 8, this was the final cf. +$$ +""", + reverse_sql=""" +COMMENT ON COLUMN commitfest_patchoncommitfest.leavedate IS NULL; +""", + ), + migrations.RunSQL( + """ +COMMENT ON TABLE commitfest_patchoncommitfest IS +$$This is a re-entrant table: patches may become associated +with a given cf multiple times, resetting the entrydate and clearing +the leavedate each time. Non-final statuses never have a leavedate +while final statuses always do. The final status of 5 (moved) is +special in that all but one of the rows a patch has in this table +must have it as the status. +$$ +""", + reverse_sql=""" +COMMENT ON TABLE commitfest_patchoncommitfest IS NULL; +""", + ), + ] diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index fcd9edb9..f67ca1c5 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -1,8 +1,9 @@ from django.contrib.auth.models import User -from django.db import models +from django.db import models, transaction +from django.db.models import Q from django.shortcuts import get_object_or_404 -from datetime import datetime +from datetime import datetime, timedelta, timezone from pgcommitfest.userprofile.models import UserProfile @@ -34,28 +35,26 @@ class Meta: class CommitFest(models.Model): - STATUS_FUTURE = 1 STATUS_OPEN = 2 STATUS_INPROGRESS = 3 STATUS_CLOSED = 4 _STATUS_CHOICES = ( - (STATUS_FUTURE, "Future"), (STATUS_OPEN, "Open"), (STATUS_INPROGRESS, "In Progress"), (STATUS_CLOSED, "Closed"), ) _STATUS_LABELS = ( - (STATUS_FUTURE, "default"), (STATUS_OPEN, "info"), (STATUS_INPROGRESS, "success"), (STATUS_CLOSED, "danger"), ) name = models.CharField(max_length=100, blank=False, null=False, unique=True) status = models.IntegerField( - null=False, blank=False, default=1, choices=_STATUS_CHOICES + null=False, blank=False, default=2, choices=_STATUS_CHOICES ) - startdate = models.DateField(blank=True, null=True) - enddate = models.DateField(blank=True, null=True) + startdate = models.DateField(blank=False, null=False) + enddate = models.DateField(blank=False, null=False) + draft = models.BooleanField(blank=False, null=False, default=False) @property def statusstring(self): @@ -63,18 +62,195 @@ def statusstring(self): @property def periodstring(self): - if self.startdate and self.enddate: - return "{0} - {1}".format(self.startdate, self.enddate) - return "" + return "{0} - {1}".format(self.startdate, self.enddate) + + @property + def dev_cycle(self) -> int: + if self.startdate.month in [1, 3]: + return self.startdate.year - 2007 + else: + return self.startdate.year - 2006 @property def title(self): return "Commitfest %s" % self.name @property - def isopen(self): + def is_closed(self): + return self.status == self.STATUS_CLOSED + + @property + def is_open(self): return self.status == self.STATUS_OPEN + @property + def is_open_regular(self): + return self.is_open and not self.draft + + @property + def is_open_draft(self): + return self.is_open and self.draft + + @property + def is_in_progress(self): + return self.status == self.STATUS_INPROGRESS + + def to_json(self): + return { + "id": self.id, + "name": self.name, + "status": self.statusstring, + "startdate": self.startdate.isoformat(), + "enddate": self.enddate.isoformat(), + } + + @staticmethod + def _are_relevant_commitfests_up_to_date(cfs, current_date): + inprogress_cf = cfs["in_progress"] + + if inprogress_cf and inprogress_cf.enddate < current_date: + return False + + if cfs["open"].startdate <= current_date: + return False + + if not cfs["draft"] or cfs["draft"].enddate < current_date: + return False + + return True + + @classmethod + def _refresh_relevant_commitfests(cls, for_update): + cfs = CommitFest.relevant_commitfests(for_update=for_update, refresh=False) + current_date = datetime.now(timezone.utc).date() + + if cls._are_relevant_commitfests_up_to_date(cfs, current_date): + return cfs + + with transaction.atomic(): + cfs = CommitFest.relevant_commitfests(for_update=True, refresh=False) + if cls._are_relevant_commitfests_up_to_date(cfs, current_date): + # Some other request has already updated the commitfests, so we + # return the new version + return cfs + + inprogress_cf = cfs["in_progress"] + if inprogress_cf and inprogress_cf.enddate < current_date: + inprogress_cf.status = CommitFest.STATUS_CLOSED + inprogress_cf.save() + + open_cf = cfs["open"] + + if open_cf.startdate <= current_date: + if open_cf.enddate < current_date: + open_cf.status = CommitFest.STATUS_CLOSED + else: + open_cf.status = CommitFest.STATUS_INPROGRESS + open_cf.save() + + cls.next_open_cf(current_date).save() + + draft_cf = cfs["draft"] + if not draft_cf: + cls.next_draft_cf(current_date).save() + elif draft_cf.enddate < current_date: + # If the draft commitfest has started, we need to update it + draft_cf.status = CommitFest.STATUS_CLOSED + draft_cf.save() + cls.next_draft_cf(current_date).save() + + return cls.relevant_commitfests(for_update=for_update) + + @classmethod + def relevant_commitfests(cls, for_update=False, refresh=True): + if refresh: + return cls._refresh_relevant_commitfests(for_update=for_update) + + query_base = CommitFest.objects.order_by("-enddate") + if for_update: + query_base = query_base.select_for_update(no_key=True) + last_three_commitfests = query_base.filter(draft=False)[:3] + + cfs = {} + cfs["open"] = last_three_commitfests[0] + + if last_three_commitfests[1].status == CommitFest.STATUS_INPROGRESS: + cfs["in_progress"] = last_three_commitfests[1] + cfs["previous"] = last_three_commitfests[2] + + else: + cfs["in_progress"] = None + cfs["previous"] = last_three_commitfests[1] + if cfs["open"].startdate.month == 3: + cfs["final"] = cfs["open"] + + if cfs["in_progress"] and cfs["in_progress"].startdate.month == 3: + cfs["final"] = cfs["in_progress"] + elif cfs["open"].startdate.month == 3: + cfs["final"] = cfs["open"] + else: + cfs["final"] = cls.next_open_cf( + datetime(year=cfs["open"].dev_cycle + 2007, month=2, day=1) + ) + + cfs["draft"] = query_base.filter(draft=True).order_by("-startdate").first() + cfs["next_open"] = cls.next_open_cf(cfs["open"].enddate + timedelta(days=1)) + + return cfs + + @staticmethod + def next_open_cf(from_date): + # We don't have a CF in december, so we don't need to worry about 12 mod 12 being 0 + cf_months = [7, 9, 11, 1, 3] + next_open_cf_month = min( + (month for month in cf_months if month > from_date.month), default=1 + ) + next_open_cf_year = from_date.year + if next_open_cf_month == 1: + next_open_cf_year += 1 + + next_open_dev_cycle = next_open_cf_year - 2006 + if next_open_cf_month in [1, 3]: + next_open_dev_cycle -= 1 + + if next_open_cf_month == 3: + name = f"PG{next_open_dev_cycle}-Final" + else: + cf_number = cf_months.index(next_open_cf_month) + 1 + name = f"PG{next_open_dev_cycle}-{cf_number}" + start_date = datetime( + year=next_open_cf_year, month=next_open_cf_month, day=1 + ).date() + end_date = datetime( + year=next_open_cf_year, month=next_open_cf_month + 1, day=1 + ).date() - timedelta(days=1) + + return CommitFest( + name=name, + status=CommitFest.STATUS_OPEN, + startdate=start_date, + enddate=end_date, + ) + + @staticmethod + def next_draft_cf(start_date): + dev_cycle = start_date.year - 2006 + if start_date.month < 3: + dev_cycle -= 1 + + end_year = dev_cycle + 2007 + + name = f"PG{dev_cycle}-Drafts" + end_date = datetime(year=end_year, month=3, day=1).date() - timedelta(days=1) + + return CommitFest( + name=name, + status=CommitFest.STATUS_OPEN, + startdate=start_date, + enddate=end_date, + draft=True, + ) + def __str__(self): return self.name @@ -102,6 +278,10 @@ def __str__(self): return self.version +class UserInputError(ValueError): + pass + + class Patch(models.Model, DiffableModel): name = models.CharField( max_length=500, blank=False, null=False, verbose_name="Description" @@ -159,11 +339,15 @@ class Patch(models.Model, DiffableModel): } def current_commitfest(self): - return self.commitfests.order_by("-startdate").first() + return self.current_patch_on_commitfest().commitfest def current_patch_on_commitfest(self): - cf = self.current_commitfest() - return get_object_or_404(PatchOnCommitFest, patch=self, commitfest=cf) + # The unique partial index poc_enforce_maxoneoutcome_idx stores the PoC + # No caching here (inside the instance) since the caller should just need + # the PoC once per request. + return get_object_or_404( + PatchOnCommitFest, Q(patch=self) & ~Q(status=PatchOnCommitFest.STATUS_MOVED) + ) # Some accessors @property @@ -208,6 +392,43 @@ def update_lastmail(self): else: self.lastmail = max(threads, key=lambda t: t.latestmessage).latestmessage + def move(self, from_cf, to_cf): + current_poc = self.current_patch_on_commitfest() + if from_cf.id != current_poc.commitfest.id: + raise UserInputError("Patch not in source commitfest.") + + if from_cf.id == to_cf.id: + raise UserInputError("Source and target commitfest are the same.") + + if current_poc.status not in ( + PatchOnCommitFest.STATUS_REVIEW, + PatchOnCommitFest.STATUS_AUTHOR, + PatchOnCommitFest.STATUS_COMMITTER, + ): + raise UserInputError( + f"Patch in state {current_poc.statusstring} cannot be moved." + ) + + if not to_cf.is_open: + raise UserInputError("Patch can only be moved to an open commitfest") + + old_status = current_poc.status + + current_poc.set_status(PatchOnCommitFest.STATUS_MOVED) + + new_poc, _ = PatchOnCommitFest.objects.update_or_create( + patch=current_poc.patch, + commitfest=to_cf, + defaults=dict( + status=old_status, + enterdate=datetime.now(), + leavedate=None, + ), + ) + new_poc.save() + self.set_modified() + self.save() + def __str__(self): return self.name @@ -224,7 +445,7 @@ class PatchOnCommitFest(models.Model): STATUS_AUTHOR = 2 STATUS_COMMITTER = 3 STATUS_COMMITTED = 4 - STATUS_NEXT = 5 + STATUS_MOVED = 5 STATUS_REJECTED = 6 STATUS_RETURNED = 7 STATUS_WITHDRAWN = 8 @@ -233,7 +454,7 @@ class PatchOnCommitFest(models.Model): (STATUS_AUTHOR, "Waiting on Author"), (STATUS_COMMITTER, "Ready for Committer"), (STATUS_COMMITTED, "Committed"), - (STATUS_NEXT, "Moved to next CF"), + (STATUS_MOVED, "Moved to different CF"), (STATUS_REJECTED, "Rejected"), (STATUS_RETURNED, "Returned with feedback"), (STATUS_WITHDRAWN, "Withdrawn"), @@ -243,7 +464,7 @@ class PatchOnCommitFest(models.Model): (STATUS_AUTHOR, "primary"), (STATUS_COMMITTER, "info"), (STATUS_COMMITTED, "success"), - (STATUS_NEXT, "warning"), + (STATUS_MOVED, "warning"), (STATUS_REJECTED, "danger"), (STATUS_RETURNED, "danger"), (STATUS_WITHDRAWN, "danger"), @@ -273,10 +494,38 @@ def is_closed(self): def is_open(self): return not self.is_closed + @property + def is_committed(self): + return self.status == self.STATUS_COMMITTED + + @property + def needs_committer(self): + return self.status == self.STATUS_COMMITTER + @property def statusstring(self): return [v for k, v in self._STATUS_CHOICES if k == self.status][0] + @classmethod + def current_for_patch(cls, patch_id): + return get_object_or_404( + cls, Q(patch_id=patch_id) & ~Q(status=cls.STATUS_MOVED) + ) + + def set_status(self, status): + self.status = status + if not self.leavedate and not self.is_open: + # If the patch was not closed before, we need to set the leavedate + # now. + self.leavedate = datetime.now() + elif self.is_open: + self.leavedate = None + + self.patch.set_modified() + + self.patch.save() + self.save() + class Meta: unique_together = ( ( diff --git a/pgcommitfest/commitfest/templates/all_commitfests.html b/pgcommitfest/commitfest/templates/all_commitfests.html new file mode 100644 index 00000000..6d983df1 --- /dev/null +++ b/pgcommitfest/commitfest/templates/all_commitfests.html @@ -0,0 +1,10 @@ +{%extends "base.html"%} +{%block contents%} + +
+{%endblock%} + diff --git a/pgcommitfest/commitfest/templates/help.html b/pgcommitfest/commitfest/templates/help.html new file mode 100644 index 00000000..c6234366 --- /dev/null +++ b/pgcommitfest/commitfest/templates/help.html @@ -0,0 +1,51 @@ +{%extends "base.html"%} +{%block contents%} +

+ This is the "CommitFest app", the website where the PostgreSQL community tracks proposed changes to PostgreSQL. If you're familiar with GitHub, then this website fulfills a similar purpose to the list of Pull Requests (PRs) on GitHub repo. The most important difference is that the CommitFest app is not the "source of truth", instead the PostgreSQL mailinglists are. The "CommitFest app" is simply a tool to help the communtiy keep track of changes proposed on the mailinglist in an organized manner. Below are the most important concepts that you should know about when using the CommitFest app. +

+

CommitFest

+

+ PostgreSQL development is organized into "CommitFests" (often abbreviated to "CF"). Each CommitFest contains a list of entries called patches (similar to PRs, see below for details). The main purpose of CommitFests are to make sure patches that people are interested in are not forgotten about, as well as running CI (aka CFBot) on these patches. Each CommitFest has a period where it is "Open", in which people can add patches to the CommitFest. This "Open" period is followed by an "In Progress" period, in which the idea is that committers and reviewers focus on reviewing and committing the patches in this "In Progress" CommitFest. At the end of the month a CommitFest gets "Closed" (and stays closed forever). Any not yet committed patches can be moved to the following "Open" CommitFest by their authors, to try again in the next cycle. Having these timebound periods has several benefits: +

    +
  1. It gives a regular cadence of development to people who like to have this.
    2. +
  2. It provides a natural age-out mechanism for patches that the submitter has lost interest in.
    3. +
  3. It provides an easy way for reviewers/committers to prioritize which patches to review.
    4. +
+ This does not mean that patches are only committed during a CommitFest, nor that people will only respond on the mainlinglist to patches in the "In Progress" CommitFest. +

+

There are 5 CommitFests per year. The first one is "In Progress" in July and starts the nine months feature development cycle of PostgreSQL. The next three are "In Progress" in September, November and January. The last CommitFest of the feature development cycle is "In Progress" in March, and ends a when the feature freeze starts. The exact date of the feature freeze depends on the year, but it's usually in early April.

+ +

Patches

+

+ A "patch" is a bit of an overloaded term in the PostgreSQL community. Email threads on the mailing list often contain "patch files" as attachments, such a file is often referred to as a "patch". A single email can even contain multiple related "patch files", which are called a "patchset". However, in the context of a CommitFest app a "patch" usually means a "patch entry" in the CommitFest app. Such a "patch entry" is a reference to a mailinglist thread on which change to PostgreSQL has been proposed, by someone sending an email that contain one or more "patch files". The CommitFest app will automatically detect new versions of the patch files and update the "patch entry" accordingly. +

+

+ There are three active categories of patch status: +

+ And there are three preferred inactive categories of patch status for when + a patch has been resolved, either by being committed or not: + +

+ +

Drafts

+

+ Appart from the regular CommitFests, there is also a "Drafts" CommitFest that is used to collect patches for new features that are not yet ready for general reviewing. There are various reasons why a patch might not be ready for general reviewing but the author still wants to track it publicly in the "CommitFest app", this is usually due to the combination of one of the following reasons: +

+ Like regular CommitFests, a Draft CommitFest also has an "Open" period and a "Closed" state, but it has no "In Progress" period. The "Open" period for a Draft CommitFest last lasts a year. When the last CommitFest of the development cycle becomes "In Progress", the Draft CommitFest for that PostgreSQL version is closed, and a new one is immediately opened for the next PostgreSQL release. +

+

Another difference between Draft CommitFests and regular CommitFests is that Draft CommitFests don't list resolved patches.

+{%endblock%} diff --git a/pgcommitfest/commitfest/templates/home.html b/pgcommitfest/commitfest/templates/home.html index a3f26da0..0933fe1d 100644 --- a/pgcommitfest/commitfest/templates/home.html +++ b/pgcommitfest/commitfest/templates/home.html @@ -1,11 +1,82 @@ {%extends "base.html"%} {%block contents%}

- {%if inprogresscf%}A commitfest is currently in progress: {{inprogresscf}}.{%endif%} + First time user? Here is a help page for you to understand how this website works.

- Useful links that you can use and bookmark: + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
DetailsIn Progress
Open:{{cfs.open}}{{cfs.open.periodstring}}
In Progress: + {%if cfs.in_progress %} + {{cfs.in_progress}} + {%else%} + None in progress + {%endif%} + + {%if cfs.in_progress %} + {{cfs.in_progress.periodstring}} + {%endif%} +
Previous:{{cfs.previous}}{{cfs.previous.periodstring}}
Draft:{{cfs.draft}}{{cfs.draft.periodstring}}
Next open:{{cfs.next_open}}{{cfs.next_open.periodstring}}
Final of this release: + {%if cfs.final.id %} + {{cfs.final}} + {%else%} + {{cfs.final}} + {%endif%} + + {{cfs.final.periodstring}} +

+

Search

+
+
+ +
+ +
+

Useful pages (bookmarkable)

-

Commands

-
-
- -
- -
-

List of commitfests

-
{%endblock%} diff --git a/pgcommitfest/commitfest/templates/me.html b/pgcommitfest/commitfest/templates/me.html index b708d4de..b6736124 100644 --- a/pgcommitfest/commitfest/templates/me.html +++ b/pgcommitfest/commitfest/templates/me.html @@ -2,8 +2,13 @@ {%load commitfest %} {%block contents%} New patch - Current commitfest - Open commitfest + {%if cfs.in_progress%} + In Progress commitfest + {%endif%} + Open commitfest + {%if cfs.draft%} + Draft commitfest + {%endif%}
diff --git a/pgcommitfest/commitfest/templates/patch_commands.inc b/pgcommitfest/commitfest/templates/patch_commands.inc index 8f09b18c..37fb2436 100644 --- a/pgcommitfest/commitfest/templates/patch_commands.inc +++ b/pgcommitfest/commitfest/templates/patch_commands.inc @@ -21,8 +21,28 @@
  • Rejected
  • Withdrawn
  • Returned with feedback
    • -
  • Move to next CF
  • Committed
    • + + + {%if not cf.is_open_regular %} +
  • + + Next CF: {{cfs.open.name}} +
    • + {%endif%} + {%if not cf.is_open_draft %} +
  • + + {%if cf.draft %} + Next Drafts: + {%else%} + Draft: + {%endif%} + {{cfs.draft.name}} +
    • + {%endif%}
    diff --git a/pgcommitfest/commitfest/templatetags/commitfest.py b/pgcommitfest/commitfest/templatetags/commitfest.py index 25fd21f2..f08c760a 100644 --- a/pgcommitfest/commitfest/templatetags/commitfest.py +++ b/pgcommitfest/commitfest/templatetags/commitfest.py @@ -24,6 +24,7 @@ def commitfeststatusstring(value): @stringfilter def commitfeststatuslabel(value): i = int(value) + print(i, CommitFest._STATUS_LABELS) return [v for k, v in CommitFest._STATUS_LABELS if k == i][0] diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 7b6dd63d..45c5be83 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -42,23 +42,18 @@ Patch, PatchHistory, PatchOnCommitFest, + UserInputError, ) def home(request): - commitfests = list(CommitFest.objects.all()) - 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 - ) + cfs = CommitFest.relevant_commitfests() return render( request, "home.html", { - "commitfests": commitfests, - "opencf": opencf, - "inprogresscf": inprogresscf, + "cfs": cfs, "title": "Commitfests", "header_activity": "Activity log", "header_activity_link": "/activity/", @@ -66,6 +61,31 @@ def home(request): ) +def commitfest_history(request): + cfs = list(CommitFest.objects.order_by("-enddate")) + + return render( + request, + "all_commitfests.html", + { + "commitfests": cfs, + "title": "Commitfest history", + "header_activity": "Activity log", + "header_activity_link": "/activity/", + }, + ) + + +def help(request): + return render( + request, + "help.html", + { + "title": "What is the CommitFest app?", + }, + ) + + @login_required def me(request): cfs = list(CommitFest.objects.filter(status=CommitFest.STATUS_INPROGRESS)) @@ -128,6 +148,7 @@ def me(request): "header_activity": "Activity log", "header_activity_link": "/activity/", "userprofile": getattr(request.user, "userprofile", UserProfile()), + "cfs": CommitFest.relevant_commitfests(), }, ) @@ -658,7 +679,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 @@ -708,6 +729,7 @@ def patch(request, patchid): {"title": cf.title, "href": "/%s/" % cf.pk}, ], "userprofile": getattr(request.user, "userprofile", UserProfile()), + "cfs": CommitFest.relevant_commitfests(), }, ) @@ -972,22 +994,16 @@ def status(request, patchid, status): patch__id=patchid, ) - if status == "review": - newstatus = PatchOnCommitFest.STATUS_REVIEW - elif status == "author": - newstatus = PatchOnCommitFest.STATUS_AUTHOR - elif status == "committer": - newstatus = PatchOnCommitFest.STATUS_COMMITTER - else: - raise Exception("Can't happen") + status_mapping = { + "review": PatchOnCommitFest.STATUS_REVIEW, + "author": PatchOnCommitFest.STATUS_AUTHOR, + "committer": PatchOnCommitFest.STATUS_COMMITTER, + } - if newstatus != poc.status: - # Only save it if something actually changed - poc.status = newstatus - poc.patch.set_modified() - poc.patch.save() - poc.save() + new_status = status_mapping[status] + if new_status != poc.status: + poc.set_status(new_status) PatchHistory( patch=poc.patch, by=request.user, what="New status: %s" % poc.statusstring ).save_and_notify() @@ -998,6 +1014,9 @@ def status(request, patchid, status): @login_required @transaction.atomic def close(request, patchid, status): + if status == "next": + raise Exception("Can't happen, use transition/ endpoint") + patch = get_object_or_404(Patch.objects.select_related(), pk=patchid) cf = patch.current_commitfest() @@ -1015,7 +1034,7 @@ def close(request, patchid, status): request, "The patch was moved to a new commitfest by someone else. Please double check if you still want to retry this operation.", ) - return HttpResponseRedirect("/%s/%s/" % (cf.id, patch.id)) + return HttpResponseRedirect(f"/patch/{patch.id}/") poc = get_object_or_404( PatchOnCommitFest.objects.select_related(), @@ -1023,98 +1042,7 @@ def close(request, patchid, status): patch__id=patchid, ) - poc.leavedate = datetime.now() - - # We know the status can't be one of the ones below, since we - # have checked that we're not closed yet. Therefor, we don't - # need to check if the individual status has changed. - if status == "reject": - poc.status = PatchOnCommitFest.STATUS_REJECTED - elif status == "withdrawn": - poc.status = PatchOnCommitFest.STATUS_WITHDRAWN - elif status == "feedback": - poc.status = PatchOnCommitFest.STATUS_RETURNED - elif status == "next": - # Only some patch statuses can actually be moved. - if poc.status in ( - PatchOnCommitFest.STATUS_COMMITTED, - PatchOnCommitFest.STATUS_NEXT, - PatchOnCommitFest.STATUS_RETURNED, - PatchOnCommitFest.STATUS_REJECTED, - ): - # Can't be moved! - messages.error( - request, - "A patch in status {0} cannot be moved to next commitfest.".format( - poc.statusstring - ), - ) - return HttpResponseRedirect("/%s/%s/" % (poc.commitfest.id, poc.patch.id)) - elif poc.status in ( - PatchOnCommitFest.STATUS_REVIEW, - PatchOnCommitFest.STATUS_AUTHOR, - PatchOnCommitFest.STATUS_COMMITTER, - ): - # This one can be moved - pass - else: - messages.error(request, "Invalid existing patch status") - - oldstatus = poc.status - - poc.status = PatchOnCommitFest.STATUS_NEXT - # Figure out the commitfest to actually put it on - newcf = CommitFest.objects.filter(status=CommitFest.STATUS_OPEN) - if len(newcf) == 0: - # Ok, there is no open CF at all. Let's see if there is a - # future one. - newcf = CommitFest.objects.filter(status=CommitFest.STATUS_FUTURE) - if len(newcf) == 0: - messages.error(request, "No open and no future commitfest exists!") - return HttpResponseRedirect( - "/%s/%s/" % (poc.commitfest.id, poc.patch.id) - ) - elif len(newcf) != 1: - messages.error( - request, "No open and multiple future commitfests exist!" - ) - return HttpResponseRedirect( - "/%s/%s/" % (poc.commitfest.id, poc.patch.id) - ) - elif len(newcf) != 1: - messages.error(request, "Multiple open commitfests exists!") - return HttpResponseRedirect("/%s/%s/" % (poc.commitfest.id, poc.patch.id)) - elif newcf[0] == poc.commitfest: - # The current open CF is the same one that we are already on. - # In this case, try to see if there is a future CF we can - # move it to. - newcf = CommitFest.objects.filter(status=CommitFest.STATUS_FUTURE) - if len(newcf) == 0: - messages.error( - request, - "Cannot move patch to the same commitfest, and no future commitfests exist!", - ) - return HttpResponseRedirect( - "/%s/%s/" % (poc.commitfest.id, poc.patch.id) - ) - elif len(newcf) != 1: - messages.error( - request, - "Cannot move patch to the same commitfest, and multiple future commitfests exist!", - ) - return HttpResponseRedirect( - "/%s/%s/" % (poc.commitfest.id, poc.patch.id) - ) - # Create a mapping to the new commitfest that we are bouncing - # this patch to. - newpoc = PatchOnCommitFest( - patch=poc.patch, - commitfest=newcf[0], - status=oldstatus, - enterdate=datetime.now(), - ) - newpoc.save() - elif status == "committed": + if status == "committed": committer = get_object_or_404(Committer, user__username=request.GET["c"]) if committer != poc.patch.committer: # Committer changed! @@ -1126,12 +1054,14 @@ def close(request, patchid, status): what="Changed committer to %s" % committer, ).save_and_notify(prevcommitter=prevcommitter) poc.status = PatchOnCommitFest.STATUS_COMMITTED - else: - raise Exception("Can't happen") - poc.patch.set_modified() - poc.patch.save() - poc.save() + status_mapping = { + "reject": PatchOnCommitFest.STATUS_REJECTED, + "withdrawn": PatchOnCommitFest.STATUS_WITHDRAWN, + "feedback": PatchOnCommitFest.STATUS_RETURNED, + "committed": PatchOnCommitFest.STATUS_COMMITTED, + } + poc.set_status(status_mapping[status]) PatchHistory( patch=poc.patch, @@ -1140,7 +1070,46 @@ def close(request, patchid, status): % (poc.commitfest, poc.statusstring), ).save_and_notify() - return HttpResponseRedirect("/%s/%s/" % (poc.commitfest.id, poc.patch.id)) + return HttpResponseRedirect(f"/patch/{patchid}") + + +def int_param_or_none(request, param): + """Helper function to convert a string to an int or return None.""" + try: + return int(request.GET.get(param, "")) + except ValueError: + return None + + +@login_required +@transaction.atomic +def move(request, patchid): + from_cf_id = int_param_or_none(request, "from_cf_id") + to_cf_id = int_param_or_none(request, "to_cf_id") + if from_cf_id is None or to_cf_id is None: + messages.error( + request, + "Invalid or missing from_cf_id or to_cf_id GET parameter", + ) + return HttpResponseRedirect(f"/patch/{patchid}/") + + from_cf = get_object_or_404(CommitFest, pk=from_cf_id) + to_cf = get_object_or_404(CommitFest, pk=to_cf_id) + + patch = get_object_or_404(Patch, pk=patchid) + try: + patch.move(from_cf, to_cf) + except UserInputError as e: + messages.error(request, f"Failed to move patch: {e}") + return HttpResponseRedirect(f"/patch/{patchid}/") + + PatchHistory( + patch=patch, + by=request.user, + what=f"Moved from CF {from_cf} to CF {to_cf}", + ).save_and_notify() + + return HttpResponseRedirect(f"/patch/{patchid}/") @login_required diff --git a/pgcommitfest/urls.py b/pgcommitfest/urls.py index a67f55fc..c4b4ff52 100644 --- a/pgcommitfest/urls.py +++ b/pgcommitfest/urls.py @@ -3,6 +3,7 @@ import pgcommitfest.auth import pgcommitfest.commitfest.ajax as ajax +import pgcommitfest.commitfest.apiv1 as apiv1 import pgcommitfest.commitfest.lookups as lookups import pgcommitfest.commitfest.reports as reports import pgcommitfest.commitfest.views as views @@ -15,6 +16,9 @@ urlpatterns = [ re_path(r"^$", views.home), + re_path(r"^api/v1/commitfests/needs_ci$", apiv1.commitfestst_that_need_ci), + re_path(r"^help/$", views.help), + re_path(r"^commitfest_history/$", views.commitfest_history), re_path(r"^me/$", views.me), re_path(r"^archive/$", views.archive), re_path(r"^activity(?P\.rss)?/", views.activity), @@ -26,9 +30,8 @@ re_path(r"^patch/(\d+)/edit/$", views.patchform), 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 - ), + re_path(r"^patch/(\d+)/close/(reject|withdrawn|feedback|committed)/$", views.close), + re_path(r"^patch/(\d+)/move/$", views.move), re_path(r"^patch/(\d+)/reviewer/(become|remove)/$", views.reviewer), re_path(r"^patch/(\d+)/committer/(become|remove)/$", views.committer), re_path(r"^patch/(\d+)/(un)?subscribe/$", views.subscribe), From 570b7875a85b60eb02580808057779f16927aacc Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 9 Jun 2025 12:43:42 +0200 Subject: [PATCH 2/8] Don't automatically create commitfests in dev environments --- pgcommitfest/commitfest/models.py | 3 ++- pgcommitfest/local_settings_example.py | 5 +++++ pgcommitfest/settings.py | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index f67ca1c5..a1336f49 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -1,3 +1,4 @@ +from django.conf import settings from django.contrib.auth.models import User from django.db import models, transaction from django.db.models import Q @@ -163,7 +164,7 @@ def _refresh_relevant_commitfests(cls, for_update): @classmethod def relevant_commitfests(cls, for_update=False, refresh=True): - if refresh: + if refresh and settings.AUTO_CREATE_COMMITFESTS: return cls._refresh_relevant_commitfests(for_update=for_update) query_base = CommitFest.objects.order_by("-enddate") diff --git a/pgcommitfest/local_settings_example.py b/pgcommitfest/local_settings_example.py index 9a80cb20..b4ad8016 100644 --- a/pgcommitfest/local_settings_example.py +++ b/pgcommitfest/local_settings_example.py @@ -32,3 +32,8 @@ ) CFBOT_SECRET = "INSECURE" + +# There are already commitfests in the default dummy database data. +# Automatically creating new ones would cause the ones that are visible on the +# homepage to have no data. +AUTO_CREATE_COMMITFESTS = False diff --git a/pgcommitfest/settings.py b/pgcommitfest/settings.py index 9b867b71..56b1b001 100644 --- a/pgcommitfest/settings.py +++ b/pgcommitfest/settings.py @@ -164,6 +164,8 @@ # Email address for outgoing system messages NOTIFICATION_FROM = "webmaster@postgresql.org" +AUTO_CREATE_COMMITFESTS = True + # Load local settings overrides try: from .local_settings import * # noqa: F403 From 975e365c910de28e31d134a244ab8f3bd1bafbbf Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 9 Jun 2025 20:35:17 +0200 Subject: [PATCH 3/8] Add "Returned with Feedback" and expand explanation of other patch statuses --- pgcommitfest/commitfest/templates/help.html | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/pgcommitfest/commitfest/templates/help.html b/pgcommitfest/commitfest/templates/help.html index c6234366..20062c5b 100644 --- a/pgcommitfest/commitfest/templates/help.html +++ b/pgcommitfest/commitfest/templates/help.html @@ -20,18 +20,19 @@

    Patches

    A "patch" is a bit of an overloaded term in the PostgreSQL community. Email threads on the mailing list often contain "patch files" as attachments, such a file is often referred to as a "patch". A single email can even contain multiple related "patch files", which are called a "patchset". However, in the context of a CommitFest app a "patch" usually means a "patch entry" in the CommitFest app. Such a "patch entry" is a reference to a mailinglist thread on which change to PostgreSQL has been proposed, by someone sending an email that contain one or more "patch files". The CommitFest app will automatically detect new versions of the patch files and update the "patch entry" accordingly.

    - There are three active categories of patch status: + There are three "Active" categories of patch status:

      -
    • Waiting on Author - the author needs to make changes
      • -
    • Needs Reviewer - the patch needs guidance or a review
      • -
    • Ready for Committer - the patch is ready to be committed
      • +
    • Waiting on Author: The author needs to make changes based on a review that took place on the mailinglist.
      • +
    • Needs Reviewer: The patch needs guidance or a review.
      • +
    • Ready for Committer: The patch has been reviewed by non-committers and is considered ready to be committed by one of those reviewers. A committer might still decide that more changes are needed.
    - And there are three preferred inactive categories of patch status for when - a patch has been resolved, either by being committed or not: + And there four "Closed" categories of patch status for when a patch has been + resolved, either by being committed or not:
      -
    • Committed - a committer has applied the patches to the git repo
      • -
    • Withdrawn - the author has withdrawn the patch from consideration
      • -
    • Rejected - a committer has decided that the patch should not be applied
      • +
    • Committed: A committer has applied the patches to the git repo.
      • +
    • Withdrawn: The author has withdrawn the patch from consideration.
      • +
    • Rejected: A committer has decided that either the feature is not desired, or the design of the patch has severe flaws. People should look at the emails on the email thread, for details on why the patch was rejected.
      • +
    • Returned with Feedback: A committer has decided that the patch needs changes, but the author has not responded for long enough that the patch is now considered inactive. The author (or someoene else) is free to re-open again when they address the required changes.

    From 4fd357f9c73de5e73aee02c54fd522b8617c953c Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 9 Jun 2025 20:49:55 +0200 Subject: [PATCH 4/8] Add commet about reasoning why we update commitfests in this kinda weird way --- pgcommitfest/commitfest/models.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index a1336f49..8f7c1df2 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -164,6 +164,19 @@ def _refresh_relevant_commitfests(cls, for_update): @classmethod def relevant_commitfests(cls, for_update=False, refresh=True): + """If refresh is True (which is the default) this will automatically + update the commitfests if their state is out of date. It will also + create a new ones automatically when needed. + + The primary reason this refreshing is not done through a cron job is + that that requires work on the infrastructure side. Which is a huge + hassle to make happen in practice, due to an overloaded infrastructure + team. + + Luckily checking if a refresh is needed is very cheap, just a few + comparisons (see _are_relevant_commitfests_up_to_date for details). And + the actual updates only happen ~once a month. + """ if refresh and settings.AUTO_CREATE_COMMITFESTS: return cls._refresh_relevant_commitfests(for_update=for_update) From a29aa00d480ecae675c2af3a0f21cee6159c9a0b Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 9 Jun 2025 20:52:28 +0200 Subject: [PATCH 5/8] Missing word in help page --- pgcommitfest/commitfest/templates/help.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pgcommitfest/commitfest/templates/help.html b/pgcommitfest/commitfest/templates/help.html index 20062c5b..7a4f1984 100644 --- a/pgcommitfest/commitfest/templates/help.html +++ b/pgcommitfest/commitfest/templates/help.html @@ -26,8 +26,8 @@

    Patches

  • Needs Reviewer: The patch needs guidance or a review.
  • Ready for Committer: The patch has been reviewed by non-committers and is considered ready to be committed by one of those reviewers. A committer might still decide that more changes are needed.
    • - And there four "Closed" categories of patch status for when a patch has been - resolved, either by being committed or not: + And there are four "Closed" categories of patch status for when a patch has + been resolved, either by being committed or not:
    • Committed: A committer has applied the patches to the git repo.
    • Withdrawn: The author has withdrawn the patch from consideration.
      • From 4c4a2c61d8ce95908c0cc00fbc1bf2cad603f204 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 9 Jun 2025 21:08:12 +0200 Subject: [PATCH 6/8] Automatically move patches in open CF to in progress CF if they get committed --- pgcommitfest/commitfest/models.py | 21 +++++++++++++++++++-- pgcommitfest/commitfest/views.py | 19 ++++++++++++------- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 8f7c1df2..8fa0a777 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -265,6 +265,10 @@ def next_draft_cf(start_date): draft=True, ) + @classmethod + def get_in_progress(cls): + return cls.objects.filter(status=CommitFest.STATUS_INPROGRESS).first() + def __str__(self): return self.name @@ -406,7 +410,9 @@ def update_lastmail(self): else: self.lastmail = max(threads, key=lambda t: t.latestmessage).latestmessage - def move(self, from_cf, to_cf): + def move(self, from_cf, to_cf, by_user, allow_move_to_in_progress=False): + """Returns the new PatchOnCommitFest object, or raises UserInputError""" + current_poc = self.current_patch_on_commitfest() if from_cf.id != current_poc.commitfest.id: raise UserInputError("Patch not in source commitfest.") @@ -423,7 +429,10 @@ def move(self, from_cf, to_cf): f"Patch in state {current_poc.statusstring} cannot be moved." ) - if not to_cf.is_open: + if to_cf.is_in_progress: + if not allow_move_to_in_progress: + raise UserInputError("Patch can only be moved to an open commitfest") + elif not to_cf.is_open: raise UserInputError("Patch can only be moved to an open commitfest") old_status = current_poc.status @@ -443,6 +452,14 @@ def move(self, from_cf, to_cf): self.set_modified() self.save() + PatchHistory( + patch=self, + by=by_user, + what=f"Moved from CF {from_cf} to CF {to_cf}", + ).save_and_notify() + + return new_poc + def __str__(self): return self.name diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 45c5be83..6be43f63 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -1053,6 +1053,17 @@ def close(request, patchid, status): by=request.user, what="Changed committer to %s" % committer, ).save_and_notify(prevcommitter=prevcommitter) + + if poc.is_open: + in_progress_cf = CommitFest.get_in_progress() + if in_progress_cf is not None: + poc = patch.move( + poc.commitfest, + in_progress_cf, + request.user, + allow_move_to_in_progress=True, + ) + poc.status = PatchOnCommitFest.STATUS_COMMITTED status_mapping = { @@ -1098,17 +1109,11 @@ def move(request, patchid): patch = get_object_or_404(Patch, pk=patchid) try: - patch.move(from_cf, to_cf) + patch.move(from_cf, to_cf, request.user) except UserInputError as e: messages.error(request, f"Failed to move patch: {e}") return HttpResponseRedirect(f"/patch/{patchid}/") - PatchHistory( - patch=patch, - by=request.user, - what=f"Moved from CF {from_cf} to CF {to_cf}", - ).save_and_notify() - return HttpResponseRedirect(f"/patch/{patchid}/") From f6c61e7d92a0f68f5098d3accdc1618002226ce7 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 9 Jun 2025 21:28:27 +0200 Subject: [PATCH 7/8] Fix bug setting committer --- pgcommitfest/commitfest/views.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 6be43f63..80029370 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -1043,6 +1043,18 @@ def close(request, patchid, status): ) if status == "committed": + if poc.is_open: + # Needs to be done before the next if condition, so the committer + # that's set there is set on the correct poc. + in_progress_cf = CommitFest.get_in_progress() + if in_progress_cf is not None: + poc = patch.move( + poc.commitfest, + in_progress_cf, + request.user, + allow_move_to_in_progress=True, + ) + committer = get_object_or_404(Committer, user__username=request.GET["c"]) if committer != poc.patch.committer: # Committer changed! @@ -1054,16 +1066,6 @@ def close(request, patchid, status): what="Changed committer to %s" % committer, ).save_and_notify(prevcommitter=prevcommitter) - if poc.is_open: - in_progress_cf = CommitFest.get_in_progress() - if in_progress_cf is not None: - poc = patch.move( - poc.commitfest, - in_progress_cf, - request.user, - allow_move_to_in_progress=True, - ) - poc.status = PatchOnCommitFest.STATUS_COMMITTED status_mapping = { From 96b046941954d3099feb61966ba77be1d4b41848 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 9 Jun 2025 23:03:46 +0200 Subject: [PATCH 8/8] Automatically move committed draft patches to the open commitfest --- pgcommitfest/commitfest/models.py | 4 ++++ pgcommitfest/commitfest/views.py | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 8fa0a777..ec417723 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -269,6 +269,10 @@ def next_draft_cf(start_date): def get_in_progress(cls): return cls.objects.filter(status=CommitFest.STATUS_INPROGRESS).first() + @classmethod + def get_open_regular(cls): + return cls.objects.filter(status=CommitFest.STATUS_OPEN, draft=False).first() + def __str__(self): return self.name diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 80029370..29bcb59a 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -1043,7 +1043,7 @@ def close(request, patchid, status): ) if status == "committed": - if poc.is_open: + if poc.commitfest.is_open: # Needs to be done before the next if condition, so the committer # that's set there is set on the correct poc. in_progress_cf = CommitFest.get_in_progress() @@ -1054,6 +1054,13 @@ def close(request, patchid, status): request.user, allow_move_to_in_progress=True, ) + elif poc.commitfest.draft: + open_cf = CommitFest.get_open_regular() + poc = patch.move( + poc.commitfest, + open_cf, + request.user, + ) committer = get_object_or_404(Committer, user__username=request.GET["c"]) if committer != poc.patch.committer: