Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#11296 closed Uncategorized (fixed)

Delete confirmation page in Admin displays circular references.

Reported by: Ben Owned by: Carl Meyer
Component: contrib.admin Version: 1.0
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A very minor bug, but also I think easy to fix. The current delete confirmation page in the admin application will display the same object multiple times if there are circular references. e.g. If we have two objects that reference each other with foreign keys then the list of dependent objects to be deleted that is displayed would be-

-object1
    -object2
        -object1
            -object2
etc.

I think it would be better if it only displayed the first object1 and object2.
I have attached a small patch that appears to do this.

Attachments (2)

django_delete_objects_in_admin.diff (2.2 KB) - added by Ben 7 years ago.
A patch.
django_delete_objects_in_admin_2.diff (2.2 KB) - added by Ben Davis 7 years ago.
Updated to not recurse through object being deleted

Download all attachments as: .zip

Change History (13)

Changed 7 years ago by Ben

A patch.

comment:1 Changed 7 years ago by Alex Gaynor

Needs documentation: unset
Needs tests: unset
Patch needs improvement: set

comment:2 Changed 7 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

Changed 7 years ago by Ben Davis

Updated to not recurse through object being deleted

comment:3 Changed 7 years ago by Ben Davis

Patch needs improvement: unset

The original object being deleted should also not be recursed into. This can be easily fixed by adding it to the seen_opts variable at the beginning of the function (as seen in updated patch):

@@ -85,7 +85,7 @@
     nh = _nest_help # Bind to local variable for performance
     if current_depth > 16:
         return # Avoid recursing too deep.
-    opts_seen = []
+    opts_seen += [obj._meta]
     for related in opts.get_all_related_objects():
         has_admin = related.model in admin_site._registry
         if related.opts in opts_seen:

comment:4 Changed 7 years ago by Alex Gaynor

Patch needs improvement: set

This patch still has a mutable default value, which means that the deleted objects will essentially be cached across calls.

comment:5 Changed 7 years ago by Carl Meyer

Owner: changed from nobody to Carl Meyer

comment:6 Changed 7 years ago by Carl Meyer

I'm exploring a possible solution for this as part of the fix for #6191.

comment:7 Changed 7 years ago by Carl Meyer

milestone: 1.2
Patch needs improvement: unset

This is fixed in the latest patch on #6191.

comment:8 Changed 7 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [12598]) Fixed #6191, #11296 -- Modified the admin deletion confirmation page to use the same object collection scheme as the actual deletion. This ensures that all objects that may be deleted are actually deleted, and that cyclic display problems are avoided. Thanks to carljm for the patch.

comment:9 Changed 7 years ago by Russell Keith-Magee

(In [12600]) [1.1.X] Fixed #6191, #11296 -- Modified the admin deletion confirmation page to use the same object collection scheme as the actual deletion. This ensures that all objects that may be deleted are actually deleted, and that cyclic display problems are avoided. Thanks to carljm for the patch.

Backport of r12598 from trunk.

comment:28 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

comment:29 Changed 5 years ago by anonymous

Easy pickings: unset
Severity: Normal
Type: Uncategorized
UI/UX: unset
Note: See TracTickets for help on using tickets.
Back to Top