Author: russellm
Date: 2010-02-26 07:17:43 -0600 (Fri, 26 Feb 2010)
New Revision: 12598

Modified:
   django/trunk/django/contrib/admin/actions.py
   django/trunk/django/contrib/admin/options.py
   
django/trunk/django/contrib/admin/templates/admin/delete_selected_confirmation.html
   django/trunk/django/contrib/admin/util.py
   django/trunk/django/db/models/base.py
   django/trunk/django/db/models/query_utils.py
   django/trunk/tests/regressiontests/admin_util/models.py
   django/trunk/tests/regressiontests/admin_util/tests.py
   django/trunk/tests/regressiontests/admin_views/models.py
   django/trunk/tests/regressiontests/admin_views/tests.py
Log:
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.

Modified: django/trunk/django/contrib/admin/actions.py
===================================================================
--- django/trunk/django/contrib/admin/actions.py        2010-02-26 11:26:41 UTC 
(rev 12597)
+++ django/trunk/django/contrib/admin/actions.py        2010-02-26 13:17:43 UTC 
(rev 12598)
@@ -36,16 +36,8 @@
 
     # Populate deletable_objects, a data structure of all related objects that
     # will also be deleted.
+    deletable_objects, perms_needed = get_deleted_objects(queryset, opts, 
request.user, modeladmin.admin_site, levels_to_root=2)
 
-    # deletable_objects must be a list if we want to use '|unordered_list' in 
the template
-    deletable_objects = []
-    perms_needed = set()
-    i = 0
-    for obj in queryset:
-        deletable_objects.append([mark_safe(u'%s: <a href="%s/">%s</a>' % 
(escape(force_unicode(capfirst(opts.verbose_name))), obj.pk, escape(obj))), []])
-        get_deleted_objects(deletable_objects[i], perms_needed, request.user, 
obj, opts, 1, modeladmin.admin_site, levels_to_root=2)
-        i=i+1
-
     # The user has already confirmed the deletion.
     # Do the deletion and return a None to display the change list view again.
     if request.POST.get('post'):
