From ddbdd516076662416ba6b9bea79d3e33408085ef Mon Sep 17 00:00:00 2001 From: Sage Abdullah Date: Thu, 1 Dec 2022 17:02:16 +0000 Subject: [PATCH] Extract generic Lock/Unlock views and make page's views extend from them (#9740) --- CHANGELOG.txt | 1 + docs/releases/4.2.md | 1 + wagtail/admin/urls/pages.py | 4 +- wagtail/admin/views/generic/__init__.py | 2 +- wagtail/admin/views/generic/base.py | 65 +++++++++++++++++++++ wagtail/admin/views/generic/lock.py | 46 +++++++++++++++ wagtail/admin/views/pages/lock.py | 76 ++++++++----------------- 7 files changed, 140 insertions(+), 55 deletions(-) create mode 100644 wagtail/admin/views/generic/lock.py diff --git a/CHANGELOG.txt b/CHANGELOG.txt index d499b66368..bb7c95ad1b 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -77,6 +77,7 @@ Changelog * Maintenance: Standardise on `classname` for passing HTML class attributes (LB (Ben Johnston)) * Maintenance: Clean up expanding formset and `InlinePanel` JavaScript initialisation code and adopt a class approach (Matt Westcott) * Maintenance: Extracted revision and draft state logic from generic views into mixins (Sage Abdullah) + * Maintenance: Extracted generic lock / unlock views from page lock / unlock views (Sage Abdullah) 4.1.2 (xx.xx.xxxx) - IN DEVELOPMENT diff --git a/docs/releases/4.2.md b/docs/releases/4.2.md index e6496202e2..3525371cbb 100644 --- a/docs/releases/4.2.md +++ b/docs/releases/4.2.md @@ -94,6 +94,7 @@ depth: 1 * Standardise on `classname` for passing HTML class attributes (LB (Ben Johnston)) * Clean up expanding formset and `InlinePanel` JavaScript initialisation code and adopt a class approach (Matt Westcott) * Extracted revision and draft state logic from generic views into mixins (Sage Abdullah) + * Extracted generic lock / unlock views from page lock / unlock views (Sage Abdullah) ## Upgrade considerations diff --git a/wagtail/admin/urls/pages.py b/wagtail/admin/urls/pages.py index 4866df1f19..b796bbefd8 100644 --- a/wagtail/admin/urls/pages.py +++ b/wagtail/admin/urls/pages.py @@ -106,8 +106,8 @@ urlpatterns = [ name="preview_for_moderation", ), path("/privacy/", page_privacy.set_privacy, name="set_privacy"), - path("/lock/", lock.lock, name="lock"), - path("/unlock/", lock.unlock, name="unlock"), + path("/lock/", lock.LockView.as_view(), name="lock"), + path("/unlock/", lock.UnlockView.as_view(), name="unlock"), path("/revisions/", revisions.revisions_index, name="revisions_index"), path( "/revisions//view/", diff --git a/wagtail/admin/views/generic/__init__.py b/wagtail/admin/views/generic/__init__.py index 5a3c8a17ad..0798004d84 100644 --- a/wagtail/admin/views/generic/__init__.py +++ b/wagtail/admin/views/generic/__init__.py @@ -1,4 +1,4 @@ -from .base import WagtailAdminTemplateMixin # noqa +from .base import BaseOperationView, WagtailAdminTemplateMixin # noqa from .mixins import ( # noqa BeforeAfterHookMixin, CreateEditViewOptionalFeaturesMixin, diff --git a/wagtail/admin/views/generic/base.py b/wagtail/admin/views/generic/base.py index 62048ccfde..064f3909bd 100644 --- a/wagtail/admin/views/generic/base.py +++ b/wagtail/admin/views/generic/base.py @@ -1,5 +1,13 @@ +from django.contrib.admin.utils import quote, unquote +from django.core.exceptions import ImproperlyConfigured +from django.shortcuts import get_object_or_404, redirect +from django.urls import reverse +from django.views import View from django.views.generic.base import ContextMixin, TemplateResponseMixin +from wagtail.admin import messages +from wagtail.admin.utils import get_valid_next_url_from_request + class WagtailAdminTemplateMixin(TemplateResponseMixin, ContextMixin): """ @@ -28,3 +36,60 @@ class WagtailAdminTemplateMixin(TemplateResponseMixin, ContextMixin): context["page_subtitle"] = self.get_page_subtitle() context["header_icon"] = self.get_header_icon() return context + + +class BaseOperationView(View): + """Base view to perform an operation on a model instance using a POST request.""" + + model = None + pk_url_kwarg = "pk" + success_message = None + success_message_extra_tags = "" + success_url_name = None + + def setup(self, request, *args, **kwargs): + super().setup(request, *args, **kwargs) + self.next_url = get_valid_next_url_from_request(request) + self.object = self.get_object() + + def get_object(self): + if not self.model: + raise ImproperlyConfigured( + "Subclasses of wagtail.admin.views.generic.base.BaseOperationView must provide a " + "model attribute or a get_object method" + ) + + pk = self.kwargs[self.pk_url_kwarg] + if isinstance(pk, str): + pk = unquote(pk) + return get_object_or_404(self.model, pk=pk) + + def perform_operation(self): + raise NotImplementedError + + def get_success_message(self): + return self.success_message + + def add_success_message(self): + success_message = self.get_success_message() + if success_message: + messages.success( + self.request, + success_message, + extra_tags=self.success_message_extra_tags, + ) + + def get_success_url(self): + if not self.success_url_name: + raise ImproperlyConfigured( + "Subclasses of wagtail.admin.views.generic.base.BaseOperationView must provide a " + "success_url_name attribute or a get_success_url method" + ) + if self.next_url: + return self.next_url + return reverse(self.success_url_name, args=[quote(self.object.pk)]) + + def post(self, request, *args, **kwargs): + self.perform_operation() + self.add_success_message() + return redirect(self.get_success_url()) diff --git a/wagtail/admin/views/generic/lock.py b/wagtail/admin/views/generic/lock.py new file mode 100644 index 0000000000..e775c32105 --- /dev/null +++ b/wagtail/admin/views/generic/lock.py @@ -0,0 +1,46 @@ +from django.utils import timezone +from django.utils.text import capfirst +from django.utils.translation import gettext as _ + +from wagtail.admin.views.generic.base import BaseOperationView +from wagtail.log_actions import log +from wagtail.models import DraftStateMixin + + +class LockView(BaseOperationView): + success_message_extra_tags = "lock" + + def perform_operation(self): + if self.object.locked: + return + self.object.locked = True + self.object.locked_by = self.request.user + self.object.locked_at = timezone.now() + self.object.save(update_fields=["locked", "locked_by", "locked_at"]) + log(instance=self.object, action="wagtail.lock", user=self.request.user) + + +class UnlockView(BaseOperationView): + success_message_extra_tags = "unlock" + + def perform_operation(self): + if not self.object.locked: + return + self.object.locked = False + self.object.locked_by = None + self.object.locked_at = None + self.object.save(update_fields=["locked", "locked_by", "locked_at"]) + log(instance=self.object, action="wagtail.unlock", user=self.request.user) + + def get_success_message(self): + title = str(self.object) + if isinstance(self.object, DraftStateMixin) and self.object.latest_revision: + title = self.object.latest_revision.object_str + + return capfirst( + _("%(model_name)s '%(title)s' is now unlocked.") + % { + "model_name": self.model._meta.verbose_name, + "title": title, + } + ) diff --git a/wagtail/admin/views/pages/lock.py b/wagtail/admin/views/pages/lock.py index 68d002d3cc..2a83838c27 100644 --- a/wagtail/admin/views/pages/lock.py +++ b/wagtail/admin/views/pages/lock.py @@ -1,67 +1,39 @@ from django.core.exceptions import PermissionDenied -from django.shortcuts import get_object_or_404, redirect -from django.utils import timezone -from django.utils.http import url_has_allowed_host_and_scheme +from django.urls import reverse from django.utils.translation import gettext as _ -from django.views.decorators.http import require_POST -from wagtail.admin import messages +from wagtail.admin.views.generic import lock from wagtail.models import Page -@require_POST -def lock(request, page_id): - # Get the page - page = get_object_or_404(Page, id=page_id).specific +class PageOperationViewMixin: + model = Page + pk_url_kwarg = "page_id" - # Check permissions - if not page.permissions_for_user(request.user).can_lock(): - raise PermissionDenied - # Lock the page - if not page.locked: - page.locked = True - page.locked_by = request.user - page.locked_at = timezone.now() - page.save(user=request.user, log_action="wagtail.lock") + def get_object(self): + return super().get_object().specific - # Redirect - redirect_to = request.POST.get("next", None) - if redirect_to and url_has_allowed_host_and_scheme( - url=redirect_to, allowed_hosts={request.get_host()} - ): - return redirect(redirect_to) - else: - return redirect("wagtailadmin_explore", page.get_parent().id) + def get_success_url(self): + if self.next_url: + return self.next_url + return reverse("wagtailadmin_explore", args=[self.object.get_parent().id]) -@require_POST -def unlock(request, page_id): - # Get the page - page = get_object_or_404(Page, id=page_id).specific +class LockView(PageOperationViewMixin, lock.LockView): + def perform_operation(self): + if not self.object.permissions_for_user(self.request.user).can_lock(): + raise PermissionDenied + return super().perform_operation() - # Check permissions - if not page.permissions_for_user(request.user).can_unlock(): - raise PermissionDenied - # Unlock the page - if page.locked: - page.locked = False - page.locked_by = None - page.locked_at = None - page.save(user=request.user, log_action="wagtail.unlock") +class UnlockView(PageOperationViewMixin, lock.UnlockView): + def perform_operation(self): + if not self.object.permissions_for_user(self.request.user).can_unlock(): + raise PermissionDenied + return super().perform_operation() - messages.success( - request, + def get_success_message(self): + return ( _("Page '%(page_title)s' is now unlocked.") - % {"page_title": page.get_admin_display_title()}, - extra_tags="unlock", + % {"page_title": self.object.get_admin_display_title()}, ) - - # Redirect - redirect_to = request.POST.get("next", None) - if redirect_to and url_has_allowed_host_and_scheme( - url=redirect_to, allowed_hosts={request.get_host()} - ): - return redirect(redirect_to) - else: - return redirect("wagtailadmin_explore", page.get_parent().id)