[GrandComicsDatabase/gcd-django] Add public api-v2 publishers, series, and issues endpoints (PR #712)

20 views
Skip to first unread message

Adam Hernandez

unread,
May 3, 2026, 9:39:05 PMMay 3
to GrandComicsDatabase/gcd-django, Subscribed

Summary

This adds the first public data endpoints on api_v2:

  • publishers
  • series
  • issues

It also wires them into the www v2 surface, adds test coverage for filters/serializers/views/perf, and updates the v2 CI workflow to run lint + pytest for beta and api-v2.

Notes

  • urls_my.py stays unchanged in this pass
  • list/detail responses support ETag and Last-Modified
  • issues detail includes nested stories
  • SPDX headers were added to the v2 files created in this work

Performance follow-up

While testing against a local production-data clone, two broad-list cases stood out, so this branch also includes a couple of follow-up fixes:

  • issues list ordering is now pinned to series_id, sort_code, id
  • conditional list metadata now uses indexed modified lookups instead of MAX(modified)
  • series queryset projections were narrowed to cut down broad-list overhead

That brought the worst issues list path down from multi-second / tens-of-seconds territory into a much more reasonable range locally.

Verification

Ran locally in Docker:

  • ruff check apps/api_v2/
  • ruff format --check apps/api_v2/
  • python manage.py check
  • pytest apps/api_v2/tests/ -q --tb=short --reuse-db

Current result: 50 passed

I also did manual endpoint checks against a local production-data copy for:

  • public list/detail responses
  • schema / Swagger / ReDoc
  • conditional 304 behavior
  • broad-list timing sanity

You can view, comment on, or merge this pull request online at:

  https://github.com/GrandComicsDatabase/gcd-django/pull/712

Commit Summary

  • 8cc985c ci(api-v2): expand v2 workflow for beta and api-v2
  • 3d9f950 feat(api-v2): add publishers list and detail endpoints
  • f189166 feat(api-v2): add series list and detail endpoints
  • f6813cf feat(api-v2): add issue list and detail endpoints
  • 0fc0755 fix(api-v2): optimize issue list ordering and conditional metadata
  • 431f854 perf(api-v2): narrow series queryset projections
  • 049e413 chore(api-v2): add SPDX license headers to v2 files

File Changes

(44 files)

Patch Links:

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.Message ID: <GrandComicsDatabase/gcd-django/pull/712@github.com>

gemini-code-assist[bot]

unread,
May 3, 2026, 9:44:21 PMMay 3
to GrandComicsDatabase/gcd-django, Subscribed

@gemini-code-assist[bot] commented on this pull request.

Code Review

This pull request implements the v2 API for issues, publishers, and series, featuring comprehensive filtering, serialization, and conditional request support. The implementation includes optimized querysets and custom logic for handling legacy credit strings. Feedback identifies a performance regression in IssueViewSet where incomplete prefetching for variant covers causes N+1 queries. Additionally, concerns were raised about the maintenance burden of using .only() in SeriesViewSet and the potential performance costs in the credit collection utility's fallback logic.

In apps/api_v2/views/issues.py:

> +
+class IssueViewSet(GCDBaseViewSet):
+    """Read-only issue endpoints for the public v2 API surface."""
+
+    queryset = (
+        Issue.objects.select_related(
+            'series',
+            'indicia_publisher',
+            'variant_of',
+        )
+        .prefetch_related(
+            'brand_emblem',
+            'keywords',
+            ACTIVE_ISSUE_CREDIT_PREFETCH,
+            ACTIVE_COVER_PREFETCH,
+            'variant_of__cover_set',

high

The prefetch for variant_of__cover_set is currently a simple string. This will prefetch all related covers, including deleted ones, and won't apply the ordering expected by the serializer. This leads to an N+1 query issue in the _cover_url helper because it calls .filter(deleted=False).order_by('id') on the related manager, which bypasses the prefetch cache.

            ACTIVE_ISSUE_CREDIT_PREFETCH,
            ACTIVE_COVER_PREFETCH,
            Prefetch(
                'variant_of__cover_set',
                queryset=Cover.objects.filter(deleted=False).order_by('id'),
                to_attr='active_cover_list',
            ),
        )

In apps/api_v2/views/series.py:

> +        .only(
+            'id',
+            'created',
+            'modified',
+            'deleted',
+            'name',
+            'sort_name',
+            'year_began',
+            'year_ended',
+            'color',
+            'dimensions',
+            'paper_stock',
+            'binding',
+            'publishing_format',
+            'notes',
+            'issue_count',
+            'country_id',
+            'language_id',
+            'publisher_id',
+            'publication_type_id',
+            'country__id',
+            'country__code',
+            'language__id',
+            'language__code',
+            'publisher__id',
+            'publisher__name',
+            'publication_type__id',
+            'publication_type__name',
+        )

medium

Using .only() can be a maintenance burden. If the SeriesSerializer is updated to include new fields, this list must be manually updated to avoid deferred attribute loading, which causes N+1 query problems. Consider if the performance gain of narrowing the projection is worth the risk of future regressions.

In apps/api_v2/utils/credits.py:

> +    if credits is None:
+        credits = obj.credits.filter(deleted=False).select_related(
+            'creator',
+            'credit_type',
+        )

medium

The collect_credit_strings function falls back to a database query if the prefetched_attr is not found. While the current viewsets seem to provide this attribute via prefetching, this fallback logic performs a select_related on every call, which could be expensive if prefetching is accidentally omitted in future views.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <GrandComicsDatabase/gcd-django/pull/712/review/4217128516@github.com>

Adam Hernandez

unread,
May 3, 2026, 9:59:02 PMMay 3
to GrandComicsDatabase/gcd-django, Push

@DeusExTaco pushed 1 commit.

  • 76b5b4a fix(api-v2): prefetch variant base covers for issue cover URLs

—
View it on GitHub or unsubscribe.


Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <GrandComicsDatabase/gcd-django/pull/712/before/049e4134546257d80e60fbd0bcd26f4a82111771/after/76b5b4a6f7917480d0d7ffb9a8e736917b7cd75a@github.com>

Adam Hernandez

unread,
May 3, 2026, 9:59:56 PMMay 3
to GrandComicsDatabase/gcd-django, Subscribed

@DeusExTaco commented on this pull request.

In apps/api_v2/views/issues.py:

> +
+class IssueViewSet(GCDBaseViewSet):
+    """Read-only issue endpoints for the public v2 API surface."""
+
+    queryset = (
+        Issue.objects.select_related(
+            'series',
+            'indicia_publisher',
+            'variant_of',
+        )
+        .prefetch_related(
+            'brand_emblem',
+            'keywords',
+            ACTIVE_ISSUE_CREDIT_PREFETCH,
+            ACTIVE_COVER_PREFETCH,
+            'variant_of__cover_set',

Good catch. I switched the nested variant cover prefetch to an explicit
Prefetch(...) with the same deleted=False / order_by('id') behavior as
the primary issue cover prefetch, and attached it via active_cover_list so
_cover_url() can use the prefetched data directly.

I also added:

  • a view test covering variant issues that inherit the base issue cover URL
  • a performance test that checks the variant-cover path does not introduce
    per-row fallback cover queries

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <GrandComicsDatabase/gcd-django/pull/712/review/4217153614@github.com>

Adam Hernandez

unread,
May 3, 2026, 10:00:29 PMMay 3
to GrandComicsDatabase/gcd-django, Subscribed

@DeusExTaco commented on this pull request.

In apps/api_v2/views/series.py:

Fair point. The .only() here is a deliberate tradeoff rather than an
accidental micro-optimization.

I added it after profiling broad series list requests against a local
production-data clone, where narrowing the selected columns materially reduced
the response cost. The current serializer field set is covered by tests, so we
should catch accidental drift if the serializer changes.

I’m happy to drop it if you’d rather favor lower maintenance over the measured
list-performance gain, but I left it in because the improvement was
significant in local profiling.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <GrandComicsDatabase/gcd-django/pull/712/review/4217155373@github.com>

Adam Hernandez

unread,
May 3, 2026, 10:01:22 PMMay 3
to GrandComicsDatabase/gcd-django, Subscribed

@DeusExTaco commented on this pull request.

In apps/api_v2/utils/credits.py:

> +    if credits is None:
+        credits = obj.credits.filter(deleted=False).select_related(
+            'creator',
+            'credit_type',
+        )

Agreed that the fallback would be more expensive if it became part of a hot path.

In the current v2 endpoints, the relevant credit relationships are prefetched
before serialization, so the fallback is there as a defensive correctness path
rather than the normal execution path. I left it in so the helper still
behaves safely if it gets reused outside the fully-prefetched viewsets.

If we add more callers later, we can revisit whether to make that expectation
stricter.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <GrandComicsDatabase/gcd-django/pull/712/review/4217156434@github.com>

JochenGCD

unread,
May 8, 2026, 3:01:06 PM (11 days ago) May 8
to GrandComicsDatabase/gcd-django, Subscribed

Merged #712 into api-v2.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <GrandComicsDatabase/gcd-django/pull/712/issue_event/25319128563@github.com>

Reply all
Reply to author
Forward
0 new messages