From 1f31e16188e3ca81e75905cd7321e30e363251b8 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 22 Oct 2025 10:25:54 +0200 Subject: [PATCH 01/38] Handle FormattingCheck specially It shouldn't be a hard failure. So this makes it yellow. --- .../commitfest/fixtures/commitfest_data.json | 76 ++++++++++++++++--- .../commitfest/templates/commitfest.html | 4 +- pgcommitfest/commitfest/templates/home.html | 4 +- pgcommitfest/commitfest/templates/patch.html | 6 +- pgcommitfest/commitfest/views.py | 14 +++- 5 files changed, 87 insertions(+), 17 deletions(-) diff --git a/pgcommitfest/commitfest/fixtures/commitfest_data.json b/pgcommitfest/commitfest/fixtures/commitfest_data.json index 0c72d3db..944ed3cb 100644 --- a/pgcommitfest/commitfest/fixtures/commitfest_data.json +++ b/pgcommitfest/commitfest/fixtures/commitfest_data.json @@ -1106,19 +1106,19 @@ "fields": { "branch_id": 345, "branch_name": "cf/2", - "commit_id": null, + "commit_id": "def456", "apply_url": "http://cfbot.cputube.org/patch_4573.log", - "status": "failed", - "needs_rebase_since": "2025-03-01T22:30:42", - "failing_since": "2025-02-01T22:30:42", + "status": "finished", + "needs_rebase_since": null, + "failing_since": null, "created": "2025-01-26T22:11:09.961", "modified": "2025-03-01T22:59:14.717", - "version": "", - "patch_count": null, - "first_additions": null, - "first_deletions": null, - "all_additions": null, - "all_deletions": null + "version": "v2", + "patch_count": 3, + "first_additions": 50, + "first_deletions": 10, + "all_additions": 120, + "all_deletions": 35 } }, { @@ -1295,5 +1295,61 @@ "created": "2025-07-03T06:29:25.086", "modified": "2025-07-03T06:29:25.086" } +}, +{ + "model": "commitfest.cfbottask", + "pk": 9, + "fields": { + "task_id": "12347", + "task_name": "FormattingCheck", + "patch": 1, + "branch_id": 123, + "position": 3, + "status": "FAILED", + "created": "2025-01-26T22:08:00.000", + "modified": "2025-01-26T22:08:30.000" + } +}, +{ + "model": "commitfest.cfbottask", + "pk": 10, + "fields": { + "task_id": "34501", + "task_name": "Linux build", + "patch": 2, + "branch_id": 345, + "position": 1, + "status": "COMPLETED", + "created": "2025-01-26T22:11:30.000", + "modified": "2025-01-26T22:12:15.000" + } +}, +{ + "model": "commitfest.cfbottask", + "pk": 11, + "fields": { + "task_id": "34502", + "task_name": "MacOS Build", + "patch": 2, + "branch_id": 345, + "position": 2, + "status": "COMPLETED", + "created": "2025-01-26T22:11:35.000", + "modified": "2025-01-26T22:13:20.000" + } +}, +{ + "model": "commitfest.cfbottask", + "pk": 12, + "fields": { + "task_id": "34503", + "task_name": "FormattingCheck", + "patch": 2, + "branch_id": 345, + "position": 3, + "status": "COMPLETED", + "created": "2025-01-26T22:11:40.000", + "modified": "2025-01-26T22:12:00.000" + } } ] diff --git a/pgcommitfest/commitfest/templates/commitfest.html b/pgcommitfest/commitfest/templates/commitfest.html index d20aa84d..ac1b778b 100644 --- a/pgcommitfest/commitfest/templates/commitfest.html +++ b/pgcommitfest/commitfest/templates/commitfest.html @@ -75,8 +75,10 @@

{{p.is_open|yesno:"Active patches,Closed patches"}}

- {%if cfb.failed > 0 or cfb.branch_status == 'failed' or cfb.branch_status == 'timeout' %} + {%if cfb.branch_status == 'failed' or cfb.branch_status == 'timeout' or cfb.failed_non_formatting > 0 %} + {%elif cfb.failed > 0 %} + {%elif cfb.completed < cfb.total %} {%else%} diff --git a/pgcommitfest/commitfest/templates/home.html b/pgcommitfest/commitfest/templates/home.html index d0fbade9..80ff99ff 100644 --- a/pgcommitfest/commitfest/templates/home.html +++ b/pgcommitfest/commitfest/templates/home.html @@ -182,8 +182,10 @@

