idlesign / django-sitetree

Reusable application for Django introducing site tree, menu and breadcrumbs navigation elements.
http://github.com/idlesign/django-sitetree
BSD 3-Clause "New" or "Revised" License
347 stars 132 forks source link

Query the AppConfig directly for its module name #292

Closed bignose-debian closed 3 years ago

bignose-debian commented 3 years ago

When sitetree.utils.import_project_sitetree_modules iterates applications for their module name, it currently uses the settings.INSTALLED_APPS sequence. But the entries in that sequence are not necessarily module names; each one might be the name of a class. Importing names from a class as though it is a package, will fail.

INSTALLED_APPS = [
    ...
    'foo',
    'bar.SpecialAppConfig',
    'baz',
]

From the documentation for INSTALLED_APPS:

Each string [in INSTALLED_APPS] should be a dotted Python path to:

  • an application configuration class (preferred), or
  • a package containing an application.

If any INSTALLED_APPS entry does not name a module, sitetree.utils.import_app_sitetree_module receives that name and attempts to use it as a module namespace. When the name is not the name of a module, this fails with ModuleNotFoundError.

  File "[…]/sitetree/utils.py", line 190, in import_project_sitetree_modules
    module = import_app_sitetree_module(app)
  File "[…]/sitetree/utils.py", line 169, in import_app_sitetree_module
    module = import_module(app)
  File "/usr/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 970, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'bar.SpecialAppConfig'; 'bar' is not a package

Instead, the iteration in sitetree.utils.import_project_sitetree_modules should use the functionality provided by the django.apps.apps registry, and directly use the AppConfig.name for its qualified module name.

bignose-debian commented 3 years ago

This change implements the correct behaviour on mine:

--- old/sitetree/utils.py
+++ new/sitetree/utils.py
@@ -186,8 +186,8 @@
     from django.conf import settings as django_settings

     submodules = []
-    for app in django_settings.INSTALLED_APPS:
-        module = import_app_sitetree_module(app)
+    for app_config in apps.app_configs.values():
+        module = import_app_sitetree_module(app_config.name)
         if module is not None:
             submodules.append(module)
idlesign commented 3 years ago

Thank you. I'll try to review in a week.

idlesign commented 3 years ago

Your patch has been incorporated in https://github.com/idlesign/django-sitetree/commit/8430b64e20f784b44c68cc03115c4fdee18fc7b0

Considered closed. Thank you.

bignose-debian commented 3 years ago

Your patch has been incorporated in 8430b64

Thank you!