From 0ae8d5fc8c5b6175defc1c2e7f3753cb01006d0f Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 5 Oct 2023 22:42:39 +0100 Subject: [PATCH] Change construct_page_listing_buttons hook to pass a user argument instead of page_perms --- docs/reference/hooks.md | 8 ++- .../admin/templatetags/wagtailadmin_tags.py | 13 ++++- .../admin/tests/pages/test_explorer_view.py | 52 ++++++++++++++++--- wagtail/test/testapp/wagtail_hooks.py | 10 ---- 4 files changed, 62 insertions(+), 21 deletions(-) diff --git a/docs/reference/hooks.md b/docs/reference/hooks.md index 6c1b5327a6..8bbfa3a8a9 100644 --- a/docs/reference/hooks.md +++ b/docs/reference/hooks.md @@ -837,15 +837,19 @@ def make_publish_default_action(menu_items, request, context): ### `construct_page_listing_buttons` -Modify the final list of page listing buttons in the page explorer. The callable passed to this hook receives a list of `PageListingButton` objects, a page, a page perms object, and a context dictionary as per `register_page_listing_buttons`, and should modify the list of listing items in-place. +Modify the final list of page listing buttons in the page explorer. The callable passed to this hook receives a list of `PageListingButton` objects, a page, a user object, and a context dictionary, and should modify the list of listing items in-place. ```python @hooks.register('construct_page_listing_buttons') -def remove_page_listing_button_item(buttons, page, page_perms, context=None): +def remove_page_listing_button_item(buttons, page, user, context=None): if page.is_root: buttons.pop() # removes the last 'more' dropdown button on the root page listing buttons ``` +```{versionchanged} 5.2 +The hook function now receives a `user` argument instead of a `page_perms` argument. To check the user's permissions on the page, use `page.permissions_for_user(user)`. +``` + (construct_wagtail_userbar)= ### `construct_wagtail_userbar` diff --git a/wagtail/admin/templatetags/wagtailadmin_tags.py b/wagtail/admin/templatetags/wagtailadmin_tags.py index 004449ee3c..428f25addd 100644 --- a/wagtail/admin/templatetags/wagtailadmin_tags.py +++ b/wagtail/admin/templatetags/wagtailadmin_tags.py @@ -486,7 +486,18 @@ def page_listing_buttons(context, page, user): page_perms = page.permissions_for_user(user) for hook in hooks.get_hooks("construct_page_listing_buttons"): - hook(buttons, page, page_perms, context) + if accepts_kwarg(hook, "user"): + hook(buttons, page=page, user=user, context=context) + else: + # old-style hook that accepts page_perms instead of user + warn( + "`construct_page_listing_buttons` hook functions should accept a `user` argument instead of `page_perms` -" + f" {hook.__module__}.{hook.__name__} needs to be updated", + category=RemovedInWagtail60Warning, + ) + + page_perms = page.permissions_for_user(user) + hook(buttons, page, page_perms, context) return {"page": page, "buttons": buttons} diff --git a/wagtail/admin/tests/pages/test_explorer_view.py b/wagtail/admin/tests/pages/test_explorer_view.py index eeea1ec407..8e3db872fa 100644 --- a/wagtail/admin/tests/pages/test_explorer_view.py +++ b/wagtail/admin/tests/pages/test_explorer_view.py @@ -1,14 +1,16 @@ -from django.contrib.auth.models import Group, Permission +from django.contrib.auth.models import AbstractBaseUser, Group, Permission from django.contrib.contenttypes.models import ContentType from django.core import paginator from django.test import TestCase, override_settings from django.urls import reverse from wagtail import hooks +from wagtail.admin.widgets import Button from wagtail.models import GroupPagePermission, Locale, Page, Workflow from wagtail.test.testapp.models import SimplePage, SingleEventPage, StandardIndex from wagtail.test.utils import WagtailTestUtils from wagtail.test.utils.timestamps import local_datetime +from wagtail.utils.deprecation import RemovedInWagtail60Warning class TestPageExplorer(WagtailTestUtils, TestCase): @@ -179,13 +181,47 @@ class TestPageExplorer(WagtailTestUtils, TestCase): page_ids, [self.old_page.id, self.new_page.id, self.child_page.id] ) - def test_construct_page_listing_buttons_hook(self): - # testapp implements a construct_page_listing_buttons hook - # that add's an dummy button with the label 'Dummy Button' which points - # to '/dummy-button' - response = self.client.get( - reverse("wagtailadmin_explore", args=(self.root_page.id,)), - ) + def test_construct_page_listing_buttons_hook_with_old_signature(self): + def add_dummy_button(buttons, page, page_perms, context=None): + item = Button( + label="Dummy Button", + url="/dummy-button", + priority=10, + ) + buttons.append(item) + + with hooks.register_temporarily( + "construct_page_listing_buttons", add_dummy_button + ): + with self.assertWarnsMessage( + RemovedInWagtail60Warning, + "`construct_page_listing_buttons` hook functions should accept a `user` argument instead of `page_perms`", + ): + response = self.client.get( + reverse("wagtailadmin_explore", args=(self.root_page.id,)) + ) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "wagtailadmin/pages/index.html") + self.assertContains(response, "Dummy Button") + self.assertContains(response, "/dummy-button") + + def test_construct_page_listing_buttons_hook_with_new_signature(self): + def add_dummy_button(buttons, page, user, context=None): + if not isinstance(user, AbstractBaseUser): + raise TypeError("expected a user instance") + item = Button( + label="Dummy Button", + url="/dummy-button", + priority=10, + ) + buttons.append(item) + + with hooks.register_temporarily( + "construct_page_listing_buttons", add_dummy_button + ): + response = self.client.get( + reverse("wagtailadmin_explore", args=(self.root_page.id,)) + ) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, "wagtailadmin/pages/index.html") self.assertContains(response, "Dummy Button") diff --git a/wagtail/test/testapp/wagtail_hooks.py b/wagtail/test/testapp/wagtail_hooks.py index c9a43a4ace..b7ce966022 100644 --- a/wagtail/test/testapp/wagtail_hooks.py +++ b/wagtail/test/testapp/wagtail_hooks.py @@ -189,16 +189,6 @@ def register_relax_menu_item(menu_items, request, context): menu_items.append(RelaxMenuItem()) -@hooks.register("construct_page_listing_buttons") -def register_page_listing_button_item(buttons, page, page_perms, context=None): - item = Button( - label="Dummy Button", - url="/dummy-button", - priority=10, - ) - buttons.append(item) - - @hooks.register("construct_snippet_listing_buttons") def register_snippet_listing_button_item(buttons, snippet, user, context=None): item = Button(