Skip to content

Commit d30264b

Browse files
committed
refactor: remove ENABLE_HOME_PAGE_COURSE_API_V2 feature toggle
1 parent da0d365 commit d30264b

File tree

7 files changed

+27
-116
lines changed

7 files changed

+27
-116
lines changed

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,6 @@
1515
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
1616

1717

18-
FEATURES_WITH_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy()
19-
FEATURES_WITH_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = True
20-
FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy()
21-
FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = False
22-
23-
2418
@ddt.ddt
2519
class HomePageViewTest(CourseTestCase):
2620
"""
@@ -99,7 +93,6 @@ def test_taxonomy_list_link(self):
9993
)
10094

10195

102-
@override_settings(FEATURES=FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API)
10396
@ddt.ddt
10497
class HomePageCoursesViewTest(CourseTestCase):
10598
"""
@@ -141,7 +134,6 @@ def test_home_page_response(self):
141134
print(response.data)
142135
self.assertDictEqual(expected_response, response.data)
143136

144-
@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
145137
def test_home_page_response_with_api_v2(self):
146138
"""Check successful response content with api v2 modifications.
147139
@@ -171,7 +163,6 @@ def test_home_page_response_with_api_v2(self):
171163
self.assertEqual(response.status_code, status.HTTP_200_OK)
172164
self.assertDictEqual(expected_response, response.data)
173165

174-
@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
175166
@ddt.data(
176167
("active_only", "true", 2, 0),
177168
("archived_only", "true", 0, 1),
@@ -214,7 +205,6 @@ def test_filter_and_ordering_courses(
214205
self.assertEqual(len(response.data["archived_courses"]), expected_archived_length)
215206
self.assertEqual(len(response.data["courses"]), expected_active_length)
216207

217-
@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
218208
@ddt.data(
219209
("active_only", "true"),
220210
("archived_only", "true"),
@@ -231,7 +221,6 @@ def test_filter_and_ordering_no_courses_staff(self, filter_key, filter_value):
231221
self.assertEqual(len(response.data["courses"]), 0)
232222
self.assertEqual(response.status_code, status.HTTP_200_OK)
233223

234-
@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
235224
@ddt.data(
236225
("active_only", "true"),
237226
("archived_only", "true"),
@@ -243,7 +232,7 @@ def test_home_page_response_no_courses_non_staff(self, filter_key, filter_value)
243232
"""Test home page with org filter and ordering when there are no courses for a non-staff user."""
244233
self.course_overview.delete()
245234

246-
response = self.non_staff_client.get(self.url)
235+
response = self.non_staff_client.get(self.url, {filter_key: filter_value})
247236

248237
self.assertEqual(len(response.data["courses"]), 0)
249238
self.assertEqual(response.status_code, status.HTTP_200_OK)

cms/djangoapps/contentstore/rest_api/v2/views/home.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,7 @@ def get(self, request: Request):
126126
"in_process_course_actions": [],
127127
}
128128
```
129-
130-
if the `ENABLE_HOME_PAGE_COURSE_API_V2` feature flag is not enabled, an HTTP 404 "Not Found" response
131-
is returned.
132129
"""
133-
if not settings.FEATURES.get('ENABLE_HOME_PAGE_COURSE_API_V2', False):
134-
return HttpResponseNotFound()
135-
136130
courses, in_process_course_actions = get_course_context_v2(request)
137131
paginator = HomePageCoursesPaginator()
138132
courses_page = paginator.paginate_queryset(

cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,7 @@
1616
from cms.djangoapps.contentstore.utils import reverse_course_url
1717
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
1818

19-
FEATURES_WITH_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy()
20-
FEATURES_WITH_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = True
2119

22-
23-
@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
2420
@ddt.ddt
2521
class HomePageCoursesViewV2Test(CourseTestCase):
2622
"""
@@ -208,21 +204,6 @@ def test_page_query_if_passed(self):
208204
self.assertEqual(response.data["count"], 2)
209205
self.assertEqual(response.status_code, status.HTTP_200_OK)
210206

211-
@patch("cms.djangoapps.contentstore.views.course.CourseOverview")
212-
@patch("cms.djangoapps.contentstore.views.course.modulestore")
213-
def test_api_v2_is_disabled(self, mock_modulestore, mock_course_overview):
214-
"""Get list of courses when home page course v2 API is disabled.
215-
216-
Expected result:
217-
- Courses are read from the modulestore.
218-
"""
219-
with override_settings(FEATURES={'ENABLE_HOME_PAGE_COURSE_API_V2': False}):
220-
response = self.client.get(self.api_v1_url)
221-
222-
self.assertEqual(response.status_code, status.HTTP_200_OK)
223-
mock_modulestore().get_course_summaries.assert_called_once()
224-
mock_course_overview.get_all_courses.assert_not_called()
225-
226207
@ddt.data(
227208
("active_only", "true"),
228209
("archived_only", "true"),
@@ -245,7 +226,6 @@ def test_if_empty_list_of_courses(self, query_param, value):
245226
self.assertEqual(len(response.data['results']['courses']), 0)
246227
self.assertEqual(response.status_code, status.HTTP_200_OK)
247228

248-
@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
249229
@ddt.data(
250230
("active_only", "true", 2, 0),
251231
("archived_only", "true", 0, 1),

cms/djangoapps/contentstore/tests/test_course_listing.py

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,13 @@
33
by reversing group name formats.
44
"""
55

6-
76
import random
87
from unittest.mock import Mock, patch
98

109
import ddt
1110
from ccx_keys.locator import CCXLocator
1211
from django.conf import settings
13-
from django.test import RequestFactory, override_settings
12+
from django.test import RequestFactory
1413
from opaque_keys.edx.locations import CourseLocator
1514

1615
from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient
@@ -35,17 +34,12 @@
3534
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
3635
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
3736
from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES
38-
from xmodule.course_block import CourseSummary # lint-amnesty, pylint: disable=wrong-import-order
3937
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
4038
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
41-
from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order
39+
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
4240

4341
TOTAL_COURSES_COUNT = 10
4442
USER_COURSES_COUNT = 1
45-
FEATURES_WITH_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy()
46-
FEATURES_WITH_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = True
47-
FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy()
48-
FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = False
4943

5044

5145
@ddt.ddt
@@ -185,19 +179,7 @@ def test_staff_course_listing(self):
185179
courses_list_by_staff, __ = get_courses_accessible_to_user(self.request)
186180

187181
self.assertEqual(len(list(courses_list_by_staff)), TOTAL_COURSES_COUNT)
188-
189-
with override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API):
190-
# Verify fetched accessible courses list is a list of CourseOverview instances when home page course v2
191-
# api is enabled.
192-
self.assertTrue(all(isinstance(course, CourseOverview) for course in courses_list_by_staff))
193-
194-
with override_settings(FEATURES=FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API):
195-
# Verify fetched accessible courses list is a list of CourseSummery instances
196-
self.assertTrue(all(isinstance(course, CourseSummary) for course in courses_list_by_staff))
197-
198-
# Now count the db queries for staff
199-
with check_mongo_calls(2):
200-
list(_accessible_courses_summary_iter(self.request))
182+
self.assertTrue(all(isinstance(course, CourseOverview) for course in courses_list_by_staff))
201183

