Merge pull request #198 from ixc/198_delete_variants_of_descendants

Variants are not deleted for page descendants

src/wagtail_personalisation/wagtail_hooks.py

@@ -2,6 +2,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
@@ -17,6 +18,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__)
 
@@ -273,14 +275,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():
+            # 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.
-                metadata.variant.delete()
+                    variant_metadata.variant.delete()
-            # Delete the page's main variant and the page itself.
+                metadata.delete()
-            page.personalisation_metadata.delete()
+                metadata.canonical_page.delete()
-            page.delete()
+
         msg = _("Page '{0}' and its variants deleted.")
         messages.success(
             request,

tests/unit/test_wagtail_hooks.py

@@ -2,6 +2,7 @@ import pytest
 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
 
@@ -122,3 +123,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
+    )