@@ -66,7 +58,7 @@
     context = {
         "title": _("Are you sure?"),
         "object_name": force_unicode(opts.verbose_name),
-        "deletable_objects": deletable_objects,
+        "deletable_objects": [deletable_objects],
         'queryset': queryset,
         "perms_lacking": perms_needed,
         "opts": opts,

Modified: django/trunk/django/contrib/admin/options.py
===================================================================
--- django/trunk/django/contrib/admin/options.py        2010-02-26 11:26:41 UTC 
(rev 12597)
+++ django/trunk/django/contrib/admin/options.py        2010-02-26 13:17:43 UTC 
(rev 12598)
@@ -1080,9 +1080,7 @@
 
         # Populate deleted_objects, a data structure of all related objects 
that
         # will also be deleted.
-        deleted_objects = [mark_safe(u'%s: <a href="../../%s/">%s</a>' % 
(escape(force_unicode(capfirst(opts.verbose_name))), object_id, escape(obj))), 
[]]
-        perms_needed = set()
-        get_deleted_objects(deleted_objects, perms_needed, request.user, obj, 
opts, 1, self.admin_site)
+        (deleted_objects, perms_needed) = get_deleted_objects((obj,), opts, 
request.user, self.admin_site)
 
         if request.POST: # The user has already confirmed the deletion.
             if perms_needed:

Modified: 
django/trunk/django/contrib/admin/templates/admin/delete_selected_confirmation.html
===================================================================
--- 
django/trunk/django/contrib/admin/templates/admin/delete_selected_confirmation.html
 2010-02-26 11:26:41 UTC (rev 12597)
+++ 
django/trunk/django/contrib/admin/templates/admin/delete_selected_confirmation.html
 2010-02-26 13:17:43 UTC (rev 12598)
@@ -20,8 +20,8 @@
     </ul>
 {% else %}
     <p>{% blocktrans %}Are you sure you want to delete the selected {{ 
object_name }} objects? All of the following objects and their related items 
will be deleted:{% endblocktrans %}</p>
-    {% for deleteable_object in deletable_objects %}
-        <ul>{{ deleteable_object|unordered_list }}</ul>
+    {% for deletable_object in deletable_objects %}
+        <ul>{{ deletable_object|unordered_list }}</ul>
     {% endfor %}
     <form action="" method="post">{% csrf_token %}
     <div>

Modified: django/trunk/django/contrib/admin/util.py
===================================================================
--- django/trunk/django/contrib/admin/util.py   2010-02-26 11:26:41 UTC (rev 
12597)
+++ django/trunk/django/contrib/admin/util.py   2010-02-26 13:17:43 UTC (rev 
12598)
@@ -7,8 +7,8 @@
 from django.utils.encoding import force_unicode, smart_unicode, smart_str
 from django.utils.translation import ungettext, ugettext as _
 from django.core.urlresolvers import reverse, NoReverseMatch
+from django.utils.datastructures import SortedDict
 
-
 def quote(s):
     """
     Ensure that primary key values do not confuse the admin URLs by escaping
@@ -57,136 +57,180 @@
                 field_names.append(field)
     return field_names
 
-def _nest_help(obj, depth, val):
-    current = obj
-    for i in range(depth):
-        current = current[-1]
-    current.append(val)
-
-def get_change_view_url(app_label, module_name, pk, admin_site, 
levels_to_root):
-    """
-    Returns the url to the admin change view for the given app_label,
-    module_name and primary key.
-    """
+def _format_callback(obj, user, admin_site, levels_to_root, perms_needed):
+    has_admin = obj.__class__ in admin_site._registry
+    opts = obj._meta
     try:
-        return reverse('%sadmin_%s_%s_change' % (admin_site.name, app_label, 
module_name), None, (pk,))
+        admin_url = reverse('%s:%s_%s_change'
+                            % (admin_site.name,
+                               opts.app_label,
+                               opts.object_name.lower()),
+                            None, (quote(obj._get_pk_val()),))
     except NoReverseMatch:
-        return '%s%s/%s/%s/' % ('../'*levels_to_root, app_label, module_name, 
pk)
+        admin_url = '%s%s/%s/%s/' % ('../'*levels_to_root,
+                                     opts.app_label,
+                                     opts.object_name.lower(),
+                                     quote(obj._get_pk_val()))
+    if has_admin:
+        p = '%s.%s' % (opts.app_label,
+                       opts.get_delete_permission())
+        if not user.has_perm(p):
+            perms_needed.add(opts.verbose_name)
+        # Display a link to the admin page.
+        return mark_safe(u'%s: <a href="%s">%s</a>' %
+                         (escape(capfirst(opts.verbose_name)),
+                          admin_url,
+                          escape(obj)))
+    else:
+        # Don't display link to edit, because it either has no
+        # admin or is edited inline.
+        return u'%s: %s' % (capfirst(opts.verbose_name),
+                            force_unicode(obj))
 
-def get_deleted_objects(deleted_objects, perms_needed, user, obj, opts, 
current_depth, admin_site, levels_to_root=4):
+def get_deleted_objects(objs, opts, user, admin_site, levels_to_root=4):
     """
-    Helper function that recursively populates deleted_objects.
+    Find all objects related to ``objs`` that should also be
+    deleted. ``objs`` should be an iterable of objects.
 
-    `levels_to_root` defines the number of directories (../) to reach the
-    admin root path. In a change_view this is 4, in a change_list view 2.
+    Returns a nested list of strings suitable for display in the
+    template with the ``unordered_list`` filter.
 
+    `levels_to_root` defines the number of directories (../) to reach
+    the admin root path. In a change_view this is 4, in a change_list
+    view 2.
+
     This is for backwards compatibility since the options.delete_selected
     method uses this function also from a change_list view.
     This will not be used if we can reverse the URL.
     """
-    nh = _nest_help # Bind to local variable for performance
-    if current_depth > 16:
-        return # Avoid recursing too deep.
-    opts_seen = []
-    for related in opts.get_all_related_objects():
-        has_admin = related.model in admin_site._registry
-        if related.opts in opts_seen:
-            continue
-        opts_seen.append(related.opts)
-        rel_opts_name = related.get_accessor_name()
-        if isinstance(related.field.rel, models.OneToOneRel):
-            try:
-                sub_obj = getattr(obj, rel_opts_name)
-            except ObjectDoesNotExist:
-                pass
-            else:
-                if has_admin:
-                    p = '%s.%s' % (related.opts.app_label, 
related.opts.get_delete_permission())
-                    if not user.has_perm(p):
-                        perms_needed.add(related.opts.verbose_name)
-                        # We don't care about populating deleted_objects now.
-                        continue
-                if not has_admin:
-                    # Don't display link to edit, because it either has no
-                    # admin or is edited inline.
-                    nh(deleted_objects, current_depth,
-                        [u'%s: %s' % (capfirst(related.opts.verbose_name), 
force_unicode(sub_obj)), []])
-                else:
-                    # Display a link to the admin page.
-                    nh(deleted_objects, current_depth, [mark_safe(u'%s: <a 
href="%s">%s</a>' %
-                        (escape(capfirst(related.opts.verbose_name)),
-                        get_change_view_url(related.opts.app_label,
-                                            related.opts.object_name.lower(),
-                                            sub_obj._get_pk_val(),
-                                            admin_site,
-                                            levels_to_root),
-                        escape(sub_obj))), []])
-                get_deleted_objects(deleted_objects, perms_needed, user, 
sub_obj, related.opts, current_depth+2, admin_site)
+    collector = NestedObjects()
+    for obj in objs:
+        # TODO using a private model API!
+        obj._collect_sub_objects(collector)
+
+    # TODO This next bit is needed only because GenericRelations are
+    # cascade-deleted way down in the internals in
+    # DeleteQuery.delete_batch_related, instead of being found by
+    # _collect_sub_objects. Refs #12593.
+    from django.contrib.contenttypes import generic
+    for f in obj._meta.many_to_many:
+        if isinstance(f, generic.GenericRelation):
+            rel_manager = f.value_from_object(obj)
+            for related in rel_manager.all():
+                # There's a wierdness here in the case that the
+                # generic-related object also has FKs pointing to it
+                # from elsewhere. DeleteQuery does not follow those
+                # FKs or delete any such objects explicitly (which is
+                # probably a bug). Some databases may cascade those
+                # deletes themselves, and some won't. So do we report
+                # those objects as to-be-deleted? No right answer; for
+                # now we opt to report only on objects that Django
+                # will explicitly delete, at risk that some further
+                # objects will be silently deleted by a
+                # referential-integrity-maintaining database.
+                collector.add(related.__class__, related.pk, related,
+                              obj.__class__, obj)
+
+    perms_needed = set()
+
+    to_delete = collector.nested(_format_callback,
+                                 user=user,
+                                 admin_site=admin_site,
+                                 levels_to_root=levels_to_root,
+                                 perms_needed=perms_needed)
+
+    return to_delete, perms_needed
+
+
+class NestedObjects(object):
+    """
+    A directed acyclic graph collection that exposes the add() API
+    expected by Model._collect_sub_objects and can present its data as
+    a nested list of objects.
+
+    """
+    def __init__(self):
+        # Use object keys of the form (model, pk) because actual model
+        # objects may not be unique
+
+        # maps object key to list of child keys
+        self.children = SortedDict()
+
+        # maps object key to parent key
+        self.parents = SortedDict()
+
+        # maps object key to actual object
+        self.seen = SortedDict()
+
+    def add(self, model, pk, obj,
+            parent_model=None, parent_obj=None, nullable=False):
+        """
+        Add item ``obj`` to the graph. Returns True (and does nothing)
+        if the item has been seen already.
+
+        The ``parent_obj`` argument must already exist in the graph; if
+        not, it's ignored (but ``obj`` is still added with no
+        parent). In any case, Model._collect_sub_objects (for whom
+        this API exists) will never pass a parent that hasn't already
+        been added itself.
+
+        These restrictions in combination ensure the graph will remain
+        acyclic (but can have multiple roots).
+
+        ``model``, ``pk``, and ``parent_model`` arguments are ignored
+        in favor of the appropriate lookups on ``obj`` and
+        ``parent_obj``; unlike CollectedObjects, we can't maintain
+        independence from the knowledge that we're operating on model
+        instances, and we don't want to allow for inconsistency.
+
+        ``nullable`` arg is ignored: it doesn't affect how the tree of
+        collected objects should be nested for display.
+        """
+        model, pk = type(obj), obj._get_pk_val()
+
+        key = model, pk
+
+        if key in self.seen:
+            return True
+        self.seen.setdefault(key, obj)
+
+        if parent_obj is not None:
+            parent_model, parent_pk = (type(parent_obj),
+                                       parent_obj._get_pk_val())
+            parent_key = (parent_model, parent_pk)
+            if parent_key in self.seen:
+                self.children.setdefault(parent_key, list()).append(key)
+                self.parents.setdefault(key, parent_key)
+
+    def _nested(self, key, format_callback=None, **kwargs):
+        obj = self.seen[key]
+        if format_callback:
+            ret = [format_callback(obj, **kwargs)]
         else:
-            has_related_objs = False
-            for sub_obj in getattr(obj, rel_opts_name).all():
-                has_related_objs = True
-                if not has_admin:
-                    # Don't display link to edit, because it either has no
-                    # admin or is edited inline.
-                    nh(deleted_objects, current_depth,
-                        [u'%s: %s' % (capfirst(related.opts.verbose_name), 
force_unicode(sub_obj)), []])
-                else:
-                    # Display a link to the admin page.
-                    nh(deleted_objects, current_depth, [mark_safe(u'%s: <a 
href="%s">%s</a>' %
-                        (escape(capfirst(related.opts.verbose_name)),
-                        get_change_view_url(related.opts.app_label,
-                                            related.opts.object_name.lower(),
-                                            sub_obj._get_pk_val(),
-                                            admin_site,
-                                            levels_to_root),
-                        escape(sub_obj))), []])
-                get_deleted_objects(deleted_objects, perms_needed, user, 
sub_obj, related.opts, current_depth+2, admin_site)
-            # If there were related objects, and the user doesn't have
-            # permission to delete them, add the missing perm to perms_needed.
-            if has_admin and has_related_objs:
-                p = '%s.%s' % (related.opts.app_label, 
related.opts.get_delete_permission())
-                if not user.has_perm(p):
-                    perms_needed.add(related.opts.verbose_name)
-    for related in opts.get_all_related_many_to_many_objects():
-        has_admin = related.model in admin_site._registry
-        if related.opts in opts_seen:
-            continue
-        opts_seen.append(related.opts)
-        rel_opts_name = related.get_accessor_name()
-        has_related_objs = False
+            ret = [obj]
 
-        # related.get_accessor_name() could return None for symmetrical 
relationships
-        if rel_opts_name:
-            rel_objs = getattr(obj, rel_opts_name, None)
-            if rel_objs:
-                has_related_objs = True
+        children = []
+        for child in self.children.get(key, ()):
+            children.extend(self._nested(child, format_callback, **kwargs))
+        if children:
+            ret.append(children)
 
-        if has_related_objs:
-            for sub_obj in rel_objs.all():
-                if not has_admin:
-                    # Don't display link to edit, because it either has no
-                    # admin or is edited inline.
-                    nh(deleted_objects, current_depth, [_('One or more 
%(fieldname)s in %(name)s: %(obj)s') % \
-                        {'fieldname': 
force_unicode(related.field.verbose_name), 'name': 
force_unicode(related.opts.verbose_name), 'obj': escape(sub_obj)}, []])
-                else:
-                    # Display a link to the admin page.
-                    nh(deleted_objects, current_depth, [
-                        mark_safe((_('One or more %(fieldname)s in %(name)s:') 
% {'fieldname': escape(force_unicode(related.field.verbose_name)), 'name': 
escape(force_unicode(related.opts.verbose_name))}) + \
-                        (u' <a href="%s">%s</a>' % \
-                            (get_change_view_url(related.opts.app_label,
-                                                 
related.opts.object_name.lower(),
-                                                 sub_obj._get_pk_val(),
-                                                 admin_site,
-                                                 levels_to_root),
-                            escape(sub_obj)))), []])
-        # If there were related objects, and the user doesn't have
-        # permission to change them, add the missing perm to perms_needed.
-        if has_admin and has_related_objs:
-            p = u'%s.%s' % (related.opts.app_label, 
related.opts.get_change_permission())
-            if not user.has_perm(p):
-                perms_needed.add(related.opts.verbose_name)
+        return ret
 
+    def nested(self, format_callback=None, **kwargs):
+        """
+        Return the graph as a nested list.
+
+        Passes **kwargs back to the format_callback as kwargs.
+
+        """
+        roots = []
+        for key in self.seen.keys():
+            if key not in self.parents:
+                roots.extend(self._nested(key, format_callback, **kwargs))
+        return roots
+
+
 def model_format_dict(obj):
     """
     Return a `dict` with keys 'verbose_name' and 'verbose_name_plural',

