From 8e56f5fe3bc8afaef52fae9785b02d9b115c8c0a Mon Sep 17 00:00:00 2001 From: Sandro Date: Tue, 3 Oct 2023 00:23:18 +0100 Subject: [PATCH] Fixed SnippetBulkAction not respecting models definition Amends c1a4528b8a91ebdb5018837c37ffd54ef34f0aba to use Django's classproperty util instead of a get_models() method that replaces the models definition that may be overridden in custom bulk actions. --- .../views/bulk_action/base_bulk_action.py | 12 ++-- wagtail/admin/views/bulk_action/registry.py | 2 +- .../bulk_actions/snippet_bulk_action.py | 13 ++--- .../test_bulk_actions/test_custom_models.py | 56 +++++++++++++++++++ wagtail/test/testapp/wagtail_hooks.py | 10 ++++ 5 files changed, 78 insertions(+), 15 deletions(-) create mode 100644 wagtail/snippets/tests/test_bulk_actions/test_custom_models.py diff --git a/wagtail/admin/views/bulk_action/base_bulk_action.py b/wagtail/admin/views/bulk_action/base_bulk_action.py index ca26086926..d796801d51 100644 --- a/wagtail/admin/views/bulk_action/base_bulk_action.py +++ b/wagtail/admin/views/bulk_action/base_bulk_action.py @@ -3,6 +3,7 @@ from abc import ABC, abstractmethod from django import forms from django.db import transaction from django.shortcuts import get_list_or_404, redirect +from django.utils.functional import classproperty from django.views.generic import FormView from wagtail import hooks @@ -28,7 +29,6 @@ class BulkAction(ABC, FormView): extras = {} action_priority = 100 - models = [] classes = set() form_class = forms.Form @@ -41,7 +41,7 @@ class BulkAction(ABC, FormView): next_url = request.path self.next_url = next_url self.num_parent_objects = self.num_child_objects = 0 - if model in self.get_models(): + if model in self.models: self.model = model else: raise Exception( @@ -50,9 +50,9 @@ class BulkAction(ABC, FormView): ) ) - @classmethod - def get_models(cls): - return cls.models + @classproperty + def models(cls): + return [] @classmethod def get_queryset(cls, model, object_ids): @@ -73,7 +73,7 @@ class BulkAction(ABC, FormView): @classmethod def get_default_model(cls): - models = cls.get_models() + models = cls.models if len(models) == 1: return models[0] raise Exception( diff --git a/wagtail/admin/views/bulk_action/registry.py b/wagtail/admin/views/bulk_action/registry.py index 5c7d6c0abe..4750494dea 100644 --- a/wagtail/admin/views/bulk_action/registry.py +++ b/wagtail/admin/views/bulk_action/registry.py @@ -16,7 +16,7 @@ class BulkActionRegistry: action_class.__name__, BulkAction.__name__ ) ) - for model in action_class.get_models(): + for model in action_class.models: self.actions.setdefault(model._meta.app_label, {}) self.actions[model._meta.app_label].setdefault( model._meta.model_name, {} diff --git a/wagtail/snippets/bulk_actions/snippet_bulk_action.py b/wagtail/snippets/bulk_actions/snippet_bulk_action.py index 64b4d2007a..7616fb3385 100644 --- a/wagtail/snippets/bulk_actions/snippet_bulk_action.py +++ b/wagtail/snippets/bulk_actions/snippet_bulk_action.py @@ -1,11 +1,13 @@ +from django.utils.functional import classproperty + from wagtail.admin.admin_url_finder import AdminURLFinder from wagtail.admin.views.bulk_action import BulkAction from wagtail.snippets.models import get_snippet_models class SnippetBulkAction(BulkAction): - @classmethod - def get_models(cls): + @classproperty + def models(cls): # We used to set `models = get_snippet_models()` directly on the class, # but this is problematic because it means that the list of models is # evaluated at import time. @@ -14,12 +16,7 @@ class SnippetBulkAction(BulkAction): # can also be registered in wagtail_hooks.py. Evaluating # get_snippet_models() at import time could result in either a circular # import or an incomplete list of models. - - # Update the models list with the latest registered snippets in case - # there is user code that still accesses cls.models instead of calling - # this get_models() method. - cls.models = get_snippet_models() - return cls.models + return get_snippet_models() def object_context(self, snippet): return { diff --git a/wagtail/snippets/tests/test_bulk_actions/test_custom_models.py b/wagtail/snippets/tests/test_bulk_actions/test_custom_models.py new file mode 100644 index 0000000000..f69bd78038 --- /dev/null +++ b/wagtail/snippets/tests/test_bulk_actions/test_custom_models.py @@ -0,0 +1,56 @@ +from django.test import TestCase +from django.urls import reverse + +from wagtail.test.testapp.models import Advert, FullFeaturedSnippet +from wagtail.test.utils.wagtail_tests import WagtailTestUtils + + +class TestCustomModels(WagtailTestUtils, TestCase): + def setUp(self): + self.user = self.login() + + def create_snippets(self, model): + return [model.objects.create(text=f"Title-{i}") for i in range(1, 6)] + + def get_action_url(self, model, snippets): + return ( + reverse( + "wagtail_bulk_action", + args=( + model._meta.app_label, + model._meta.model_name, + "disable", + ), + ) + + "?" + + "&".join(f"id={item.pk}" for item in snippets) + ) + + def get_list_url(self, model): + return reverse(model.snippet_viewset.get_url_name("list")) + + def test_action_shown_for_custom_models(self): + self.create_snippets(FullFeaturedSnippet) + response = self.client.get(self.get_list_url(FullFeaturedSnippet)) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Disable selected full-featured snippets") + + def test_action_confirmation_accessible_for_custom_models(self): + snippets = self.create_snippets(FullFeaturedSnippet) + response = self.client.get(self.get_action_url(FullFeaturedSnippet, snippets)) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed( + response, + "wagtailadmin/bulk_actions/confirmation/base.html", + ) + + def test_action_not_shown_for_other_models(self): + self.create_snippets(Advert) + response = self.client.get(self.get_list_url(Advert)) + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, "Disable selected full-featured snippets") + + def test_action_confirmation_inaccessible_for_other_models(self): + snippets = self.create_snippets(Advert) + response = self.client.get(self.get_action_url(Advert, snippets)) + self.assertEqual(response.status_code, 404) diff --git a/wagtail/test/testapp/wagtail_hooks.py b/wagtail/test/testapp/wagtail_hooks.py index 0e3e95d38c..4a0a134e9a 100644 --- a/wagtail/test/testapp/wagtail_hooks.py +++ b/wagtail/test/testapp/wagtail_hooks.py @@ -20,6 +20,7 @@ from wagtail.admin.ui.components import Component from wagtail.admin.ui.tables import BooleanColumn, UpdatedAtColumn from wagtail.admin.views.account import BaseSettingsPanel from wagtail.admin.widgets import Button +from wagtail.snippets.bulk_actions.snippet_bulk_action import SnippetBulkAction from wagtail.snippets.models import register_snippet from wagtail.snippets.views.snippets import SnippetViewSet, SnippetViewSetGroup from wagtail.test.testapp.models import ( @@ -361,3 +362,12 @@ register_snippet(DraftStateModel, viewset=DraftStateModelViewSet) register_snippet(ModeratedModelViewSet) register_snippet(RevisableViewSetGroup) register_snippet(VariousOnDeleteModelViewSet) + + +@hooks.register("register_bulk_action") +class DisableBulkAction(SnippetBulkAction): + template_name = "wagtailadmin/bulk_actions/confirmation/base.html" + models = [FullFeaturedSnippet] + display_name = "Disable" + aria_label = "Disable selected full-featured snippets" + action_type = "disable"