Bulk actions - Minor fixes (#7440)

* [refactor] Use django's bulk updates and deletes instead of updating objects one by one
* [refactor] Declare form fields as class variables rather than in __init__ method
* [refactor] Refactor the pages bulk actions to work if called from a management command
* [refactor] Specify implicitly the variables required in execute_action
* [refactor] Return early if no tags are supplied
* [refactor] Add **kwargs to all overriden execute_action method for additional keyword arguments
* [refactor] Remove unnecessary use of class variables
* [feat] execute_action now returns the count of updated objects, to be used in success message

wagtail/admin/views/bulk_action.py

@@ -26,8 +26,6 @@ class BulkAction(ABC, FormView):
     def aria_label(self):
         pass
 
-    num_child_objects = 0
-    num_parent_objects = 0
     extras = dict()
     action_priority = 100
     model = None
@@ -43,7 +41,6 @@ class BulkAction(ABC, FormView):
         if not next_url:
             next_url = request.path
         self.next_url = next_url
-        self.num_parent_objects = self.num_child_objects = 0
 
     @classmethod
     def get_queryset(cls, object_ids):
@@ -58,7 +55,7 @@ class BulkAction(ABC, FormView):
     def execute_action(cls, objects, **kwargs):
         raise NotImplementedError("execute_action needs to be implemented")
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         pass
 
     def object_context(self, obj):
@@ -123,11 +120,11 @@ class BulkAction(ABC, FormView):
             before_hook_result = self.__run_before_hooks(self.action_type, request, objects)
             if before_hook_result is not None:
                 return before_hook_result
-            self.execute_action(objects, **self.get_execution_context())
+            num_parent_objects, num_child_objects = self.execute_action(objects, **self.get_execution_context())
             after_hook_result = self.__run_after_hooks(self.action_type, request, objects)
             if after_hook_result is not None:
                 return after_hook_result
-            success_message = self.get_success_message()
+            success_message = self.get_success_message(num_parent_objects, num_child_objects)
             if success_message is not None:
                 messages.success(request, success_message)
         return redirect(self.next_url)

wagtail/admin/views/pages/bulk_actions/delete.py

@@ -23,38 +23,37 @@ class DeleteBulkAction(PageBulkAction):
         }
 
     @classmethod
-    def execute_action(cls, objects, **kwargs):
-        user = kwargs.get('user', None)
-        if user is None:
-            return
+    def execute_action(cls, objects, user=None, **kwargs):
+        num_parent_objects, num_child_objects = 0, 0
         for page in objects:
-            cls.num_parent_objects += 1
-            cls.num_child_objects += page.get_descendant_count()
+            num_parent_objects += 1
+            num_child_objects += page.get_descendant_count()
             page.delete(user=user)
+        return num_parent_objects, num_child_objects
 
-    def get_success_message(self):
-        if self.num_parent_objects == 1:
-            if self.num_child_objects == 0:
+    def get_success_message(self, num_parent_objects, num_child_objects):
+        if num_parent_objects == 1:
+            if num_child_objects == 0:
                 success_message = _("1 page has been deleted")
             else:
                 success_message = ngettext(
                     "1 page and %(num_child_objects)d child page have been deleted",
                     "1 page and %(num_child_objects)d child pages have been deleted",
-                    self.num_child_objects
+                    num_child_objects
                 ) % {
-                    'num_child_objects': self.num_child_objects
+                    'num_child_objects': num_child_objects
                 }
         else:
-            if self.num_child_objects == 0:
-                success_message = _("%(num_parent_objects)d pages have been deleted") % {'num_parent_objects': self.num_parent_objects}
+            if num_child_objects == 0:
+                success_message = _("%(num_parent_objects)d pages have been deleted") % {'num_parent_objects': num_parent_objects}
             else:
                 success_message = ngettext(
                     "%(num_parent_objects)d pages and %(num_child_objects)d child page have been deleted",
                     "%(num_parent_objects)d pages and %(num_child_objects)d child pages have been deleted",
-                    self.num_child_objects
+                    num_child_objects
                 ) % {
-                    'num_child_objects': self.num_child_objects,
-                    'num_parent_objects': self.num_parent_objects
+                    'num_child_objects': num_child_objects,
+                    'num_parent_objects': num_parent_objects
                 }
         return success_message
 

wagtail/admin/views/pages/bulk_actions/move.py

@@ -10,6 +10,11 @@ from wagtail.core.models import Page
 
 
 class MoveForm(forms.Form):
