From 5fe131c85890857673e41e12a144ec213c91af26 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Thu, 11 Apr 2024 12:04:35 -0400 Subject: [PATCH] fix: `--theme-dirs` argument to `compile_sass` management command 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. --- openedx/core/djangoapps/theming/helpers.py | 8 +++++--- .../theming/management/commands/compile_sass.py | 2 +- openedx/core/djangoapps/theming/tests/test_helpers.py | 1 + 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/theming/helpers.py b/openedx/core/djangoapps/theming/helpers.py index 4d1df6418082..b88bbdca4382 100644 --- a/openedx/core/djangoapps/theming/helpers.py +++ b/openedx/core/djangoapps/theming/helpers.py @@ -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(): diff --git a/openedx/core/djangoapps/theming/management/commands/compile_sass.py b/openedx/core/djangoapps/theming/management/commands/compile_sass.py index 0e193fb10174..765ef98aeac3 100644 --- a/openedx/core/djangoapps/theming/management/commands/compile_sass.py +++ b/openedx/core/djangoapps/theming/management/commands/compile_sass.py @@ -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()} diff --git a/openedx/core/djangoapps/theming/tests/test_helpers.py b/openedx/core/djangoapps/theming/tests/test_helpers.py index 955e723df145..95829a16b416 100644 --- a/openedx/core/djangoapps/theming/tests/test_helpers.py +++ b/openedx/core/djangoapps/theming/tests/test_helpers.py @@ -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)