Permalink
Browse files

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
  • Loading branch information...
carljm committed Jan 20, 2011
1 parent e01cb07 commit 93a4d46184bba069fc6a9aa5517802b2488032ac
@@ -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,
@@ -1177,7 +1177,7 @@ def delete_view(self, request, object_id, extra_context=None):
# 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 @@ def delete_view(self, request, object_id, extra_context=None):
"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,
@@ -12,13 +12,23 @@
{% endblock %}
{% block content %}
-{% 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>
- {% for obj in perms_lacking %}
- <li>{{ obj }}</li>
- {% endfor %}
- </ul>
+{% 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>
+ {% for obj in perms_lacking %}
+ <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>
@@ -11,13 +11,23 @@
{% endblock %}
{% block content %}
-{% 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>
- {% for obj in perms_lacking %}
- <li>{{ obj }}</li>
- {% endfor %}
- </ul>
+{% 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>
+ {% for obj in perms_lacking %}
+ <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 %}
@@ -104,13 +104,16 @@ def format_callback(obj):
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 @@ def collect(self, objs, source_attr=None, **kwargs):
self.add_edge(getattr(obj, source_attr), obj)
else:
self.add_edge(None, obj)
- return super(NestedObjects, self).collect(objs, source_attr=source_attr, **kwargs)
+ 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)
@@ -11,7 +11,7 @@
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.
@@ -7,17 +7,27 @@
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_on_delete(collector, field, sub_objs, using):
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 decorated(self, *args, **kwargs):
transaction.leave_transaction_management(using=self.using)
return decorated
+
class Collector(object):
def __init__(self, using):
self.using = using
@@ -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``.
@@ -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 @@ class WorkHourAdmin(admin.ModelAdmin):
admin.site.register(Pizza, PizzaAdmin)
admin.site.register(Topping)
admin.site.register(Album, AlbumAdmin)
+admin.site.register(Question)
+admin.site.register(Answer)
@@ -29,11 +29,12 @@
from django.utils import unittest
# local test models
-from models import Article, BarAccount, CustomArticle, EmptyModel, \
- FooAccount, Gallery, ModelWithStringPrimaryKey, \
- Person, Persona, Picture, Podcast, Section, Subscriber, Vodcast, \
- Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit, \
- Category, Post, Plot, FunkyTag, Chapter, Book, Promo, WorkHour, Employee
+from models import (Article, BarAccount, CustomArticle, EmptyModel,
+ FooAccount, Gallery, ModelWithStringPrimaryKey,
+ Person, Persona, Picture, Podcast, Section, Subscriber, Vodcast,
+ Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit,
+ Category, Post, Plot, FunkyTag, Chapter, Book, Promo, WorkHour, Employee,
+ Question, Answer)
class AdminViewBasicTest(TestCase):
@@ -884,6 +885,15 @@ def test_perms_needed(self):
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 @@ def test_model_admin_default_delete_action(self):
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 = {

0 comments on commit 93a4d46

Please sign in to comment.