From 99cb3154ccf3ee03df925e4bbe2b77d2e664acd1 Mon Sep 17 00:00:00 2001 From: jacobtm Date: Thu, 2 Jan 2020 09:41:42 +0000 Subject: [PATCH 1/7] Correct model reference for page, page cannot be edited when locked is True, rather than False --- docs/reference/pages/model_reference.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/pages/model_reference.rst b/docs/reference/pages/model_reference.rst index efc18fc30..3533ae209 100644 --- a/docs/reference/pages/model_reference.rst +++ b/docs/reference/pages/model_reference.rst @@ -115,7 +115,7 @@ Database fields The user who has currently locked the page. Only this user can edit the page. - If this is ``None`` when ``locked`` is ``False``, nobody can edit the page. + If this is ``None`` when ``locked`` is ``True``, nobody can edit the page. .. attribute:: locked_at From 7421590dc723866e36ca3ba2f6f5d4e760306093 Mon Sep 17 00:00:00 2001 From: jacobtm Date: Thu, 2 Jan 2020 09:47:52 +0000 Subject: [PATCH 2/7] Document reports submenu hooks --- docs/reference/hooks.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/reference/hooks.rst b/docs/reference/hooks.rst index ca449a610..5e1f5c58d 100644 --- a/docs/reference/hooks.rst +++ b/docs/reference/hooks.rst @@ -224,6 +224,22 @@ Hooks for building new areas of the admin interface (alongside pages, images, do As ``construct_main_menu``, but modifies the 'Settings' sub-menu rather than the top-level menu. +.. _register_reports_menu_item: + +``register_reports_menu_item`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + As ``register_admin_menu_item``, but registers menu items into the 'Reports' sub-menu rather than the top-level menu. + + +.. _construct_reports_menu: + +``construct_reports_menu`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + As ``construct_main_menu``, but modifies the 'Reports' sub-menu rather than the top-level menu. + + .. _register_admin_search_area: ``register_admin_search_area`` From 7a6ec88838700e9de96f2776394db21b8a241ef9 Mon Sep 17 00:00:00 2001 From: jacobtm Date: Thu, 2 Jan 2020 11:03:45 +0000 Subject: [PATCH 3/7] Make locked pages listing compatible with translation --- .../reports/listing/_list_unlock.html | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/wagtail/admin/templates/wagtailadmin/reports/listing/_list_unlock.html b/wagtail/admin/templates/wagtailadmin/reports/listing/_list_unlock.html index f1cc01d0e..22e7806cc 100644 --- a/wagtail/admin/templates/wagtailadmin/reports/listing/_list_unlock.html +++ b/wagtail/admin/templates/wagtailadmin/reports/listing/_list_unlock.html @@ -2,13 +2,28 @@ {% load i18n wagtailadmin_tags %} + {% block page_navigation %} {% page_permissions page as perms %}

- Locked - {% if page.locked_by %}by {% if page.locked_by_id == request.user.pk %}you{% else %}{{ page.locked_by }}{% endif %}{% endif %} - {% if page.locked_at %}at {{ page.locked_at|date:"d M Y H:i" }}{% endif %} + {% with page.locked_at|date:"d M Y H:i" as locking_date %} + {% if page.locked_by %} + {% if page.locked_by_id == request.user.pk %} + {% blocktrans %} + Locked by you at {{ locking_date }} + {% endblocktrans %} + {% else %} + {% blocktrans with locked_by=page.locked_by %} + Locked by {{ locked_by }} at {{ locking_date }} + {% endblocktrans %} + {% endif %} + {% else %} + {% blocktrans %} + Locked at {{ locking_date }} + {% endblocktrans %} + {% endif %} + {% endwith %}

{% if perms.can_unlock %}
From 3de77ba5d71845c4fc4afba01e96fce4a0dcecad Mon Sep 17 00:00:00 2001 From: jacobtm Date: Thu, 2 Jan 2020 11:59:26 +0000 Subject: [PATCH 4/7] Add can_unlock_pages to UserPagePermissionsProxy and use to add permissions checks to locked pages report --- wagtail/admin/views/reports.py | 6 ++++++ wagtail/admin/wagtail_hooks.py | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/wagtail/admin/views/reports.py b/wagtail/admin/views/reports.py index 2b75f716e..c6e987333 100644 --- a/wagtail/admin/views/reports.py +++ b/wagtail/admin/views/reports.py @@ -2,6 +2,7 @@ from django.utils.translation import ugettext_lazy as _ from django.views.generic.base import TemplateResponseMixin from django.views.generic.list import BaseListView +from wagtail.admin.auth import permission_denied from wagtail.core.models import UserPagePermissionsProxy @@ -28,3 +29,8 @@ class LockedPagesView(ReportView): pages = UserPagePermissionsProxy(self.request.user).editable_pages().filter(locked=True) self.queryset = pages return super().get_queryset() + + def dispatch(self, request, *args, **kwargs): + if not UserPagePermissionsProxy(request.user).can_unlock_pages(): + return permission_denied(request) + return super().dispatch(request, *args, **kwargs) diff --git a/wagtail/admin/wagtail_hooks.py b/wagtail/admin/wagtail_hooks.py index 1cb2aa833..1d27c57fd 100644 --- a/wagtail/admin/wagtail_hooks.py +++ b/wagtail/admin/wagtail_hooks.py @@ -620,9 +620,14 @@ class ReportsMenuItem(SubmenuMenuItem): template = 'wagtailadmin/shared/menu_submenu_item.html' +class LockedPagesMenuItem(MenuItem): + def is_shown(self, request): + return UserPagePermissionsProxy(request.user).can_unlock_pages() + + @hooks.register('register_reports_menu_item') def register_locked_pages_menu_item(): - return MenuItem(_('Locked Pages'), reverse('wagtailadmin_reports:locked_pages'), classnames='icon icon-locked', order=700) + return LockedPagesMenuItem(_('Locked Pages'), reverse('wagtailadmin_reports:locked_pages'), classnames='icon icon-locked', order=700) @hooks.register('register_admin_menu_item') From 8a6165add8a6f934b44d4131ec83ecad02913bf7 Mon Sep 17 00:00:00 2001 From: jacobtm Date: Fri, 3 Jan 2020 11:05:55 +0000 Subject: [PATCH 5/7] Account for missing locked_at --- .../reports/listing/_list_unlock.html | 32 +++++++++++-------- wagtail/core/models.py | 24 ++++++++++++++ 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/wagtail/admin/templates/wagtailadmin/reports/listing/_list_unlock.html b/wagtail/admin/templates/wagtailadmin/reports/listing/_list_unlock.html index 22e7806cc..4e386f1b4 100644 --- a/wagtail/admin/templates/wagtailadmin/reports/listing/_list_unlock.html +++ b/wagtail/admin/templates/wagtailadmin/reports/listing/_list_unlock.html @@ -7,23 +7,27 @@ {% page_permissions page as perms %}

- {% with page.locked_at|date:"d M Y H:i" as locking_date %} - {% if page.locked_by %} - {% if page.locked_by_id == request.user.pk %} - {% blocktrans %} - Locked by you at {{ locking_date }} - {% endblocktrans %} + {% if page.locked_at %} + {% with page.locked_at|date:"d M Y H:i" as locking_date %} + {% if page.locked_by %} + {% if page.locked_by_id == request.user.pk %} + {% blocktrans %} + Locked by you at {{ locking_date }} + {% endblocktrans %} + {% else %} + {% blocktrans with locked_by=page.locked_by %} + Locked by {{ locked_by }} at {{ locking_date }} + {% endblocktrans %} + {% endif %} {% else %} - {% blocktrans with locked_by=page.locked_by %} - Locked by {{ locked_by }} at {{ locking_date }} + {% blocktrans %} + Locked at {{ locking_date }} {% endblocktrans %} {% endif %} - {% else %} - {% blocktrans %} - Locked at {{ locking_date }} - {% endblocktrans %} - {% endif %} - {% endwith %} + {% endwith %} + {% else %} + {% trans 'Locked' %} + {% endif %}

{% if perms.can_unlock %} diff --git a/wagtail/core/models.py b/wagtail/core/models.py index 68750448e..3dcff0b4d 100644 --- a/wagtail/core/models.py +++ b/wagtail/core/models.py @@ -1833,6 +1833,30 @@ class UserPagePermissionsProxy: """Return True if the user has permission to publish any pages""" return self.publishable_pages().exists() + def unlockable_pages(self): + """Return a queryset of the pages that this user has permission to unlock""" + # Deal with the trivial cases first... + if not self.user.is_active: + return Page.objects.none() + if self.user.is_superuser: + return Page.objects.all() + + unlockable_pages = Page.objects.none() + + for perm in self.permissions.filter(permission_type='unlock'): + # user has publish permission on any subpage of perm.page + # (including perm.page itself) + unlockable_pages |= Page.objects.descendant_of(perm.page, inclusive=True) + + pages_locked_by_user = Page.objects.filter(locked_by=self.user) + unlockable_pages |= pages_locked_by_user + + return unlockable_pages + + def can_unlock_pages(self): + """Return True if the user has permission to unlock any pages""" + return self.unlockable_pages().exists() + class PagePermissionTester: def __init__(self, user_perms, page): From df194b40f348fcfe0b447286b7c57ac96b2a12d1 Mon Sep 17 00:00:00 2001 From: jacobtm Date: Tue, 7 Jan 2020 14:57:00 +0000 Subject: [PATCH 6/7] Display locked pages report only if user has 'unlock any page' permission for consistency with rfc --- .../wagtailadmin/home/locked_pages.html | 4 ++- wagtail/admin/views/home.py | 3 ++- wagtail/admin/wagtail_hooks.py | 2 +- wagtail/core/models.py | 26 +++---------------- 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/wagtail/admin/templates/wagtailadmin/home/locked_pages.html b/wagtail/admin/templates/wagtailadmin/home/locked_pages.html index 5e649d1cf..9f579cd21 100644 --- a/wagtail/admin/templates/wagtailadmin/home/locked_pages.html +++ b/wagtail/admin/templates/wagtailadmin/home/locked_pages.html @@ -3,7 +3,9 @@
{# TODO try moving these classes onto the section tag #}

{% trans "Your locked pages" %}

- {% trans "See all locked pages" %} + {% if can_remove_locks %} + {% trans "See all locked pages" %} + {% endif %} diff --git a/wagtail/admin/views/home.py b/wagtail/admin/views/home.py index cd9edc0d8..80ad25fc4 100644 --- a/wagtail/admin/views/home.py +++ b/wagtail/admin/views/home.py @@ -59,7 +59,8 @@ class LockedPagesPanel: 'locked_pages': Page.objects.filter( locked=True, locked_by=self.request.user, - ) + ), + 'can_remove_locks': UserPagePermissionsProxy(self.request.user).can_remove_locks() }, request=self.request) diff --git a/wagtail/admin/wagtail_hooks.py b/wagtail/admin/wagtail_hooks.py index 1d27c57fd..19842168d 100644 --- a/wagtail/admin/wagtail_hooks.py +++ b/wagtail/admin/wagtail_hooks.py @@ -622,7 +622,7 @@ class ReportsMenuItem(SubmenuMenuItem): class LockedPagesMenuItem(MenuItem): def is_shown(self, request): - return UserPagePermissionsProxy(request.user).can_unlock_pages() + return UserPagePermissionsProxy(request.user).can_remove_locks() @hooks.register('register_reports_menu_item') diff --git a/wagtail/core/models.py b/wagtail/core/models.py index 3dcff0b4d..9c8ff9495 100644 --- a/wagtail/core/models.py +++ b/wagtail/core/models.py @@ -1833,29 +1833,9 @@ class UserPagePermissionsProxy: """Return True if the user has permission to publish any pages""" return self.publishable_pages().exists() - def unlockable_pages(self): - """Return a queryset of the pages that this user has permission to unlock""" - # Deal with the trivial cases first... - if not self.user.is_active: - return Page.objects.none() - if self.user.is_superuser: - return Page.objects.all() - - unlockable_pages = Page.objects.none() - - for perm in self.permissions.filter(permission_type='unlock'): - # user has publish permission on any subpage of perm.page - # (including perm.page itself) - unlockable_pages |= Page.objects.descendant_of(perm.page, inclusive=True) - - pages_locked_by_user = Page.objects.filter(locked_by=self.user) - unlockable_pages |= pages_locked_by_user - - return unlockable_pages - - def can_unlock_pages(self): - """Return True if the user has permission to unlock any pages""" - return self.unlockable_pages().exists() + def can_remove_locks(self): + """Returns True if the user has permission to unlock pages they have not locked""" + return self.user.is_superuser or self.permissions.filter(permission_type='unlock').exists() class PagePermissionTester: From 9fbeff41b362d3138ecc8a3d48d68d58a73e2b59 Mon Sep 17 00:00:00 2001 From: jacobtm Date: Tue, 7 Jan 2020 17:00:17 +0000 Subject: [PATCH 7/7] Update reference to , add basic tests for locked pages report, and update to cope with missing UserPagePermissionsProxy.permissions if inactive or superuser --- wagtail/admin/tests/test_reports_views.py | 35 +++++++++++++++++++++++ wagtail/admin/views/reports.py | 2 +- wagtail/core/models.py | 7 ++++- 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 wagtail/admin/tests/test_reports_views.py diff --git a/wagtail/admin/tests/test_reports_views.py b/wagtail/admin/tests/test_reports_views.py new file mode 100644 index 000000000..be8e3470c --- /dev/null +++ b/wagtail/admin/tests/test_reports_views.py @@ -0,0 +1,35 @@ +from django.test import TestCase +from django.urls import reverse +from django.utils import timezone + +from wagtail.core.models import Page +from wagtail.tests.utils import WagtailTestUtils + + +class TestLockedPagesView(TestCase, WagtailTestUtils): + def setUp(self): + self.user = self.login() + + def get(self, params={}): + return self.client.get(reverse('wagtailadmin_reports:locked_pages'), params) + + def test_simple(self): + response = self.get() + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/reports/locked_pages.html') + + # Initially there should be no locked pages + self.assertContains(response, "No locked pages found.") + + self.page = Page.objects.first() + self.page.locked = True + self.page.locked_by = self.user + self.page.locked_at = timezone.now() + self.page.save() + + # Now the listing should contain our locked page + response = self.get() + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/reports/locked_pages.html') + self.assertNotContains(response, "No locked pages found.") + self.assertContains(response, self.page.title) diff --git a/wagtail/admin/views/reports.py b/wagtail/admin/views/reports.py index c6e987333..9ab2ca8bd 100644 --- a/wagtail/admin/views/reports.py +++ b/wagtail/admin/views/reports.py @@ -31,6 +31,6 @@ class LockedPagesView(ReportView): return super().get_queryset() def dispatch(self, request, *args, **kwargs): - if not UserPagePermissionsProxy(request.user).can_unlock_pages(): + if not UserPagePermissionsProxy(request.user).can_remove_locks(): return permission_denied(request) return super().dispatch(request, *args, **kwargs) diff --git a/wagtail/core/models.py b/wagtail/core/models.py index 9c8ff9495..10bd73d7c 100644 --- a/wagtail/core/models.py +++ b/wagtail/core/models.py @@ -1835,7 +1835,12 @@ class UserPagePermissionsProxy: def can_remove_locks(self): """Returns True if the user has permission to unlock pages they have not locked""" - return self.user.is_superuser or self.permissions.filter(permission_type='unlock').exists() + if self.user.is_superuser: + return True + if not self.user.is_active: + return False + else: + return self.permissions.filter(permission_type='unlock').exists() class PagePermissionTester: