Fixed #14672 - Added admin handling for on_delete=PROTECT. Thanks to jtiai for the report.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@15249 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/admin/actions.py

@@ -32,7 +32,7 @@ def delete_selected(modeladmin, request, queryset):
 
     # Populate deletable_objects, a data structure of all related objects that
     # will also be deleted.
-    deletable_objects, perms_needed = get_deleted_objects(
+    deletable_objects, perms_needed, protected = get_deleted_objects(
         queryset, opts, request.user, modeladmin.admin_site, using)
 
     # The user has already confirmed the deletion.
@@ -58,6 +58,7 @@ def delete_selected(modeladmin, request, queryset):
         "deletable_objects": [deletable_objects],
         'queryset': queryset,
         "perms_lacking": perms_needed,
+        "protected": protected,
         "opts": opts,
         "root_path": modeladmin.admin_site.root_path,
         "app_label": app_label,

django/contrib/admin/options.py

@@ -1177,7 +1177,7 @@ class ModelAdmin(BaseModelAdmin):
 
         # Populate deleted_objects, a data structure of all related objects that
         # will also be deleted.
-        (deleted_objects, perms_needed) = get_deleted_objects(
+        (deleted_objects, perms_needed, protected) = get_deleted_objects(
             [obj], opts, request.user, self.admin_site, using)
 
         if request.POST: # The user has already confirmed the deletion.
@@ -1199,6 +1199,7 @@ class ModelAdmin(BaseModelAdmin):
             "object": obj,
             "deleted_objects": deleted_objects,
             "perms_lacking": perms_needed,
+            "protected": protected,
             "opts": opts,
             "root_path": self.admin_site.root_path,
             "app_label": app_label,

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

@@ -12,6 +12,7 @@
 {% endblock %}
 
 {% block content %}
+{% if perms_lacking or protected %}
     {% if perms_lacking %}
         <p>{% blocktrans with object as escaped_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>
@@ -19,6 +20,15 @@
             <li>{{ obj }}</li>
         {% endfor %}
         </ul>
+    {% endif %}
+    {% if protected %}
+        <p>{% blocktrans with object as escaped_object %}Deleting the {{ object_name }} '{{ escaped_object }}' is not possible, because it would require deleting the following protected related objects:{% endblocktrans %}</p>
+        <ul>
+        {% for obj in protected %}
+            <li>{{ obj }}</li>
+        {% endfor %}
+        </ul>
+    {% endif %}
 {% else %}
     <p>{% blocktrans with object as escaped_object %}Are you sure you want to delete the {{ object_name }} "{{ escaped_object }}"? All of the following related items will be deleted:{% endblocktrans %}</p>
     <ul>{{ deleted_objects|unordered_list }}</ul>

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

@@ -11,6 +11,7 @@
 {% endblock %}
 
 {% block content %}
+{% if perms_lacking or protected %}
     {% if perms_lacking %}
         <p>{% blocktrans %}Deleting the {{ object_name }} would result in deleting related objects, but your account doesn't have permission to delete the following types of objects:{% endblocktrans %}</p>
         <ul>
@@ -18,6 +19,15 @@
             <li>{{ obj }}</li>
         {% endfor %}
         </ul>
+    {% endif %}
+    {% if protected %}
+        <p>{% blocktrans %}Deleting the {{ object_name }} is not possible, because it would require deleting the following protected related objects:{% endblocktrans %}</p>
+        <ul>
+        {% for obj in protected %}
+            <li>{{ obj }}</li>
+        {% endfor %}
+        </ul>
+    {% endif %}
 {% 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 deletable_object in deletable_objects %}

django/contrib/admin/util.py

@@ -104,13 +104,16 @@ def get_deleted_objects(objs, opts, user, admin_site, using):
 
     to_delete = collector.nested(format_callback)
 
-    return to_delete, perms_needed
+    protected = [format_callback(obj) for obj in collector.protected]
+
+    return to_delete, perms_needed, protected
 
 
 class NestedObjects(Collector):
     def __init__(self, *args, **kwargs):
         super(NestedObjects, self).__init__(*args, **kwargs)
         self.edges = {} # {from_instance: [to_instances]}
+        self.protected = set()
 
     def add_edge(self, source, target):
         self.edges.setdefault(source, []).append(target)
@@ -121,7 +124,10 @@ class NestedObjects(Collector):
                 self.add_edge(getattr(obj, source_attr), obj)
             else:
                 self.add_edge(None, obj)
+        try:
             return super(NestedObjects, self).collect(objs, source_attr=source_attr, **kwargs)
+        except models.ProtectedError, e:
+            self.protected.update(e.protected_objects)
 
     def related_objects(self, related, objs):
         qs = super(NestedObjects, self).related_objects(related, objs)

django/db/models/__init__.py

@@ -11,7 +11,7 @@ from django.db.models.fields import *
 from django.db.models.fields.subclassing import SubfieldBase
 from django.db.models.fields.files import FileField, ImageField
 from django.db.models.fields.related import ForeignKey, OneToOneField, ManyToManyField, ManyToOneRel, ManyToManyRel, OneToOneRel
-from django.db.models.deletion import CASCADE, PROTECT, SET, SET_NULL, SET_DEFAULT, DO_NOTHING
+from django.db.models.deletion import CASCADE, PROTECT, SET, SET_NULL, SET_DEFAULT, DO_NOTHING, ProtectedError
 from django.db.models import signals
 
 # Admin stages.

django/db/models/deletion.py

@@ -7,17 +7,27 @@ from django.utils.datastructures import SortedDict
 from django.utils.functional import wraps
 
 
+class ProtectedError(IntegrityError):
+    def __init__(self, msg, protected_objects):
+        self.protected_objects = protected_objects
+        super(ProtectedError, self).__init__(msg, protected_objects)
+
+
 def CASCADE(collector, field, sub_objs, using):
     collector.collect(sub_objs, source=field.rel.to,
                       source_attr=field.name, nullable=field.null)
     if field.null and not connections[using].features.can_defer_constraint_checks:
         collector.add_field_update(field, None, sub_objs)
 
+
 def PROTECT(collector, field, sub_objs, using):
-    raise IntegrityError("Cannot delete some instances of model '%s' because "
+    raise ProtectedError("Cannot delete some instances of model '%s' because "
         "they are referenced through a protected foreign key: '%s.%s'" % (
             field.rel.to.__name__, sub_objs[0].__class__.__name__, field.name
-    ))
+        ),
+        sub_objs
+    )
+
 
 def SET(value):
     if callable(value):
@@ -28,14 +38,18 @@ def SET(value):
             collector.add_field_update(field, value, sub_objs)
     return set_on_delete
 
+
 SET_NULL = SET(None)
 
+
 def SET_DEFAULT(collector, field, sub_objs, using):
     collector.add_field_update(field, field.get_default(), sub_objs)
 
+
 def DO_NOTHING(collector, field, sub_objs, using):
     pass
 
+
 def force_managed(func):
     @wraps(func)
     def decorated(self, *args, **kwargs):
@@ -55,6 +69,7 @@ def force_managed(func):
                 transaction.leave_transaction_management(using=self.using)
     return decorated
 
+
 class Collector(object):
     def __init__(self, using):
         self.using = using

docs/ref/models/fields.txt

@@ -971,7 +971,8 @@ define the details of how the relation works.
     * :attr:`~django.db.models.CASCADE`: Cascade deletes; the default.
 
     * :attr:`~django.db.models.PROTECT`: Prevent deletion of the referenced
-      object by raising :exc:`django.db.IntegrityError`.
+      object by raising :exc:`django.db.models.ProtectedError`, a subclass of
+      :exc:`django.db.IntegrityError`.
 
     * :attr:`~django.db.models.SET_NULL`: Set the :class:`ForeignKey` null;
       this is only possible if :attr:`null` is ``True``.

tests/regressiontests/admin_views/models.py

@@ -626,6 +626,16 @@ class WorkHourAdmin(admin.ModelAdmin):
     list_display = ('datum', 'employee')
     list_filter = ('employee',)
 
+class Question(models.Model):
+    question = models.CharField(max_length=20)
+
+class Answer(models.Model):
+    question = models.ForeignKey(Question, on_delete=models.PROTECT)
+    answer = models.CharField(max_length=20)
+
+    def __unicode__(self):
+        return self.answer
+
 admin.site.register(Article, ArticleAdmin)
 admin.site.register(CustomArticle, CustomArticleAdmin)
 admin.site.register(Section, save_as=True, inlines=[ArticleInline])
@@ -674,3 +684,5 @@ admin.site.register(ChapterXtra1, ChapterXtra1Admin)
 admin.site.register(Pizza, PizzaAdmin)
 admin.site.register(Topping)
 admin.site.register(Album, AlbumAdmin)
+admin.site.register(Question)
+admin.site.register(Answer)

tests/regressiontests/admin_views/tests.py

@@ -29,11 +29,12 @@ from django.utils.translation import activate, deactivate
 from django.utils import unittest
 
 # local test models
-from models import Article, BarAccount, CustomArticle, EmptyModel, \
+from models import (Article, BarAccount, CustomArticle, EmptyModel,
-    FooAccount, Gallery, ModelWithStringPrimaryKey, \
+    FooAccount, Gallery, ModelWithStringPrimaryKey,
-    Person, Persona, Picture, Podcast, Section, Subscriber, Vodcast, \
+    Person, Persona, Picture, Podcast, Section, Subscriber, Vodcast,
-    Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit, \
+    Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit,
-    Category, Post, Plot, FunkyTag, Chapter, Book, Promo, WorkHour, Employee
+    Category, Post, Plot, FunkyTag, Chapter, Book, Promo, WorkHour, Employee,
+    Question, Answer)
 
 
 class AdminViewBasicTest(TestCase):
@@ -884,6 +885,15 @@ class AdminViewDeletedObjectsTest(TestCase):
         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_protected(self):
+        q = Question.objects.create(question="Why?")
+        a1 = Answer.objects.create(question=q, answer="Because.")
+        a2 = Answer.objects.create(question=q, answer="Yes.")
+
+        response = self.client.get("/test_admin/admin/admin_views/question/%s/delete/" % quote(q.pk))
+        self.assertContains(response, "would require deleting the following protected related objects")
+        self.assertContains(response, '<li>Answer: <a href="/test_admin/admin/admin_views/answer/%s/">Because.</a></li>' % a1.pk)
+        self.assertContains(response, '<li>Answer: <a href="/test_admin/admin/admin_views/answer/%s/">Yes.</a></li>' % a2.pk)
 
     def test_not_registered(self):
         should_contain = """<li>Secret hideout: underground bunker"""
@@ -1627,6 +1637,28 @@ class AdminActionsTest(TestCase):
         response = self.client.post('/test_admin/admin/admin_views/subscriber/', delete_confirmation_data)
         self.assertEqual(Subscriber.objects.count(), 0)
 
+    def test_model_admin_default_delete_action_protected(self):
+        """
+        Tests the default delete action defined as a ModelAdmin method in the
+        case where some related objects are protected from deletion.
+        """
+        q1 = Question.objects.create(question="Why?")
+        a1 = Answer.objects.create(question=q1, answer="Because.")
+        a2 = Answer.objects.create(question=q1, answer="Yes.")
+        q2 = Question.objects.create(question="Wherefore?")
+
+        action_data = {
+            ACTION_CHECKBOX_NAME: [q1.pk, q2.pk],
+            'action' : 'delete_selected',
+            'index': 0,
+        }
+
+        response = self.client.post("/test_admin/admin/admin_views/question/", action_data)
+
+        self.assertContains(response, "would require deleting the following protected related objects")
+        self.assertContains(response, '<li>Answer: <a href="/test_admin/admin/admin_views/answer/%s/">Because.</a></li>' % a1.pk)
+        self.assertContains(response, '<li>Answer: <a href="/test_admin/admin/admin_views/answer/%s/">Yes.</a></li>' % a2.pk)
+
     def test_custom_function_mail_action(self):
         "Tests a custom action defined in a function"
         action_data = {