From 9ffa4927a1bdb682ae59785105ae882ab0731fee Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Mon, 27 Oct 2025 16:39:59 +0500 Subject: [PATCH 1/6] fix: use configured storage_kwargs if available --- edxval/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/edxval/utils.py b/edxval/utils.py index 483bce94..7052c8a7 100644 --- a/edxval/utils.py +++ b/edxval/utils.py @@ -170,8 +170,9 @@ def get_configured_storage(settings_key): 'django.core.files.storage.FileSystemStorage' ) - # For Django 5.x, pick options if available - options = getattr(settings, 'STORAGES', {}).get('default', {}).get('OPTIONS', {}) + if not options: + # For Django 5.x, pick options if available + options = getattr(settings, 'STORAGES', {}).get('default', {}).get('OPTIONS', {}) # Import the storage class dynamically storage_class = import_string(storage_class_path) From 5119692a92ef6157a88c504546d9367f38ce7d0f Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Mon, 27 Oct 2025 19:30:46 +0500 Subject: [PATCH 2/6] test: tests added for configuration update and version bump --- edxval/__init__.py | 2 +- edxval/tests/test_storages.py | 41 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/edxval/__init__.py b/edxval/__init__.py index bddfdd5f..b5a91c28 100644 --- a/edxval/__init__.py +++ b/edxval/__init__.py @@ -2,4 +2,4 @@ init """ -__version__ = '3.1.0' +__version__ = '3.2.0' diff --git a/edxval/tests/test_storages.py b/edxval/tests/test_storages.py index 0814a221..94844d68 100644 --- a/edxval/tests/test_storages.py +++ b/edxval/tests/test_storages.py @@ -97,3 +97,44 @@ def test_video_transcript_without_storages_settings(self): 'django.core.files.storage.filesystem.FileSystemStorage', f"{storage_class.__module__}.{storage_class.__name__}", ) + + @override_settings(VIDEO_TRANSCRIPTS_SETTINGS={ + 'STORAGE_KWARGS': { + 'location': 'only/kwargs/' + } + }) + def test_video_transcript_with_storage_kwargs_only(self): + """ + Test case where STORAGE_CLASS is not defined but STORAGE_KWARGS is defined. + """ + storage = get_video_transcript_storage() + storage_class = storage.__class__ + + self.assertEqual( + 'django.core.files.storage.filesystem.FileSystemStorage', + f"{storage_class.__module__}.{storage_class.__name__}", + ) + + self.assertEqual(storage.base_location, 'only/kwargs/') + + @override_settings( + VIDEO_TRANSCRIPTS_SETTINGS={ + 'STORAGE_KWARGS': { + 'bucket_name': 'test-bucket', + 'default_acl': 'private', + 'location': 'only/kwargs/', + } + }, + DEFAULT_FILE_STORAGE='storages.backends.s3boto3.S3Boto3Storage' + ) + def test_video_transcript_with_storage_kwargs_only_and_default_storage(self): + """ + Test case where STORAGE_CLASS is not defined but STORAGE_KWARGS is defined + and DEFAULT_FILE_STORAGE is set to S3Boto3Storage in settings. + """ + storage = get_video_transcript_storage() + + self.assertIsInstance(storage, S3Boto3Storage) + self.assertEqual(storage.default_acl, 'private') + self.assertEqual(storage.bucket_name, 'test-bucket') + self.assertEqual(storage.location, 'only/kwargs/') From 1cf1f08cb2e8ff90c347a5f42ffc5fbabba51074 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Fri, 31 Oct 2025 17:58:39 +0500 Subject: [PATCH 3/6] fix: add precedence to django5.2 STORAGES config over custom settings --- edxval/tests/test_storages.py | 53 +++++++++++++---------------------- edxval/utils.py | 41 +++++++++++++++------------ 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/edxval/tests/test_storages.py b/edxval/tests/test_storages.py index 94844d68..b5cabff3 100644 --- a/edxval/tests/test_storages.py +++ b/edxval/tests/test_storages.py @@ -98,43 +98,28 @@ def test_video_transcript_without_storages_settings(self): f"{storage_class.__module__}.{storage_class.__name__}", ) - @override_settings(VIDEO_TRANSCRIPTS_SETTINGS={ + @override_settings(STORAGES={ + 'video_transcripts_settings': { + 'BACKEND': "storages.backends.s3boto3.S3Boto3Storage", + "OPTIONS": { + 'bucket_name': 'test', + 'default_acl': 'private', + 'location': 'abc/' + } + } + }, + VIDEO_TRANSCRIPTS_SETTINGS={ + 'STORAGE_CLASS': 'storages.backends.s3boto3.S3Boto3Storage', 'STORAGE_KWARGS': { - 'location': 'only/kwargs/' + 'bucket_name': 'custom-bucket', + 'default_acl': 'private', + 'location': 'custom/abc/' } }) - def test_video_transcript_with_storage_kwargs_only(self): - """ - Test case where STORAGE_CLASS is not defined but STORAGE_KWARGS is defined. - """ - storage = get_video_transcript_storage() - storage_class = storage.__class__ - - self.assertEqual( - 'django.core.files.storage.filesystem.FileSystemStorage', - f"{storage_class.__module__}.{storage_class.__name__}", - ) - - self.assertEqual(storage.base_location, 'only/kwargs/') - - @override_settings( - VIDEO_TRANSCRIPTS_SETTINGS={ - 'STORAGE_KWARGS': { - 'bucket_name': 'test-bucket', - 'default_acl': 'private', - 'location': 'only/kwargs/', - } - }, - DEFAULT_FILE_STORAGE='storages.backends.s3boto3.S3Boto3Storage' - ) - def test_video_transcript_with_storage_kwargs_only_and_default_storage(self): - """ - Test case where STORAGE_CLASS is not defined but STORAGE_KWARGS is defined - and DEFAULT_FILE_STORAGE is set to S3Boto3Storage in settings. - """ + def test_django52_storages_takes_priority_over_custom(self): + # storage = get_video_transcript_storage() - self.assertIsInstance(storage, S3Boto3Storage) + self.assertEqual(storage.bucket_name, "test") self.assertEqual(storage.default_acl, 'private') - self.assertEqual(storage.bucket_name, 'test-bucket') - self.assertEqual(storage.location, 'only/kwargs/') + self.assertEqual(storage.location, 'abc/') diff --git a/edxval/utils.py b/edxval/utils.py index 7052c8a7..7939c86d 100644 --- a/edxval/utils.py +++ b/edxval/utils.py @@ -153,26 +153,31 @@ def video_image_path(video_image_instance, filename): # pylint:disable=unused-a def get_configured_storage(settings_key): """ Generic function to return a configured Django storage backend - based on the settings dictionary at `settings_key`. This function falls - back to the `default` storage class if there is no `STORAGE_CLASS` entry - under the `setting_key` object. + based on the settings dictionary at `settings_key`. This function prioritizes + Django 5.2's STORAGES dictionary over custom settings with the same key, + falling back to legacy settings if no STORAGES entry exists. """ - config = getattr(settings, settings_key, {}) - # Retrieve the storage class path and kwargs from the settings - storage_class_path = config.get('STORAGE_CLASS') - options = config.get('STORAGE_KWARGS', {}) - - # following code only runs for default storages - if not storage_class_path: - storage_class_path = ( - getattr(settings, 'DEFAULT_FILE_STORAGE', None) or - getattr(settings, 'STORAGES', {}).get('default', {}).get('BACKEND') or - 'django.core.files.storage.FileSystemStorage' - ) - if not options: - # For Django 5.x, pick options if available - options = getattr(settings, 'STORAGES', {}).get('default', {}).get('OPTIONS', {}) + storages_config = getattr(settings, 'STORAGES', {}) + storage_settings = storages_config.get(settings_key.lower(), {}) + + if storage_settings: + storage_class_path = storage_settings.get('BACKEND') + options = storage_settings.get('OPTIONS', {}) + else: + # Fall back to custom settings configuration + config = getattr(settings, settings_key, {}) + storage_class_path = config.get('STORAGE_CLASS') + options = config.get('STORAGE_KWARGS', {}) + + if not storage_class_path: + storage_class_path = ( + storages_config.get('default', {}).get('BACKEND') or + getattr(settings, 'DEFAULT_FILE_STORAGE', None) or + 'django.core.files.storage.FileSystemStorage' + ) + + options = storages_config.get('default', {}).get('OPTIONS', {}) or options # Import the storage class dynamically storage_class = import_string(storage_class_path) From 54ecba1ccfa9093f8d1ef2238078ac2f10140cfe Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Mon, 10 Nov 2025 15:56:12 +0500 Subject: [PATCH 4/6] fix: fix name for STORAGES to video_transcripts_storage --- edxval/tests/test_storages.py | 2 +- edxval/utils.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/edxval/tests/test_storages.py b/edxval/tests/test_storages.py index b5cabff3..c5849015 100644 --- a/edxval/tests/test_storages.py +++ b/edxval/tests/test_storages.py @@ -99,7 +99,7 @@ def test_video_transcript_without_storages_settings(self): ) @override_settings(STORAGES={ - 'video_transcripts_settings': { + 'video_transcripts_storage': { 'BACKEND': "storages.backends.s3boto3.S3Boto3Storage", "OPTIONS": { 'bucket_name': 'test', diff --git a/edxval/utils.py b/edxval/utils.py index 7939c86d..75261f0a 100644 --- a/edxval/utils.py +++ b/edxval/utils.py @@ -159,7 +159,8 @@ def get_configured_storage(settings_key): """ storages_config = getattr(settings, 'STORAGES', {}) - storage_settings = storages_config.get(settings_key.lower(), {}) + storage_key = settings_key.replace("SETTINGS", "storage") + storage_settings = storages_config.get(storage_key.lower(), {}) if storage_settings: storage_class_path = storage_settings.get('BACKEND') From 8401fd093d72f21c6bc7d6842e36d3b5b4f1deed Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Tue, 18 Nov 2025 17:33:30 +0500 Subject: [PATCH 5/6] fix: linting error fixed --- edxval/tests/test_storages.py | 3 +-- edxval/utils.py | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/edxval/tests/test_storages.py b/edxval/tests/test_storages.py index c5849015..8b94cbf1 100644 --- a/edxval/tests/test_storages.py +++ b/edxval/tests/test_storages.py @@ -107,8 +107,7 @@ def test_video_transcript_without_storages_settings(self): 'location': 'abc/' } } - }, - VIDEO_TRANSCRIPTS_SETTINGS={ + }, VIDEO_TRANSCRIPTS_SETTINGS={ 'STORAGE_CLASS': 'storages.backends.s3boto3.S3Boto3Storage', 'STORAGE_KWARGS': { 'bucket_name': 'custom-bucket', diff --git a/edxval/utils.py b/edxval/utils.py index 75261f0a..560ac196 100644 --- a/edxval/utils.py +++ b/edxval/utils.py @@ -157,7 +157,6 @@ def get_configured_storage(settings_key): Django 5.2's STORAGES dictionary over custom settings with the same key, falling back to legacy settings if no STORAGES entry exists. """ - storages_config = getattr(settings, 'STORAGES', {}) storage_key = settings_key.replace("SETTINGS", "storage") storage_settings = storages_config.get(storage_key.lower(), {}) From 71df494d35747f32ddfd7373f3d6e74b164900d4 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Abdul Rauf Date: Wed, 19 Nov 2025 19:14:08 +0500 Subject: [PATCH 6/6] fix: setting name updated for STORAGES --- edxval/tests/test_storages.py | 2 +- edxval/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/edxval/tests/test_storages.py b/edxval/tests/test_storages.py index 8b94cbf1..0f9d627f 100644 --- a/edxval/tests/test_storages.py +++ b/edxval/tests/test_storages.py @@ -99,7 +99,7 @@ def test_video_transcript_without_storages_settings(self): ) @override_settings(STORAGES={ - 'video_transcripts_storage': { + 'video_transcripts': { 'BACKEND': "storages.backends.s3boto3.S3Boto3Storage", "OPTIONS": { 'bucket_name': 'test', diff --git a/edxval/utils.py b/edxval/utils.py index 560ac196..757c51bb 100644 --- a/edxval/utils.py +++ b/edxval/utils.py @@ -158,7 +158,7 @@ def get_configured_storage(settings_key): falling back to legacy settings if no STORAGES entry exists. """ storages_config = getattr(settings, 'STORAGES', {}) - storage_key = settings_key.replace("SETTINGS", "storage") + storage_key = settings_key.replace("_SETTINGS", "") storage_settings = storages_config.get(storage_key.lower(), {}) if storage_settings: