Opened 10 years ago

Closed 8 years ago

#21734 closed Bug (fixed)

admin's delete_selected action doesn't catch ProtectedError

Reported by: sander@… Owned by: Akshesh Doshi
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Sander Steffann Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When deleting objects through the admin interface ProtectedError exceptions aren't handled. This patch catches such exceptions and displays an error message instead.

Attachments (2)

django-admin-protectederror.patch​ (1.5 KB ) - added by sander@… 10 years ago.
Patch to show error message in admin on ProtectedError
21734-test.diff​ (946 bytes ) - added by Tim Graham 8 years ago.

Download all attachments as: .zip

Change History (16)

by sander@…, 10 years ago

Attachment: django-admin-protectederror.patch​ added

Patch to show error message in admin on ProtectedError

comment:1 by Sander Steffann, 10 years ago

Cc: Sander Steffann added

comment:2 by Tim Graham, 10 years ago

Easy pickings: unset
Needs tests: set

Is this error present on 1.6 and/or master? Is the issue different from #19838? A regression test will be required in order to commit the fix.

comment:3 by Tim Graham, 10 years ago

Triage Stage: Unreviewed → Accepted

Tentatively accepting pending an answer to the above question.

in reply to:  2 comment:4 by ANUBHAV JOSHI, 10 years ago

Replying to timo:

Is this error present on 1.6 and/or master? Is the issue different from #19838? A regression test will be required in order to commit the fix.

I can't see ProtectedError being handled anywhere in actions.py in master.

comment:5 by afuna, 10 years ago

It does not appear to the be the same as #19838; this ticket looks like it's for "delete all selected" dropdown from the top of the page.

The good thing is, there does appear to be some protection in place already. You have to go through an intermediate page to confirm, and if a protected (nested) object is detected, then the button to confirm deletion is simply not there, so you normally can't do a delete.

You can still trigger the ProtectedError by skipping the intermediate page, that might be what happened here. The other possibility is that the check for protected via get_deleted_objects is missing something (but without more information that'll be harder to track down)

comment:6 by ANUBHAV JOSHI, 10 years ago

As far as I know that intermediate page has nothing to do here.

comment:7 by Tim Graham, 10 years ago

Resolution: → needsinfo
Status: new → closed

Closing as needsinfo absent additional details from the reporter.

comment:8 by Tim Graham, 8 years ago

Needs tests: unset
Patch needs improvement: set
Summary: Admin doesn't catch ProtectedError → admin's delete_selected action doesn't catch ProtectedError

You can create a crash by posting data and bypassing the confirmation page. Seems low priority given the admin is for "trusted users" but wouldn't hurt to fix. See #26235 for the same issue for the regular delete view.

by Tim Graham, 8 years ago

Attachment: 21734-test.diff​ added

comment:9 by Tim Graham, 8 years ago

Resolution: needsinfo
Status: closed → new

comment:10 by Akshesh Doshi, 8 years ago

Owner: changed from nobody to Akshesh Doshi
Status: new → assigned

comment:11 by Akshesh Doshi, 8 years ago

Patch needs improvement: unset

PR: ​https://github.com/django/django/pull/6192

comment:12 by Tim Graham, 8 years ago

Patch needs improvement: set

Left comments for improvement on the pull request.

comment:13 by Tim Graham, 8 years ago

Patch needs improvement: unset
Triage Stage: Accepted → Ready for checkin

​PR

comment:14 by Tim Graham <timograham@…>, 8 years ago

Resolution: → fixed
Status: assigned → closed

In a7c813ba:

Fixed #21734 -- Handled ProtectedError in a POST to admin's delete_selected action.

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