Opened 14 years ago
Closed 14 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)
Change History (4)
by , 14 years ago
Attachment: | admin_action_permission_check.patch added |
---|
comment:1 by , 14 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 14 years ago
I don't like the way it is implemented, because of ...
- 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.
- 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.
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.