fix issue with edit page header delete button showing an invalid next_url

- fixes 
- header button on edit page & page listing - unpublish now correctly includes the next url (was missing on page listing previously)
- header button on edit page - delete button does not include next url (as this would be the edit page for what was deleted)
- adds more robust unit tests for the page listing & page header more hooks, including separating the tests out to separate classes
pull/9235/head
LB Johnston 2022-09-14 16:29:37 +10:00 zatwierdzone przez Matt Westcott
rodzic f8bdcda5de
commit bf65fa94ea
4 zmienionych plików z 136 dodań i 30 usunięć

Wyświetl plik

@ -52,6 +52,7 @@ Changelog
* Fix: Use the correct type scale for heading levels in rich text (Steven Steinwand)
* Fix: Update alignment and reveal logic of fields comment buttons (Steven Steinwand)
* Fix: Regression from Markdown conversion in documentation for API configuration - update to correctly use PEP-8 for example code (Storm Heg)
* Fix: Prevent 'Delete' link on page edit view from redirecting back to the deleted page (LB (Ben) Johnston)
4.0.1 (05.09.2022)

Wyświetl plik

@ -27,3 +27,4 @@ depth: 1
* Use the correct type scale for heading levels in rich text (Steven Steinwand)
* Update alignment and reveal logic of fields comment buttons (Steven Steinwand)
* Regression from Markdown conversion in documentation for API configuration - update to correctly use PEP-8 for example code (Storm Heg)
* Prevent 'Delete' link on page edit view from redirecting back to the deleted page (LB (Ben) Johnston)

Wyświetl plik

