Merge pull request #3867 from yakky/future/reversion_31

Mini reversion refactoring

cms/admin/pageadmin.py

@@ -770,133 +770,38 @@ def render_revision_form(self, request, obj, version, context, revert=False, rec
     def undo(self, request, object_id):
         if not is_installed('reversion'):
             return HttpResponseBadRequest('django reversion not installed')
-        from reversion.models import Revision
-        from cms.utils.page_resolver import is_valid_url
-        import reversion
 
         page = get_object_or_404(Page, pk=object_id)
-        old_titles = list(page.title_set.all())
         if not page.publisher_is_draft:
             page = page.publisher_draft
         if not page.has_change_permission(request):
             return HttpResponseForbidden(force_text(_("You do not have permission to change this page")))
-        versions = reversion.get_for_object(page)
-        if page.revision_id:
-            current_revision = Revision.objects.get(pk=page.revision_id)
-        else:
-            try:
-                current_version = versions[0]
-            except IndexError:
-                return HttpResponseBadRequest("no current revision found")
-            current_revision = current_version.revision
         try:
-            previous_version = versions.filter(revision__pk__lt=current_revision.pk)[0]
-        except IndexError:
-            return HttpResponseBadRequest("no previous revision found")
-        previous_revision = previous_version.revision
-        # clear all plugins
-        placeholders = page.placeholders.all()
-        placeholder_ids = []
-        for placeholder in placeholders:
-            placeholder_ids.append(placeholder.pk)
-        plugins = CMSPlugin.objects.filter(placeholder__in=placeholder_ids).order_by('-depth')
-        for plugin in plugins:
-            plugin._no_reorder = True
-            plugin.delete()
-        # TODO: delete placeholders instead of finding duplicates for 3.1
-        #page.placeholders.all().delete()
-
-        previous_revision.revert(True)
-        rev_page = get_object_or_404(Page, pk=page.pk)
-        rev_page.revision_id = previous_revision.pk
-        rev_page.publisher_public_id = page.publisher_public_id
-        rev_page.save()
-        new_placeholders = rev_page.placeholders.all()
-        slots = {}
-        for new_ph in new_placeholders:
-            if not new_ph.slot in slots:
-                slots[new_ph.slot] = new_ph
-            else:
-                if new_ph in placeholder_ids:
-                    new_ph.delete()
-                elif slots[new_ph.slot] in placeholder_ids:
-                    slots[new_ph.slot].delete()
-        new_titles = rev_page.title_set.all()
-        for title in new_titles:
-            try:
-                is_valid_url(title.path, rev_page)
-            except ValidationError:
-                for old_title in old_titles:
-                    if old_title.language == title.language:
-                        title.slug = old_title.slug
-                        title.save()
-                        messages.error(request, _("Page reverted but slug stays the same because of url collisions."))
+            reverted, clean = page.undo()
+            if not clean:
+                messages.error(request, _("Page reverted but slug stays the same because of url collisions."))
+        except IndexError as e:
+            return HttpResponseBadRequest(e.message)
+
         return HttpResponse("ok")
 
     @require_POST
     def redo(self, request, object_id):
         if not is_installed('reversion'):
             return HttpResponseBadRequest('django reversion not installed')
-        from reversion.models import Revision
-        import reversion
-        from cms.utils.page_resolver import is_valid_url
 
         page = get_object_or_404(Page, pk=object_id)
-        old_titles = list(page.title_set.all())
         if not page.publisher_is_draft:
             page = page.publisher_draft
         if not page.has_change_permission(request):
             return HttpResponseForbidden(force_text(_("You do not have permission to change this page")))
-        versions = reversion.get_for_object(page)
-        if page.revision_id:
-            current_revision = Revision.objects.get(pk=page.revision_id)
-        else:
-            try:
-                current_version = versions[0]
-            except IndexError:
-                return HttpResponseBadRequest("no current revision found")
-            current_revision = current_version.revision
         try:
-            previous_version = versions.filter(revision__pk__gt=current_revision.pk).order_by('pk')[0]
-        except IndexError:
-            return HttpResponseBadRequest("no next revision found")
-        next_revision = previous_version.revision
-        # clear all plugins
-        placeholders = page.placeholders.all()
-        placeholder_ids = []
-        for placeholder in placeholders:
-            placeholder_ids.append(placeholder.pk)
-        plugins = CMSPlugin.objects.filter(placeholder__in=placeholder_ids).order_by('-depth')
-        for plugin in plugins:
-            plugin._no_reorder = True
-            plugin.delete()
-        # TODO: 3.1 remove the placeholder matching from below and just delete them
-        #page.placeholders.all().delete()
-        next_revision.revert(True)
-        rev_page = get_object_or_404(Page, pk=page.pk)
-        rev_page.revision_id = next_revision.pk
-        rev_page.publisher_public_id = page.publisher_public_id
-        rev_page.save()
-        new_placeholders = rev_page.placeholders.all()
-        slots = {}
-        for new_ph in new_placeholders:
-            if not new_ph.slot in slots:
-                slots[new_ph.slot] = new_ph
-            else:
-                if new_ph in placeholder_ids:
-                    new_ph.delete()
-                elif slots[new_ph.slot] in placeholder_ids:
-                    slots[new_ph.slot].delete()
-        new_titles = rev_page.title_set.all()
-        for title in new_titles:
-            try:
-                is_valid_url(title.path, rev_page)
-            except ValidationError:
-                for old_title in old_titles:
-                    if old_title.language == title.language:
-                        title.slug = old_title.slug
-                        title.save()
-                        messages.error(request, _("Page reverted but slug stays the same because of url collisions."))
+            reverted, clean = page.redo()
+            if not clean:
+                messages.error(request, _("Page reverted but slug stays the same because of url collisions."))
+        except IndexError as e:
+            return HttpResponseBadRequest(e.message)
+
         return HttpResponse("ok")
 
     @require_POST

cms/models/pagemodel.py

@@ -6,6 +6,7 @@
 from django.contrib.auth import get_permission_codename
 from django.contrib.sites.models import Site
 from django.core.cache import cache
+from django.core.exceptions import ValidationError
 from django.core.urlresolvers import reverse
 from django.db import models
 from django.shortcuts import get_object_or_404
@@ -1224,6 +1225,110 @@ def get_xframe_options(self):
 
         return xframe_options
 
+    def undo(self):
+        """
+        Revert the current page to the previous revision
+        """
+        import reversion
+
+        # Get current reversion version by matching the reversion_id for the page
+        versions = reversion.get_for_object(self)
+        if self.revision_id:
+            current_revision = reversion.models.Revision.objects.get(pk=self.revision_id)
+        else:
+            try:
+                current_version = versions[0]
+            except IndexError as e:
+                e.message = "no current revision found"
+                raise
+            current_revision = current_version.revision
+        try:
+            previous_version = versions.filter(revision__pk__lt=current_revision.pk)[0]
+        except IndexError as e:
+            e.message = "no previous revision found"
+            raise
+        previous_revision = previous_version.revision
+
+        clean = self._apply_revision(previous_revision)
+        return Page.objects.get(pk=self.pk), clean
+
+    def redo(self):
+        """
+        Revert the current page to the next revision
+        """
+        import reversion
+
+        # Get current reversion version by matching the reversion_id for the page
+        versions = reversion.get_for_object(self)
+        if self.revision_id:
+            current_revision = reversion.models.Revision.objects.get(pk=self.revision_id)
+        else:
+            try:
+                current_version = versions[0]
+            except IndexError as e:
+                e.message = "no current revision found"
+                raise
+            current_revision = current_version.revision
+        try:
+            previous_version = versions.filter(revision__pk__gt=current_revision.pk).order_by('pk')[0]
+        except IndexError as e:
+            e.message = "no next revision found"
+            raise
+        next_revision = previous_version.revision
+
+        clean = self._apply_revision(next_revision)
+        return Page.objects.get(pk=self.pk), clean
+
+    def _apply_revision(self, target_revision):
+        """
+        Revert to a specific revision
+        """
+        from cms.utils.page_resolver import is_valid_url
+        # Get current titles
+        old_titles = list(self.title_set.all())
+
+        # remove existing plugins / placeholders in the current page version
+        placeholder_ids = self.placeholders.all().values_list('pk', flat=True)
+        plugins = CMSPlugin.objects.filter(placeholder__in=placeholder_ids).order_by('-depth')
+        for plugin in plugins:
+            plugin._no_reorder = True
+            plugin.delete()
+        self.placeholders.all().delete()
+
+        # populate the page status data from the target version
+        target_revision.revert(True)
+        rev_page = get_object_or_404(Page, pk=self.pk)
+        rev_page.revision_id = target_revision.pk
+        rev_page.publisher_public_id = self.publisher_public_id
+        rev_page.save()
+
+        # cleanup placeholders
+        new_placeholders = rev_page.placeholders.all()
+        slots = {}
+        for new_ph in new_placeholders:
+            if not new_ph.slot in slots:
+                slots[new_ph.slot] = new_ph
+            else:
+                if new_ph in placeholder_ids:
+                    new_ph.delete()
+                elif slots[new_ph.slot] in placeholder_ids:
+                    slots[new_ph.slot].delete()
+
+        # check reverted titles for slug collisions
+        new_titles = rev_page.title_set.all()
+        clean = True
+        for title in new_titles:
+            try:
+                is_valid_url(title.path, rev_page)
+            except ValidationError:
+                for old_title in old_titles:
+                    if old_title.language == title.language:
+                        title.slug = old_title.slug
+                        title.save()
+                        clean = False
+        return clean
+
+
 def _reversion():
     exclude_fields = ['publisher_is_draft', 'publisher_public', 'publisher_state']
 

cms/tests/reversion_tests.py

@@ -276,10 +276,6 @@ def test_recover_path_collision(self):
             self.assertRedirects(response, URL_CMS_PAGE_CHANGE % page2_pk)
             self.assertEqual(Page.objects.all().count(), 5)
 
-
-
-
-
     def test_publish_limits(self):
         with self.login_user_context(self.user):
             with self.settings(CMS_MAX_PAGE_PUBLISH_REVERSIONS=2, CMS_MAX_PAGE_HISTORY_REVERSIONS=2):