-
Notifications
You must be signed in to change notification settings - Fork 49
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
added snippets changelog to the toolbar under admin menu #181
base: master
Are you sure you want to change the base?
added snippets changelog to the toolbar under admin menu #181
Conversation
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 your dashboard to:
Getting Help
|
for more information, see https://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:
@classmethod
def ensure_shortcuts_break(cls, admin_menu):
start = admin_menu.find_first(Break, identifier=SHORTCUTS_BREAK)
if not start:
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)
return start, end
@classmethod
def compute_insert_index(cls, admin_menu, start, end, item_name):
items = admin_menu.get_items()[start.index + 1: end.index]
for idx, item in enumerate(items):
# Check if item has a name attribute instead of using try/except.
if hasattr(item, 'name'):
if force_str(item_name.lower()) < force_str(item.name.lower()):
return idx + start.index + 1
return end.index
@classmethod
def get_insert_position(cls, admin_menu, item_name):
start, end = cls.ensure_shortcuts_break(admin_menu)
return cls.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... |
for more information, see https://pre-commit.ci
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.