[GrandComicsDatabase/gcd-django] Bugfix/issue 619 json export series overview (PR #718)
jhunterjActual
Adding the API pieces needed for a JSON download of the Cover and Main Story Overview page on a series, for #619 .
The button works the same as on the issue: you have to click on the (JSON) portion, even though that's the only format available here. And it's just in the upper left of the overview page, sitting there by itself. So perhaps there are better user experience choices?
Should this also be added for the issue overview for creators https://www.comics.org/creator/18/overview/ and/or the issue overview for features https://www.comics.org/feature/4340/overview/ ?
You can view, comment on, or merge this pull request online at:
https://github.com/GrandComicsDatabase/gcd-django/pull/718
Commit Summary
- 4b38446 Fix series issue count for cross-series variants (For #235 and #157)
- 132fae1 Fix cross-series variant issue counting
- 3d6fc63 Incorporate Gemini Code Assist feedback
- 777503b update comment blocks
- 13a6ffc Add api and button for JSON export of cover and main story series overview.
File Changes
(8 files)
- M apps/api/serializers.py (61)
- M apps/api/urls.py (3)
- M apps/api/views.py (33)
- M apps/gcd/models/issue.py (7)
- M apps/gcd/views/details.py (1)
- M apps/oi/models.py (28)
- M scripts/reset_stats.py (41)
- M templates/gcd/search/tw_list_sortable.html (12)
Patch Links:
- https://github.com/GrandComicsDatabase/gcd-django/pull/718.patch
- https://github.com/GrandComicsDatabase/gcd-django/pull/718.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 a new API endpoint and serializer to provide a JSON overview of a series' covers and main stories, alongside a download button on the series overview page. It also updates the caching and routing logic for cross-series variant statistics. The review feedback highlights critical N+1 query bottlenecks in the serializer and model, dead fallback logic in the serializer, a hardcoded URL path that should use Django's reverse utility, and invalid HTML nesting in the template.
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.
> + def get_cover_url(self, obj) -> str:
+ covers = obj.active_covers()
+ if covers:
+ cover = covers[0]
+ return cover.get_base_url() + ("/w400/%d.jpg" % cover.id)
+ return ""
N+1 Query Bottleneck
Calling obj.active_covers() inside the serializer field for every issue triggers a database query per row. For a series with $N$ issues, this results in $N$ extra queries. We can completely eliminate this by using Django's Prefetch in the view's queryset to pre-load the active covers in bulk.
- def get_cover_url(self, obj) -> str:
- covers = obj.active_covers()
- if covers:
- cover = covers[0]
- return cover.get_base_url() + ("/w400/%d.jpg" % cover.id)
- return ""
+ def get_cover_url(self, obj) -> str:
+ covers = getattr(obj, 'active_covers_list', [])
+ if covers:
+ cover = covers[0]
+ return cover.get_base_url() + ("/w400/%d.jpg" % cover.id)
+ return ""
> + def get_longest_story(self, obj):
+ from apps.gcd.models.story import Story
+
+ story_id = getattr(obj, 'longest_story_id', None)
+ if story_id:
+ try:
+ story = Story.objects.prefetch_related(
+ 'credits__creator__creator').get(id=story_id)
+ return StorySerializer(story).data
+ except Story.DoesNotExist:
+ pass
+
+ # Fallback: variant issues — check parent issue's longest story
+ if obj.variant_of_id:
+ story_qs = obj.variant_of.active_stories()\
+ .filter(type_id=19, deleted=False)\
+ .prefetch_related('credits__creator__creator')\
+ .order_by('-page_count')
+ if story_qs.exists():
+ return StorySerializer(story_qs[0]).data
+
+ return None
N+1 Query Bottleneck & Dead Fallback Logic
-
N+1 Query Issue: Calling
Story.objects.get(id=story_id)inside the serializer field for every issue triggers a database query per row. We can completely eliminate this by using Django'sPrefetchin the view's queryset to pre-load the longest stories in bulk. -
Dead Fallback Logic: The fallback check
if obj.variant_of_id:will never execute because the view's queryset explicitly filters out variants using.filter(variant_of=None). Thus,variant_of_idis alwaysNonehere.
def get_longest_story(self, obj): stories = getattr(obj, 'longest_story_list', []) if stories: return StorySerializer(stories[0]).data return None
> + def get_queryset(self):
+ series_id = self.kwargs['series_id']
+ series = get_object_or_404(Series, id=series_id, deleted=False)
+ return (
+ series.active_issues()
+ .filter(variant_of=None)
+ .annotate(
+ longest_story_id=Subquery(
+ Story.objects.filter(
+ issue_id=OuterRef('pk'),
+ type_id=19,
+ deleted=False,
+ ).values('pk').order_by('-page_count', 'sequence_number')[:1]
+ )
+ )
+ .select_related('series__publisher')
+ )
Query Optimization
Optimize the queryset using Prefetch to pre-load the active covers and the longest stories (including their creator credits) in bulk. This reduces the database roundtrips from $2N + 1$ to exactly 3 queries.
def get_queryset(self): from django.db.models import Prefetch from apps.gcd.models import Cover series_id = self.kwargs['series_id'] series = get_object_or_404(Series, id=series_id, deleted=False) return ( series.active_issues() .filter(variant_of=None) .prefetch_related( Prefetch( 'cover_set', queryset=Cover.objects.filter(deleted=False), to_attr='active_covers_list' ), Prefetch( 'story_set', queryset=Story.objects.filter(type_id=19, deleted=False) .order_by('-page_count', 'sequence_number') .prefetch_related('credits__creator__creator'), to_attr='longest_story_list' ) ) .select_related('series__publisher') )
> + for variant in variants: + # 1. Variant left behind: Goes from Standard -> Cross-Series (+1) + if variant.series == old_series and variant.series != new_series: + variant.series.issue_count += 1 + variant.series.save() + + # 2. Base issue returns: Goes from Cross-Series -> Standard (-1) + elif variant.series != old_series and variant.series == new_series: + if variant.series.issue_count > 0: + variant.series.issue_count -= 1 + variant.series.save()
N+1 Query and Object Sync Issue
- N+1 Queries: Accessing
variant.seriesinside the loop triggers a database query for each variant to fetch its series. Comparingvariant.series_idwithold_series_idandnew_series_idavoids these queries entirely. - In-memory Sync: Instead of calling
variant.series.save(), update and save the already-fetchedold_seriesandnew_seriesinstances directly to ensure in-memory state remains synchronized.
- for variant in variants: - # 1. Variant left behind: Goes from Standard -> Cross-Series (+1) - if variant.series == old_series and variant.series != new_series: - variant.series.issue_count += 1 - variant.series.save() - - # 2. Base issue returns: Goes from Cross-Series -> Standard (-1) - elif variant.series != old_series and variant.series == new_series: - if variant.series.issue_count > 0: - variant.series.issue_count -= 1 - variant.series.save() + old_series_id = old_series.id if old_series else None + new_series_id = self.issue.series_id + for variant in variants: + # 1. Variant left behind: Goes from Standard -> Cross-Series (+1) + if variant.series_id == old_series_id and variant.series_id != new_series_id: + old_series.issue_count += 1 + old_series.save() + + # 2. Base issue returns: Goes from Cross-Series -> Standard (-1) + elif variant.series_id != old_series_id and variant.series_id == new_series_id: + if new_series.issue_count > 0: + new_series.issue_count -= 1 + new_series.save()
> @@ -2292,6 +2292,7 @@ def series_overview(request, series_id):
'item_name': 'issue',
'plural_suffix': 's',
'heading': heading,
+ 'json_download_url': '/api/series/%d/overview/?format=json' % series_id,
Hardcoded URL Path
Avoid hardcoding the API URL path. Instead, name the route in apps/api/urls.py and use Django's reverse utility to dynamically generate the URL. This prevents broken links if the API routing structure changes in the future.
- 'json_download_url': '/api/series/%d/overview/?format=json' % series_id,
+ 'json_download_url': urlresolvers.reverse('series-overview-list', kwargs={'series_id': series_id}) + '?format=json',
In apps/api/urls.py:
> + path('series/<int:series_id>/overview/',
+ views.SeriesOverviewList.as_view()),
Missing Route Name
Add a name parameter to the path to allow reversing the URL dynamically using reverse().
path('series/<int:series_id>/overview/', views.SeriesOverviewList.as_view(), name='series-overview-list'),
In templates/gcd/search/tw_list_sortable.html:
> + <btn class="btn btn-blue-editing hover:bg-stone-200">
+ <div class="flex justify-center">
+ Download:
+ <a class="hover:bg-stone-400" href="{{ json_download_url }}"><span class="px-1">(JSON)</span></a>
+ </div>
+ </btn>
Invalid HTML Nesting & Non-standard Tag
- Non-standard Tag:
<btn>is not a standard HTML element. Use<a>or<button>instead. - Invalid Nesting: Nesting an
<a>tag inside a button-like element is invalid HTML because both are interactive elements. This can cause click events to fail or behave inconsistently across browsers.
Style the <a> tag directly as a button to keep the HTML valid and clean.
<div class="me-1">
<a class="btn btn-blue-editing hover:bg-stone-200 flex justify-center" href="{{ json_download_url }}">
Download: <span class="px-1 font-semibold">(JSON)</span>
</a>
</div>
—
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.
jhunterjActual
@jhunterjActual pushed 1 commit.
- 32bb4bb Incorporate Gemini Code Assist feedback
—
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.
jhunterjActual
@jhunterjActual commented on this pull request.
In templates/gcd/search/tw_list_sortable.html:
> + <btn class="btn btn-blue-editing hover:bg-stone-200">
+ <div class="flex justify-center">
+ Download:
+ <a class="hover:bg-stone-400" href="{{ json_download_url }}"><span class="px-1">(JSON)</span></a>
+ </div>
+ </btn>
Incorporated
—
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
jochengcd left a comment (GrandComicsDatabase/gcd-django#718)There is now a conflict with beta that needs to be resolved.
Did you address the gemini comments ? Without replying to these I can't easily what was done.
—
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.
jhunterjActual
@jhunterjActual pushed 1 commit.
- 511764d Merge branch 'beta' into bugfix/issue-619-JSON-export-series-overview
—
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.
jhunterjActual
jhunterjActual left a comment (GrandComicsDatabase/gcd-django#718)Sorry, forgot to mark the "resolves" when I committed the Gemini feedback changes earlier. I also fixed the line lengths when resolving the conflict just now.
—
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.
jhunterjActual
jhunterjActual left a comment (GrandComicsDatabase/gcd-django#718)The changes for the Gemini comments: 32bb4bb
—
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
jochengcd left a comment (GrandComicsDatabase/gcd-django#718)I didn't mean the resolve status, but also what was done. For my comments, I know the context. But the gemini comments I don't necessarily follow and don't see what was done afterwards. Which means I have to look at it from scratch, which takes time and effort.
—
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
jochengcd left a comment (GrandComicsDatabase/gcd-django#718)I don't think that we should add issue overview for creators or features to the existing API at this time. There is current work on an v2 of the API, so any API additions should be considered with that in mind.
—
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
—
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.