Clean up

django-cms · czpython · Jul 11, 2018 · Jul 2, 2018 · Jul 2, 2018 · Jul 3, 2018
commit 56931e8a218c4be8a85662601287a3a5cda0980d
diff --git a/cms/cms_config.py b/cms/cms_config.py
@@ -13,9 +13,13 @@ class CMSCoreConfig(CMSAppConfig):
 class CMSCoreExtensions(CMSAppExtension):
 
     def __init__(self):
-        self.wizards = {}  # this is instead of wizard_pool._entries
+        self.wizards = {}
 
     def configure_wizards(self, cms_config):
+        """
+        Adds all registered wizards from apps that define them to the
+        wizards dictionary on this class
+        """
         if not hasattr(cms_config, 'cms_wizards'):
             # The cms_wizards settings is optional. If it's not here
             # just move on.
@@ -27,8 +31,8 @@ def configure_wizards(self, cms_config):
                 raise ImproperlyConfigured(
                     "all wizards defined in cms_wizards must inherit "
                     "from cms.wizards.wizard_base.Wizard")
-        for wizard in cms_config.cms_wizards:
-            self.wizards[wizard.id] = wizard
+            else:
+                self.wizards[wizard.id] = wizard
 
     def configure_app(self, cms_config):
         self.configure_wizards(cms_config)
diff --git a/cms/cms_wizards.py b/cms/cms_wizards.py
@@ -4,7 +4,6 @@
 from cms.models import Page
 from cms.utils.page_permissions import user_can_add_page, user_can_add_subpage
 
-from .wizards.wizard_pool import wizard_pool
 from .wizards.wizard_base import Wizard
 
 from .forms.wizards import CreateCMSPageForm, CreateCMSSubPageForm

diff --git a/cms/tests/test_cms_config.py b/cms/tests/test_cms_config.py
@@ -15,6 +15,10 @@
 class ConfigureWizardsUnitTestCase(CMSTestCase):
 
     def test_adds_wizards_to_dict(self):
+        """
+        If the wizard setting is correctly defined, add the wizards to the
+        dictionary of wizards
+        """
         extensions = CMSCoreExtensions()
         wizard1 = Mock(id=111, spec=Wizard)
         wizard2 = Mock(id=222, spec=Wizard)
@@ -27,6 +31,10 @@ def test_adds_wizards_to_dict(self):
             extensions.wizards, {111: wizard1, 222: wizard2})
 
     def test_doesnt_raise_exception_when_wizards_dict_undefined(self):
+        """
+        If the wizard setting is not present in the config, simply
+        ignore this. The wizard setting is optional.
+        """
         extensions = CMSCoreExtensions()
         cms_config = Mock(cms_enabled=True, spec=[])
 
@@ -36,6 +44,10 @@ def test_doesnt_raise_exception_when_wizards_dict_undefined(self):
             self.fail("Raises exception when cms_wizards undefined")
 
     def test_raises_exception_if_doesnt_inherit_from_wizard_class(self):
