0
0
mirror of https://github.com/wagtail/wagtail.git synced 2024-11-30 01:46:24 +01:00

Refactor how lock banner messages are constructed

This commit is contained in:
Karl Hobley 2022-07-26 15:56:41 +00:00 committed by Matt Westcott
parent 9dcf09d27f
commit 315d9a938a
3 changed files with 144 additions and 87 deletions

View File

@ -10,7 +10,6 @@ from django.shortcuts import get_object_or_404, redirect
from django.urls import reverse
from django.utils import timezone
from django.utils.html import format_html
from django.utils.safestring import mark_safe
from django.utils.translation import gettext as _
from django.views.generic.base import ContextMixin, TemplateResponseMixin, View
@ -22,6 +21,7 @@ from wagtail.admin.ui.side_panels import PageSidePanels
from wagtail.admin.views.generic import HookResponseMixin
from wagtail.admin.views.pages.utils import get_valid_next_url_from_request
from wagtail.exceptions import PageClassNotFoundError
from wagtail.locks import BasicLock
from wagtail.models import (
COMMENTS_RELATION_NAME,
Comment,
@ -334,6 +334,10 @@ class EditView(TemplateResponseMixin, ContextMixin, HookResponseMixin, View):
self.parent = self.page.get_parent()
self.page_perms = self.page.permissions_for_user(self.request.user)
self.lock = self.page.get_lock()
self.locked_for_user = self.lock is not None and self.lock.for_user(
self.request.user
)
if not self.page_perms.can_edit():
raise PermissionDenied
@ -375,79 +379,21 @@ class EditView(TemplateResponseMixin, ContextMixin, HookResponseMixin, View):
return super().dispatch(request)
def get(self, request):
if self.page_perms.user_has_lock():
if self.page.locked_at:
lock_message = format_html(
_("<b>Page '{}' was locked</b> by <b>you</b> on <b>{}</b>."),
self.page.get_admin_display_title(),
self.page.locked_at.strftime("%d %b %Y %H:%M"),
)
else:
lock_message = format_html(
_("<b>Page '{}' is locked</b> by <b>you</b>."),
self.page.get_admin_display_title(),
)
lock_message += format_html(
'<span class="buttons"><button type="button" class="button button-small button-secondary" data-action-lock-unlock data-url="{}">{}</button></span>',
reverse("wagtailadmin_pages:unlock", args=(self.page.id,)),
_("Unlock"),
)
messages.warning(self.request, lock_message, extra_tags="lock")
elif self.page.locked and self.page_perms.page_locked():
# the page can also be locked at a permissions level if in a workflow, on a task the user is not a reviewer for
# this should be indicated separately
if self.page.locked_by and self.page.locked_at:
lock_message = format_html(
_("<b>Page '{}' was locked</b> by <b>{}</b> on <b>{}</b>."),
self.page.get_admin_display_title(),
str(self.page.locked_by),
self.page.locked_at.strftime("%d %b %Y %H:%M"),
)
else:
# Page was probably locked with an old version of Wagtail, or a script
lock_message = format_html(
_("<b>Page '{}' is locked</b>."),
self.page.get_admin_display_title(),
)
if self.page_perms.can_unlock():
lock_message += format_html(
'<span class="buttons"><button type="button" class="button button-small button-secondary" data-action-lock-unlock data-url="{}">{}</button></span>',
reverse("wagtailadmin_pages:unlock", args=(self.page.id,)),
_("Unlock"),
)
messages.error(self.request, lock_message, extra_tags="lock")
if self.page.current_workflow_state:
workflow = self.workflow_state.workflow
task = self.workflow_state.current_task_state.task
if (
self.workflow_state.status != WorkflowState.STATUS_NEEDS_CHANGES
and task.specific.page_locked_for_user(self.page, self.request.user)
):
# Check for revisions still undergoing moderation and warn
if len(self.workflow_tasks) == 1:
# If only one task in workflow, show simple message
workflow_info = _("This page is currently awaiting moderation.")
else:
workflow_info = format_html(
_(
"This page is awaiting <b>'{}'</b> in the <b>'{}'</b> workflow."
),
task.name,
workflow.name,
if self.lock:
lock_message = self.lock.get_message(self.request.user)
if lock_message:
if isinstance(self.lock, BasicLock) and self.page_perms.can_unlock():
lock_message = format_html(
'{} <span class="buttons"><button type="button" class="button button-small button-secondary" data-action-lock-unlock data-url="{}">{}</button></span>',
lock_message,
reverse("wagtailadmin_pages:unlock", args=(self.page.id,)),
_("Unlock"),
)
messages.error(
self.request,
mark_safe(
workflow_info
+ " "
+ _("Only reviewers for this task can edit the page.")
),
extra_tags="lock",
)
if self.locked_for_user:
messages.error(self.request, lock_message, extra_tags="lock")
else:
messages.warning(self.request, lock_message, extra_tags="lock")
self.form = self.form_class(
instance=self.page,
@ -496,7 +442,7 @@ class EditView(TemplateResponseMixin, ContextMixin, HookResponseMixin, View):
and self.workflow_state.user_can_cancel(self.request.user)
)
if self.form.is_valid() and not self.page_perms.page_locked():
if self.form.is_valid() and not self.locked_for_user:
return self.form_valid(self.form)
else:
return self.form_invalid(self.form)
@ -840,7 +786,7 @@ class EditView(TemplateResponseMixin, ContextMixin, HookResponseMixin, View):
self.workflow_state.cancel(user=self.request.user)
self.add_cancel_workflow_confirmation_message()
if self.page_perms.page_locked():
if self.locked_for_user:
messages.error(
self.request, _("The page could not be saved as it is locked")
)
@ -889,7 +835,7 @@ class EditView(TemplateResponseMixin, ContextMixin, HookResponseMixin, View):
"form": self.form,
"next": self.next_url,
"has_unsaved_changes": self.has_unsaved_changes,
"page_locked": self.page_perms.page_locked(),
"page_locked": self.locked_for_user,
"workflow_state": self.workflow_state
if self.workflow_state and self.workflow_state.is_active
else None,

View File

@ -1,4 +1,7 @@
from django.conf import settings
from django.utils.html import format_html
from django.utils.safestring import mark_safe
from django.utils.translation import gettext as _
class BaseLock:
@ -14,6 +17,12 @@ class BaseLock:
"""
return NotImplemented
def get_message(self, user):
"""
Returns a message to display to the given user describing the lock.
"""
return None
class BasicLock(BaseLock):
"""
@ -32,6 +41,35 @@ class BasicLock(BaseLock):
else:
return user.pk != self.page.locked_by_id
def get_message(self, user):
if self.page.locked_by_id == user.pk:
if self.page.locked_at:
return format_html(
_("<b>Page '{}' was locked</b> by <b>you</b> on <b>{}</b>."),
self.page.get_admin_display_title(),
self.page.locked_at.strftime("%d %b %Y %H:%M"),
)
else:
return format_html(
_("<b>Page '{}' is locked</b> by <b>you</b>."),
self.page.get_admin_display_title(),
)
else:
if self.page.locked_by and self.page.locked_at:
return format_html(
_("<b>Page '{}' was locked</b> by <b>{}</b> on <b>{}</b>."),
self.page.get_admin_display_title(),
str(self.page.locked_by),
self.page.locked_at.strftime("%d %b %Y %H:%M"),
)
else:
# Page was probably locked with an old version of Wagtail, or a script
return format_html(
_("<b>Page '{}' is locked</b>."),
self.page.get_admin_display_title(),
)
class WorkflowLock(BaseLock):
"""
@ -46,3 +84,21 @@ class WorkflowLock(BaseLock):
def for_user(self, user):
return self.task.page_locked_for_user(self.page, user)
def get_message(self, user):
if self.for_user(user):
if len(self.page.current_workflow_state.all_tasks_with_status()) == 1:
# If only one task in workflow, show simple message
workflow_info = _("This page is currently awaiting moderation.")
else:
workflow_info = format_html(
_("This page is awaiting <b>'{}'</b> in the <b>'{}'</b> workflow."),
self.task.name,
self.page.current_workflow_state.workflow.name,
)
return mark_safe(
workflow_info
+ " "
+ _("Only reviewers for this task can edit the page.")
)

View File

@ -3642,11 +3642,38 @@ class TestGetLock(TestCase):
christmas_event = EventPage.objects.get(url_path="/home/events/christmas/")
christmas_event.locked = True
christmas_event.locked_by = christmas_event.locked_by = moderator
christmas_event.locked_by = moderator
christmas_event.locked_at = datetime.datetime(2022, 7, 29, 12, 19, 0)
self.assertIsInstance(christmas_event.get_lock(), BasicLock)
self.assertTrue(christmas_event.get_lock().for_user(christmas_event.owner))
self.assertFalse(christmas_event.get_lock().for_user(moderator))
lock = christmas_event.get_lock()
self.assertIsInstance(lock, BasicLock)
self.assertTrue(lock.for_user(christmas_event.owner))
self.assertFalse(lock.for_user(moderator))
self.assertEqual(
lock.get_message(christmas_event.owner),
f"<b>Page 'Christmas' was locked</b> by <b>{str(moderator)}</b> on <b>29 Jul 2022 12:19</b>.",
)
self.assertEqual(
lock.get_message(moderator),
"<b>Page 'Christmas' was locked</b> by <b>you</b> on <b>29 Jul 2022 12:19</b>.",
)
def test_when_locked_without_locked_at(self):
moderator = get_user_model().objects.get(email="eventmoderator@example.com")
christmas_event = EventPage.objects.get(url_path="/home/events/christmas/")
christmas_event.locked = True
christmas_event.locked_by = moderator
lock = christmas_event.get_lock()
self.assertEqual(
lock.get_message(christmas_event.owner),
"<b>Page 'Christmas' is locked</b>.",
)
self.assertEqual(
lock.get_message(moderator),
"<b>Page 'Christmas' is locked</b> by <b>you</b>.",
)
@override_settings(WAGTAILADMIN_GLOBAL_PAGE_EDIT_LOCK=True)
def test_when_locked_globally(self):
@ -3654,11 +3681,21 @@ class TestGetLock(TestCase):
christmas_event = EventPage.objects.get(url_path="/home/events/christmas/")
christmas_event.locked = True
christmas_event.locked_by = christmas_event.locked_by = moderator
christmas_event.locked_by = moderator
christmas_event.locked_at = datetime.datetime(2022, 7, 29, 12, 19, 0)
self.assertIsInstance(christmas_event.get_lock(), BasicLock)
self.assertTrue(christmas_event.get_lock().for_user(christmas_event.owner))
self.assertTrue(christmas_event.get_lock().for_user(moderator))
lock = christmas_event.get_lock()
self.assertIsInstance(lock, BasicLock)
self.assertTrue(lock.for_user(christmas_event.owner))
self.assertTrue(lock.for_user(moderator))
self.assertEqual(
lock.get_message(christmas_event.owner),
f"<b>Page 'Christmas' was locked</b> by <b>{str(moderator)}</b> on <b>29 Jul 2022 12:19</b>.",
)
self.assertEqual(
lock.get_message(moderator),
"<b>Page 'Christmas' was locked</b> by <b>you</b> on <b>29 Jul 2022 12:19</b>.",
)
def test_when_locked_by_workflow(self):
moderator = get_user_model().objects.get(email="eventmoderator@example.com")
@ -3672,6 +3709,24 @@ class TestGetLock(TestCase):
WorkflowTask.objects.create(workflow=workflow, task=task, sort_order=1)
workflow.start(christmas_event, moderator)
self.assertIsInstance(christmas_event.get_lock(), WorkflowLock)
self.assertTrue(christmas_event.get_lock().for_user(christmas_event.owner))
self.assertFalse(christmas_event.get_lock().for_user(moderator))
lock = christmas_event.get_lock()
self.assertIsInstance(lock, WorkflowLock)
self.assertTrue(lock.for_user(christmas_event.owner))
self.assertFalse(lock.for_user(moderator))
self.assertEqual(
lock.get_message(christmas_event.owner),
"This page is currently awaiting moderation. Only reviewers for this task can edit the page.",
)
self.assertIsNone(lock.get_message(moderator))
# When visiting a page in a workflow with multiple tasks, the message displayed to users changes to show the current task the page is on
# Add a second task to the workflow
other_task = GroupApprovalTask.objects.create(name="another_task")
WorkflowTask.objects.create(workflow=workflow, task=other_task, sort_order=2)
lock = christmas_event.get_lock()
self.assertEqual(
lock.get_message(christmas_event.owner),
"This page is awaiting <b>'test_task'</b> in the <b>'test_workflow'</b> workflow. Only reviewers for this task can edit the page.",
)