Opened 14 years ago

Closed 13 years ago

#13709 closed (duplicate)

Delete action is visble if no delete permission given [PATCH]

Reported by: Sebastian Noack Owned by: nobody
Component: contrib.admin Version: 1.2
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Admin actions are missing some permission checking hooks, in order to only show the actions that are usable. You can choose the delete action also if you don't have the delete permission in the admin, but it will raise PermissionDenied. However I think it would be nice to have an optional callback which determines wheter an action is available for a given request or not. My patch checks for a has_permission attribute on the action function and calls it in ModelAdmin.get_actions(), so it retruns only the actions, where permissions are granted.

Attachments (1)

admin_action_permission_check.patch (1.8 KB ) - added by Sebastian Noack 14 years ago.

Download all attachments as: .zip

Change History (4)

by Sebastian Noack, 14 years ago

comment:1 by Jacob, 14 years ago

Needs documentation: set
Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

I'm not sure I really like the way this is implemented - hanging information off the function feels a little nasty. But it's a good idea.

comment:2 by Sebastian Noack, 14 years ago

I don't like the way it is implemented, because of ...

  1. it is not consistent. The delete link in the change form is not there if you don't have the delete permission but the action in the changelist is there.
  2. you should never show functionality if it is not usable. This is just a waste of pixels (you can save if there weren't action checkboxes, in the case where no action is usable). And it confuses the user, especially if you have a lot of custom actions, which probably even are named similar, makes it harder for the user to choose the right action, if some of them are actually not permitted to use.

If hanging additional information as attribute at a function is nasty, what is about the short_description? ;) I have already thought about alternative implementations, e.g. passing two functions (the view function and the function checking the permissions) when registering an action. But this one is the most elegent method I could think about, especially because of in the admin quite often attributes are attached to callables for similiar things.

comment:3 by Julien Phalip, 13 years ago

Resolution: duplicate
Status: newclosed

Dupe of #10609.

Note: See TracTickets for help on using tickets.
Back to Top