[GrandComicsDatabase/gcd-django] Bugfix/issue 505 relations same object (PR #719)
jhunterjActual
Added checks for submitting relationships between an object and itself. #505
You can view, comment on, or merge this pull request online at:
https://github.com/GrandComicsDatabase/gcd-django/pull/719
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.
- 32bb4bb Incorporate Gemini Code Assist feedback
- 8f08811 Add error checks for submitting Relation edits between one object and itself. Issue 505
File Changes
(12 files)
- M apps/api/serializers.py (44)
- M apps/api/urls.py (4)
- M apps/api/views.py (40)
- M apps/gcd/models/issue.py (7)
- M apps/gcd/views/details.py (3)
- M apps/oi/forms/character.py (6)
- M apps/oi/forms/creator.py (4)
- M apps/oi/forms/feature.py (3)
- M apps/oi/forms/story.py (3)
- M apps/oi/models.py (34)
- M scripts/reset_stats.py (41)
- M templates/gcd/search/tw_list_sortable.html (9)
Patch Links:
- https://github.com/GrandComicsDatabase/gcd-django/pull/719.patch
- https://github.com/GrandComicsDatabase/gcd-django/pull/719.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 series overview API endpoint and a JSON download option, refactors series issue count caching and cross-series variant statistics, and adds validation to prevent self-relations across various forms. The review feedback highlights several critical improvements: ensuring form clean methods safely check for keys in cleaned_data to avoid potential KeyError exceptions, optimizing the series overview queryset with additional prefetches to prevent N+1 queries during serialization, and using update_fields when saving series counts to avoid concurrent write issues.
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/oi/forms/character.py:
> + if cd['from_character'] == cd['to_character']: + raise forms.ValidationError( + 'Character A and Character B cannot be the same character.')
Accessing cd['from_character'] and cd['to_character'] directly can raise a KeyError if either field fails validation and is omitted from cleaned_data. It is safer to check if both keys exist in cd before comparing them, similar to the pattern used in apps/oi/forms/creator.py.
- if cd['from_character'] == cd['to_character']: - raise forms.ValidationError( - 'Character A and Character B cannot be the same character.') + if 'from_character' in cd and 'to_character' in cd and cd['from_character'] == cd['to_character']: + raise forms.ValidationError( + 'Character A and Character B cannot be the same character.')
In apps/oi/forms/character.py:
> + if cd['from_group'] == cd['to_group']: + raise forms.ValidationError( + 'Group A and Group B cannot be the same group.')
Accessing cd['from_group'] and cd['to_group'] directly can raise a KeyError if either field fails validation and is omitted from cleaned_data. It is safer to check if both keys exist in cd before comparing them.
- if cd['from_group'] == cd['to_group']: - raise forms.ValidationError( - 'Group A and Group B cannot be the same group.') + if 'from_group' in cd and 'to_group' in cd and cd['from_group'] == cd['to_group']: + raise forms.ValidationError( + 'Group A and Group B cannot be the same group.')
> + if cd['from_feature'] == cd['to_feature']: + raise forms.ValidationError( + 'Feature A and Feature B cannot be the same feature.')
Accessing cd['from_feature'] and cd['to_feature'] directly can raise a KeyError if either field fails validation and is omitted from cleaned_data. It is safer to check if both keys exist in cd before comparing them.
- if cd['from_feature'] == cd['to_feature']: - raise forms.ValidationError( - 'Feature A and Feature B cannot be the same feature.') + if 'from_feature' in cd and 'to_feature' in cd and cd['from_feature'] == cd['to_feature']: + raise forms.ValidationError( + 'Feature A and Feature B cannot be the same feature.')
> + if cd['from_story_arc'] == cd['to_story_arc']: + raise forms.ValidationError( + 'Story Arc A and Story Arc B cannot be the same story arc.')
Accessing cd['from_story_arc'] and cd['to_story_arc'] directly can raise a KeyError if either field fails validation and is omitted from cleaned_data. It is safer to check if both keys exist in cd before comparing them.
- if cd['from_story_arc'] == cd['to_story_arc']: - raise forms.ValidationError( - 'Story Arc A and Story Arc B cannot be the same story arc.') + if 'from_story_arc' in cd and 'to_story_arc' in cd and cd['from_story_arc'] == cd['to_story_arc']: + raise forms.ValidationError( + 'Story Arc A and Story Arc B cannot be the same story arc.')
> + queryset=Story.objects.filter(type_id=19, deleted=False)
+ .order_by('-page_count', 'sequence_number')
+ .prefetch_related('credits__creator__creator'),
To avoid N+1 queries when serializing the longest_story field, we should select the type relation and prefetch credits__creator__type on the Story queryset. StorySerializer accesses these relations during serialization, which would otherwise trigger database queries for each story.
- queryset=Story.objects.filter(type_id=19, deleted=False)
- .order_by('-page_count', 'sequence_number')
- .prefetch_related('credits__creator__creator'),
+ queryset=Story.objects.filter(type_id=19, deleted=False)
+ .select_related('type')
+ .order_by('-page_count', 'sequence_number')
+ .prefetch_related('credits__creator__creator', 'credits__creator__type'),
> + 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()
When updating cached counts like issue_count on Series models, it is safer and more efficient to use update_fields=['issue_count']. This prevents overwriting other fields of the Series model that might have been modified concurrently.
- 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() + old_series.issue_count += 1 + old_series.save(update_fields=['issue_count']) + + # 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(update_fields=['issue_count'])
—
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.
- 04512b6 Add implementation for current bug report
—
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.
- 53067d1 Apply suggestions from code review
—
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.
Untangled my git checkout snafu, applied Gemini Code Review comments
—
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 commented on this pull request.
In apps/oi/forms/character.py:
> @@ -437,6 +437,9 @@ def clean(self):
else:
cd['relation_type'] = CharacterRelationType.objects.get(id=type)
+ if 'from_character' in cd and 'to_character' in cd and cd['from_character'] == cd['to_character']:
This line is longer than 80 chars ? Same for others.
—
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 9 commits.
- 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.
- 32bb4bb Incorporate Gemini Code Assist feedback
- 8f08811 Add error checks for submitting Relation edits between one object and itself. Issue 505
—
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 0 commits.
—
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.
- 99819f0 fix line lengths
—
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 apps/oi/forms/character.py:
> @@ -437,6 +437,9 @@ def clean(self):
else:
cd['relation_type'] = CharacterRelationType.objects.get(id=type)
+ if 'from_character' in cd and 'to_character' in cd and cd['from_character'] == cd['to_character']:
Updated.
—
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.
JochenGCD
jochengcd left a comment (GrandComicsDatabase/gcd-django#719)—
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.