Ticket #6191: 6191_r12203.3.diff

django/contrib/admin/actions.py

@@ -36 +36 @@
 
     # Populate deletable_objects, a data structure of all related objects that
     # will also be deleted.
-
-    # 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
+    (deletable_objects, perms_needed) = get_deleted_objects(queryset, opts, request.user, modeladmin.admin_site, levels_to_root=2)
 
     # The user has already confirmed the deletion.
     # Do the deletion and return a None to display the change list view again.

django/contrib/admin/options.py

@@ -1068 +1068 @@
 
         # 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:

django/contrib/admin/templates/admin/delete_selected_confirmation.html

@@ -20 +20 @@
     </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>
-    {% endfor %}
+    <ul>{{ deletable_objects|unordered_list }}</ul>
     <form action="" method="post">{% csrf_token %}
     <div>
     {% for obj in queryset %}

django/contrib/admin/util.py

@@ +1 @@
+import urllib
+
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import models
 from django.utils import formats
@@ -7 +9 @@
 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):
@@ -57 +60 @@
                 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_deleted_objects(objs, opts, user, admin_site, levels_to_root=4):
+    """
+    Find all objects related to ``objs`` that should also be
+    deleted. ``objs`` should be an iterable of objects.
 
-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.
-    """
-    try:
-        return reverse('%sadmin_%s_%s_change' % (admin_site.name, app_label, module_name), None, (pk,))
-    except NoReverseMatch:
-        return '%s%s/%s/%s/' % ('../'*levels_to_root, app_label, module_name, pk)
+    Returns a nested list of strings suitable for display in the
+    template with the ``unordered_list`` filter.
 
-def get_deleted_objects(deleted_objects, perms_needed, user, obj, opts, current_depth, admin_site, levels_to_root=4):
-    """
-    Helper function that recursively populates deleted_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.
+    `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)
+
+    perms_needed = set()
+
+    def _format_callback(obj):
+        has_admin = obj.__class__ in admin_site._registry
+        opts = obj._meta
+        try:
+            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:
+            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:
-            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
+            # 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))
 
-        # 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
+    to_delete = collector.nested(_format_callback)
 
-        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 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 set 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, set()).add(key)
+                self.parents.setdefault(key, parent_key)
+
+    def _nested(self, key, format_callback=None):
+        obj = self.seen[key]
+        if format_callback:
+            ret = [format_callback(obj)]
+        else:
+            ret = [obj]
+
+        children = []
+        for child in self.children.get(key, ()):
+            children.extend(self._nested(child, format_callback))
+        if children:
+            ret.append(children)
+
+        return ret
+
+    def nested(self, format_callback=None):
+        """
+        Return the graph as a nested list.
+
+        """
+        roots = []
+        for key in self.seen.keys():
+            if key not in self.parents:
+                roots.extend(self._nested(key, format_callback))
+        return roots
+
 
 def model_format_dict(obj):
     """

django/db/models/base.py

@@ -545 +545 @@
              (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():
@@ -556 +557 @@
                 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
@@ -574 +575 @@
                         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

django/db/models/query_utils.py

@@ -50 +50 @@
         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 +60 @@
         * 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

tests/regressiontests/admin_util/models.py

@@ -16 +16 @@
     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()

tests/regressiontests/admin_util/tests.py

@@ -1 +1 @@
 import unittest
 
 from django.db import models
+from django.test import TestCase
 
 from django.contrib import admin
 from django.contrib.admin.util import display_for_field, label_for_field
 from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE
+from django.contrib.admin.util import NestedObjects
 
-from models import Article
+from models import Article, Count
 
 
+class NestedObjectsTests(TestCase):
+    """
+    Tests for ``NestedObject`` utility collection.
 
-class UtilTests(unittest.TestCase):
+    """
+    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 FieldDisplayTests(unittest.TestCase):
 
     def test_null_display_for_field(self):
         """

new file tests/regressiontests/admin_views/fixtures/deleted-objects.xml

@@ +1 @@
+<?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="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="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>
+</django-objects>

tests/regressiontests/admin_views/models.py

@@ -478 +478 @@
     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 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')
+
+    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 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])
@@ -503 +546 @@
 admin.site.register(Category, CategoryAdmin)
 admin.site.register(Post, PostAdmin)
 admin.site.register(Gadget, GadgetAdmin)
+admin.site.register(Villain)
+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:

tests/regressiontests/admin_views/tests.py

@@ -14 +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
 
 
 class AdminViewBasicTest(TestCase):
@@ -615 +616 @@
         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)
+
 class AdminViewStringPrimaryKeyTest(TestCase):
     fixtures = ['admin-views-users.xml', 'string-primary-key.xml']
 
@@ -677 +753 @@
     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):