Modified: django/trunk/django/db/models/base.py
===================================================================
--- django/trunk/django/db/models/base.py       2010-02-26 11:26:41 UTC (rev 
12597)
+++ django/trunk/django/db/models/base.py       2010-02-26 13:17:43 UTC (rev 
12598)
@@ -549,7 +549,8 @@
              (model_class, {pk_val: obj, pk_val: obj, ...}), ...]
         """
         pk_val = self._get_pk_val()
-        if seen_objs.add(self.__class__, pk_val, self, parent, nullable):
+        if seen_objs.add(self.__class__, pk_val, self,
+                         type(parent), parent, nullable):
             return
 
         for related in self._meta.get_all_related_objects():
@@ -560,7 +561,7 @@
                 except ObjectDoesNotExist:
                     pass
                 else:
-                    sub_obj._collect_sub_objects(seen_objs, self.__class__, 
related.field.null)
+                    sub_obj._collect_sub_objects(seen_objs, self, 
related.field.null)
             else:
                 # To make sure we can access all elements, we can't use the
                 # normal manager on the related object. So we work directly
@@ -578,7 +579,7 @@
                         continue
                 delete_qs = rel_descriptor.delete_manager(self).all()
                 for sub_obj in delete_qs:
-                    sub_obj._collect_sub_objects(seen_objs, self.__class__, 
related.field.null)
+                    sub_obj._collect_sub_objects(seen_objs, self, 
related.field.null)
 
         # Handle any ancestors (for the model-inheritance case). We do this by
         # traversing to the most remote parent classes -- those with no parents

Modified: django/trunk/django/db/models/query_utils.py
===================================================================
--- django/trunk/django/db/models/query_utils.py        2010-02-26 11:26:41 UTC 
(rev 12597)
+++ django/trunk/django/db/models/query_utils.py        2010-02-26 13:17:43 UTC 
(rev 12598)
@@ -50,7 +50,7 @@
         else:
             self.blocked = {}
 
-    def add(self, model, pk, obj, parent_model, nullable=False):
+    def add(self, model, pk, obj, parent_model, parent_obj=None, 
nullable=False):
         """
         Adds an item to the container.
 
