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..ec417723 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -1,8 +1,10 @@ +from django.conf import settings 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 +36,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 +63,216 @@ 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 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) + + 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, + ) + + @classmethod + 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 @@ -102,6 +300,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 +361,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 +414,56 @@ def update_lastmail(self): else: self.lastmail = max(threads, key=lambda t: t.latestmessage).latestmessage + 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.") + + 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 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 + + 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() + + 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 @@ -224,7 +480,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 +489,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 +499,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 +529,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..7a4f1984 --- /dev/null +++ b/pgcommitfest/commitfest/templates/help.html @@ -0,0 +1,52 @@ +{%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 four "Closed" 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..29bcb59a 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,26 @@ 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) + if status == "committed": + 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() + if in_progress_cf is not None: + poc = patch.move( + poc.commitfest, + in_progress_cf, + request.user, + allow_move_to_in_progress=True, ) - elif len(newcf) != 1: - messages.error( - request, "No open and multiple future commitfests exist!" + elif poc.commitfest.draft: + open_cf = CommitFest.get_open_regular() + poc = patch.move( + poc.commitfest, + open_cf, + request.user, ) - 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": + committer = get_object_or_404(Committer, user__username=request.GET["c"]) if committer != poc.patch.committer: # Committer changed! @@ -1125,13 +1072,16 @@ def close(request, patchid, status): by=request.user, 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 +1090,40 @@ 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, request.user) + except UserInputError as e: + messages.error(request, f"Failed to move patch: {e}") + return HttpResponseRedirect(f"/patch/{patchid}/") + + return HttpResponseRedirect(f"/patch/{patchid}/") @login_required 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 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),