+        """
+        If one or more of the wizards defined in the wizard setting
+        do not inherit from the Wizard class, raise an exception
+        """
         extensions = CMSCoreExtensions()
         wizard = Mock(id=3, spec=object)
         cms_config = Mock(
@@ -45,6 +57,9 @@ def test_raises_exception_if_doesnt_inherit_from_wizard_class(self):
             extensions.configure_wizards(cms_config)
 
     def test_raises_exception_if_not_list(self):
+        """
+        If the wizard setting isn't a list, raise an exception.
+        """
         extensions = CMSCoreExtensions()
         wizard = Mock(id=6, spec=Wizard)
         cms_config = Mock(
@@ -65,6 +80,10 @@ def setUp(self):
         get_cms_config_apps.cache_clear()
 
     def test_adds_all_wizards_to_dict(self):
+        """
+        Check that all wizards are picked up from both cms.cms_config
+        and sampleapp.cms_config
+        """
         setup_cms_apps()
 
         app = apps.get_app_config('cms')

diff --git a/cms/tests/test_wizards.py b/cms/tests/test_wizards.py
@@ -176,39 +176,63 @@ def tearDown(self):
             extension.wizards = expected_wizards
 
     def test_discovered_returns_true(self):
-        # Backwards compatibility
+        """
+        Test for backwards compatibility of the discovered property.
+        This should always return True because discovering of wizards
+        now happens in the app registration system so as far as the
+        wizard_pool is concerned they are always discovered.
+        """
         self.assertTrue(wizard_pool.discovered)
 
     def test_is_registered_for_registered_wizard(self):
-        # Backwards compatibility
+        """
+        Test for backwards compatibility of is_registered when checking
+        a registered wizard.
+        """
         is_registered = wizard_pool.is_registered(cms_page_wizard)
         self.assertTrue(is_registered)
 
     def test_is_registered_for_unregistered_wizard(self):
-        # Backwards compatibility
+        """
+        Test for backwards compatibility of is_registered when checking
+        an unregistered wizard.
+        """
         is_registered = wizard_pool.is_registered(self.page_wizard)
         self.assertFalse(is_registered)
 
     def test_unregister_registered_wizard(self):
-        # Test for backwards compatibility only.
+        """
+        Test for backwards compatibility of the unregister method.
+        Removes a wizard from the wizards dict.
+        """
         was_unregistered = wizard_pool.unregister(cms_page_wizard)
 
         registered_wizards = apps.get_app_config('cms').cms_extension.wizards
         self.assertNotIn(cms_page_wizard.id, registered_wizards)
         self.assertTrue(was_unregistered)
 
     def test_unregister_unregistered_wizard(self):
-        # Test for backwards compatibility only.
+        """
+        Test for backwards compatibility of the unregister method.
+        Returns False if wizard not fount.
+        """
         was_unregistered = wizard_pool.unregister(self.page_wizard)
         self.assertFalse(was_unregistered)
 
     def test_register_already_registered_wizard(self):
-        # Test for backwards compatibility only.
+        """
+        Test for backwards compatibility of the register method.
+        Raises AlreadyRegisteredException if the wizard is already
+        registered.
+        """
         with self.assertRaises(AlreadyRegisteredException):
             wizard_pool.register(cms_page_wizard)
 
     def test_register_unregistered_wizard(self):
-        # Test for backwards compatibility only.
+        """
+        Test for backwards compatibility of the register method.
+        Adds the wizard to the wizards dict.
+        """
         wizard_pool.register(self.page_wizard)
 
         registered_wizards = apps.get_app_config('cms').cms_extension.wizards
@@ -540,6 +564,10 @@ def setUp(self):
         get_cms_config_apps.cache_clear()
 
     def test_get_entries_orders_by_weight(self):
+        """
+        The get_entries function returns the registered wizards
+        ordered by weight.
+        """
         # The test setup registers two wizards from cms itself
         # (cms_page_wizard and cms_subpage_wizard) and one from
         # test_utils.project.sampleapp (sample_wizard)
@@ -553,17 +581,32 @@ def test_get_entries_orders_by_weight(self):
         self.assertListEqual(entries, expected)
 
     def test_get_entry_returns_wizard_by_id(self):
+        """
+        The get_entry function returns the wizard when a wizard id is
+        supplied.
+        """
         entry = get_entry(sample_wizard.id)
         self.assertEqual(entry, sample_wizard)
 
     def test_get_entry_returns_wizard_by_object(self):
+        """
+        The get_entry function returns the wizard when a wizard is
+        supplied.
+
+        NOTE: This is a little weird as we're simply returning the
+        object we're passing to get_entry, but keeping it this way for
+        backwards compatibility.
+        """
         entry = get_entry(sample_wizard)
         self.assertEqual(entry, sample_wizard)
 
 
 class TestEntryChoices(CMSTestCase):
 
     def test_generates_choices_in_weighted_order(self):
+        """
+        The entry_choices function returns the wizards ordered by weight
+        """
         user = self.get_superuser()
         page = create_page('home', 'nav_playground.html', 'en', published=True)
 
@@ -581,6 +624,10 @@ def test_generates_choices_in_weighted_order(self):
         Mock(return_value=False)
     )
     def test_doesnt_generate_choice_if_user_doesnt_have_permission(self):
+        """
+        The entry_choices function only returns the wizards that the
+        user has permissions for
+        """
         user = self.get_superuser()
         page = create_page('home', 'nav_playground.html', 'en', published=True)
 

diff --git a/cms/wizards/helpers.py b/cms/wizards/helpers.py
@@ -5,12 +5,19 @@
 
 
 def get_entries():
+    """
+    Returns a list of (wizard.id, wizard) tuples (for all registered
+    wizards) ordered by weight
+    """
     wizards = apps.get_app_config('cms').cms_extension.wizards
     return [value for (key, value) in sorted(
             wizards.items(), key=lambda e: getattr(e[1], 'weight'))]
 
 
 def get_entry(entry_key):
+    """
+    Returns a wizard object based on its id.
+    """
     if isinstance(entry_key, Wizard):
         entry_key = entry_key.id
     return apps.get_app_config('cms').cms_extension.wizards[entry_key]
diff --git a/cms/wizards/wizard_pool.py b/cms/wizards/wizard_pool.py
@@ -1,6 +1,5 @@
 # -*- coding: utf-8 -*-
 from django.apps import apps
-from django.utils.module_loading import autodiscover_modules
 from django.utils.translation import ugettext as _
 
 from cms.wizards.helpers import get_entries, get_entry
@@ -51,7 +50,6 @@ def register(self, entry):
 
         Raises AlreadyRegisteredException if the entry is already registered.
         """
-        # TODO: Use the new app registration instead of this
         # TODO: Add deprecation warning
         assert isinstance(entry, Wizard), u"entry must be an instance of Wizard"
         if self.is_registered(entry, passive=True):