+    move_applicable = forms.BooleanField(
+        label=_("Move only applicable pages"),
+        required=False
+    )
+
     def __init__(self, *args, **kwargs):
         destination = kwargs.pop('destination')
         super().__init__(*args, **kwargs)
@@ -19,10 +24,6 @@ class MoveForm(forms.Form):
             widget=widgets.AdminPageChooser(can_choose_root=True, user_perms='move_to'),
             label=_("Select a new parent page"),
         )
-        self.fields['move_applicable'] = forms.BooleanField(
-            label=_("Move only applicable pages"),
-            required=False
-        )
 
 
 class MoveBulkAction(PageBulkAction):
@@ -42,13 +43,13 @@ class MoveBulkAction(PageBulkAction):
     def check_perm(self, page):
         return page.permissions_for_user(self.request.user).can_move()
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         success_message = ngettext(
             "%(num_pages)d page has been moved",
             "%(num_pages)d pages have been moved",
-            self.num_parent_objects
+            num_parent_objects
         ) % {
-            'num_pages': self.num_parent_objects
+            'num_pages': num_parent_objects
         }
         return success_message
 
@@ -93,20 +94,18 @@ class MoveBulkAction(PageBulkAction):
         }
 
     @classmethod
-    def execute_action(cls, objects, **kwargs):
-        destination = kwargs.get('destination', None)
+    def execute_action(cls, objects, destination=None, user=None, **kwargs):
+        num_parent_objects = 0
         if destination is None:
             return
-        user = kwargs.get('user', None)
-        if user is None:
-            return
         for page in objects:
             if not page.permissions_for_user(user).can_move_to(destination):
                 continue
             if not Page._slug_is_available(page.slug, destination, page=page):
                 continue
             page.move(destination, pos='last-child', user=user)
-            cls.num_parent_objects += 1
+            num_parent_objects += 1
+        return num_parent_objects, 0
 
 
 @hooks.register('register_page_bulk_action')

wagtail/admin/views/pages/bulk_actions/page_bulk_action.py

@@ -5,11 +5,7 @@ from wagtail.core.models import Page
 
 
 class DefaultPageForm(forms.Form):
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.fields['include_descendants'] = forms.BooleanField(
-            required=False
-        )
+    include_descendants = forms.BooleanField(required=False)
 
 
 class PageBulkAction(BulkAction):

wagtail/admin/views/pages/bulk_actions/publish.py

@@ -32,54 +32,52 @@ class PublishBulkAction(PageBulkAction):
         }
 
     @classmethod
-    def execute_action(cls, objects, **kwargs):
-        include_descendants = kwargs.get('include_descendants', False)
-        user = kwargs.get('user', None)
-        if user is None:
-            return
+    def execute_action(cls, objects, include_descendants=False, user=None, **kwargs):
+        num_parent_objects, num_child_objects = 0, 0
         for page in objects:
             revision = page.save_revision(user=user)
             revision.publish(user=user)
-            cls.num_parent_objects += 1
+            num_parent_objects += 1
 
             if include_descendants:
                 for draft_descendant_page in page.get_descendants().not_live().defer_streamfields().specific():
-                    if draft_descendant_page.permissions_for_user(user).can_publish():
+                    if user is None or draft_descendant_page.permissions_for_user(user).can_publish():
                         revision = draft_descendant_page.save_revision(user=user)
                         revision.publish(user=user)
-                        cls.num_child_objects += 1
+                        num_child_objects += 1
+        return num_parent_objects, num_child_objects
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         include_descendants = self.cleaned_form.cleaned_data['include_descendants']
-        if self.num_parent_objects == 1:
+        if num_parent_objects == 1:
             if include_descendants:
-                if self.num_child_objects == 0:
+                if num_child_objects == 0:
                     success_message = _("1 page has been published")
                 else:
                     success_message = ngettext(
                         "1 page and %(num_child_objects)d child page have been published",
                         "1 page and %(num_child_objects)d child pages have been published",
-                        self.num_child_objects
+                        num_child_objects
                     ) % {
-                        'num_child_objects': self.num_child_objects
+                        'num_child_objects': num_child_objects
                     }
             else:
                 success_message = _("1 page has been published")
         else:
             if include_descendants:
-                if self.num_child_objects == 0:
-                    success_message = _("%(num_parent_objects)d pages have been published") % {'num_parent_objects': self.num_parent_objects}
+                if num_child_objects == 0:
+                    success_message = _("%(num_parent_objects)d pages have been published") % {'num_parent_objects': num_parent_objects}
                 else:
                     success_message = ngettext(
                         "%(num_parent_objects)d pages and %(num_child_objects)d child page have been published",
                         "%(num_parent_objects)d pages and %(num_child_objects)d child pages have been published",
-                        self.num_child_objects
+                        num_child_objects
                     ) % {
-                        'num_child_objects': self.num_child_objects,
-                        'num_parent_objects': self.num_parent_objects
+                        'num_child_objects': num_child_objects,
+                        'num_parent_objects': num_parent_objects
                     }
             else:
