diff --git a/client/src/entrypoints/admin/bulk-actions.js b/client/src/entrypoints/admin/bulk-actions.js index e60cf08efa..4f53ae9d9a 100644 --- a/client/src/entrypoints/admin/bulk-actions.js +++ b/client/src/entrypoints/admin/bulk-actions.js @@ -1,5 +1,4 @@ /* global wagtailConfig */ -/* global $ */ const BULK_ACTION_PAGE_CHECKBOX_INPUT = 'bulk-action-checkbox'; const BULK_ACTION_SELECT_ALL_CHECKBOX_TH = 'bulk-actions-filter-checkbox'; @@ -12,7 +11,6 @@ const TABLE_HEADERS_TR = 'table-headers'; const checkedState = { checkedPages: new Set(), numPages: 0, - listingPageIds: [], selectAllInListing: false }; @@ -39,7 +37,7 @@ function SelectBulkActionsCheckboxes(e) { } if (checkedState.checkedPages.size === 0) { - /* when all checboxes are unchecked */ + /* when all checkboxes are unchecked */ document.querySelectorAll(`.${TABLE_HEADERS_TR} > th`).forEach(el => el.classList.remove('u-hidden')); document.querySelector(`.${BULK_ACTION_CHOICES_DIV}`).classList.add('u-hidden'); document.querySelectorAll(`.${BULK_ACTION_PAGE_CHECKBOX_INPUT}`).forEach(el => el.classList.remove('show')); @@ -47,7 +45,6 @@ function SelectBulkActionsCheckboxes(e) { } else if (checkedState.checkedPages.size === 1 && prevLength === 0) { /* when 1 checkbox is checked for the first time */ document.querySelectorAll(`.${BULK_ACTION_PAGE_CHECKBOX_INPUT}`).forEach(el => { - el.classList.remove('show'); el.classList.add('show'); }); document.querySelectorAll(`.${TABLE_HEADERS_TR} > th`).forEach(el => el.classList.add('u-hidden')); @@ -85,16 +82,7 @@ function SelectBulkActionsCheckboxes(e) { } } -function fetchAllPageIdsInListing(e) { - if (checkedState.listingPageIds.length === 0) { - e.preventDefault(); - $.ajax({ - url: '?fetchPageIds=1', - success: (response) => { - checkedState.listingPageIds = response.page_ids; - }, - }); - } +function selectAllPageIdsInListing() { checkedState.selectAllInListing = true; document.querySelector(`.${BULK_ACTION_NUM_PAGES_SPAN}`). textContent = wagtailConfig.STRINGS.NUM_PAGES_SELECTED_ALL_IN_LISTING; @@ -138,9 +126,14 @@ function BulkActionEventListeners(e) { e.preventDefault(); const url = e.target.getAttribute('href'); let queryString = ''; - (checkedState.selectAllInListing ? checkedState.listingPageIds : checkedState.checkedPages).forEach(pageId => { - queryString += `&id=${pageId}`; - }); + if (checkedState.selectAllInListing) { + const parentPageId = document.querySelector(`.${BULK_ACTION_SELECT_ALL_CHECKBOX_TH}`).dataset.parentId; + queryString += `&id=all&childOf=${parentPageId}`; + } else { + checkedState.checkedPages.forEach(pageId => { + queryString += `&id=${pageId}`; + }); + } window.location.href = url + queryString; } @@ -159,7 +152,7 @@ function addBulkActionListeners() { document.querySelectorAll(`.${BULK_ACTION_CHOICES_DIV} > ul > li > a`).forEach( elem => elem.addEventListener('click', BulkActionEventListeners) ); - document.querySelector(`.${BULK_ACTION_NUM_PAGES_IN_LISTING}`).addEventListener('click', fetchAllPageIdsInListing); + document.querySelector(`.${BULK_ACTION_NUM_PAGES_IN_LISTING}`).addEventListener('click', selectAllPageIdsInListing); } addBulkActionListeners(); diff --git a/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_delete.html b/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_delete.html index 0285be7928..e50e14f730 100644 --- a/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_delete.html +++ b/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_delete.html @@ -12,26 +12,31 @@ {% endif %} {% if pages_with_no_access %} -

{% blocktrans count counter=pages_with_no_access|length %}The following page cannot be deleted{% plural %}The following pages cannot be deleted{% endblocktrans %}

+

{% blocktrans count counter=pages_with_no_access|length %}You don't have permission to delete this page{% plural %} + You don't have permission to delete these pages{% endblocktrans %}

diff --git a/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_move.html b/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_move.html index be0bced520..769928eb01 100644 --- a/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_move.html +++ b/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_move.html @@ -27,11 +27,15 @@ {% endif %} {% if pages_with_no_access %} -

{% blocktrans count counter=pages_with_no_access|length %}The following page cannot be moved{% plural %}The following pages cannot be moved{% endblocktrans %}

+

{% blocktrans count counter=pages_with_no_access|length %}You don't have permission to move this page{% plural %}You don't have permission to move these pages{% endblocktrans %}

@@ -65,16 +69,7 @@ diff --git a/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_publish.html b/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_publish.html index cf0ea0cba9..f7a06318ae 100644 --- a/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_publish.html +++ b/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_publish.html @@ -26,11 +26,15 @@ {% endif %} {% if pages_with_no_access %} -

{% blocktrans count counter=pages_with_no_access|length %}The following page cannot be published{% plural %}The following pages cannot be published{% endblocktrans %}

+

{% blocktrans count counter=pages_with_no_access|length %}You don't have permission to publish this page{% plural %}You don't have permission to publish these pages{% endblocktrans %}

@@ -40,16 +44,7 @@ {% csrf_token %} {% if has_draft_descendants %} {% endif %} diff --git a/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_unpublish.html b/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_unpublish.html index 75a6b06c7e..aef963fa08 100644 --- a/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_unpublish.html +++ b/wagtail/admin/templates/wagtailadmin/pages/bulk_actions/confirm_bulk_unpublish.html @@ -26,11 +26,15 @@ {% endif %} {% if pages_with_no_access %} -

{% blocktrans count counter=pages_with_no_access|length %}The following page cannot be unpublished{% plural %}The following pages cannot be unpublished{% endblocktrans %}

+

{% blocktrans count counter=pages_with_no_access|length %}You don't have permission to unpublish this page{% plural %}You don't have permission to unpublish these pages{% endblocktrans %}

@@ -40,16 +44,7 @@ {% csrf_token %} {% if has_live_descendants %} {% endif %} diff --git a/wagtail/admin/templates/wagtailadmin/pages/search.html b/wagtail/admin/templates/wagtailadmin/pages/search.html index 1b2bd061aa..4623c96175 100644 --- a/wagtail/admin/templates/wagtailadmin/pages/search.html +++ b/wagtail/admin/templates/wagtailadmin/pages/search.html @@ -1,5 +1,5 @@ {% extends "wagtailadmin/base.html" %} -{% load i18n %} +{% load wagtailadmin_tags i18n %} {% block titletag %}{% trans 'Search' %}{% endblock %} {% block extra_js %} {{ block.super }} @@ -11,6 +11,7 @@ targetOutput: "#page-results" } + {% endblock %} {% block content %} diff --git a/wagtail/admin/templatetags/wagtailadmin_tags.py b/wagtail/admin/templatetags/wagtailadmin_tags.py index 7830f765f8..bf2138f956 100644 --- a/wagtail/admin/templatetags/wagtailadmin_tags.py +++ b/wagtail/admin/templatetags/wagtailadmin_tags.py @@ -497,7 +497,7 @@ def bulk_action_filters(context): for hook in button_hooks: buttons.extend(hook()) - buttons.sort() + buttons.sort(key=lambda x: x.priority) return {'buttons': buttons} diff --git a/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_delete.py b/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_delete.py index 610096709b..b666c4a43f 100644 --- a/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_delete.py +++ b/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_delete.py @@ -103,17 +103,15 @@ class TestBulkDelete(TestCase, WagtailTestUtils): html = response.content.decode() - self.assertInHTML('

The following pages cannot be deleted

', html) + self.assertInHTML("

You don't have permission to delete these pages

", html) needle = '' self.assertInHTML(needle, html) + self.assertTagInHTML('''
'''.format(self.url), html, count=0) def test_bulk_delete_post(self): # Connect a mock signal handler to page_unpublished signal diff --git a/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_move.py b/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_move.py index de1494f7a5..6238255b18 100644 --- a/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_move.py +++ b/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_move.py @@ -96,18 +96,17 @@ class TestBulkMove(TestCase, WagtailTestUtils): html = response.content.decode() - self.assertInHTML('

The following pages cannot be moved

', html) + self.assertInHTML("

You don't have permission to move these pages

", html) needle = '' self.assertInHTML(needle, html) + self.assertTagInHTML('''
'''.format(self.url), html, count=0) + def test_user_without_bulk_delete_permission_can_move(self): # to verify that a user without bulk delete permission is able to move a page with a child page @@ -144,7 +143,7 @@ class TestBulkMove(TestCase, WagtailTestUtils): needle += '' self.assertInHTML(needle, html) - self.assertContains(response, '') + self.assertContains(response, '') def test_bulk_move_slug_already_taken(self): temp_page_1 = SimplePage(title="Hello world!", slug="hello-world-b", content="hello") @@ -167,7 +166,7 @@ class TestBulkMove(TestCase, WagtailTestUtils): needle += '' self.assertInHTML(needle, html) - self.assertContains(response, '') + self.assertContains(response, '') def test_bulk_move_ignore_permission_errors(self): temp_page_1 = SimplePage(title="Hello world!", slug="hello-world-b", content="hello") diff --git a/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_publish.py b/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_publish.py index 3c62149532..355bfe457a 100644 --- a/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_publish.py +++ b/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_publish.py @@ -74,14 +74,11 @@ class TestBulkPublish(TestCase, WagtailTestUtils): html = response.content.decode() - self.assertInHTML('

The following pages cannot be published

', html) + self.assertInHTML("

You don't have permission to publish these pages

", html) needle = '' self.assertInHTML(needle, html) @@ -213,7 +210,7 @@ class TestBulkPublishIncludingDescendants(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailadmin/pages/bulk_actions/confirm_bulk_publish.html') # Check the form contains the checkbox field include_descendants - self.assertContains(response, '') + self.assertContains(response, '') def test_publish_include_children_view_post(self): """ diff --git a/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_unpublish.py b/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_unpublish.py index da3d8fed82..3efe2f5ffd 100644 --- a/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_unpublish.py +++ b/wagtail/admin/tests/pages/test_bulk_actions/test_bulk_unpublish.py @@ -75,14 +75,11 @@ class TestBulkUnpublish(TestCase, WagtailTestUtils): html = response.content.decode() - self.assertInHTML('

The following pages cannot be unpublished

', html) + self.assertInHTML("

You don't have permission to unpublish these pages

", html) needle = '' self.assertInHTML(needle, html) @@ -167,7 +164,7 @@ class TestBulkUnpublish(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailadmin/pages/bulk_actions/confirm_bulk_unpublish.html') # Check the form does not contain the checkbox field include_descendants - self.assertNotContains(response, '') + self.assertContains(response, '', count=0) class TestBulkUnpublishIncludingDescendants(TestCase, WagtailTestUtils): @@ -216,7 +213,7 @@ class TestBulkUnpublishIncludingDescendants(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailadmin/pages/bulk_actions/confirm_bulk_unpublish.html') # Check the form contains the checkbox field include_descendants - self.assertContains(response, '') + self.assertContains(response, '') def test_unpublish_include_children_view_post(self): """ diff --git a/wagtail/admin/views/bulk_action.py b/wagtail/admin/views/bulk_action.py index a79d44c0b7..5dd2cf8127 100644 --- a/wagtail/admin/views/bulk_action.py +++ b/wagtail/admin/views/bulk_action.py @@ -1,4 +1,4 @@ -from abc import ABC, abstractproperty +from abc import ABC, abstractmethod from django.db import transaction from django.shortcuts import get_list_or_404, redirect @@ -10,15 +10,18 @@ from wagtail.core import hooks class BulkAction(ABC, TemplateView): - @abstractproperty + @property + @abstractmethod def display_name(self): pass - @abstractproperty + @property + @abstractmethod def action_type(self): pass - @abstractproperty + @property + @abstractmethod def aria_label(self): pass @@ -78,7 +81,11 @@ class BulkAction(ABC, TemplateView): def get_actionable_objects(self): objects = [] objects_with_no_access = [] - object_ids = list(map(int, self.request.GET.getlist('id'))) + object_ids = self.request.GET.getlist('id') + if 'all' in object_ids: + parent_page_id = int(self.request.GET.get('childOf')) + object_ids = self.model.objects.get(id=parent_page_id).get_children().values_list('id', flat=True) + object_ids = list(map(int, object_ids)) for obj in self.get_queryset(object_ids): if not self.check_perm(obj): @@ -99,8 +106,14 @@ class BulkAction(ABC, TemplateView): 'submit_url': self.request.path + '?' + self.request.META['QUERY_STRING'] } + def prepare_action(self, objects): + return + def post(self, request): objects, _ = self.get_actionable_objects() + resp = self.prepare_action(objects) + if hasattr(resp, 'status_code'): + return resp with transaction.atomic(): before_hook_result = self.__run_before_hooks(self.action_type, request, objects) if before_hook_result is not None: diff --git a/wagtail/admin/views/pages/bulk_actions/move.py b/wagtail/admin/views/pages/bulk_actions/move.py index 3ba94e06a8..113c8baf49 100644 --- a/wagtail/admin/views/pages/bulk_actions/move.py +++ b/wagtail/admin/views/pages/bulk_actions/move.py @@ -20,30 +20,10 @@ class MoveForm(forms.Form): widget=widgets.AdminPageChooser(can_choose_root=True, user_perms='move_to'), label=_("Select a new parent page"), ) - - -def before_bulk_move(request, action_type, pages, action_instance): - if action_type != 'move': - return - move_applicable = request.POST.dict().get("move_applicable", False) - if move_applicable: - return - destination_page_id = request.POST.dict()['chooser'] - destination = get_object_or_404(Page, id=destination_page_id) - pages_without_destination_access = [] - pages_with_duplicate_slugs = [] - for page in pages: - if not page.permissions_for_user(request.user).can_move_to(destination): - pages_without_destination_access.append(page) - if not Page._slug_is_available(page.slug, destination, page=page): - pages_with_duplicate_slugs.append(page) - if pages_without_destination_access or pages_with_duplicate_slugs: - return TemplateResponse(request, action_instance.template_name, { - 'pages_without_destination_access': pages_without_destination_access, - "pages_with_duplicate_slugs": pages_with_duplicate_slugs, - 'destination': destination, - **action_instance.get_context_data(destination=destination) - }) + self.fields['move_applicable'] = forms.BooleanField( + label=_("Move only applicable pages"), + required=False + ) class MoveBulkAction(PageBulkAction): @@ -77,6 +57,28 @@ class MoveBulkAction(PageBulkAction): context['form'] = MoveForm(destination=destination) return context + def prepare_action(self, pages): + request = self.request + move_applicable = request.POST.dict().get("move_applicable", False) + if move_applicable: + return + destination_page_id = request.POST.dict()['chooser'] + destination = get_object_or_404(Page, id=destination_page_id) + pages_without_destination_access = [] + pages_with_duplicate_slugs = [] + for page in pages: + if not page.permissions_for_user(request.user).can_move_to(destination): + pages_without_destination_access.append(page) + if not Page._slug_is_available(page.slug, destination, page=page): + pages_with_duplicate_slugs.append(page) + if pages_without_destination_access or pages_with_duplicate_slugs: + return TemplateResponse(request, self.template_name, { + 'pages_without_destination_access': pages_without_destination_access, + "pages_with_duplicate_slugs": pages_with_duplicate_slugs, + 'destination': destination, + **self.get_context_data(destination=destination) + }) + def execute_action(cls, pages): destination_page_id = cls.request.POST.dict().pop("chooser") destination = get_object_or_404(Page, id=destination_page_id) @@ -89,8 +91,6 @@ class MoveBulkAction(PageBulkAction): page.move(destination, pos='last-child', user=cls.request.user) cls.num_parent_objects += 1 - # register temporary hook to check for move related permissions - @hooks.register_temporarily('before_bulk_action', before_bulk_move) def post(self, request): return super().post(request) diff --git a/wagtail/admin/views/pages/bulk_actions/page_bulk_action.py b/wagtail/admin/views/pages/bulk_actions/page_bulk_action.py index 677cefaa46..705cf09be1 100644 --- a/wagtail/admin/views/pages/bulk_actions/page_bulk_action.py +++ b/wagtail/admin/views/pages/bulk_actions/page_bulk_action.py @@ -1,7 +1,25 @@ +from django import forms + from wagtail.admin.views.bulk_action import BulkAction from wagtail.core.models import Page +class DefaultForm(forms.Form): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields['include_descendants'] = forms.BooleanField( + required=False + ) + + class PageBulkAction(BulkAction): model = Page object_key = 'page' + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context['form'] = DefaultForm() + context['pages_with_no_access'] = [ + {'page': page, 'can_edit': page.permissions_for_user(self.request.user).can_edit()} for page in context['pages_with_no_access'] + ] + return context diff --git a/wagtail/admin/views/pages/listing.py b/wagtail/admin/views/pages/listing.py index 965d8106c3..4f0cfdc8e8 100644 --- a/wagtail/admin/views/pages/listing.py +++ b/wagtail/admin/views/pages/listing.py @@ -1,7 +1,6 @@ from django.conf import settings from django.core.paginator import Paginator from django.db.models import Count -from django.http.response import JsonResponse from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse from django.urls import reverse @@ -40,12 +39,6 @@ def index(request, parent_page_id=None): & user_perms.explorable_pages() ) - fetch_page_ids = request.GET.get('fetchPageIds', '0') - if fetch_page_ids == '1': - return JsonResponse({ - 'page_ids': list(pages.values_list('id', flat=True)) - }) - # Get page ordering ordering = request.GET.get('ordering', '-latest_revision_created_at') if ordering not in [