[Django] #21734: Admin doesn't catch ProtectedError
Django
-------------------------------+--------------------
Reporter: sander@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------
When deleting objects through the admin interface ProtectedError
exceptions aren't handled. This patch catches such exceptions and displays
an error message instead.
--
Ticket URL: <https://code.djangoproject.com/ticket/21734>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: sander@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by steffann):
* cc: steffann (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:1>
Django
Reporter: sander@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by timo):
* needs_tests: 0 => 1
* easy: 1 => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:2>
Django
Reporter: sander@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* stage: Unreviewed => Accepted
Comment:
Tentatively accepting pending an answer to the above question.
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:3>
Django
Reporter: sander@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by anubhav9042):
Replying to [comment:2 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:4>
Django
Reporter: sander@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by afuna):
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)
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:5>
Django
Reporter: sander@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by anubhav9042):
As far as I know that intermediate page has nothing to do here.
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:6>
Django
Reporter: sander@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* status: new => closed
* resolution: => needsinfo
Comment:
Closing as needsinfo absent additional details from the reporter.
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:7>
Django
-------------------------------+-------------------------------------
Reporter: sander@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 1 => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:8>
Django
-------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+-------------------------------------
Changes (by timgraham):
* Attachment "21734-test.diff" added.
Django
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* status: closed => new
* resolution: needsinfo =>
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:9>
Django
-------------------------------+------------------------------------
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* owner: nobody => akki
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:10>
Django
-------------------------------+------------------------------------
Reporter: sander@… | Owner: akki
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by akki):
* needs_better_patch: 1 => 0
Comment:
PR: https://github.com/django/django/pull/6192
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:11>
Django
-------------------------------+------------------------------------
Reporter: sander@… | Owner: akki
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* needs_better_patch: 0 => 1
Comment:
Left comments for improvement on the pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:12>
Django
Reporter: sander@… | Owner: akki
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/6348 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: sander@… | Owner: akki
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"a7c813ba045044ec23bb656ef9a23a0e38e36514" a7c813ba]:
{{{
#!CommitTicketReference repository=""
revision="a7c813ba045044ec23bb656ef9a23a0e38e36514"
Fixed #21734 -- Handled ProtectedError in a POST to admin's
delete_selected action.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21734#comment:14>