Opened 6 years ago

Closed 5 years ago

Last modified 3 years ago

#11296 closed Uncategorized (fixed)

Delete confirmation page in Admin displays circular references.

Reported by: benreynwar Owned by: carljm
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 benreynwar 6 years ago.
A patch.
django_delete_objects_in_admin_2.diff (2.2 KB) - added by bendavis78 6 years ago.
Updated to not recurse through object being deleted

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by benreynwar

A patch.

comment:1 Changed 6 years ago by Alex

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

comment:2 Changed 6 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

Changed 6 years ago by bendavis78

Updated to not recurse through object being deleted

comment:3 Changed 6 years ago by bendavis78

  • 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 6 years ago by Alex

  • 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 6 years ago by carljm

  • Owner changed from nobody to carljm

comment:6 Changed 6 years ago by carljm

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

comment:7 Changed 6 years ago by carljm

  • milestone set to 1.2
  • Patch needs improvement unset

This is fixed in the latest patch on #6191.

comment:8 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to 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 Changed 5 years ago by russellm

(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 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

comment:29 Changed 3 years ago by anonymous

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