Merge branch 'master' into feature/709-flag-invalid-prs

Author: prathmeshkulkarni-coder

docs/developer/admin-theme.rst

@@ -122,7 +122,7 @@ customize admin theme.
     to use the ``STATICFILES_DIR`` setting in ``settings.py``.
 
     You can learn more in the `Django documentation
-    <https://docs.djangoproject.com/en/4.2/ref/settings/#std:setting-STATICFILES_DIRS>`_.
+    <https://docs.djangoproject.com/en/5.2/ref/settings/#std:setting-STATICFILES_DIRS>`_.
 
 Extend Admin Theme Programmatically
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

docs/developer/admin-utilities.rst

@@ -157,7 +157,7 @@ by modifying ``InputFilter`` as following:
 
 To know about other lookups that can be used please check `Django Lookup
 API Reference
-<https://docs.djangoproject.com/en/4.2/ref/models/lookups/#django.db.models.Lookup>`__
+<https://docs.djangoproject.com/en/5.2/ref/models/lookups/#django.db.models.Lookup>`__
 
 ``openwisp_utils.admin_theme.filters.SimpleInputFilter``
 --------------------------------------------------------

docs/developer/custom-fields.rst

@@ -20,7 +20,7 @@ openwisp modules.
 ----------------------------------------------------
 
 This field extends Django's `BooleanField
-<https://docs.djangoproject.com/en/4.2/ref/models/fields/#booleanfield>`_
+<https://docs.djangoproject.com/en/5.2/ref/models/fields/#booleanfield>`_
 and provides additional functionality for handling choices with a fallback
 value.
 
@@ -45,7 +45,7 @@ between enabled and disabled options.
 -------------------------------------------------
 
 This field extends Django's `CharField
-<https://docs.djangoproject.com/en/4.2/ref/models/fields/#charfield>`_ and
+<https://docs.djangoproject.com/en/5.2/ref/models/fields/#charfield>`_ and
 provides additional functionality for handling choices with a fallback
 value.
 
@@ -73,7 +73,7 @@ value.
 -------------------------------------------
 
 This field extends Django's `CharField
-<https://docs.djangoproject.com/en/4.2/ref/models/fields/#charfield>`_ and
+<https://docs.djangoproject.com/en/5.2/ref/models/fields/#charfield>`_ and
 provides additional functionality for handling text fields with a fallback
 value.
 
@@ -96,7 +96,7 @@ value.
 ------------------------------------------
 
 This field extends Django's `URLField
-<https://docs.djangoproject.com/en/4.2/ref/models/fields/#urlfield>`_ and
+<https://docs.djangoproject.com/en/5.2/ref/models/fields/#urlfield>`_ and
 provides additional functionality for handling URL fields with a fallback
 value.
 
@@ -119,7 +119,7 @@ value.
 -------------------------------------------
 
 This extends Django's `TextField
-<https://docs.djangoproject.com/en/4.2/ref/models/fields/#django.db.models.TextField>`_
+<https://docs.djangoproject.com/en/5.2/ref/models/fields/#django.db.models.TextField>`_
 and provides additional functionality for handling text fields with a
 fallback value.
 
@@ -142,7 +142,7 @@ fallback value.
 ------------------------------------------------------
 
 This extends Django's `PositiveIntegerField
-<https://docs.djangoproject.com/en/4.2/ref/models/fields/#positiveintegerfield>`_
+<https://docs.djangoproject.com/en/5.2/ref/models/fields/#positiveintegerfield>`_
 and provides additional functionality for handling positive integer fields
 with a fallback value.
 
@@ -164,7 +164,7 @@ with a fallback value.
 ----------------------------------------------
 
 This extends Django's `DecimalField
-<https://docs.djangoproject.com/en/4.2/ref/models/fields/#decimalfield>`_
+<https://docs.djangoproject.com/en/5.2/ref/models/fields/#decimalfield>`_
 and provides additional functionality for handling decimal fields with a
 fallback value.
 

docs/developer/navigation-menu.rst

@@ -97,8 +97,12 @@ Code example:
         position=3, config={"label": _("My Link"), "url": "https://link.com"}
     )
 
-An ``ImproperlyConfigured`` exception is raised if a menu element is
-already registered at the same position.
+If the same ``config`` is registered at the same position more than once
+(e.g. because ``AppConfig.ready()`` is called again during testing), the
+call is a no-op and a log message is emitted at ``INFO`` level.
+
+An ``ImproperlyConfigured`` exception is raised if a *different* menu
+element is already registered at the same position.
 
 An ``ImproperlyConfigured`` exception is raised if the supplied
 configuration does not match with the different types of possible
