-
Notifications
You must be signed in to change notification settings - Fork 26
fix: Admin complained about missing methods get_author and get_modified_date #307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis refactors the AliasAdmin list_display setup by removing the old alias_admin_list_display and versioning flag logic, and instead overriding get_list_display to dynamically inject get_author and get_modified_date when available. Class diagram for updated AliasAdmin list_display logicclassDiagram
class AliasAdmin {
+list_display
+list_display_links
+list_filter
+get_queryset(request)
+get_list_display(request)
+used(obj)
}
AliasAdmin --|> GrouperModelAdmin
class GrouperModelAdmin {
}
class CategoryAdmin {
}
CategoryAdmin --|> TranslatableAdmin
class TranslatableAdmin {
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `djangocms_alias/admin.py:82-88` </location>
<code_context>
# Annotate each Alias with a boolean indicating if related cmsplugins exist
return qs.annotate(cmsplugins_count=models.Count("cms_plugins"))
+ def get_list_display(self, request):
+ list_display = super().get_list_display(request)
+ if hasattr(self, "get_author"):
+ list_display = list(list_display)
+ list_display.insert(-1, "get_author")
+ list_display.insert(-1, "get_modified_date")
+ return list_display
+
@admin.display(description=_("Used"), boolean=True, ordering="cmsplugins_count")
</code_context>
<issue_to_address>
**suggestion:** Consider checking for versioning enablement rather than method presence.
Checking for 'get_author' may not accurately reflect versioning status. Using the configuration flag ensures the list display is updated only when versioning is enabled.
Suggested implementation:
```python
def get_list_display(self, request):
list_display = super().get_list_display(request)
if getattr(self, "versioning_enabled", False):
list_display = list(list_display)
list_display.insert(-1, "get_author")
list_display.insert(-1, "get_modified_date")
return list_display
```
Make sure that your admin class has a `versioning_enabled` attribute or property that accurately reflects whether versioning is enabled. If the flag is named differently or is set elsewhere, adjust the attribute name accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Description
Avoid admin errors by conditionally including get_author and get_modified_date in the AliasAdmin list display and cleanup legacy versioning list configuration.
Bug Fixes:
Enhancements:
Fixes #302
Related resources
Checklist