openwisp-users icon indicating copy to clipboard operation
openwisp-users copied to clipboard

[bug] Cannot initialize openwisp_users app multiple times

Open asmodehn opened this issue 6 months ago • 2 comments

Describe the bug Initializing the app multiple times gives error. Although this is not a main usecase, it can happen when running tests that override settings.

I noticed it when I tried to override installed apps to test migrations between two databases (created with slightly different settings).

Steps To Reproduce Add a test which is overriding the INSTALLED_APPS setting.

For instance:

from django.test import TestCase
from django.test.utils import modify_settings

class TestDB(TestCase):

    @modify_settings(INSTALLED_APPS={'remove': 'django_filters'})
    def fake_test(self):
        pass

Expected behavior

The app should be able to be initialize multiple times, so that test example should simply pass.

Screenshots

$ ./manage.py test testapp.tests.test_db.TestDB.fake_test
Found 1 test(s).
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
E
======================================================================
ERROR: fake_test (testapp.tests.test_db.TestDB.fake_test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/alexv/Projects/openwisp-users/.direnv/python-3.12/lib/python3.12/site-packages/django/test/utils.py", line 458, in inner
    with self as context:
         ^^^^
  File "/home/alexv/Projects/openwisp-users/.direnv/python-3.12/lib/python3.12/site-packages/django/test/utils.py", line 423, in __enter__
    return self.enable()
           ^^^^^^^^^^^^^
  File "/home/alexv/Projects/openwisp-users/.direnv/python-3.12/lib/python3.12/site-packages/django/test/utils.py", line 605, in enable
    super().enable()
  File "/home/alexv/Projects/openwisp-users/.direnv/python-3.12/lib/python3.12/site-packages/django/test/utils.py", line 492, in enable
    apps.set_installed_apps(self.options["INSTALLED_APPS"])
  File "/home/alexv/Projects/openwisp-users/.direnv/python-3.12/lib/python3.12/site-packages/django/apps/registry.py", line 362, in set_installed_apps
    self.populate(installed)
  File "/home/alexv/Projects/openwisp-users/.direnv/python-3.12/lib/python3.12/site-packages/django/apps/registry.py", line 124, in populate
    app_config.ready()
  File "/home/alexv/Projects/openwisp-users/openwisp_users/apps.py", line 27, in ready
    self.register_menu_group()
  File "/home/alexv/Projects/openwisp-users/openwisp_users/apps.py", line 66, in register_menu_group
    register_menu_group(
  File "/home/alexv/Projects/openwisp-users/.direnv/python-3.12/lib/python3.12/site-packages/openwisp_utils/admin_theme/menu.py", line 192, in register_menu_group
    raise ImproperlyConfigured(
django.core.exceptions.ImproperlyConfigured: A group/link with config {'label': 'Users & Organizations', 'items': {1: {'label': 'Users', 'model': 'openwisp_users.User', 'name': 'changelist', 'icon': 'user'}, 2: {'label': 'Organizations', 'model': 'openwisp_users.Organization', 'name': 'changelist', 'icon': 'ow-org'}, 3: {'label': 'Groups & Permissions', 'model': 'openwisp_users.Group', 'name': 'changelist', 'icon': 'ow-permission'}, 4: {'label': 'Organization Owners', 'model': 'openwisp_users.OrganizationOwner', 'name': 'changelist', 'icon': 'ow-org-owner'}, 5: {'label': 'Organization Users', 'model': 'openwisp_users.OrganizationUser', 'name': 'changelist', 'icon': 'ow-org-user'}}, 'icon': 'ow-user-and-org'} is being registered at position "40",                but another group named "Users & Organizations" is already registered at the same position.

----------------------------------------------------------------------
Ran 1 test in 0.003s

System Informatioon:

  • OS:
$ cat /etc/issue
Pop!_OS 22.04 LTS
  • Python Version: Python 3.12.7
$ pip list | grep 'openwisp\|django'
django-admin-autocomplete-filter 0.7.1
django-allauth                   65.7.0
django-compress-staticfiles      1.0.1b0
django-extensions                4.1
django-filter                    23.5
django-model-utils               5.0.0
django-organizations             2.5.0
django-phonenumber-field         8.1.0
django-redis                     5.4.0
django-reversion                 5.1.0
django-sesame                    3.2.3
djangorestframework              3.15.2
openwisp-users                   1.2.0a0     /home/alexv/Projects/openwisp-users
openwisp-utils                   1.2a0

asmodehn avatar May 15 '25 12:05 asmodehn

@asmodehn is it the modify_settings which causes this?

Maybe we can print a warning instead of raising ImproperlyConfigured here: https://github.com/openwisp/openwisp-utils/blob/master/openwisp_utils/admin_theme/menu.py#L192-L195

nemesifier avatar May 15 '25 14:05 nemesifier

@nemesifier The error is still useful to have to force the user to debug issues with the menu, which can occur when working with extended apps...

What I ended up doing to workaround this in my app is :

  • import MENU dynamically
  • check if the position is already assigned to something, if so, skip the register_menu_group() call. Not sure if it is a good way, but it seemed to work (I wonder about the import cache tho...), and the code is directly visible in the apps module.

Maybe we could have a check there, that verify this is the same app, wanting to register the same group in the same place ? But I m not sure we have all the information at this level...

Thinking about this, maybe BaseMenuItem could have some kind of id or == operator, that checks the config to determine id / equality. Then we could use this in register_menu_group to skip it, if it is already in the menu at that position...

It would work, if the config parameter is actually enough (and not too "vague") to determine if we want to error or silently ignore: MenuItemA == MenuItemB <=> we can skip the register, it is already in MENU, and it is the "exact same" MenuItem.

asmodehn avatar May 15 '25 16:05 asmodehn