Fixed #5745 -- Set page as dirty when changing the overwrite url field (

#5810)

CHANGELOG.txt

@@ -1,6 +1,8 @@
 === 3.3.4 (unreleased) ===
 
 * Fixed a bug which prevented certain migrations from running in a multi-db setup.
+* Fixed a bug which prevented the page from being marked as dirty (pending changes)
+  when changing the value of the overwrite url field.
 
 
 === 3.3.3 (2016-10-04) ===

cms/models/managers.py

@@ -155,7 +155,7 @@ def set_or_create(self, request, page, form, language):
                 else:
                     data['has_url_overwrite'] = False
                 for field in advanced_fields:
-                    value = cleaned_data.get(field, None)
+                    value = cleaned_data.get(field) or None
                     data[field] = value
             return self.create(**data)
         for name in base_fields:
@@ -169,7 +169,7 @@ def set_or_create(self, request, page, form, language):
                 obj.path = overwrite_url
             for field in advanced_fields:
                 if field in form.base_fields:
-                    value = cleaned_data.get(field, None)
+                    value = cleaned_data.get(field) or None
                     setattr(obj, field, value)
         obj.save()
         return obj

cms/models/titlemodels.py

@@ -14,6 +14,18 @@
 
 @python_2_unicode_compatible
 class Title(models.Model):
+    # These are the fields whose values are compared when saving
+    # a Title object to know if it has changed.
+    editable_fields = [
+        'title',
+        'slug',
+        'redirect',
+        'page_title',
+        'menu_title',
+        'meta_description',
+        'has_url_overwrite',
+    ]
+
     language = models.CharField(_("language"), max_length=15, db_index=True)
     title = models.CharField(_("title"), max_length=255)
     page_title = models.CharField(_("title"), max_length=255, blank=True, null=True,
@@ -86,27 +98,33 @@ def save_base(self, *args, **kwargs):
 
         if self.publisher_is_draft and not keep_state and self.is_new_dirty():
             self.publisher_state = PUBLISHER_STATE_DIRTY
+
         if keep_state:
             delattr(self, '_publisher_keep_state')
-        ret = super(Title, self).save_base(*args, **kwargs)
-        return ret
+        return super(Title, self).save_base(*args, **kwargs)
 
     def is_new_dirty(self):
-        if self.pk:
-            fields = [
-                'title', 'page_title', 'menu_title', 'meta_description', 'slug', 'has_url_overwrite', 'redirect'
-            ]
-            try:
-                old_title = Title.objects.get(pk=self.pk)
-            except Title.DoesNotExist:
+        if not self.pk:
+            return True
+
+        try:
+            old_title = Title.objects.get(pk=self.pk)
+        except Title.DoesNotExist:
+            return True
+
+        for field in self.editable_fields:
+            old_val = getattr(old_title, field)
+            new_val = getattr(self, field)
+            if not old_val == new_val:
                 return True
-            for field in fields:
-                old_val = getattr(old_title, field)
-                new_val = getattr(self, field)
-                if not old_val == new_val:
-                    return True
-            return False
-        return True
+
+        if old_title.path != self.path and self.has_url_overwrite:
+            # path is handled individually because its a special field.
+            # The path field is both an internal and user facing field,
+            # as such we can't mark the title as dirty on any change,
+            # instead we need to check if the url overwrite flag is set.
+            return True
+        return False
 
 
 class EmptyTitle(object):

cms/tests/test_page.py

@@ -461,6 +461,67 @@ def test_edit_page(self):
             self.assertRedirects(response, URL_CMS_PAGE)
             self.assertEqual(page.get_title(), 'changed title')
 
+    def test_edit_page_sets_publisher_dirty(self):
+        """
+        Test that setting and changing a value for a title/page field
+        will cause the title to be marked as dirty (pending changes).
+        """
+        superuser = self.get_superuser()
+
+        with self.login_user_context(superuser):
+            page_data = self.get_new_page_data()
+            self.client.post(URL_CMS_PAGE_ADD, page_data)
+
+        page = Page.objects.get(title_set__slug=page_data['slug'], publisher_is_draft=True)
+
+        basic_fields = {
+            'title': ('new title', 'new title 2'),
+            'slug': ('new-slug', 'new-slug-2'),
+            'page_title': ('new page title', 'new page title 2'),
+            'menu_title': ('new menu title', 'new menu title 2'),
+            'meta_description': ('new menu description', 'new menu description 2'),
+        }
+        advanced_fields = {
+            'overwrite_url': ('title-override', 'title-override-2'),
+            'redirect': ('/title-redirect/', '/title-redirect-2/'),
+        }
+
+        set_message = 'setting field {} is not updating publisher status'
+        change_message = 'changing field {} is not updating publisher status'
+
+        with self.login_user_context(superuser):
+            endpoint = URL_CMS_PAGE_CHANGE % page.id
+
+            for field, values in basic_fields.items():
+                # Set the initial value
+                page_data[field] = values[0]
+                self.client.post(endpoint, page_data)
+                self.assertTrue(page.reload().is_dirty('en'), set_message.format(field))
+
+                # Reset the publisher dirty status
+                page.reload().publish('en')
+
+                # Change the initial value=
+                page_data[field] = values[1]
+                self.client.post(endpoint, page_data)
+                self.assertTrue(page.reload().is_dirty('en'), change_message.format(field))
+
+            endpoint = URL_CMS_PAGE_ADVANCED_CHANGE % page.id
+
+            for field, values in advanced_fields.items():
+                # Set the initial value
+                page_data[field] = values[0]
+                self.client.post(endpoint, page_data)
+                self.assertTrue(page.reload().is_dirty('en'), set_message.format(field))
+
+                # Reset the publisher dirty status
+                page.reload().publish('en')
+
+                # Change the initial value
+                page_data[field] = values[1]
+                self.client.post(endpoint, page_data)
+                self.assertTrue(page.reload().is_dirty('en'), change_message.format(field))
+
     def test_moderator_edit_page_redirect(self):
         """
         Test that a page can be edited multiple times with moderator