search

idlesign / django-sitetree

Reusable application for Django introducing site tree, menu and breadcrumbs navigation elements.
http://github.com/idlesign/django-sitetree
BSD 3-Clause "New" or "Revised" License
348 stars 130 forks source link

Finer add permission check for displaying the "Add site tree Item" button #277

Open romaricpascal opened 4 years ago

romaricpascal commented 4 years ago

Hello,

When editing a Site Tree, it'd be great if there was a specific permission check deciding whether to show the "Add site tree item", say "has_add_tree_item_permission". This would allow to remove the control for adding new Trees in the admin, but allow adding new Tree item.

Ran into the issue trying to allow admins to just edit pre-configured menus, without risking them adding new ones for nothing.

idlesign commented 4 years ago

Hi,

Thank you for the idea.

Have you tried overriding ModelAdmin.has_add_permission() approach? It seem rather easy and flexible. Possibly more flexible than restrictions per-user.

You see, I'm afraid there's no convenient way to support that out of the box in sitetree. Since if we simply add a permission admin users will stil need to override the method mentioned above; and if we override it out-of-box, new users won't have it automatically and manual assignment will be required (that'll be not backward compatible).

What do you think?

romaricpascal commented 4 years ago

Hi,

I'm not sure I understand what you're proposing, and I realise I was not super detailed with my suggestion and the use case leading to it.

What I was trying to achieve is the following:

  1. Remove the possibility to add new Site Trees, by removing the buttons from the admin UI
  2. Maintain the possibility to add new Site Tree Items

Overriding ModelAdmin.has_add_permission() took care of #1, as expected. The unfortunate side effect, because the value is also used to decide the rendering of the button to a Site Tree Item, was to remove the "Add Site Tree Item" button too.

This is due to this if in the template: https://github.com/idlesign/django-sitetree/blob/09c9d2cc2264133a0f9475dfcb1c08f131781fa0/sitetree/templates/admin/sitetree/tree/change_form.html#L20

Rather than overriding the whole template to adapt the check (which might lead to discrepancies as the library gets update), it would have been ideal that the check was made on a separate context variable (the has_add_tree_item_permission I mentioned).

Its value could be overridden in the ModelAdmin, say by overriding a had_add_tree_item_permission() method. That method default implementation would return ModelAdmin.has_add_permission(), to keep its behaviour in line with what happens currently. Or maybe it could pull whatever has_add_permission the admin registered for SiteTreeItem, but that sounds lots of work for a marginal gain.

How does that sound?

idlesign commented 4 years ago

Ah, I see. Thank you for the clarification.

Adding new has_add_tree_item_permission() with backward compatible implementation sounds quite reasonable.

Pull request is welcome, if you will.