@@ -60,6 +60,8 @@
         * obj - the object itself.
         * parent_model - the model of the parent object that this object was
           reached through.
+        * parent_obj - the parent object this object was reached
+          through (not used here, but needed in the API for use elsewhere)
         * nullable - should be True if this relation is nullable.
 
         Returns True if the item already existed in the structure and

Modified: django/trunk/tests/regressiontests/admin_util/models.py
===================================================================
--- django/trunk/tests/regressiontests/admin_util/models.py     2010-02-26 
11:26:41 UTC (rev 12597)
+++ django/trunk/tests/regressiontests/admin_util/models.py     2010-02-26 
13:17:43 UTC (rev 12598)
@@ -17,3 +17,6 @@
     def test_from_model_with_override(self):
         return "nothing"
     test_from_model_with_override.short_description = "not what you expect"
+
+class Count(models.Model):
+    num = models.PositiveSmallIntegerField()

Modified: django/trunk/tests/regressiontests/admin_util/tests.py
===================================================================
--- django/trunk/tests/regressiontests/admin_util/tests.py      2010-02-26 
11:26:41 UTC (rev 12597)
+++ django/trunk/tests/regressiontests/admin_util/tests.py      2010-02-26 
13:17:43 UTC (rev 12598)
@@ -2,17 +2,70 @@
 import unittest
 
 from django.db import models
