Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

added snippets changelog to the toolbar under admin menu #181

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c939e71
added snippets changelog to the toolbar under admin menu
svandeneertwegh Mar 5, 2025
3f1b78e
ci: auto fixes from pre-commit hooks
pre-commit-ci[bot] Mar 5, 2025
8c3d837
added snippets changelog to the toolbar under admin menu v2
svandeneertwegh Mar 5, 2025
387de15
ci: auto fixes from pre-commit hooks
pre-commit-ci[bot] Mar 5, 2025
065f908
added if added or change permissions are true
svandeneertwegh Mar 5, 2025
fbde4bc
ci: auto fixes from pre-commit hooks
pre-commit-ci[bot] Mar 5, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
added if added or change permissions are true
  • Loading branch information
@svandeneertwegh
svandeneertwegh committed Mar 5, 2025
commit 065f9082b45b4455374b4af5205e4ad00964d858
20 changes: 12 additions & 8 deletions src/djangocms_snippet/cms_toolbars.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ def populate(self):
def add_snippet_link_to_admin_menu(self):
admin_menu = self.toolbar.get_or_create_menu(ADMIN_MENU_IDENTIFIER)

url = admin_reverse("djangocms_snippet_snippet_changelist")
url = admin_reverse('djangocms_snippet_snippet_changelist')

admin_menu.add_sideframe_item(
_("Snippets"),
url=url,
position=self.get_insert_position(admin_menu, self.plural_name),
)
add_perm = self.request.user.has_perm('djangocms_snippet.add_snippet')
change_perm = self.request.user.has_perm('djangocms_snippet.change_snippet')

if add_perm or change_perm:
admin_menu.add_sideframe_item(
_("Snippets"),
url=url,
position=self.get_insert_position(admin_menu, self.plural_name),
)

@classmethod
Copy link
Contributor

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.
def get_insert_position(cls, admin_menu, item_name):
Expand All @@ -47,10 +51,10 @@ def get_insert_position(cls, admin_menu, item_name):
start = admin_menu.find_first(Break, identifier=SHORTCUTS_BREAK)
end = admin_menu.find_first(Break, identifier=ADMINISTRATION_BREAK)

items = admin_menu.get_items()[start.index + 1 : end.index]
items = admin_menu.get_items()[start.index + 1: end.index]
for idx, item in enumerate(items):
try:
if force_str(item_name.lower()) < force_str(item.name.lower()):
if force_str(item_name.lower()) < force_str(item.name.lower()): # noqa: E501
return idx + start.index + 1
except AttributeError:
# Some item types do not have a 'name' attribute.
Expand Down
Loading