From 8f5876f491bf285bea025a52a9af4d1eb6576283 Mon Sep 17 00:00:00 2001 From: Kevin Rocker Date: Mon, 26 Oct 2026 22:25:32 +0100 Subject: [PATCH] Always include threads in the per-commitfest patches endpoint GET /api/v1/commitfests//patches now returns each patch's threads unconditionally, so the ?include=threads parameter is removed. Unknown query parameters are ignored, so existing callers passing it keep working. --- pgcommitfest/commitfest/apiv1.py | 22 +++++------- pgcommitfest/commitfest/tests/test_apiv1.py | 37 +++++---------------- 2 files changed, 17 insertions(+), 42 deletions(-) diff --git a/pgcommitfest/commitfest/apiv1.py b/pgcommitfest/commitfest/apiv1.py index 9d6109ad..3d3ed15c 100644 --- a/pgcommitfest/commitfest/apiv1.py +++ b/pgcommitfest/commitfest/apiv1.py @@ -66,34 +66,28 @@ def commitfest_patches(request, cfid): This endpoint provides the data that cfbot previously scraped from the commitfest HTML page. - Threads for every patch can be included with ?include=threads. Unknown - tokens are ignored. + Each patch includes its threads, in the same shape as + /patches//threads. """ cf = get_object_or_404(CommitFest, pk=cfid) - include = {i.strip() for i in request.GET.get("include", "").split(",")} - include_threads = "threads" in include - pocs = ( PatchOnCommitFest.objects.filter(commitfest=cf) .select_related("patch") - .prefetch_related("patch__authors") - .order_by("patch__id") - ) - - if include_threads: - pocs = pocs.prefetch_related( + .prefetch_related( + "patch__authors", Prefetch( "patch__mailthread_set", queryset=MailThread.objects.with_has_attachment(), - ) + ), ) + .order_by("patch__id") + ) patches = [] for poc in pocs: entry = poc.to_json() - if include_threads: - entry["threads"] = poc.patch.mailthread_set.all() + entry["threads"] = poc.patch.mailthread_set.all() patches.append(entry) return api_response( diff --git a/pgcommitfest/commitfest/tests/test_apiv1.py b/pgcommitfest/commitfest/tests/test_apiv1.py index 539f29e1..6b58e392 100644 --- a/pgcommitfest/commitfest/tests/test_apiv1.py +++ b/pgcommitfest/commitfest/tests/test_apiv1.py @@ -174,7 +174,7 @@ def test_commitfest_patches_endpoint(client, open_cf, alice, bob): ) url = f"/api/v1/commitfests/{open_cf.id}/patches" - with CaptureQueriesContext(connection) as queries_without_threads: + with CaptureQueriesContext(connection) as queries: response = client.get(url) assert response.status_code == 200 @@ -205,14 +205,6 @@ def test_commitfest_patches_endpoint(client, open_cf, alice, bob): assert p2["enterdate"] == "2025-01-02T08:00:00+00:00" assert p2["leavedate"] == "2025-01-20T16:45:00+00:00" - # Threads are opt-in - assert "threads" not in p1 - - with CaptureQueriesContext(connection) as queries_with_threads: - response = client.get(f"{url}?include=threads") - - assert response.status_code == 200 - p1, p2 = json.loads(response.content)["patches"] assert p1["threads"] == [ { "messageid": "abc123@example.com", @@ -223,25 +215,14 @@ def test_commitfest_patches_endpoint(client, open_cf, alice, bob): } ] assert p2["threads"] == [] - # A single prefetch query for all patches, not one per patch - assert len(queries_with_threads) == len(queries_without_threads) + 1 - - -def test_commitfest_patches_endpoint_include_parsing(client, open_cf): - """Unknown include tokens are ignored and spaces around tokens allowed.""" - PatchOnCommitFest.objects.create( - patch=Patch.objects.create(name="Add feature X"), - commitfest=open_cf, - enterdate=datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc), - ) - url = f"/api/v1/commitfests/{open_cf.id}/patches" - - response = client.get(f"{url}?include=bogus") - assert response.status_code == 200 - assert "threads" not in json.loads(response.content)["patches"][0] - - response = client.get(f"{url}?include=bogus, threads") - assert json.loads(response.content)["patches"][0]["threads"] == [] + # Threads (with has_attachment) are fetched in a single prefetch query for + # all patches, not one per patch + thread_queries = [ + q + for q in queries.captured_queries + if "commitfest_mailthreadattachment" in q["sql"] + ] + assert len(thread_queries) == 1 def test_commitfest_patches_endpoint_not_found(client, commitfests):