+from django.utils.formats import localize
+from django.test import TestCase
 
 from django.contrib import admin
 from django.contrib.admin.util import display_for_field, label_for_field, 
lookup_field
 from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE
 from django.contrib.sites.models import Site
-from django.utils.formats import localize
+from django.contrib.admin.util import NestedObjects
 
-from models import Article
+from models import Article, Count
 
 
+class NestedObjectsTests(TestCase):
+    """
+    Tests for ``NestedObject`` utility collection.
 
+    """
+    def setUp(self):
+        self.n = NestedObjects()
+        self.objs = [Count.objects.create(num=i) for i in range(5)]
+
+    def _check(self, target):
+        self.assertEquals(self.n.nested(lambda obj: obj.num), target)
+
+    def _add(self, obj, parent=None):
+        # don't bother providing the extra args that NestedObjects ignores
+        self.n.add(None, None, obj, None, parent)
+
+    def test_unrelated_roots(self):
+        self._add(self.objs[0])
+        self._add(self.objs[1])
+        self._add(self.objs[2], self.objs[1])
+
+        self._check([0, 1, [2]])
+
+    def test_siblings(self):
+        self._add(self.objs[0])
+        self._add(self.objs[1], self.objs[0])
+        self._add(self.objs[2], self.objs[0])
+
+        self._check([0, [1, 2]])
+
+    def test_duplicate_instances(self):
+        self._add(self.objs[0])
+        self._add(self.objs[1])
+        dupe = Count.objects.get(num=1)
+        self._add(dupe, self.objs[0])
+
+        self._check([0, 1])
+
+    def test_non_added_parent(self):
+        self._add(self.objs[0], self.objs[1])
+
+        self._check([0])
+
+    def test_cyclic(self):
+        self._add(self.objs[0], self.objs[2])
+        self._add(self.objs[1], self.objs[0])
+        self._add(self.objs[2], self.objs[1])
+        self._add(self.objs[0], self.objs[2])
+
+        self._check([0, [1, [2]]])
+
+
 class UtilTests(unittest.TestCase):
     def test_values_from_lookup_field(self):
         """

Modified: django/trunk/tests/regressiontests/admin_views/models.py
===================================================================
--- django/trunk/tests/regressiontests/admin_views/models.py    2010-02-26 
11:26:41 UTC (rev 12597)
+++ django/trunk/tests/regressiontests/admin_views/models.py    2010-02-26 
13:17:43 UTC (rev 12598)
@@ -10,8 +10,9 @@
 from django.db import models
 from django import forms
 from django.forms.models import BaseModelFormSet
+from django.contrib.contenttypes import generic
+from django.contrib.contenttypes.models import ContentType
 
-
 class Section(models.Model):
     """
     A simple section that links to articles, to test linking to related items