@@ -278,8 +282,10 @@ Code example:
 An ``ImproperlyConfigured`` exception is raised if the group is not
 already registered at ``group_position``.
 
-An ``ImproperlyConfigured`` exception is raised if the group already has
-an item registered at ``item_position``.
+If the same ``config`` is registered at the same ``item_position`` more
+than once, the call is a no-op and a log message is emitted at ``INFO``
+level. An ``ImproperlyConfigured`` exception is raised if a *different*
+item is already registered at ``item_position``.
 
 It is only possible to register links to specific models or custom URL. An
 ``ImproperlyConfigured`` exception is raised if the configuration of group

docs/developer/test-utilities.rst

@@ -37,7 +37,7 @@ Usage example as a context-manager:
     :align: center
 
 This class extends the `default test runner provided by Django
-<https://docs.djangoproject.com/en/4.2/ref/settings/#std:setting-TEST_RUNNER>`_
+<https://docs.djangoproject.com/en/5.2/ref/settings/#std:setting-TEST_RUNNER>`_
 and logs the time spent by each test, making it easier to spot slow tests
 by highlighting time taken by it in yellow (time shall be highlighted in
 red if it crosses the second threshold).
@@ -152,7 +152,7 @@ Example usage:
 -----------------------------------------------------
 
 This mixin overrides the `assertNumQueries
-<https://docs.djangoproject.com/en/4.2/topics/testing/tools/#django.test.TransactionTestCase.assertNumQueries>`_
+<https://docs.djangoproject.com/en/5.2/topics/testing/tools/#django.test.TransactionTestCase.assertNumQueries>`_
 assertion from the django test case to run in a ``subTest`` so that the
 query check does not block the whole test if it fails.
 

openwisp_utils/admin_theme/menu.py

@@ -1,10 +1,14 @@
+import logging
+
 from django.apps import registry
 from django.core.exceptions import ImproperlyConfigured
 from django.urls import reverse
 from django.urls.exceptions import NoReverseMatch
 
 from ..utils import SortedOrderedDict
 
+logger = logging.getLogger(__name__)
+
 MENU = SortedOrderedDict()
 
 
@@ -19,6 +23,7 @@ def __init__(self, config):
             raise ImproperlyConfigured(
                 f'"config" should be a type of "dict". Error for config- {config}'
             )
+        self.config = config
 
     def get_context(self, request=None):
         return self.create_context(request)
@@ -60,7 +65,6 @@ def __init__(self, config):
         self.model = model
         self.set_label(config)
         self.icon = config.get("icon")
-        self.config = config
 
     def set_label(self, config=None):
         if config.get("label"):
@@ -185,6 +189,12 @@ def register_menu_group(position, config):
     if not isinstance(config, dict):
         raise ImproperlyConfigured('config should be a type of "dict"')
     if position in MENU:
+        if MENU[position].config == config:
+            logger.info(
+                f"A group/link with config {config} is already registered at"
+                f' position "{position}", skipping re-registration.'
+            )
+            return
         item_description = "link"
         if isinstance(MENU[position], MenuGroup):
             item_description = "group"
@@ -244,6 +254,13 @@ def register_menu_subitem(group_position, item_position, config):
             f'Invalid config "{config}" provided for sub group item'
         )
     if item_position in group.items:
+        if group.items[item_position].config == config:
+            logger.info(
+                f"A group item with config {config} is already registered at"
+                f' position "{item_position}" in the group at position'
+                f' "{group_position}", skipping re-registration.'
+            )
+            return
         name = group.items[item_position]
         raise ImproperlyConfigured(
             f'A group item with config {config} is being registered at position\

openwisp_utils/tests/selenium.py

@@ -367,26 +367,32 @@ def login(self, username=None, password=None, driver=None):
         driver.get(f"{self.live_server_url}/admin/login/")
         self._wait_until_page_ready(driver=driver)
         if "admin/login" in driver.current_url:
-            driver.find_element(by=By.NAME, value="username").send_keys(username)
-            driver.find_element(by=By.NAME, value="password").send_keys(password)
-            driver.find_element(by=By.XPATH, value='//input[@type="submit"]').click()
+            self.find_element(by=By.NAME, value="username", driver=driver).send_keys(
+                username
+            )
+            self.find_element(by=By.NAME, value="password", driver=driver).send_keys(
+                password
+            )
+            self.find_element(
+                by=By.XPATH, value='//input[@type="submit"]', driver=driver
+            ).click()
         self._wait_until_page_ready(driver=driver)
 
     def logout(self, driver=None):
         driver = driver or self.web_driver
-        self.web_driver.find_element(By.CSS_SELECTOR, ".account-button").click()
-        self.web_driver.find_element(By.CSS_SELECTOR, "#logout-form button").click()
+        self.find_element(By.CSS_SELECTOR, ".account-button", driver=driver).click()
+        self.find_element(By.CSS_SELECTOR, "#logout-form button", driver=driver).click()
 
     def find_element(self, by, value, timeout=2, driver=None, wait_for="visibility"):
         driver = driver or self.web_driver
         method = f"wait_for_{wait_for}"
-        getattr(self, method)(by, value, timeout)
+        getattr(self, method)(by, value, timeout, driver=driver)
         return driver.find_element(by=by, value=value)
 
     def find_elements(self, by, value, timeout=2, driver=None, wait_for="visibility"):
         driver = driver or self.web_driver
         method = f"wait_for_{wait_for}"
-        getattr(self, method)(by, value, timeout)
+        getattr(self, method)(by, value, timeout, driver=driver)
         return driver.find_elements(by=by, value=value)
 
     def wait_for_visibility(self, by, value, timeout=2, driver=None):

requirements-test.txt

@@ -1,5 +1,5 @@
 # For testing Dependency loaders
-openwisp_controller @ https://github.com/openwisp/openwisp-controller/tarball/master
+openwisp_controller @ https://github.com/openwisp/openwisp-controller/archive/refs/heads/master.tar.gz
 freezegun
 pytest
 pytest-mock

tests/test_project/tests/test_menu.py

@@ -88,6 +88,32 @@ def test_register_menu_groups(self):
             with self.assertRaises(ImproperlyConfigured):
                 register_menu_group(position=1, config=model_link_config)
 
+        with self.subTest(
+            "Registering identical config at an occupied position is idempotent"
+        ):
+            with self.assertLogs(
+                "openwisp_utils.admin_theme.menu", level="INFO"
+            ) as captured:
+                register_menu_group(position=1, config=menu_link_config)
+            self.assertIn("already registered", captured.output[0])
+            self.assertIsInstance(MENU[1], MenuLink)
+
+        with self.subTest("Re-registering identical ModelLink config is idempotent"):
+            with self.assertLogs(
+                "openwisp_utils.admin_theme.menu", level="INFO"
+            ) as captured:
+                register_menu_group(position=2, config=model_link_config)
+            self.assertIn("already registered", captured.output[0])
+            self.assertIsInstance(MENU[2], ModelLink)
+
+        with self.subTest("Re-registering identical MenuGroup config is idempotent"):
+            with self.assertLogs(
+                "openwisp_utils.admin_theme.menu", level="INFO"
+            ) as captured:
+                register_menu_group(position=3, config=menu_group_config)
+            self.assertIn("already registered", captured.output[0])
+            self.assertIsInstance(MENU[3], MenuGroup)
+
         with self.subTest("Registering with invalid position"):
             with self.assertRaises(ImproperlyConfigured):
                 register_menu_group(
@@ -143,7 +169,9 @@ def test_register_menu_subitem(self):
                 register_menu_subitem(group_position=100, item_position=1, config=[])
             with self.assertRaises(ImproperlyConfigured):
                 register_menu_subitem(
-                    group_position=100, item_position=1, config=config
+                    group_position=100,
+                    item_position=1,
+                    config=self._get_menu_link_config(label="a different label"),
                 )
             with self.assertRaises(ImproperlyConfigured):
                 register_menu_subitem(
@@ -156,6 +184,18 @@ def test_register_menu_subitem(self):
                     config=self._get_menu_group_config(),
                 )
 
+        with self.subTest(
+            "Registering identical config at an occupied item position is idempotent"
+        ):
+            with self.assertLogs(
+                "openwisp_utils.admin_theme.menu", level="INFO"
+            ) as captured:
+                register_menu_subitem(
+                    group_position=100, item_position=1, config=config
+                )
+            self.assertIn("already registered", captured.output[0])
+            self.assertIsInstance(MENU[100].items[1], MenuLink)
+
         with self.subTest("Test menu subitem with valid data"):
             model_link_config = self._get_model_link_config()
             register_menu_subitem(group_position=100, item_position=3, config=config)