From e5184a07e5acfd0e08305116c2cec978e521bb55 Mon Sep 17 00:00:00 2001 From: "M. Sumair Khokhar" Date: Wed, 6 Aug 2025 15:51:46 +0500 Subject: [PATCH] feat!: update finder method to work with django 42 and 52 (#37131) --- openedx/core/djangoapps/theming/finders.py | 15 +++++++++++++-- .../djangoapps/theming/tests/test_finders.py | 18 ++++++++++++++---- openedx/core/lib/xblock_pipeline/finder.py | 14 ++++++++++++-- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/theming/finders.py b/openedx/core/djangoapps/theming/finders.py index 3929111ab7..f75124efb1 100644 --- a/openedx/core/djangoapps/theming/finders.py +++ b/openedx/core/djangoapps/theming/finders.py @@ -65,10 +65,21 @@ class ThemeFilesFinder(BaseFinder): # lint-amnesty, pylint: disable=abstract-me for path in utils.get_files(storage, ignore_patterns): yield path, storage - def find(self, path, all=False): # pylint: disable=redefined-builtin + def find(self, path, *args, **kwargs): # pylint: disable=redefined-builtin """ Looks for files in the theme directories. """ + if 'all' in kwargs: + # Note this method signature where we accept all and find_all is being used so that we can be + # compatible with both Django 4.2 and Django 5.2 at the same time. After we have fully + # dropped Django 4.2 support, the method signature can be updated to just consume the + # `find_all` paramater. + find_all = kwargs.get('all', False) + elif 'find_all' in kwargs: + find_all = kwargs.get('find_all', False) + else: + find_all = args[0] if args else False + matches = [] theme_dir_name = path.split("/", 1)[0] @@ -79,7 +90,7 @@ class ThemeFilesFinder(BaseFinder): # lint-amnesty, pylint: disable=abstract-me path = "/".join(path.split("/")[1:]) match = self.find_in_theme(theme.theme_dir_name, path) if match: - if not all: + if not find_all: return match matches.append(match) return matches diff --git a/openedx/core/djangoapps/theming/tests/test_finders.py b/openedx/core/djangoapps/theming/tests/test_finders.py index d5e21f56da..f781f65795 100644 --- a/openedx/core/djangoapps/theming/tests/test_finders.py +++ b/openedx/core/djangoapps/theming/tests/test_finders.py @@ -35,14 +35,24 @@ class TestThemeFinders(TestCase): Verify Theme Finder returns themed assets """ themes_dir = settings.COMPREHENSIVE_THEME_DIRS[1] + expected_path = (((((themes_dir / 'test-theme') / 'lms') / 'static') / 'images') / 'logo.png') asset = "test-theme/images/logo.png" - matches = self.finder.find(asset, all=True) + matches_test_1 = self.finder.find(asset, True) + matches_test_2 = self.finder.find(asset, all=True) + matches_test_3 = self.finder.find(asset, find_all=True) - # Make sure only first match was returned - assert 1 == len(matches) + # 1 Make sure only first match was returned + assert 1 == len(matches_test_1) + assert matches_test_1[0] == expected_path - assert matches[0] == (((((themes_dir / 'test-theme') / 'lms') / 'static') / 'images') / 'logo.png') + # 2 Make sure only first match was returned + assert 1 == len(matches_test_2) + assert matches_test_2[0] == expected_path + + # 3 Make sure only first match was returned + assert 1 == len(matches_test_3) + assert matches_test_3[0] == expected_path def test_find_in_theme(self): """ diff --git a/openedx/core/lib/xblock_pipeline/finder.py b/openedx/core/lib/xblock_pipeline/finder.py index 4893e5e048..c4a0c4d478 100644 --- a/openedx/core/lib/xblock_pipeline/finder.py +++ b/openedx/core/lib/xblock_pipeline/finder.py @@ -144,15 +144,25 @@ class XBlockPipelineFinder(BaseFinder): # lint-amnesty, pylint: disable=abstrac for path in utils.get_files(storage, ignore_patterns): yield path, storage - def find(self, path, all=False): # pylint: disable=redefined-builtin + def find(self, path, *args, **kwargs): # pylint: disable=redefined-builtin """ Looks for files in the xblock package directories. """ + if 'all' in kwargs: + # Note this method signature where we accept all and find_all is being used so that we can be + # compatible with both Django 4.2 and Django 5.2 at the same time. After we have fully + # dropped Django 4.2 support, the method signature can be updated to just consume the + # `find_all` paramater. + find_all = kwargs.get('all', False) + elif 'find_all' in kwargs: + find_all = kwargs.get('find_all', False) + else: + find_all = args[0] if args else False matches = [] for storage in self.package_storages: if storage.exists(path): match = storage.path(path) - if not all: + if not find_all: return match matches.append(match) return matches