202184
def test_get_course_list_with_invalid_course_location(self):
203185
"""
@@ -212,21 +194,10 @@ def test_get_course_list_with_invalid_course_location(self):
212194
courses_list = list(courses_iter)
213195
self.assertEqual(len(courses_list), 1)
214196

215-
with override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API):
216-
# Verify fetched accessible courses list is a list of CourseOverview instances when home page course v2
217-
# api is enabled.
218-
courses_summary_iter, __ = _accessible_courses_summary_iter(self.request)
219-
courses_summary_list = list(courses_summary_iter)
220-
self.assertTrue(all(isinstance(course, CourseOverview) for course in courses_summary_list))
221-
self.assertEqual(len(courses_summary_list), 1)
222-
223-
with override_settings(FEATURES=FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API):
224-
# Verify fetched accessible courses list is a list of CourseSummery instances and only one course
225-
# is returned
226-
courses_summary_iter, __ = _accessible_courses_summary_iter(self.request)
227-
courses_summary_list = list(courses_summary_iter)
228-
self.assertTrue(all(isinstance(course, CourseSummary) for course in courses_summary_list))
229-
self.assertEqual(len(courses_summary_list), 1)
197+
courses_summary_iter, __ = _accessible_courses_summary_iter(self.request)
198+
courses_summary_list = list(courses_summary_iter)
199+
self.assertTrue(all(isinstance(course, CourseOverview) for course in courses_summary_list))
200+
self.assertEqual(len(courses_summary_list), 1)
230201

231202
# get courses by reversing group name formats
232203
courses_list_by_groups, __ = _accessible_courses_list_from_groups(self.request)

cms/djangoapps/contentstore/views/course.py

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -405,23 +405,15 @@ def course_filter(course_summary):
405405

406406
return has_studio_read_access(request.user, course_summary.id)
407407

408-
enable_home_page_api_v2 = settings.FEATURES["ENABLE_HOME_PAGE_COURSE_API_V2"]
409-
410-
if enable_home_page_api_v2:
411-
# If the new home page API is enabled, we should use the Django ORM to filter and order the courses
412-
courses_summary = CourseOverview.get_all_courses()
413-
else:
414-
courses_summary = modulestore().get_course_summaries()
415-
416-
if enable_home_page_api_v2:
417-
search_query, order, active_only, archived_only = get_query_params_if_present(request)
418-
courses_summary = get_filtered_and_ordered_courses(
419-
courses_summary,
420-
active_only,
421-
archived_only,
422-
search_query,
423-
order,
424-
)
408+
courses_summary = CourseOverview.get_all_courses()
409+
search_query, order, active_only, archived_only = get_query_params_if_present(request)
410+
courses_summary = get_filtered_and_ordered_courses(
411+
courses_summary,
412+
active_only,
413+
archived_only,
414+
search_query,
415+
order,
416+
)
425417

426418
courses_summary = filter(course_filter, courses_summary)
427419
in_process_course_actions = get_in_process_course_actions(request)

cms/djangoapps/contentstore/views/tests/test_course_index.py

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,6 @@
4040
from ..course import _deprecated_blocks_info, course_outline_initial_state, reindex_course_and_check_access
4141
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import VisibilityState, create_xblock_info
4242

43-
FEATURES_WITH_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy()
44-
FEATURES_WITH_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = True
45-
FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy()
46-
FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = False
47-
4843

4944
class TestCourseIndex(CourseTestCase):
5045
"""
@@ -430,16 +425,15 @@ def check_index_page(self, separate_archived_courses, org):
430425
archived_course_tab = parsed_html.find_class('archived-courses')
431426
self.assertEqual(len(archived_course_tab), 1 if separate_archived_courses else 0)
432427

