From a7f39f35241421caa8bb4432d01e346aaf204e44 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 17 Jun 2026 10:26:26 -0400 Subject: [PATCH 01/25] 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 2596216549b0926abd2f1988c2cb41fa701f40d6 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 17 Jun 2026 12:06:41 -0400 Subject: [PATCH 02/25] 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 d3da7b9ec064..e1fa67ce0ba5 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -2093,7 +2093,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. @@ -2110,7 +2110,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 [] @@ -2130,7 +2130,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) @@ -2140,7 +2140,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 ecce6b1378ff..8b740f16628d 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -67,7 +67,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 d1c04fec7df0..ac7536b005e2 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -69,7 +69,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. @@ -87,14 +87,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 @@ -103,7 +103,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 @@ -113,7 +113,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 @@ -205,7 +205,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. @@ -219,7 +219,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 @@ -233,7 +233,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 @@ -247,7 +247,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 @@ -464,14 +464,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 5d0508b5822b..185ad112c83c 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 @@ -48,8 +49,7 @@ 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, } self.context = { 'studio_request_email': self.studio_request_email, @@ -59,6 +59,7 @@ def setUp(self): 'user_email': 'test_user+courses@edx.org', } + @override_settings(ENABLE_CREATOR_GROUP=True) @mock.patch('django.contrib.auth.models.User.email_user') def test_change_status(self, email_user): """ @@ -101,6 +102,7 @@ def change_state_and_verify_email(state, is_creator): change_state_and_verify_email(CourseCreator.DENIED, False) + @override_settings(ENABLE_CREATOR_GROUP=True) def test_mail_admin_on_pending(self): """ Tests that the admin account is notified when a user is in the 'pending' state. 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 f7c3a20e0753..8f3a90d2147d 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 d4503aa121d1..a128b41177ff 100644 --- a/openedx/core/djangoapps/content_libraries/permissions.py +++ b/openedx/core/djangoapps/content_libraries/permissions.py @@ -239,7 +239,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 From 9972999f8df584f9577bd111030f0274921c8866 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 17 Jun 2026 12:33:40 -0400 Subject: [PATCH 03/25] refactor: migrate CERTIFICATES_HTML_VIEW off FEATURES-as-dict - Production reads use getattr(settings, ...); tests use @override_settings(CERTIFICATES_HTML_VIEW=...). - Drop now-unused FEATURES_WITH_CERTS_ENABLED/_DISABLED helpers. - FEATURES_WITH_CUSTOM_CERTS_ENABLED keeps CUSTOM_CERTIFICATE_TEMPLATES_ENABLED until that flag migrates. - Mixed-flag patches keep the other flag in patch.dict. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../contentstore/api/tests/test_validation.py | 4 +- .../contentstore/views/certificate_manager.py | 2 +- .../contentstore/views/tests/test_assets.py | 6 +- .../views/tests/test_certificates.py | 8 +- .../views/tests/test_exam_settings_view.py | 2 +- cms/urls.py | 2 +- .../student/tests/test_certificates.py | 4 +- common/djangoapps/student/tests/tests.py | 4 +- common/djangoapps/util/course.py | 2 +- .../certificates/apis/v0/tests/test_views.py | 4 +- lms/djangoapps/certificates/tests/test_api.py | 24 ++-- .../certificates/tests/test_support_views.py | 5 +- .../certificates/tests/test_utils.py | 8 +- .../certificates/tests/test_views.py | 18 +-- .../certificates/tests/test_webview_views.py | 108 +++++++++--------- lms/djangoapps/certificates/utils.py | 2 +- .../certificates/views/tests/test_filters.py | 14 +-- lms/djangoapps/certificates/views/webview.py | 2 +- lms/djangoapps/courseware/tests/test_views.py | 8 +- .../instructor/tests/test_certificates.py | 9 +- lms/djangoapps/mobile_api/users/tests.py | 9 +- .../courseware_api/tests/test_views.py | 4 +- 22 files changed, 104 insertions(+), 145 deletions(-) diff --git a/cms/djangoapps/contentstore/api/tests/test_validation.py b/cms/djangoapps/contentstore/api/tests/test_validation.py index a34faf516219..579f72c4e5f7 100644 --- a/cms/djangoapps/contentstore/api/tests/test_validation.py +++ b/cms/djangoapps/contentstore/api/tests/test_validation.py @@ -7,7 +7,6 @@ import ddt import factory -from django.conf import settings from django.contrib.auth import get_user_model from django.test.utils import override_settings from django.urls import reverse @@ -106,8 +105,7 @@ def test_student_fails(self): ) @ddt.unpack def test_staff_succeeds(self, certs_html_view, with_modes): - features = dict(settings.FEATURES, CERTIFICATES_HTML_VIEW=certs_html_view) - with override_settings(FEATURES=features): + with override_settings(CERTIFICATES_HTML_VIEW=certs_html_view): if with_modes: CourseModeFactory.create_batch( 2, diff --git a/cms/djangoapps/contentstore/views/certificate_manager.py b/cms/djangoapps/contentstore/views/certificate_manager.py index 0c36762f8967..f5f4224d1822 100644 --- a/cms/djangoapps/contentstore/views/certificate_manager.py +++ b/cms/djangoapps/contentstore/views/certificate_manager.py @@ -121,7 +121,7 @@ def is_activated(course): """ is_active = False certificates = [] - if settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False): + if settings.CERTIFICATES_HTML_VIEW: certificates = CertificateManager.get_certificates(course) # we are assuming only one certificate in certificates collection. for certificate in certificates: diff --git a/cms/djangoapps/contentstore/views/tests/test_assets.py b/cms/djangoapps/contentstore/views/tests/test_assets.py index 513a39fe82ab..1df352b70893 100644 --- a/cms/djangoapps/contentstore/views/tests/test_assets.py +++ b/cms/djangoapps/contentstore/views/tests/test_assets.py @@ -38,11 +38,7 @@ MAX_FILE_SIZE = settings.MAX_ASSET_UPLOAD_FILE_SIZE_IN_MB * 1000 ** 2 -FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() -FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True - - -@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) +@override_settings(CERTIFICATES_HTML_VIEW=True) class AssetsTestCase(CourseTestCase): """ Parent class for all asset tests. diff --git a/cms/djangoapps/contentstore/views/tests/test_certificates.py b/cms/djangoapps/contentstore/views/tests/test_certificates.py index 0da1596b3986..64942cf7e7a9 100644 --- a/cms/djangoapps/contentstore/views/tests/test_certificates.py +++ b/cms/djangoapps/contentstore/views/tests/test_certificates.py @@ -7,7 +7,6 @@ import json import ddt -from django.conf import settings from django.test.utils import override_settings from opaque_keys.edx.keys import AssetKey @@ -24,9 +23,6 @@ from ..certificate_manager import CERTIFICATE_SCHEMA_VERSION, CertificateManager -FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() -FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True - CERTIFICATE_JSON = { 'name': 'Test certificate', 'description': 'Test description', @@ -195,7 +191,7 @@ def test_certificate_data_validation(self): @ddt.ddt -@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) +@override_settings(CERTIFICATES_HTML_VIEW=True) class CertificatesListHandlerTestCase( EventTestMixin, CourseTestCase, HelperMethods, UrlResetMixin ): @@ -350,7 +346,7 @@ def test_assign_unique_identifier_to_certificates(self): @ddt.ddt -@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) +@override_settings(CERTIFICATES_HTML_VIEW=True) class CertificatesDetailHandlerTestCase( EventTestMixin, CourseTestCase, CertificatesBaseTestCase, HelperMethods, UrlResetMixin ): diff --git a/cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py b/cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py index 6a746b8d67ec..4b1404dc2cb1 100644 --- a/cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py +++ b/cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py @@ -10,9 +10,9 @@ @override_settings( + CERTIFICATES_HTML_VIEW=True, FEATURES={ **settings.FEATURES, - "CERTIFICATES_HTML_VIEW": True, "ENABLE_PROCTORED_EXAMS": True, }, ) diff --git a/cms/urls.py b/cms/urls.py index 7256bf1c5606..6861b45c3f9b 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -265,7 +265,7 @@ contentstore_views.entrance_exam)) # Enable Web/HTML Certificates -if settings.FEATURES.get('CERTIFICATES_HTML_VIEW'): +if settings.CERTIFICATES_HTML_VIEW: from cms.djangoapps.contentstore.views.certificates import ( # noqa: I001 - conditional import inside if block CertificateActivationAPIView, CertificateDetailAPIView, diff --git a/common/djangoapps/student/tests/test_certificates.py b/common/djangoapps/student/tests/test_certificates.py index a25fe5560ec8..9de3e95844a5 100644 --- a/common/djangoapps/student/tests/test_certificates.py +++ b/common/djangoapps/student/tests/test_certificates.py @@ -1,7 +1,6 @@ """Tests for display of certificates on the student dashboard. """ import datetime -from unittest.mock import patch from zoneinfo import ZoneInfo import ddt @@ -206,8 +205,7 @@ def setUpClass(cls): cls.store.update_item(cls.course, cls.USERNAME) @ddt.data('verified') - @override_settings(CERT_NAME_SHORT='Test_Certificate') - @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True}) + @override_settings(CERT_NAME_SHORT='Test_Certificate', CERTIFICATES_HTML_VIEW=True) def test_linked_student_to_web_view_credential(self, enrollment_mode): cert = self._create_certificate(enrollment_mode) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index c7a0b4c5037c..20819b6b8859 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -80,7 +80,7 @@ def test_process_survey_link(self): link2_expected = f"http://www.mysurvey.com?unique={user_id}" assert process_survey_link(link2, user) == link2_expected - @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False}) + @override_settings(CERTIFICATES_HTML_VIEW=False) def test_cert_info(self): user = UserFactory.create() survey_url = "http://a_survey.com" @@ -477,7 +477,7 @@ def test_linked_in_add_to_profile_btn_not_appearing_without_config(self): self.assertNotContains(response, escape(response_url)) @skip_unless_lms - @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False}) + @override_settings(CERTIFICATES_HTML_VIEW=False) def test_linked_in_add_to_profile_btn_with_certificate(self): # If user has a certificate with valid linked-in config then Add Certificate to LinkedIn button # should be visible. and it has URL value with valid parameters. diff --git a/common/djangoapps/util/course.py b/common/djangoapps/util/course.py index 6eeaec0966f6..4e58997c684a 100644 --- a/common/djangoapps/util/course.py +++ b/common/djangoapps/util/course.py @@ -80,7 +80,7 @@ def has_certificates_enabled(course): course: This can be either a course overview object or a course block. Returns a boolean if the course has enabled certificates """ - if not settings.FEATURES.get('CERTIFICATES_HTML_VIEW', False): + if not settings.CERTIFICATES_HTML_VIEW: return False return course.cert_html_view_enabled diff --git a/lms/djangoapps/certificates/apis/v0/tests/test_views.py b/lms/djangoapps/certificates/apis/v0/tests/test_views.py index d08241b2cd79..7b27a03446a4 100644 --- a/lms/djangoapps/certificates/apis/v0/tests/test_views.py +++ b/lms/djangoapps/certificates/apis/v0/tests/test_views.py @@ -6,7 +6,7 @@ from unittest.mock import patch import ddt -from django.conf import settings +from django.test import override_settings from django.urls import reverse from django.utils import timezone from freezegun import freeze_time @@ -382,7 +382,7 @@ def test_query_counts(self): assert resp.status_code == status.HTTP_200_OK assert len(resp.data) == 2 - @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_with_no_certificate_configuration(self): """ Verify that certificates are not returned until there is an active diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 08c75a271056..f53011d3c31a 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -76,10 +76,6 @@ BETA_TESTER_METHOD = "lms.djangoapps.certificates.api.access.is_beta_tester" CERTS_VIEWABLE_METHOD = "lms.djangoapps.certificates.api.certificates_viewable_for_course" PASSED_OR_ALLOWLISTED_METHOD = "lms.djangoapps.certificates.api._has_passed_or_is_allowlisted" -FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() -FEATURES_WITH_CERTS_ENABLED["CERTIFICATES_HTML_VIEW"] = True - - class WebCertificateTestMixin: """ Mixin with helpers for testing Web Certificates. @@ -191,14 +187,14 @@ def verify_downloadable_pdf_cert(self): "uuid": cert.verify_uuid, } - @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_pdf_cert_with_html_enabled(self): self.verify_downloadable_pdf_cert() def test_pdf_cert_with_html_disabled(self): self.verify_downloadable_pdf_cert() - @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_with_downloadable_web_cert(self): cert_status = certificate_status_for_student(self.student, self.course.id) assert certificate_downloadable_status(self.student, self.course.id) == { @@ -221,7 +217,7 @@ def test_with_downloadable_web_cert(self): (False, timedelta(days=2), CertificatesDisplayBehaviors.END_WITH_DATE, False, None, True), ) @ddt.unpack - @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_cert_api_return( self, self_paced, @@ -476,7 +472,7 @@ def test_no_certificates_for_user(self): """ assert not get_certificates_for_user(self.student_no_cert.username) - @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_get_web_certificate_url(self): """ Test the get_certificate_url with a web cert course @@ -490,7 +486,7 @@ def test_get_web_certificate_url(self): cert_url = get_certificate_url(user_id=self.student.id, course_id=self.web_cert_course.id, uuid=self.uuid) assert expected_url == cert_url - @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_get_pdf_certificate_url(self): """ Test the get_certificate_url with a pdf cert course @@ -522,7 +518,7 @@ def setUp(self): mode=CourseMode.VERIFIED, ) - @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": False}) + @override_settings(CERTIFICATES_HTML_VIEW=False) def test_cert_url_empty_with_invalid_certificate(self): """ Test certificate url is empty if html view is not enabled and certificate is not yet generated @@ -530,7 +526,7 @@ def test_cert_url_empty_with_invalid_certificate(self): url = get_certificate_url(self.user.id, self.course_run_key) assert url == "" - @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_generation(self): """ Test that a cert is successfully generated @@ -546,7 +542,7 @@ def test_generation(self): assert cert.status == CertificateStatuses.downloadable assert cert.mode == CourseMode.VERIFIED - @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) @ddt.data(True, False) def test_generation_unverified(self, enable_idv_requirement): """ @@ -567,7 +563,7 @@ def test_generation_unverified(self, enable_idv_requirement): else: assert cert.status == CertificateStatuses.downloadable - @patch.dict(settings.FEATURES, {"CERTIFICATES_HTML_VIEW": True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_generation_notpassing(self): """ Test that a cert is successfully generated with a status of notpassing @@ -652,7 +648,7 @@ def _assert_enabled_for_course(self, course_key, expect_enabled): assert expect_enabled == actual_enabled -@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) +@override_settings(CERTIFICATES_HTML_VIEW=True) class CertificatesBrandingTest(ModuleStoreTestCase): """Test certificates branding.""" diff --git a/lms/djangoapps/certificates/tests/test_support_views.py b/lms/djangoapps/certificates/tests/test_support_views.py index 80b15112fcac..00cb4a13db33 100644 --- a/lms/djangoapps/certificates/tests/test_support_views.py +++ b/lms/djangoapps/certificates/tests/test_support_views.py @@ -8,7 +8,6 @@ from uuid import uuid4 import ddt -from django.conf import settings from django.test.utils import override_settings from django.urls import reverse from opaque_keys.edx.keys import CourseKey @@ -25,8 +24,6 @@ from xmodule.modulestore.tests.factories import CourseFactory # pylint: disable=wrong-import-order CAN_GENERATE_METHOD = 'lms.djangoapps.certificates.generation_handler._can_generate_regular_certificate' -FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() -FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True class CertificateSupportTestCase(ModuleStoreTestCase): @@ -213,7 +210,7 @@ def test_results(self): assert retrieved_cert['download_url'] == self.CERT_DOWNLOAD_URL assert not retrieved_cert['regenerate'] - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_download_link(self): self.cert.course_id = self.course_key self.cert.download_url = '' diff --git a/lms/djangoapps/certificates/tests/test_utils.py b/lms/djangoapps/certificates/tests/test_utils.py index e9c7b8787868..8d52140cbdb2 100644 --- a/lms/djangoapps/certificates/tests/test_utils.py +++ b/lms/djangoapps/certificates/tests/test_utils.py @@ -2,10 +2,10 @@ Tests for Certificates app utility functions """ from datetime import datetime, timedelta -from unittest.mock import patch import ddt from django.test import TestCase +from django.test.utils import override_settings from pytz import utc from lms.djangoapps.certificates.utils import has_html_certificates_enabled, should_certificate_be_visible @@ -27,14 +27,14 @@ def setUp(self): super().setUp() self.course_overview = CourseOverviewFactory.create() - @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': False}) + @override_settings(CERTIFICATES_HTML_VIEW=False) def test_has_html_certificates_enabled_from_course_overview_cert_html_view_disabled(self): """ Test to ensure we return the correct value when the `CERTIFICATES_HTML_VIEW` setting is disabled. """ assert not has_html_certificates_enabled(self.course_overview) - @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_has_html_certificates_enabled_from_course_overview_enabled(self): """ Test to ensure we return the correct value when the HTML certificates are enabled in a course-run. @@ -44,7 +44,7 @@ def test_has_html_certificates_enabled_from_course_overview_enabled(self): assert has_html_certificates_enabled(self.course_overview) - @patch.dict('django.conf.settings.FEATURES', {'CERTIFICATES_HTML_VIEW': True}) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_has_html_certificates_enabled_from_course_overview_disabled(self): """ Test to ensure we return the correct value when the HTML certificates are disabled in a course-run. diff --git a/lms/djangoapps/certificates/tests/test_views.py b/lms/djangoapps/certificates/tests/test_views.py index 14c5e027d5c1..4977d2166157 100644 --- a/lms/djangoapps/certificates/tests/test_views.py +++ b/lms/djangoapps/certificates/tests/test_views.py @@ -4,7 +4,6 @@ import datetime from uuid import uuid4 -from django.conf import settings from django.test.client import Client from django.test.utils import override_settings @@ -19,17 +18,6 @@ ) from xmodule.modulestore.tests.factories import CourseFactory # pylint: disable=wrong-import-order -FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() -FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True - -FEATURES_WITH_CERTS_DISABLED = settings.FEATURES.copy() -FEATURES_WITH_CERTS_DISABLED['CERTIFICATES_HTML_VIEW'] = False - -FEATURES_WITH_CUSTOM_CERTS_ENABLED = { - "CUSTOM_CERTIFICATE_TEMPLATES_ENABLED": True -} -FEATURES_WITH_CUSTOM_CERTS_ENABLED.update(FEATURES_WITH_CERTS_ENABLED) - class CertificatesViewsSiteTests(ModuleStoreTestCase): """ @@ -129,7 +117,7 @@ def _add_course_certificates(self, count=1, signatory_count=0, is_active=True): self.course.save() self.store.update_item(self.course, self.user.id) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) @with_site_configuration(configuration={'platform_name': 'My Platform Site'}) def test_html_view_for_site(self): test_url = get_certificate_url( @@ -149,7 +137,7 @@ def test_html_view_for_site(self): ) self.assertContains(response, 'About My Platform Site') - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_html_view_site_configuration_missing(self): test_url = get_certificate_url( user_id=self.user.id, @@ -165,7 +153,7 @@ def test_html_view_site_configuration_missing(self): 'This should not survive being overwritten by static content', ) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED, GOOGLE_ANALYTICS_4_ID='GA-abc') + @override_settings(CERTIFICATES_HTML_VIEW=True, GOOGLE_ANALYTICS_4_ID='GA-abc') @with_site_configuration(configuration={'platform_name': 'My Platform Site'}) def test_html_view_with_g4(self): test_url = get_certificate_url( diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 69831716565f..0d5d4d8dad09 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -51,13 +51,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() -FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True - -FEATURES_WITH_CERTS_DISABLED = settings.FEATURES.copy() -FEATURES_WITH_CERTS_DISABLED['CERTIFICATES_HTML_VIEW'] = False - -FEATURES_WITH_CUSTOM_CERTS_ENABLED = FEATURES_WITH_CERTS_ENABLED.copy() +FEATURES_WITH_CUSTOM_CERTS_ENABLED = settings.FEATURES.copy() FEATURES_WITH_CUSTOM_CERTS_ENABLED['CUSTOM_CERTIFICATE_TEMPLATES_ENABLED'] = True name_affirmation_service = get_name_affirmation_service() @@ -289,7 +283,7 @@ def setUp(self): 'max_effort': '10' } - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_linkedin_share_url(self): """ Test: LinkedIn share URL. @@ -314,7 +308,7 @@ def test_linkedin_share_url(self): js_escaped_string(self.linkedin_url.format(params=urlencode(params))), ) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) @with_site_configuration( configuration={ 'platform_name': 'My Platform Site', 'LINKEDIN_COMPANY_ID': 2448, @@ -348,7 +342,7 @@ def test_linkedin_share_url_site(self): "CERTIFICATE_FACEBOOK": True, "CERTIFICATE_FACEBOOK_TEXT": "test FB text" }) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_render_certificate_html_view_with_facebook_meta_tags(self): """ Test view html certificate if share to FB is enabled. @@ -381,7 +375,7 @@ def test_render_certificate_html_view_with_facebook_meta_tags(self): @patch.dict("django.conf.settings.SOCIAL_SHARING_SETTINGS", { "CERTIFICATE_FACEBOOK": False, }) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_render_certificate_html_view_without_facebook_meta_tags(self): """ Test view html certificate if share to FB is disabled. @@ -408,7 +402,7 @@ def test_render_certificate_html_view_without_facebook_meta_tags(self): self.assertNotContains(response, 'test_organization {self.course.number} Certificate |') self.assertContains(response, 'logo_test1.png') - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) @patch.dict("django.conf.settings.SOCIAL_SHARING_SETTINGS", { "CERTIFICATE_TWITTER": True, "CERTIFICATE_FACEBOOK": True, @@ -597,7 +591,7 @@ def test_rendering_maximum_data(self): # Test course overrides self.assertContains(response, "/static/certificates/images/course_override_logo.png") - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_render_html_view_valid_certificate(self): self._add_course_certificates(count=1, signatory_count=2) test_url = get_certificate_url( @@ -620,7 +614,7 @@ def test_render_html_view_valid_certificate(self): response = self.client.get(test_url) self.assertContains(response, str(self.cert.verify_uuid)) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_render_certificate_only_for_downloadable_status(self): """ Tests that Certificate HTML Web View returns Certificate only if certificate status is 'downloadable', @@ -649,7 +643,7 @@ def test_render_certificate_only_for_downloadable_status(self): (CertificateStatuses.audit_notpassing, False), ) @ddt.unpack - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_audit_certificate_display(self, status, eligible_for_certificate): """ Ensure that audit-mode certs are only shown in the web view if they @@ -672,7 +666,7 @@ def test_audit_certificate_display(self, status, eligible_for_certificate): else: assert response.status_code == 404 - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_html_view_returns_404_for_invalid_certificate(self): """ Tests that Certificate HTML Web View successfully retrieves certificate only @@ -694,7 +688,7 @@ def test_html_view_returns_404_for_invalid_certificate(self): response = self.client.get(test_url) assert response.status_code == 404 - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_html_lang_attribute_is_dynamic_for_certificate_html_view(self): """ Tests that Certificate HTML Web View's lang attribute is based on user language. @@ -717,7 +711,7 @@ def test_html_lang_attribute_is_dynamic_for_certificate_html_view(self): self.assertContains(response, '') @ddt.data(False, True) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_html_view_for_non_viewable_certificate_and_for_student_user(self, date_override): """ Tests that Certificate HTML Web View returns "Cannot Find Certificate" @@ -755,7 +749,7 @@ def test_html_view_for_non_viewable_certificate_and_for_student_user(self, date_ self.assertContains(response, "Cannot Find Certificate") self.assertContains(response, "We cannot find a certificate with this URL or ID number.") - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_render_html_view_with_valid_signatories(self): self._add_course_certificates(count=1, signatory_count=2) test_url = get_certificate_url( @@ -771,7 +765,7 @@ def test_render_html_view_with_valid_signatories(self): self.assertContains(response, 'Signatory_Organization 0') self.assertContains(response, '/static/certificates/images/demo-sig0.png') - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_course_display_name_not_override_with_course_title(self): # if certificate in block has not course_title then course name should not be overridden with this title. test_certificates = [ @@ -798,7 +792,7 @@ def test_course_display_name_not_override_with_course_title(self): self.assertNotContains(response, 'test_course_title_0') self.assertContains(response, 'refundable course') - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_course_display_overrides(self): """ Tests if `Course Number Display String` or `Course Organization Display` is set for a course @@ -821,7 +815,7 @@ def test_course_display_overrides(self): self.assertContains(response, 'overridden_number') self.assertContains(response, 'overridden_org') - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_certificate_view_without_org_logo(self): test_certificates = [ { @@ -846,7 +840,7 @@ def test_certificate_view_without_org_logo(self): # make sure response html has only one organization logo container for edX self.assertContains(response, "
  • ", 1) - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_render_html_view_without_signatories(self): self._add_course_certificates(count=1, signatory_count=0) test_url = get_certificate_url( @@ -858,7 +852,7 @@ def test_render_html_view_without_signatories(self): self.assertNotContains(response, 'Signatory_Name 0') self.assertNotContains(response, 'Signatory_Title 0') - @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_settings(CERTIFICATES_HTML_VIEW=True) def test_render_html_view_is_html_escaped(self): test_certificates = [ { @@ -887,7 +881,7 @@ def test_render_html_view_is_html_escaped(self): self.assertNotContains(response, '