Skip to content

Commit

Permalink
fix: --theme-dirs argument to compile_sass management command
Browse files Browse the repository at this point in the history
This fixes the ability to pass custom theme directories to
the management command which compiles site themes, a la:

   ./manage.py lms compile_sass --theme-dirs /my/custom/themes/dir

The exception, which was due to a incompatible use of @lru_cache, was:

   File "openedx/core/djangoapps/theming/management/commands/compile_sass.py",
   line 93, in parse_arguments:
     available_themes.update({t.theme_dir_name: t for t in get_themes([theme_dir])})
   TypeError: unhashable type: 'list'

This has been broken since the @lru_cache decorator was added, but it
wasn't noticed because:

* We weren't compiling any comprehensive themes in CI.
* Tutor supports compehensive theming, but not *site theming*, so
  it doesn't use this management command at all
  (site themeing == comp theming * site configuration).
* Although edx.org executes this management command, it does not provide
  use the `--theme-dirs` argument, so the bug was not hit.
  • Loading branch information
kdmccormick committed Apr 12, 2024
1 parent 0d52e37 commit 5fe131c
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 4 deletions.
8 changes: 5 additions & 3 deletions openedx/core/djangoapps/theming/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,11 @@ def get_themes(themes_dir=None):
"""
if not is_comprehensive_theming_enabled():
return []
if themes_dir is None:
themes_dir = get_theme_base_dirs_unchecked()
return get_themes_unchecked(themes_dir, settings.PROJECT_ROOT)
if themes_dir:
themes_dirs = [themes_dir]
else:
themes_dirs = get_theme_base_dirs_unchecked()
return get_themes_unchecked(themes_dirs, settings.PROJECT_ROOT)


def get_theme_base_dirs_unchecked():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def parse_arguments(*args, **options): # pylint: disable=unused-argument
if theme_dirs:
available_themes = {}
for theme_dir in theme_dirs:
available_themes.update({t.theme_dir_name: t for t in get_themes([theme_dir])})
available_themes.update({t.theme_dir_name: t for t in get_themes(theme_dir)})
else:
theme_dirs = get_theme_base_dirs()
available_themes = {t.theme_dir_name: t for t in get_themes()}
Expand Down
1 change: 1 addition & 0 deletions openedx/core/djangoapps/theming/tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def test_get_themes(self):
Theme('red-theme', 'red-theme', get_theme_base_dir('red-theme'), settings.PROJECT_ROOT),
Theme('stanford-style', 'stanford-style', get_theme_base_dir('stanford-style'), settings.PROJECT_ROOT),
Theme('test-theme', 'test-theme', get_theme_base_dir('test-theme'), settings.PROJECT_ROOT),
Theme('empty-theme', 'empty-theme', get_theme_base_dir('empty-theme'), settings.PROJECT_ROOT),
]
actual_themes = get_themes()
self.assertCountEqual(expected_themes, actual_themes)
Expand Down

0 comments on commit 5fe131c

Please sign in to comment.