@@ -494,6 +495,71 @@
     def get_changelist(self, request, **kwargs):
         return CustomChangeList
 
+class Villain(models.Model):
+    name = models.CharField(max_length=100)
+
+    def __unicode__(self):
+        return self.name
+
+class SuperVillain(Villain):
+    pass
+
+class FunkyTag(models.Model):
+    "Because we all know there's only one real use case for GFKs."
+    name = models.CharField(max_length=25)
+    content_type = models.ForeignKey(ContentType)
+    object_id = models.PositiveIntegerField()
+    content_object = generic.GenericForeignKey('content_type', 'object_id')
+
+    def __unicode__(self):
+        return self.name
+
+class Plot(models.Model):
+    name = models.CharField(max_length=100)
+    team_leader = models.ForeignKey(Villain, related_name='lead_plots')
+    contact = models.ForeignKey(Villain, related_name='contact_plots')
+    tags = generic.GenericRelation(FunkyTag)
+
+    def __unicode__(self):
+        return self.name
+
+class PlotDetails(models.Model):
+    details = models.CharField(max_length=100)
+    plot = models.OneToOneField(Plot)
+
+    def __unicode__(self):
+        return self.details
+
+class SecretHideout(models.Model):
+    """ Secret! Not registered with the admin! """
+    location = models.CharField(max_length=100)
+    villain = models.ForeignKey(Villain)
+
+    def __unicode__(self):
+        return self.location
+
+class SuperSecretHideout(models.Model):
+    """ Secret! Not registered with the admin! """
+    location = models.CharField(max_length=100)
+    supervillain = models.ForeignKey(SuperVillain)
+
+    def __unicode__(self):
+        return self.location
+
+class CyclicOne(models.Model):
+    name = models.CharField(max_length=25)
+    two = models.ForeignKey('CyclicTwo')
+
+    def __unicode__(self):
+        return self.name
+
+class CyclicTwo(models.Model):
+    name = models.CharField(max_length=25)
+    one = models.ForeignKey(CyclicOne)
+
+    def __unicode__(self):
+        return self.name
+
 admin.site.register(Article, ArticleAdmin)
 admin.site.register(CustomArticle, CustomArticleAdmin)
 admin.site.register(Section, save_as=True, inlines=[ArticleInline])
