Ticket #20133: patch1.patch

django/contrib/admin/actions.py

@@ -31 +31 @@
 
     # Populate deletable_objects, a data structure of all related objects that
     # will also be deleted.
-    deletable_objects, perms_needed, protected = get_deleted_objects(
+    deletable_objects, model_count, perms_needed, protected = get_deleted_objects(
         queryset, opts, request.user, modeladmin.admin_site, using)
 
     # The user has already confirmed the deletion.
@@ -65 +65 @@
         "title": title,
         "objects_name": objects_name,
         "deletable_objects": [deletable_objects],
+        "model_count": dict(model_count),
         'queryset': queryset,
         "perms_lacking": perms_needed,
         "protected": protected,

django/contrib/admin/options.py

@@ -1336 +1336 @@
 
         # Populate deleted_objects, a data structure of all related objects that
         # will also be deleted.
-        (deleted_objects, perms_needed, protected) = get_deleted_objects(
+        (deleted_objects, model_count, perms_needed, protected) = get_deleted_objects(
             [obj], opts, request.user, self.admin_site, using)
 
         if request.POST: # The user has already confirmed the deletion.
@@ -1367 +1367 @@
             "object_name": object_name,
             "object": obj,
             "deleted_objects": deleted_objects,
+            "model_count": dict(model_count),
             "perms_lacking": perms_needed,
             "protected": protected,
             "opts": opts,

django/contrib/admin/templates/admin/delete_confirmation.html

@@ -13 +13 @@
 {% endblock %}
 
 {% block content %}
-{% if perms_lacking or protected %}
-    {% if perms_lacking %}
-        <p>{% blocktrans with escaped_object=object %}Deleting the {{ object_name }} '{{ escaped_object }}' would result in deleting related objects, but your account doesn't have permission to delete the following types of objects:{% endblocktrans %}</p>
-        <ul>
-        {% for obj in perms_lacking %}
-            <li>{{ obj }}</li>
-        {% endfor %}
-        </ul>
-    {% endif %}
-    {% if protected %}
-        <p>{% blocktrans with escaped_object=object %}Deleting the {{ object_name }} '{{ escaped_object }}' would require deleting the following protected related objects:{% endblocktrans %}</p>
-        <ul>
-        {% for obj in protected %}
-            <li>{{ obj }}</li>
-        {% endfor %}
-        </ul>
-    {% endif %}
+{% if perms_lacking %}
+    <p>{% blocktrans with escaped_object=object %}Deleting the {{ object_name }} '{{ escaped_object }}' would result in deleting related objects, but your account doesn't have permission to delete the following types of objects:{% endblocktrans %}</p>
+    <ul>
+    {% for obj in perms_lacking %}
+        <li>{{ obj }}</li>
+    {% endfor %}
+    </ul>
+{% elif protected %}
+    <p>{% blocktrans with escaped_object=object %}Deleting the {{ object_name }} '{{ escaped_object }}' would require deleting the following protected related objects:{% endblocktrans %}</p>
+    <ul>
+    {% for obj in protected %}
+        <li>{{ obj }}</li>
+    {% endfor %}
+    </ul>
 {% else %}
     <p>{% blocktrans with escaped_object=object %}Are you sure you want to delete the {{ object_name }} "{{ escaped_object }}"? All of the following related items will be deleted:{% endblocktrans %}</p>
+    <h2>{% trans "Summary" %}</h2>
+    <ul>
+        {% for model_name, object_count in model_count.items %}
+        <li>{{ model_name }}: {{ object_count }}</li>
+        {% endfor %}
+    </ul>
+    <h2>{% trans "Objects" %}</h2>
     <ul>{{ deleted_objects|unordered_list }}</ul>
     <form action="" method="post">{% csrf_token %}
     <div>

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

@@ -12 +12 @@
 {% endblock %}
 
 {% block content %}
-{% if perms_lacking or protected %}
-    {% if perms_lacking %}
-        <p>{% blocktrans %}Deleting the selected {{ objects_name }} would result in deleting related objects, but your account doesn't have permission to delete the following types of objects:{% endblocktrans %}</p>
-        <ul>
-        {% for obj in perms_lacking %}
-            <li>{{ obj }}</li>
-        {% endfor %}
-        </ul>
-    {% endif %}
-    {% if protected %}
-        <p>{% blocktrans %}Deleting the selected {{ objects_name }} would require deleting the following protected related objects:{% endblocktrans %}</p>
-        <ul>
-        {% for obj in protected %}
-            <li>{{ obj }}</li>
-        {% endfor %}
-        </ul>
-    {% endif %}
+{% if perms_lacking %}
+    <p>{% blocktrans %}Deleting the selected {{ objects_name }} would result in deleting related objects, but your account doesn't have permission to delete the following types of objects:{% endblocktrans %}</p>
+    <ul>
+    {% for obj in perms_lacking %}
+        <li>{{ obj }}</li>
+    {% endfor %}
+    </ul>
+{% endif %}
+{% if protected %}
+    <p>{% blocktrans %}Deleting the selected {{ objects_name }} would require deleting the following protected related objects:{% endblocktrans %}</p>
+    <ul>
+    {% for obj in protected %}
+        <li>{{ obj }}</li>
+    {% endfor %}
+    </ul>
 {% else %}
     <p>{% blocktrans %}Are you sure you want to delete the selected {{ objects_name }}? All of the following objects and their related items will be deleted:{% endblocktrans %}</p>
+    <h2>{% trans "Summary" %}</h2>
+    <ul>
+        {% for model_name, object_count in model_count.items %}
+        <li>{{ model_name }}: {{ object_count }}</li>
+        {% endfor %}
+    </ul>
+    <h2>{% trans "Objects" %}</h2>
     {% for deletable_object in deletable_objects %}
         <ul>{{ deletable_object|unordered_list }}</ul>
     {% endfor %}

django/contrib/admin/util.py

@@ -2 +2 @@
 
 import datetime
 import decimal
+from collections import defaultdict
 
 from django.db import models
 from django.db.models.constants import LOOKUP_SEP
@@ -95 +96 @@
     return field_names
 
 
+def map_nested_list(func, ls):
+    if isinstance(ls, (tuple, list)):
+        return [map_nested_list(func, x) for x in ls]
+    else:
+        return func(ls)
+
+
 def get_deleted_objects(objs, opts, user, admin_site, using):
     """
     Find all objects related to ``objs`` that should also be deleted. ``objs``
     must be a homogenous iterable of objects (e.g. a QuerySet).
 
-    Returns a nested list of strings suitable for display in the
-    template with the ``unordered_list`` filter.
+    Returns a 4-tuple ((deleted_objects, model_count), perms_needed, protected) where
 
+    * deleted_objects is a nested list of strings suitable for display in the
+      template with the ``unordered_list`` filter.
+    * model_count is a map from "model name" to the "number of objects of that
+      type involved in the deletion", e.g. {'User': 3, 'Group': 5}
+    * perms_needed is a list of model names that hinder the deletion because of
+      lacking permissions
+    * protected is a list of objects that hinder the deletion because their
+      deletion strategy is set to PROTECTED in the model definition.
     """
-    collector = NestedObjects(using=using)
+    collector = NestedObjects(user, using=using)
     collector.collect(objs)
-    perms_needed = set()
+
+    if collector.perms_needed:
+        return [], {}, collector.perms_needed, []
 
     def format_callback(obj):
         has_admin = obj.__class__ in admin_site._registry
         opts = obj._meta
 
         if has_admin:
+            # Display a link to the admin page.
             admin_url = reverse('%s:%s_%s_change'
                                 % (admin_site.name,
                                    opts.app_label,
                                    opts.model_name),
                                 None, (quote(obj._get_pk_val()),))
-            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 format_html('{0}: <a href="{1}">{2}</a>',
                                capfirst(opts.verbose_name),
                                admin_url,
@@ -133 +146 @@
             return '%s: %s' % (capfirst(opts.verbose_name),
                                 force_text(obj))
 
-    to_delete = collector.nested(format_callback)
-
-    protected = [format_callback(obj) for obj in collector.protected]
+    if collector.protected:
+        return [], {}, [], map(format_callback, collector.protected)
 
-    return to_delete, perms_needed, protected
+    to_delete = map_nested_list(format_callback, collector.as_nested_list())
+    return to_delete, collector.model_count, [], []
 
 
 class NestedObjects(Collector):
-    def __init__(self, *args, **kwargs):
+    def __init__(self, user, *args, **kwargs):
         super(NestedObjects, self).__init__(*args, **kwargs)
+        self.user = user
         self.edges = {} # {from_instance: [to_instances]}
         self.protected = set()
+        self.perms_needed = set()
+        self.model_count = defaultdict(int)
 
     def add_edge(self, source, target):
         self.edges.setdefault(source, []).append(target)
 
     def collect(self, objs, source_attr=None, **kwargs):
         for obj in objs:
+            opts = obj._meta
+            permission_name = '%s.%s' % (opts.app_label,
+                                         opts.get_delete_permission())
+            if not self.user.has_perm(permission_name):
+                self.perms_needed.add(opts.verbose_name)
+
             if source_attr:
                 self.add_edge(getattr(obj, source_attr), obj)
             else:
                 if obj._meta.proxy:
                     # Take concrete model's instance to avoid mismatch in edges
-                    obj = obj._meta.concrete_model(pk=obj.pk)
+                    obj = opts.concrete_model(pk=obj.pk)
                 self.add_edge(None, obj)
+
+            self.model_count[opts.verbose_name] += 1
+
         try:
             return super(NestedObjects, self).collect(objs, source_attr=source_attr, **kwargs)
         except models.ProtectedError as e:
@@ -167 +192 @@
         qs = super(NestedObjects, self).related_objects(related, objs)
         return qs.select_related(related.field.name)
 
-    def _nested(self, obj, seen, format_callback):
+    def _as_nested_list(self, obj, seen):
         if obj in seen:
             return []
         seen.add(obj)
         children = []
         for child in self.edges.get(obj, ()):
-            children.extend(self._nested(child, seen, format_callback))
-        if format_callback:
-            ret = [format_callback(obj)]
-        else:
-            ret = [obj]
+            children.extend(self._as_nested_list(child, seen))
         if children:
-            ret.append(children)
-        return ret
+            return [obj, children]
+        else:
+            return [obj]
 
-    def nested(self, format_callback=None):
+    def as_nested_list(self):
         """
         Return the graph as a nested list.
-
         """
         seen = set()
         roots = []
         for root in self.edges.get(None, ()):
-            roots.extend(self._nested(root, seen, format_callback))
+            roots.extend(self._as_nested_list(root, seen))
         return roots
 
     def can_fast_delete(self, *args, **kwargs):

tests/admin_util/tests.py

@@ -4 +4 @@
 
 from django.conf import settings
 from django.contrib import admin
+from django.contrib.auth.models import User
 from django.contrib.admin import helpers
 from django.contrib.admin.util import (display_for_field, flatten_fieldsets,
-    label_for_field, lookup_field, NestedObjects)
+    label_for_field, lookup_field, NestedObjects, map_nested_list)
 from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE
 from django.contrib.sites.models import Site
 from django.db import models, DEFAULT_DB_ALIAS
@@ -26 +27 @@
 
     """
     def setUp(self):
-        self.n = NestedObjects(using=DEFAULT_DB_ALIAS)
+        self.n = NestedObjects(User.objects.create(), using=DEFAULT_DB_ALIAS)
         self.objs = [Count.objects.create(num=i) for i in range(5)]
 
     def _check(self, target):
-        self.assertEqual(self.n.nested(lambda obj: obj.num), target)
+        self.assertEqual(
+            map_nested_list(lambda obj: obj.num, self.n.as_nested_list()),
+            target
+        )
 
     def _connect(self, i, j):
         self.objs[i].parent = self.objs[j]
@@ -68 +72 @@
         self._connect(2, 0)
         # 1 query to fetch all children of 0 (1 and 2)
         # 1 query to fetch all children of 1 and 2 (none)
+        # 2 queries to check for permissions of 2 objects
         # Should not require additional queries to populate the nested graph.
-        self.assertNumQueries(2, self._collect, 0)
+        self.assertNumQueries(4, self._collect, 0)
 
     def test_on_delete_do_nothing(self):
         """
         Check that the nested collector doesn't query for DO_NOTHING objects.
         """
-        n = NestedObjects(using=DEFAULT_DB_ALIAS)
         objs = [Event.objects.create()]
         EventGuide.objects.create(event=objs[0])
-        with self.assertNumQueries(2):
-            # One for Location, one for Guest, and no query for EventGuide
-            n.collect(objs)
+        with self.assertNumQueries(4):
+            # One for Location, one for Guest, two for permissions,
+            # and no query for EventGuide
+            self.n.collect(objs)
 
 class UtilTests(unittest.TestCase):
     def test_values_from_lookup_field(self):