433-
@override_settings(FEATURES=FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API)
434428
@ddt.data(
435429
# Staff user has course staff access
436430
(True, 'staff', None, 0, 23),
437431
(False, 'staff', None, 0, 23),
438432
# Base user has global staff access
439-
(True, 'user', ORG, 2, 23),
440-
(False, 'user', ORG, 2, 23),
441-
(True, 'user', None, 2, 23),
442-
(False, 'user', None, 2, 23),
433+
(True, 'user', ORG, 0, 23),
434+
(False, 'user', ORG, 0, 23),
435+
(True, 'user', None, 0, 23),
436+
(False, 'user', None, 0, 23),
443437
)
444438
@ddt.unpack
445439
def test_separate_archived_courses(self, separate_archived_courses, username, org, mongo_queries, sql_queries):
@@ -456,12 +450,13 @@ def test_separate_archived_courses(self, separate_archived_courses, username, or
456450
features = settings.FEATURES.copy()
457451
features['ENABLE_SEPARATE_ARCHIVED_COURSES'] = separate_archived_courses
458452
with override_settings(FEATURES=features):
459-
self.check_index_page_with_query_count(separate_archived_courses=separate_archived_courses,
460-
org=org,
461-
mongo_queries=mongo_queries,
462-
sql_queries=sql_queries)
453+
self.check_index_page_with_query_count(
454+
separate_archived_courses=separate_archived_courses,
455+
org=org,
456+
mongo_queries=mongo_queries,
457+
sql_queries=sql_queries,
458+
)
463459

464-
@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API)
465460
@ddt.data(
466461
# Staff user has course staff access
467462
(True, 'staff', None, 0, 23),

cms/envs/common.py

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -547,16 +547,6 @@
547547
# .. toggle_tickets: https://github.com/openedx/edx-platform/pull/33952
548548
'ENABLE_HIDE_FROM_TOC_UI': False,
549549

550-
# .. toggle_name: FEATURES['ENABLE_HOME_PAGE_COURSE_API_V2']
551-
# .. toggle_implementation: DjangoSetting
552-
# .. toggle_default: True
553-
# .. toggle_description: Enables the new home page course v2 API, which is a new version of the home page course
554-
# API with pagination, filter and ordering capabilities.
555-
# .. toggle_use_cases: open_edx
556-
# .. toggle_creation_date: 2024-03-14
557-
# .. toggle_tickets: https://github.com/openedx/edx-platform/pull/34173
558-
'ENABLE_HOME_PAGE_COURSE_API_V2': True,
559-
560550
# .. toggle_name: FEATURES['ENABLE_GRADING_METHOD_IN_PROBLEMS']
561551
# .. toggle_implementation: DjangoSetting
562552
# .. toggle_default: False

0 commit comments

Comments
 (0)