@@ -519,6 +585,12 @@
 admin.site.register(Category, CategoryAdmin)
 admin.site.register(Post, PostAdmin)
 admin.site.register(Gadget, GadgetAdmin)
+admin.site.register(Villain)
+admin.site.register(SuperVillain)
+admin.site.register(Plot)
+admin.site.register(PlotDetails)
+admin.site.register(CyclicOne)
+admin.site.register(CyclicTwo)
 
 # We intentionally register Promo and ChapterXtra1 but not Chapter nor 
ChapterXtra2.
 # That way we cover all four cases:

Modified: django/trunk/tests/regressiontests/admin_views/tests.py
===================================================================
--- django/trunk/tests/regressiontests/admin_views/tests.py     2010-02-26 
11:26:41 UTC (rev 12597)
+++ django/trunk/tests/regressiontests/admin_views/tests.py     2010-02-26 
13:17:43 UTC (rev 12598)
@@ -16,13 +16,14 @@
 from django.utils.cache import get_max_age
 from django.utils.html import escape
 from django.utils.translation import get_date_formats
+from django.utils.encoding import iri_to_uri
 
 # local test models
 from models import Article, BarAccount, CustomArticle, EmptyModel, \
     ExternalSubscriber, FooAccount, Gallery, ModelWithStringPrimaryKey, \
     Person, Persona, Picture, Podcast, Section, Subscriber, Vodcast, \
     Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit, \
-    Category, Post
+    Category, Post, Plot, FunkyTag
 
 
 class AdminViewBasicTest(TestCase):
@@ -637,6 +638,113 @@
         response = self.client.get('/test_admin/admin/secure-view/')
         self.assertContains(response, 'id="login-form"')
 
