Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions pgcommitfest/commitfest/apiv1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/<id>/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(
Expand Down
37 changes: 9 additions & 28 deletions pgcommitfest/commitfest/tests/test_apiv1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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):
Expand Down
Loading