From e792242b5f0d52f70652549f89a974fcce0fa82e Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Tue, 2 Mar 2021 11:36:43 -0500 Subject: [PATCH] refactor!: remove redundant ENABLE_COURSEWARE_MICROFRONTEND toggle (#26792) The Django setting FEATURES['ENABLE_COURSEWARE_MICROFRONTEND'] has been an additional gate to activating usage of the Learning MFE for an Open edX instance. The toggle is redundant with the `courseware.courseware_mfe` Waffle flag. By removing it, we simplify our config and simplify our path towards making the Learning MFE the default courseware experience. TNL-7796 --- lms/djangoapps/courseware/tests/test_views.py | 32 ++----------------- lms/djangoapps/courseware/toggles.py | 14 ++++---- lms/djangoapps/courseware/views/index.py | 7 ---- lms/envs/common.py | 16 +--------- lms/envs/devstack.py | 3 -- lms/envs/devstack_decentralized.py | 3 -- .../core/djangoapps/courseware_api/views.py | 12 ++----- 7 files changed, 13 insertions(+), 74 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 8f57110b09..a166eee551 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -3244,41 +3244,16 @@ class TestShowCoursewareMFE(TestCase): There are an unfortunate number of state permutations here since we have the product of the following binary states: - * the ENABLE_COURSEWARE_MICROFRONTEND Django setting * user is global staff member * user is member of the course team * whether the course_key is an old Mongo style of key * the COURSEWARE_MICROFRONTEND_COURSE_TEAM_PREVIEW CourseWaffleFlag * the REDIRECT_TO_COURSEWARE_MICROFRONTEND ExperimentWaffleFlag - Giving us theoretically 2^6 = 64 states. >_< + Giving us theoretically 2^5 = 32 states. >_< """ - @patch.dict(settings.FEATURES, {'ENABLE_COURSEWARE_MICROFRONTEND': False}) - def test_disabled_at_platform_level(self): - """Test every permutation where the platform feature is disabled.""" - old_course_key = CourseKey.from_string("OpenEdX/Old/2020") - new_course_key = CourseKey.from_string("course-v1:OpenEdX+New+2020") - global_staff_user = UserFactory(username="global_staff", is_staff=True) - regular_user = UserFactory(username="normal", is_staff=False) - - # We never show when the feature is entirely disabled, no matter what - # the waffle flags are set to, who the user is, or what the course_key - # type is. - combos = itertools.product( - [regular_user, global_staff_user], # User (is global staff) - [old_course_key, new_course_key], # Course Key (old vs. new) - [True, False], # is_course_staff - [True, False], # preview_active (COURSEWARE_MICROFRONTEND_COURSE_TEAM_PREVIEW) - [True, False], # redirect_active (REDIRECT_TO_COURSEWARE_MICROFRONTEND) - ) - for user, course_key, is_course_staff, preview_active, redirect_active in combos: - with override_waffle_flag(COURSEWARE_MICROFRONTEND_COURSE_TEAM_PREVIEW, preview_active): - with override_waffle_flag(REDIRECT_TO_COURSEWARE_MICROFRONTEND, active=redirect_active): - assert show_courseware_mfe_link(user, is_course_staff, course_key) is False - - @patch.dict(settings.FEATURES, {'ENABLE_COURSEWARE_MICROFRONTEND': True}) - def test_enabled_at_platform_level(self): - """Test every permutation where the platform feature is enabled.""" + def test_permuations(self): + """Test every permutation""" old_course_key = CourseKey.from_string("OpenEdX/Old/2020") new_course_key = CourseKey.from_string("course-v1:OpenEdX+New+2020") global_staff_user = UserFactory(username="global_staff", is_staff=True) @@ -3366,7 +3341,6 @@ class TestShowCoursewareMFE(TestCase): ) -@patch.dict('django.conf.settings.FEATURES', {'ENABLE_COURSEWARE_MICROFRONTEND': True}) @ddt.ddt class MFERedirectTests(BaseViewsTestCase): # lint-amnesty, pylint: disable=missing-class-docstring MODULESTORE = TEST_DATA_SPLIT_MODULESTORE diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py index f1f4924798..4b9e96be9b 100644 --- a/lms/djangoapps/courseware/toggles.py +++ b/lms/djangoapps/courseware/toggles.py @@ -16,8 +16,7 @@ WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='courseware') # .. toggle_use_cases: temporary, open_edx # .. toggle_creation_date: 2020-01-29 # .. toggle_target_removal_date: 2020-12-31 -# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and -# ENABLE_COURSEWARE_MICROFRONTEND. +# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL. # .. toggle_tickets: DEPR-109 REDIRECT_TO_COURSEWARE_MICROFRONTEND = CourseWaffleFlag( WAFFLE_FLAG_NAMESPACE, 'courseware_mfe', __name__ @@ -32,8 +31,7 @@ REDIRECT_TO_COURSEWARE_MICROFRONTEND = CourseWaffleFlag( # .. toggle_use_cases: temporary, open_edx # .. toggle_creation_date: 2020-03-09 # .. toggle_target_removal_date: 2020-12-31 -# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and -# ENABLE_COURSEWARE_MICROFRONTEND. +# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL. # .. toggle_tickets: DEPR-109 COURSEWARE_MICROFRONTEND_COURSE_TEAM_PREVIEW = CourseWaffleFlag( WAFFLE_FLAG_NAMESPACE, 'microfrontend_course_team_preview', __name__ @@ -49,7 +47,7 @@ COURSEWARE_MICROFRONTEND_COURSE_TEAM_PREVIEW = CourseWaffleFlag( # .. toggle_use_cases: open_edx, temporary # .. toggle_creation_date: 2020-10-02 # .. toggle_target_removal_date: None -# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and ENABLE_COURSEWARE_MICROFRONTEND. +# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL. # .. toggle_tickets: AA-188 COURSEWARE_MICROFRONTEND_COURSE_EXIT_PAGE = CourseWaffleFlag( WAFFLE_FLAG_NAMESPACE, 'microfrontend_course_exit_page', __name__ @@ -63,7 +61,7 @@ COURSEWARE_MICROFRONTEND_COURSE_EXIT_PAGE = CourseWaffleFlag( # .. toggle_use_cases: temporary, open_edx # .. toggle_creation_date: 2020-10-07 # .. toggle_target_removal_date: none -# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and ENABLE_COURSEWARE_MICROFRONTEND. +# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL. # .. toggle_tickets: AA-371 COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES = CourseWaffleFlag( WAFFLE_FLAG_NAMESPACE, 'mfe_progress_milestones', __name__ @@ -78,7 +76,7 @@ COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES = CourseWaffleFlag( # .. toggle_use_cases: temporary, open_edx # .. toggle_creation_date: 2020-10-07 # .. toggle_target_removal_date: None -# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and ENABLE_COURSEWARE_MICROFRONTEND and +# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and # COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES. # .. toggle_tickets: AA-371 COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_FIRST_SECTION_CELEBRATION = CourseWaffleFlag( @@ -94,7 +92,7 @@ COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_FIRST_SECTION_CELEBRATION = CourseW # .. toggle_use_cases: temporary, open_edx # .. toggle_creation_date: 2021-02-16 # .. toggle_target_removal_date: None -# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and ENABLE_COURSEWARE_MICROFRONTEND and +# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and # COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES. # .. toggle_tickets: AA-304 COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_STREAK_CELEBRATION = CourseWaffleFlag( diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 1255f48c30..5438ada7d8 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -173,9 +173,6 @@ class CoursewareIndex(View): Redirect to the new courseware micro frontend, unless this is a time limited exam. """ - # DENY: feature disabled globally - if not settings.FEATURES.get('ENABLE_COURSEWARE_MICROFRONTEND'): - return # DENY: staff access if self.is_staff: return @@ -625,10 +622,6 @@ def show_courseware_mfe_link(user, staff_access, course_key): """ Return whether to display the button to switch to the Courseware MFE. """ - # The MFE isn't enabled at all, so don't show the button. - if not settings.FEATURES.get('ENABLE_COURSEWARE_MICROFRONTEND'): - return False - # MFE does not work for Old Mongo courses. if course_key.deprecated: return False diff --git a/lms/envs/common.py b/lms/envs/common.py index bf2af92ea3..d77ddfcac5 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -837,19 +837,6 @@ FEATURES = { # .. toggle_tickets: 'https://github.com/edx/edx-platform/pull/21616' 'ENABLE_CHANGE_USER_PASSWORD_ADMIN': False, - # .. toggle_name: FEATURES['ENABLE_COURSEWARE_MICROFRONTEND'] - # .. toggle_implementation: DjangoSetting - # .. toggle_default: False - # .. toggle_description: Set to True to enable the Courseware MFE at the platform level for global staff (see - # REDIRECT_TO_COURSEWARE_MICROFRONTEND for course rollout) - # .. toggle_use_cases: open_edx - # .. toggle_creation_date: 2020-03-05 - # .. toggle_target_removal_date: None - # .. toggle_tickets: DEPR-109 - # .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and see REDIRECT_TO_COURSEWARE_MICROFRONTEND for - # rollout. - 'ENABLE_COURSEWARE_MICROFRONTEND': False, - # .. toggle_name: FEATURES['ENABLE_AUTHN_MICROFRONTEND'] # .. toggle_implementation: DjangoSetting # .. toggle_default: False @@ -4530,8 +4517,7 @@ PROGRAM_CONSOLE_MICROFRONTEND_URL = None # .. setting_name: LEARNING_MICROFRONTEND_URL # .. setting_default: None # .. setting_description: Base URL of the micro-frontend-based courseware page. -# .. setting_warning: Also set site's ENABLE_COURSEWARE_MICROFRONTEND or -# FEATURES['ENABLE_COURSEWARE_MICROFRONTEND'] and courseware.courseware_mfe waffle flag +# .. setting_warning: Also set site's courseware.courseware_mfe waffle flag. LEARNING_MICROFRONTEND_URL = None ############### Settings for the ace_common plugin ################# diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index f34dc185a2..6ae9d0605e 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -215,9 +215,6 @@ FEATURES['ENABLE_COSMETIC_DISPLAY_PRICE'] = True ######################### Program Enrollments ##################### FEATURES['ENABLE_ENROLLMENT_RESET'] = True -######################### New Courseware MFE ##################### -FEATURES['ENABLE_COURSEWARE_MICROFRONTEND'] = True - ########################## Third Party Auth ####################### if FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and ( diff --git a/lms/envs/devstack_decentralized.py b/lms/envs/devstack_decentralized.py index f288787b1a..6268a10e99 100644 --- a/lms/envs/devstack_decentralized.py +++ b/lms/envs/devstack_decentralized.py @@ -180,9 +180,6 @@ FEATURES['ENABLE_COSMETIC_DISPLAY_PRICE'] = True ######################### Program Enrollments ##################### FEATURES['ENABLE_ENROLLMENT_RESET'] = True -######################### New Courseware MFE ##################### -FEATURES['ENABLE_COURSEWARE_MICROFRONTEND'] = True - ########################## Third Party Auth ####################### if FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and ( diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index c24c84732f..e64501ff89 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -89,20 +89,14 @@ class CoursewareMeta: This method is the "opposite" of _redirect_to_learning_mfe in lms/djangoapps/courseware/views/index.py. But not exactly... - 1. It needs to respect the global - ENABLE_COURSEWARE_MICROFRONTEND feature flag and redirect users - out of the MFE experience if it's turned off. - 2. It needs to redirect for old Mongo courses. - 3. It does NOT need to worry about exams - the MFE will handle + 1. It needs to redirect for old Mongo courses. + 2. It does NOT need to worry about exams - the MFE will handle those on its own. As of this writing, it will redirect back to the LMS experience, but that may change soon. - 4. Finally, it needs to redirect users who are bucketed out of + 3. Finally, it needs to redirect users who are bucketed out of the MFE experience, but who aren't staff. Staff are allowed to stay. """ - # REDIRECT: feature disabled globally - if not settings.FEATURES.get('ENABLE_COURSEWARE_MICROFRONTEND'): - return False # REDIRECT: Old Mongo courses, until removed from platform if self.course_key.deprecated: return False