From c09ec0de4fc200e3f5260565d65fc7e284a15fc0 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 16 Jun 2026 14:27:07 -0400 Subject: [PATCH 1/4] temp: report FEATURES-as-dict warning at the real caller Pushed standalone first so CI surfaces the full list of caller sites before the migration commits land. FeaturesProxy.__setitem__ used stacklevel=2 and update() delegated to __setitem__, so every warning was reported at update()'s own 'self[key] = other[key]' line in this file (89) instead of the test or app code that actually mutated FEATURES. Why the helper instead of just bumping stacklevel: stacklevel is a frame count from warnings.warn upward. Bumping __setitem__'s stacklevel from 2 to 3 would fix the update()->__setitem__ path but would overshoot by one frame on direct __setitem__ calls (e.g. fp['X'] = True), reporting from the caller's caller. The two paths need different stacklevel arithmetic. Factors the emit into _warn_dict_access(stacklevel=3) so both __setitem__ and update() can call it with the same constant; each contributes exactly one intermediate frame. update() now writes to self.ns directly to avoid double-warning. No behavior change beyond the reported source location. Co-authored-by: Claude Opus 4.7 (1M context) --- openedx/core/lib/features_setting_proxy.py | 41 +++++++++++++++++----- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/openedx/core/lib/features_setting_proxy.py b/openedx/core/lib/features_setting_proxy.py index eb38ad7f9100..d24d4ceda053 100644 --- a/openedx/core/lib/features_setting_proxy.py +++ b/openedx/core/lib/features_setting_proxy.py @@ -1,6 +1,7 @@ """ Features Proxy Implementation """ +import traceback import warnings from collections.abc import Mapping, MutableMapping @@ -28,14 +29,32 @@ def __getitem__(self, key): """Retrieve a feature flag by key""" return self.ns[key] - def __setitem__(self, key, value): - """Sets a key-value pair while emitting a deprecation warning about using FEATURES as a dict.""" + # Frames to skip when walking the stack for the [caller: ...] suffix. + # mock.patch.dict's machinery is several frames deep, so simple stacklevel + # arithmetic can't reach the user test code on its own. + _STACK_SKIP_PATTERNS = ('features_setting_proxy.py', '/unittest/mock.py', '/contextlib.py') + + def _warn_dict_access(self, key, value): + """Emit the FEATURES-as-dict deprecation warning, attributed to the caller of __setitem__/update.""" + # stacklevel=3 walks past warnings.warn -> _warn_dict_access -> caller-of-_warn_dict_access, + # so the warning is reported from the line that actually mutated FEATURES (test code or app code). + stack = traceback.extract_stack() + user_frame = next( + (f for f in reversed(stack[:-1]) if not any(p in f.filename for p in self._STACK_SKIP_PATTERNS)), + None, + ) + suffix = f" [caller: {user_frame.filename}:{user_frame.lineno}]" if user_frame else "" warnings.warn( f"Accessing FEATURES as a dict is deprecated. " - f"Add '{key} = {value!r}' to your Django settings module instead of modifying FEATURES.", + f"Add '{key} = {value!r}' to your Django settings module instead of modifying FEATURES." + f"{suffix}", DeprecationWarning, - stacklevel=2 + stacklevel=3, ) + + def __setitem__(self, key, value): + """Sets a key-value pair while emitting a deprecation warning about using FEATURES as a dict.""" + self._warn_dict_access(key, value) self.ns[key] = value def __delitem__(self, key): @@ -82,21 +101,27 @@ def update(self, other=(), /, **kwds): proxy.update({'FEATURE_A': True}, FEATURE_B=False) -> other={'FEATURE_A': True}; kwds = {'FEATURE_B': False} """ + # We bypass __setitem__ and emit the warning here so that stacklevel + # points at the caller of update() rather than at this method's body. if isinstance(other, Mapping): # Handles objects that formally conform to the Mapping interface # Mapping-like types: defaultdict, OrderedDict, Counter for key in other: - self[key] = other[key] + self._warn_dict_access(key, other[key]) + self.ns[key] = other[key] elif hasattr(other, "keys"): # Fallback for objects that implement a .keys() method but # may not formally subclass Mapping for key in other.keys(): - self[key] = other[key] + self._warn_dict_access(key, other[key]) + self.ns[key] = other[key] else: for key, value in other: - self[key] = value + self._warn_dict_access(key, value) + self.ns[key] = value for key, value in kwds.items(): - self[key] = value + self._warn_dict_access(key, value) + self.ns[key] = value def copy(self): """ From 9a0cb01d0abb4ed0f51f967543c1496f97546d40 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 17 Jun 2026 09:23:06 -0400 Subject: [PATCH 2/4] feat: bridge FEATURES proxy reads to @override_settings overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lets @override_settings(KEY=val) be seen through both settings.KEY and settings.FEATURES['KEY']/.get('KEY'), so tests can migrate to the flat setting without first updating every production reader that still does settings.FEATURES['KEY']. We walk only the UserSettingsHolder chain that @override_settings pushes onto django.conf.settings._wrapped, NOT the bottom Settings layer. Django's bottom Settings is a *snapshot* of the settings module's globals taken at init time, so it doesn't reflect runtime mutations of the module's globals — which is exactly what legacy patch.dict(settings.FEATURES, ...) does via proxy.ns. Reading the snapshot would mask those mutations and break legacy tests; walking only the explicit override layers lets both paths coexist during the migration. ns kept as-is. A stateless redesign (drop ns, route everything through django.conf.settings, set FEATURES = FeaturesProxy() in each env file) was considered but rejected for this step because FEATURES.copy() and __iter__ are widely used to snapshot the full set of feature flags; without ns there is no clean source of truth for "which keys are features". Once all callers have moved off the dict pattern the proxy can be deleted outright (every settings.FEATURES['X'] becomes settings.X), making the stateless refactor unnecessary. Co-authored-by: Claude Opus 4.7 (1M context) --- openedx/core/lib/features_setting_proxy.py | 38 ++++++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/openedx/core/lib/features_setting_proxy.py b/openedx/core/lib/features_setting_proxy.py index d24d4ceda053..6ff7c872888c 100644 --- a/openedx/core/lib/features_setting_proxy.py +++ b/openedx/core/lib/features_setting_proxy.py @@ -25,8 +25,37 @@ def __init__(self, namespace=None): """Store the namespace (as a dict)""" self.ns = namespace or {} + _MISSING = object() + + def _resolve(self, key): + """Return key's value if it's been overridden via @override_settings, else self._MISSING. + + We deliberately walk only the UserSettingsHolder chain (the layers that + @override_settings pushes onto django.conf.settings._wrapped) rather + than reading django.conf.settings.X directly. Django's bottom Settings + layer is a *snapshot* taken at init time, so it doesn't reflect runtime + mutations of the settings module's globals (which is what + proxy.ns mutations and legacy patch.dict(settings.FEATURES, ...) do). + Walking only the explicit override layers lets the new + @override_settings(X=Y) path work while leaving the legacy patch.dict + path untouched. + """ + from django.conf import settings as django_settings + wrapped = django_settings._wrapped # pylint: disable=protected-access + # UserSettingsHolder has a `default_settings` attribute and stores + # explicit overrides in its own __dict__; the bottom Settings has no + # default_settings, so the loop terminates there. + while hasattr(wrapped, 'default_settings'): + if key in wrapped.__dict__: + return wrapped.__dict__[key] + wrapped = wrapped.default_settings + return self._MISSING + def __getitem__(self, key): - """Retrieve a feature flag by key""" + """Retrieve a feature flag by key, preferring @override_settings overrides.""" + value = self._resolve(key) + if value is not self._MISSING: + return value return self.ns[key] # Frames to skip when walking the stack for the [caller: ...] suffix. @@ -68,14 +97,17 @@ def __len__(self): return len(self.ns) def __contains__(self, key): - return key in self.ns + return self._resolve(key) is not self._MISSING or key in self.ns def clear(self): """Remove all feature flags from the namespace.""" self.ns.clear() def get(self, key, default=None): - """Standard dict-style get with default""" + """Standard dict-style get with default; prefers @override_settings overrides.""" + value = self._resolve(key) + if value is not self._MISSING: + return value return self.ns.get(key, default) def update(self, other=(), /, **kwds): From 749f3066b6a6f57d6a421938c9c93253b82c83d4 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 17 Jun 2026 10:26:26 -0400 Subject: [PATCH 3/4] refactor: migrate ENABLE_AUTHN_MICROFRONTEND off FEATURES-as-dict Co-authored-by: Claude Opus 4.7 (1M context) --- .../student/management/tests/test_recover_account.py | 4 +--- common/djangoapps/student/tests/test_activate_account.py | 9 +++------ openedx/core/djangoapps/user_authn/toggles.py | 2 +- .../core/djangoapps/user_authn/views/tests/test_login.py | 9 +++------ .../user_authn/views/tests/test_logistration.py | 7 ++----- .../user_authn/views/tests/test_reset_password.py | 5 +---- 6 files changed, 11 insertions(+), 25 deletions(-) diff --git a/common/djangoapps/student/management/tests/test_recover_account.py b/common/djangoapps/student/management/tests/test_recover_account.py index b0cacdb5d77e..bd1cbf3a8e9b 100644 --- a/common/djangoapps/student/management/tests/test_recover_account.py +++ b/common/djangoapps/student/management/tests/test_recover_account.py @@ -18,8 +18,6 @@ from openedx.core.djangolib.testing.utils import skip_unless_lms LOGGER_NAME = 'common.djangoapps.student.management.commands.recover_account' -FEATURES_WITH_AUTHN_MFE_ENABLED = settings.FEATURES.copy() -FEATURES_WITH_AUTHN_MFE_ENABLED['ENABLE_AUTHN_MICROFRONTEND'] = True class RecoverAccountTests(TestCase): @@ -67,7 +65,7 @@ def test_account_recovery(self): # try to login with previous password assert not self.client.login(username=self.user.username, password='password') - @override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True) @skip_unless_lms def test_authn_mfe_url_in_reset_link(self): """ diff --git a/common/djangoapps/student/tests/test_activate_account.py b/common/djangoapps/student/tests/test_activate_account.py index 12fef9f4de89..3fd6a1b432e8 100644 --- a/common/djangoapps/student/tests/test_activate_account.py +++ b/common/djangoapps/student/tests/test_activate_account.py @@ -17,9 +17,6 @@ from openedx.core.djangolib.testing.utils import skip_unless_lms from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerUserFactory -FEATURES_WITH_AUTHN_MFE_ENABLED = settings.FEATURES.copy() -FEATURES_WITH_AUTHN_MFE_ENABLED['ENABLE_AUTHN_MICROFRONTEND'] = True - @skip_unless_lms class TestActivateAccount(TestCase): @@ -180,7 +177,7 @@ def test_email_confirmation_notification_on_logistration(self): self.assertContains(response, 'Your email could not be confirmed') @override_settings(LOGIN_REDIRECT_WHITELIST=['localhost:1991']) - @override_settings(FEATURES={**FEATURES_WITH_AUTHN_MFE_ENABLED, 'ENABLE_ENTERPRISE_INTEGRATION': True}) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True, ENABLE_ENTERPRISE_INTEGRATION=True) def test_authenticated_account_activation_with_valid_next_url(self): """ Verify that an activation link with a valid next URL will redirect @@ -234,7 +231,7 @@ def test_account_activation_invalid_next_url_redirects_dashboard(self): self.assertRedirects(response, expected_destination) self._assert_user_active_state(expected_active_state=True) - @override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True) def test_unauthenticated_user_redirects_to_mfe(self): """ Verify that if Authn MFE is enabled then authenticated user redirects to @@ -259,7 +256,7 @@ def test_unauthenticated_user_redirects_to_mfe(self): assert response.url == (login_page_url + 'error') @override_settings(LOGIN_REDIRECT_WHITELIST=['localhost:1991']) - @override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True) def test_unauthenticated_user_redirects_to_mfe_with_valid_next_url(self): """ Verify that if Authn MFE is enabled then authenticated user redirects to diff --git a/openedx/core/djangoapps/user_authn/toggles.py b/openedx/core/djangoapps/user_authn/toggles.py index 67da464cbb5f..fb9e928bf5f4 100644 --- a/openedx/core/djangoapps/user_authn/toggles.py +++ b/openedx/core/djangoapps/user_authn/toggles.py @@ -22,7 +22,7 @@ def should_redirect_to_authn_microfrontend(): if request and request.GET.get('skip_authn_mfe'): return False return configuration_helpers.get_value( - 'ENABLE_AUTHN_MICROFRONTEND', settings.FEATURES.get('ENABLE_AUTHN_MICROFRONTEND') + 'ENABLE_AUTHN_MICROFRONTEND', getattr(settings, 'ENABLE_AUTHN_MICROFRONTEND', None) ) diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 4e4d0a9e8894..98db117aca20 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -93,9 +93,6 @@ def test_login_success(self): self._assert_response(response, success=True) self._assert_audit_log(mock_audit_log, 'info', ['Login success', self.user_email]) - FEATURES_WITH_AUTHN_MFE_ENABLED = settings.FEATURES.copy() - FEATURES_WITH_AUTHN_MFE_ENABLED['ENABLE_AUTHN_MICROFRONTEND'] = True - @override_settings(MARKETING_EMAILS_OPT_IN=True) def test_login_success_with_opt_in_flag_enabled(self): self.user.is_active = False @@ -188,7 +185,7 @@ def test_public_login_failure_with_only_third_part_auth_enabled(self): ) @ddt.unpack @override_settings(LOGIN_REDIRECT_WHITELIST=['openedx.service']) - @override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True) @skip_unless_lms def test_login_success_with_redirect(self, next_url, course_id, expected_redirect): post_params = {} @@ -209,7 +206,7 @@ def test_login_success_with_redirect(self, next_url, course_id, expected_redirec @ddt.data(('/dashboard', False), ('/enterprise/select/active/?success_url=/dashboard', True)) @ddt.unpack - @patch.dict(settings.FEATURES, {'ENABLE_AUTHN_MICROFRONTEND': True, 'ENABLE_ENTERPRISE_INTEGRATION': True}) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True, ENABLE_ENTERPRISE_INTEGRATION=True) @override_settings(LOGIN_REDIRECT_WHITELIST=['openedx.service']) @patch('openedx.features.enterprise_support.api.EnterpriseApiClient') @patch('openedx.core.djangoapps.user_authn.views.login.reverse') @@ -259,7 +256,7 @@ def test_login_success_for_multiple_enterprises( @ddt.data(('', True), ('/enterprise/select/active/?success_url=', False)) @ddt.unpack - @patch.dict(settings.FEATURES, {'ENABLE_AUTHN_MICROFRONTEND': True, 'ENABLE_ENTERPRISE_INTEGRATION': True}) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True, ENABLE_ENTERPRISE_INTEGRATION=True) @patch('openedx.features.enterprise_support.api.EnterpriseApiClient') @patch('openedx.core.djangoapps.user_authn.views.login.activate_learner_enterprise') @patch('openedx.core.djangoapps.user_authn.views.login.reverse') diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py index b94da138ea5d..15c90e5dea71 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py @@ -64,16 +64,13 @@ def setUp(self): # pylint: disable=arguments-differ ) self.hidden_disabled_provider = self.configure_azure_ad_provider() - FEATURES_WITH_AUTHN_MFE_ENABLED = settings.FEATURES.copy() - FEATURES_WITH_AUTHN_MFE_ENABLED['ENABLE_AUTHN_MICROFRONTEND'] = True - @ddt.data( ("signin_user", "/login"), ("register_user", "/register"), ("password_assistance", "/reset"), ) @ddt.unpack - @override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True) def test_logistration_mfe_redirects(self, url_name, path): """ Test that if Logistration MFE is enabled, then we redirect to @@ -96,7 +93,7 @@ def test_logistration_mfe_redirects(self, url_name, path): ) ) @ddt.unpack - @override_settings(FEATURES=FEATURES_WITH_AUTHN_MFE_ENABLED) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True) def test_logistration_redirect_params(self, url_name, path, query_params): """ Test that if request is redirected to logistration MFE, diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py index 7388e51edddc..1acc9eeaaa55 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py @@ -47,9 +47,6 @@ ) from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms -ENABLE_AUTHN_MICROFRONTEND = settings.FEATURES.copy() -ENABLE_AUTHN_MICROFRONTEND['ENABLE_AUTHN_MICROFRONTEND'] = True - def process_request(request): middleware = SessionMiddleware(get_response=lambda request: None) @@ -370,7 +367,7 @@ def test_reset_password_email_https(self, is_secure, protocol): SETTING_CHANGE_INITIATED, user_id=self.user.id, setting='password', old=None, new=None ) - @override_settings(FEATURES=ENABLE_AUTHN_MICROFRONTEND) + @override_settings(ENABLE_AUTHN_MICROFRONTEND=True) @skip_unless_lms @ddt.data(('Crazy Awesome Site', 'Crazy Awesome Site'), ('edX', 'edX')) @ddt.unpack From 3fe87743afeedbe124fb8358e106d3c9bb9e7b8d Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 17 Jun 2026 12:06:41 -0400 Subject: [PATCH 4/4] refactor: migrate ENABLE_CREATOR_GROUP off FEATURES-as-dict Production reads use getattr(settings, ...); tests use @override_settings(ENABLE_CREATOR_GROUP=...). Mixed-flag patches keep the other flag in patch.dict until its own migration. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../contentstore/tests/test_contentstore.py | 30 ++++++------- .../tests/test_course_create_rerun.py | 10 ++--- .../contentstore/tests/test_libraries.py | 4 +- cms/djangoapps/contentstore/views/course.py | 8 ++-- cms/djangoapps/contentstore/views/library.py | 2 +- .../contentstore/views/tests/test_library.py | 25 +++++------ .../views/tests/test_organizations.py | 16 +++---- .../course_creators/tests/test_admin.py | 6 ++- .../course_creators/tests/test_views.py | 45 +++++++++---------- common/djangoapps/student/auth.py | 2 +- common/djangoapps/student/tests/test_authz.py | 44 +++++++++--------- .../content_libraries/permissions.py | 2 +- 12 files changed, 94 insertions(+), 100 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 4561926c1b8d..65cbc682a365 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1345,23 +1345,23 @@ def test_create_course_with_course_creation_disabled_not_staff(self): self.user.save() self.assert_course_permission_denied() + @override_settings(ENABLE_CREATOR_GROUP=True) def test_create_course_no_course_creators_staff(self): """Test new course creation -- course creation group enabled, staff, group is empty.""" - with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}): - self.assert_created_course() + self.assert_created_course() + @override_settings(ENABLE_CREATOR_GROUP=True) def test_create_course_no_course_creators_not_staff(self): """Test new course creation -- error path for course creator group enabled, not staff, group is empty.""" - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - self.user.is_staff = False - self.user.save() - self.assert_course_permission_denied() + self.user.is_staff = False + self.user.save() + self.assert_course_permission_denied() + @override_settings(ENABLE_CREATOR_GROUP=True) def test_create_course_with_course_creator(self): """Test new course creation -- use course creator group""" - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - auth.add_users(self.user, CourseCreatorRole(), self.user) - self.assert_created_course() + auth.add_users(self.user, CourseCreatorRole(), self.user) + self.assert_created_course() def test_create_course_with_unicode_in_id_disabled(self): """ @@ -2014,13 +2014,13 @@ def test_rerun_course_fail_duplicate_course(self): # Verify that the existing course continues to be in the course listing self.assertInCourseListing(existent_course_key) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_rerun_with_permission_denied(self): - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - source_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) - auth.add_users(self.user, CourseCreatorRole(), self.user) - self.user.is_staff = False - self.user.save() - self.post_rerun_request(source_course.id, response_code=403, expect_error=True) + source_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) + auth.add_users(self.user, CourseCreatorRole(), self.user) + self.user.is_staff = False + self.user.save() + self.post_rerun_request(source_course.id, response_code=403, expect_error=True) def test_rerun_error(self): error_message = "Mock Error Message" diff --git a/cms/djangoapps/contentstore/tests/test_course_create_rerun.py b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py index fbcf56067ac1..8d274c1c9564 100644 --- a/cms/djangoapps/contentstore/tests/test_course_create_rerun.py +++ b/cms/djangoapps/contentstore/tests/test_course_create_rerun.py @@ -195,7 +195,7 @@ def test_course_creation_for_known_organization(self, organizations_autocreate): self.assertEqual(len(course_orgs), 1) # noqa: PT009 self.assertEqual(course_orgs[0]['short_name'], 'orgX') # noqa: PT009 - @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_course_creation_when_user_not_in_org(self): """ Tests course creation when user doesn't have the required role. @@ -208,7 +208,7 @@ def test_course_creation_when_user_not_in_org(self): }) self.assertEqual(response.status_code, 403) # noqa: PT009 - @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) + @override_settings(ENABLE_CREATOR_GROUP=True) @mock.patch( 'cms.djangoapps.course_creators.admin.render_to_string', mock.Mock(side_effect=mock_render_to_string, autospec=True) @@ -235,7 +235,7 @@ def test_course_creation_when_user_in_org_with_creator_role(self): }) self.assertEqual(response.status_code, 200) # noqa: PT009 - @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) + @override_settings(ENABLE_CREATOR_GROUP=True) @mock.patch( 'cms.djangoapps.course_creators.admin.render_to_string', mock.Mock(side_effect=mock_render_to_string, autospec=True) @@ -262,7 +262,7 @@ def test_course_creation_with_all_org_checked(self): }) self.assertEqual(response.status_code, 200) # noqa: PT009 - @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) + @override_settings(ENABLE_CREATOR_GROUP=True) @mock.patch( 'cms.djangoapps.course_creators.admin.render_to_string', mock.Mock(side_effect=mock_render_to_string, autospec=True) @@ -291,7 +291,7 @@ def test_course_creation_with_permission_for_specific_organization(self): }) self.assertEqual(response.status_code, 200) # noqa: PT009 - @override_settings(FEATURES={'ENABLE_CREATOR_GROUP': True}) + @override_settings(ENABLE_CREATOR_GROUP=True) @mock.patch( 'cms.djangoapps.course_creators.admin.render_to_string', mock.Mock(side_effect=mock_render_to_string, autospec=True) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 29a194fd017e..9ce85d525d26 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -574,12 +574,12 @@ def test_creation(self): # Now check that logged-in users without CourseCreator role cannot create libraries self._login_as_non_staff_user(logout_first=False) - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}): + with override_settings(ENABLE_CREATOR_GROUP=True): self._assert_cannot_create_library(expected_code=403) # 403 user is not CourseCreator # Now check that logged-in users with CourseCreator role can create libraries add_user_with_status_granted(self.user, self.non_staff_user) - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}): + with override_settings(ENABLE_CREATOR_GROUP=True): lib_key2 = self._create_library(library="lib2", display_name="Test Library 2") library2 = modulestore().get_library(lib_key2) self.assertIsNotNone(library2) # noqa: PT009 diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 569871b95bea..89eac5762cc3 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -2021,7 +2021,7 @@ def _get_course_creator_status(user): course_creator_status = 'granted' elif settings.FEATURES.get('DISABLE_COURSE_CREATION', False): course_creator_status = 'disallowed_for_this_site' - elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + elif getattr(settings, 'ENABLE_CREATOR_GROUP', False): course_creator_status = get_course_creator_status(user) if course_creator_status is None: # User not grandfathered in as an existing user, has not previously visited the dashboard page. @@ -2038,7 +2038,7 @@ def get_allowed_organizations(user): """ Helper method for returning the list of organizations for which the user is allowed to create courses. """ - if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + if getattr(settings, 'ENABLE_CREATOR_GROUP', False): return get_organizations(user) else: return [] @@ -2058,7 +2058,7 @@ def get_allowed_organizations_for_libraries(user): # This allows people in the course creator group for an org to create # libraries, which mimics course behavior. - if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + if getattr(settings, 'ENABLE_CREATOR_GROUP', False): organizations_set.update(get_organizations(user)) return sorted(organizations_set) @@ -2068,7 +2068,7 @@ def user_can_create_organizations(user): """ Returns True if the user can create organizations. """ - return user.is_staff or not settings.FEATURES.get('ENABLE_CREATOR_GROUP', False) + return user.is_staff or not getattr(settings, 'ENABLE_CREATOR_GROUP', False) def get_organizations_for_non_course_creators(user): diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 70ae0bce2c2a..31ae21ce7d60 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -72,7 +72,7 @@ def _user_can_create_library_for_org(user, org=None): return False elif user.is_staff: return True - elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + elif getattr(settings, 'ENABLE_CREATOR_GROUP', False): is_course_creator = get_course_creator_status(user) == 'granted' if is_course_creator: return True diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index 172f87addb56..203f2b765a52 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -70,7 +70,7 @@ def test_library_creator_status_with_no_course_creator_role(self): @mock.patch("cms.djangoapps.contentstore.toggles.libraries_v1_enabled", True) def test_library_creator_status_for_enabled_creator_group_setting_for_non_staff_users(self): _, nostaff_user = self.create_non_staff_authed_user_client() - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): + with override_settings(ENABLE_CREATOR_GROUP=True): self.assertEqual(user_can_create_library(nostaff_user, None), False) # noqa: PT009 # Global staff can create libraries for any org, even ones that don't exist. @@ -88,14 +88,14 @@ def test_library_creator_status_with_is_staff_user_no_org(self): # When creator groups are enabled, global staff can create libraries in any org @mock.patch("cms.djangoapps.contentstore.toggles.libraries_v1_enabled", True) def test_library_creator_status_for_enabled_creator_group_setting_with_is_staff_user(self): - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): + with override_settings(ENABLE_CREATOR_GROUP=True): self.assertEqual(user_can_create_library(self.user, 'RandomOrg'), True) # noqa: PT009 # When creator groups are enabled, course creators can create libraries in any org. @mock.patch("cms.djangoapps.contentstore.toggles.libraries_v1_enabled", True) def test_library_creator_status_with_course_creator_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): + with override_settings(ENABLE_CREATOR_GROUP=True): grant_course_creator_status(self.user, nostaff_user) self.assertEqual(user_can_create_library(nostaff_user, 'soMeRandOmoRg'), True) # noqa: PT009 @@ -104,7 +104,7 @@ def test_library_creator_status_with_course_creator_role_for_enabled_creator_gro @mock.patch("cms.djangoapps.contentstore.toggles.libraries_v1_enabled", True) def test_library_creator_status_with_course_staff_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): + with override_settings(ENABLE_CREATOR_GROUP=True): auth.add_users(self.user, CourseStaffRole(self.course.id), nostaff_user) self.assertEqual(user_can_create_library(nostaff_user, self.course.org), True) # noqa: PT009 self.assertEqual(user_can_create_library(nostaff_user, 'SomEOtherOrg'), False) # noqa: PT009 @@ -114,7 +114,7 @@ def test_library_creator_status_with_course_staff_role_for_enabled_creator_group @mock.patch("cms.djangoapps.contentstore.toggles.libraries_v1_enabled", True) def test_library_creator_status_with_course_instructor_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): + with override_settings(ENABLE_CREATOR_GROUP=True): auth.add_users(self.user, CourseInstructorRole(self.course.id), nostaff_user) self.assertEqual(user_can_create_library(nostaff_user, self.course.org), True) # noqa: PT009 self.assertEqual(user_can_create_library(nostaff_user, 'SomEOtherOrg'), False) # noqa: PT009 @@ -206,7 +206,7 @@ def test_create_library(self): self.assertEqual(response.status_code, 200) # noqa: PT009 # That's all we check. More detailed tests are in contentstore.tests.test_libraries... - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_lib_create_permission(self): """ Users who are given course creator roles should be able to create libraries. @@ -220,7 +220,7 @@ def test_lib_create_permission(self): }) self.assertEqual(response.status_code, 200) # noqa: PT009 - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': False}) + @override_settings(ENABLE_CREATOR_GROUP=False) def test_lib_create_permission_no_course_creator_role_and_no_course_creator_group(self): """ Users who are not given course creator roles should still be able to create libraries @@ -234,7 +234,7 @@ def test_lib_create_permission_no_course_creator_role_and_no_course_creator_grou }) self.assertEqual(response.status_code, 200) # noqa: PT009 - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_lib_create_permission_no_course_creator_role_and_no_course_creator_group_and_no_course_staff_role(self): """ Users who are not given course creator roles or course staff role should not be able to create libraries @@ -248,7 +248,7 @@ def test_lib_create_permission_no_course_creator_role_and_no_course_creator_grou }) self.assertEqual(response.status_code, 403) # noqa: PT009 - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_lib_create_permission_course_staff_role(self): """ Users who are staff on any existing course should able to create libraries @@ -492,14 +492,11 @@ def test_allowed_organizations_for_library(self): 'django.conf.settings.FEATURES', {"ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES": False} ): - with mock.patch.dict( - 'django.conf.settings.FEATURES', - {"ENABLE_CREATOR_GROUP": False} - ): + with override_settings(ENABLE_CREATOR_GROUP=False): organizations = get_allowed_organizations_for_libraries(self.user) # Assert that the method returned the expected value self.assertEqual(organizations, []) # noqa: PT009 - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): + with override_settings(ENABLE_CREATOR_GROUP=True): # Assert that correct org values are returned based on course creator state for course_creator_state in CourseCreator.STATES: course_creator.state = course_creator_state diff --git a/cms/djangoapps/contentstore/views/tests/test_organizations.py b/cms/djangoapps/contentstore/views/tests/test_organizations.py index d88420eac8c2..be9feb2c3000 100644 --- a/cms/djangoapps/contentstore/views/tests/test_organizations.py +++ b/cms/djangoapps/contentstore/views/tests/test_organizations.py @@ -89,44 +89,44 @@ def setUpTestData(cls): ) @override_settings( + ENABLE_CREATOR_GROUP=False, FEATURES={ **settings.FEATURES, 'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': False, - 'ENABLE_CREATOR_GROUP': False, - } + }, ) def test_both_toggles_disabled(self): allowed_orgs = get_allowed_organizations_for_libraries(self.library_author) assert allowed_orgs == [] @override_settings( + ENABLE_CREATOR_GROUP=True, FEATURES={ **settings.FEATURES, 'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': True, - 'ENABLE_CREATOR_GROUP': True, - } + }, ) def test_both_toggles_enabled(self): allowed_orgs = get_allowed_organizations_for_libraries(self.library_author) assert allowed_orgs == ["CreatorOrg", "OrgStaffOrg"] @override_settings( + ENABLE_CREATOR_GROUP=False, FEATURES={ **settings.FEATURES, 'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': True, - 'ENABLE_CREATOR_GROUP': False, - } + }, ) def test_org_staff_enabled(self): allowed_orgs = get_allowed_organizations_for_libraries(self.library_author) assert allowed_orgs == ["OrgStaffOrg"] @override_settings( + ENABLE_CREATOR_GROUP=True, FEATURES={ **settings.FEATURES, 'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': False, - 'ENABLE_CREATOR_GROUP': True, - } + }, ) def test_creator_group_enabled(self): allowed_orgs = get_allowed_organizations_for_libraries(self.library_author) diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 28b9a6a61862..48fcc7ae7569 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -9,6 +9,7 @@ from django.core import mail from django.http import HttpRequest from django.test import TestCase +from django.test.utils import override_settings from cms.djangoapps.course_creators.admin import CourseCreatorAdmin from cms.djangoapps.course_creators.models import CourseCreator @@ -52,10 +53,10 @@ def setUp(self): self.studio_request_email = 'mark@marky.mark' self.enable_creator_group_patch = { - "ENABLE_CREATOR_GROUP": True, - "STUDIO_REQUEST_EMAIL": self.studio_request_email + "STUDIO_REQUEST_EMAIL": self.studio_request_email, } + @override_settings(ENABLE_CREATOR_GROUP=True) @mock.patch( 'cms.djangoapps.course_creators.admin.render_to_string', mock.Mock(side_effect=mock_render_to_string, autospec=True) @@ -103,6 +104,7 @@ def change_state_and_verify_email(state, is_creator): change_state_and_verify_email(CourseCreator.DENIED, False) + @override_settings(ENABLE_CREATOR_GROUP=True) @mock.patch( 'cms.djangoapps.course_creators.admin.render_to_string', mock.Mock(side_effect=mock_render_to_string, autospec=True) diff --git a/cms/djangoapps/course_creators/tests/test_views.py b/cms/djangoapps/course_creators/tests/test_views.py index 49b7631861dc..d4554263a8bd 100644 --- a/cms/djangoapps/course_creators/tests/test_views.py +++ b/cms/djangoapps/course_creators/tests/test_views.py @@ -3,10 +3,9 @@ """ -from unittest import mock - from django.core.exceptions import PermissionDenied from django.test import TestCase +from django.test.utils import override_settings from django.urls import reverse from cms.djangoapps.course_creators.views import ( @@ -64,35 +63,35 @@ def test_add_unrequested(self): add_user_with_status_granted(self.admin, self.user) self.assertEqual('unrequested', get_course_creator_status(self.user)) # noqa: PT009 + @override_settings(ENABLE_CREATOR_GROUP=True) def test_add_granted(self): - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - # Calling add_user_with_status_granted impacts is_user_in_course_group_role. - self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 + # Calling add_user_with_status_granted impacts is_user_in_course_group_role. + self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 - add_user_with_status_granted(self.admin, self.user) - self.assertEqual('granted', get_course_creator_status(self.user)) # noqa: PT009 + add_user_with_status_granted(self.admin, self.user) + self.assertEqual('granted', get_course_creator_status(self.user)) # noqa: PT009 - # Calling add again will be a no-op (even if state is different). - add_user_with_status_unrequested(self.user) - self.assertEqual('granted', get_course_creator_status(self.user)) # noqa: PT009 + # Calling add again will be a no-op (even if state is different). + add_user_with_status_unrequested(self.user) + self.assertEqual('granted', get_course_creator_status(self.user)) # noqa: PT009 - self.assertTrue(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 + self.assertTrue(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 + @override_settings(ENABLE_CREATOR_GROUP=True) def test_update_creator_group(self): - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 - update_course_creator_group(self.admin, self.user, True) - self.assertTrue(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 - update_course_creator_group(self.admin, self.user, False) - self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 + self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 + update_course_creator_group(self.admin, self.user, True) + self.assertTrue(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 + update_course_creator_group(self.admin, self.user, False) + self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole())) # noqa: PT009 + @override_settings(ENABLE_CREATOR_GROUP=True) def test_update_org_content_creator_role(self): - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - self.assertFalse(auth.user_has_role(self.user, OrgContentCreatorRole(self.org))) # noqa: PT009 - update_org_content_creator_role(self.admin, self.user, [self.org]) - self.assertTrue(auth.user_has_role(self.user, OrgContentCreatorRole(self.org))) # noqa: PT009 - update_org_content_creator_role(self.admin, self.user, []) - self.assertFalse(auth.user_has_role(self.user, OrgContentCreatorRole(self.org))) # noqa: PT009 + self.assertFalse(auth.user_has_role(self.user, OrgContentCreatorRole(self.org))) # noqa: PT009 + update_org_content_creator_role(self.admin, self.user, [self.org]) + self.assertTrue(auth.user_has_role(self.user, OrgContentCreatorRole(self.org))) # noqa: PT009 + update_org_content_creator_role(self.admin, self.user, []) + self.assertFalse(auth.user_has_role(self.user, OrgContentCreatorRole(self.org))) # noqa: PT009 def test_user_requested_access(self): add_user_with_status_unrequested(self.user) diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index d7fdb0bcae3c..378fae50cf35 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -64,7 +64,7 @@ def user_has_role(user, role): if settings.FEATURES.get('DISABLE_COURSE_CREATION', False): return False # wide open course creation setting - if not settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + if not getattr(settings, 'ENABLE_CREATOR_GROUP', False): return True if role.has_user(user): diff --git a/common/djangoapps/student/tests/test_authz.py b/common/djangoapps/student/tests/test_authz.py index d349da4b1bf6..54ee9dbd526b 100644 --- a/common/djangoapps/student/tests/test_authz.py +++ b/common/djangoapps/student/tests/test_authz.py @@ -53,33 +53,33 @@ def test_creator_group_not_enabled(self): """ assert user_has_role(self.user, CourseCreatorRole()) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_creator_group_enabled_but_empty(self): """ Tests creator group feature on, but group empty. """ - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - assert not user_has_role(self.user, CourseCreatorRole()) + assert not user_has_role(self.user, CourseCreatorRole()) - # Make user staff. This will cause CourseCreatorRole().has_user to return True. - self.user.is_staff = True - assert user_has_role(self.user, CourseCreatorRole()) + # Make user staff. This will cause CourseCreatorRole().has_user to return True. + self.user.is_staff = True + assert user_has_role(self.user, CourseCreatorRole()) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_creator_group_enabled_nonempty(self): """ Tests creator group feature on, user added. """ - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - add_users(self.admin, CourseCreatorRole(), self.user) - assert user_has_role(self.user, CourseCreatorRole()) + add_users(self.admin, CourseCreatorRole(), self.user) + assert user_has_role(self.user, CourseCreatorRole()) - # check that a user who has not been added to the group still returns false - user_not_added = UserFactory.create(username='testuser2', email='test+courses2@edx.org', password='foo2') - assert not user_has_role(user_not_added, CourseCreatorRole()) + # check that a user who has not been added to the group still returns false + user_not_added = UserFactory.create(username='testuser2', email='test+courses2@edx.org', password='foo2') + assert not user_has_role(user_not_added, CourseCreatorRole()) - # remove first user from the group and verify that CourseCreatorRole().has_user now returns false - remove_users(self.admin, CourseCreatorRole(), self.user) - assert not user_has_role(self.user, CourseCreatorRole()) + # remove first user from the group and verify that CourseCreatorRole().has_user now returns false + remove_users(self.admin, CourseCreatorRole(), self.user) + assert not user_has_role(self.user, CourseCreatorRole()) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_course_creation_disabled(self): """ Tests that the COURSE_CREATION_DISABLED flag overrides course creator group settings. """ - with mock.patch.dict('django.conf.settings.FEATURES', - {'DISABLE_COURSE_CREATION': True, "ENABLE_CREATOR_GROUP": True}): + with mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_COURSE_CREATION': True}): # Add user to creator group. add_users(self.admin, CourseCreatorRole(), self.user) @@ -94,27 +94,23 @@ def test_course_creation_disabled(self): remove_users(self.admin, CourseCreatorRole(), self.user) assert user_has_role(self.user, CourseCreatorRole()) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_add_user_not_authenticated(self): """ Tests that adding to creator group fails if user is not authenticated """ - with mock.patch.dict( - 'django.conf.settings.FEATURES', - {'DISABLE_COURSE_CREATION': False, "ENABLE_CREATOR_GROUP": True} - ): + with mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_COURSE_CREATION': False}): anonymous_user = AnonymousUser() role = CourseCreatorRole() add_users(self.admin, role, anonymous_user) assert not user_has_role(anonymous_user, role) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_add_user_not_active(self): """ Tests that adding to creator group fails if user is not active """ - with mock.patch.dict( - 'django.conf.settings.FEATURES', - {'DISABLE_COURSE_CREATION': False, "ENABLE_CREATOR_GROUP": True} - ): + with mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_COURSE_CREATION': False}): self.user.is_active = False add_users(self.admin, CourseCreatorRole(), self.user) assert not user_has_role(self.user, CourseCreatorRole()) diff --git a/openedx/core/djangoapps/content_libraries/permissions.py b/openedx/core/djangoapps/content_libraries/permissions.py index 1d0c7f9c0544..96b8615c0efa 100644 --- a/openedx/core/djangoapps/content_libraries/permissions.py +++ b/openedx/core/djangoapps/content_libraries/permissions.py @@ -228,7 +228,7 @@ def check(self, user, instance, *args, **kwargs): # pylint: disable=arguments-d # Is the user allowed to create content libraries? CAN_CREATE_CONTENT_LIBRARY = 'content_libraries.create_library' -if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): +if getattr(settings, 'ENABLE_CREATOR_GROUP', False): perms[CAN_CREATE_CONTENT_LIBRARY] = is_global_staff | (is_user_active & is_course_creator) else: perms[CAN_CREATE_CONTENT_LIBRARY] = is_global_staff