8000 fix: Allow lazy wizard initialization by fsbraun · Pull Request #8266 · django-cms/django-cms · GitHub
[go: up one dir, main page]

Skip to content

fix: Allow lazy wizard initialization #8266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
8000
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8000
35 changes: 24 additions & 11 deletions cms/cms_config.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from collections.abc import Iterable
from functools import cached_property
from logging import getLogger

from django.core.exceptions import ImproperlyConfigured
Expand All @@ -19,9 +20,8 @@ class CMSCoreConfig(CMSAppConfig):


class CMSCoreExtensions(CMSAppExtension):

def __init__(self):
self.wizards = {}
self.lazy_wizards = []
self.toolbar_enabled_models = {}
self.model_groupers = {}
self.toolbar_mixins = []
Expand All @@ -33,17 +33,30 @@ def configure_wizards(self, cms_config):
"""
if not isinstance(cms_config.cms_wizards, Iterable):
raise ImproperlyConfigured("cms_wizards must be iterable")
for wizard in cms_config.cms_wizards:
if not isinstance(wizard, Wizard):

self.lazy_wizards.append(cms_config.cms_wizards)

@cached_property
def wizards(self) -> dict[str, Wizard]:
"""
Returns a dictionary of wizard instances keyed by their unique IDs.
Iterates over all iterables in `self.lazy_wizards`, filters out objects that are instances
of the `Wizard` class, and constructs a dictionary where each key is the wizard's `id`
and the value is the corresponding `Wizard` instance.
Returns:
dict: A dictionary mapping wizard IDs to `Wizard` instances.
"""

wizards = {}
for iterable in self.lazy_wizards:
new_wizards = {wizard.id: wizard for wizard in iterable}
if wizard := next((wizard for wizard in new_wizards.values() if not isinstance(wizard, Wizard)), None):
# If any wizard in the iterable is not an instance of Wizard, raise an exception
raise ImproperlyConfigured(
"All wizards defined in cms_wizards must inherit "
"from cms.wizards.wizard_base.Wizard"
f"cms_wizards must be iterable of Wizard instances, got {type(wizard)}"
)
elif wizard.id in self.wizards:
msg = f"Wizard for model {wizard.get_model()} has already been registered"
logger.warning(msg)
else:
self.wizards[wizard.id] = wizard
wizards |= new_wizards
return wizards

def configure_toolbar_enabled_models(self, cms_config):
if not isinstance(cms_config.cms_toolbar_enabled_models, Iterable):
Expand Down
10 changes: 3 additions & 7 deletions cms/cms_toolbars.py
10000
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,9 @@ def add_wizard_button(self):
from cms.wizards.wizard_pool import entry_choices
title = _("Create")

if self.page:
user = self.request.user
page_pk = self.page.pk
disabled = len(list(entry_choices(user, self.page))) == 0
else:
page_pk = ''
disabled = True
user = self.request.user
page_pk = self.page.pk if self.page else ""
disabled = not list(entry_choices(user, self.page))

url = '{url}?page={page}&language={lang}&edit'.format(
url=admin_reverse("cms_wizard_create"),
Expand Down
20 changes: 3 additions & 17 deletions cms/tests/test_cms_config_wizards.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ def test_raises_exception_if_doesnt_inherit_from_wizard_class(self):
wizard = Mock(id=3, spec=object)
cms_config = Mock(
cms_enabled=True, cms_wizards=[wizard])
extensions.configure_wizards(cms_config)
with self.assertRaises(ImproperlyConfigured):
extensions.configure_wizards(cms_config)
extensions.wizards

Comment on lines 49 to +55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Test for exception when non-Wizard instance is present now triggers on property access.

Consider adding a comment or separate test to document that the exception is now raised on 'wizards' property access, not during configuration, to clarify intent and timing.

Suggested change
wizard = Mock(id=3, spec=object)
cms_config = Mock(
cms_enabled=True, cms_wizards=[wizard])
extensions.configure_wizards(cms_config)
with self.assertRaises(ImproperlyConfigured):
extensions.configure_wizards(cms_config)
extensions.wizards
# Note: Exception is now raised on 'wizards' property access, not during configuration.
wizard = Mock(id=3, spec=object)
cms_config = Mock(
cms_enabled=True, cms_wizards=[wizard])
extensions.configure_wizards(cms_config)
with self.assertRaises(ImproperlyConfigured):
extensions.wizards
def test_raises_exception_on_wizards_property_access_with_non_wizard(self):
"""
Test that ImproperlyConfigured is raised when a non-Wizard instance is present,
and that the exception is triggered on 'wizards' property access, not during configuration.
"""
wizard = Mock(id=3, spec=object)
cms_config = Mock(
cms_enabled=True, cms_wizards=[wizard])
extensions.configure_wizards(cms_config)
with self.assertRaises(ImproperlyConfigured):
_ = extensions.wizards


def test_raises_exception_if_not_iterable(self):
"""
Expand All @@ -63,22 +65,6 @@ def test_raises_exception_if_not_iterable(self):
with self.assertRaises(ImproperlyConfigured):
extensions.configure_wizards(cms_config)

