[GrandComicsDatabase/gcd-django] API v2 phase 3 stories, reprints, and story arcs (PR #716)
Adam Hernandez
Summary
- Add public API v2 endpoints for stories, reprints, and story arcs, plus structured story credit output.
- Keep the Sprint 3 routes on the public surface only and absent from MYCOMICS routing/schema.
- Add browse indexes and modified-delta no-count pagination for the heaviest story/reprint list paths.
Validation
ruff check apps/api_v2/ apps/gcd/migrations/0072_story_reprint_api_v2_browse_indexes.pyruff format --check apps/api_v2/ apps/gcd/migrations/0072_story_reprint_api_v2_browse_indexes.pypython manage.py checkpython -m pytest apps/api_v2/tests/ -v --tb=shortpython -m pytest apps/gcd/tests/test_story.py apps/gcd/tests/test_reprint.py -q --tb=short
Manual performance notes
- Applied migration
gcd.0072_story_reprint_api_v2_browse_indexesto the local production-copy Docker DB for measurement. - Optimized JSON harness runs: stories list about 306 ms, stories page 5 about 284 ms, stories modified delta about 11 ms.
- Optimized JSON harness runs: reprints list about 126 ms, reprints page 5 about 90 ms, reprints modified delta about 14 ms.
- Remaining known slow path: exact count for
title__icontainstext search.
You can view, comment on, or merge this pull request online at:
https://github.com/GrandComicsDatabase/gcd-django/pull/716
Commit Summary
- 6270867 feat(api-v2): add structured story credit helper
- d76579b feat(api-v2): add stories endpoint
- 0b0944c feat(api-v2): add reprints endpoint
- ec8ddb5 feat(api-v2): add story arcs endpoint
- 2c2046e feat(api-v2): optimize story and reprint browse
File Changes
(32 files)
- A apps/api_v2/filters/reprints.py (40)
- A apps/api_v2/filters/stories.py (39)
- A apps/api_v2/filters/story_arcs.py (27)
- M apps/api_v2/pagination.py (25)
- A apps/api_v2/serializers/reprints.py (67)
- A apps/api_v2/serializers/stories.py (200)
- A apps/api_v2/serializers/story_arcs.py (89)
- A apps/api_v2/tests/test_credits.py (210)
- A apps/api_v2/tests/test_filters/test_reprints.py (232)
- A apps/api_v2/tests/test_filters/test_stories.py (216)
- A apps/api_v2/tests/test_filters/test_story_arcs.py (104)
- A apps/api_v2/tests/test_performance/test_reprints.py (123)
- A apps/api_v2/tests/test_performance/test_stories.py (233)
- A apps/api_v2/tests/test_performance/test_story_arcs.py (85)
- A apps/api_v2/tests/test_serializers/test_reprints.py (151)
- A apps/api_v2/tests/test_serializers/test_stories.py (304)
- A apps/api_v2/tests/test_serializers/test_story_arcs.py (199)
- M apps/api_v2/tests/test_url_dispatch.py (71)
- A apps/api_v2/tests/test_views/test_reprints.py (304)
- A apps/api_v2/tests/test_views/test_stories.py (473)
- A apps/api_v2/tests/test_views/test_story_arcs.py (198)
- M apps/api_v2/urls_www.py (6)
- M apps/api_v2/utils/credits.py (47)
- M apps/api_v2/views/__init__.py (27)
- A apps/api_v2/views/reprints.py (109)
- A apps/api_v2/views/stories.py (182)
- A apps/api_v2/views/story_arcs.py (108)
- A apps/gcd/migrations/0072_story_reprint_api_v2_browse_indexes.py (34)
- M apps/gcd/models/reprint.py (6)
- M apps/gcd/models/story.py (10)
- M apps/gcd/tests/test_reprint.py (13)
- M apps/gcd/tests/test_story.py (19)
Patch Links:
- https://github.com/GrandComicsDatabase/gcd-django/pull/716.patch
- https://github.com/GrandComicsDatabase/gcd-django/pull/716.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
gemini-code-assist[bot]
@gemini-code-assist[bot] commented on this pull request.
Code Review
This pull request introduces new API v2 endpoints, filters, serializers, and tests for stories, reprints, and story arcs, along with database indexes to optimize browsing and delta sync queries. The code review feedback highlights three performance and robustness improvements: resolving a potential N+1 query bottleneck in the story arc serializer by prefetching reprint relations, optimizing database queries in the story viewset by removing an unnecessary SQL JOIN on the character model, and adding a defensive guard check in the feature object serializer to prevent a potential AttributeError when the feature type is null.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
In apps/api_v2/serializers/story_arcs.py:
> +def _primary_stories(story_arc): + """Return ordered primary story memberships for ``story_arc``.""" + stories = getattr(story_arc, 'active_story_arc_story_list', None) + if stories is None: + stories, _reprinted_stories = story_arc.stories() + return [ + story + for story in stories + if not _is_same_language_reprint(story, story_arc.language_id) + ]
In _primary_stories, if active_story_arc_story_list is not prefetched (i.e., stories is None), the code falls back to calling story_arc.stories(). However, story_arc.stories() does not prefetch the from_all_reprints relation. This causes _is_same_language_reprint to execute a separate database query for every story in the list to check for same-language reprints, leading to a severe N+1 query performance bottleneck.
To resolve this, we should explicitly prefetch from_all_reprints with origin_issue__series when stories is fetched dynamically.
def _primary_stories(story_arc): """Return ordered primary story memberships for 'story_arc'.""" stories = getattr(story_arc, 'active_story_arc_story_list', None) if stories is None: from django.db.models import Prefetch from apps.gcd.models import Reprint stories, _reprinted_stories = story_arc.stories() stories = stories.prefetch_related( Prefetch( 'from_all_reprints', queryset=Reprint.objects.select_related('origin_issue__series'), to_attr='active_story_arc_reprint_list', ) ) return [ story for story in stories if not _is_same_language_reprint(story, story_arc.language_id) ]
In apps/api_v2/views/stories.py:
> +ACTIVE_STORY_CHARACTER_PREFETCH = Prefetch( + 'appearing_characters', + queryset=StoryCharacter.objects.filter(deleted=False) + .select_related( + 'character__character', + 'role', + ) + .order_by( + 'character__sort_name', + 'character__id', + ), + to_attr='active_character_list', +)
In ACTIVE_STORY_CHARACTER_PREFETCH, the select_related call includes 'character__character'. However, the StorySerializer.get_characters method only accesses appearance.character.character_id (which is the local foreign key on CharacterNameDetail) and appearance.character.name (which is a field on CharacterNameDetail). It does not access any fields on the parent Character model itself.
By changing 'character__character' to 'character', we can avoid an unnecessary SQL JOIN with the heavily populated Character table, improving query performance.
-ACTIVE_STORY_CHARACTER_PREFETCH = Prefetch( - 'appearing_characters', - queryset=StoryCharacter.objects.filter(deleted=False) - .select_related( - 'character__character', - 'role', - ) - .order_by( - 'character__sort_name', - 'character__id', - ), - to_attr='active_character_list', -) +ACTIVE_STORY_CHARACTER_PREFETCH = Prefetch( + 'appearing_characters', + queryset=StoryCharacter.objects.filter(deleted=False) + .select_related( + 'character', + 'role', + ) + .order_by( + 'character__sort_name', + 'character__id', + ), + to_attr='active_character_list', +)
In apps/api_v2/serializers/stories.py:
> + def get_feature_type(self, obj):
+ """Return the minimal nested feature type reference."""
+ return {
+ 'id': obj.feature_type_id,
+ 'name': obj.feature_type.name,
+ }
In FeatureObjectSerializer.get_feature_type, obj.feature_type is accessed directly to retrieve its name. If feature_type is nullable or missing due to data integrity issues, this will raise an AttributeError.
To adhere to defensive programming practices, we should add a guard check to ensure obj.feature_type is not None before accessing its properties.
- def get_feature_type(self, obj):
- """Return the minimal nested feature type reference."""
- return {
- 'id': obj.feature_type_id,
- 'name': obj.feature_type.name,
- }
+ def get_feature_type(self, obj):
+ """Return the minimal nested feature type reference."""
+ if obj.feature_type_id is None or obj.feature_type is None:
+ return None
+ return {
+ 'id': obj.feature_type_id,
+ 'name': obj.feature_type.name,
+ }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
@DeusExTaco commented on this pull request.
In apps/api_v2/serializers/story_arcs.py:
> +def _primary_stories(story_arc): + """Return ordered primary story memberships for ``story_arc``.""" + stories = getattr(story_arc, 'active_story_arc_story_list', None) + if stories is None: + stories, _reprinted_stories = story_arc.stories() + return [ + story + for story in stories + if not _is_same_language_reprint(story, story_arc.language_id) + ]
I added a fallback Prefetch for from_all_reprints with origin_issue__series when the serializer is used outside the view-prefetched path, and added a direct serializer query-count regression test.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
@DeusExTaco commented on this pull request.
In apps/api_v2/views/stories.py:
> +ACTIVE_STORY_CHARACTER_PREFETCH = Prefetch( + 'appearing_characters', + queryset=StoryCharacter.objects.filter(deleted=False) + .select_related( + 'character__character', + 'role', + ) + .order_by( + 'character__sort_name', + 'character__id', + ), + to_attr='active_character_list', +)
The serializer only needs CharacterNameDetail and role, so I changed the prefetch and fallback query set from character__character to character and added a test locking the select-related shape.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
@DeusExTaco commented on this pull request.
In apps/api_v2/serializers/stories.py:
> + def get_feature_type(self, obj):
+ """Return the minimal nested feature type reference."""
+ return {
+ 'id': obj.feature_type_id,
+ 'name': obj.feature_type.name,
+ }
Addressed defensively. Feature.feature_type is non-null in the model, but the serializer now returns feature_type: null for malformed objects with a missing or dangling feature type, with coverage for both cases.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
@DeusExTaco pushed 3 commits.
- f8ae628 fix(api-v2): prefetch story arc fallback reprints
- a0d07cb fix(api-v2): trim story character prefetch join
- e4d7b1b fix(api-v2): guard missing story feature types
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
JochenGCD
@jochengcd commented on this pull request.
In apps/api_v2/filters/stories.py:
> +from apps.api_v2.filters.common import ( + TIMESTAMP_FILTER_FIELDS, + TimestampFilterSet, +) +from apps.gcd.models import Story + + +class StoryFilterSet(TimestampFilterSet): + """Filters for story list endpoints.""" + + title = django_filters.CharFilter( + field_name='title', + lookup_expr='icontains', + ) + type = django_filters.NumberFilter(field_name='type_id') + genre = django_filters.CharFilter(field_name='genre')
genre can also come from the genres of the feature
In apps/api_v2/serializers/stories.py:
> + feature_logo = serializers.SerializerMethodField() + credits = serializers.SerializerMethodField() + characters = serializers.SerializerMethodField() + keywords = serializers.SlugRelatedField( + many=True, + read_only=True, + slug_field='name', + ) + reprint_origins = serializers.SerializerMethodField() + reprint_targets = serializers.SerializerMethodField() + + class Meta(StoryListSerializer.Meta): + """Serializer metadata for story detail fields.""" + + fields = StoryListSerializer.Meta.fields + ( + 'feature_object',
Shouldn't existing text credit and character fields also be returned ? The function, but I don't see it being used ?
In apps/api_v2/serializers/stories.py:
> + return [
+ {
+ 'character': {
+ 'id': appearance.character.character_id,
+ 'name': appearance.character.name,
+ },
+ 'role': (
+ appearance.role.name
+ if appearance.role_id is not None
+ else None
+ ),
+ }
+ for appearance in appearances
+ ]
+
+ def get_reprint_origins(self, obj):
Are you intentionally only returning reprints with known stories ? And no reprints where only the issue is known ?
In apps/api_v2/serializers/stories.py:
> + def get_feature_type(self, obj):
+ """Return the minimal nested feature type reference."""
+ return {
+ 'id': obj.feature_type_id,
+ 'name': obj.feature_type.name,
+ }
Doesn't hurt, but a bit overly defensive in this case.
In some sense, we would like to get errors if there are data integrity issues, otherwise we would never see them. But we would have these on the main site anyway.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
@DeusExTaco commented on this pull request.
In apps/api_v2/filters/stories.py:
> +from apps.api_v2.filters.common import ( + TIMESTAMP_FILTER_FIELDS, + TimestampFilterSet, +) +from apps.gcd.models import Story + + +class StoryFilterSet(TimestampFilterSet): + """Filters for story list endpoints.""" + + title = django_filters.CharFilter( + field_name='title', + lookup_expr='icontains', + ) + type = django_filters.NumberFilter(field_name='type_id') + genre = django_filters.CharFilter(field_name='genre')
Good catch. The v2 filter is only checking Story.genre, but the existing site/search path also treats feature genres as story genre content. I’ll update the filter to include feature_object__genre and cover it with tests.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
@DeusExTaco commented on this pull request.
In apps/api_v2/serializers/stories.py:
> + feature_logo = serializers.SerializerMethodField() + credits = serializers.SerializerMethodField() + characters = serializers.SerializerMethodField() + keywords = serializers.SlugRelatedField( + many=True, + read_only=True, + slug_field='name', + ) + reprint_origins = serializers.SerializerMethodField() + reprint_targets = serializers.SerializerMethodField() + + class Meta(StoryListSerializer.Meta): + """Serializer metadata for story detail fields.""" + + fields = StoryListSerializer.Meta.fields + ( + 'feature_object',
You’re right. The direct story endpoint currently exposes normalized structured credits/appearances, but it can hide legacy-only text data. I’d like to keep the structured fields and add explicit plain legacy fields so the API stays lossless without mixing object and text shapes. What do you think?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
> + return [
+ {
+ 'character': {
+ 'id': appearance.character.character_id,
+ 'name': appearance.character.name,
+ },
+ 'role': (
+ appearance.role.name+ if appearance.role_id is not None + else None + ), + } + for appearance in appearances + ] + + def get_reprint_origins(self, obj):
It was intentional in the first pass: those fields only list story-level reprints, while issue-level reprints are represented on /api/v2/reprints/. But I agree the current story detail shape makes that omission too implicit. I think we should either rename these as story-level fields or switch/add object-shaped reprint refs that can include nullable story plus issue data. Thoughts?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
@DeusExTaco pushed 1 commit.
- cf723bf fix(api-v2): include feature genres in story filter
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
JochenGCD
@jochengcd commented on this pull request.
In apps/api_v2/serializers/stories.py:
> + feature_logo = serializers.SerializerMethodField() + credits = serializers.SerializerMethodField() + characters = serializers.SerializerMethodField() + keywords = serializers.SlugRelatedField( + many=True, + read_only=True, + slug_field='name', + ) + reprint_origins = serializers.SerializerMethodField() + reprint_targets = serializers.SerializerMethodField() + + class Meta(StoryListSerializer.Meta): + """Serializer metadata for story detail fields.""" + + fields = StoryListSerializer.Meta.fields + ( + 'feature_object',
Yes, that would be the way to proceed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
JochenGCD
> + return [
+ {
+ 'character': {
+ 'id': appearance.character.character_id,
+ 'name': appearance.character.name,
+ },
+ 'role': (
+ appearance.role.name+ if appearance.role_id is not None + else None + ), + } + for appearance in appearances + ] + + def get_reprint_origins(self, obj):
Both options are valid.
But I think that reprint refs that can include nullable story plus issue data are the way to go, replicating the db and display.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
@DeusExTaco pushed 1 commit.
- 6b23810 fix(api-v2): expose complete story detail reprints
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
@DeusExTaco commented on this pull request.
In apps/api_v2/serializers/stories.py:
> + feature_logo = serializers.SerializerMethodField() + credits = serializers.SerializerMethodField() + characters = serializers.SerializerMethodField() + keywords = serializers.SlugRelatedField( + many=True, + read_only=True, + slug_field='name', + ) + reprint_origins = serializers.SerializerMethodField() + reprint_targets = serializers.SerializerMethodField() + + class Meta(StoryListSerializer.Meta): + """Serializer metadata for story detail fields.""" + + fields = StoryListSerializer.Meta.fields + ( + 'feature_object',
Made the changes.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Adam Hernandez
> + return [
+ {
+ 'character': {
+ 'id': appearance.character.character_id,
+ 'name': appearance.character.name,
+ },
+ 'role': (
+ appearance.role.name+ if appearance.role_id is not None + else None + ), + } + for appearance in appearances + ] + + def get_reprint_origins(self, obj):
Made the changes.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.