-                success_message = _("%(num_parent_objects)d pages have been published") % {'num_parent_objects': self.num_parent_objects}
+                success_message = _("%(num_parent_objects)d pages have been published") % {'num_parent_objects': num_parent_objects}
         return success_message
 
 

wagtail/admin/views/pages/bulk_actions/unpublish.py

@@ -34,55 +34,50 @@ class UnpublishBulkAction(PageBulkAction):
         }
 
     @classmethod
-    def execute_action(cls, objects, **kwargs):
-        include_descendants = kwargs.get('include_descendants', False)
-        user = kwargs.get('user', None)
-        if user is None:
-            return
-        permission_checker = kwargs.get('permission_checker', None)
-        if permission_checker is None:
-            return
+    def execute_action(cls, objects, include_descendants=False, user=None, permission_checker=None, **kwargs):
+        num_parent_objects, num_child_objects = 0, 0
         for page in objects:
             page.unpublish(user=user)
-            cls.num_parent_objects += 1
+            num_parent_objects += 1
 
             if include_descendants:
                 for live_descendant_page in page.get_descendants().live().defer_streamfields().specific():
-                    if permission_checker(live_descendant_page):
+                    if user is None or permission_checker(live_descendant_page):
                         live_descendant_page.unpublish()
-                        cls.num_child_objects += 1
+                        num_child_objects += 1
+        return num_parent_objects, num_child_objects
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         include_descendants = self.cleaned_form.cleaned_data['include_descendants']
-        if self.num_parent_objects == 1:
+        if num_parent_objects == 1:
             if include_descendants:
-                if self.num_child_objects == 0:
+                if num_child_objects == 0:
                     success_message = _("1 page has been unpublished")
                 else:
                     success_message = ngettext(
                         "1 page and %(num_child_objects)d child page have been unpublished",
                         "1 page and %(num_child_objects)d child pages have been unpublished",
-                        self.num_child_objects
+                        num_child_objects
                     ) % {
-                        'num_child_objects': self.num_child_objects
+                        'num_child_objects': num_child_objects
                     }
             else:
                 success_message = _("1 page has been unpublished")
         else:
             if include_descendants:
-                if self.num_child_objects == 0:
-                    success_message = _("%(num_parent_objects)d pages have been unpublished") % {'num_parent_objects': self.num_parent_objects}
+                if num_child_objects == 0:
+                    success_message = _("%(num_parent_objects)d pages have been unpublished") % {'num_parent_objects': num_parent_objects}
                 else:
                     success_message = ngettext(
                         "%(num_parent_objects)d pages and %(num_child_objects)d child page have been unpublished",
                         "%(num_parent_objects)d pages and %(num_child_objects)d child pages have been unpublished",
-                        self.num_child_objects
+                        num_child_objects
                     ) % {
-                        'num_child_objects': self.num_child_objects,
-                        'num_parent_objects': self.num_parent_objects
+                        'num_child_objects': num_child_objects,
+                        'num_parent_objects': num_parent_objects
                     }
             else:
-                success_message = _("%(num_parent_objects)d pages have been unpublished") % {'num_parent_objects': self.num_parent_objects}
+                success_message = _("%(num_parent_objects)d pages have been unpublished") % {'num_parent_objects': num_parent_objects}
         return success_message
 
 

wagtail/documents/views/bulk_actions/add_tags.py

@@ -8,11 +8,7 @@ from wagtail.documents.views.bulk_actions.document_bulk_action import DocumentBu
 
 
 class TagForm(forms.Form):
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.fields['tags'] = forms.Field(
-            widget=widgets.AdminTagWidget
-        )
+    tags = forms.Field(widget=widgets.AdminTagWidget)
 
 
 class AddTagsBulkAction(DocumentBulkAction):
@@ -32,19 +28,22 @@ class AddTagsBulkAction(DocumentBulkAction):
         }
 
     @classmethod
-    def execute_action(cls, objects, **kwargs):
-        tags = kwargs.get('tags', [])
+    def execute_action(cls, objects, tags=[], **kwargs):
+        num_parent_objects = 0
+        if not tags:
+            return
         for document in objects:
-            cls.num_parent_objects += 1
+            num_parent_objects += 1
             document.tags.add(*tags)
