API v2 phase 3 stories, reprints, and story arcs#716
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
| lookup_expr='icontains', | ||
| ) | ||
| type = django_filters.NumberFilter(field_name='type_id') | ||
| genre = django_filters.CharFilter(field_name='genre') |
There was a problem hiding this comment.
genre can also come from the genres of the feature
There was a problem hiding this comment.
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.
| """Serializer metadata for story detail fields.""" | ||
|
|
||
| fields = StoryListSerializer.Meta.fields + ( | ||
| 'feature_object', |
There was a problem hiding this comment.
Shouldn't existing text credit and character fields also be returned ? The function, but I don't see it being used ?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yes, that would be the way to proceed.
| for appearance in appearances | ||
| ] | ||
|
|
||
| def get_reprint_origins(self, obj): |
There was a problem hiding this comment.
Are you intentionally only returning reprints with known stories ? And no reprints where only the issue is known ?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
2 participants
Summary
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=shortManual performance notes
gcd.0072_story_reprint_api_v2_browse_indexesto the local production-copy Docker DB for measurement.title__icontainstext search.