[GrandComicsDatabase/gcd-django] Character ordering (PR #741)
JochenGCD
You can view, comment on, or merge this pull request online at:
https://github.com/GrandComicsDatabase/gcd-django/pull/741
Commit Summary
- 308b172 add multiple feature names
- 65bb0d4 display for feature names
- 5bedca8 feature name display changes
- ab2c71d feature logo updates
- debf7fa filter_issues does the filter, add feature logo with feature
- a06c5cc filter_issues does the filter
- ad2536e remove feature_object from editing
- c90086f fix feature logo list for feature
- dcc502e remove feature_object from editing logos
- c9c0679 copy
- 497bd6a Merge remote-tracking branch 'origin' into character-ordering
- 2cdd4be add feature description
File Changes
(24 files)
- A apps/gcd/migrations/0068_remove_issue_brand_from_db_add_multiple_feature_names.py (35)
- A apps/gcd/migrations/0069_add_feature_names_to_story.py (18)
- A apps/gcd/migrations/0070_feature_logo_2_feature_name.py (18)
- A apps/gcd/migrations/0071_feature_description.py (19)
- M apps/gcd/models/__init__.py (2)
- M apps/gcd/models/feature.py (71)
- M apps/gcd/models/story.py (9)
- M apps/gcd/urls.py (6)
- M apps/gcd/views/details.py (201)
- M apps/oi/forms/__init__.py (3)
- M apps/oi/forms/feature.py (100)
- M apps/oi/forms/story.py (27)
- M apps/oi/forms/support.py (2)
- A apps/oi/migrations/0063_remove_issue_brand_from_db_add_multiple_feature_names.py (53)
- A apps/oi/migrations/0064_add_feature_names_to_story.py (19)
- A apps/oi/migrations/0065_feature_logo_2_feature_name.py (19)
- A apps/oi/migrations/0066_featurerevision_description.py (18)
- M apps/oi/models.py (296)
- M apps/oi/templatetags/compare.py (33)
- M apps/oi/views.py (4)
- M apps/select/urls.py (5)
- M apps/select/views.py (69)
- M templates/gcd/details/tw_feature.html (14)
- M templates/gcd/details/tw_feature_logo.html (6)
Patch Links:
- https://github.com/GrandComicsDatabase/gcd-django/pull/741.patch
- https://github.com/GrandComicsDatabase/gcd-django/pull/741.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 support for multiple feature names by adding the FeatureNameDetail model and its revision counterpart, updating references across stories, logos, views, and forms. The review feedback identifies several critical issues, including the use of deprecated urlresolvers which will fail in Django 5.x, a potential AttributeError in FeatureNameDetailRevision due to a missing sort_name attribute, a missing import of Formset in the feature forms, and an optimization opportunity to avoid a redundant database query in feature_logo_feature_issues.
In apps/gcd/models/feature.py:
> + def get_absolute_url(self):
+ return urlresolvers.reverse(
+ 'show_feature',
+ kwargs={'feature_id': self.feature.id})
In Django 2.0+, django.core.urlresolvers was removed in favor of django.urls. Using urlresolvers.reverse will raise a NameError or ImportError in Django 5.x. Please import and use reverse from django.urls instead.
- def get_absolute_url(self):
- return urlresolvers.reverse(
- 'show_feature',
- kwargs={'feature_id': self.feature.id})
+ def get_absolute_url(self):
+ from django.urls import reverse
+ return reverse(
+ 'show_feature',
+ kwargs={'feature_id': self.feature.id})
In apps/gcd/models/feature.py:
> + def render_issue_count(self, record):
+ url = urlresolvers.reverse(
+ 'feature_logo_feature_issues',
+ kwargs={'feature_id': self.feature_id,
+ 'feature_logo_id': record.id})
+ return mark_safe('<a href="%s">%s</a>' % (url,
+ record.issue_count))
Similar to get_absolute_url, urlresolvers.reverse should be replaced with django.urls.reverse to ensure compatibility with Django 5.x.
- def render_issue_count(self, record):
- url = urlresolvers.reverse(
- 'feature_logo_feature_issues',
- kwargs={'feature_id': self.feature_id,
- 'feature_logo_id': record.id})
- return mark_safe('<a href="%s">%s</a>' % (url,
- record.issue_count))
+ def render_issue_count(self, record):
+ from django.urls import reverse
+ url = reverse(
+ 'feature_logo_feature_issues',
+ kwargs={'feature_id': self.feature_id,
+ 'feature_logo_id': record.id})
+ return mark_safe('<a href="%s">%s</a>' % (url,
+ record.issue_count))
> + def _imps_for(self, field_name): + if field_name == 'sort_name': + if self.sort_name == self.name: + return 0 + else: + return 1
FeatureNameDetailRevision does not have a sort_name attribute, so accessing self.sort_name will raise an AttributeError. Instead, you can compute the sort name based on self.name and self.leading_article using remove_leading_article if leading_article is True.
- def _imps_for(self, field_name): - if field_name == 'sort_name': - if self.sort_name == self.name: - return 0 - else: - return 1 + def _imps_for(self, field_name): + if field_name == 'sort_name': + sort_name = remove_leading_article(self.name) if self.leading_article else self.name + if sort_name == self.name: + return 0 + else: + return 1
> +from .support import (KeywordBaseForm, FEATURE_HELP_LINKS, HiddenInputWithHelp,
_get_comments_form_field, combine_reverse_relations,
GENERIC_ERROR_MESSAGE, _create_embedded_image_revision,
_save_runtime_embedded_image_revision)
Formset is used in the layout of FeatureRevisionForm but is not imported in this file, which will cause a NameError at runtime. Please import Formset from .support.
-from .support import (KeywordBaseForm, FEATURE_HELP_LINKS, HiddenInputWithHelp, - _get_comments_form_field, combine_reverse_relations, - GENERIC_ERROR_MESSAGE, _create_embedded_image_revision, - _save_runtime_embedded_image_revision) +from .support import (KeywordBaseForm, FEATURE_HELP_LINKS, HiddenInputWithHelp, + _get_comments_form_field, combine_reverse_relations, + GENERIC_ERROR_MESSAGE, _create_embedded_image_revision, + _save_runtime_embedded_image_revision, Formset)
> 'heading': 'for feature logo %s' % (feature_logo)
}
template = 'gcd/search/tw_list_sortable.html'
- table = IssueTable(issues,
- attrs={'class': 'sortable_listing'},
- template_name=TW_SORT_TABLE_TEMPLATE,
- order_by=('publication_date'))
+ table = _table_issues_list_or_grid(request, issues, context)
+ return generic_sortable_list(request, issues, table, template, context)
+
+
+def feature_logo_feature_issues(request, feature_logo_id, feature_id):
+ feature_logo = get_gcd_object(FeatureLogo, feature_logo_id)
+ feature = get_gcd_object(Feature, feature_id)
+
+ if feature_logo.feature.all()[0].feature_type.id == 1:
Since the feature object is already retrieved and passed into this function, you can directly access feature.feature_type.id instead of performing an unnecessary database query and list indexing with feature_logo.feature.all()[0].feature_type.id. This is both more efficient and safer (avoids potential IndexError).
- if feature_logo.feature.all()[0].feature_type.id == 1: + if feature.feature_type.id == 1:
—
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 pushed 1 commit.
- 30a62e1 Apply suggestion from @gemini-code-assist[bot]
—
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/gcd/models/feature.py:
> + def get_absolute_url(self):
+ return urlresolvers.reverse(
+ 'show_feature',
+ kwargs={'feature_id': self.feature.id})
import django.urls as urlresolvers
—
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.