- Notifications
You must be signed in to change notification settings - Fork52
added snippets changelog to the toolbar under admin menu#181
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 ourterms of service andprivacy 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?
added snippets changelog to the toolbar under admin menu#181
Uh oh!
There was an error while loading.Please reload this page.
Conversation
sourcery-aibot commentedMar 5, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Reviewer's Guide by SourceryThis pull request adds a 'Snippets' link to the admin menu in the CMS toolbar. This allows users to quickly access and manage snippets from the toolbar. Sequence diagram for adding Snippets link to admin menusequenceDiagram participant User participant CMSToolbar participant AdminMenu User->>CMSToolbar: Open CMS Toolbar CMSToolbar->>AliasToolbar: populate() AliasToolbar->>AliasToolbar: add_aliases_link_to_admin_menu() AliasToolbar->>CMSToolbar: get_or_create_menu(ADMIN_MENU_IDENTIFIER) CMSToolbar->>AdminMenu: Get or create Admin Menu AdminMenu-->>CMSToolbar: Returns AdminMenu AliasToolbar->>AliasToolbar: admin_reverse('djangocms_snippet_snippet_changelist') AliasToolbar->>AdminMenu: add_sideframe_item(_("Snippets"), url, position) AdminMenu-->>AliasToolbar: Returns AliasToolbar-->>CMSToolbar: Returns CMSToolbar-->>User: Renders toolbar with Snippets link File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess yourdashboard to:
Getting Help
|
for more information, seehttps://pre-commit.ci
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@svandeneertwegh - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a permission check to
add_aliases_link_to_admin_menu
to ensure only authorized users see the link.
Here's what I looked at during the review
- 🟢General issues: all looks good
- 🟢Security: all looks good
- 🟢Testing: all looks good
- 🟡Complexity: 1 issue found
- 🟢Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
position=self.get_insert_position(admin_menu, self.plural_name), | ||
) | ||
@classmethod |
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.
issue (complexity): Consider refactoring to separate the break setup and insertion point calculation into distinct helper functions, and use hasattr to check for attributes instead of try/except blocks, to improve code organization and readability..
You can reduce complexity by separating the two concerns: ensuring the breaks are set up and calculating the alphabetical insertion point. For instance, you could extract the break logic into its own helper and refactor the loop to check for the attribute rather than handling an exception. For example:
@classmethoddefensure_shortcuts_break(cls,admin_menu):start=admin_menu.find_first(Break,identifier=SHORTCUTS_BREAK)ifnotstart:end=admin_menu.find_first(Break,identifier=ADMINISTRATION_BREAK)admin_menu.add_break(SHORTCUTS_BREAK,position=end.index)start=admin_menu.find_first(Break,identifier=SHORTCUTS_BREAK)end=admin_menu.find_first(Break,identifier=ADMINISTRATION_BREAK)returnstart,end@classmethoddefcompute_insert_index(cls,admin_menu,start,end,item_name):items=admin_menu.get_items()[start.index+1:end.index]foridx,iteminenumerate(items):# Check if item has a name attribute instead of using try/except.ifhasattr(item,'name'):ifforce_str(item_name.lower())<force_str(item.name.lower()):returnidx+start.index+1returnend.index@classmethoddefget_insert_position(cls,admin_menu,item_name):start,end=cls.ensure_shortcuts_break(admin_menu)returncls.compute_insert_index(admin_menu,start,end,item_name)
This refactoring keeps functionality intact while isolating break management and order computation, increasing clarity and maintainability.
@svandeneertwegh Thanks for the PR! Can you add a check if the user has add or change permissions for snippets? Only then the menu entry makes sense... |
Uh oh!
There was an error while loading.Please reload this page.
Description
This PR adds the snippets manage url to the cms toolbar admin menu.
Related resources
Summary by Sourcery
Adds a link to the snippets changelist in the admin menu of the CMS toolbar, allowing users to manage snippets directly from the toolbar.