+        return num_parent_objects, 0
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         return ngettext(
             "New tags have been added to %(num_parent_objects)d document",
             "New tags have been added to %(num_parent_objects)d documents",
-            self.num_parent_objects
+            num_parent_objects
         ) % {
-            'num_parent_objects': self.num_parent_objects
+            'num_parent_objects': num_parent_objects
         }
 
 

wagtail/documents/views/bulk_actions/add_to_collection.py

@@ -39,21 +39,21 @@ class AddToCollectionBulkAction(DocumentBulkAction):
         }
 
     @classmethod
-    def execute_action(cls, objects, **kwargs):
-        cls.collection = kwargs.get('collection', None)
-        for document in objects:
-            cls.num_parent_objects += 1
-            document.collection = cls.collection
-            document.save()
-
-    def get_success_message(self):
+    def execute_action(cls, objects, collection=None, **kwargs):
+        if collection is None:
+            return
+        num_parent_objects = cls.model.objects.filter(pk__in=[obj.pk for obj in objects]).update(collection=collection)
+        return num_parent_objects, 0
+
+    def get_success_message(self, num_parent_objects, num_child_objects):
+        collection = self.cleaned_form.cleaned_data['collection']
         return ngettext(
             "%(num_parent_objects)d document has been added to %(collection)s",
             "%(num_parent_objects)d documents have been added to %(collection)s",
-            self.num_parent_objects
+            num_parent_objects
         ) % {
-            'num_parent_objects': self.num_parent_objects,
-            'collection': self.collection.name
+            'num_parent_objects': num_parent_objects,
+            'collection': collection
         }
 
 

wagtail/documents/views/bulk_actions/delete.py

@@ -18,17 +18,17 @@ class DeleteBulkAction(DocumentBulkAction):
 
     @classmethod
     def execute_action(cls, objects, **kwargs):
-        for document in objects:
-            cls.num_parent_objects += 1
-            document.delete()
+        num_parent_objects = len(objects)
+        cls.model.objects.filter(pk__in=[obj.pk for obj in objects]).delete()
+        return num_parent_objects, 0
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         return ngettext(
             "%(num_parent_objects)d document has been deleted",
             "%(num_parent_objects)d documents have been deleted",
-            self.num_parent_objects
+            num_parent_objects
         ) % {
-            'num_parent_objects': self.num_parent_objects
+            'num_parent_objects': num_parent_objects
         }
 
 

wagtail/images/views/bulk_actions/add_tags.py

@@ -8,11 +8,7 @@ from wagtail.images.views.bulk_actions.image_bulk_action import ImageBulkAction
 
 
 class TagForm(forms.Form):
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.fields['tags'] = forms.Field(
-            widget=widgets.AdminTagWidget
-        )
+    tags = forms.Field(widget=widgets.AdminTagWidget)
 
 
 class AddTagsBulkAction(ImageBulkAction):
@@ -32,19 +28,22 @@ class AddTagsBulkAction(ImageBulkAction):
         }
 
     @classmethod
-    def execute_action(cls, images, **kwargs):
-        tags = kwargs.get('tags', [])
+    def execute_action(cls, images, tags=[], **kwargs):
+        num_parent_objects = 0
+        if not tags:
+            return
         for image in images:
-            cls.num_parent_objects += 1
+            num_parent_objects += 1
             image.tags.add(*tags)
+        return num_parent_objects, 0
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         return ngettext(
             "New tags have been added to %(num_parent_objects)d image",
             "New tags have been added to %(num_parent_objects)d images",
-            self.num_parent_objects
+            num_parent_objects
         ) % {
-            'num_parent_objects': self.num_parent_objects
+            'num_parent_objects': num_parent_objects
         }
 
 

wagtail/images/views/bulk_actions/add_to_collection.py

@@ -39,23 +39,21 @@ class AddToCollectionBulkAction(ImageBulkAction):
         }
 
     @classmethod
-    def execute_action(cls, images, **kwargs):
-        cls.collection = kwargs.get('collection', None)
-        if cls.collection is None:
+    def execute_action(cls, images, collection=None, **kwargs):
+        if collection is None:
             return
-        for image in images:
-            cls.num_parent_objects += 1
-            image.collection = cls.collection
-            image.save()
+        num_parent_objects = cls.model.objects.filter(pk__in=[obj.pk for obj in images]).update(collection=collection)
+        return num_parent_objects, 0
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
+        collection = self.cleaned_form.cleaned_data['collection']
         return ngettext(
             "%(num_parent_objects)d image has been added to %(collection)s",
             "%(num_parent_objects)d images have been added to %(collection)s",
-            self.num_parent_objects
+            num_parent_objects
         ) % {
-            'num_parent_objects': self.num_parent_objects,
-            'collection': self.collection.name
+            'num_parent_objects': num_parent_objects,
+            'collection': collection.name
         }
 
 

