Activate draft page content at the view level rather than in the model

Doing this in model.save() is buggy, because the modelform logic calls
this before child objects have been written. In particular, experiments
that are created in the live state would fail to activate the page content.

Author: gasman

experiments/models.py

@@ -52,20 +52,16 @@ def __init__(self, *args, **kwargs):
         super(Experiment, self).__init__(*args, **kwargs)
         self._initial_status = self.status
 
-    def save(self, *args, **kwargs):
-        result = super(Experiment, self).save(*args, **kwargs)
-        if self._initial_status == 'draft' and self.status == 'live':
-            # For any alternative pages that are unpublished, copy the latest draft revision
-            # to the main table (with is_live=False) so that the revision shown as an alternative
-            # is not an out-of-date one
-            for alternative in self.alternatives.select_related('page'):
-                if not alternative.page.live:
-                    revision = alternative.page.get_latest_revision_as_page()
-                    revision.live = False
-                    revision.has_unpublished_changes = True
-                    revision.save()
-
-        return result
+    def activate_alternative_draft_content(self):
+        # For any alternative pages that are unpublished, copy the latest draft revision
+        # to the main table (with is_live=False) so that the revision shown as an alternative
+        # is not an out-of-date one
+        for alternative in self.alternatives.select_related('page'):
+            if not alternative.page.live:
+                revision = alternative.page.get_latest_revision_as_page()
+                revision.live = False
+                revision.has_unpublished_changes = True
+                revision.save()
 
     def get_variations(self):
         return [self.control_page] + [alt.page for alt in self.alternatives.select_related('page')]

experiments/wagtail_hooks.py

@@ -7,6 +7,7 @@
 from experiments import admin_urls
 from wagtail.contrib.modeladmin.helpers import ButtonHelper
 from wagtail.contrib.modeladmin.options import ModelAdmin, modeladmin_register
+from wagtail.contrib.modeladmin.views import CreateView, EditView
 from wagtail.wagtailcore import hooks
 
 from .models import Experiment
@@ -44,10 +45,29 @@ def get_buttons_for_obj(self, obj, exclude=[], classnames_add=[],
         return btns
 
 
+class CreateExperimentView(CreateView):
+    def form_valid(self, form):
+        response = super(CreateExperimentView, self).form_valid(form)
+        if form.instance.status == 'live':
+            form.instance.activate_alternative_draft_content()
+        return response
+
+
+class EditExperimentView(EditView):
+    def form_valid(self, form):
+        response = super(EditExperimentView, self).form_valid(form)
+        if self.instance._initial_status == 'draft' and self.instance.status == 'live':
+            self.instance.activate_alternative_draft_content()
+
+        return response
+
+
 class ExperimentModelAdmin(ModelAdmin):
     model = Experiment
     add_to_settings_menu = True
     button_helper_class = ExperimentButtonHelper
+    create_view_class = CreateExperimentView
+    edit_view_class = EditExperimentView
 
 modeladmin_register(ExperimentModelAdmin)
 

tests/tests.py

@@ -364,6 +364,34 @@ def test_draft_page_content_is_activated_when_experiment_goes_live(self):
         homepage_alternative_1 = Page.objects.get(pk=self.homepage_alternative_1.pk).specific
         self.assertEqual(homepage_alternative_1.body, 'updated')
 
+    def test_draft_page_content_is_activated_when_creating_experiment_as_live(self):
+        # make a draft edit to homepage_alternative_1
+        self.homepage_alternative_1.body = 'updated'
+        self.homepage_alternative_1.save_revision()
+
+        # create a new experiment with an immediate live status
+        response = self.client.post('/admin/experiments/experiment/create/', {
+            'name': "Another experiment",
+            'slug': 'another-experiment',
+            'control_page': self.homepage.pk,
+            'alternatives-TOTAL_FORMS': 1,
+            'alternatives-INITIAL_FORMS': 0,
+            'alternatives-MIN_NUM_FORMS': 0,
+            'alternatives-MAX_NUM_FORMS': 1000,
+            'alternatives-0-page': self.homepage_alternative_1.pk,
+            'alternatives-0-id': '',
+            'alternatives-0-ORDER': '1',
+            'alternatives-0-DELETE': '',
+            'goal': '',
+            'status': 'live',
+        })
+
+        self.assertRedirects(response, '/admin/experiments/experiment/')
+
+        # page content should be updated to follow the draft revision now
+        homepage_alternative_1 = Page.objects.get(pk=self.homepage_alternative_1.pk).specific
+        self.assertEqual(homepage_alternative_1.body, 'updated')
+
     def test_draft_page_content_is_not_activated_on_published_pages(self):
         # publish homepage_alternative_1
         self.homepage_alternative_1.save_revision().publish()
@@ -387,6 +415,37 @@ def test_draft_page_content_is_not_activated_on_published_pages(self):
         homepage_alternative_1 = Page.objects.get(pk=self.homepage_alternative_1.pk).specific
         self.assertEqual(homepage_alternative_1.body, "Welcome to our site! It's lovely to meet you.")
 
+    def test_draft_page_content_is_not_activated_on_published_pages_when_creating_experiment_as_live(self):
+        # publish homepage_alternative_1
+        self.homepage_alternative_1.save_revision().publish()
+
+        # make a draft edit to homepage_alternative_1
+        self.homepage_alternative_1.body = 'updated'
+        self.homepage_alternative_1.save_revision()
+
+        # create a new experiment with an immediate live status
+        response = self.client.post('/admin/experiments/experiment/create/', {
+            'name': "Another experiment",
+            'slug': 'another-experiment',
+            'control_page': self.homepage.pk,
+            'alternatives-TOTAL_FORMS': 1,
+            'alternatives-INITIAL_FORMS': 0,
+            'alternatives-MIN_NUM_FORMS': 0,
+            'alternatives-MAX_NUM_FORMS': 1000,
+            'alternatives-0-page': self.homepage_alternative_1.pk,
+            'alternatives-0-id': '',
+            'alternatives-0-ORDER': '1',
+            'alternatives-0-DELETE': '',
+            'goal': '',
+            'status': 'live',
+        })
+
+        self.assertRedirects(response, '/admin/experiments/experiment/')
+
+        # page content should still be unchanged
+        homepage_alternative_1 = Page.objects.get(pk=self.homepage_alternative_1.pk).specific
+        self.assertEqual(homepage_alternative_1.body, "Welcome to our site! It's lovely to meet you.")
+
     def test_experiment_delete(self):
         response = self.client.get('/admin/experiments/experiment/delete/%d/' % self.experiment.pk)
         self.assertEqual(response.status_code, 200)