From 4a934246542cd5ad5364017381353a3329191c73 Mon Sep 17 00:00:00 2001 From: Neal Todd Date: Wed, 17 Jul 2019 19:56:52 +0100 Subject: [PATCH] Prevent exception when attempting to delete a model with a protected 1-to-1 relation Modeladmin handles notification to the user if a model instance has protected ForeignKey relationships. However, if the protected relation is a OneToOneField it raises an exception: File ".../wagtail/wagtail/contrib/modeladmin/views.py", line 742, in post for obj in qs.all(): AttributeError: 'MyRelatedModel' object has no attribute 'all' because qs in this case is the related instance rather than a queryset of related instances (as is the case for a ForeignKey). This commit handles the OneToOneField case as well. --- CHANGELOG.txt | 1 + docs/releases/2.7.rst | 1 + .../tests/test_simple_modeladmin.py | 17 ++++++++++++++ wagtail/contrib/modeladmin/views.py | 19 +++++++++++----- .../fixtures/modeladmintest_test.json | 16 ++++++++++++++ .../migrations/0008_solobook.py | 22 +++++++++++++++++++ wagtail/tests/modeladmintest/models.py | 8 +++++++ 7 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 wagtail/tests/modeladmintest/migrations/0008_solobook.py diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 5db5087f2e..7b1c41eb00 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -10,6 +10,7 @@ Changelog * Fix: Added https support for Scribd oEmbed provider (Rodrigo) * Fix: Changed StreamField group labels color so labels are visible (Catherine Farman) * Fix: Prevented images with a very wide aspect ratio from being displayed distorted in the rich text editor (Iman Syed) + * Fix: Prevent exception when deleting a model with a protected One-to-one relationship (Neal Todd) 2.6.1 (xx.xx.xxxx) - IN DEVELOPMENT diff --git a/docs/releases/2.7.rst b/docs/releases/2.7.rst index 20d215883e..d9aa001ab6 100644 --- a/docs/releases/2.7.rst +++ b/docs/releases/2.7.rst @@ -28,6 +28,7 @@ Bug fixes * Added https support for Scribd oEmbed provider (Rodrigo) * Changed StreamField group label color so labels are visible (Catherine Farman) * Prevented images with a very wide aspect ratio from being displayed distorted in the rich text editor (Iman Syed) + * Prevent exception when deleting a model with a protected One-to-one relationship (Neal Todd) Upgrade considerations diff --git a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py index e079cdf454..2df6dafee5 100644 --- a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py +++ b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py @@ -439,6 +439,23 @@ class TestDeleteViewWithProtectedRelation(TestCase, WagtailTestUtils): self.assertFalse(Author.objects.filter(id=4).exists()) + def test_post_with_1to1_dependent_object(self): + response = self.post(5) + + self.assertEqual(response.status_code, 200) + self.assertContains( + response, + "'Harper Lee' is currently referenced by other objects" + ) + self.assertContains( + response, + "
  • Solo Book: To Kill a Mockingbird
    • " + ) + + # Author not deleted + self.assertTrue(Author.objects.filter(id=5).exists()) + + class TestDeleteViewModelReprPrimary(TestCase, WagtailTestUtils): fixtures = ['modeladmintest_test.json'] diff --git a/wagtail/contrib/modeladmin/views.py b/wagtail/contrib/modeladmin/views.py index 98dd44b162..e4194d436e 100644 --- a/wagtail/contrib/modeladmin/views.py +++ b/wagtail/contrib/modeladmin/views.py @@ -6,11 +6,12 @@ from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.admin.utils import ( get_fields_from_path, label_for_field, lookup_needs_distinct, prepare_lookup_value, quote, unquote) from django.contrib.auth.decorators import login_required -from django.core.exceptions import ImproperlyConfigured, PermissionDenied, SuspiciousOperation +from django.core.exceptions import ( + ImproperlyConfigured, ObjectDoesNotExist, PermissionDenied, SuspiciousOperation) from django.core.paginator import InvalidPage, Paginator from django.db import models from django.db.models.fields import FieldDoesNotExist -from django.db.models.fields.related import ManyToManyField +from django.db.models.fields.related import ManyToManyField, OneToOneRel from django.shortcuts import get_object_or_404, redirect from django.template.defaultfilters import filesizeformat from django.utils.decorators import method_decorator @@ -738,9 +739,17 @@ class DeleteView(InstanceSpecificView): obj.field, ManyToManyField)) for rel in fields: if rel.on_delete == models.PROTECT: - qs = getattr(self.instance, rel.get_accessor_name()) - for obj in qs.all(): - linked_objects.append(obj) + if isinstance(rel, OneToOneRel): + try: + obj = getattr(self.instance, rel.get_accessor_name()) + except ObjectDoesNotExist: + pass + else: + linked_objects.append(obj) + else: + qs = getattr(self.instance, rel.get_accessor_name()) + for obj in qs.all(): + linked_objects.append(obj) context = self.get_context_data( protected_error=True, linked_objects=linked_objects diff --git a/wagtail/tests/modeladmintest/fixtures/modeladmintest_test.json b/wagtail/tests/modeladmintest/fixtures/modeladmintest_test.json index b7597d05c4..d167fcf89e 100644 --- a/wagtail/tests/modeladmintest/fixtures/modeladmintest_test.json +++ b/wagtail/tests/modeladmintest/fixtures/modeladmintest_test.json @@ -31,6 +31,14 @@ "date_of_birth": "1898-11-29" } }, +{ + "pk": 5, + "model": "modeladmintest.author", + "fields": { + "name": "Harper Lee", + "date_of_birth": "1926-04-28" + } +}, { "pk": 1, "model": "modeladmintest.book", @@ -63,6 +71,14 @@ "author_id": 3 } }, +{ + "pk": 4, + "model": "modeladmintest.solobook", + "fields": { + "title": "To Kill a Mockingbird", + "author_id": 5 + } +}, { "pk": "boom", "model": "modeladmintest.token", diff --git a/wagtail/tests/modeladmintest/migrations/0008_solobook.py b/wagtail/tests/modeladmintest/migrations/0008_solobook.py new file mode 100644 index 0000000000..2e9d0f0687 --- /dev/null +++ b/wagtail/tests/modeladmintest/migrations/0008_solobook.py @@ -0,0 +1,22 @@ +# Generated by Django 2.1.10 on 2019-07-17 17:46 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('modeladmintest', '0007_friend'), + ] + + operations = [ + migrations.CreateModel( + name='SoloBook', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('title', models.CharField(max_length=255)), + ('author', models.OneToOneField(on_delete=django.db.models.deletion.PROTECT, to='modeladmintest.Author')), + ], + ), + ] diff --git a/wagtail/tests/modeladmintest/models.py b/wagtail/tests/modeladmintest/models.py index 250ea4b868..1c1b43fe50 100644 --- a/wagtail/tests/modeladmintest/models.py +++ b/wagtail/tests/modeladmintest/models.py @@ -40,6 +40,14 @@ class Book(models.Model, index.Indexed): return self.title +class SoloBook(models.Model): + author = models.OneToOneField(Author, on_delete=models.PROTECT) + title = models.CharField(max_length=255) + + def __str__(self): + return self.title + + class Token(models.Model): key = models.CharField(max_length=40, primary_key=True)