django-sitetree icon indicating copy to clipboard operation
django-sitetree copied to clipboard

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

Open romaricpascal opened this issue 4 years ago • 3 comments

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.

romaricpascal avatar May 15 '20 14:05 romaricpascal

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?

idlesign avatar May 17 '20 02:05 idlesign

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?

romaricpascal avatar May 18 '20 09:05 romaricpascal

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.

idlesign avatar May 20 '20 02:05 idlesign