Opened 8 years ago

Last modified 11 months ago

#11383 new Bug

Admin action 'Delete selected' check only global model delete permission

Reported by: krejcik@… Owned by:
Component: contrib.admin Version: master
Severity: Normal Keywords: delete permission admin
Cc: barton@…, Florian Apolloner, bas@…, IanMLewis@…, nils@…, kmike84@…, adi@…, tomc Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Action 'delete_selected' calls ModelAdmin's has_delete_permission method only once without obj argument.
(This action is run from object list with checked records)
It is problem if has_delete_permission contains more complex logic which returns different values for a particular objects.
If one of deleted objects must not be delete whole action should fail.

Simple workaround is always forbid global delete (it means return False if obj argument is not given) and allow delete only for specified objects.
But such solutuion still disallow to do multiple delete on objects which can be deleted separately from it's detail form.

Change History (16)

comment:1 Changed 8 years ago by whiskybar

Summary: Admin actiion 'Delete selected' check only global model delete permissionAdmin action 'Delete selected' check only global model delete permission

Since no one has commented on this issue, I will try to put it another way.

Deleting objects in the admin is inconsistent between

  • deleting object by the action delete_selected
  • deleting object from the detail view in the change form

The action delete_selected does not check has_delete_permission for each selected object. Instead, it calls has_delete_permission for all objects.
On the other hand, the admin will check if one has permission to delete the specific object in the view (the change form).

You have to disable the action delete_selected virtually if has_delete_permission is in effect. In my humble opinion, the admin should call has_delete_permission for each selected object with the action delete_selected.

comment:2 Changed 8 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

comment:3 Changed 8 years ago by Florian Apolloner

Cc: Florian Apolloner added

comment:4 Changed 7 years ago by Vasily Ivanov

Cc: bas@… added

comment:5 Changed 7 years ago by Ian Lewis

Cc: IanMLewis@… added

comment:6 Changed 7 years ago by Nils Fredrik Gjerull

Cc: nils@… added

comment:7 Changed 7 years ago by Julien Phalip

Related issue: #13539.

comment:8 Changed 7 years ago by Julien Phalip

Check #10609 for yet another related issue.

comment:9 Changed 6 years ago by Julien Phalip

Severity: Normal
Type: Bug

comment:10 Changed 6 years ago by cyrus

Easy pickings: unset
Owner: changed from nobody to cyrus
Status: newassigned
UI/UX: unset

comment:11 Changed 6 years ago by Mikhail Korobov

Cc: kmike84@… added

comment:12 Changed 5 years ago by cyrus

Owner: cyrus deleted
Status: assignednew

comment:13 Changed 4 years ago by Adi J. Sieker

Cc: adi@… added

comment:14 Changed 3 years ago by tomc

Cc: tomc added

comment:15 Changed 11 months ago by Al Johri

Running into exactly this issue. Is this documented somewhere?

comment:16 Changed 11 months ago by adsworth

What would the preferred UX be for this?

  1. on POST, abort on the first object for which modeladmin.has_permission returns False, with a message that the user doesn't have permission to delete the obj.
  2. on POST, iterate over all objects in the queryset and collect the ones modeladmin.has_permission returns False for. If any can't be deleted then abort the action with a list of records which couldn't be delete.
  3. on POST, iterate over all objects in the queryset and only delete the ones modeladmin.has_permission returns True for and add an the admin message for the user stating that N objects were skipped.
  4. on POST, iterate over all objects in the queryset and collect the ones modeladmin.has_permission returns False for. If any can't be deleted display them to the user and ask him if he want's to delete the other ones.
  5. on the confirm deletion page split the list in two parts. First part lists the deletable objects, second part lists the non deletable objects. Then on POST submit the deletable object IDs and only delete those.

If we start checking for object permissions here, this actually open the whole box of how to handle the related objects. All the related objects are already iterated using admin.utils.get_deleted_objects, but this only calls user.has_perm

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