wagtail/images/views/bulk_actions/delete.py

@@ -18,17 +18,17 @@ class DeleteBulkAction(ImageBulkAction):
 
     @classmethod
     def execute_action(cls, objects, **kwargs):
-        for image in objects:
-            cls.num_parent_objects += 1
-            image.delete()
+        num_parent_objects = len(objects)
+        cls.model.objects.filter(pk__in=[obj.pk for obj in objects]).delete()
+        return num_parent_objects, 0
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         return ngettext(
             "%(num_parent_objects)d image has been deleted",
             "%(num_parent_objects)d images have been deleted",
-            self.num_parent_objects
+            num_parent_objects
         ) % {
-            'num_parent_objects': self.num_parent_objects
+            'num_parent_objects': num_parent_objects
         }
 
 

wagtail/users/views/bulk_actions/assign_role.py

@@ -29,7 +29,6 @@ class AssignRoleBulkAction(UserBulkAction):
     def get_execution_context(self):
         return {
             'role': self.cleaned_form.cleaned_data['role'],
-            'user': self.request.user
         }
 
     def prepare_action(self, objects):
@@ -47,15 +46,16 @@ class AssignRoleBulkAction(UserBulkAction):
         if role is None:
             return
         role.user_set.add(*objects)
-        cls.num_parent_objects = len(objects)
+        num_parent_objects = len(objects)
+        return num_parent_objects, 0
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         return ngettext(
             "%(num_parent_objects)d user has been assigned as %(role)s",
             "%(num_parent_objects)d users have been assigned as %(role)s",
-            self.num_parent_objects
+            num_parent_objects
         ) % {
-            'num_parent_objects': self.num_parent_objects,
+            'num_parent_objects': num_parent_objects,
             'role': self.cleaned_form.cleaned_data['role'].name
         }
 

wagtail/users/views/bulk_actions/delete.py

@@ -20,15 +20,16 @@ class DeleteBulkAction(UserBulkAction):
     @classmethod
     def execute_action(cls, objects, **kwargs):
         cls.model.objects.filter(pk__in=[obj.pk for obj in objects]).delete()
-        cls.num_parent_objects = len(objects)
+        num_parent_objects = len(objects)
+        return num_parent_objects, 0
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         return ngettext(
             "%(num_parent_objects)d user has been deleted",
             "%(num_parent_objects)d users have been deleted",
-            self.num_parent_objects
+            num_parent_objects
         ) % {
-            'num_parent_objects': self.num_parent_objects
+            'num_parent_objects': num_parent_objects
         }
 
 

wagtail/users/views/bulk_actions/toggle_activity.py

@@ -43,28 +43,28 @@ class ToggleActivityBulkAction(UserBulkAction):
                     return TemplateResponse(self.request, self.template_name, context)
 
     @classmethod
-    def execute_action(cls, objects, mark_as_active=False, **kwargs):
-        user = kwargs.get('user', None)
+    def execute_action(cls, objects, mark_as_active=False, user=None, **kwargs):
         if user is not None:
             objects = list(filter(lambda x: x.pk != user.pk, objects))
-        cls.num_parent_objects = cls.model.objects.filter(pk__in=[obj.pk for obj in objects]).update(is_active=mark_as_active)
+        num_parent_objects = cls.model.objects.filter(pk__in=[obj.pk for obj in objects]).update(is_active=mark_as_active)
+        return num_parent_objects, 0
 
-    def get_success_message(self):
+    def get_success_message(self, num_parent_objects, num_child_objects):
         if self.cleaned_form.cleaned_data['mark_as_active']:
             return ngettext(
                 "%(num_parent_objects)d user has been marked as active",
                 "%(num_parent_objects)d users have been marked as active",
-                self.num_parent_objects
+                num_parent_objects
             ) % {
-                'num_parent_objects': self.num_parent_objects,
+                'num_parent_objects': num_parent_objects,
             }
         else:
             return ngettext(
                 "%(num_parent_objects)d user has been marked as inactive",
                 "%(num_parent_objects)d users have been marked as inactive",
-                self.num_parent_objects
+                num_parent_objects
             ) % {
-                'num_parent_objects': self.num_parent_objects,
+                'num_parent_objects': num_parent_objects,
             }