Skip to content

Commit 33cf208

Browse files
polobopolobo
committed
Introduce Workflow, Drafts, and API
The Commitfest Workflow is a formalization and extension of the existing commitfest-only protocol. There is now some in-app documentation and the idea of moving patches to the "next CF" has begun being deprecated. Instead the UI provides the user valid choices on where to transition patches, which, with the introduction of the Drafts commitfest, is basically one or the other. Committers gain the ability to transition patches into In Progress if desired. Patches within In Progress commitfests can be moved into either Open or Drafts. Begin enforcement of a maximum of one commifest in each of Parked, Open, and In Progress. The workflow defines the drafts commitfest as lasting for one year matching up with the major release process. Namely, once the commifest leading up to feature freeze is In Progress a new Drafts commitfest should be created. With the workflow changes it is necessary for CFBot to be made aware of the specific commitfest ids that are active. Provide a JSON API for this purpose to begin weaning CFBot off scraping html. The workflow in-app documentation includes a month-based schedule showing which commifests are active, and their names, in each month (among other related details). Begin enforcing the existing rule that future commitfests cannot have patches via triggers. Future work on the administrative and UX aspects of the CFApp likely will do away with future commitfests altogether. Thoughts on the topic are welcome in the GitHub issues.
1 parent a3a7ecd commit 33cf208

File tree

11 files changed

+653
-90
lines changed

11 files changed

+653
-90
lines changed

‎pgcommitfest/commitfest/apiv1.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
from django.http import (
2+
HttpResponse,
3+
)
4+
5+
import json
6+
from datetime import datetime
7+
8+
from .models import (
9+
Workflow,
10+
)
11+
12+
13+
def datetime_serializer(obj):
14+
if isinstance(obj, datetime):
15+
return obj.strftime("%Y-%m-%dT%H:%M:%S%z")
16+
raise TypeError("Type not serializable")
17+
18+
19+
def apiResponse(request, payload, status=200, content_type="application/json"):
20+
response = HttpResponse(
21+
json.dumps(payload, default=datetime_serializer), status=status
22+
)
23+
response["Content-Type"] = content_type
24+
response["Access-Control-Allow-Origin"] = "*"
25+
return response
26+
27+
28+
def optional_as_json(obj):
29+
if obj is None:
30+
return None
31+
return obj.json()
32+
33+
34+
def active_commitfests(request):
35+
payload = {
36+
"workflow": {
37+
"open": optional_as_json(Workflow.open_cf()),
38+
"inprogress": optional_as_json(Workflow.inprogress_cf()),
39+
"parked": optional_as_json(Workflow.parked_cf()),
40+
},
41+
}
42+
return apiResponse(request, payload)

‎pgcommitfest/commitfest/fixtures/auth_data.json

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,41 @@
8888
"groups": [],
8989
"user_permissions": []
9090
}
91+
},
92+
{
93+
"model": "auth.user",
94+
"pk": 6,
95+
"fields": {
96+
"password": "",
97+
"last_login": null,
98+
"is_superuser": false,
99+
"username": "prolific-author",
100+
"first_name": "Prolific",
101+
"last_name": "Author",
102+
"email": "",
103+
"is_staff": false,
104+
"is_active": true,
105+
"date_joined": "2025-01-01T00:00:00",
106+
"groups": [],
107+
"user_permissions": []
108+
}
109+
},
110+
{
111+
"model": "auth.user",
112+
"pk": 7,
113+
"fields": {
114+
"password": "",
115+
"last_login": null,
116+
"is_superuser": false,
117+
"username": "prolific-reviewer",
118+
"first_name": "Prolific",
119+
"last_name": "Reviewer",
120+
"email": "",
121+
"is_staff": false,
122+
"is_active": true,
123+
"date_joined": "2025-01-01T00:00:00",
124+
"groups": [],
125+
"user_permissions": []
126+
}
91127
}
92128
]