@ -4,7 +4,7 @@ from django.utils.http import urlencode
from wagtail import hooks
from wagtail.admin import widgets as wagtailadmin_widgets
from wagtail.admin.wagtail_hooks import page_listing_more_buttons
from wagtail.admin.wagtail_hooks import page_header_buttons, page_listing_more_buttons
from wagtail.models import Page
from wagtail.test.testapp.models import SimplePage
from wagtail.test.utils import WagtailTestUtils
@ -17,6 +17,9 @@ class BasePagePerms:
def can_copy(self):
return False
def can_edit(self):
return False
def can_delete(self):
return False
@ -29,12 +32,23 @@ class BasePagePerms:
def can_reorder_children(self):
return False
def can_add_subpage(self):
return False
class DeleteOnlyPagePerms(BasePagePerms):
def can_delete(self):
return True
class DeleteAndUnpublishPagePerms(BasePagePerms):
def can_delete(self):
return True
def can_unpublish(self):
return True
class ReorderOnlyPagePerms(BasePagePerms):
def can_reorder_children(self):
return True
@ -53,6 +67,8 @@ class TestButtonsHooks(TestCase, WagtailTestUtils):
)
)
class TestPageListingButtonsHooks(TestButtonsHooks):
def test_register_page_listing_buttons(self):
def page_listing_buttons(page, page_perms, next_url=None):
yield wagtailadmin_widgets.PageListingButton(
@ -74,6 +90,8 @@ class TestButtonsHooks(TestCase, WagtailTestUtils):
self.assertContains(response, "Another useless page listing button")
class TestPageListingMoreButtonsHooks(TestButtonsHooks):
def test_register_page_listing_more_buttons(self):
def page_listing_more_buttons(page, page_perms, next_url=None):
yield wagtailadmin_widgets.Button(
@ -140,27 +158,12 @@ class TestButtonsHooks(TestCase, WagtailTestUtils):
"Another useless dropdown button in "One more more button" dropdown",
)
def test_register_page_header_buttons(self):
def page_header_buttons(page, page_perms, next_url=None):
yield wagtailadmin_widgets.Button(
"Another useless header button", "/custom-url", priority=10
)
def test_delete_button_with_next_url(self):
"""
Ensure that the built in delete button supports a next_url provided.
"""
with hooks.register_temporarily(
"register_page_header_buttons", page_header_buttons
):
response = self.client.get(
reverse("wagtailadmin_pages:edit", args=(self.root_page.id,))
)
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(
response, "wagtailadmin/pages/listing/_modern_dropdown.html"
)
self.assertContains(response, "Another useless header button")
def test_delete_button_next_url(self):
# page_listing_more_button generator yields only `Delete button` with this permission set
page_perms = DeleteOnlyPagePerms()
page = self.root_page
base_url = reverse("wagtailadmin_pages:delete", args=[page.id])
@ -168,20 +171,39 @@ class TestButtonsHooks(TestCase, WagtailTestUtils):
next_url = "a/random/url/"
full_url = base_url + "?" + urlencode({"next": next_url})
# page_listing_more_button generator yields only `Delete button`
delete_button = next(
page_listing_more_buttons(page, page_perms, next_url=next_url)
)
self.assertEqual(delete_button.url, full_url)
next_url = reverse("wagtailadmin_explore", args=[page.id])
delete_button = next(
page_listing_more_buttons(page, page_perms, next_url=next_url)
)
def test_delete_button_with_invalid_next_url(self):
"""
Ensure that the built in delete button on page listing will not use
the next_url provided if that URL is directing the user to edit the page.
As the page is now deleted and cannot be edited.
"""
# permissions should yield two buttons, delete and unpublish
page_perms = DeleteAndUnpublishPagePerms()
page = self.root_page
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)
delete_button = next(buttons)
# check that the next_url is NOT included as it will not be available after deletion
self.assertEqual(delete_button.url, base_url)
# check that any buttons after do correctly still include the next_url
unpublish_base_url = reverse("wagtailadmin_pages:unpublish", args=[page.id])
unpublish_button = next(buttons)
full_url = unpublish_base_url + "?" + urlencode({"next": next_url})
self.assertEqual(unpublish_button.url, full_url)
def test_reorder_button_visibility(self):
page = self.root_page
page_perms = BasePagePerms()
@ -195,3 +217,79 @@ class TestButtonsHooks(TestCase, WagtailTestUtils):
reorder_button = next(page_listing_more_buttons(page, page_perms))
self.assertEqual(reorder_button.url, "?ordering=ord")
class TestPageHeaderButtonsHooks(TestButtonsHooks):
def test_register_page_header_buttons(self):
def custom_page_header_buttons(page, page_perms, next_url=None):
yield wagtailadmin_widgets.Button(
"Another useless header button", "/custom-url", priority=10
)
with hooks.register_temporarily(
"register_page_header_buttons", custom_page_header_buttons
):
response = self.client.get(
reverse("wagtailadmin_pages:edit", args=(self.root_page.id,))
)
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(
response, "wagtailadmin/pages/listing/_modern_dropdown.html"
)
self.assertContains(response, "Another useless header button")
def test_delete_button_with_next_url(self):
"""
Ensure that the built in delete button supports a next_url provided.
"""
# page_listing_more_button generator yields only `Delete button` with this permission set
page_perms = DeleteOnlyPagePerms()
page = self.root_page
base_url = reverse("wagtailadmin_pages:delete", args=[page.id])
next_url = "a/random/url/"
full_url = base_url + "?" + urlencode({"next": next_url})
delete_button = next(page_header_buttons(page, page_perms, next_url=next_url))
self.assertEqual(delete_button.url, full_url)
def test_delete_button_with_invalid_next_url(self):
"""
Ensure that the built in delete button on page edit/home (header button) will not use
the next_url provided if that URL is directing the user to edit the page.
As the page is now deleted and cannot be edited.
"""
# permissions should yield two buttons, delete and unpublish
page_perms = DeleteAndUnpublishPagePerms()
page = self.root_page
base_url = reverse("wagtailadmin_pages:delete", args=[page.id])
next_url = reverse("wagtailadmin_explore", args=[page.id])
buttons = page_header_buttons(page, page_perms, next_url=next_url)
delete_button = next(buttons)
# check that the next_url is NOT included as it will not be available after deletion (page listing)
self.assertEqual(delete_button.url, base_url)
base_url = reverse("wagtailadmin_pages:delete", args=[page.id])
next_url = reverse("wagtailadmin_pages:edit", args=[page.id])
buttons = page_header_buttons(page, page_perms, next_url=next_url)
delete_button = next(buttons)
# check that the next_url is NOT included as it will not be available after deletion (edit page)
self.assertEqual(delete_button.url, base_url)
# check that any buttons after do correctly still include the next_url
unpublish_base_url = reverse("wagtailadmin_pages:unpublish", args=[page.id])
unpublish_button = next(buttons)
full_url = unpublish_base_url + "?" + urlencode({"next": next_url})
self.assertEqual(unpublish_button.url, full_url)

Wyświetl plik

@ -295,12 +295,13 @@ def page_listing_more_buttons(page, page_perms, next_url=None):
)
if page_perms.can_delete():
url = reverse("wagtailadmin_pages:delete", args=[page.id])
include_next_url = True
# After deleting the page, it is impossible to redirect to it.
if next_url == reverse("wagtailadmin_explore", args=[page.id]):
next_url = None
include_next_url = False
if next_url:
if next_url and include_next_url:
url += "?" + urlencode({"next": next_url})
yield Button(
@ -402,11 +403,16 @@ def page_header_buttons(page, page_perms, next_url=None):
if page_perms.can_delete():
url = reverse("wagtailadmin_pages:delete", args=[page.id])
include_next_url = True
# After deleting the page, it is impossible to redirect to it.
if next_url == reverse("wagtailadmin_explore", args=[page.id]):
next_url = None
include_next_url = False
if next_url:
if next_url == reverse("wagtailadmin_pages:edit", args=[page.id]):
include_next_url = False
if next_url and include_next_url:
url += "?" + urlencode({"next": next_url})
yield Button(