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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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)