diff --git a/docs/reference/hooks.md b/docs/reference/hooks.md index 6ef0307542..4fc4759c6e 100644 --- a/docs/reference/hooks.md +++ b/docs/reference/hooks.md @@ -1042,7 +1042,7 @@ This example will add a simple button to the dropdown menu: from wagtail.admin import widgets as wagtailadmin_widgets @hooks.register('register_page_listing_more_buttons') -def page_listing_more_buttons(page, page_perms, next_url=None): +def page_listing_more_buttons(page, user, next_url=None): yield wagtailadmin_widgets.Button( 'A dropdown button', '/goes/to/a/url/', @@ -1053,11 +1053,15 @@ def page_listing_more_buttons(page, page_perms, next_url=None): The arguments passed to the hook are as follows: - `page` - the page object to generate the button for -- `page_perms` - a `PagePermissionTester` object that can be queried to determine the current user's permissions on the given page +- `user` - the logged-in user - `next_url` - the URL that the linked action should redirect back to on completion of the action, if the view supports it The `priority` argument controls the order the buttons are displayed in the dropdown. Buttons are ordered from low to high priority, so a button with `priority=10` will be displayed before a button with `priority=60`. +```{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)`. +``` + #### Buttons with dropdown lists The admin widgets also provide `ButtonWithDropdownFromHook`, which allows you to define a custom hook for generating a dropdown menu that gets attached to your button. @@ -1065,7 +1069,7 @@ The admin widgets also provide `ButtonWithDropdownFromHook`, which allows you to Creating a button with a dropdown menu involves two steps. Firstly, you add your button to the `register_page_listing_buttons` hook, just like the example above. Secondly, you register a new hook that yields the contents of the dropdown menu. -This example shows how Wagtail's default admin dropdown is implemented. You can also see how to register buttons conditionally, in this case by evaluating the `page_perms`: +This example shows how Wagtail's default admin dropdown is implemented. You can also see how to register buttons conditionally, in this case by testing the user's permission with `page.permissions_for_user`: ```python from wagtail.admin import widgets as wagtailadmin_widgets @@ -1076,13 +1080,14 @@ def page_custom_listing_buttons(page, user, next_url=None): 'More actions', hook_name='my_button_dropdown_hook', page=page, - page_perms=page.permissions_for_user(user), + user=user, next_url=next_url, priority=50 ) @hooks.register('my_button_dropdown_hook') -def page_custom_listing_more_buttons(page, page_perms, next_url=None): +def page_custom_listing_more_buttons(page, user, next_url=None): + page_perms = page.permissions_for_user(user) if page_perms.can_move(): yield wagtailadmin_widgets.Button('Move', reverse('wagtailadmin_pages:move', args=[page.id]), priority=10) if page_perms.can_delete(): @@ -1093,6 +1098,10 @@ def page_custom_listing_more_buttons(page, page_perms, next_url=None): The template for the dropdown button can be customised by overriding `wagtailadmin/pages/listing/_button_with_dropdown.html`. Make sure to leave the dropdown UI component itself as-is. +```{versionchanged} 5.2 +The `ButtonWithDropdownFromHook` constructor, and the corresponding hook function, now receive a `user` argument instead of a `page_perms` argument. +``` + (construct_page_listing_buttons)= ### `construct_page_listing_buttons` diff --git a/wagtail/admin/tests/test_buttons_hooks.py b/wagtail/admin/tests/test_buttons_hooks.py index c20a2e1390..dac2317e9e 100644 --- a/wagtail/admin/tests/test_buttons_hooks.py +++ b/wagtail/admin/tests/test_buttons_hooks.py @@ -79,7 +79,7 @@ class TestPageListingButtonsHooks(TestButtonsHooks): class TestPageListingMoreButtonsHooks(TestButtonsHooks): - def test_register_page_listing_more_buttons(self): + def test_register_page_listing_more_buttons_with_old_signature(self): def page_listing_more_buttons(page, page_perms, next_url=None): yield wagtailadmin_widgets.Button( 'Another useless button in default "More" dropdown', @@ -87,6 +87,37 @@ class TestPageListingMoreButtonsHooks(TestButtonsHooks): priority=10, ) + with hooks.register_temporarily( + "register_page_listing_more_buttons", page_listing_more_buttons + ), self.assertWarnsMessage( + RemovedInWagtail60Warning, + "`register_page_listing_more_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/listing/_button_with_dropdown.html" + ) + self.assertTemplateUsed(response, "wagtailadmin/shared/buttons.html") + + self.assertContains( + response, "Another useless button in default "More" dropdown" + ) + + def test_register_page_listing_more_buttons_with_new_signature(self): + def page_listing_more_buttons(page, user, next_url=None): + if not isinstance(user, AbstractBaseUser): + raise TypeError("expected a user instance") + + yield wagtailadmin_widgets.Button( + 'Another useless button in default "More" dropdown', + "/custom-url", + priority=10, + ) + with hooks.register_temporarily( "register_page_listing_more_buttons", page_listing_more_buttons ): @@ -104,13 +135,32 @@ class TestPageListingMoreButtonsHooks(TestButtonsHooks): response, "Another useless button in default "More" dropdown" ) - def test_custom_button_with_dropdown(self): + def test_button_with_dropdown_from_hook_accepts_page_perms_argument(self): + page = self.root_page + + with self.assertWarnsMessage( + RemovedInWagtail60Warning, + "ButtonWithDropdownFromHook should be passed a `user` argument instead of `page_perms`", + ): + button = wagtailadmin_widgets.ButtonWithDropdownFromHook( + "One more more button", + hook_name="register_page_listing_one_more_more_buttons", + page=page, + page_perms=page.permissions_for_user(self.user), + next_url="/custom-url", + attrs={"target": "_blank", "rel": "noreferrer"}, + priority=50, + ) + + self.assertEqual(button.user, self.user) + + def test_custom_button_with_dropdown_with_old_signature(self): def page_custom_listing_buttons(page, user, next_url=None): yield wagtailadmin_widgets.ButtonWithDropdownFromHook( "One more more button", hook_name="register_page_listing_one_more_more_buttons", page=page, - page_perms=page.permissions_for_user(user), + user=user, next_url=next_url, attrs={"target": "_blank", "rel": "noreferrer"}, priority=50, @@ -123,6 +173,53 @@ class TestPageListingMoreButtonsHooks(TestButtonsHooks): priority=10, ) + with hooks.register_temporarily( + "register_page_listing_buttons", page_custom_listing_buttons + ), hooks.register_temporarily( + "register_page_listing_one_more_more_buttons", + page_custom_listing_more_buttons, + ), self.assertWarnsMessage( + RemovedInWagtail60Warning, + "`register_page_listing_one_more_more_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/listing/_button_with_dropdown.html" + ) + self.assertTemplateUsed(response, "wagtailadmin/shared/buttons.html") + + self.assertContains(response, "One more more button") + self.assertContains( + response, + "Another useless dropdown button in "One more more button" dropdown", + ) + + def test_custom_button_with_dropdown_with_new_signature(self): + def page_custom_listing_buttons(page, user, next_url=None): + yield wagtailadmin_widgets.ButtonWithDropdownFromHook( + "One more more button", + hook_name="register_page_listing_one_more_more_buttons", + page=page, + user=user, + next_url=next_url, + attrs={"target": "_blank", "rel": "noreferrer"}, + priority=50, + ) + + def page_custom_listing_more_buttons(page, user, next_url=None): + if not isinstance(user, AbstractBaseUser): + raise TypeError("expected a user instance") + + yield wagtailadmin_widgets.Button( + 'Another useless dropdown button in "One more more button" dropdown', + "/custom-url", + priority=10, + ) + with hooks.register_temporarily( "register_page_listing_buttons", page_custom_listing_buttons ), hooks.register_temporarily( @@ -152,13 +249,12 @@ class TestPageListingMoreButtonsHooks(TestButtonsHooks): # page_listing_more_button generator yields only `Delete button` with this permission set page = self.root_page - page_perms = page.permissions_for_user(self.user) base_url = reverse("wagtailadmin_pages:delete", args=[page.id]) next_url = "a/random/url/" full_url = base_url + "?" + urlencode({"next": next_url}) - buttons = page_listing_more_buttons(page, page_perms, next_url=next_url) + buttons = page_listing_more_buttons(page, user=self.user, next_url=next_url) delete_button = next(button for button in buttons if button.label == "Delete") self.assertEqual(delete_button.url, full_url) @@ -171,12 +267,11 @@ class TestPageListingMoreButtonsHooks(TestButtonsHooks): """ page = self.root_page - page_perms = page.permissions_for_user(self.user) base_url = reverse("wagtailadmin_pages:delete", args=[page.id]) next_url = reverse("wagtailadmin_explore", args=[page.id]) - buttons = page_listing_more_buttons(page, page_perms, next_url=next_url) + buttons = page_listing_more_buttons(page, user=self.user, next_url=next_url) delete_button = next(button for button in buttons if button.label == "Delete") @@ -197,12 +292,11 @@ class TestPageListingMoreButtonsHooks(TestButtonsHooks): # Test with a user with no publish permission (and thus no ability to reorder) editor = self.create_user(username="editor", password="password") editor.groups.add(Group.objects.get(name="Editors")) - page_perms = page.permissions_for_user(editor) # no button returned buttons = [ button - for button in page_listing_more_buttons(page, page_perms) + for button in page_listing_more_buttons(page, user=editor) if button.show ] self.assertEqual( @@ -212,12 +306,11 @@ class TestPageListingMoreButtonsHooks(TestButtonsHooks): # Test with a user with publish permission publisher = self.create_user(username="publisher", password="password") publisher.groups.add(Group.objects.get(name="Moderators")) - page_perms = page.permissions_for_user(publisher) # page_listing_more_button generator yields `Sort menu order button` buttons = [ button - for button in page_listing_more_buttons(page, page_perms) + for button in page_listing_more_buttons(page, user=publisher) if button.show ] reorder_button = next( diff --git a/wagtail/admin/wagtail_hooks.py b/wagtail/admin/wagtail_hooks.py index 1482dcd645..e0b08cb2d0 100644 --- a/wagtail/admin/wagtail_hooks.py +++ b/wagtail/admin/wagtail_hooks.py @@ -227,7 +227,7 @@ def page_listing_buttons(page, user, next_url=None): "", hook_name="register_page_listing_more_buttons", page=page, - page_perms=page.permissions_for_user(user), + user=user, next_url=next_url, icon_name="dots-horizontal", attrs={ @@ -374,7 +374,8 @@ class PageListingSortMenuOrderButton(PageListingButton): @hooks.register("register_page_listing_more_buttons") -def page_listing_more_buttons(page, page_perms, next_url=None): +def page_listing_more_buttons(page, user, next_url=None): + page_perms = page.permissions_for_user(user) yield PageListingEditButton( page=page, page_perms=page_perms, diff --git a/wagtail/admin/widgets/button.py b/wagtail/admin/widgets/button.py index 0321c4cf34..ae108e73a8 100644 --- a/wagtail/admin/widgets/button.py +++ b/wagtail/admin/widgets/button.py @@ -1,3 +1,5 @@ +from warnings import warn + from django.forms.utils import flatatt from django.urls import reverse from django.utils.functional import cached_property @@ -6,6 +8,8 @@ from django.utils.http import urlencode from wagtail import hooks from wagtail.admin.ui.components import Component +from wagtail.coreutils import accepts_kwarg +from wagtail.utils.deprecation import RemovedInWagtail60Warning class Button(Component): @@ -163,10 +167,32 @@ class ButtonWithDropdown(BaseDropdownMenuButton): class ButtonWithDropdownFromHook(BaseDropdownMenuButton): - def __init__(self, label, hook_name, page, page_perms, next_url=None, **kwargs): + def __init__( + self, + label, + hook_name, + page, + user=None, + page_perms=None, + next_url=None, + **kwargs, + ): self.hook_name = hook_name self.page = page - self.page_perms = page_perms + + if user is None: + if page_perms is not None: + warn( + "ButtonWithDropdownFromHook should be passed a `user` argument instead of `page_perms`", + category=RemovedInWagtail60Warning, + stacklevel=2, + ) + self.user = page_perms.user + else: + raise TypeError("ButtonWithDropdownFromHook requires a `user` argument") + else: + self.user = user + self.next_url = next_url super().__init__(label, **kwargs) @@ -181,7 +207,19 @@ class ButtonWithDropdownFromHook(BaseDropdownMenuButton): buttons = [] for hook in button_hooks: - buttons.extend(hook(self.page, self.page_perms, self.next_url)) + if accepts_kwarg(hook, "user"): + buttons.extend( + hook(page=self.page, user=self.user, next_url=self.next_url) + ) + else: + # old-style hook that accepts page_perms instead of user + warn( + f"`{self.hook_name}` hook functions should accept a `user` argument instead of `page_perms` -" + f" {hook.__module__}.{hook.__name__} needs to be updated", + category=RemovedInWagtail60Warning, + ) + page_perms = self.page.permissions_for_user(self.user) + buttons.extend(hook(self.page, page_perms, self.next_url)) buttons = [b for b in buttons if b.show] return buttons diff --git a/wagtail/contrib/simple_translation/tests/test_wagtail_hooks.py b/wagtail/contrib/simple_translation/tests/test_wagtail_hooks.py index 7af81b1596..ebd3484bea 100644 --- a/wagtail/contrib/simple_translation/tests/test_wagtail_hooks.py +++ b/wagtail/contrib/simple_translation/tests/test_wagtail_hooks.py @@ -66,10 +66,6 @@ class TestWagtailHooksPermission(Utils): class TestWagtailHooksButtons(Utils): - class PagePerms: - def __init__(self, user): - self.user = user - def test_page_listing_more_buttons(self): # Root, no button root_page = self.en_blog_index.get_root() @@ -78,11 +74,11 @@ class TestWagtailHooksButtons(Utils): user = get_user_model().objects.create_user(email="jos@example.com") else: user = get_user_model().objects.create_user(username="jos") - assert list(page_listing_more_buttons(root_page, self.PagePerms(user))) == [] + assert list(page_listing_more_buttons(root_page, user)) == [] # No permissions, no button home_page = self.en_homepage - assert list(page_listing_more_buttons(root_page, self.PagePerms(user))) == [] + assert list(page_listing_more_buttons(root_page, user)) == [] # Homepage is translated to all languages, no button perm = Permission.objects.get(codename="submit_translation") @@ -96,13 +92,12 @@ class TestWagtailHooksButtons(Utils): user.user_permissions.add(perm) group = Group.objects.get(name="Editors") user.groups.add(group) - page_perms = self.PagePerms(user) - assert list(page_listing_more_buttons(home_page, page_perms)) == [] + assert list(page_listing_more_buttons(home_page, user)) == [] # Page does not have translations yet... button! blog_page = self.en_blog_post assert isinstance( - list(page_listing_more_buttons(blog_page, page_perms))[0], + list(page_listing_more_buttons(blog_page, user))[0], wagtailadmin_widgets.Button, ) diff --git a/wagtail/contrib/simple_translation/wagtail_hooks.py b/wagtail/contrib/simple_translation/wagtail_hooks.py index b7977f3879..bc52f6729d 100644 --- a/wagtail/contrib/simple_translation/wagtail_hooks.py +++ b/wagtail/contrib/simple_translation/wagtail_hooks.py @@ -48,11 +48,8 @@ def register_submit_translation_permission(): @hooks.register("register_page_listing_more_buttons") -def page_listing_more_buttons(page, page_perms, next_url=None): - if ( - page_perms.user.has_perm("simple_translation.submit_translation") - and not page.is_root() - ): +def page_listing_more_buttons(page, user, next_url=None): + if user.has_perm("simple_translation.submit_translation") and not page.is_root(): # If there's at least one locale that we haven't translated into yet, # show "Translate this page" button has_locale_to_translate_to = Locale.objects.exclude(