@patch('cms.cms_config.logger.warning')
def test_warning_if_registering_the_same_wizard_twice(self, mocked_logger):
"""
If a wizard is already added to the dict log a warning.
"""
extensions = CMSCoreExtensions()
wizard = Mock(id=81, spec=Wizard)
cms_config = Mock(
cms_enabled=True, cms_wizards=[wizard, wizard])
extensions.configure_wizards(cms_config)
warning_msg = f"Wizard for model {wizard.get_model()} has already been registered"
# Warning message displayed
mocked_logger.assert_called_once_with(warning_msg)
# wizards dict is still what we expect it to be
self.assertDictEqual(extensions.wizards, {81: wizard})


class ConfigureWizardsIntegrationTestCase(CMSTestCase):

Expand Down
7 changes: 6 additions & 1 deletion cms/tests/test_wizards.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,9 @@ def tearDown(self):
# Clean up in case anything has been removed or added to the
# registered wizards, so other tests don't have problems
extension = apps.get_app_config('cms').cms_extension
extension.wizards = {}
# Reset the cached property 'wizards'
if hasattr(extension, 'wizards'):
del extension.wizards
configs_with_wizards = [
app.cms_config for app in app_registration.get_cms_config_apps()
if hasattr(app.cms_config, 'cms_wizards')
Expand All @@ -235,6 +237,9 @@ def test_is_registered_for_registered_wizard(self):
Test for backwards compatibility of is_registered when checking
a registered wizard.
"""
from django.apps import apps

self.assertTrue(apps.get_app_config("cms").cms_extension.wizards)
is_registered = wizard_pool.is_registered(cms_page_wizard)
self.assertTrue(is_registered)

Expand Down
32 changes: 9 additions & 23 deletions cms/wizards/helpers.py
Original file line number Diff line number Diff line change
@@ -1,26 +1,12 @@
from django.apps import apps
import warnings

from cms.utils.compat.warnings import RemovedInDjangoCMS60Warning

def get_entries():
"""
Returns a list of (wizard.id, wizard) tuples (for all registered
wizards) ordered by weight
from .wizard_base import get_entries, get_entry # noqa: F401

``get_entries()`` is useful if it is required to have a list of all registered
wizards. Typically, this is used to iterate over them all. Note that they will
be returned in the order of their ``weight``: smallest numbers for weight are
returned first.::

for wizard_id, wizard in get_entries():
# do something with a wizard...
"""
wizards = apps.get_app_config('cms').cms_extension.wizards
return [value for (key, value) in sorted(
wizards.items(), key=lambda e: e[1].weight)]


def get_entry(entry_key):
"""
Returns a wizard object based on its :attr:`~.cms.wizards.wizard_base.Wizard.id`.
"""
return apps.get_app_config('cms').cms_extension.wizards[entry_key]
warnings.warn(
"The cms.wizards.helpers module is deprecated and will be removed in django CMS 5.1. "
"Use cms.wizards.wizard_base.get_entries and cms.wizards.wizard_pool.get_entry instead.",
RemovedInDjangoCMS60Warning,
stacklevel=2,
)
4 changes: 2 additions & 2 deletions cms/wizards/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from cms.utils.i18n import get_site_language_from_request

from .forms import WizardStep1Form, WizardStep2BaseForm, step2_form_factory
from .wizard_pool import wizard_pool
from .wizard_base import get_entry


class WizardCreateView(SessionWizardView):
Expand Down Expand Up @@ -150,7 +150,7 @@ def done(self, form_list, **kwargs):

def get_selected_entry(self):
data = self.get_cleaned_data_for_step('0')
return wizard_pool.get_entry(data['entry'])
return get_entry(data['entry'])

def get_origin_page(self):
data = self.get_cleaned_data_for_step('0')
Expand Down
43 changes: 43 additions & 0 deletions cms/wizards/wizard_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,49 @@
)


def get_entries():
"""
Returns a list of Wizard objects (for all registered wizards) ordered by weight

``get_entries()`` is useful if it is required to have a list of all registered
wizards. Typically, this is used to iterate over them all. Note that they will
be returned in the order of their ``weight``: smallest numbers for weight are
returned first.::

for wizard in get_entries():
# do something with a wizard...
"""
wizards = apps.get_app_config('cms').cms_extension.wizards
return sorted(wizards.values(), key=lambda e: e.weight)


def get_entry(entry_key):
"""
Returns a wizard object based on its :attr:`~.cms.wizards.wizard_base.Wizard.id`.
"""
return apps.get_app_config('cms').cms_extension.wizards[entry_key]


def entry_choices(user, page):
"""
Yields a list of wizard entry tuples of the form (wizard.id, wizard.title) that
the current user can use based on their permission to add instances of the
underlying model objects.
"""
for entry in get_entries():
if entry.user_has_add_permission(user, page=page):
yield (entry.id, entry.title)


def clear_wizard_cache():
"""
Clears the wizard cache. This is useful if you have added or removed wizards
and want to ensure that the changes are reflected immediately.
"""
del apps.get_app_config('cms').cms_extension.wizards



class WizardBase:
"""

Expand Down
23 changes: 10 additions & 13 deletions cms/wizards/wizard_pool.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,15 @@
from django.apps import apps
from django.utils.translation import gettext as _

from cms.wizards.helpers import get_entries, get_entry
from cms.wizards.wizard_base import Wizard
from cms.utils.compat.warnings import RemovedInDjangoCMS60Warning
from cms.wizards.helpers import get_entries, get_entry # noqa: F401
from cms.wizards.wizard_base import Wizard, entry_choices # noqa: F401


class AlreadyRegisteredException(Exception):
pass


def entry_choices(user, page):
"""
Yields a list of wizard entries that the current user can use based on their
permission to add instances of the underlying model objects.
"""
for entry in get_entries():
if entry.user_has_add_permission(user, page=page):
yield (entry.id, entry.title)


class WizardPool:
"""
.. deprecated:: 4.0
Expand Down Expand Up @@ -49,7 +40,13 @@ def register(self, entry):
name. In this case, the register method will raise a
``cms.wizards.wizard_pool.AlreadyRegisteredException``.
"""
# TODO: Add deprecation warning
import warnings

warnings.warn(
"Using wizard_pool is deprecated. Use the cms_config instead.",
RemovedInDjangoCMS60Warning,
stacklevel=2,
)
assert isinstance(entry, Wizard), "entry must be an instance of Wizard"
if self.is_registered(entry, passive=True):
model = entry.get_model()
Expand Down
12 changes: 4 additions & 8 deletions docs/reference/wizards.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ When instantiating a Wizard object, use the keywords:
CMS will create one from the pattern:
"Create a new «model.verbose_name» instance."
:edit_mode_on_success: Whether the user will get redirected to object edit url after a
successful creation or not. This only works if the object is registered
successful creation or not. This only works if the object is registered
for toolbar enabled models.


Expand Down Expand Up @@ -78,17 +78,13 @@ Wizard class
:members:
:inherited-members:


*******
Helpers
*******

.. module:: cms.wizards.helpers

.. autofunction:: get_entry

.. autofunction:: get_entries

.. autofunction:: entry_choices



***********
wizard_pool
Expand Down
Loading
0