From 567c1cac782e4f2e74b54f1daaef89e275979ed9 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 29 Apr 2025 20:16:21 +0500 Subject: [PATCH 01/22] feat!: upgrade codebase for compatibility with Django 4.2 and 5.2 Replaced usage of `get_storage_class`, which was deprecated in Django 4.2 and removed in 5.1, with `import_string` to support dynamic storage class loading. --- .../user_api/accounts/image_helpers.py | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 43aa4a60aeb7..cdf3d080f418 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -8,7 +8,7 @@ from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage from django.core.exceptions import ObjectDoesNotExist -from django.core.files.storage import get_storage_class +from django.utils.module_loading import import_string from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from common.djangoapps.student.models import UserProfile @@ -22,12 +22,23 @@ def get_profile_image_storage(): """ - Configures and returns a django Storage instance that can be used - to physically locate, read and write profile images. + Returns a configured Django Storage instance for handling profile images. + The storage backend is defined in the Django setting `PROFILE_IMAGE_BACKEND`, + which should be a dictionary with the following structure: + { + 'class': 'full.path.to.StorageClass', + 'options': { + # Optional keyword arguments passed to the storage class constructor + } + } + + This function dynamically loads the specified storage class and initializes it + with the provided options. """ config = settings.PROFILE_IMAGE_BACKEND - storage_class = get_storage_class(config['class']) - return storage_class(**config['options']) + storage_class_path = config.get('class') + storage_class = import_string(storage_class_path) + return storage_class(**config.get('options', {})) def _make_profile_image_name(username): From 1a76237d188f73ea0ff1a84d739838e54269062e Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Fri, 2 May 2025 18:11:25 +0500 Subject: [PATCH 02/22] fix: updating code. --- .../user_api/accounts/image_helpers.py | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index cdf3d080f418..18b039a00ae6 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -22,23 +22,35 @@ def get_profile_image_storage(): """ - Returns a configured Django Storage instance for handling profile images. - The storage backend is defined in the Django setting `PROFILE_IMAGE_BACKEND`, - which should be a dictionary with the following structure: - { - 'class': 'full.path.to.StorageClass', - 'options': { - # Optional keyword arguments passed to the storage class constructor - } + Returns a configured Django Storage instance for handling profile images. + + The storage backend is defined in the Django setting `PROFILE_IMAGE_BACKEND`, + which should be a dictionary with the following structure: + { + 'class': 'full.path.to.StorageClass', + 'options': { + # Optional keyword arguments passed to the storage class constructor } + } - This function dynamically loads the specified storage class and initializes it - with the provided options. + If `PROFILE_IMAGE_BACKEND` is not defined or does not include a 'class' key, + the default storage backend defined in `DEFAULT_FILE_STORAGE` is used. + + Returns: + An instance of the configured Django storage class. """ - config = settings.PROFILE_IMAGE_BACKEND - storage_class_path = config.get('class') + + if hasattr(settings, 'PROFILE_IMAGE_BACKEND') and 'class' in settings.PROFILE_IMAGE_BACKEND: + storage_class_path = settings.PROFILE_IMAGE_BACKEND['class'] + options = settings.PROFILE_IMAGE_BACKEND.get('options', {}) + else: + storage_class_path = getattr( + settings, 'DEFAULT_FILE_STORAGE', 'django.core.files.storage.FileSystemStorage' + ) + options = {} + storage_class = import_string(storage_class_path) - return storage_class(**config.get('options', {})) + return storage_class(**options) def _make_profile_image_name(username): From ef4fc2527b1c1b31c1ca0e46baf3235ff9ef64c1 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Fri, 2 May 2025 18:56:54 +0500 Subject: [PATCH 03/22] fix: updating code. --- .../djangoapps/user_api/accounts/image_helpers.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 18b039a00ae6..fee16362058e 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -39,14 +39,13 @@ def get_profile_image_storage(): Returns: An instance of the configured Django storage class. """ + config = getattr(settings, 'PROFILE_IMAGE_BACKEND', None) - if hasattr(settings, 'PROFILE_IMAGE_BACKEND') and 'class' in settings.PROFILE_IMAGE_BACKEND: - storage_class_path = settings.PROFILE_IMAGE_BACKEND['class'] - options = settings.PROFILE_IMAGE_BACKEND.get('options', {}) + if config and isinstance(config, dict): + storage_class_path = config.get('class') or config.get('STORAGE_CLASS') + options = config.get('options') or config.get('STORAGE_KWARGS', {}) else: - storage_class_path = getattr( - settings, 'DEFAULT_FILE_STORAGE', 'django.core.files.storage.FileSystemStorage' - ) + storage_class_path = getattr(settings, 'DEFAULT_FILE_STORAGE', 'django.core.files.storage.FileSystemStorage') options = {} storage_class = import_string(storage_class_path) From 7e2767089af7c406684aec5ac17929b7591236d2 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Sat, 3 May 2025 20:51:00 +0500 Subject: [PATCH 04/22] fix: adding tests. --- .../user_api/accounts/image_helpers.py | 16 ++++----- .../user_api/accounts/tests/test_views.py | 34 +++++++++++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index fee16362058e..dc9b5d211a9d 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -39,17 +39,17 @@ def get_profile_image_storage(): Returns: An instance of the configured Django storage class. """ - config = getattr(settings, 'PROFILE_IMAGE_BACKEND', None) + config = settings.PROFILE_IMAGE_BACKEND + storage_class_path = config.get('class') - if config and isinstance(config, dict): - storage_class_path = config.get('class') or config.get('STORAGE_CLASS') - options = config.get('options') or config.get('STORAGE_KWARGS', {}) - else: - storage_class_path = getattr(settings, 'DEFAULT_FILE_STORAGE', 'django.core.files.storage.FileSystemStorage') - options = {} + if not storage_class_path: + # for Django==4.2 DEFAULT_FILE_STORAGE exists but with django5.2 need to add dict `STORAGES` in settings + storage_class_path = getattr( + settings, 'DEFAULT_FILE_STORAGE', 'django.core.files.storage.FileSystemStorage' + ) storage_class = import_string(storage_class_path) - return storage_class(**options) + return storage_class(**config.get('options')) def _make_profile_image_name(username): diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index ff0fb7abe4eb..82a1a9bd6934 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -12,11 +12,13 @@ import ddt import pytz from django.conf import settings +from django.core.files.storage import FileSystemStorage from django.test.testcases import TransactionTestCase from django.test.utils import override_settings from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient, APITestCase +from storages.backends.s3boto3 import S3Boto3Storage from common.djangoapps.student.models import PendingEmailChange, UserProfile from common.djangoapps.student.models_api import do_name_change_request, get_pending_name_change @@ -33,6 +35,7 @@ RetirementStateFactory, UserRetirementStatusFactory ) +from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage from openedx.core.djangoapps.user_api.models import UserPreference, UserRetirementStatus from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES @@ -1156,6 +1159,37 @@ def test_patch_serializer_save_fails(self, serializer_save): assert "Error thrown when saving account updates: 'bummer'" == error_response.data['developer_message'] assert error_response.data['user_message'] is None + def test_profile_image_backend(self): + # settings file contains the `VIDEO_IMAGE_SETTINGS` but dont'have STORAGE_CLASS + # so it returns the default storage. + storage = get_profile_image_storage() + storage_class = storage.__class__ + self.assertEqual( + settings.PROFILE_IMAGE_BACKEND['class'], + f"{storage_class.__module__}.{storage_class.__name__}", + ) + self.assertEqual(storage.base_url, settings.PROFILE_IMAGE_BACKEND['options']['base_url']) + + @override_settings(PROFILE_IMAGE_BACKEND={ + 'class': 'storages.backends.s3boto3.S3Boto3Storage', + 'options': { + 'bucket_name': 'test', + 'default_acl': 'public', + 'location': 'abc/def' + } + }) + def test_profile_backend_with_params(self): + storage = get_profile_image_storage() + self.assertIsInstance(storage, S3Boto3Storage) + self.assertEqual(storage.bucket_name, "test") + self.assertEqual(storage.default_acl, 'public') + self.assertEqual(storage.location, "abc/def") + + @override_settings(PROFILE_IMAGE_BACKEND={'class': None, 'options':{}}) + def test_profile_backend_without_backend(self): + storage = get_profile_image_storage() + self.assertIsInstance(storage, FileSystemStorage) + @override_settings(PROFILE_IMAGE_BACKEND=TEST_PROFILE_IMAGE_BACKEND) def test_convert_relative_profile_url(self): """ From 5b70ad8e7c025722c7c841022ad4307e81081778 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Sun, 4 May 2025 01:21:14 +0500 Subject: [PATCH 05/22] fix: adding tests. --- .../core/djangoapps/user_api/accounts/image_helpers.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index dc9b5d211a9d..3af4bbf5ddcb 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -40,14 +40,7 @@ def get_profile_image_storage(): An instance of the configured Django storage class. """ config = settings.PROFILE_IMAGE_BACKEND - storage_class_path = config.get('class') - - if not storage_class_path: - # for Django==4.2 DEFAULT_FILE_STORAGE exists but with django5.2 need to add dict `STORAGES` in settings - storage_class_path = getattr( - settings, 'DEFAULT_FILE_STORAGE', 'django.core.files.storage.FileSystemStorage' - ) - + storage_class_path = config.get('class') or getattr(settings, 'DEFAULT_FILE_STORAGE') storage_class = import_string(storage_class_path) return storage_class(**config.get('options')) From 8ba4bb1f315c4ea35a3b654c0bb2ab8c8219d7b6 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Sun, 4 May 2025 01:25:14 +0500 Subject: [PATCH 06/22] fix: adding tests. --- .../user_api/accounts/image_helpers.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 3af4bbf5ddcb..422049b2efdf 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -22,22 +22,15 @@ def get_profile_image_storage(): """ - Returns a configured Django Storage instance for handling profile images. + Returns an instance of the configured Django storage class for profile images. - The storage backend is defined in the Django setting `PROFILE_IMAGE_BACKEND`, - which should be a dictionary with the following structure: - { - 'class': 'full.path.to.StorageClass', - 'options': { - # Optional keyword arguments passed to the storage class constructor - } - } - - If `PROFILE_IMAGE_BACKEND` is not defined or does not include a 'class' key, - the default storage backend defined in `DEFAULT_FILE_STORAGE` is used. + The function looks for the `PROFILE_IMAGE_BACKEND` setting in the Django settings. + If it exists and includes a `'class'` key, that class is used as the storage backend. + If the setting is missing or does not include a `'class'`, the default storage backend + DEFAULT_FILE_STORAGE will return. Returns: - An instance of the configured Django storage class. + An instance of the configured storage backend. """ config = settings.PROFILE_IMAGE_BACKEND storage_class_path = config.get('class') or getattr(settings, 'DEFAULT_FILE_STORAGE') From 1075cc85b6430f6202ece1e1e13288113c6f5d64 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Tue, 6 May 2025 15:16:50 +0500 Subject: [PATCH 07/22] fix: Update image_helpers.py --- openedx/core/djangoapps/user_api/accounts/image_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 422049b2efdf..92597f3330ab 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -33,7 +33,7 @@ def get_profile_image_storage(): An instance of the configured storage backend. """ config = settings.PROFILE_IMAGE_BACKEND - storage_class_path = config.get('class') or getattr(settings, 'DEFAULT_FILE_STORAGE') + storage_class_path = config.get('class', 'django.core.files.storage.FileSystemStorage') storage_class = import_string(storage_class_path) return storage_class(**config.get('options')) From 05bd1d3919fac22620327fb50f322084aa726661 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 6 May 2025 15:26:44 +0500 Subject: [PATCH 08/22] fix: fixing code. --- .../core/djangoapps/user_api/accounts/image_helpers.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 92597f3330ab..6195e3ae3182 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -10,8 +10,8 @@ from django.core.exceptions import ObjectDoesNotExist from django.utils.module_loading import import_string -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from common.djangoapps.student.models import UserProfile +from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from ..errors import UserNotFound @@ -32,10 +32,12 @@ def get_profile_image_storage(): Returns: An instance of the configured storage backend. """ - config = settings.PROFILE_IMAGE_BACKEND - storage_class_path = config.get('class', 'django.core.files.storage.FileSystemStorage') + config = getattr(settings, 'PROFILE_IMAGE_BACKEND', {}) + storage_class_path = config.get('class') or getattr( + settings, 'DEFAULT_FILE_STORAGE', 'django.core.files.storage.FileSystemStorage' + ) storage_class = import_string(storage_class_path) - return storage_class(**config.get('options')) + return storage_class(**config.get('options', {})) def _make_profile_image_name(username): From 03e8c83250a4460dede0a23f504d346964228633 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 6 May 2025 16:16:28 +0500 Subject: [PATCH 09/22] fix: fixing code. --- .../user_api/accounts/image_helpers.py | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 6195e3ae3182..59e82ec03edd 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -5,8 +5,10 @@ import hashlib +from django import VERSION from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage +from django.core.files.storage import storages from django.core.exceptions import ObjectDoesNotExist from django.utils.module_loading import import_string @@ -33,11 +35,22 @@ def get_profile_image_storage(): An instance of the configured storage backend. """ config = getattr(settings, 'PROFILE_IMAGE_BACKEND', {}) - storage_class_path = config.get('class') or getattr( - settings, 'DEFAULT_FILE_STORAGE', 'django.core.files.storage.FileSystemStorage' - ) + storage_class_path = config.get('class') + options = config.get('options', {}) + + if not storage_class_path: + # Django 5.x STORAGES fallback + if django_version[0] >= 5 and hasattr(settings, 'STORAGES') and 'default' in settings.STORAGES: + return storages['default'] + else: + # Legacy fallback + storage_class_path = getattr( + settings, 'DEFAULT_FILE_STORAGE', + 'django.core.files.storage.FileSystemStorage' + ) + storage_class = import_string(storage_class_path) - return storage_class(**config.get('options', {})) + return storage_class(**options) def _make_profile_image_name(username): From 3f30323b1a2ded9c79945f662758c0ae55dbec81 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Thu, 8 May 2025 15:19:41 +0500 Subject: [PATCH 10/22] fix: fixing code. --- .../user_api/accounts/image_helpers.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 59e82ec03edd..f7d4e6eddbd2 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -39,15 +39,14 @@ def get_profile_image_storage(): options = config.get('options', {}) if not storage_class_path: - # Django 5.x STORAGES fallback - if django_version[0] >= 5 and hasattr(settings, 'STORAGES') and 'default' in settings.STORAGES: - return storages['default'] - else: - # Legacy fallback - storage_class_path = getattr( - settings, 'DEFAULT_FILE_STORAGE', - 'django.core.files.storage.FileSystemStorage' - ) + storage_class_path = ( + getattr(settings, 'DEFAULT_FILE_STORAGE', None) or + getattr(settings, 'STORAGES', {}).get('default', {}).get('BACKEND') or + 'django.core.files.storage.FileSystemStorage' + ) + + # For Django 5.x, pick options if available + options = getattr(settings, 'STORAGES', {}).get('default', {}).get('OPTIONS', {}) storage_class = import_string(storage_class_path) return storage_class(**options) From b9d27bece7078f076536ff5daa9f2ff74296e0ec Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Tue, 13 May 2025 15:45:39 +0500 Subject: [PATCH 11/22] chore: fix quality. --- openedx/core/djangoapps/user_api/accounts/tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 82a1a9bd6934..997e3a482f99 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -1185,7 +1185,7 @@ def test_profile_backend_with_params(self): self.assertEqual(storage.default_acl, 'public') self.assertEqual(storage.location, "abc/def") - @override_settings(PROFILE_IMAGE_BACKEND={'class': None, 'options':{}}) + @override_settings(PROFILE_IMAGE_BACKEND={'class': None, 'options': {}}) def test_profile_backend_without_backend(self): storage = get_profile_image_storage() self.assertIsInstance(storage, FileSystemStorage) From 58ce3de7a0b44f774b48490b33b92f3cca3884ae Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Tue, 13 May 2025 15:46:08 +0500 Subject: [PATCH 12/22] chore: Update image_helpers.py --- openedx/core/djangoapps/user_api/accounts/image_helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index f7d4e6eddbd2..80ed6581412a 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -5,7 +5,6 @@ import hashlib -from django import VERSION from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage from django.core.files.storage import storages From 04e341de7c59511e3ec3fc3878e025a7949b5548 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Tue, 13 May 2025 17:52:39 +0500 Subject: [PATCH 13/22] chore: Update image_helpers.py --- openedx/core/djangoapps/user_api/accounts/image_helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 80ed6581412a..9b904484f820 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -7,7 +7,6 @@ from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage -from django.core.files.storage import storages from django.core.exceptions import ObjectDoesNotExist from django.utils.module_loading import import_string From 54d3f245ddc971cc53fa663040dd2e1abe63ec65 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Thu, 15 May 2025 17:03:33 +0500 Subject: [PATCH 14/22] chore: Update image_helpers.py --- openedx/core/djangoapps/user_api/accounts/image_helpers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 9b904484f820..1cc398a46eb4 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -38,6 +38,7 @@ def get_profile_image_storage(): if not storage_class_path: storage_class_path = ( + getattr(settings, 'STORAGES', {}).get('profile_images', {}).get('BACKEND') or # named storages getattr(settings, 'DEFAULT_FILE_STORAGE', None) or getattr(settings, 'STORAGES', {}).get('default', {}).get('BACKEND') or 'django.core.files.storage.FileSystemStorage' From 1561a84f15b17b6a189f0ccf4cf4bcb867b412f0 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Thu, 15 May 2025 18:56:08 +0500 Subject: [PATCH 15/22] chore: Update image_helpers.py --- openedx/core/djangoapps/user_api/accounts/image_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 1cc398a46eb4..15e309a5a576 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -38,7 +38,7 @@ def get_profile_image_storage(): if not storage_class_path: storage_class_path = ( - getattr(settings, 'STORAGES', {}).get('profile_images', {}).get('BACKEND') or # named storages + getattr(settings, 'STORAGES', {}).get('profile_images', {}).get('BACKEND') or # custom named storages getattr(settings, 'DEFAULT_FILE_STORAGE', None) or getattr(settings, 'STORAGES', {}).get('default', {}).get('BACKEND') or 'django.core.files.storage.FileSystemStorage' From 18bfa4cdbd7e056d71affbd520fb602614f473f8 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Fri, 16 May 2025 12:39:00 +0500 Subject: [PATCH 16/22] chore: adding future settings options. --- .../user_api/accounts/image_helpers.py | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 15e309a5a576..2f980ac637df 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -37,15 +37,18 @@ def get_profile_image_storage(): options = config.get('options', {}) if not storage_class_path: - storage_class_path = ( - getattr(settings, 'STORAGES', {}).get('profile_images', {}).get('BACKEND') or # custom named storages - getattr(settings, 'DEFAULT_FILE_STORAGE', None) or - getattr(settings, 'STORAGES', {}).get('default', {}).get('BACKEND') or - 'django.core.files.storage.FileSystemStorage' - ) - - # For Django 5.x, pick options if available - options = getattr(settings, 'STORAGES', {}).get('default', {}).get('OPTIONS', {}) + storages_config = getattr(settings, 'STORAGES', {}) + + if 'profile_images' in storages_config: + storage_class_path = storages_config['profile_images'].get('BACKEND') + options = storages_config['profile_images'].get('OPTIONS', {}) + elif 'default' in storages_config: + storage_class_path = storages_config['default'].get('BACKEND') + options = storages_config['default'].get('OPTIONS', {}) + else: + storage_class_path = getattr(settings, 'DEFAULT_FILE_STORAGE', None) or \ + 'django.core.files.storage.FileSystemStorage' + options = {} storage_class = import_string(storage_class_path) return storage_class(**options) From a7790f26308d4828cdabc988c6e091a16c587ce6 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Fri, 16 May 2025 12:40:41 +0500 Subject: [PATCH 17/22] chore: fixing tests. --- .../user_api/accounts/tests/test_views.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 997e3a482f99..959701665450 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -1204,6 +1204,37 @@ def test_convert_relative_profile_url(self): 'image_url_full': 'http://testserver/static/default_50.png', 'image_url_small': 'http://testserver/static/default_10.png'} + @override_settings( + PROFILE_IMAGE_BACKEND={}, + STORAGES={ + 'profile_images': { + 'BACKEND': 'storages.backends.s3boto3.S3Boto3Storage', + 'OPTIONS': { + 'bucket_name': 'profiles', + 'default_acl': 'public', + 'location': 'profile/images', + } + } + } + ) + def test_profile_backend_with_profile_image_settings(self): + """ It will use the storages dict with profile_images backend""" + storage = get_profile_image_storage() + self.assertIsInstance(storage, S3Boto3Storage) + self.assertEqual(storage.bucket_name, "profiles") + self.assertEqual(storage.default_acl, 'public') + self.assertEqual(storage.location, "profile/images") + + @override_settings( + PROFILE_IMAGE_BACKEND={}, + STORAGES={} + ) + def test_profile_backend_with_default_hardcoded_backend(self): + """ In case of empty storages scenario uses the hardcoded backend.""" + del settings.DEFAULT_FILE_STORAGE + storage = get_profile_image_storage() + self.assertIsInstance(storage, FileSystemStorage) + @ddt.data( ("client", "user", True), ("different_client", "different_user", False), From 2918ecd8269d243b5f63209cf19b41112538647b Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Fri, 16 May 2025 12:48:23 +0500 Subject: [PATCH 18/22] chore: Update image_helpers.py --- openedx/core/djangoapps/user_api/accounts/image_helpers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 2f980ac637df..267372e44d00 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -7,6 +7,7 @@ from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage +from django.core.files.storage import storages from django.core.exceptions import ObjectDoesNotExist from django.utils.module_loading import import_string @@ -39,9 +40,8 @@ def get_profile_image_storage(): if not storage_class_path: storages_config = getattr(settings, 'STORAGES', {}) - if 'profile_images' in storages_config: - storage_class_path = storages_config['profile_images'].get('BACKEND') - options = storages_config['profile_images'].get('OPTIONS', {}) + if 'PROFILE_IMAGE_STORAGE' in storages_config: + return storages['PROFILE_IMAGE_STORAGE'] elif 'default' in storages_config: storage_class_path = storages_config['default'].get('BACKEND') options = storages_config['default'].get('OPTIONS', {}) From df119634bdfedf1f2444df2afd7732307346404f Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Fri, 16 May 2025 12:49:15 +0500 Subject: [PATCH 19/22] chore: Update test_views.py --- openedx/core/djangoapps/user_api/accounts/tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 959701665450..d27734d432d7 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -1207,7 +1207,7 @@ def test_convert_relative_profile_url(self): @override_settings( PROFILE_IMAGE_BACKEND={}, STORAGES={ - 'profile_images': { + 'PROFILE_IMAGE_STORAGE': { 'BACKEND': 'storages.backends.s3boto3.S3Boto3Storage', 'OPTIONS': { 'bucket_name': 'profiles', From a8e562122ef023d6f312cdf1053b85d5c60a3705 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Fri, 16 May 2025 13:10:47 +0500 Subject: [PATCH 20/22] chore: fixing code. --- .../user_api/accounts/image_helpers.py | 31 ++++++++++++------- .../user_api/accounts/tests/test_views.py | 6 ++-- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 267372e44d00..35d0f183fa14 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -7,8 +7,8 @@ from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage -from django.core.files.storage import storages from django.core.exceptions import ObjectDoesNotExist +from django.core.files.storage import storages from django.utils.module_loading import import_string from common.djangoapps.student.models import UserProfile @@ -23,28 +23,35 @@ def get_profile_image_storage(): """ - Returns an instance of the configured Django storage class for profile images. + Returns an instance of the configured storage backend for profile images. + + This function prioritizes different settings in the following order to determine + which storage class to use: - The function looks for the `PROFILE_IMAGE_BACKEND` setting in the Django settings. - If it exists and includes a `'class'` key, that class is used as the storage backend. - If the setting is missing or does not include a `'class'`, the default storage backend - DEFAULT_FILE_STORAGE will return. + 1. If the `PROFILE_IMAGE_BACKEND` setting is defined and includes a `'class'` key, + it uses the specified storage backend and options. + 2. If not, it checks the Django 5+ `STORAGES` setting for a named storage called + `'profile_image'` and uses its configured backend. + 3. If that is also unavailable, it falls back to the `'default'` storage defined in `STORAGES`. + 4. Finally, if no STORAGES are configured, it uses `DEFAULT_FILE_STORAGE` (legacy setting), + or falls back to `'django.core.files.storage.FileSystemStorage'` as the last resort. Returns: - An instance of the configured storage backend. + An instance of the configured storage backend for handling profile images. + + Raises: + ImportError: If the specified storage class cannot be imported. """ config = getattr(settings, 'PROFILE_IMAGE_BACKEND', {}) storage_class_path = config.get('class') options = config.get('options', {}) - if not storage_class_path: storages_config = getattr(settings, 'STORAGES', {}) - if 'PROFILE_IMAGE_STORAGE' in storages_config: - return storages['PROFILE_IMAGE_STORAGE'] + if 'profile_image' in storages_config: + return storages['profile_image'] elif 'default' in storages_config: - storage_class_path = storages_config['default'].get('BACKEND') - options = storages_config['default'].get('OPTIONS', {}) + return storages['default'] else: storage_class_path = getattr(settings, 'DEFAULT_FILE_STORAGE', None) or \ 'django.core.files.storage.FileSystemStorage' diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index d27734d432d7..1c92aa22c706 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -1207,7 +1207,7 @@ def test_convert_relative_profile_url(self): @override_settings( PROFILE_IMAGE_BACKEND={}, STORAGES={ - 'PROFILE_IMAGE_STORAGE': { + 'profile_image': { 'BACKEND': 'storages.backends.s3boto3.S3Boto3Storage', 'OPTIONS': { 'bucket_name': 'profiles', @@ -1227,14 +1227,14 @@ def test_profile_backend_with_profile_image_settings(self): @override_settings( PROFILE_IMAGE_BACKEND={}, - STORAGES={} ) def test_profile_backend_with_default_hardcoded_backend(self): """ In case of empty storages scenario uses the hardcoded backend.""" del settings.DEFAULT_FILE_STORAGE + del settings.STORAGES storage = get_profile_image_storage() self.assertIsInstance(storage, FileSystemStorage) - + @ddt.data( ("client", "user", True), ("different_client", "different_user", False), From 3cc08568e8cf42a5dcfeed8188ff804ae318cbb3 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Fri, 16 May 2025 14:20:29 +0500 Subject: [PATCH 21/22] chore: fixing code. --- openedx/core/djangoapps/user_api/accounts/image_helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index 35d0f183fa14..f612cef9e2fd 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -53,8 +53,8 @@ def get_profile_image_storage(): elif 'default' in storages_config: return storages['default'] else: - storage_class_path = getattr(settings, 'DEFAULT_FILE_STORAGE', None) or \ - 'django.core.files.storage.FileSystemStorage' + storage_class_path = (getattr(settings, 'DEFAULT_FILE_STORAGE', None) or + 'django.core.files.storage.FileSystemStorage') options = {} storage_class = import_string(storage_class_path) From 9cbee76c8a7a5244a066255418e11d205de8cfc2 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Mon, 2 Jun 2025 16:21:57 -0400 Subject: [PATCH 22/22] feat!: changing the order to pick the storage backend. --- .../user_api/accounts/image_helpers.py | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/image_helpers.py b/openedx/core/djangoapps/user_api/accounts/image_helpers.py index f612cef9e2fd..eff2ad272b28 100644 --- a/openedx/core/djangoapps/user_api/accounts/image_helpers.py +++ b/openedx/core/djangoapps/user_api/accounts/image_helpers.py @@ -8,7 +8,7 @@ from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage from django.core.exceptions import ObjectDoesNotExist -from django.core.files.storage import storages +from django.core.files.storage import default_storage, storages from django.utils.module_loading import import_string from common.djangoapps.student.models import UserProfile @@ -28,13 +28,14 @@ def get_profile_image_storage(): This function prioritizes different settings in the following order to determine which storage class to use: - 1. If the `PROFILE_IMAGE_BACKEND` setting is defined and includes a `'class'` key, - it uses the specified storage backend and options. - 2. If not, it checks the Django 5+ `STORAGES` setting for a named storage called - `'profile_image'` and uses its configured backend. - 3. If that is also unavailable, it falls back to the `'default'` storage defined in `STORAGES`. - 4. Finally, if no STORAGES are configured, it uses `DEFAULT_FILE_STORAGE` (legacy setting), - or falls back to `'django.core.files.storage.FileSystemStorage'` as the last resort. + 1. Use 'profile_image' storage from Django's STORAGES if defined (Django 4.2+). + 2. If not available, check the legacy PROFILE_IMAGE_BACKEND setting. + 3. If still undefined, fall back to Django's default_storage. + + Note: + - Starting in Django 5+, `DEFAULT_FILE_STORAGE` and the `STORAGES` setting + are mutually exclusive. Only one of them should be used to avoid + `ImproperlyConfigured` errors. Returns: An instance of the configured storage backend for handling profile images. @@ -42,20 +43,19 @@ def get_profile_image_storage(): Raises: ImportError: If the specified storage class cannot be imported. """ + # Prefer new-style Django 4.2+ STORAGES + storages_config = getattr(settings, 'STORAGES', {}) + + if 'profile_image' in storages_config: + return storages['profile_image'] + + # Legacy fallback: PROFILE_IMAGE_BACKEND config = getattr(settings, 'PROFILE_IMAGE_BACKEND', {}) storage_class_path = config.get('class') options = config.get('options', {}) - if not storage_class_path: - storages_config = getattr(settings, 'STORAGES', {}) - if 'profile_image' in storages_config: - return storages['profile_image'] - elif 'default' in storages_config: - return storages['default'] - else: - storage_class_path = (getattr(settings, 'DEFAULT_FILE_STORAGE', None) or - 'django.core.files.storage.FileSystemStorage') - options = {} + if not storage_class_path: + return default_storage storage_class = import_string(storage_class_path) return storage_class(**options)