{%if user.is_authenticated%}Open patches you are subscribed to{%elif p.is_op - {%if cfb.failed > 0 or cfb.branch_status == 'failed' or cfb.branch_status == 'timeout' %} + {%if cfb.branch_status == 'failed' or cfb.branch_status == 'timeout' or cfb.failed_non_formatting > 0 %} + {%elif cfb.failed > 0 %} + {%elif cfb.completed < cfb.total %} {%else%} diff --git a/pgcommitfest/commitfest/templates/patch.html b/pgcommitfest/commitfest/templates/patch.html index 1c37a05a..2ad98256 100644 --- a/pgcommitfest/commitfest/templates/patch.html +++ b/pgcommitfest/commitfest/templates/patch.html @@ -38,7 +38,11 @@ {%elif c.status == 'EXECUTING' %} {%else %} - + {%if c.task_name == 'FormattingCheck' %} + + {%else%} + + {%endif%} {%endif%} {%endfor%} {%endif%} diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 6b3bd403..1f82b607 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -564,6 +564,7 @@ def patchlist(request, cf, personalized=False): count(*) FILTER (WHERE task.status in ('COMPLETED', 'PAUSED')) as completed, count(*) FILTER (WHERE task.status in ('CREATED', 'SCHEDULED', 'EXECUTING')) running, count(*) FILTER (WHERE task.status in ('ABORTED', 'ERRORED', 'FAILED')) failed, + count(*) FILTER (WHERE task.status in ('ABORTED', 'ERRORED', 'FAILED') AND task.task_name != 'FormattingCheck') as failed_non_formatting, count(*) total, string_agg(task.task_name, ', ') FILTER (WHERE task.status in ('ABORTED', 'ERRORED', 'FAILED')) as failed_task_names, branch.status as branch_status, @@ -1649,10 +1650,15 @@ def cfbot_ingest(message): failing = branch_status["status"] in ("failed", "timeout") or needs_rebase finished = branch_status["status"] == "finished" - if "task_status" in message and message["task_status"]["status"] in ( - "ABORTED", - "ERRORED", - "FAILED", + if ( + "task_status" in message + and message["task_status"]["status"] + in ( + "ABORTED", + "ERRORED", + "FAILED", + ) + and message["task_status"]["task_name"] != "FormattingCheck" ): failing = True From 572e7d548a41b53a0b8c1fce1d652ace22623990 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 22 Oct 2025 10:37:33 +0200 Subject: [PATCH 02/38] Add missing svg --- media/commitfest/formatting_failure.svg | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 media/commitfest/formatting_failure.svg diff --git a/media/commitfest/formatting_failure.svg b/media/commitfest/formatting_failure.svg new file mode 100644 index 00000000..a1f5c756 --- /dev/null +++ b/media/commitfest/formatting_failure.svg @@ -0,0 +1,5 @@ + + + + + From ab3abb2dba68de03b372d6bd880277d61f377724 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 22 Oct 2025 11:08:27 +0200 Subject: [PATCH 03/38] Change formatting failure icon --- media/commitfest/formatting_failure.svg | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/media/commitfest/formatting_failure.svg b/media/commitfest/formatting_failure.svg index a1f5c756..e46c84bc 100644 --- a/media/commitfest/formatting_failure.svg +++ b/media/commitfest/formatting_failure.svg @@ -1,5 +1,5 @@ - - - + + + From fdf5d9276e316e99417c962d34eb9d8af51c8879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20=C5=A0im=C3=A1nek?= Date: Sun, 2 Nov 2025 14:59:46 +0100 Subject: [PATCH 04/38] Fix project name typo. (#97) pgocmmitfest -> pgcommitfest --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 3ef0d3bb..da8daa09 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [project] -name = "pgocmmitfest" +name = "pgcommitfest" description = "Commitfest app for the PostgreSQL community" dynamic = ["version"] readme = "README.md" From 7d12350a5aa6b430edba720a8dd234d8fedc413e Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 10 Nov 2025 22:55:56 +0100 Subject: [PATCH 05/38] Add basic tests (#99) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add initial testing infrastructure. Close #98 --------- Co-authored-by: Josef Šimánek --- .github/workflows/ci.yaml | 34 ++++++++ pgcommitfest/commitfest/tests/__init__.py | 1 + pgcommitfest/commitfest/tests/test_apiv1.py | 94 +++++++++++++++++++++ pyproject.toml | 12 +++ 4 files changed, 141 insertions(+) create mode 100644 pgcommitfest/commitfest/tests/__init__.py create mode 100644 pgcommitfest/commitfest/tests/test_apiv1.py diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2beb0666..232eb040 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -33,3 +33,37 @@ jobs: - name: Run djhtml run: djhtml pgcommitfest/*/templates/*.html pgcommitfest/*/templates/*.inc --tabwidth=1 --check + + test: + runs-on: ubuntu-24.04 + name: "Django Tests" + + services: + postgres: + image: postgres:18 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Python with uv + uses: astral-sh/setup-uv@v4 + + - name: Create CI settings + run: cp pgcommitfest/local_settings_example.py pgcommitfest/local_settings.py + + - name: Install dependencies + run: uv sync --extra dev + + - name: Run tests + run: uv run pytest --create-db diff --git a/pgcommitfest/commitfest/tests/__init__.py b/pgcommitfest/commitfest/tests/__init__.py new file mode 100644 index 00000000..7d5a9d90 --- /dev/null +++ b/pgcommitfest/commitfest/tests/__init__.py @@ -0,0 +1 @@ +# Tests for the commitfest application diff --git a/pgcommitfest/commitfest/tests/test_apiv1.py b/pgcommitfest/commitfest/tests/test_apiv1.py new file mode 100644 index 00000000..196d03c0 --- /dev/null +++ b/pgcommitfest/commitfest/tests/test_apiv1.py @@ -0,0 +1,94 @@ +from django.test import override_settings + +import json +from datetime import date + +import pytest + +from pgcommitfest.commitfest.models import CommitFest + +pytestmark = pytest.mark.django_db + + +@pytest.fixture +def commitfests(): + """Create test commitfests with various statuses.""" + return { + "open": CommitFest.objects.create( + name="2025-01", + status=CommitFest.STATUS_OPEN, + startdate=date(2025, 1, 1), + enddate=date(2025, 1, 31), + draft=False, + ), + "in_progress": CommitFest.objects.create( + name="2024-11", + status=CommitFest.STATUS_INPROGRESS, + startdate=date(2024, 11, 1), + enddate=date(2024, 11, 30), + draft=False, + ), + "recent_previous": CommitFest.objects.create( + name="2024-09", + status=CommitFest.STATUS_CLOSED, + startdate=date(2024, 9, 1), + enddate=date(2024, 9, 30), + draft=False, + ), + "old_previous": CommitFest.objects.create( + name="2024-07", + status=CommitFest.STATUS_CLOSED, + startdate=date(2024, 7, 1), + enddate=date(2024, 7, 31), + draft=False, + ), + "draft": CommitFest.objects.create( + name="2025-03-draft", + status=CommitFest.STATUS_OPEN, + startdate=date(2025, 3, 1), + enddate=date(2025, 3, 31), + draft=True, + ), + } + + +def test_needs_ci_endpoint(client, commitfests): + """Test the /api/v1/commitfests/needs_ci endpoint returns correct data.""" + with override_settings(AUTO_CREATE_COMMITFESTS=False): + response = client.get("/api/v1/commitfests/needs_ci") + + # Check response metadata + assert response.status_code == 200 + assert response["Content-Type"] == "application/json" + assert response["Access-Control-Allow-Origin"] == "*" + + # Parse and compare response + data = json.loads(response.content) + + expected = { + "commitfests": { + "open": { + "id": commitfests["open"].id, + "name": "2025-01", + "status": "Open", + "startdate": "2025-01-01", + "enddate": "2025-01-31", + }, + "in_progress": { + "id": commitfests["in_progress"].id, + "name": "2024-11", + "status": "In Progress", + "startdate": "2024-11-01", + "enddate": "2024-11-30", + }, + "draft": { + "id": commitfests["draft"].id, + "name": "2025-03-draft", + "status": "Open", + "startdate": "2025-03-01", + "enddate": "2025-03-31", + }, + } + } + + assert data == expected diff --git a/pyproject.toml b/pyproject.toml index da8daa09..53551b67 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,6 +18,8 @@ dev = [ "pycodestyle", "ruff", "djhtml", + "pytest", + "pytest-django", ] [tool.setuptools.packages.find] @@ -47,3 +49,13 @@ section-order = [ [tool.ruff.lint.isort.sections] # Group all Django imports into a separate section. django = ["django"] + +[tool.pytest.ini_options] +DJANGO_SETTINGS_MODULE = "pgcommitfest.settings" +python_files = ["tests.py", "test_*.py", "*_tests.py"] +testpaths = ["pgcommitfest"] +addopts = [ + "--reuse-db", + "--strict-markers", + "-vv", +] From 41730fd105d237d06d1efa3defb8bd0d8e9b7d70 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 10 Nov 2025 23:16:08 +0100 Subject: [PATCH 06/38] Fix deprecations for django 5 --- pgcommitfest/commitfest/templates/base.html | 207 ++++++++++---------- pgcommitfest/settings.py | 5 +- 2 files changed, 106 insertions(+), 106 deletions(-) diff --git a/pgcommitfest/commitfest/templates/base.html b/pgcommitfest/commitfest/templates/base.html index 8970990c..c7f2d113 100644 --- a/pgcommitfest/commitfest/templates/base.html +++ b/pgcommitfest/commitfest/templates/base.html @@ -1,112 +1,115 @@ {%load commitfest%} - - - - {{title|default:'Commitfest' }} - - - - - - - {%block extrahead%}{%endblock%} - {%if rss_alternate%} {%endif%} - - +{%load l10n%} +{%localize off%} + + + + {{title|default:'Commitfest' }} + + + + + + + {%block extrahead%}{%endblock%} + {%if rss_alternate%} {%endif%} + + - + -
+
- + - {%if title %} -

{{title}}

- {%endif%} + {%if title %} +

{{title}}

+ {%endif%} - {%if messages%} - {%for m in messages%} -
{{m}}
- {%endfor%} - {%endif%} + {%if messages%} + {%for m in messages%} +
{{m}}
+ {%endfor%} + {%endif%} - {%block contents%} - {%endblock%} -
- - - - - - {%block morescript%}{%endblock%} - + {%block contents%} + {%endblock%} +
+ + + + + + {%block morescript%}{%endblock%} +{%endlocalize%} + diff --git a/pgcommitfest/settings.py b/pgcommitfest/settings.py index 30e5d1cf..36d62f3d 100644 --- a/pgcommitfest/settings.py +++ b/pgcommitfest/settings.py @@ -29,6 +29,7 @@ # If running in a Windows environment this must be set to the same as your # system time zone. TIME_ZONE = "GMT" +USE_TZ = True # Language code for this installation. All choices can be found here: # http://www.i18nguy.com/unicode/language-identifiers.html @@ -40,10 +41,6 @@ # to load the internationalization machinery. USE_I18N = False -# If you set this to False, Django will not format dates, numbers and -# calendars according to the current locale -USE_L10N = False - # Absolute filesystem path to the directory that will hold user-uploaded files. # Example: "/home/media/media.lawrence.com/media/" MEDIA_ROOT = "" From abac679214697a8842bcdfed930dbd710287bf52 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 10 Nov 2025 23:17:59 +0100 Subject: [PATCH 07/38] Switch TZ support off again, needs more fixes --- pgcommitfest/settings.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pgcommitfest/settings.py b/pgcommitfest/settings.py index 36d62f3d..4cd1e179 100644 --- a/pgcommitfest/settings.py +++ b/pgcommitfest/settings.py @@ -29,7 +29,9 @@ # If running in a Windows environment this must be set to the same as your # system time zone. TIME_ZONE = "GMT" -USE_TZ = True + +# Our code currently compares naive datetimes +USE_TZ = False # Language code for this installation. All choices can be found here: # http://www.i18nguy.com/unicode/language-identifiers.html From 641ae520a00238969016b6987b538316e6448542 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 11 Nov 2025 10:46:31 +0100 Subject: [PATCH 08/38] Support requeueing CFBot builds --- pgcommitfest/commitfest/templates/patch.html | 6 ++++ pgcommitfest/commitfest/views.py | 32 ++++++++++++++++++++ pgcommitfest/local_settings_example.py | 1 + pgcommitfest/settings.py | 2 ++ pgcommitfest/urls.py | 1 + 5 files changed, 42 insertions(+) diff --git a/pgcommitfest/commitfest/templates/patch.html b/pgcommitfest/commitfest/templates/patch.html index 2ad98256..5a9ac905 100644 --- a/pgcommitfest/commitfest/templates/patch.html +++ b/pgcommitfest/commitfest/templates/patch.html @@ -52,6 +52,12 @@ git fetch commitfest cf/{{patch.id}} git checkout commitfest/cf/{{patch.id}}" onclick="addGitCheckoutToClipboard({{patch.id}})">Copy git checkout commands {%endif%} + {%if user.is_authenticated%} +
+ {%csrf_token%} + +
+ {%endif%} diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 1f82b607..44057da9 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -1700,3 +1700,35 @@ def thread_notify(request): refresh_single_thread(t) return HttpResponse(status=200) + + +@login_required +def cfbot_requeue(request, patchid): + """Trigger a requeue of a patch in the cfbot.""" + if request.method != "POST": + return HttpResponseForbidden(b"Invalid method") + + patch = get_object_or_404(Patch, pk=patchid) + cf = patch.current_commitfest() + + # Make API call to cfbot + import requests + + payload = { + "commitfest_id": cf.id, + "submission_id": patchid, + "shared_secret": settings.CFBOT_SECRET, + } + + try: + response = requests.post( + f"{settings.CFBOT_API_URL}/requeue-patch", json=payload, timeout=10 + ) + if response.status_code == 200: + messages.success(request, "Patch requeued successfully") + else: + messages.error(request, f"Failed to requeue patch: {response.status_code}") + except requests.exceptions.RequestException as e: + messages.error(request, f"Failed to requeue patch: {e}") + + return HttpResponseRedirect(f"/patch/{patchid}/") diff --git a/pgcommitfest/local_settings_example.py b/pgcommitfest/local_settings_example.py index b4ad8016..392277b5 100644 --- a/pgcommitfest/local_settings_example.py +++ b/pgcommitfest/local_settings_example.py @@ -32,6 +32,7 @@ ) CFBOT_SECRET = "INSECURE" +CFBOT_API_URL = "http://localhost:5000/api" # There are already commitfests in the default dummy database data. # Automatically creating new ones would cause the ones that are visible on the diff --git a/pgcommitfest/settings.py b/pgcommitfest/settings.py index 4cd1e179..e48cf09e 100644 --- a/pgcommitfest/settings.py +++ b/pgcommitfest/settings.py @@ -166,6 +166,8 @@ AUTO_CREATE_COMMITFESTS = True +CFBOT_API_URL = "https://cfbot.cputube.org/api" + # Load local settings overrides try: from .local_settings import * # noqa: F403 diff --git a/pgcommitfest/urls.py b/pgcommitfest/urls.py index 7165fffc..95c988e4 100644 --- a/pgcommitfest/urls.py +++ b/pgcommitfest/urls.py @@ -36,6 +36,7 @@ re_path(r"^patch/(\d+)/committer/(become|remove)/$", views.committer), re_path(r"^patch/(\d+)/(un)?subscribe/$", views.subscribe), re_path(r"^patch/(\d+)/(comment|review)/", views.comment), + re_path(r"^patch/(\d+)/cfbot_requeue/$", views.cfbot_requeue), re_path(r"^(\d+)/send_email/$", views.send_email), re_path(r"^patch/(\d+)/send_email/$", views.send_patch_email), re_path(r"^(\d+)/reports/authorstats/$", reports.authorstats), From 9c2e63d83ea6be4cd8554016980aee1b1e798ad5 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sun, 30 Nov 2025 23:39:31 +0100 Subject: [PATCH 09/38] Don't allow global search of users without login anymore Apparently login requirement for the user lookup was added to make scraping harder. It doesn't seem like the most interesting data honestly, but this adds the requirement back for global user search. Instead it will allow anonymous searching of users involved in a certain commitfest. Those you could scrape from the commitfest page anyway. Finally this adds a logged in requirement for the global search page. Otherwise people will run into the same problem there as they did on the commitfest page previously: the search box would not autocomplete without giving feedback as to why to the user. Since the global search page is only really useful for power users, having it require a login seems fine. --- pgcommitfest/commitfest/forms.py | 11 +- pgcommitfest/commitfest/lookups.py | 20 +- pgcommitfest/commitfest/tests/conftest.py | 136 ++++++++++++ pgcommitfest/commitfest/tests/test_apiv1.py | 50 +---- pgcommitfest/commitfest/tests/test_lookups.py | 208 ++++++++++++++++++ pgcommitfest/commitfest/views.py | 11 +- pgcommitfest/test_settings.py | 7 + pyproject.toml | 2 +- 8 files changed, 390 insertions(+), 55 deletions(-) create mode 100644 pgcommitfest/commitfest/tests/conftest.py create mode 100644 pgcommitfest/commitfest/tests/test_lookups.py create mode 100644 pgcommitfest/test_settings.py diff --git a/pgcommitfest/commitfest/forms.py b/pgcommitfest/commitfest/forms.py index 2ec9fc2c..b7f9ef9d 100644 --- a/pgcommitfest/commitfest/forms.py +++ b/pgcommitfest/commitfest/forms.py @@ -24,8 +24,17 @@ class CommitFestFilterForm(forms.Form): reviewer = forms.ChoiceField(required=False, label="Reviewer (type to search)") sortkey = forms.IntegerField(required=False) - def __init__(self, data, *args, **kwargs): + def __init__(self, data, commitfest=None, *args, **kwargs): super(CommitFestFilterForm, self).__init__(data, *args, **kwargs) + self.commitfest = commitfest + + # Update selectize_fields with cf parameter if commitfest is provided + if commitfest: + self.selectize_fields = { + "author": f"/lookups/user?cf={commitfest.id}", + "reviewer": f"/lookups/user?cf={commitfest.id}", + "tag": None, + } self.fields["sortkey"].widget = forms.HiddenInput() diff --git a/pgcommitfest/commitfest/lookups.py b/pgcommitfest/commitfest/lookups.py index 3765ce47..b97c66d7 100644 --- a/pgcommitfest/commitfest/lookups.py +++ b/pgcommitfest/commitfest/lookups.py @@ -1,15 +1,18 @@ from django.contrib.auth.models import User from django.db.models import Q -from django.http import Http404, HttpResponse +from django.http import Http404, HttpResponse, HttpResponseForbidden import json def userlookup(request): query = request.GET.get("query", None) + cf = request.GET.get("cf", None) + if not query: raise Http404() + # Start with base filters for active users matching the query users = User.objects.filter( Q(is_active=True), Q(username__icontains=query) @@ -17,6 +20,21 @@ def userlookup(request): | Q(last_name__icontains=query), ) + # If no commitfest filter is provided, require login + if not cf: + if not request.user.is_authenticated: + return HttpResponseForbidden( + "Login required when not filtering by commitfest" + ) + else: + # Filter users to only those who have participated in the specified commitfest + # This includes authors, reviewers, and committers of patches in that commitfest + users = users.filter( + Q(patch_author__commitfests__id=cf) + | Q(patch_reviewer__commitfests__id=cf) + | Q(committer__patch__commitfests__id=cf) + ).distinct() + return HttpResponse( json.dumps( { diff --git a/pgcommitfest/commitfest/tests/conftest.py b/pgcommitfest/commitfest/tests/conftest.py new file mode 100644 index 00000000..f05f570a --- /dev/null +++ b/pgcommitfest/commitfest/tests/conftest.py @@ -0,0 +1,136 @@ +"""Shared test fixtures for commitfest tests.""" + +from django.contrib.auth.models import User + +from datetime import date + +import pytest + +from pgcommitfest.commitfest.models import CommitFest + + +@pytest.fixture +def alice(): + """Create test user Alice.""" + return User.objects.create_user( + username="alice", + first_name="Alice", + last_name="Anderson", + email="alice@example.com", + ) + + +@pytest.fixture +def bob(): + """Create test user Bob.""" + return User.objects.create_user( + username="bob", + first_name="Bob", + last_name="Brown", + email="bob@example.com", + ) + + +@pytest.fixture +def charlie(): + """Create test user Charlie.""" + return User.objects.create_user( + username="charlie", + first_name="Charlie", + last_name="Chen", + email="charlie@example.com", + ) + + +@pytest.fixture +def dave(): + """Create test user Dave.""" + return User.objects.create_user( + username="dave", + first_name="Dave", + last_name="Davis", + email="dave@example.com", + ) + + +@pytest.fixture +def users(alice, bob, charlie, dave): + """Create all test users and return as a dictionary.""" + return { + "alice": alice, + "bob": bob, + "charlie": charlie, + "dave": dave, + } + + +@pytest.fixture +def open_cf(): + """Create an open commitfest.""" + return CommitFest.objects.create( + name="2025-01", + status=CommitFest.STATUS_OPEN, + startdate=date(2025, 1, 1), + enddate=date(2025, 1, 31), + draft=False, + ) + + +@pytest.fixture +def in_progress_cf(): + """Create an in-progress commitfest.""" + return CommitFest.objects.create( + name="2024-11", + status=CommitFest.STATUS_INPROGRESS, + startdate=date(2024, 11, 1), + enddate=date(2024, 11, 30), + draft=False, + ) + + +@pytest.fixture +def recent_closed_cf(): + """Create a recently closed commitfest.""" + return CommitFest.objects.create( + name="2024-09", + status=CommitFest.STATUS_CLOSED, + startdate=date(2024, 9, 1), + enddate=date(2024, 9, 30), + draft=False, + ) + + +@pytest.fixture +def old_closed_cf(): + """Create an old closed commitfest.""" + return CommitFest.objects.create( + name="2024-07", + status=CommitFest.STATUS_CLOSED, + startdate=date(2024, 7, 1), + enddate=date(2024, 7, 31), + draft=False, + ) + + +@pytest.fixture +def draft_cf(): + """Create a draft commitfest.""" + return CommitFest.objects.create( + name="2025-03-draft", + status=CommitFest.STATUS_OPEN, + startdate=date(2025, 3, 1), + enddate=date(2025, 3, 31), + draft=True, + ) + + +@pytest.fixture +def commitfests(open_cf, in_progress_cf, recent_closed_cf, old_closed_cf, draft_cf): + """Create all test commitfests and return as a dictionary.""" + return { + "open": open_cf, + "in_progress": in_progress_cf, + "recent_previous": recent_closed_cf, + "old_previous": old_closed_cf, + "draft": draft_cf, + } diff --git a/pgcommitfest/commitfest/tests/test_apiv1.py b/pgcommitfest/commitfest/tests/test_apiv1.py index 196d03c0..f3b94595 100644 --- a/pgcommitfest/commitfest/tests/test_apiv1.py +++ b/pgcommitfest/commitfest/tests/test_apiv1.py @@ -1,61 +1,13 @@ -from django.test import override_settings - import json -from datetime import date import pytest -from pgcommitfest.commitfest.models import CommitFest - pytestmark = pytest.mark.django_db -@pytest.fixture -def commitfests(): - """Create test commitfests with various statuses.""" - return { - "open": CommitFest.objects.create( - name="2025-01", - status=CommitFest.STATUS_OPEN, - startdate=date(2025, 1, 1), - enddate=date(2025, 1, 31), - draft=False, - ), - "in_progress": CommitFest.objects.create( - name="2024-11", - status=CommitFest.STATUS_INPROGRESS, - startdate=date(2024, 11, 1), - enddate=date(2024, 11, 30), - draft=False, - ), - "recent_previous": CommitFest.objects.create( - name="2024-09", - status=CommitFest.STATUS_CLOSED, - startdate=date(2024, 9, 1), - enddate=date(2024, 9, 30), - draft=False, - ), - "old_previous": CommitFest.objects.create( - name="2024-07", - status=CommitFest.STATUS_CLOSED, - startdate=date(2024, 7, 1), - enddate=date(2024, 7, 31), - draft=False, - ), - "draft": CommitFest.objects.create( - name="2025-03-draft", - status=CommitFest.STATUS_OPEN, - startdate=date(2025, 3, 1), - enddate=date(2025, 3, 31), - draft=True, - ), - } - - def test_needs_ci_endpoint(client, commitfests): """Test the /api/v1/commitfests/needs_ci endpoint returns correct data.""" - with override_settings(AUTO_CREATE_COMMITFESTS=False): - response = client.get("/api/v1/commitfests/needs_ci") + response = client.get("/api/v1/commitfests/needs_ci") # Check response metadata assert response.status_code == 200 diff --git a/pgcommitfest/commitfest/tests/test_lookups.py b/pgcommitfest/commitfest/tests/test_lookups.py new file mode 100644 index 00000000..8a514211 --- /dev/null +++ b/pgcommitfest/commitfest/tests/test_lookups.py @@ -0,0 +1,208 @@ +import json +from datetime import datetime + +import pytest + +from pgcommitfest.commitfest.models import Committer, Patch, PatchOnCommitFest, Topic + +pytestmark = pytest.mark.django_db + + +@pytest.fixture +def topic(): + """Create a test topic.""" + return Topic.objects.create(topic="General") + + +@pytest.fixture +def patches_with_users(users, open_cf, topic): + """Create patches with authors and reviewers in a commitfest.""" + # Alice is an author on patch 1 + patch1 = Patch.objects.create(name="Test Patch 1", topic=topic) + patch1.authors.add(users["alice"]) + PatchOnCommitFest.objects.create( + patch=patch1, commitfest=open_cf, enterdate=datetime.now() + ) + + # Bob is a reviewer on patch 2 + patch2 = Patch.objects.create(name="Test Patch 2", topic=topic) + patch2.reviewers.add(users["bob"]) + PatchOnCommitFest.objects.create( + patch=patch2, commitfest=open_cf, enterdate=datetime.now() + ) + + # Dave is a committer on patch 3 + dave_committer = Committer.objects.create(user=users["dave"]) + patch3 = Patch.objects.create( + name="Test Patch 3", topic=topic, committer=dave_committer + ) + PatchOnCommitFest.objects.create( + patch=patch3, commitfest=open_cf, enterdate=datetime.now() + ) + + # Charlie has no involvement in this commitfest + return {"patch1": patch1, "patch2": patch2, "patch3": patch3} + + +def test_userlookup_without_cf_requires_login(client, alice): + """Test that userlookup without cf parameter requires login.""" + response = client.get("/lookups/user/", {"query": "alice"}) + + assert response.status_code == 403 + assert b"Login required" in response.content + + +def test_userlookup_without_cf_works_when_logged_in(client, alice, bob): + """Test that userlookup without cf parameter works when logged in.""" + client.force_login(bob) + response = client.get("/lookups/user/", {"query": "alice"}) + + assert response.status_code == 200 + assert json.loads(response.content) == { + "values": [ + { + "id": alice.id, + "value": "Alice Anderson (alice)", + } + ] + } + + +def test_userlookup_with_cf_no_login_required( + client, alice, open_cf, patches_with_users +): + """Test that userlookup with cf parameter works without login.""" + response = client.get("/lookups/user/", {"query": "alice", "cf": open_cf.id}) + + assert response.status_code == 200 + assert json.loads(response.content) == { + "values": [ + { + "id": alice.id, + "value": "Alice Anderson (alice)", + } + ] + } + + +def test_userlookup_with_cf_filters_to_commitfest_participants( + client, alice, bob, dave, open_cf, patches_with_users +): + """Test that userlookup with cf parameter only returns users in that commitfest.""" + # Search for all users with 'a' in their name + response = client.get("/lookups/user/", {"query": "a", "cf": open_cf.id}) + + assert response.status_code == 200 + # Should return Alice and Dave (both involved) but not Charlie (has 'a' but not involved) + # Results are returned in order by user ID + data = json.loads(response.content) + # Sort by id to ensure consistent ordering + data["values"].sort(key=lambda x: x["id"]) + expected_values = [ + { + "id": alice.id, + "value": "Alice Anderson (alice)", + }, + { + "id": dave.id, + "value": "Dave Davis (dave)", + }, + ] + expected_values.sort(key=lambda x: x["id"]) + assert data == {"values": expected_values} + + +def test_userlookup_with_cf_includes_reviewers( + client, bob, open_cf, patches_with_users +): + """Test that userlookup with cf parameter includes reviewers.""" + response = client.get("/lookups/user/", {"query": "bob", "cf": open_cf.id}) + + assert response.status_code == 200 + assert json.loads(response.content) == { + "values": [ + { + "id": bob.id, + "value": "Bob Brown (bob)", + } + ] + } + + +def test_userlookup_with_cf_includes_committers( + client, dave, open_cf, patches_with_users +): + """Test that userlookup with cf parameter includes committers.""" + response = client.get("/lookups/user/", {"query": "dave", "cf": open_cf.id}) + + assert response.status_code == 200 + assert json.loads(response.content) == { + "values": [ + { + "id": dave.id, + "value": "Dave Davis (dave)", + } + ] + } + + +def test_userlookup_excludes_uninvolved_users( + client, charlie, open_cf, patches_with_users +): + """Test that users not involved in the commitfest are excluded.""" + response = client.get("/lookups/user/", {"query": "charlie", "cf": open_cf.id}) + + assert response.status_code == 200 + assert json.loads(response.content) == {"values": []} + + +def test_userlookup_requires_query_parameter(client, commitfests): + """Test that userlookup returns 404 without query parameter.""" + response = client.get("/lookups/user/") + + assert response.status_code == 404 + + +def test_userlookup_searches_first_name(client, alice, open_cf, patches_with_users): + """Test that userlookup searches by first name.""" + response = client.get("/lookups/user/", {"query": "Alice", "cf": open_cf.id}) + + assert response.status_code == 200 + assert json.loads(response.content) == { + "values": [ + { + "id": alice.id, + "value": "Alice Anderson (alice)", + } + ] + } + + +def test_userlookup_searches_last_name(client, bob, open_cf, patches_with_users): + """Test that userlookup searches by last name.""" + response = client.get("/lookups/user/", {"query": "Brown", "cf": open_cf.id}) + + assert response.status_code == 200 + assert json.loads(response.content) == { + "values": [ + { + "id": bob.id, + "value": "Bob Brown (bob)", + } + ] + } + + +def test_userlookup_case_insensitive(client, alice, open_cf, patches_with_users): + """Test that userlookup is case insensitive.""" + response = client.get("/lookups/user/", {"query": "ALICE", "cf": open_cf.id}) + + assert response.status_code == 200 + assert json.loads(response.content) == { + "values": [ + { + "id": alice.id, + "value": "Alice Anderson (alice)", + } + ] + } diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 44057da9..12cd8a6f 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -94,7 +94,7 @@ def home(request): # Use existing cfs data instead of querying again cf = cfs.get("in_progress") or cfs.get("open") - form = CommitFestFilterForm(request.GET) + form = CommitFestFilterForm(request.GET, commitfest=cf) patch_list = patchlist(request, cf, personalized=True) if patch_list.redirect: @@ -627,7 +627,7 @@ def commitfest(request, cfid): # Generates a fairly expensive query, which we shouldn't do unless # the user is logged in. XXX: Figure out how to avoid doing that.. - form = CommitFestFilterForm(request.GET) + form = CommitFestFilterForm(request.GET, commitfest=cf) return render( request, @@ -687,6 +687,11 @@ def patches_by_messageid(messageid): ) +# We require login for this page primarily so that the author/reviewer filter +# boxes can always be searched. Since searching for users outside of a +# commitfest requires users to be logged in to not make the data too easy to +# scrape. +@login_required def global_search(request): if "searchterm" not in request.GET: return HttpResponseRedirect("/") @@ -816,7 +821,7 @@ def global_search(request): patch = patches[0] return HttpResponseRedirect(f"/patch/{patch.id}/") - # Use the existing filter form + # Use the existing filter form (no cf parameter, will require login for user lookups) form = CommitFestFilterForm(request.GET) # Get user profile for timestamp preferences diff --git a/pgcommitfest/test_settings.py b/pgcommitfest/test_settings.py new file mode 100644 index 00000000..7fd6341f --- /dev/null +++ b/pgcommitfest/test_settings.py @@ -0,0 +1,7 @@ +"""Test settings for pgcommitfest.""" + +from pgcommitfest.settings import * # noqa: F403 + +# Disable automatic creation of commitfests during tests +# Tests should explicitly create the commitfests they need +AUTO_CREATE_COMMITFESTS = False diff --git a/pyproject.toml b/pyproject.toml index 53551b67..411d4075 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -51,7 +51,7 @@ section-order = [ django = ["django"] [tool.pytest.ini_options] -DJANGO_SETTINGS_MODULE = "pgcommitfest.settings" +DJANGO_SETTINGS_MODULE = "pgcommitfest.test_settings" python_files = ["tests.py", "test_*.py", "*_tests.py"] testpaths = ["pgcommitfest"] addopts = [ From 115fa5ccb8781df436b534a1e8574bafc5a0f529 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sun, 30 Nov 2025 23:53:48 +0100 Subject: [PATCH 10/38] Actually test that first name search works --- pgcommitfest/commitfest/tests/conftest.py | 2 +- pgcommitfest/commitfest/tests/test_lookups.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pgcommitfest/commitfest/tests/conftest.py b/pgcommitfest/commitfest/tests/conftest.py index f05f570a..9ce147e6 100644 --- a/pgcommitfest/commitfest/tests/conftest.py +++ b/pgcommitfest/commitfest/tests/conftest.py @@ -24,7 +24,7 @@ def alice(): def bob(): """Create test user Bob.""" return User.objects.create_user( - username="bob", + username="b", first_name="Bob", last_name="Brown", email="bob@example.com", diff --git a/pgcommitfest/commitfest/tests/test_lookups.py b/pgcommitfest/commitfest/tests/test_lookups.py index 8a514211..3dbcc3a6 100644 --- a/pgcommitfest/commitfest/tests/test_lookups.py +++ b/pgcommitfest/commitfest/tests/test_lookups.py @@ -123,7 +123,7 @@ def test_userlookup_with_cf_includes_reviewers( "values": [ { "id": bob.id, - "value": "Bob Brown (bob)", + "value": "Bob Brown (b)", } ] } @@ -163,16 +163,16 @@ def test_userlookup_requires_query_parameter(client, commitfests): assert response.status_code == 404 -def test_userlookup_searches_first_name(client, alice, open_cf, patches_with_users): +def test_userlookup_searches_first_name(client, bob, open_cf, patches_with_users): """Test that userlookup searches by first name.""" - response = client.get("/lookups/user/", {"query": "Alice", "cf": open_cf.id}) + response = client.get("/lookups/user/", {"query": "Bob", "cf": open_cf.id}) assert response.status_code == 200 assert json.loads(response.content) == { "values": [ { - "id": alice.id, - "value": "Alice Anderson (alice)", + "id": bob.id, + "value": "Bob Brown (b)", } ] } @@ -187,7 +187,7 @@ def test_userlookup_searches_last_name(client, bob, open_cf, patches_with_users) "values": [ { "id": bob.id, - "value": "Bob Brown (bob)", + "value": "Bob Brown (b)", } ] } From 938e7af23c4afd0112573068302b34773ebe46ad Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 1 Dec 2025 00:02:06 +0100 Subject: [PATCH 11/38] Deploy v* tags to prod --- .github/workflows/deploy.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deploy.yaml b/.github/workflows/deploy.yaml index bd660ac8..810ed81a 100644 --- a/.github/workflows/deploy.yaml +++ b/.github/workflows/deploy.yaml @@ -11,7 +11,7 @@ on: jobs: deployment: runs-on: ubuntu-latest - environment: ${{ startsWith(github.ref, 'refs/tags/') && 'main' || github.ref_name }} + environment: ${{ startsWith(github.ref, 'refs/tags/') && 'prod' || github.ref_name }} steps: - name: Trigger deploy run: | From 01c1e7e743843d5663a45915a5e3646dd30e73c0 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 2 Dec 2025 00:23:09 +0100 Subject: [PATCH 12/38] Try to fix missing updates Somehow new emails on a specific patch in production were not being detected. Maybe this will fix it. Patch in question: https://commitfest.postgresql.org/patch/4351/ --- pgcommitfest/commitfest/ajax.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pgcommitfest/commitfest/ajax.py b/pgcommitfest/commitfest/ajax.py index 329a83f9..077a0bad 100644 --- a/pgcommitfest/commitfest/ajax.py +++ b/pgcommitfest/commitfest/ajax.py @@ -114,11 +114,6 @@ def refresh_single_thread(thread): ) if thread.latestmsgid != r[-1]["msgid"]: # There is now a newer mail in the thread! - thread.latestmsgid = r[-1]["msgid"] - thread.latestmessage = r[-1]["date"] - thread.latestauthor = r[-1]["from"] - thread.latestsubject = r[-1]["subj"] - thread.save() parse_and_add_attachments(r, thread) # Potentially update the last mail date - if there wasn't already a mail on each patch # from a *different* thread that had an earlier date. @@ -126,6 +121,16 @@ def refresh_single_thread(thread): p.lastmail = thread.latestmessage p.save() + # Finally, we update the thread entry itself. We should only do that at + # the end, in case any of the previous steps fail. Then we'll retry on + # the next run. We could also do this in a transaction, but that would + # mean we lock a bunch of rows for potentially a long time. + thread.latestmsgid = r[-1]["msgid"] + thread.latestmessage = r[-1]["date"] + thread.latestauthor = r[-1]["from"] + thread.latestsubject = r[-1]["subj"] + thread.save() + @transaction.atomic def annotateMessage(request): From c7088f9f859fdbac5d026199306be192293b1a8a Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 2 Dec 2025 00:43:10 +0100 Subject: [PATCH 13/38] Add more fields to the MailThread admin overview --- pgcommitfest/commitfest/admin.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pgcommitfest/commitfest/admin.py b/pgcommitfest/commitfest/admin.py index 90d54c4a..2982c223 100644 --- a/pgcommitfest/commitfest/admin.py +++ b/pgcommitfest/commitfest/admin.py @@ -32,6 +32,16 @@ class PatchAdmin(admin.ModelAdmin): list_display = ("name",) +class MailThreadAdmin(admin.ModelAdmin): + list_display = ( + "messageid", + "subject", + "firstmessage", + "latestmsgid", + "latestmessage", + ) + + class MailThreadAttachmentAdmin(admin.ModelAdmin): list_display = ( "date", @@ -68,5 +78,5 @@ class TagAdmin(admin.ModelAdmin): admin.site.register(CfbotBranch) admin.site.register(CfbotTask) -admin.site.register(MailThread) +admin.site.register(MailThread, MailThreadAdmin) admin.site.register(MailThreadAttachment, MailThreadAttachmentAdmin) From 5389294b7c3c75f5b4e6de7066f28244ee72305f Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Thu, 11 Dec 2025 13:27:40 +0100 Subject: [PATCH 14/38] Don't send heavy queries anymore --- pgcommitfest/commitfest/lookups.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/pgcommitfest/commitfest/lookups.py b/pgcommitfest/commitfest/lookups.py index b97c66d7..af7a0712 100644 --- a/pgcommitfest/commitfest/lookups.py +++ b/pgcommitfest/commitfest/lookups.py @@ -20,20 +20,23 @@ def userlookup(request): | Q(last_name__icontains=query), ) + if not request.user.is_authenticated: + return HttpResponseForbidden("Login required when not filtering by commitfest") + _ = cf # If no commitfest filter is provided, require login - if not cf: - if not request.user.is_authenticated: - return HttpResponseForbidden( - "Login required when not filtering by commitfest" - ) - else: - # Filter users to only those who have participated in the specified commitfest - # This includes authors, reviewers, and committers of patches in that commitfest - users = users.filter( - Q(patch_author__commitfests__id=cf) - | Q(patch_reviewer__commitfests__id=cf) - | Q(committer__patch__commitfests__id=cf) - ).distinct() + # if not cf: + # if not request.user.is_authenticated: + # return HttpResponseForbidden( + # "Login required when not filtering by commitfest" + # ) + # else: + # # Filter users to only those who have participated in the specified commitfest + # # This includes authors, reviewers, and committers of patches in that commitfest + # users = users.filter( + # Q(patch_author__commitfests__id=cf) + # | Q(patch_reviewer__commitfests__id=cf) + # | Q(committer__patch__commitfests__id=cf) + # ).distinct() return HttpResponse( json.dumps( From bb3e46fcdf80013a09cadb98f2bb4d1c6d52298f Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sat, 3 Jan 2026 11:28:01 +0100 Subject: [PATCH 15/38] Fix bug where latest email timestamp of patch was outdated --- pgcommitfest/commitfest/ajax.py | 5 +- .../commitfest/tests/test_refresh_thread.py | 52 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 pgcommitfest/commitfest/tests/test_refresh_thread.py diff --git a/pgcommitfest/commitfest/ajax.py b/pgcommitfest/commitfest/ajax.py index 077a0bad..8137dfdf 100644 --- a/pgcommitfest/commitfest/ajax.py +++ b/pgcommitfest/commitfest/ajax.py @@ -117,8 +117,9 @@ def refresh_single_thread(thread): parse_and_add_attachments(r, thread) # Potentially update the last mail date - if there wasn't already a mail on each patch # from a *different* thread that had an earlier date. - for p in thread.patches.filter(lastmail__lt=thread.latestmessage): - p.lastmail = thread.latestmessage + new_latestmessage = r[-1]["date"] + for p in thread.patches.filter(lastmail__lt=new_latestmessage): + p.lastmail = new_latestmessage p.save() # Finally, we update the thread entry itself. We should only do that at diff --git a/pgcommitfest/commitfest/tests/test_refresh_thread.py b/pgcommitfest/commitfest/tests/test_refresh_thread.py new file mode 100644 index 00000000..348a8a70 --- /dev/null +++ b/pgcommitfest/commitfest/tests/test_refresh_thread.py @@ -0,0 +1,52 @@ +from datetime import datetime +from unittest.mock import patch + +import pytest + +from pgcommitfest.commitfest.ajax import refresh_single_thread +from pgcommitfest.commitfest.models import MailThread, Patch, Topic + +pytestmark = pytest.mark.django_db + + +def test_refresh_single_thread_updates_patch_lastmail(): + """Regression test: patch.lastmail should get the new date, not the old one.""" + old_date = datetime(2024, 1, 1) + new_date = datetime(2024, 6, 15) + + topic = Topic.objects.create(topic="Test") + thread = MailThread.objects.create( + messageid="old@example.com", + subject="Test", + firstmessage=old_date, + firstauthor="a@example.com", + latestmessage=old_date, + latestauthor="a@example.com", + latestsubject="Test", + latestmsgid="old@example.com", + ) + p = Patch.objects.create(name="Test Patch", topic=topic, lastmail=old_date) + p.mailthread_set.add(thread) + + api_response = [ + { + "msgid": "old@example.com", + "date": old_date, + "from": "a", + "subj": "T", + "atts": [], + }, + { + "msgid": "new@example.com", + "date": new_date, + "from": "b", + "subj": "T", + "atts": [], + }, + ] + + with patch("pgcommitfest.commitfest.ajax._archivesAPI", return_value=api_response): + refresh_single_thread(thread) + + p.refresh_from_db() + assert p.lastmail == new_date From 55220c0447301aafdd40d07262da41b7d1fb932a Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sat, 3 Jan 2026 11:08:40 +0100 Subject: [PATCH 16/38] Use cheaper filter --- pgcommitfest/commitfest/lookups.py | 43 +++++++++++++++++++----------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/pgcommitfest/commitfest/lookups.py b/pgcommitfest/commitfest/lookups.py index af7a0712..e285c883 100644 --- a/pgcommitfest/commitfest/lookups.py +++ b/pgcommitfest/commitfest/lookups.py @@ -1,4 +1,5 @@ from django.contrib.auth.models import User +from django.db import connection from django.db.models import Q from django.http import Http404, HttpResponse, HttpResponseForbidden @@ -20,23 +21,33 @@ def userlookup(request): | Q(last_name__icontains=query), ) - if not request.user.is_authenticated: - return HttpResponseForbidden("Login required when not filtering by commitfest") - _ = cf # If no commitfest filter is provided, require login - # if not cf: - # if not request.user.is_authenticated: - # return HttpResponseForbidden( - # "Login required when not filtering by commitfest" - # ) - # else: - # # Filter users to only those who have participated in the specified commitfest - # # This includes authors, reviewers, and committers of patches in that commitfest - # users = users.filter( - # Q(patch_author__commitfests__id=cf) - # | Q(patch_reviewer__commitfests__id=cf) - # | Q(committer__patch__commitfests__id=cf) - # ).distinct() + if not cf: + if not request.user.is_authenticated: + return HttpResponseForbidden( + "Login required when not filtering by commitfest" + ) + else: + # Filter users to only those who have participated in the specified commitfest. + with connection.cursor() as cursor: + cursor.execute( + """ + SELECT cpa.user_id FROM commitfest_patch_authors cpa + INNER JOIN commitfest_patchoncommitfest poc ON poc.patch_id = cpa.patch_id + WHERE poc.commitfest_id = %(cf)s + UNION + SELECT cpr.user_id FROM commitfest_patch_reviewers cpr + INNER JOIN commitfest_patchoncommitfest poc ON poc.patch_id = cpr.patch_id + WHERE poc.commitfest_id = %(cf)s + UNION + SELECT p.committer_id FROM commitfest_patch p + INNER JOIN commitfest_patchoncommitfest poc ON poc.patch_id = p.id + WHERE poc.commitfest_id = %(cf)s AND p.committer_id IS NOT NULL + """, + {"cf": cf}, + ) + user_ids = [row[0] for row in cursor.fetchall()] + users = users.filter(id__in=user_ids) return HttpResponse( json.dumps( From 238b4fe90ce7880fdc533a91f92c8274ef4e077a Mon Sep 17 00:00:00 2001 From: Aditya-Sarna <107273722+Aditya-Sarna@users.noreply.github.com> Date: Sun, 4 Jan 2026 19:50:43 +0530 Subject: [PATCH 17/38] Filters out 'Moved to other CF' patches from draft commitfest pages (#100) Reduces clutter on draft commitfest pages by hiding moved patches (including from the status summary at the top of the page). --------- Co-authored-by: Aditya Sarna Co-authored-by: Jelte Fennema-Nio --- pgcommitfest/commitfest/views.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 12cd8a6f..35d42ed1 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -407,6 +407,10 @@ def patchlist(request, cf, personalized=False): whereclauses.append("poc.status=ANY(%(openstatuses)s)") else: whereclauses.append("poc.commitfest_id=%(cid)s") + # Exclude "Moved to other CF" patches from draft commitfests + if cf.draft: + whereclauses.append("poc.status != %(status_moved)s") + whereparams["status_moved"] = PatchOnCommitFest.STATUS_MOVED if personalized: # For now we can just order by these names in descending order, because @@ -616,10 +620,13 @@ def commitfest(request, cfid): return patch_list.redirect # Generate patch status summary. + # Exclude "Moved to other CF" status from draft commitfests + status_filter = "AND poc.status != %(status_moved)s" if cf.draft else "" curs.execute( - "SELECT ps.status, ps.statusstring, count(*) FROM commitfest_patchoncommitfest poc INNER JOIN commitfest_patchstatus ps ON ps.status=poc.status WHERE commitfest_id=%(id)s GROUP BY ps.status ORDER BY ps.sortkey", + f"SELECT ps.status, ps.statusstring, count(*) FROM commitfest_patchoncommitfest poc INNER JOIN commitfest_patchstatus ps ON ps.status=poc.status WHERE commitfest_id=%(id)s {status_filter} GROUP BY ps.status ORDER BY ps.sortkey", { "id": cf.id, + "status_moved": PatchOnCommitFest.STATUS_MOVED, }, ) statussummary = curs.fetchall() From 19ee7d36180485debab004ae07af888ac5f795f6 Mon Sep 17 00:00:00 2001 From: Magnus Hagander Date: Sun, 4 Jan 2026 16:58:52 +0100 Subject: [PATCH 18/38] Give better error message when invalid data is passed to cauth in d param (#94) This imports the latest update of the cauth provider from upstream --- pgcommitfest/auth.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pgcommitfest/auth.py b/pgcommitfest/auth.py index c3756902..06cc609c 100644 --- a/pgcommitfest/auth.py +++ b/pgcommitfest/auth.py @@ -211,7 +211,10 @@ def auth_receive(request): # Finally, check of we have a data package that tells us where to # redirect the user. if 'd' in data: - (nonces, datas, tags) = data['d'][0].split('$') + splitdata = data['d'][0].split('$') + if len(splitdata) != 3: + return HttpResponse("Invalid login pass-through data received, likely because of an old link. Please try again.") + (nonces, datas, tags) = splitdata decryptor = AES.new( SHA256.new(settings.SECRET_KEY.encode('ascii')).digest()[:32], AES.MODE_SIV, From 33d014a48fcd94c3fc52a6140186e648ec09c9dd Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Thu, 7 Aug 2025 21:31:57 +0200 Subject: [PATCH 19/38] Migrate topics to tags --- pgcommitfest/commitfest/forms.py | 2 + .../migrations/0016_migrate_topics_to_tags.py | 82 +++++++++++++++++++ .../migrations/0017_make_topic_optional.py | 23 ++++++ pgcommitfest/commitfest/models.py | 4 +- .../commitfest/templates/commitfest.html | 5 -- pgcommitfest/commitfest/templates/home.html | 4 +- pgcommitfest/commitfest/templates/patch.html | 4 - pgcommitfest/commitfest/tests/test_lookups.py | 18 ++-- .../commitfest/tests/test_refresh_thread.py | 5 +- pgcommitfest/commitfest/views.py | 14 ++-- 10 files changed, 125 insertions(+), 36 deletions(-) create mode 100644 pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py create mode 100644 pgcommitfest/commitfest/migrations/0017_make_topic_optional.py diff --git a/pgcommitfest/commitfest/forms.py b/pgcommitfest/commitfest/forms.py index b7f9ef9d..6321a5b5 100644 --- a/pgcommitfest/commitfest/forms.py +++ b/pgcommitfest/commitfest/forms.py @@ -95,6 +95,8 @@ class Meta: "modified", "lastmail", "subscribers", + # TODO: Remove once we fully get rid of topics + "topic", ) def __init__(self, *args, **kwargs): diff --git a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py new file mode 100644 index 00000000..b6af9b7f --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py @@ -0,0 +1,82 @@ +# Generated by Django 4.2.19 on 2025-08-07 17:25 + +from django.db import migrations + + +def create_missing_tags_and_map_topics(apps, schema_editor): + """ + Create missing tags and map existing topics to appropriate tags + """ + Tag = apps.get_model("commitfest", "Tag") + Patch = apps.get_model("commitfest", "Patch") + Topic = apps.get_model("commitfest", "Topic") + + # Create missing tags + Tag.objects.get_or_create( + name="SQL Commands", + defaults={ + "color": "#198754", + "description": "SQL command implementation and enhancement", + }, + ) + + Tag.objects.get_or_create( + name="System Administration", + defaults={ + "color": "#495057", + "description": "System administration and configuration", + }, + ) + + # Topic to Tag mapping + topic_tag_mapping = { + "Testing": "Testing", + "Refactoring": "Refactoring Only", + "Documentation": "Docs Only", + "Code Comments": "Comments Only", + "Bug Fixes": "Bugfix", + "Performance": "Performance", + "Security": "Security", + "Monitoring & Control": "Monitoring", + "Procedural Languages": "PL/pgSQL", + "Replication & Recovery": "Physical Replication", + "Clients": "libpq", + "SQL Commands": "SQL Commands", + "System Administration": "System Administration", + # 'Miscellaneous' and 'Server Features' are left untagged + } + + # Apply tags to existing patches based on their topics + for topic_name, tag_name in topic_tag_mapping.items(): + try: + topic = Topic.objects.get(topic=topic_name) + tag = Tag.objects.get(name=tag_name) + + # Get all patches with this topic + patches_with_topic = Patch.objects.filter(topic=topic) + + # Add the corresponding tag to each patch + for patch in patches_with_topic: + patch.tags.add(tag) + + print( + f"Mapped {patches_with_topic.count()} patches from topic '{topic_name}' to tag '{tag_name}'" + ) + + except Topic.DoesNotExist: + print(f"Topic '{topic_name}' not found, skipping...") + except Tag.DoesNotExist: + print(f"Tag '{tag_name}' not found, skipping...") + + +class Migration(migrations.Migration): + dependencies = [ + ("commitfest", "0015_cfbot_duplicate_task_fix"), + ] + + operations = [ + migrations.RunPython( + create_missing_tags_and_map_topics, + migrations.RunPython.noop, + ), + ] diff --git a/pgcommitfest/commitfest/migrations/0017_make_topic_optional.py b/pgcommitfest/commitfest/migrations/0017_make_topic_optional.py new file mode 100644 index 00000000..c0b3e059 --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0017_make_topic_optional.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.19 on 2026-01-04 16:09 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("commitfest", "0016_migrate_topics_to_tags"), + ] + + operations = [ + migrations.AlterField( + model_name="patch", + name="topic", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="commitfest.topic", + ), + ), + ] diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 4583cd79..26444990 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -347,7 +347,9 @@ class Patch(models.Model, DiffableModel): name = models.CharField( max_length=500, blank=False, null=False, verbose_name="Description" ) - topic = models.ForeignKey(Topic, blank=False, null=False, on_delete=models.CASCADE) + # Topic is deprecated, tags are used instead. For now this field is kept + # for debugging purposes in case of problems with the migration. + topic = models.ForeignKey(Topic, blank=True, null=True, on_delete=models.CASCADE) tags = models.ManyToManyField(Tag, related_name="patches", blank=True) # One patch can be in multiple commitfests, if it has history diff --git a/pgcommitfest/commitfest/templates/commitfest.html b/pgcommitfest/commitfest/templates/commitfest.html index ac1b778b..082eb72c 100644 --- a/pgcommitfest/commitfest/templates/commitfest.html +++ b/pgcommitfest/commitfest/templates/commitfest.html @@ -46,11 +46,6 @@

{{p.is_open|yesno:"Active patches,Closed patches"}}

{%endifchanged%} - {%if grouping%} - {%ifchanged p.topic%} - {{p.topic}} - {%endifchanged%} - {%endif%} {{p.name}} {{p.id}} diff --git a/pgcommitfest/commitfest/templates/home.html b/pgcommitfest/commitfest/templates/home.html index 80ff99ff..e83b2ab7 100644 --- a/pgcommitfest/commitfest/templates/home.html +++ b/pgcommitfest/commitfest/templates/home.html @@ -151,8 +151,8 @@

{%if user.is_authenticated%}Open patches you are subscribed to{%elif p.is_op {%endifchanged%} {%if grouping%} - {%ifchanged p.topic%} - {{p.topic}} + {%ifchanged p.group_name%} + {{p.group_name}} {%endifchanged%} {%endif%} diff --git a/pgcommitfest/commitfest/templates/patch.html b/pgcommitfest/commitfest/templates/patch.html index 5a9ac905..03cafb5c 100644 --- a/pgcommitfest/commitfest/templates/patch.html +++ b/pgcommitfest/commitfest/templates/patch.html @@ -75,10 +75,6 @@ Unknown {%endif%} - - Topic - {{patch.topic}} - Tags diff --git a/pgcommitfest/commitfest/tests/test_lookups.py b/pgcommitfest/commitfest/tests/test_lookups.py index 3dbcc3a6..816a0462 100644 --- a/pgcommitfest/commitfest/tests/test_lookups.py +++ b/pgcommitfest/commitfest/tests/test_lookups.py @@ -3,29 +3,23 @@ import pytest -from pgcommitfest.commitfest.models import Committer, Patch, PatchOnCommitFest, Topic +from pgcommitfest.commitfest.models import Committer, Patch, PatchOnCommitFest pytestmark = pytest.mark.django_db @pytest.fixture -def topic(): - """Create a test topic.""" - return Topic.objects.create(topic="General") - - -@pytest.fixture -def patches_with_users(users, open_cf, topic): +def patches_with_users(users, open_cf): """Create patches with authors and reviewers in a commitfest.""" # Alice is an author on patch 1 - patch1 = Patch.objects.create(name="Test Patch 1", topic=topic) + patch1 = Patch.objects.create(name="Test Patch 1") patch1.authors.add(users["alice"]) PatchOnCommitFest.objects.create( patch=patch1, commitfest=open_cf, enterdate=datetime.now() ) # Bob is a reviewer on patch 2 - patch2 = Patch.objects.create(name="Test Patch 2", topic=topic) + patch2 = Patch.objects.create(name="Test Patch 2") patch2.reviewers.add(users["bob"]) PatchOnCommitFest.objects.create( patch=patch2, commitfest=open_cf, enterdate=datetime.now() @@ -33,9 +27,7 @@ def patches_with_users(users, open_cf, topic): # Dave is a committer on patch 3 dave_committer = Committer.objects.create(user=users["dave"]) - patch3 = Patch.objects.create( - name="Test Patch 3", topic=topic, committer=dave_committer - ) + patch3 = Patch.objects.create(name="Test Patch 3", committer=dave_committer) PatchOnCommitFest.objects.create( patch=patch3, commitfest=open_cf, enterdate=datetime.now() ) diff --git a/pgcommitfest/commitfest/tests/test_refresh_thread.py b/pgcommitfest/commitfest/tests/test_refresh_thread.py index 348a8a70..55d22a22 100644 --- a/pgcommitfest/commitfest/tests/test_refresh_thread.py +++ b/pgcommitfest/commitfest/tests/test_refresh_thread.py @@ -4,7 +4,7 @@ import pytest from pgcommitfest.commitfest.ajax import refresh_single_thread -from pgcommitfest.commitfest.models import MailThread, Patch, Topic +from pgcommitfest.commitfest.models import MailThread, Patch pytestmark = pytest.mark.django_db @@ -14,7 +14,6 @@ def test_refresh_single_thread_updates_patch_lastmail(): old_date = datetime(2024, 1, 1) new_date = datetime(2024, 6, 15) - topic = Topic.objects.create(topic="Test") thread = MailThread.objects.create( messageid="old@example.com", subject="Test", @@ -25,7 +24,7 @@ def test_refresh_single_thread_updates_patch_lastmail(): latestsubject="Test", latestmsgid="old@example.com", ) - p = Patch.objects.create(name="Test Patch", topic=topic, lastmail=old_date) + p = Patch.objects.create(name="Test Patch", lastmail=old_date) p.mailthread_set.add(thread) api_response = [ diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 35d42ed1..1f6a30d9 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -442,7 +442,7 @@ def patchlist(request, cf, personalized=False): ) THEN 'Patches that are ready for your review' ELSE 'Blocked on others' - END AS topic, + END AS group_name, cf.id AS cf_id, cf.name AS cf_name, cf.status AS cf_status, @@ -465,7 +465,7 @@ def patchlist(request, cf, personalized=False): joins_str = "INNER JOIN commitfest_commitfest cf ON poc.commitfest_id=cf.id" groupby_str = "cf.id," else: - columns_str = "t.topic as topic," + columns_str = "" joins_str = "" groupby_str = "" @@ -507,7 +507,7 @@ def patchlist(request, cf, personalized=False): orderby_str = "poc.commitfest_id DESC, lastmail" else: if personalized: - # First we sort by topic, to have the grouping work. + # First we sort by group_name, to have the grouping work. # Then we show non-failing patches first, and the ones that are # shortest failing we show first. We consider patches in a closed # commitfest, as if they are failing since that commitfest was @@ -516,7 +516,7 @@ def patchlist(request, cf, personalized=False): # progress" commitfest before ones in the "Open" commitfest. # And then to break ties, we put ones with the most recent email at # the top. - orderby_str = """topic DESC, + orderby_str = """group_name DESC, COALESCE( branch.failing_since, CASE WHEN cf.status = %(cf_closed_status)s @@ -526,7 +526,7 @@ def patchlist(request, cf, personalized=False): lastmail DESC""" whereparams["cf_closed_status"] = CommitFest.STATUS_CLOSED else: - orderby_str = "topic, created" + orderby_str = "created" sortkey = 0 if not has_filter and sortkey == 0 and request.GET: @@ -584,13 +584,12 @@ def patchlist(request, cf, personalized=False): ) FROM commitfest_patch p INNER JOIN commitfest_patchoncommitfest poc ON poc.patch_id=p.id -INNER JOIN commitfest_topic t ON t.id=p.topic_id {joins_str} LEFT JOIN auth_user committer ON committer.id=p.committer_id LEFT JOIN commitfest_targetversion v ON p.targetversion_id=v.id LEFT JOIN commitfest_cfbotbranch branch ON branch.patch_id=p.id WHERE {where_str} -GROUP BY p.id, poc.id, {groupby_str} committer.id, t.id, v.version, branch.patch_id +GROUP BY p.id, poc.id, {groupby_str} committer.id, v.version, branch.patch_id ORDER BY is_open DESC, {orderby_str}""", params, ) @@ -648,7 +647,6 @@ def commitfest(request, cfid): "all_tags": {t.id: t for t in Tag.objects.all()}, "has_filter": patch_list.has_filter, "title": f"{cf.title} ({cf.periodstring})", - "grouping": patch_list.sortkey == 0, "sortkey": patch_list.sortkey, "openpatchids": [p["id"] for p in patch_list.patches if p["is_open"]], "header_activity": "Activity log", From 1652933ea6e6e0cb3874b83046b011cd5e4723b5 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sun, 4 Jan 2026 17:43:08 +0100 Subject: [PATCH 20/38] Only migrate to tags on open commitfests --- .../migrations/0016_migrate_topics_to_tags.py | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py index b6af9b7f..eee3db7b 100644 --- a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py +++ b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py @@ -5,11 +5,17 @@ def create_missing_tags_and_map_topics(apps, schema_editor): """ - Create missing tags and map existing topics to appropriate tags + Create missing tags and map existing topics to appropriate tags. + Only applies to patches in active (open or in-progress) commitfests. """ Tag = apps.get_model("commitfest", "Tag") Patch = apps.get_model("commitfest", "Patch") Topic = apps.get_model("commitfest", "Topic") + PatchOnCommitFest = apps.get_model("commitfest", "PatchOnCommitFest") + + # CommitFest status constants (matching models.py) + STATUS_OPEN = 2 + STATUS_INPROGRESS = 3 # Create missing tags Tag.objects.get_or_create( @@ -46,14 +52,23 @@ def create_missing_tags_and_map_topics(apps, schema_editor): # 'Miscellaneous' and 'Server Features' are left untagged } + # Get patch IDs that are in active commitfests (open or in-progress) + active_patch_ids = set( + PatchOnCommitFest.objects.filter( + commitfest__status__in=[STATUS_OPEN, STATUS_INPROGRESS] + ).values_list("patch_id", flat=True) + ) + # Apply tags to existing patches based on their topics for topic_name, tag_name in topic_tag_mapping.items(): try: topic = Topic.objects.get(topic=topic_name) tag = Tag.objects.get(name=tag_name) - # Get all patches with this topic - patches_with_topic = Patch.objects.filter(topic=topic) + # Get patches with this topic that are in active commitfests + patches_with_topic = Patch.objects.filter( + topic=topic, id__in=active_patch_ids + ) # Add the corresponding tag to each patch for patch in patches_with_topic: From 795264fd6a83c129f1b226897f9930fa0a37147f Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sun, 4 Jan 2026 17:47:21 +0100 Subject: [PATCH 21/38] Update comment --- .../commitfest/migrations/0016_migrate_topics_to_tags.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py index eee3db7b..596f40aa 100644 --- a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py +++ b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py @@ -44,12 +44,15 @@ def create_missing_tags_and_map_topics(apps, schema_editor): "Performance": "Performance", "Security": "Security", "Monitoring & Control": "Monitoring", - "Procedural Languages": "PL/pgSQL", "Replication & Recovery": "Physical Replication", "Clients": "libpq", "SQL Commands": "SQL Commands", "System Administration": "System Administration", - # 'Miscellaneous' and 'Server Features' are left untagged + # 'Miscellaneous' and 'Server Features' are left untagged, because they + # are too vague. + # 'Procedural Languages' has no direct tag equivalent, because new + # there are tags per language. So there's no clear tag that should be + # chosen there. } # Get patch IDs that are in active commitfests (open or in-progress) From 96288e606d0b619cdefd31a7cffab8a90e4bc2d9 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sun, 4 Jan 2026 17:54:31 +0100 Subject: [PATCH 22/38] Try adding tags for all patches again --- .../migrations/0016_migrate_topics_to_tags.py | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py index 596f40aa..61f26dfc 100644 --- a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py +++ b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py @@ -6,16 +6,11 @@ def create_missing_tags_and_map_topics(apps, schema_editor): """ Create missing tags and map existing topics to appropriate tags. - Only applies to patches in active (open or in-progress) commitfests. """ Tag = apps.get_model("commitfest", "Tag") Patch = apps.get_model("commitfest", "Patch") Topic = apps.get_model("commitfest", "Topic") - PatchOnCommitFest = apps.get_model("commitfest", "PatchOnCommitFest") - - # CommitFest status constants (matching models.py) - STATUS_OPEN = 2 - STATUS_INPROGRESS = 3 + PatchTag = Patch.tags.through # Create missing tags Tag.objects.get_or_create( @@ -55,30 +50,25 @@ def create_missing_tags_and_map_topics(apps, schema_editor): # chosen there. } - # Get patch IDs that are in active commitfests (open or in-progress) - active_patch_ids = set( - PatchOnCommitFest.objects.filter( - commitfest__status__in=[STATUS_OPEN, STATUS_INPROGRESS] - ).values_list("patch_id", flat=True) - ) - # Apply tags to existing patches based on their topics for topic_name, tag_name in topic_tag_mapping.items(): try: topic = Topic.objects.get(topic=topic_name) tag = Tag.objects.get(name=tag_name) - # Get patches with this topic that are in active commitfests - patches_with_topic = Patch.objects.filter( - topic=topic, id__in=active_patch_ids + # Get all patch IDs with this topic + patch_ids = list( + Patch.objects.filter(topic=topic).values_list("id", flat=True) ) - # Add the corresponding tag to each patch - for patch in patches_with_topic: - patch.tags.add(tag) + # Bulk create the patch-tag relationships + PatchTag.objects.bulk_create( + [PatchTag(patch_id=pid, tag_id=tag.id) for pid in patch_ids], + ignore_conflicts=True, + ) print( - f"Mapped {patches_with_topic.count()} patches from topic '{topic_name}' to tag '{tag_name}'" + f"Mapped {len(patch_ids)} patches from topic '{topic_name}' to tag '{tag_name}'" ) except Topic.DoesNotExist: From 3e50ddce1fbddd49a731cb622a848e0a0183e917 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sun, 4 Jan 2026 18:02:23 +0100 Subject: [PATCH 23/38] Don't auto-migrate "Replication & Recovery" --- .../commitfest/migrations/0016_migrate_topics_to_tags.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py index 61f26dfc..0ecf3025 100644 --- a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py +++ b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py @@ -39,7 +39,6 @@ def create_missing_tags_and_map_topics(apps, schema_editor): "Performance": "Performance", "Security": "Security", "Monitoring & Control": "Monitoring", - "Replication & Recovery": "Physical Replication", "Clients": "libpq", "SQL Commands": "SQL Commands", "System Administration": "System Administration", @@ -47,7 +46,8 @@ def create_missing_tags_and_map_topics(apps, schema_editor): # are too vague. # 'Procedural Languages' has no direct tag equivalent, because new # there are tags per language. So there's no clear tag that should be - # chosen there. + # chosen there. Similar for 'Replication & Recovery', which also has + # separate tags now for logical and physical replication. } # Apply tags to existing patches based on their topics From bc839b61f90b165c5657b0c4079f0ea3cab58e7d Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sun, 4 Jan 2026 22:41:40 +0100 Subject: [PATCH 24/38] Update a few patch links to modern format --- pgcommitfest/commitfest/feeds.py | 7 +------ pgcommitfest/commitfest/templates/mail/patch_notify.txt | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/pgcommitfest/commitfest/feeds.py b/pgcommitfest/commitfest/feeds.py index 9aff9025..8a1250df 100644 --- a/pgcommitfest/commitfest/feeds.py +++ b/pgcommitfest/commitfest/feeds.py @@ -30,12 +30,7 @@ def item_description(self, item): ) def item_link(self, item): - if self.cfid: - return "https://commitfest.postgresql.org/{0}/{1}/".format( - self.cfid, item["patchid"] - ) - else: - return "https://commitfest.postgresql.org/{cfid}/{patchid}/".format(**item) + return "https://commitfest.postgresql.org/patch/{patchid}/".format(**item) def item_pubdate(self, item): return item["date"] diff --git a/pgcommitfest/commitfest/templates/mail/patch_notify.txt b/pgcommitfest/commitfest/templates/mail/patch_notify.txt index a00918af..3153e9d5 100644 --- a/pgcommitfest/commitfest/templates/mail/patch_notify.txt +++ b/pgcommitfest/commitfest/templates/mail/patch_notify.txt @@ -3,7 +3,7 @@ have received updates in the PostgreSQL commitfest app: {%for p in patches.values %} {{p.patch.name}} -https://commitfest.postgresql.org/{{p.patch.patchoncommitfest_set.all.0.commitfest.id}}/{{p.patch.id}}/ +https://commitfest.postgresql.org/patch/{{p.patch.id}}/ {%for h in p.entries%} * {{h.what}} by {{h.by_string}}{%endfor%} From 208d89b3c35e0033a9d12ed37d2cc022d18177dd Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 5 Jan 2026 09:18:26 +0100 Subject: [PATCH 25/38] More friendly closing of commitfests (#101) When a commitfest is closed authors currently need to move their patches. This is an attempt to "age out" patches without interest automatically. This has a few downsides though. For patches that have activity going on, this is just busywork for the author. And currently it's not even very clear that authors are expected to do this (i.e. it's only shown in the dashboard). So this PR tries to improve the situation in three ways: 1. Automatically move "active" patches to the next commitfest 2. Send email notifications to authors for patches that were not moved automatically 3. Show a warning on the patch page --- .../commitfest/fixtures/commitfest_data.json | 58 +++ pgcommitfest/commitfest/models.py | 134 +++++- .../commitfest/templates/commitfest.html | 6 + pgcommitfest/commitfest/templates/help.html | 5 + .../templates/mail/commitfest_closure.txt | 14 + pgcommitfest/commitfest/templates/patch.html | 7 + pgcommitfest/commitfest/tests/conftest.py | 7 +- .../tests/test_closure_notifications.py | 431 ++++++++++++++++++ .../tests/test_refresh_commitfests.py | 158 +++++++ pgcommitfest/commitfest/views.py | 2 + pgcommitfest/settings.py | 6 + pyproject.toml | 1 + 12 files changed, 822 insertions(+), 7 deletions(-) create mode 100644 pgcommitfest/commitfest/templates/mail/commitfest_closure.txt create mode 100644 pgcommitfest/commitfest/tests/test_closure_notifications.py create mode 100644 pgcommitfest/commitfest/tests/test_refresh_commitfests.py diff --git a/pgcommitfest/commitfest/fixtures/commitfest_data.json b/pgcommitfest/commitfest/fixtures/commitfest_data.json index 944ed3cb..eac5a329 100644 --- a/pgcommitfest/commitfest/fixtures/commitfest_data.json +++ b/pgcommitfest/commitfest/fixtures/commitfest_data.json @@ -1351,5 +1351,63 @@ "created": "2025-01-26T22:11:40.000", "modified": "2025-01-26T22:12:00.000" } +}, +{ + "model": "commitfest.patch", + "pk": 9, + "fields": { + "name": "Abandoned patch in closed commitfest", + "topic": 3, + "wikilink": "", + "gitlink": "", + "targetversion": null, + "committer": null, + "created": "2024-10-15T10:00:00", + "modified": "2024-10-15T10:00:00", + "lastmail": "2024-10-01T10:00:00", + "tags": [], + "authors": [], + "reviewers": [], + "subscribers": [], + "mailthread_set": [ + 9 + ] + } +}, +{ + "model": "commitfest.mailthread", + "pk": 9, + "fields": { + "messageid": "abandoned@message-09", + "subject": "Abandoned patch in closed commitfest", + "firstmessage": "2024-10-01T10:00:00", + "firstauthor": "test@test.com", + "latestmessage": "2024-10-01T10:00:00", + "latestauthor": "test@test.com", + "latestsubject": "Abandoned patch in closed commitfest", + "latestmsgid": "abandoned@message-09" + } +}, +{ + "model": "commitfest.patchoncommitfest", + "pk": 10, + "fields": { + "patch": 9, + "commitfest": 1, + "enterdate": "2024-10-15T10:00:00", + "leavedate": null, + "status": 1 + } +}, +{ + "model": "commitfest.patchhistory", + "pk": 23, + "fields": { + "patch": 9, + "date": "2024-10-15T10:00:00", + "by": 1, + "by_cfbot": false, + "what": "Created patch record" + } } ] diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 26444990..921fcf22 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -6,6 +6,7 @@ from datetime import datetime, timedelta, timezone +from pgcommitfest.mailqueue.util import send_template_mail from pgcommitfest.userprofile.models import UserProfile from .util import DiffableModel @@ -109,6 +110,119 @@ def to_json(self): "enddate": self.enddate.isoformat(), } + def _should_auto_move_patch(self, patch, current_date): + """Determine if a patch should be automatically moved to the next commitfest. + + A patch qualifies for auto-move if it both: + 1. Has had email activity within the configured number of days + 2. Hasn't been failing CI for longer than the configured threshold + """ + activity_cutoff = current_date - timedelta( + days=settings.AUTO_MOVE_EMAIL_ACTIVITY_DAYS + ) + failing_cutoff = current_date - timedelta( + days=settings.AUTO_MOVE_MAX_FAILING_DAYS + ) + + # Check for recent email activity + if not patch.lastmail or patch.lastmail < activity_cutoff: + return False + + # Check if CI has been failing too long + try: + cfbot_branch = patch.cfbot_branch + if ( + cfbot_branch.failing_since + and cfbot_branch.failing_since < failing_cutoff + ): + return False + except CfbotBranch.DoesNotExist: + # IF no CFBot data exists, the patch is probably very new (i.e. no + # CI run has ever taken place for it yet). So we auto-move it in + # that case. + pass + + return True + + def auto_move_active_patches(self): + """Automatically move active patches to the next commitfest. + + A patch is moved if it has recent email activity and hasn't been + failing CI for too long. + """ + current_date = datetime.now() + + # Get the next open commitfest (must exist, raises IndexError otherwise) + # For draft CFs, find the next draft CF + # For regular CFs, find the next regular CF by start date + if self.draft: + next_cf = CommitFest.objects.filter( + status=CommitFest.STATUS_OPEN, + draft=True, + startdate__gt=self.enddate, + ).order_by("startdate")[0] + else: + next_cf = CommitFest.objects.filter( + status=CommitFest.STATUS_OPEN, + draft=False, + startdate__gt=self.enddate, + ).order_by("startdate")[0] + + # Get all patches with open status in this commitfest + open_pocs = self.patchoncommitfest_set.filter( + status__in=PatchOnCommitFest.OPEN_STATUSES + ).select_related("patch") + + for poc in open_pocs: + if self._should_auto_move_patch(poc.patch, current_date): + poc.patch.move(self, next_cf, by_user=None, by_cfbot=True) + + def send_closure_notifications(self): + """Send email notifications to authors of patches that are still open.""" + # Get patches that still need action (not moved, not closed) + open_pocs = list( + self.patchoncommitfest_set.filter( + status__in=PatchOnCommitFest.OPEN_STATUSES + ) + .select_related("patch") + .prefetch_related("patch__authors") + ) + + if not open_pocs: + return + + # Collect unique authors and their patches + authors_patches = {} + for poc in open_pocs: + for author in poc.patch.authors.all(): + if author not in authors_patches: + authors_patches[author] = [] + authors_patches[author].append(poc) + + # Send email to each author who has notifications enabled + for author, patches in authors_patches.items(): + try: + if not author.userprofile.notify_all_author: + continue + notifyemail = author.userprofile.notifyemail + except UserProfile.DoesNotExist: + continue + + email = notifyemail.email if notifyemail else author.email + + send_template_mail( + settings.NOTIFICATION_FROM, + None, + email, + f"Commitfest {self.name} has closed and you have unmoved patches", + "mail/commitfest_closure.txt", + { + "user": author, + "commitfest": self, + "patches": patches, + }, + ) + @staticmethod def _are_relevant_commitfests_up_to_date(cfs, current_date): inprogress_cf = cfs["in_progress"] @@ -143,26 +257,33 @@ def _refresh_relevant_commitfests(cls, for_update): if inprogress_cf and inprogress_cf.enddate < current_date: inprogress_cf.status = CommitFest.STATUS_CLOSED inprogress_cf.save() + inprogress_cf.auto_move_active_patches() + inprogress_cf.send_closure_notifications() open_cf = cfs["open"] if open_cf.startdate <= current_date: if open_cf.enddate < current_date: open_cf.status = CommitFest.STATUS_CLOSED + open_cf.save() + cls.next_open_cf(current_date).save() + open_cf.auto_move_active_patches() + open_cf.send_closure_notifications() else: open_cf.status = CommitFest.STATUS_INPROGRESS - open_cf.save() - - cls.next_open_cf(current_date).save() + 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 + # Create next CF first so auto_move has somewhere to move patches draft_cf.status = CommitFest.STATUS_CLOSED draft_cf.save() cls.next_draft_cf(current_date).save() + draft_cf.auto_move_active_patches() + draft_cf.send_closure_notifications() return cls.relevant_commitfests(for_update=for_update) @@ -456,7 +577,9 @@ 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): + def move( + self, from_cf, to_cf, by_user, allow_move_to_in_progress=False, by_cfbot=False + ): """Returns the new PatchOnCommitFest object, or raises UserInputError""" current_poc = self.current_patch_on_commitfest() @@ -501,6 +624,7 @@ def move(self, from_cf, to_cf, by_user, allow_move_to_in_progress=False): PatchHistory( patch=self, by=by_user, + by_cfbot=by_cfbot, what=f"Moved from CF {from_cf} to CF {to_cf}", ).save_and_notify() diff --git a/pgcommitfest/commitfest/templates/commitfest.html b/pgcommitfest/commitfest/templates/commitfest.html index 082eb72c..357d6bc1 100644 --- a/pgcommitfest/commitfest/templates/commitfest.html +++ b/pgcommitfest/commitfest/templates/commitfest.html @@ -2,6 +2,12 @@ {%load commitfest %} {%block contents%} + {%if cf.is_closed%} + + {%endif%} + No reviewers My patches diff --git a/pgcommitfest/commitfest/templates/help.html b/pgcommitfest/commitfest/templates/help.html index 0723d8f4..5807add2 100644 --- a/pgcommitfest/commitfest/templates/help.html +++ b/pgcommitfest/commitfest/templates/help.html @@ -15,6 +15,11 @@

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.

+

Commitfest closure

+

+ When a Commitfest closes, patches that have been active recently are automatically moved to the next Commitfest. A patch is considered "active" if it has had email activity in the past {{auto_move_email_activity_days}} days and has not been failing CI for more than {{auto_move_max_failing_days}} days. Patches that are not automatically moved will stay in the closed Commitfest, where they will no longer be picked up by CI. Authors of such patches that have enabled "Notify on all where author" in their profile settings will receive an email notification asking them to either move the patch to the next Commitfest or close it with an appropriate status. +

+

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. diff --git a/pgcommitfest/commitfest/templates/mail/commitfest_closure.txt b/pgcommitfest/commitfest/templates/mail/commitfest_closure.txt new file mode 100644 index 00000000..298ff090 --- /dev/null +++ b/pgcommitfest/commitfest/templates/mail/commitfest_closure.txt @@ -0,0 +1,14 @@ +Commitfest {{commitfest.name}} has now closed. + +You have {{patches|length}} open patch{{patches|length|pluralize:"es"}} that need{{patches|length|pluralize:"s,"}} attention: + +{% for poc in patches %} + - {{poc.patch.name}} + https://commitfest.postgresql.org/patch/{{poc.patch.id}}/ +{% endfor %} + +Please take action on {{patches|length|pluralize:"these patches,this patch"}} by doing either of the following: + +1. If you want to continue working on {{patches|length|pluralize:"them,it"}}, move {{patches|length|pluralize:"them,it"}} to the next commitfest. + +2. If you no longer wish to pursue {{patches|length|pluralize:"these patches,this patch"}}, please close {{patches|length|pluralize:"them,it"}} with an appropriate status (Withdrawn, Returned with feedback, etc.) diff --git a/pgcommitfest/commitfest/templates/patch.html b/pgcommitfest/commitfest/templates/patch.html index 03cafb5c..e48b5bd2 100644 --- a/pgcommitfest/commitfest/templates/patch.html +++ b/pgcommitfest/commitfest/templates/patch.html @@ -1,6 +1,13 @@ {%extends "base.html"%} {%load commitfest%} {%block contents%} + {%with current_poc=patch_commitfests.0%} + {%if cf.is_closed and not current_poc.is_closed%} +

+ {%endif%} + {%endwith%} {%include "patch_commands.inc"%} diff --git a/pgcommitfest/commitfest/tests/conftest.py b/pgcommitfest/commitfest/tests/conftest.py index 9ce147e6..54ee47a8 100644 --- a/pgcommitfest/commitfest/tests/conftest.py +++ b/pgcommitfest/commitfest/tests/conftest.py @@ -7,17 +7,20 @@ import pytest from pgcommitfest.commitfest.models import CommitFest +from pgcommitfest.userprofile.models import UserProfile @pytest.fixture def alice(): - """Create test user Alice.""" - return User.objects.create_user( + """Create test user Alice with notify_all_author enabled.""" + user = User.objects.create_user( username="alice", first_name="Alice", last_name="Anderson", email="alice@example.com", ) + UserProfile.objects.create(user=user, notify_all_author=True) + return user @pytest.fixture diff --git a/pgcommitfest/commitfest/tests/test_closure_notifications.py b/pgcommitfest/commitfest/tests/test_closure_notifications.py new file mode 100644 index 00000000..3973d43d --- /dev/null +++ b/pgcommitfest/commitfest/tests/test_closure_notifications.py @@ -0,0 +1,431 @@ +from django.conf import settings + +import base64 +from datetime import date, datetime, timedelta +from email import message_from_string + +import pytest + +from pgcommitfest.commitfest.models import ( + CfbotBranch, + CommitFest, + Patch, + PatchHistory, + PatchOnCommitFest, + PendingNotification, +) +from pgcommitfest.mailqueue.models import QueuedMail +from pgcommitfest.userprofile.models import UserProfile + +pytestmark = pytest.mark.django_db + + +def get_email_body(queued_mail): + """Extract and decode the email body from a QueuedMail object.""" + msg = message_from_string(queued_mail.fullmsg) + for part in msg.walk(): + if part.get_content_type() == "text/plain": + payload = part.get_payload() + return base64.b64decode(payload).decode("utf-8") + return "" + + +def test_send_closure_notifications_to_authors_of_open_patches(alice, in_progress_cf): + """Authors of patches with open status should receive closure notifications.""" + patch = Patch.objects.create(name="Test Patch") + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.send_closure_notifications() + + assert QueuedMail.objects.count() == 1 + mail = QueuedMail.objects.first() + assert mail.receiver == alice.email + assert f"Commitfest {in_progress_cf.name} has closed" in mail.fullmsg + body = get_email_body(mail) + assert "Test Patch" in body + + +def test_no_notification_for_committed_patches(alice, in_progress_cf): + """Authors of committed patches should not receive notifications.""" + patch = Patch.objects.create(name="Committed Patch") + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + leavedate=datetime.now(), + status=PatchOnCommitFest.STATUS_COMMITTED, + ) + + in_progress_cf.send_closure_notifications() + + assert QueuedMail.objects.count() == 0 + + +def test_no_notification_for_withdrawn_patches(alice, in_progress_cf, open_cf): + """Withdrawn patches should not receive notifications or be auto-moved.""" + patch = Patch.objects.create( + name="Withdrawn Patch", + lastmail=datetime.now() - timedelta(days=5), + ) + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + leavedate=datetime.now(), + status=PatchOnCommitFest.STATUS_WITHDRAWN, + ) + + in_progress_cf.auto_move_active_patches() + in_progress_cf.send_closure_notifications() + + assert QueuedMail.objects.count() == 0 + + +def test_one_email_per_author_with_multiple_patches(alice, in_progress_cf): + """An author with multiple open patches should receive one email listing all patches.""" + patch1 = Patch.objects.create(name="Patch One") + patch1.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch1, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + patch2 = Patch.objects.create(name="Patch Two") + patch2.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch2, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_AUTHOR, + ) + + in_progress_cf.send_closure_notifications() + + assert QueuedMail.objects.count() == 1 + mail = QueuedMail.objects.first() + body = get_email_body(mail) + assert "Patch One" in body + assert "Patch Two" in body + + +def test_multiple_authors_receive_separate_emails(alice, bob, in_progress_cf): + """Each author of open patches should receive their own notification (if opted in).""" + # Bob also needs notify_all_author enabled to receive closure emails + UserProfile.objects.create(user=bob, notify_all_author=True) + + patch1 = Patch.objects.create(name="Alice Patch") + patch1.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch1, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + patch2 = Patch.objects.create(name="Bob Patch") + patch2.authors.add(bob) + PatchOnCommitFest.objects.create( + patch=patch2, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_COMMITTER, + ) + + in_progress_cf.send_closure_notifications() + + assert QueuedMail.objects.count() == 2 + receivers = set(QueuedMail.objects.values_list("receiver", flat=True)) + assert receivers == {alice.email, bob.email} + + +def test_coauthors_both_receive_notification(alice, bob, in_progress_cf): + """Both co-authors of a patch should receive notifications (if opted in).""" + # Bob also needs notify_all_author enabled to receive closure emails + UserProfile.objects.create(user=bob, notify_all_author=True) + + patch = Patch.objects.create(name="Coauthored Patch") + patch.authors.add(alice) + patch.authors.add(bob) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.send_closure_notifications() + + assert QueuedMail.objects.count() == 2 + receivers = set(QueuedMail.objects.values_list("receiver", flat=True)) + assert receivers == {alice.email, bob.email} + + +def test_no_notification_for_author_without_notify_all_author(bob, in_progress_cf): + """Authors without notify_all_author enabled should not receive closure notifications.""" + # bob has no UserProfile, so notify_all_author is not enabled + patch = Patch.objects.create(name="Test Patch") + patch.authors.add(bob) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.send_closure_notifications() + + assert QueuedMail.objects.count() == 0 + + +# Auto-move tests + + +def test_auto_move_patch_with_recent_email_activity( + alice, bob, in_progress_cf, open_cf +): + """Patches with recent email activity should be auto-moved to the next commitfest.""" + patch = Patch.objects.create( + name="Active Patch", + lastmail=datetime.now() - timedelta(days=5), + ) + patch.authors.add(alice) + patch.subscribers.add(bob) # Bob subscribes to get notifications + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.auto_move_active_patches() + in_progress_cf.send_closure_notifications() + + # Patch should be moved + patch.refresh_from_db() + assert patch.current_commitfest().id == open_cf.id + + # Move should create a history entry with by_cfbot=True + history = PatchHistory.objects.filter(patch=patch).first() + assert history is not None + assert history.by_cfbot is True + assert "Moved from CF" in history.what + + # PendingNotification should be created for author and subscriber + assert PendingNotification.objects.filter(history=history, user=alice).exists() + assert PendingNotification.objects.filter(history=history, user=bob).exists() + + # No closure email for moved patches (move triggers its own notification) + assert QueuedMail.objects.count() == 0 + + +def test_no_auto_move_without_email_activity(alice, in_progress_cf, open_cf): + """Patches without recent email activity should NOT be auto-moved.""" + patch = Patch.objects.create( + name="Inactive Patch", + lastmail=datetime.now() + - timedelta(days=settings.AUTO_MOVE_EMAIL_ACTIVITY_DAYS + 10), + ) + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.auto_move_active_patches() + in_progress_cf.send_closure_notifications() + + # Patch should NOT be moved + patch.refresh_from_db() + assert patch.current_commitfest().id == in_progress_cf.id + + # Closure email should be sent for non-moved patches + assert QueuedMail.objects.count() == 1 + mail = QueuedMail.objects.first() + body = get_email_body(mail) + assert "Inactive Patch" in body + assert "needs attention" in body + + +def test_no_auto_move_when_failing_too_long(alice, in_progress_cf, open_cf): + """Patches failing CI for too long should NOT be auto-moved even with recent activity.""" + patch = Patch.objects.create( + name="Failing Patch", + lastmail=datetime.now() - timedelta(days=5), + ) + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + # Add CfbotBranch with long-standing failure + CfbotBranch.objects.create( + patch=patch, + branch_id=1, + branch_name="test-branch", + apply_url="https://example.com", + status="failed", + failing_since=datetime.now() + - timedelta(days=settings.AUTO_MOVE_MAX_FAILING_DAYS + 10), + ) + + in_progress_cf.auto_move_active_patches() + in_progress_cf.send_closure_notifications() + + # Patch should NOT be moved + patch.refresh_from_db() + assert patch.current_commitfest().id == in_progress_cf.id + + # Author should receive closure notification + assert QueuedMail.objects.count() == 1 + mail = QueuedMail.objects.first() + body = get_email_body(mail) + assert "Failing Patch" in body + + +def test_auto_move_when_failing_within_threshold(alice, in_progress_cf, open_cf): + """Patches failing CI within the threshold should still be auto-moved.""" + patch = Patch.objects.create( + name="Recently Failing Patch", + lastmail=datetime.now() - timedelta(days=5), + ) + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + # Add CfbotBranch with recent failure (within threshold) + CfbotBranch.objects.create( + patch=patch, + branch_id=2, + branch_name="test-branch-2", + apply_url="https://example.com", + status="failed", + failing_since=datetime.now() + - timedelta(days=settings.AUTO_MOVE_MAX_FAILING_DAYS - 5), + ) + + in_progress_cf.auto_move_active_patches() + in_progress_cf.send_closure_notifications() + + # Patch should be moved (failure is recent enough) + patch.refresh_from_db() + assert patch.current_commitfest().id == open_cf.id + + # No closure email for moved patches + assert QueuedMail.objects.count() == 0 + + +def test_regular_cf_does_not_move_to_draft_cf(alice, in_progress_cf): + """Regular commitfest should move patches to the next regular CF, not a draft CF.""" + # Create a draft CF with earlier startdate - should be ignored for regular CF patches + draft_cf = CommitFest.objects.create( + name="2024-12-draft", + status=CommitFest.STATUS_OPEN, + startdate=date(2024, 12, 1), + enddate=date(2024, 12, 31), + draft=True, + ) + # Create a regular CF with later startdate - this is where patches should go + regular_cf = CommitFest.objects.create( + name="2025-03", + status=CommitFest.STATUS_OPEN, + startdate=date(2025, 3, 1), + enddate=date(2025, 3, 31), + draft=False, + ) + + patch = Patch.objects.create( + name="Regular Patch", + lastmail=datetime.now() - timedelta(days=5), + ) + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.auto_move_active_patches() + + # Should be moved to regular CF, not draft CF + patch.refresh_from_db() + assert patch.current_commitfest().id == regular_cf.id + assert patch.current_commitfest().id != draft_cf.id + + +def test_draft_cf_moves_active_patches_to_next_draft(alice, bob): + """Active patches in a draft commitfest should be auto-moved to the next draft CF.""" + # Create two draft CFs - one closing and one to receive patches + closing_draft_cf = CommitFest.objects.create( + name="2025-03-draft", + status=CommitFest.STATUS_INPROGRESS, + startdate=date(2025, 3, 1), + enddate=date(2025, 3, 31), + draft=True, + ) + # Create a regular open CF with earlier startdate - should be ignored for draft patches + CommitFest.objects.create( + name="2025-05", + status=CommitFest.STATUS_OPEN, + startdate=date(2025, 5, 1), + enddate=date(2025, 5, 31), + draft=False, + ) + next_draft_cf = CommitFest.objects.create( + name="2026-03-draft", + status=CommitFest.STATUS_OPEN, + startdate=date(2026, 3, 1), + enddate=date(2026, 3, 31), + draft=True, + ) + + patch = Patch.objects.create( + name="Draft Patch", + lastmail=datetime.now() - timedelta(days=5), + ) + patch.authors.add(alice) + patch.subscribers.add(bob) # Bob subscribes to get notifications + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=closing_draft_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + closing_draft_cf.auto_move_active_patches() + closing_draft_cf.send_closure_notifications() + + # Patch should be moved to the next draft CF + patch.refresh_from_db() + assert patch.current_commitfest().id == next_draft_cf.id + + # Move should create a history entry with by_cfbot=True + history = PatchHistory.objects.filter(patch=patch).first() + assert history is not None + assert history.by_cfbot is True + + # PendingNotification should be created for author and subscriber + assert PendingNotification.objects.filter(history=history, user=alice).exists() + assert PendingNotification.objects.filter(history=history, user=bob).exists() + + # No closure email for moved patches + assert QueuedMail.objects.count() == 0 diff --git a/pgcommitfest/commitfest/tests/test_refresh_commitfests.py b/pgcommitfest/commitfest/tests/test_refresh_commitfests.py new file mode 100644 index 00000000..dda1fc9d --- /dev/null +++ b/pgcommitfest/commitfest/tests/test_refresh_commitfests.py @@ -0,0 +1,158 @@ +from datetime import datetime + +import pytest +from freezegun import freeze_time + +from pgcommitfest.commitfest.models import ( + CommitFest, + Patch, + PatchOnCommitFest, +) + +pytestmark = pytest.mark.django_db + + +@freeze_time("2024-12-05") +def test_inprogress_cf_closes_when_enddate_passed(commitfests, alice): + """When an in_progress CF's enddate has passed, it should be closed.""" + in_progress_cf = commitfests["in_progress"] + open_cf = commitfests["open"] + + patch = Patch.objects.create( + name="Test Patch", + lastmail=datetime(2024, 11, 25), + ) + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + CommitFest._refresh_relevant_commitfests(for_update=False) + + in_progress_cf.refresh_from_db() + assert in_progress_cf.status == CommitFest.STATUS_CLOSED + + # Patch should be auto-moved + patch.refresh_from_db() + assert patch.current_commitfest().id == open_cf.id + + +@freeze_time("2025-01-15") +def test_open_cf_becomes_inprogress_when_startdate_reached(commitfests): + """When an open CF's startdate is reached, it becomes in_progress.""" + open_cf = commitfests["open"] + + CommitFest._refresh_relevant_commitfests(for_update=False) + + open_cf.refresh_from_db() + assert open_cf.status == CommitFest.STATUS_INPROGRESS + + new_open = CommitFest.objects.filter( + status=CommitFest.STATUS_OPEN, draft=False + ).first() + assert new_open is not None + assert new_open.startdate > open_cf.enddate + + +@freeze_time("2025-02-05") +def test_open_cf_closes_when_enddate_passed(commitfests, alice): + """When an open CF's enddate has passed (skipping in_progress), it closes.""" + open_cf = commitfests["open"] + + patch = Patch.objects.create( + name="Test Patch", + lastmail=datetime(2025, 1, 25), + ) + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=open_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + CommitFest._refresh_relevant_commitfests(for_update=False) + + open_cf.refresh_from_db() + assert open_cf.status == CommitFest.STATUS_CLOSED + + new_open = CommitFest.objects.filter( + status=CommitFest.STATUS_OPEN, draft=False + ).first() + assert new_open is not None + + # Patch should be auto-moved + patch.refresh_from_db() + assert patch.current_commitfest().id == new_open.id + + +@freeze_time("2025-01-15") +def test_draft_cf_created_when_missing(commitfests): + """When no draft CF exists, one should be created.""" + # Delete the draft CF + commitfests["draft"].delete() + + assert not CommitFest.objects.filter(draft=True).exists() + + CommitFest._refresh_relevant_commitfests(for_update=False) + + draft_cf = CommitFest.objects.filter(draft=True).first() + assert draft_cf is not None + assert draft_cf.status == CommitFest.STATUS_OPEN + + +@freeze_time("2025-04-05") +def test_draft_cf_closes_when_enddate_passed(commitfests, alice): + """When a draft CF's enddate has passed, it should be closed.""" + draft_cf = commitfests["draft"] + + patch = Patch.objects.create( + name="Draft Patch", + lastmail=datetime(2025, 3, 25), + ) + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=draft_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + CommitFest._refresh_relevant_commitfests(for_update=False) + + draft_cf.refresh_from_db() + assert draft_cf.status == CommitFest.STATUS_CLOSED + + new_draft = CommitFest.objects.filter( + draft=True, status=CommitFest.STATUS_OPEN + ).first() + assert new_draft is not None + assert new_draft.startdate > draft_cf.enddate + + # Patch should be auto-moved + patch.refresh_from_db() + assert patch.current_commitfest().id == new_draft.id + + +@freeze_time("2024-11-15") +def test_no_changes_when_up_to_date(commitfests): + """When commitfests are up to date, no changes should be made.""" + in_progress_cf = commitfests["in_progress"] + open_cf = commitfests["open"] + draft_cf = commitfests["draft"] + + initial_cf_count = CommitFest.objects.count() + + CommitFest._refresh_relevant_commitfests(for_update=False) + + in_progress_cf.refresh_from_db() + open_cf.refresh_from_db() + draft_cf.refresh_from_db() + + assert in_progress_cf.status == CommitFest.STATUS_INPROGRESS + assert open_cf.status == CommitFest.STATUS_OPEN + assert draft_cf.status == CommitFest.STATUS_OPEN + assert CommitFest.objects.count() == initial_cf_count diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index 1f6a30d9..b2d64c48 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -174,6 +174,8 @@ def help(request): "help.html", { "title": "What is the Commitfest app?", + "auto_move_email_activity_days": settings.AUTO_MOVE_EMAIL_ACTIVITY_DAYS, + "auto_move_max_failing_days": settings.AUTO_MOVE_MAX_FAILING_DAYS, }, ) diff --git a/pgcommitfest/settings.py b/pgcommitfest/settings.py index e48cf09e..07e4adaf 100644 --- a/pgcommitfest/settings.py +++ b/pgcommitfest/settings.py @@ -168,6 +168,12 @@ CFBOT_API_URL = "https://cfbot.cputube.org/api" +# Auto-move settings for commitfest closure +# Patches with email activity within this many days are considered active +AUTO_MOVE_EMAIL_ACTIVITY_DAYS = 30 +# Patches failing CI for longer than this many days will NOT be auto-moved +AUTO_MOVE_MAX_FAILING_DAYS = 21 + # Load local settings overrides try: from .local_settings import * # noqa: F403 diff --git a/pyproject.toml b/pyproject.toml index 411d4075..4ed170eb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,6 +20,7 @@ dev = [ "djhtml", "pytest", "pytest-django", + "freezegun", ] [tool.setuptools.packages.find] From 8f8b416e35959648c28835b4bceb9b573e23cb42 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 5 Jan 2026 09:34:30 +0100 Subject: [PATCH 26/38] Better user profile admin page --- pgcommitfest/userprofile/admin.py | 32 ++++++++++++++- .../userprofile/userprofile/change_list.html | 41 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 pgcommitfest/userprofile/templates/admin/userprofile/userprofile/change_list.html diff --git a/pgcommitfest/userprofile/admin.py b/pgcommitfest/userprofile/admin.py index bc847464..54ffe714 100644 --- a/pgcommitfest/userprofile/admin.py +++ b/pgcommitfest/userprofile/admin.py @@ -1,10 +1,40 @@ from django.contrib import admin +from django.db.models import Count, Q from .models import UserProfile class UserProfileAdmin(admin.ModelAdmin): - list_display = ("user",) + list_display = ( + "user", + "notify_all_author", + "notify_all_reviewer", + "notify_all_committer", + "show_relative_timestamps", + ) + list_filter = ( + "notify_all_author", + "notify_all_reviewer", + "notify_all_committer", + "show_relative_timestamps", + ) + search_fields = ("user__username", "user__first_name", "user__last_name") + + def changelist_view(self, request, extra_context=None): + stats = UserProfile.objects.aggregate( + total=Count("id"), + author_on=Count("id", filter=Q(notify_all_author=True)), + author_off=Count("id", filter=Q(notify_all_author=False)), + reviewer_on=Count("id", filter=Q(notify_all_reviewer=True)), + reviewer_off=Count("id", filter=Q(notify_all_reviewer=False)), + committer_on=Count("id", filter=Q(notify_all_committer=True)), + committer_off=Count("id", filter=Q(notify_all_committer=False)), + timestamps_on=Count("id", filter=Q(show_relative_timestamps=True)), + timestamps_off=Count("id", filter=Q(show_relative_timestamps=False)), + ) + extra_context = extra_context or {} + extra_context["notification_stats"] = stats + return super().changelist_view(request, extra_context=extra_context) admin.site.register(UserProfile, UserProfileAdmin) diff --git a/pgcommitfest/userprofile/templates/admin/userprofile/userprofile/change_list.html b/pgcommitfest/userprofile/templates/admin/userprofile/userprofile/change_list.html new file mode 100644 index 00000000..7e8aa3f4 --- /dev/null +++ b/pgcommitfest/userprofile/templates/admin/userprofile/userprofile/change_list.html @@ -0,0 +1,41 @@ +{% extends "admin/change_list.html" %} + +{% block content %} +{% if notification_stats %} +
+

Notification Settings Statistics ({{ notification_stats.total }} profiles)

+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
SettingOnOff
Notify on all where author{{ notification_stats.author_on }}{{ notification_stats.author_off }}
Notify on all where reviewer{{ notification_stats.reviewer_on }}{{ notification_stats.reviewer_off }}
Notify on all where committer{{ notification_stats.committer_on }}{{ notification_stats.committer_off }}
Show relative timestamps{{ notification_stats.timestamps_on }}{{ notification_stats.timestamps_off }}
+ +{% endif %} +{{ block.super }} +{% endblock %} From fbd6aa760d8cc694e0a7e879ce793f51eb421915 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Mon, 5 Jan 2026 09:30:36 +0100 Subject: [PATCH 27/38] Default to sending emails for patches where you are the author This only applies to new users or users that never clicked the profile button. --- pgcommitfest/commitfest/models.py | 15 +++++--- .../templates/mail/commitfest_closure.txt | 5 +++ .../templates/mail/patch_notify.txt | 4 +++ pgcommitfest/commitfest/tests/conftest.py | 4 +-- .../tests/test_closure_notifications.py | 34 +++++++++++++------ pgcommitfest/userprofile/models.py | 2 +- 6 files changed, 45 insertions(+), 19 deletions(-) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 921fcf22..6594e3c4 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -202,11 +202,13 @@ def send_closure_notifications(self): # Send email to each author who has notifications enabled for author, patches in authors_patches.items(): try: - if not author.userprofile.notify_all_author: + profile = author.userprofile + if not profile.notify_all_author: continue - notifyemail = author.userprofile.notifyemail + notifyemail = profile.notifyemail except UserProfile.DoesNotExist: - continue + # No profile means use default (notify=True), so continue + notifyemail = None email = notifyemail.email if notifyemail else author.email @@ -813,9 +815,12 @@ def save_and_notify( ) ) - # Current or previous authors wants all notifications + # Current or previous authors wants all notifications (default is True, + # so include users without a profile) recipients.extend( - self.patch.authors.filter(userprofile__notify_all_author=True) + self.patch.authors.filter( + Q(userprofile__isnull=True) | Q(userprofile__notify_all_author=True) + ) ) for u in set(recipients): diff --git a/pgcommitfest/commitfest/templates/mail/commitfest_closure.txt b/pgcommitfest/commitfest/templates/mail/commitfest_closure.txt index 298ff090..8dc3ecf1 100644 --- a/pgcommitfest/commitfest/templates/mail/commitfest_closure.txt +++ b/pgcommitfest/commitfest/templates/mail/commitfest_closure.txt @@ -12,3 +12,8 @@ Please take action on {{patches|length|pluralize:"these patches,this patch"}} by 1. If you want to continue working on {{patches|length|pluralize:"them,it"}}, move {{patches|length|pluralize:"them,it"}} to the next commitfest. 2. If you no longer wish to pursue {{patches|length|pluralize:"these patches,this patch"}}, please close {{patches|length|pluralize:"them,it"}} with an appropriate status (Withdrawn, Returned with feedback, etc.) + +-- +This is an automated message from the PostgreSQL Commitfest app. +To manage your notification preferences, visit: +https://commitfest.postgresql.org/userprofile/ diff --git a/pgcommitfest/commitfest/templates/mail/patch_notify.txt b/pgcommitfest/commitfest/templates/mail/patch_notify.txt index 3153e9d5..ebd5f2ed 100644 --- a/pgcommitfest/commitfest/templates/mail/patch_notify.txt +++ b/pgcommitfest/commitfest/templates/mail/patch_notify.txt @@ -9,3 +9,7 @@ https://commitfest.postgresql.org/patch/{{p.patch.id}}/ {%endfor%} +-- +This is an automated message from the PostgreSQL Commitfest app. +To manage your notification preferences, visit: +https://commitfest.postgresql.org/userprofile/ diff --git a/pgcommitfest/commitfest/tests/conftest.py b/pgcommitfest/commitfest/tests/conftest.py index 54ee47a8..baff4328 100644 --- a/pgcommitfest/commitfest/tests/conftest.py +++ b/pgcommitfest/commitfest/tests/conftest.py @@ -12,14 +12,14 @@ @pytest.fixture def alice(): - """Create test user Alice with notify_all_author enabled.""" + """Create test user Alice with a profile (uses default notify_all_author=True).""" user = User.objects.create_user( username="alice", first_name="Alice", last_name="Anderson", email="alice@example.com", ) - UserProfile.objects.create(user=user, notify_all_author=True) + UserProfile.objects.create(user=user) return user diff --git a/pgcommitfest/commitfest/tests/test_closure_notifications.py b/pgcommitfest/commitfest/tests/test_closure_notifications.py index 3973d43d..eb7cfc7a 100644 --- a/pgcommitfest/commitfest/tests/test_closure_notifications.py +++ b/pgcommitfest/commitfest/tests/test_closure_notifications.py @@ -119,10 +119,7 @@ def test_one_email_per_author_with_multiple_patches(alice, in_progress_cf): def test_multiple_authors_receive_separate_emails(alice, bob, in_progress_cf): - """Each author of open patches should receive their own notification (if opted in).""" - # Bob also needs notify_all_author enabled to receive closure emails - UserProfile.objects.create(user=bob, notify_all_author=True) - + """Each author of open patches should receive their own notification.""" patch1 = Patch.objects.create(name="Alice Patch") patch1.authors.add(alice) PatchOnCommitFest.objects.create( @@ -149,10 +146,7 @@ def test_multiple_authors_receive_separate_emails(alice, bob, in_progress_cf): def test_coauthors_both_receive_notification(alice, bob, in_progress_cf): - """Both co-authors of a patch should receive notifications (if opted in).""" - # Bob also needs notify_all_author enabled to receive closure emails - UserProfile.objects.create(user=bob, notify_all_author=True) - + """Both co-authors of a patch should receive notifications.""" patch = Patch.objects.create(name="Coauthored Patch") patch.authors.add(alice) patch.authors.add(bob) @@ -170,9 +164,9 @@ def test_coauthors_both_receive_notification(alice, bob, in_progress_cf): assert receivers == {alice.email, bob.email} -def test_no_notification_for_author_without_notify_all_author(bob, in_progress_cf): - """Authors without notify_all_author enabled should not receive closure notifications.""" - # bob has no UserProfile, so notify_all_author is not enabled +def test_no_notification_for_author_who_opted_out(bob, in_progress_cf): + """Authors who explicitly opted out should not receive closure notifications.""" + UserProfile.objects.create(user=bob, notify_all_author=False) patch = Patch.objects.create(name="Test Patch") patch.authors.add(bob) PatchOnCommitFest.objects.create( @@ -187,6 +181,24 @@ def test_no_notification_for_author_without_notify_all_author(bob, in_progress_c assert QueuedMail.objects.count() == 0 +def test_notification_for_author_without_profile(bob, in_progress_cf): + """Authors without a UserProfile should receive notifications (opt-out default).""" + patch = Patch.objects.create(name="Test Patch") + patch.authors.add(bob) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.send_closure_notifications() + + assert QueuedMail.objects.count() == 1 + mail = QueuedMail.objects.first() + assert mail.receiver == bob.email + + # Auto-move tests diff --git a/pgcommitfest/userprofile/models.py b/pgcommitfest/userprofile/models.py index 03cbbb8a..5bd2e1cc 100644 --- a/pgcommitfest/userprofile/models.py +++ b/pgcommitfest/userprofile/models.py @@ -36,7 +36,7 @@ class UserProfile(models.Model): notify_all_author = models.BooleanField( null=False, blank=False, - default=False, + default=True, verbose_name="Notify on all where author", ) notify_all_reviewer = models.BooleanField( From 11257a94483c75369457c4cc0546b804ebc3b940 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 7 Jan 2026 10:20:57 +0100 Subject: [PATCH 28/38] Fix typo --- .../commitfest/migrations/0016_migrate_topics_to_tags.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py index 0ecf3025..9f1cb3bf 100644 --- a/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py +++ b/pgcommitfest/commitfest/migrations/0016_migrate_topics_to_tags.py @@ -44,7 +44,7 @@ def create_missing_tags_and_map_topics(apps, schema_editor): "System Administration": "System Administration", # 'Miscellaneous' and 'Server Features' are left untagged, because they # are too vague. - # 'Procedural Languages' has no direct tag equivalent, because new + # 'Procedural Languages' has no direct tag equivalent, because now # there are tags per language. So there's no clear tag that should be # chosen there. Similar for 'Replication & Recovery', which also has # separate tags now for logical and physical replication. From fa58274546de5684b1aab6d0f87fc157d8bb8241 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 7 Jan 2026 11:21:08 +0100 Subject: [PATCH 29/38] Make the table more responsive by breaking on other characters too --- media/commitfest/css/commitfest.css | 7 +++++++ pgcommitfest/commitfest/templates/commitfest.html | 8 ++++---- pgcommitfest/commitfest/templates/home.html | 8 ++++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/media/commitfest/css/commitfest.css b/media/commitfest/css/commitfest.css index 8bce3826..69a62e5d 100644 --- a/media/commitfest/css/commitfest.css +++ b/media/commitfest/css/commitfest.css @@ -222,3 +222,10 @@ a:hover .badge { width: 300px; } } + +/* Allow long text to wrap in table cells */ +.table td.wrap-text { + overflow-wrap: break-word; + word-break: break-word; + max-width: 300px; +} diff --git a/pgcommitfest/commitfest/templates/commitfest.html b/pgcommitfest/commitfest/templates/commitfest.html index 357d6bc1..0dcedb69 100644 --- a/pgcommitfest/commitfest/templates/commitfest.html +++ b/pgcommitfest/commitfest/templates/commitfest.html @@ -53,7 +53,7 @@

{{p.is_open|yesno:"Active patches,Closed patches"}}

{%endifchanged%} - {{p.name}} + {{p.name}} {{p.id}} {{p.status|patchstatusstring}} @@ -97,9 +97,9 @@

{{p.is_open|yesno:"Active patches,Closed patches"}}

{%endif%} {%endwith%} - {{p.author_names|default:''}} - {{p.reviewer_names|default:''}} - {{p.committer|default:''}} + {{p.author_names|default:''}} + {{p.reviewer_names|default:''}} + {{p.committer|default:''}} {{p.num_cfs}} {%if p.lastmail and userprofile.show_relative_timestamps %}{% cfwhen p.lastmail %}{%elif p.lastmail %}{{p.lastmail|date:"Y-m-d"}}
{{p.lastmail|date:"H:i"}}{%endif%} {%if user.is_staff%} diff --git a/pgcommitfest/commitfest/templates/home.html b/pgcommitfest/commitfest/templates/home.html index e83b2ab7..cabe1e3a 100644 --- a/pgcommitfest/commitfest/templates/home.html +++ b/pgcommitfest/commitfest/templates/home.html @@ -156,7 +156,7 @@

{%if user.is_authenticated%}Open patches you are subscribed to{%elif p.is_op {%endifchanged%} {%endif%} - {{p.name}} + {{p.name}} {{p.id}} {%if user.is_authenticated %} {{p.cf_name}} @@ -203,9 +203,9 @@

{%if user.is_authenticated%}Open patches you are subscribed to{%elif p.is_op {%endif%} {%endwith%} - {{p.author_names|default:''}} - {{p.reviewer_names|default:''}} - {{p.committer|default:''}} + {{p.author_names|default:''}} + {{p.reviewer_names|default:''}} + {{p.committer|default:''}} {{p.num_cfs}} {%if p.lastmail and userprofile.show_relative_timestamps %}{% cfwhen p.lastmail %}{%elif p.lastmail %}{{p.lastmail|date:"Y-m-d"}}
{{p.lastmail|date:"H:i"}}{%endif%} From af62277ca5e6c7aaa792931baa93d15da2519838 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sat, 10 Jan 2026 12:39:07 +0100 Subject: [PATCH 30/38] Force people to consider adding a tag when creating a patch --- pgcommitfest/commitfest/forms.py | 36 ++++++++ .../commitfest/templates/base_form.html | 88 +++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/pgcommitfest/commitfest/forms.py b/pgcommitfest/commitfest/forms.py index 6321a5b5..49d7366f 100644 --- a/pgcommitfest/commitfest/forms.py +++ b/pgcommitfest/commitfest/forms.py @@ -80,6 +80,10 @@ def __init__(self, data, commitfest=None, *args, **kwargs): self.fields[f].widget.attrs = {"class": "input-medium"} +# Tags that get their own checkbox buttons as shortcuts (also appear in selectize dropdown) +CHECKBOX_TAG_NAMES = ["Bugfix", "Security", "Performance"] + + class PatchForm(forms.ModelForm): selectize_fields = { "authors": "/lookups/user", @@ -112,6 +116,13 @@ def __init__(self, *args, **kwargs): self.fields["authors"].widget.attrs["class"] = "add-user-picker" self.fields["reviewers"].widget.attrs["class"] = "add-user-picker" + # Cache checkbox tags for use in JavaScript syncing and template rendering + self.checkbox_tags = list( + Tag.objects.filter(name__in=CHECKBOX_TAG_NAMES).values( + "id", "name", "color" + ) + ) + # Selectize multiple fields -- don't pre-populate everything for field, url in list(self.selectize_fields.items()): if url is None: @@ -157,6 +168,13 @@ class NewPatchForm(PatchForm): widget=ThreadPickWidget, ) + # "No tags apply" checkbox to force users to explicitly opt out of tagging + no_tags_apply = forms.BooleanField( + required=False, + label="No tags apply", + help_text="Check this if none of the tags above apply to this patch", + ) + def __init__(self, *args, **kwargs): request = kwargs.pop("request", None) super(NewPatchForm, self).__init__(*args, **kwargs) @@ -165,6 +183,24 @@ def __init__(self, *args, **kwargs): self.fields["authors"].queryset = User.objects.filter(pk=request.user.id) self.fields["authors"].initial = [request.user.id] + def clean(self): + cleaned_data = super().clean() + + has_tags = bool(cleaned_data.get("tags")) + no_tags = cleaned_data.get("no_tags_apply") + + if no_tags and has_tags: + raise ValidationError( + "You cannot select 'No tags apply' while also selecting tags." + ) + + if not no_tags and not has_tags: + raise ValidationError( + "Please select at least one tag, or check 'No tags apply' if none are applicable." + ) + + return cleaned_data + def clean_threadmsgid(self): try: _archivesAPI("/message-id.json/%s" % self.cleaned_data["threadmsgid"]) diff --git a/pgcommitfest/commitfest/templates/base_form.html b/pgcommitfest/commitfest/templates/base_form.html index 3705bfe4..4a581329 100644 --- a/pgcommitfest/commitfest/templates/base_form.html +++ b/pgcommitfest/commitfest/templates/base_form.html @@ -14,6 +14,35 @@ {%endif%} {%for field in form%} {%if not field.is_hidden%} + {%if field.name == 'tags' and form.checkbox_tags %} + {# Render the tags field with checkbox shortcuts #} +
+ +
+ {# Checkbox shortcuts for common tags #} +
+ + {%for tag in form.checkbox_tags%} + + + {%endfor%} +
+ {# The actual selectize field for all tags #} + {{field}} + {%if field.help_text%}
{{field.help_text|safe}}{%endif%} + {# "No tags apply" checkbox below the selectize (only for new patches) #} + {%if form.no_tags_apply %} +
+ + + {%if form.no_tags_apply.help_text%}{{form.no_tags_apply.help_text}}{%endif%} +
+ {%endif%} +
+
+ {%elif field.name == 'no_tags_apply' %} + {# Skip - rendered as part of the tags group above #} + {%else%}
{{field|label_class:"control-label"}}
@@ -32,6 +61,7 @@ {%elif not field.name in form.selectize_fields%}{{field|field_class:"form-control"}}{%else%}{{field}}{%endif%} {%if field.help_text%}
{{field.help_text|safe}}{%endif%}
+ {%endif%} {%else%} {{field}} {%endif%} @@ -92,5 +122,63 @@

Search user

$('#searchUserSearchField').focus(); }); {%endif%} + + // Tag shortcut checkbox synchronization with selectize + (function() { + var $tagsSelect = $('#id_tags'); + if (!$tagsSelect.length || !$tagsSelect[0].selectize) return; + + var selectize = $tagsSelect[0].selectize; + var $checkboxes = $('.tag-shortcut-checkbox'); + var $noTagsCheckbox = $('#id_no_tags_apply'); + + // Initialize checkbox state from current selectize values + function syncCheckboxesFromSelectize() { + var selectedIds = selectize.getValue(); + if (typeof selectedIds === 'string') { + selectedIds = selectedIds ? selectedIds.split(',') : []; + } + selectedIds = selectedIds.map(function(id) { return String(id); }); + + $checkboxes.each(function() { + var tagId = String($(this).data('tag-id')); + $(this).prop('checked', selectedIds.indexOf(tagId) !== -1); + }); + + // Uncheck "no tags apply" if any tags are selected + if ($noTagsCheckbox.length && selectedIds.length > 0) { + $noTagsCheckbox.prop('checked', false); + } + } + + // Sync on selectize change + selectize.on('change', syncCheckboxesFromSelectize); + + // Handle checkbox clicks + $checkboxes.on('change', function() { + var tagId = String($(this).data('tag-id')); + if ($(this).is(':checked')) { + selectize.addItem(tagId, true); + if ($noTagsCheckbox.length) { + $noTagsCheckbox.prop('checked', false); + } + } else { + selectize.removeItem(tagId, true); + } + }); + + // Handle "no tags apply" checkbox (only exists for new patches) + if ($noTagsCheckbox.length) { + $noTagsCheckbox.on('change', function() { + if ($(this).is(':checked')) { + selectize.clear(true); + $checkboxes.prop('checked', false); + } + }); + } + + // Initial sync + syncCheckboxesFromSelectize(); + })(); {%endblock%} From 9576aea4fbcac07e3a525085bdb63604fd02af64 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sat, 10 Jan 2026 12:49:46 +0100 Subject: [PATCH 31/38] Improve styling for tag dropdown --- media/commitfest/css/commitfest.css | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/media/commitfest/css/commitfest.css b/media/commitfest/css/commitfest.css index 69a62e5d..aefd4346 100644 --- a/media/commitfest/css/commitfest.css +++ b/media/commitfest/css/commitfest.css @@ -229,3 +229,25 @@ a:hover .badge { word-break: break-word; max-width: 300px; } + +/* Make the tags selectize dropdown taller with bottom padding for overlay */ +#id_tags + .selectize-control .selectize-dropdown-content, +#id_tag + .selectize-control .selectize-dropdown-content { + max-height: 325px; + padding-bottom: 2.5em; +} + +/* Gradient fade overlay for scroll hint - shows items through transparent top */ +.selectize-scroll-hint { + position: absolute; + bottom: 0; + left: 0; + right: 0; + background: linear-gradient( + to bottom, + rgba(248, 249, 250, 0) 0%, + rgba(248, 249, 250, 1) 50% + ) !important; + border-top: none !important; + pointer-events: none; +} From c0345bf226043d581067c70c53a03d8e171c63a7 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Fri, 23 Jan 2026 21:55:17 +0100 Subject: [PATCH 32/38] Handle uncaught exception with unexpected /close/committed Apparently some people/tools send these requests without the "c" GET argument. That caused exceptions to be sent to admins. --- pgcommitfest/commitfest/views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index b2d64c48..4af316bd 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -1259,7 +1259,11 @@ def close(request, patchid, status): request.user, ) - committer = get_object_or_404(Committer, user__username=request.GET["c"]) + committer_username = request.GET.get("c") + if committer_username is None: + messages.error(request, "No committer specified.") + return HttpResponseRedirect(f"/patch/{patchid}/") + committer = get_object_or_404(Committer, user__username=committer_username) if committer != poc.patch.committer: # Committer changed! prevcommitter = poc.patch.committer From 30fb47e3b41450e90f653b0abff73516ea6b2ab5 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sun, 25 Jan 2026 17:09:25 +0100 Subject: [PATCH 33/38] Add an API for cfbot to get the needed data --- pgcommitfest/commitfest/apiv1.py | 77 ++++++++++++- pgcommitfest/commitfest/tests/test_apiv1.py | 116 ++++++++++++++++++++ pgcommitfest/urls.py | 2 + 3 files changed, 192 insertions(+), 3 deletions(-) diff --git a/pgcommitfest/commitfest/apiv1.py b/pgcommitfest/commitfest/apiv1.py index a90ba4d3..86a53062 100644 --- a/pgcommitfest/commitfest/apiv1.py +++ b/pgcommitfest/commitfest/apiv1.py @@ -1,22 +1,26 @@ from django.http import ( HttpResponse, ) +from django.shortcuts import get_object_or_404 import json from datetime import date, datetime, timedelta, timezone from .models import ( CommitFest, + Patch, + PatchOnCommitFest, ) def datetime_serializer(obj): - if isinstance(obj, date): - return obj.isoformat() - + # datetime must be checked before date, since datetime is a subclass of date if isinstance(obj, datetime): return obj.replace(tzinfo=timezone.utc).isoformat() + if isinstance(obj, date): + return obj.isoformat() + if hasattr(obj, "to_json"): return obj.to_json() @@ -44,3 +48,70 @@ def commitfestst_that_need_ci(request): del cfs["final"] return api_response({"commitfests": cfs}) + + +def commitfest_patches(request, cfid): + """Return all patches for a commitfest. + + This endpoint provides the data that cfbot previously scraped from the + commitfest HTML page. + """ + cf = get_object_or_404(CommitFest, pk=cfid) + + pocs = ( + PatchOnCommitFest.objects.filter(commitfest=cf) + .select_related("patch") + .prefetch_related("patch__authors") + .order_by("patch__id") + ) + + patches = [] + for poc in pocs: + patch = poc.patch + authors = [f"{a.first_name} {a.last_name}" for a in patch.authors.all()] + patches.append( + { + "id": patch.id, + "name": patch.name, + "status": poc.statusstring, + "authors": authors, + "last_email_time": patch.lastmail, + } + ) + + return api_response( + { + "commitfest_id": cf.id, + "patches": patches, + } + ) + + +def patch_threads(request, patch_id): + """Return thread information for a patch. + + This endpoint provides the data that cfbot previously scraped from the + patch HTML page to construct thread URLs. + """ + patch = get_object_or_404(Patch, pk=patch_id) + + threads = [] + for thread in patch.mailthread_set.all(): + latest_attachment = thread.mailthreadattachment_set.first() + threads.append( + { + "messageid": thread.messageid, + "subject": thread.subject, + "latest_message_id": thread.latestmsgid, + "latest_message_time": thread.latestmessage, + "has_attachment": latest_attachment is not None, + } + ) + + return api_response( + { + "patch_id": patch.id, + "name": patch.name, + "threads": threads, + } + ) diff --git a/pgcommitfest/commitfest/tests/test_apiv1.py b/pgcommitfest/commitfest/tests/test_apiv1.py index f3b94595..10e1b718 100644 --- a/pgcommitfest/commitfest/tests/test_apiv1.py +++ b/pgcommitfest/commitfest/tests/test_apiv1.py @@ -1,7 +1,14 @@ import json +from datetime import datetime, timezone import pytest +from pgcommitfest.commitfest.models import ( + MailThread, + Patch, + PatchOnCommitFest, +) + pytestmark = pytest.mark.django_db @@ -44,3 +51,112 @@ def test_needs_ci_endpoint(client, commitfests): } assert data == expected + + +def test_commitfest_patches_endpoint(client, open_cf, alice, bob): + """Test the /api/v1/commitfests//patches endpoint.""" + # Create test patches + patch1 = Patch.objects.create(name="Add feature X") + patch1.authors.add(alice) + patch1.lastmail = datetime(2025, 1, 15, 10, 30, 0, tzinfo=timezone.utc) + patch1.save() + + patch2 = Patch.objects.create(name="Fix bug Y") + patch2.authors.add(alice, bob) + patch2.save() + + # Link patches to commitfest + PatchOnCommitFest.objects.create( + patch=patch1, + commitfest=open_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + PatchOnCommitFest.objects.create( + patch=patch2, + commitfest=open_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_AUTHOR, + ) + + response = client.get(f"/api/v1/commitfests/{open_cf.id}/patches") + + assert response.status_code == 200 + assert response["Content-Type"] == "application/json" + assert response["Access-Control-Allow-Origin"] == "*" + + data = json.loads(response.content) + + assert data["commitfest_id"] == open_cf.id + assert len(data["patches"]) == 2 + + # Patches are ordered by id + p1 = data["patches"][0] + assert p1["id"] == patch1.id + assert p1["name"] == "Add feature X" + assert p1["status"] == "Needs review" + assert p1["authors"] == ["Alice Anderson"] + assert p1["last_email_time"] == "2025-01-15T10:30:00+00:00" + + p2 = data["patches"][1] + assert p2["id"] == patch2.id + assert p2["name"] == "Fix bug Y" + assert p2["status"] == "Waiting on Author" + assert sorted(p2["authors"]) == ["Alice Anderson", "Bob Brown"] + assert p2["last_email_time"] is None + + +def test_commitfest_patches_endpoint_not_found(client, commitfests): + """Test the patches endpoint returns 404 for non-existent commitfest.""" + response = client.get("/api/v1/commitfests/99999/patches") + assert response.status_code == 404 + + +def test_patch_threads_endpoint(client, open_cf, alice): + """Test the /api/v1/patches//threads endpoint.""" + patch = Patch.objects.create(name="Test patch") + patch.authors.add(alice) + + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=open_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + # Create mail threads + thread1 = MailThread.objects.create( + messageid="abc123@example.com", + subject="[PATCH] Test patch v1", + firstmessage=datetime(2025, 1, 10, 9, 0, 0, tzinfo=timezone.utc), + firstauthor="alice@example.com", + latestmessage=datetime(2025, 1, 12, 14, 30, 0, tzinfo=timezone.utc), + latestauthor="bob@example.com", + latestsubject="Re: [PATCH] Test patch v1", + latestmsgid="def456@example.com", + ) + patch.mailthread_set.add(thread1) + + response = client.get(f"/api/v1/patches/{patch.id}/threads") + + assert response.status_code == 200 + assert response["Content-Type"] == "application/json" + + data = json.loads(response.content) + + assert data["patch_id"] == patch.id + assert data["name"] == "Test patch" + assert len(data["threads"]) == 1 + + t = data["threads"][0] + assert t["messageid"] == "abc123@example.com" + assert t["subject"] == "[PATCH] Test patch v1" + assert t["latest_message_id"] == "def456@example.com" + assert t["latest_message_time"] == "2025-01-12T14:30:00+00:00" + assert t["has_attachment"] is False + + +def test_patch_threads_endpoint_not_found(client, commitfests): + """Test the threads endpoint returns 404 for non-existent patch.""" + response = client.get("/api/v1/patches/99999/threads") + assert response.status_code == 404 diff --git a/pgcommitfest/urls.py b/pgcommitfest/urls.py index 95c988e4..67920a01 100644 --- a/pgcommitfest/urls.py +++ b/pgcommitfest/urls.py @@ -17,6 +17,8 @@ urlpatterns = [ re_path(r"^$", views.home), re_path(r"^api/v1/commitfests/needs_ci$", apiv1.commitfestst_that_need_ci), + re_path(r"^api/v1/commitfests/(\d+)/patches$", apiv1.commitfest_patches), + re_path(r"^api/v1/patches/(\d+)/threads$", apiv1.patch_threads), re_path(r"^help/$", views.help), re_path(r"^commitfest_history/$", views.commitfest_history), re_path(r"^me/$", views.me_legacy_redirect), From aea89ab2cd566379e0c2c28ef53b78ff4811e842 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Fri, 30 Jan 2026 11:30:55 +0100 Subject: [PATCH 34/38] Add a test paragraph --- pgcommitfest/commitfest/templates/help.html | 1 + 1 file changed, 1 insertion(+) diff --git a/pgcommitfest/commitfest/templates/help.html b/pgcommitfest/commitfest/templates/help.html index 5807add2..9abb4a91 100644 --- a/pgcommitfest/commitfest/templates/help.html +++ b/pgcommitfest/commitfest/templates/help.html @@ -54,4 +54,5 @@

Drafts

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.

+

This is a test paragragh, please ignore, it will be removed soon

{%endblock%} From 8e3c92a3e4099cab0fab4f7b2213229da1fd2622 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 3 Feb 2026 14:38:56 +0100 Subject: [PATCH 35/38] Revert "Add a test paragraph" This reverts commit aea89ab2cd566379e0c2c28ef53b78ff4811e842. --- pgcommitfest/commitfest/templates/help.html | 1 - 1 file changed, 1 deletion(-) diff --git a/pgcommitfest/commitfest/templates/help.html b/pgcommitfest/commitfest/templates/help.html index 9abb4a91..5807add2 100644 --- a/pgcommitfest/commitfest/templates/help.html +++ b/pgcommitfest/commitfest/templates/help.html @@ -54,5 +54,4 @@

Drafts

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.

-

This is a test paragragh, please ignore, it will be removed soon

{%endblock%} From 04aeb94f7037ec6f77537a86fec7771a448ed172 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 3 Feb 2026 14:41:15 +0100 Subject: [PATCH 36/38] Change default in original migration (no sql changes needed) --- pgcommitfest/userprofile/migrations/0002_notifications.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pgcommitfest/userprofile/migrations/0002_notifications.py b/pgcommitfest/userprofile/migrations/0002_notifications.py index 69eaf03a..0d4b0357 100644 --- a/pgcommitfest/userprofile/migrations/0002_notifications.py +++ b/pgcommitfest/userprofile/migrations/0002_notifications.py @@ -15,7 +15,7 @@ class Migration(migrations.Migration): model_name="userprofile", name="notify_all_author", field=models.BooleanField( - default=False, verbose_name="Notify on all where author" + default=True, verbose_name="Notify on all where author" ), ), migrations.AddField( From b33a3f5442e5c14b9c16163fae9685038fc4049c Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 3 Feb 2026 14:41:42 +0100 Subject: [PATCH 37/38] Fix formatting --- .../commitfest/templates/base_form.html | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/pgcommitfest/commitfest/templates/base_form.html b/pgcommitfest/commitfest/templates/base_form.html index 4a581329..d4bb7805 100644 --- a/pgcommitfest/commitfest/templates/base_form.html +++ b/pgcommitfest/commitfest/templates/base_form.html @@ -32,35 +32,35 @@ {%if field.help_text%}
{{field.help_text|safe}}{%endif%} {# "No tags apply" checkbox below the selectize (only for new patches) #} {%if form.no_tags_apply %} -
- - - {%if form.no_tags_apply.help_text%}{{form.no_tags_apply.help_text}}{%endif%} -
+
+ + + {%if form.no_tags_apply.help_text%}{{form.no_tags_apply.help_text}}{%endif%} +
{%endif%} {%elif field.name == 'no_tags_apply' %} {# Skip - rendered as part of the tags group above #} {%else%} -
- {{field|label_class:"control-label"}} -
- {%if field.errors %} - {%for e in field.errors%} -
{{e}}
- {%endfor%} - {%endif%} - {%if field.name|slice:":7" == 'review_' %} - {%for choice in field %} -
- - -
- {%endfor%} - {%elif not field.name in form.selectize_fields%}{{field|field_class:"form-control"}}{%else%}{{field}}{%endif%} - {%if field.help_text%}
{{field.help_text|safe}}{%endif%}
-
+
+ {{field|label_class:"control-label"}} +
+ {%if field.errors %} + {%for e in field.errors%} +
{{e}}
+ {%endfor%} + {%endif%} + {%if field.name|slice:":7" == 'review_' %} + {%for choice in field %} +
+ + +
+ {%endfor%} + {%elif not field.name in form.selectize_fields%}{{field|field_class:"form-control"}}{%else%}{{field}}{%endif%} + {%if field.help_text%}
{{field.help_text|safe}}{%endif%}
+
{%endif%} {%else%} {{field}} From fe0e31d4c0cc517b513759519642434e300ffb83 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Thu, 5 Feb 2026 08:26:34 +0100 Subject: [PATCH 38/38] Update responsive table css to include min-width On small screens (e.g. mobile) the Patch column especially became really narrow. --- media/commitfest/css/commitfest.css | 5 +++++ pgcommitfest/commitfest/templates/commitfest.html | 2 +- pgcommitfest/commitfest/templates/home.html | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/media/commitfest/css/commitfest.css b/media/commitfest/css/commitfest.css index aefd4346..21cbb2d6 100644 --- a/media/commitfest/css/commitfest.css +++ b/media/commitfest/css/commitfest.css @@ -227,9 +227,14 @@ a:hover .badge { .table td.wrap-text { overflow-wrap: break-word; word-break: break-word; + min-width: 100px; max-width: 300px; } +.table td.patch-column { + min-width: 150px; +} + /* Make the tags selectize dropdown taller with bottom padding for overlay */ #id_tags + .selectize-control .selectize-dropdown-content, #id_tag + .selectize-control .selectize-dropdown-content { diff --git a/pgcommitfest/commitfest/templates/commitfest.html b/pgcommitfest/commitfest/templates/commitfest.html index 0dcedb69..886712fc 100644 --- a/pgcommitfest/commitfest/templates/commitfest.html +++ b/pgcommitfest/commitfest/templates/commitfest.html @@ -53,7 +53,7 @@

{{p.is_open|yesno:"Active patches,Closed patches"}}

{%endifchanged%} - {{p.name}} + {{p.name}} {{p.id}} {{p.status|patchstatusstring}} diff --git a/pgcommitfest/commitfest/templates/home.html b/pgcommitfest/commitfest/templates/home.html index cabe1e3a..0bc45520 100644 --- a/pgcommitfest/commitfest/templates/home.html +++ b/pgcommitfest/commitfest/templates/home.html @@ -156,7 +156,7 @@

{%if user.is_authenticated%}Open patches you are subscribed to{%elif p.is_op {%endifchanged%} {%endif%} - {{p.name}} + {{p.name}} {{p.id}} {%if user.is_authenticated %} {{p.cf_name}}