Opened 15 years ago

Closed 15 years ago

Last modified 13 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 15 years ago.
A patch.
django_delete_objects_in_admin_2.diff​ (2.2 KB ) - added by Ben Davis 15 years ago.
Updated to not recurse through object being deleted

Download all attachments as: .zip

Change History (13)

by Ben, 15 years ago

Attachment: django_delete_objects_in_admin.diff​ added

A patch.

comment:1 by Alex Gaynor, 15 years ago

Patch needs improvement: set

comment:2 by Alex Gaynor, 15 years ago

Triage Stage: Unreviewed → Accepted

by Ben Davis, 15 years ago

Attachment: django_delete_objects_in_admin_2.diff​ added

Updated to not recurse through object being deleted

comment:3 by Ben Davis, 15 years ago

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 by Alex Gaynor, 15 years ago

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 by Carl Meyer, 15 years ago

Owner: changed from nobody to Carl Meyer

comment:6 by Carl Meyer, 15 years ago

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

comment:7 by Carl Meyer, 15 years ago

milestone: → 1.2
Patch needs improvement: unset

This is fixed in the latest patch on #6191.

comment:8 by Russell Keith-Magee, 15 years ago

Resolution: → fixed
Status: new → closed

(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 by Russell Keith-Magee, 15 years ago

(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 by Jacob, 13 years ago

milestone: 1.2

Milestone 1.2 deleted

comment:29 by anonymous, 13 years ago

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