+
+class AdminViewDeletedObjectsTest(TestCase):
+    fixtures = ['admin-views-users.xml', 'deleted-objects.xml']
+
+    def setUp(self):
+        self.client.login(username='super', password='secret')
+
+    def tearDown(self):
+        self.client.logout()
+
+    def test_nesting(self):
+        """
+        Objects should be nested to display the relationships that
+        cause them to be scheduled for deletion.
+        """
+        pattern = re.compile(r"""<li>Plot: <a 
href=".+/admin_views/plot/1/">World Domination</a>\s*<ul>\s*<li>Plot details: 
<a href=".+/admin_views/plotdetails/1/">almost finished</a>""")
+        response = 
self.client.get('/test_admin/admin/admin_views/villain/%s/delete/' % quote(1))
+        self.failUnless(pattern.search(response.content))
+
+    def test_cyclic(self):
+        """
+        Cyclic relationships should still cause each object to only be
+        listed once.
+
+        """
+        one = """<li>Cyclic one: <a 
href="/test_admin/admin/admin_views/cyclicone/1/">I am recursive</a>"""
+        two = """<li>Cyclic two: <a 
href="/test_admin/admin/admin_views/cyclictwo/1/">I am recursive too</a>"""
+        response = 
self.client.get('/test_admin/admin/admin_views/cyclicone/%s/delete/' % quote(1))
+
+        self.assertContains(response, one, 1)
+        self.assertContains(response, two, 1)
+
+    def test_perms_needed(self):
+        self.client.logout()
+        delete_user = User.objects.get(username='deleteuser')
+        delete_user.user_permissions.add(get_perm(Plot,
+            Plot._meta.get_delete_permission()))
+
+        self.failUnless(self.client.login(username='deleteuser',
+                                          password='secret'))
+
+        response = 
self.client.get('/test_admin/admin/admin_views/plot/%s/delete/' % quote(1))
+        self.assertContains(response, "your account doesn't have permission to 
delete the following types of objects")
+        self.assertContains(response, "<li>plot details</li>")
+
+
+    def test_not_registered(self):
+        should_contain = """<li>Secret hideout: underground bunker"""
+        response = 
self.client.get('/test_admin/admin/admin_views/villain/%s/delete/' % quote(1))
+        self.assertContains(response, should_contain, 1)
+
+    def test_multiple_fkeys_to_same_model(self):
+        """
+        If a deleted object has two relationships from another model,
+        both of those should be followed in looking for related
+        objects to delete.
+
+        """
+        should_contain = """<li>Plot: <a 
href="/test_admin/admin/admin_views/plot/1/">World Domination</a>"""
+        response = 
self.client.get('/test_admin/admin/admin_views/villain/%s/delete/' % quote(1))
+        self.assertContains(response, should_contain)
+        response = 
self.client.get('/test_admin/admin/admin_views/villain/%s/delete/' % quote(2))
+        self.assertContains(response, should_contain)
+
+    def test_multiple_fkeys_to_same_instance(self):
+        """
+        If a deleted object has two relationships pointing to it from
+        another object, the other object should still only be listed
+        once.
+
+        """
+        should_contain = """<li>Plot: <a 
href="/test_admin/admin/admin_views/plot/2/">World Peace</a></li>"""
+        response = 
self.client.get('/test_admin/admin/admin_views/villain/%s/delete/' % quote(2))
+        self.assertContains(response, should_contain, 1)
+
+    def test_inheritance(self):
+        """
+        In the case of an inherited model, if either the child or
+        parent-model instance is deleted, both instances are listed
+        for deletion, as well as any relationships they have.
+
+        """
+        should_contain = [
+            """<li>Villain: <a 
href="/test_admin/admin/admin_views/villain/3/">Bob</a>""",
+            """<li>Super villain: <a 
href="/test_admin/admin/admin_views/supervillain/3/">Bob</a>""",
+            """<li>Secret hideout: floating castle""",
+            """<li>Super secret hideout: super floating castle!"""
+            ]
+        response = 
self.client.get('/test_admin/admin/admin_views/villain/%s/delete/' % quote(3))
+        for should in should_contain:
+            self.assertContains(response, should, 1)
+        response = 
self.client.get('/test_admin/admin/admin_views/supervillain/%s/delete/' % 
quote(3))
+        for should in should_contain:
+            self.assertContains(response, should, 1)
+
+    def test_generic_relations(self):
+        """
+        If a deleted object has GenericForeignKeys pointing to it,
+        those objects should be listed for deletion.
+
+        """
+        plot = Plot.objects.get(pk=3)
+        tag = FunkyTag.objects.create(content_object=plot, name='hott')
+        should_contain = """<li>Funky tag: hott"""
+        response = 
self.client.get('/test_admin/admin/admin_views/plot/%s/delete/' % quote(3))
+        self.assertContains(response, should_contain)
+
 class AdminViewStringPrimaryKeyTest(TestCase):
     fixtures = ['admin-views-users.xml', 'string-primary-key.xml']
 
@@ -699,7 +807,8 @@
     def test_deleteconfirmation_link(self):
         "The link from the delete confirmation page referring back to the 
changeform of the object should be quoted"
         response = 
self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/delete/'
 % quote(self.pk))
-        should_contain = """<a href="../../%s/">%s</a>""" % (quote(self.pk), 
escape(self.pk))
+        # this URL now comes through reverse(), thus iri_to_uri encoding
+        should_contain = """/%s/">%s</a>""" % (iri_to_uri(quote(self.pk)), 
escape(self.pk))
         self.assertContains(response, should_contain)
 
     def test_url_conflicts_with_add(self):

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Reply via email to