From 838977a8f3981be491a7fa2441724e4e6e92d895 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Date: Wed, 1 Jan 2025 20:18:10 +0500 Subject: [PATCH 1/3] feat!: Remove DEPR waffle switch: ENABLE_GLOBAL_STAFF_OPTIMIZATION --- .../rest_api/v1/serializers/home.py | 1 - .../contentstore/rest_api/v1/views/home.py | 1 - .../rest_api/v1/views/tests/test_home.py | 23 ----------------- .../rest_api/v2/views/tests/test_home.py | 25 ------------------- cms/djangoapps/contentstore/utils.py | 16 ++---------- cms/djangoapps/contentstore/views/course.py | 20 +++------------ 6 files changed, 6 insertions(+), 80 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py index fa2a651f8a28..36166e42a5a6 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py @@ -62,7 +62,6 @@ class StudioHomeSerializer(serializers.Serializer): libraries_v2_enabled = serializers.BooleanField() taxonomies_enabled = serializers.BooleanField() taxonomy_list_mfe_url = serializers.CharField() - optimization_enabled = serializers.BooleanField() request_course_creator_url = serializers.CharField() rerun_creator_status = serializers.BooleanField() show_new_library_button = serializers.BooleanField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/home.py b/cms/djangoapps/contentstore/rest_api/v1/views/home.py index 3de536d78092..2d5360d38d6c 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/home.py @@ -62,7 +62,6 @@ def get(self, request: Request): "libraries_v1_enabled": true, "libraries_v2_enabled": true, "library_authoring_mfe_url": "//localhost:3001/course/course-v1:edX+P315+2T2023", - "optimization_enabled": true, "request_course_creator_url": "/request_course_creator", "rerun_creator_status": true, "show_new_library_button": true, diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index a4a6909c5dcb..e78558bba330 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -15,7 +15,6 @@ from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase -from cms.djangoapps.contentstore.views.course import ENABLE_GLOBAL_STAFF_OPTIMIZATION from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from xmodule.modulestore.tests.factories import CourseFactory @@ -52,7 +51,6 @@ def setUp(self): "libraries_v2_enabled": False, "taxonomies_enabled": True, "taxonomy_list_mfe_url": 'http://course-authoring-mfe/taxonomies', - "optimization_enabled": False, "request_course_creator_url": "/request_course_creator", "rerun_creator_status": True, "show_new_library_button": True, @@ -242,27 +240,6 @@ def test_home_page_response_no_courses_non_staff(self, filter_key, filter_value) self.assertEqual(len(response.data["courses"]), 0) self.assertEqual(response.status_code, status.HTTP_200_OK) - @override_waffle_switch(ENABLE_GLOBAL_STAFF_OPTIMIZATION, True) - def test_org_query_if_passed(self): - """Test home page when org filter passed as a query param""" - foo_course = self.store.make_course_key('foo-org', 'bar-number', 'baz-run') - test_course = CourseFactory.create( - org=foo_course.org, - number=foo_course.course, - run=foo_course.run - ) - CourseOverviewFactory.create(id=test_course.id, org='foo-org') - response = self.client.get(self.url, {"org": "foo-org"}) - self.assertEqual(len(response.data['courses']), 1) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - @override_waffle_switch(ENABLE_GLOBAL_STAFF_OPTIMIZATION, True) - def test_org_query_if_empty(self): - """Test home page with an empty org query param""" - response = self.client.get(self.url) - self.assertEqual(len(response.data['courses']), 0) - self.assertEqual(response.status_code, status.HTTP_200_OK) - @ddt.ddt class HomePageLibrariesViewTest(LibraryTestCase): diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py index e773e7f213c6..bf3332e1e3cd 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -15,7 +15,6 @@ from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.utils import reverse_course_url -from cms.djangoapps.contentstore.views.course import ENABLE_GLOBAL_STAFF_OPTIMIZATION from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory FEATURES_WITH_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy() @@ -104,30 +103,6 @@ def test_home_page_response(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertDictEqual(expected_response, response.data) - @override_waffle_switch(ENABLE_GLOBAL_STAFF_OPTIMIZATION, True) - def test_org_query_if_passed(self): - """Get list of courses when org filter passed as a query param. - - Expected result: - - A list of courses available to the logged in user for the specified org. - """ - response = self.client.get(self.api_v2_url, {"org": "demo-org"}) - - self.assertEqual(len(response.data['results']['courses']), 1) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - @override_waffle_switch(ENABLE_GLOBAL_STAFF_OPTIMIZATION, True) - def test_org_query_if_empty(self): - """Get home page with an empty org query param. - - Expected result: - - An empty list of courses available to the logged in user. - """ - response = self.client.get(self.api_v2_url) - - self.assertEqual(len(response.data['results']['courses']), 0) - self.assertEqual(response.status_code, status.HTTP_200_OK) - def test_active_only_query_if_passed(self): """Get list of active courses only. diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 7023bcaefaf7..a220b8d91399 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1595,7 +1595,6 @@ def get_course_context(request): from cms.djangoapps.contentstore.views.course import ( get_courses_accessible_to_user, _process_courses_list, - ENABLE_GLOBAL_STAFF_OPTIMIZATION, ) def format_in_process_course_view(uca): @@ -1619,10 +1618,7 @@ def format_in_process_course_view(uca): ) if uca.state == CourseRerunUIStateManager.State.FAILED else '' } - optimization_enabled = GlobalStaff().has_user(request.user) and ENABLE_GLOBAL_STAFF_OPTIMIZATION.is_enabled() - - org = request.GET.get('org', '') if optimization_enabled else None - courses_iter, in_process_course_actions = get_courses_accessible_to_user(request, org) + courses_iter, in_process_course_actions = get_courses_accessible_to_user(request) split_archived = settings.FEATURES.get('ENABLE_SEPARATE_ARCHIVED_COURSES', False) active_courses, archived_courses = _process_courses_list(courses_iter, in_process_course_actions, split_archived) in_process_course_actions = [format_in_process_course_view(uca) for uca in in_process_course_actions] @@ -1637,7 +1633,6 @@ def get_course_context_v2(request): # 'cms.djangoapps.contentstore.utils' (most likely due to a circular import) from cms.djangoapps.contentstore.views.course import ( get_courses_accessible_to_user, - ENABLE_GLOBAL_STAFF_OPTIMIZATION, ) def format_in_process_course_view(uca): @@ -1664,10 +1659,7 @@ def format_in_process_course_view(uca): ) if uca.state == CourseRerunUIStateManager.State.FAILED else '' } - optimization_enabled = GlobalStaff().has_user(request.user) and ENABLE_GLOBAL_STAFF_OPTIMIZATION.is_enabled() - - org = request.GET.get('org', '') if optimization_enabled else None - courses_iter, in_process_course_actions = get_courses_accessible_to_user(request, org) + courses_iter, in_process_course_actions = get_courses_accessible_to_user(request) in_process_course_actions = [format_in_process_course_view(uca) for uca in in_process_course_actions] return courses_iter, in_process_course_actions @@ -1685,7 +1677,6 @@ def get_home_context(request, no_course=False): _accessible_libraries_iter, _get_course_creator_status, _format_library_for_view, - ENABLE_GLOBAL_STAFF_OPTIMIZATION, ) from cms.djangoapps.contentstore.views.library import ( user_can_view_create_library_button, @@ -1698,8 +1689,6 @@ def get_home_context(request, no_course=False): archived_courses = [] in_process_course_actions = [] - optimization_enabled = GlobalStaff().has_user(request.user) and ENABLE_GLOBAL_STAFF_OPTIMIZATION.is_enabled() - user = request.user libraries = [] @@ -1728,7 +1717,6 @@ def get_home_context(request, no_course=False): 'rerun_creator_status': GlobalStaff().has_user(user), 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False), 'allow_course_reruns': settings.FEATURES.get('ALLOW_COURSE_RERUNS', True), - 'optimization_enabled': optimization_enabled, 'active_tab': 'courses', 'allowed_organizations': get_allowed_organizations(user), 'allowed_organizations_for_libraries': get_allowed_organizations_for_libraries(user), diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 244804c3062b..e05ac894d6d8 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -139,9 +139,6 @@ 'get_course_and_check_access'] WAFFLE_NAMESPACE = 'studio_home' -ENABLE_GLOBAL_STAFF_OPTIMIZATION = WaffleSwitch( # lint-amnesty, pylint: disable=toggle-missing-annotation - f'{WAFFLE_NAMESPACE}.enable_global_staff_optimization', __name__ -) class AccessListFallback(Exception): @@ -394,15 +391,12 @@ def get_in_process_course_actions(request): ] -def _accessible_courses_summary_iter(request, org=None): +def _accessible_courses_summary_iter(request): """ List all courses available to the logged in user by iterating through all the courses Arguments: request: the request object - org (string): if not None, this value will limit the courses returned. An empty - string will result in no courses, and otherwise only courses with the - specified org will be returned. The default value is None. """ def course_filter(course_summary): """ @@ -416,9 +410,7 @@ def course_filter(course_summary): enable_home_page_api_v2 = settings.FEATURES["ENABLE_HOME_PAGE_COURSE_API_V2"] - if org is not None: - courses_summary = [] if org == '' else CourseOverview.get_all_courses(orgs=[org]) - elif enable_home_page_api_v2: + if enable_home_page_api_v2: # If the new home page API is enabled, we should use the Django ORM to filter and order the courses courses_summary = CourseOverview.get_all_courses() else: @@ -765,21 +757,17 @@ def course_index(request, course_key): @function_trace('get_courses_accessible_to_user') -def get_courses_accessible_to_user(request, org=None): +def get_courses_accessible_to_user(request): """ Try to get all courses by first reversing django groups and fallback to old method if it fails Note: overhead of pymongo reads will increase if getting courses from django groups fails Arguments: request: the request object - org (string): for global staff users ONLY, this value will be used to limit - the courses returned. A value of None will have no effect (all courses - returned), an empty string will result in no courses, and otherwise only courses with the - specified org will be returned. The default value is None. """ if GlobalStaff().has_user(request.user): # user has global access so no need to get courses from django groups - courses, in_process_course_actions = _accessible_courses_summary_iter(request, org) + courses, in_process_course_actions = _accessible_courses_summary_iter(request) else: try: courses, in_process_course_actions = _accessible_courses_list_from_groups(request) From f6f72290b79de36f3c045355a5acc725c8fef3a9 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Date: Wed, 1 Jan 2025 21:14:21 +0500 Subject: [PATCH 2/3] build: Remove unused imports --- .../contentstore/rest_api/v1/views/tests/test_home.py | 4 ---- .../contentstore/rest_api/v2/views/tests/test_home.py | 1 - cms/djangoapps/contentstore/views/course.py | 1 - 3 files changed, 6 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index e78558bba330..31cb606b5d4b 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -8,15 +8,11 @@ from django.conf import settings from django.test import override_settings from django.urls import reverse -from edx_toggles.toggles.testutils import ( - override_waffle_switch, -) from rest_framework import status from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory -from xmodule.modulestore.tests.factories import CourseFactory FEATURES_WITH_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy() diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py index bf3332e1e3cd..e899019b4f17 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -10,7 +10,6 @@ from django.conf import settings from django.test import override_settings from django.urls import reverse -from edx_toggles.toggles.testutils import override_waffle_switch from rest_framework import status from cms.djangoapps.contentstore.tests.utils import CourseTestCase diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index e05ac894d6d8..3b8cb7659e0e 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -23,7 +23,6 @@ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.http import require_GET, require_http_methods from edx_django_utils.monitoring import function_trace -from edx_toggles.toggles import WaffleSwitch from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import BlockUsageLocator From 343d5216193ed4df539d6bd14fa59cad82821963 Mon Sep 17 00:00:00 2001 From: Muhammad Arslan Date: Thu, 2 Jan 2025 22:42:19 +0500 Subject: [PATCH 3/3] chore: Remove unused variable and rebased --- cms/djangoapps/contentstore/views/course.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 3b8cb7659e0e..064cb1ad25e0 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -137,8 +137,6 @@ 'group_configurations_list_handler', 'group_configurations_detail_handler', 'get_course_and_check_access'] -WAFFLE_NAMESPACE = 'studio_home' - class AccessListFallback(Exception): """