diff --git a/src/wagtail_personalisation/wagtail_hooks.py b/src/wagtail_personalisation/wagtail_hooks.py index a65e57c..6d585b0 100644 --- a/src/wagtail_personalisation/wagtail_hooks.py +++ b/src/wagtail_personalisation/wagtail_hooks.py @@ -4,6 +4,7 @@ import logging from django.conf.urls import include, url from django.db import transaction +from django.db.models import F from django.http import Http404 from django.shortcuts import redirect, render from django.template.defaultfilters import pluralize @@ -19,6 +20,7 @@ from wagtail.core.models import Page from wagtail_personalisation import admin_urls, models, utils from wagtail_personalisation.adapters import get_segment_adapter +from wagtail_personalisation.models import PersonalisablePageMetadata logger = logging.getLogger(__name__) @@ -276,14 +278,22 @@ def delete_related_variants(request, page): if request.method == 'POST': parent_id = page.get_parent().id - variants_metadata = variants_metadata.select_related('variant') with transaction.atomic(): - for metadata in variants_metadata.iterator(): - # Call delete() on objects to trigger any signals or hooks. - metadata.variant.delete() - # Delete the page's main variant and the page itself. - page.personalisation_metadata.delete() - page.delete() + # To ensure variants are deleted for all descendants, start with + # the deepest ones, and explicitly delete variants and metadata + # for all of them, including the page itself. Otherwise protected + # foreign key constraints are violated. Only consider canonical + # pages. + for metadata in PersonalisablePageMetadata.objects.filter( + canonical_page__in=page.get_descendants(inclusive=True), + variant=F("canonical_page"), + ).order_by('-canonical_page__depth'): + for variant_metadata in metadata.variants_metadata.select_related('variant'): + # Call delete() on objects to trigger any signals or hooks. + variant_metadata.variant.delete() + metadata.delete() + metadata.canonical_page.delete() + msg = _("Page '{0}' and its variants deleted.") messages.success( request, diff --git a/tests/unit/test_wagtail_hooks.py b/tests/unit/test_wagtail_hooks.py index 7dc2633..91f3576 100644 --- a/tests/unit/test_wagtail_hooks.py +++ b/tests/unit/test_wagtail_hooks.py @@ -4,6 +4,7 @@ from django.http import Http404 from wagtail.core.models import Page +from tests.factories.page import ContentPageFactory from tests.factories.segment import SegmentFactory from wagtail_personalisation import adapters, wagtail_hooks @@ -124,3 +125,21 @@ def test_custom_delete_page_view_deletes_variants(rf, segmented_page, user): # Make sure all the variant pages have been deleted. assert not len(variants.all()) assert not len(variants_metadata.all()) + + +@pytest.mark.django_db +def test_custom_delete_page_view_deletes_variants_of_child_pages(rf, segmented_page, user): + """ + Regression test for deleting pages that have children with variants + """ + post_request = rf.post('/') + user.is_superuser = True + rf.user = user + canonical_page = segmented_page.personalisation_metadata.canonical_page + # Create a child with a variant + child_page = ContentPageFactory(parent=canonical_page, slug='personalised-child') + child_page.personalisation_metadata.copy_for_segment(segmented_page.personalisation_metadata.segment) + # A ProtectedError would be raised if the bug persists + wagtail_hooks.delete_related_variants( + post_request, canonical_page + )