Refactor pages revisions_revert view to be a subclass of EditView (#12690)

Currently, the code that handles the POST request for reverting a
revision lives in the EditView class, while the revisions_revert view is
a smaller view that tries to "mimic" the EditView for rendering the view
as part of a GET request. The view injects the revision ID into the
form, which has the action URL hardcoded to the EditView. Including the
revision ID in the form allows the EditView to tell whether it's in a
"reverting" mode or not, and adjust the POST logic accordingly.

However, this results in possible inconsistencies in both views.
Whenever we want to change EditView code or template, we need to make
sure to also update the revisions_revert view. The fact that the
revisions_revert view is a function-based view doesn't help.

Instead of copying the view code and reusing the template with the
addition of injecting the revision ID in the form, turn it into a proper
subclass of the EditView, and make use of Django's URL patterns to
retrieve the revision ID in the EditView.

This approach is similar to how reverting revisions is handled for
snippets.

Ideally, all the code for handling revisions revert should live in the
RevisionsRevertView, and the EditView shouldn't know about it at all.
This is how it's done for snippets: all the revisions revert-related
code is put in RevisionsRevertMixin.

However, this is currently not possible for pages without more
significant refactoring, so this commit does the minimal change needed
to achieve the goal of keeping the revisions_revert view in sync with
the EditView.

CHANGELOG.txt

@@ -100,6 +100,7 @@ Changelog
  * Maintenance: Use built-in `venv` instead of `pipenv` in CircleCI (Sage Abdullah)
  * Maintenance: Add a new Stimulus `FormsetController` (`w-formset`) to support dynamic formset insertion/deletion behavior (LB (Ben) Johnston)
  * Maintenance: Enable breadcrumbs by default on admin views using generic templates (Sage Abdullah)
+ * Maintenance: Refactor pages `revisions_revert` view to be a subclass of `EditView` (Sage Abdullah)
 
 
 6.3.2 (xx.xx.xxxx) - IN DEVELOPMENT

docs/releases/6.4.md

@@ -118,6 +118,7 @@ depth: 1
  * Use built-in `venv` instead of `pipenv` in CircleCI (Sage Abdullah)
  * Add a new Stimulus `FormsetController` (`w-formset`) to support dynamic formset insertion/deletion behavior (LB (Ben) Johnston)
  * Enable breadcrumbs by default on admin views using generic templates (Sage Abdullah)
+ * Refactor pages `revisions_revert` view to be a subclass of `EditView` (Sage Abdullah)
 
 ## Upgrade considerations - changes affecting all projects
 

wagtail/admin/templates/wagtailadmin/pages/edit.html

@@ -14,7 +14,7 @@
 
         <form
             id="page-edit-form"
-            action="{% url 'wagtailadmin_pages:edit' page.id %}"
+            action="{{ action_url }}"
             method="POST"
             novalidate
             {% if form.is_multipart %}enctype="multipart/form-data"{% endif %}
@@ -31,10 +31,6 @@
             <input type="hidden" name="next" value="{{ next }}">
             {{ edit_handler.render_form_content }}
 
-            {% if is_revision %}
-                <input type="hidden" name="revision" value="{{ revision.id|unlocalize }}" />
-            {% endif %}
-
             {% block footer %}
                 {{ block.super }}
                 {# Reuse the footer container, but redefine the actions content down below to use component classes #}

wagtail/admin/tests/pages/test_revisions.py

@@ -120,12 +120,17 @@ class TestRevisions(WagtailTestUtils, TestCase):
         # Form should show the content of the revision, not the current draft
         self.assertContains(response, "Last Christmas I gave you my heart")
 
-        # Form should include a hidden 'revision' field
-        revision_field = (
-            """<input type="hidden" name="revision" value="%d" />"""
-            % self.last_christmas_revision.id
+        # Form should use the revisions revert URL as the action
+        soup = self.get_soup(response.content)
+        form = soup.select_one("form[data-edit-form]")
+        self.assertIsNotNone(form)
+        self.assertEqual(
+            form.get("action"),
+            reverse(
+                "wagtailadmin_pages:revisions_revert",
+                args=(self.christmas_event.id, self.last_christmas_revision.id),
+            ),
         )
-        self.assertContains(response, revision_field)
 
         # Buttons should be relabelled
         self.assertContains(response, "Replace current draft")

wagtail/admin/tests/test_audit_log.py

@@ -374,12 +374,14 @@ class TestAuditLogAdmin(AdminTemplateTestUtils, WagtailTestUtils, TestCase):
 
         self.login(user=self.administrator)
         response = self.client.post(
-            reverse("wagtailadmin_pages:edit", args=(self.hello_page.id,)),
+            reverse(
+                "wagtailadmin_pages:revisions_revert",
+                args=(self.hello_page.id, revision.id),
+            ),
             {
                 "title": "Hello World!",
                 "content": "another hello",
                 "slug": "hello-world",
-                "revision": revision.id,
                 "action-publish": "action-publish",
             },
             follow=True,

wagtail/admin/urls/pages.py

@@ -106,7 +106,7 @@ urlpatterns = [
     ),
     path(
         "<int:page_id>/revisions/<int:revision_id>/revert/",
-        revisions.revisions_revert,
+        revisions.RevisionsRevertView.as_view(),
         name="revisions_revert",
     ),
     path(

wagtail/admin/views/pages/edit.py

@@ -290,9 +290,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
                 reply.log_delete(page_revision=revision, user=self.request.user)
 
     def get_edit_message_button(self):
-        return messages.button(
-            reverse("wagtailadmin_pages:edit", args=(self.page.id,)), _("Edit")
-        )
+        return messages.button(self.get_edit_url(), _("Edit"))
 
     def get_view_draft_message_button(self):
         return messages.button(
@@ -320,7 +318,13 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
         else:
             return self.page
 
-    def dispatch(self, request, page_id):
+    def get_object(self):
+        return self.real_page_record.get_latest_revision_as_object()
+
+    def get_edit_url(self):
+        return reverse("wagtailadmin_pages:edit", args=(self.page.id,))
+
+    def dispatch(self, request, page_id, **kwargs):
         self.real_page_record = get_object_or_404(
             Page.objects.prefetch_workflow_states(), id=page_id
         )
@@ -340,7 +344,14 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
                 "back to a branch where the model class is still present."
             )
 
-        self.page = self.real_page_record.get_latest_revision_as_object()
+        self.revision_id = kwargs.get("revision_id")
+        self.is_reverting = bool(self.revision_id)
+        self.previous_revision = None
+        if self.is_reverting:
+            self.previous_revision = get_object_or_404(
+                self.real_page_record.revisions, id=self.revision_id
+            )
+        self.page = self.get_object()
         self.parent = self.page.get_parent()
         self.scheduled_page = self.real_page_record.get_scheduled_revision_as_object()
 
@@ -393,9 +404,9 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
 
         self.errors_debug = None
 
-        return super().dispatch(request)
+        return super().dispatch(request, page_id, **kwargs)
 
-    def get(self, request):
+    def get(self, request, *args, **kwargs):
         if self.lock:
             lock_message = self.lock.get_message(self.request.user)
             if lock_message:
@@ -454,7 +465,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
             ],
         )
 
-    def post(self, request):
+    def post(self, request, *args, **kwargs):
         # Don't allow POST requests if the page is an alias
         if self.page.alias_of_id:
             # Return 405 "Method Not Allowed" response
@@ -491,14 +502,6 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
         return self.workflow_action in available_action_names
 
     def form_valid(self, form):
-        self.is_reverting = bool(self.request.POST.get("revision"))
-        # If a revision ID was passed in the form, get that revision so its
-        # date can be referenced in notification messages
-        if self.is_reverting:
-            self.previous_revision = get_object_or_404(
-                self.page.revisions, id=self.request.POST.get("revision")
-            )
-
         self.has_content_changes = self.form.has_changed()
 
         if self.request.POST.get("action-publish") and self.page_perms.can_publish():
@@ -533,7 +536,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
         revision = self.page.save_revision(
             user=self.request.user,
             log_action=True,  # Always log the new revision on edit
-            previous_revision=(self.previous_revision if self.is_reverting else None),
+            previous_revision=self.previous_revision,
         )
 
         self.add_save_confirmation_message()
@@ -558,7 +561,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
         revision = self.page.save_revision(
             user=self.request.user,
             log_action=True,  # Always log the new revision on edit
-            previous_revision=(self.previous_revision if self.is_reverting else None),
+            previous_revision=self.previous_revision,
         )
 
         # store submitted go_live_at for messaging below
@@ -572,7 +575,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
             revision,
             user=self.request.user,
             changed=self.has_content_changes,
-            previous_revision=(self.previous_revision if self.is_reverting else None),
+            previous_revision=self.previous_revision,
         )
         action.execute(skip_permission_checks=True)
 
@@ -654,7 +657,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
         revision = self.page.save_revision(
             user=self.request.user,
             log_action=True,  # Always log the new revision on edit
-            previous_revision=(self.previous_revision if self.is_reverting else None),
+            previous_revision=self.previous_revision,
         )
 
         if self.has_content_changes and "comments" in self.form.formsets:
@@ -701,7 +704,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
         revision = self.page.save_revision(
             user=self.request.user,
             log_action=True,  # Always log the new revision on edit
-            previous_revision=(self.previous_revision if self.is_reverting else None),
+            previous_revision=self.previous_revision,
         )
 
         if self.has_content_changes and "comments" in self.form.formsets:
@@ -783,7 +786,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
         revision = self.page.save_revision(
             user=self.request.user,
             log_action=True,  # Always log the new revision on edit
-            previous_revision=(self.previous_revision if self.is_reverting else None),
+            previous_revision=self.previous_revision,
         )
 
         if self.has_content_changes and "comments" in self.form.formsets:
@@ -810,7 +813,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
             return redirect("wagtailadmin_explore", self.page.get_parent().id)
 
     def redirect_and_remain(self):
-        target_url = reverse("wagtailadmin_pages:edit", args=[self.page.id])
+        target_url = self.get_edit_url()
         if self.next_url:
             # Ensure the 'next' url is passed through again if present
             target_url += "?next=%s" % quote(self.next_url)
@@ -905,19 +908,22 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
             self.page.latest_revision_id,
         )
 
-    def get_context_data(self, **kwargs):
-        context = super().get_context_data(**kwargs)
-        user_perms = self.page.permissions_for_user(self.request.user)
-        bound_panel = self.edit_handler.get_bound_panel(
-            instance=self.page, request=self.request, form=self.form
-        )
-        action_menu = PageActionMenu(
+    def get_action_menu(self):
+        return PageActionMenu(
             self.request,
             view="edit",
             page=self.page,
             lock=self.lock,
             locked_for_user=self.locked_for_user,
         )
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        user_perms = self.page.permissions_for_user(self.request.user)
+        bound_panel = self.edit_handler.get_bound_panel(
+            instance=self.page, request=self.request, form=self.form
+        )
+        action_menu = self.get_action_menu()
         side_panels = self.get_side_panels()
 
         media = MediaContainer([bound_panel, self.form, action_menu, side_panels]).media
@@ -933,6 +939,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View):
                 "side_panels": side_panels,
                 "form": self.form,
                 "next": self.next_url,
+                "action_url": self.get_edit_url(),
                 "history_url": self.get_history_url(),
                 "has_unsaved_changes": self.has_unsaved_changes,
                 "page_locked": self.locked_for_user,

wagtail/admin/views/pages/revisions.py

@@ -1,9 +1,6 @@
-from django.conf import settings
-from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied
 from django.shortcuts import get_object_or_404, redirect
 from django.template.loader import render_to_string
-from django.template.response import TemplateResponse
 from django.urls import reverse
 from django.utils.decorators import method_decorator
 from django.utils.safestring import mark_safe
@@ -12,18 +9,12 @@ from django.utils.translation import gettext as _
 from wagtail.admin import messages
 from wagtail.admin.action_menu import PageActionMenu
 from wagtail.admin.auth import user_has_any_page_permission, user_passes_test
-from wagtail.admin.ui.components import MediaContainer
-from wagtail.admin.ui.side_panels import (
-    ChecksSidePanel,
-    CommentsSidePanel,
-    PageStatusSidePanel,
-    PreviewSidePanel,
-)
 from wagtail.admin.views.generic.models import (
     RevisionsCompareView,
     RevisionsUnscheduleView,
 )
 from wagtail.admin.views.generic.preview import PreviewRevision
+from wagtail.admin.views.pages.edit import EditView
 from wagtail.admin.views.pages.utils import GenericPageBreadcrumbsMixin
 from wagtail.models import Page
 from wagtail.utils.timestamps import render_timestamp
@@ -33,116 +24,55 @@ def revisions_index(request, page_id):
     return redirect("wagtailadmin_pages:history", page_id)
 
 
-def revisions_revert(request, page_id, revision_id):
-    # TODO: refactor this into a class-based view that extends the EditView
-    page = get_object_or_404(Page, id=page_id).specific
-    page_perms = page.permissions_for_user(request.user)
-    if not page_perms.can_edit():
-        raise PermissionDenied
+class RevisionsRevertView(EditView):
+    revisions_revert_url_name = "wagtailadmin_pages:revisions_revert"
 
-    revision = get_object_or_404(page.revisions, id=revision_id)
-    revision_page = revision.as_object()
+    def get_action_menu(self):
+        return PageActionMenu(
+            self.request,
+            view="revisions_revert",
+            is_revision=True,
+            page=self.page,
+            lock=self.lock,
+            locked_for_user=self.locked_for_user,
+        )
 
-    scheduled_page = page.get_scheduled_revision_as_object()
+    def get(self, request, *args, **kwargs):
+        self._add_warning_message()
+        return super().get(request, *args, **kwargs)
 
-    content_type = ContentType.objects.get_for_model(page)
-    page_class = content_type.model_class()
+    def _add_warning_message(self):
+        messages.warning(self.request, self.get_warning_message())
 
-    if getattr(settings, "WAGTAIL_I18N_ENABLED", False):
-        locale = page.locale
-        translations = [
-            {
-                "locale": translation.locale,
-                "url": reverse("wagtailadmin_pages:edit", args=[translation.id]),
-            }
-            for translation in page.get_translations()
-            .only("id", "locale", "depth")
-            .select_related("locale")
-            if translation.permissions_for_user(request.user).can_edit()
-        ]
-    else:
-        locale = None
-        translations = []
+    def get_object(self):
+        return self.previous_revision.as_object()
 
-    edit_handler = page_class.get_edit_handler()
-    form_class = edit_handler.get_form_class()
+    def get_revisions_revert_url(self):
+        return reverse(
+            self.revisions_revert_url_name,
+            args=[self.page.pk, self.revision_id],
+        )
 
-    form = form_class(instance=revision_page, for_user=request.user)
-    edit_handler = edit_handler.get_bound_panel(
-        instance=revision_page, request=request, form=form
-    )
+    def get_warning_message(self):
+        user_avatar = render_to_string(
+            "wagtailadmin/shared/user_avatar.html",
+            {"user": self.previous_revision.user},
+        )
 
-    preview_url = reverse("wagtailadmin_pages:preview_on_edit", args=[page.id])
-    lock = page.get_lock()
-
-    action_menu = PageActionMenu(
-        request,
-        view="revisions_revert",
-        is_revision=True,
-        page=page,
-        lock=lock,
-        locked_for_user=lock is not None and lock.for_user(request.user),
-    )
-    side_panels = [
-        PageStatusSidePanel(
-            revision_page,
-            request,
-            show_schedule_publishing_toggle=form.show_schedule_publishing_toggle,
-            live_object=page,
-            scheduled_object=scheduled_page,
-            locale=locale,
-            translations=translations,
-        ),
-    ]
-    if page.is_previewable():
-        side_panels.append(PreviewSidePanel(page, request, preview_url=preview_url))
-        side_panels.append(ChecksSidePanel(page, request))
-    if form.show_comments_toggle:
-        side_panels.append(CommentsSidePanel(page, request))
-    side_panels = MediaContainer(side_panels)
-
-    media = MediaContainer([edit_handler, form, action_menu, side_panels]).media
-
-    user_avatar = render_to_string(
-        "wagtailadmin/shared/user_avatar.html", {"user": revision.user}
-    )
-
-    messages.warning(
-        request,
-        mark_safe(
+        return mark_safe(
             _(
                 "You are viewing a previous version of this page from <b>%(created_at)s</b> by %(user)s"
             )
             % {
-                "created_at": render_timestamp(revision.created_at),
+                "created_at": render_timestamp(self.previous_revision.created_at),
                 "user": user_avatar,
             }
-        ),
-    )
+        )
 
-    page_title = _("Editing %(page_type)s") % {
-        "page_type": page_class.get_verbose_name()
-    }
-    page_subtitle = page.get_admin_display_title()
-    header_title = f"{page_title}: {page_subtitle}"
-
-    return TemplateResponse(
-        request,
-        "wagtailadmin/pages/edit.html",
-        {
-            "page": page,
-            "revision": revision,
-            "is_revision": True,
-            "content_type": content_type,
-            "edit_handler": edit_handler,
-            "errors_debug": None,
-            "action_menu": action_menu,
-            "side_panels": side_panels,
-            "header_title": header_title,
-            "form": form,  # Used in unit tests
-            "media": media,
-        },
-    )
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context["action_url"] = self.get_revisions_revert_url()
+        return context
 
 
 @method_decorator(user_passes_test(user_has_any_page_permission), name="dispatch")