[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.

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.1.X@12600 bcc190cf-cafb-0310-a4f2-bffc1f526a37

6 files changed, 303 insertions, 3 deletions

diff --git a/tests/regressiontests/admin_util/__init__.py b/tests/regressiontests/admin_util/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/regressiontests/admin_util/__init__.py
diff --git a/tests/regressiontests/admin_util/models.py b/tests/regressiontests/admin_util/models.py
new file mode 100644
index 0000000000..ce1858585e
--- /dev/null
+++ b/tests/regressiontests/admin_util/models.py
@@ -0,0 +1,5 @@
+from django.db import models
+
+
+class Count(models.Model):
+    num = models.PositiveSmallIntegerField()
diff --git a/tests/regressiontests/admin_util/tests.py b/tests/regressiontests/admin_util/tests.py
new file mode 100644
index 0000000000..782a6e70bb
--- /dev/null
+++ b/tests/regressiontests/admin_util/tests.py
@@ -0,0 +1,58 @@
+from django.test import TestCase
+
+from django.contrib.admin.util import NestedObjects
+
+from models import 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]]])
+
diff --git a/tests/regressiontests/admin_views/fixtures/deleted-objects.xml b/tests/regressiontests/admin_views/fixtures/deleted-objects.xml
new file mode 100644
index 0000000000..92e43dba1c
--- /dev/null
+++ b/tests/regressiontests/admin_views/fixtures/deleted-objects.xml
@@ -0,0 +1,53 @@
+<?xml version="1.0" encoding="utf-8"?>
+<django-objects version="1.0">
+    <object pk="1" model="admin_views.villain">
+        <field type="CharField" name="name">Adam</field>
+    </object>
+    <object pk="2" model="admin_views.villain">
+        <field type="CharField" name="name">Sue</field>
+    </object>
+    <object pk="3" model="admin_views.villain">
+        <field type="CharField" name="name">Bob</field>
+    </object>
+    <object pk="3" model="admin_views.supervillain">
+    </object>
+    <object pk="1" model="admin_views.plot">
+        <field type="CharField" name="name">World Domination</field>
+        <field type="ForeignKey" name="team_leader">1</field>
+        <field type="ForeignKey" name="contact">2</field>
+    </object>
+    <object pk="2" model="admin_views.plot">
+        <field type="CharField" name="name">World Peace</field>
+        <field type="ForeignKey" name="team_leader">2</field>
+        <field type="ForeignKey" name="contact">2</field>
+    </object>
+    <object pk="1" model="admin_views.plotdetails">
+        <field type="CharField" name="details">almost finished</field>
+        <field type="ForeignKey" name="plot">1</field>
+    </object>
+    <object pk="1" model="admin_views.secrethideout">
+        <field type="CharField" name="location">underground bunker</field>
+        <field type="ForeignKey" name="villain">1</field>
+    </object>
+    <object pk="2" model="admin_views.secrethideout">
+        <field type="CharField" name="location">floating castle</field>
+        <field type="ForeignKey" name="villain">3</field>
+    </object>
+    <object pk="1" model="admin_views.supersecrethideout">
+        <field type="CharField" name="location">super floating castle!</field>
+        <field type="ForeignKey" name="supervillain">3</field>
+    </object>
+    <object pk="1" model="admin_views.cyclicone">
+        <field type="CharField" name="name">I am recursive</field>
+        <field type="ForeignKey" name="two">1</field>
+    </object>
+    <object pk="1" model="admin_views.cyclictwo">
+        <field type="CharField" name="name">I am recursive too</field>
+        <field type="ForeignKey" name="one">1</field>
+    </object>
+    <object pk="3" model="admin_views.plot">
+        <field type="CharField" name="name">Corn Conspiracy</field>
+        <field type="ForeignKey" name="team_leader">1</field>
+        <field type="ForeignKey" name="contact">1</field>
+    </object>
+</django-objects>
diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py
index 5a648c6331..6e73b2ec1c 100644
--- a/tests/regressiontests/admin_views/models.py
+++ b/tests/regressiontests/admin_views/models.py
@@ -8,6 +8,10 @@ from django.core.mail import EmailMessage
 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
@@ -434,6 +438,71 @@ class CategoryInline(admin.StackedInline):
 class CollectorAdmin(admin.ModelAdmin):
     inlines = [WidgetInline, DooHickeyInline, GrommetInline, WhatsitInline, FancyDoodadInline, CategoryInline]
 
+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])
@@ -457,6 +526,12 @@ admin.site.register(Recommendation, RecommendationAdmin)
 admin.site.register(Recommender)
 admin.site.register(Collector, CollectorAdmin)
 admin.site.register(Category, CategoryAdmin)
+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:
diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py
index 07e3385cb3..8ab0ed4817 100644
--- a/tests/regressiontests/admin_views/tests.py
+++ b/tests/regressiontests/admin_views/tests.py
@@ -13,13 +13,14 @@ from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
 from django.forms.util import ErrorList
 from django.utils.cache import get_max_age
 from django.utils.html import escape
+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
+    Category, Plot, FunkyTag
 
 try:
     set
@@ -605,6 +606,113 @@ class AdminViewPermissionsTest(TestCase):
         self.failUnlessEqual(logged.object_id, u'1')
         self.client.get('/test_admin/admin/logout/')
 
+
+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']
 
@@ -625,7 +733,7 @@ class AdminViewStringPrimaryKeyTest(TestCase):
         response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/history/' % quote(self.pk))
         self.assertContains(response, escape(self.pk))
         self.failUnlessEqual(response.status_code, 200)
- 
+
     def test_get_change_view(self):
         "Retrieving the object using urlencoded form of primary key should work"
         response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/' % quote(self.pk))
@@ -667,7 +775,8 @@ class AdminViewStringPrimaryKeyTest(TestCase):
     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):