‎pgcommitfest/commitfest/fixtures/commitfest_data.json

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@
6060
"enddate": "2025-05-31"
6161
}
6262
},
63+
{
64+
"model": "commitfest.commitfest",
65+
"pk": 5,
66+
"fields": {
67+
"name": "Drafts PG19",
68+
"status": 5,
69+
"startdate": "2024-09-01",
70+
"enddate": "2025-08-31"
71+
}
72+
},
6373
{
6474
"model": "commitfest.topic",
6575
"pk": 1,
@@ -237,6 +247,25 @@
237247
]
238248
}
239249
},
250+
{
251+
"model": "commitfest.patch",
252+
"pk": 8,
253+
"fields": {
254+
"name": "Test DGJ Multi-Author and Reviewer",
255+
"topic": 3,
256+
"wikilink": "",
257+
"gitlink": "",
258+
"targetversion": 1,
259+
"committer": 4,
260+
"created": "2025-02-01T00:00",
261+
"modified": "2025-02-01T00:00",
262+
"lastmail": "2025-02-01T00:00",
263+
"authors": [6,3],
264+
"reviewers": [7,1],
265+
"subscribers": [],
266+
"mailthread_set": [8]
267+
}
268+
},
240269
{
241270
"model": "commitfest.patchoncommitfest",
242271
"pk": 1,
@@ -325,6 +354,17 @@
325354
"status": 1
326355
}
327356
},
357+
{
358+
"model": "commitfest.patchoncommitfest",
359+
"pk": 9,
360+
"fields": {
361+
"patch": 8,
362+
"commitfest": 5,
363+
"enterdate": "2025-02-01T00:00:00",
364+
"leavedate": null,
365+
"status": 1
366+
}
367+
},
328368
{
329369
"model": "commitfest.patchhistory",
330370
"pk": 1,
@@ -632,6 +672,33 @@
632672
"latestmsgid": "example@message-31"
633673
}
634674
},
675+
{
676+
"model": "commitfest.mailthread",
677+
"pk": 8,
678+
"fields": {
679+
"messageid": "dgj-example@message-08",
680+
"subject": "Test DGJ Multi-Author and Reviewer",
681+
"firstmessage": "2025-02-01T00:00",
682+
"firstauthor": "[email protected]",
683+
"latestmessage": "2025-02-01T00:00",
684+
"latestauthor": "[email protected]",
685+
"latestsubject": "Test DGJ Multi-Author and Reviewer",
686+
"latestmsgid": "dgj-example@message-08"
687+
}
688+
},
689+
{
690+
"model": "commitfest.mailthreadattachment",
691+
"pk": 8,
692+
"fields": {
693+
"messageid": "dgj-example@message-08",
694+
"attachmentid": 1,
695+
"filename": "v1-0001-content.patch",
696+
"date": "2025-02-01T00:00",
697+
"author": "[email protected]",
698+
"ispatch": true,
699+
"mailthread_id": 8
700+
}
701+
},
635702
{
636703
"model": "commitfest.patchstatus",
637704
"pk": 1,

‎pgcommitfest/commitfest/migrations/0011_add_status_related_constraints.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
from django.db import migrations
2+
3+
4+
class Migration(migrations.Migration):
5+
dependencies = [
6+
("commitfest", "0010_add_failing_since_column"),
7+
]
8+
operations = [
9+
migrations.RunSQL(
10+
"""
11+
CREATE UNIQUE INDEX cf_enforce_maxoneopen_idx
12+
ON commitfest_commitfest (status)
13+
WHERE status not in (1,4);
14+
""",
15+
reverse_sql="""
16+
DROP INDEX IF EXISTS cf_enforce_maxoneopen_idx;
17+
""",
18+
),
19+
migrations.RunSQL(
20+
"""
21+
CREATE UNIQUE INDEX poc_enforce_maxoneoutcome_idx
22+
ON commitfest_patchoncommitfest (patch_id)
23+
WHERE status not in (5);
24+
""",
25+
reverse_sql="""
26+
DROP INDEX IF EXISTS poc_enforce_maxoneoutcome_idx;
27+
""",
28+
),
29+
migrations.RunSQL(
30+
"""
31+
ALTER TABLE commitfest_patchoncommitfest
32+
ADD CONSTRAINT status_and_leavedate_correlation
33+
CHECK ((status IN (4,5,6,7,8)) = (leavedate IS NOT NULL));
34+
""",
35+
reverse_sql="""
36+
ALTER TABLE commitfest_patchoncommitfest
37+
DROP CONSTRAINT IF EXISTS status_and_leavedate_correlation;
38+
""",
39+
),
40+
migrations.RunSQL(
41+
"""
42+
COMMENT ON COLUMN commitfest_patchoncommitfest.leavedate IS
43+
$$A leave date is recorded in two situations, both of which
44+
means this particular patch-cf combination became inactive
45+
on the corresponding date. For status 5 the patch was moved
46+
to some other cf. For 4,6,7, and 8, this was the final cf.
47+
$$
48+
""",
49+
reverse_sql="""
50+
COMMENT ON COLUMN commitfest_patchoncommitfest.leavedate IS NULL;
51+
""",
52+
),
53+
migrations.RunSQL(
54+
"""
55+
COMMENT ON TABLE commitfest_patchoncommitfest IS
56+
$$This is a re-entrant table: patches may become associated
57+
with a given cf multiple times, resetting the entrydate and clearing
58+
the leavedate each time. Non-final statuses never have a leavedate
59+
while final statuses always do. The final status of 5 (moved) is
60+
special in that all but one of the rows a patch has in this table
61+
must have it as the status.
62+
$$
63+
""",
64+
reverse_sql="""
65+
COMMENT ON TABLE commitfest_patchoncommitfest IS NULL;
66+
""",
67+
),
68+
]

‎pgcommitfest/commitfest/migrations/0012_add_parked_cf_status.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
from django.db import migrations, models
2+
3+
4+
class Migration(migrations.Migration):
5+
dependencies = [
6+
("commitfest", "0011_add_status_related_constraints"),
7+
]
8+
operations = [
9+
migrations.AlterField(
10+
model_name="commitfest",
11+
name="status",
12+
field=models.IntegerField(
13+
choices=[
14+
(1, "Future"),
15+
(2, "Open"),
16+
(3, "In Progress"),
17+
(4, "Closed"),
18+
(5, "Parked"),
19+
],
20+
default=1,
21+
),
22+
)
23+
]

‎pgcommitfest/commitfest/migrations/0013_no_patches_in_future_cfs.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
from django.db import migrations
2+
3+
4+
class Migration(migrations.Migration):
5+
dependencies = [
6+
("commitfest", "0012_add_parked_cf_status"),
7+
]
8+
operations = [
9+
migrations.RunSQL(
10+
"""
11+
CREATE FUNCTION assert_poc_not_future_for_poc()
12+
RETURNS TRIGGER AS $$
13+
DECLARE
14+
cfstatus int;
15+
BEGIN
16+
SELECT status INTO cfstatus
17+
FROM commitfest_commitfest
18+
WHERE id = NEW.commitfest_id;
19+
IF cfstatus = 1 THEN
20+
RAISE EXCEPTION 'Patches cannot exist on future commitfests';
21+
END IF;
22+
RETURN NEW;
23+
END;
24+
$$
25+
LANGUAGE plpgsql;
26+
27+
CREATE FUNCTION assert_poc_not_future_for_cf()
28+
RETURNS trigger AS $$
29+
BEGIN
30+
-- Trigger checks that we only get called when status is 1
31+
PERFORM 1
32+
FROM commitfest_patchoncommitfest
33+
WHERE commitfest_id = NEW.id
34+
LIMIT 1;
35+
IF FOUND THEN
36+
RAISE EXCEPTION 'Cannot change commitfest status to 1, patches exist.';
37+
END IF;
38+
RETURN NEW;
39+
END;
40+
$$
41+
LANGUAGE plpgsql;
42+
43+
CREATE TRIGGER assert_poc_commitfest_is_not_future
44+
BEFORE INSERT OR UPDATE ON commitfest_patchoncommitfest
45+
FOR EACH ROW
46+
EXECUTE FUNCTION assert_poc_not_future_for_poc();
47+
48+
CREATE TRIGGER assert_poc_commitfest_is_not_future
49+
-- Newly inserted cfs can't have patches
50+
BEFORE UPDATE ON commitfest_commitfest
51+
FOR EACH ROW
52+
WHEN (NEW.status = 1)
53+
EXECUTE FUNCTION assert_poc_not_future_for_cf();
54+
""",
55+
reverse_sql="""
56+
DROP TRIGGER IF EXISTS assert_poc_commitfest_is_not_future ON commitfest_commitfest;
57+
DROP TRIGGER IF EXISTS assert_poc_commitfest_is_not_future ON commitfest_patchoncommitfest;
58+
DROP FUNCTION IF EXISTS assert_poc_not_future_for_poc();
59+
DROP FUNCTION IF EXISTS assert_poc_not_future_for_cf();
60+
""",
61+
),
62+
]

0 commit comments

Comments
 (0)