diff --git a/cms/djangoapps/contentstore/rest_api/urls.py b/cms/djangoapps/contentstore/rest_api/urls.py index 1d4ff8d4ab6d..7296f7bb9858 100644 --- a/cms/djangoapps/contentstore/rest_api/urls.py +++ b/cms/djangoapps/contentstore/rest_api/urls.py @@ -7,10 +7,12 @@ from .v0 import urls as v0_urls from .v1 import urls as v1_urls +from .v2 import urls as v2_urls app_name = 'cms.djangoapps.contentstore' urlpatterns = [ path('v0/', include(v0_urls)), path('v1/', include(v1_urls)), + path('v2/', include(v2_urls)) ] 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 5279af0b1297..a7e2e8f03aa8 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 @@ -2,7 +2,9 @@ Unit tests for home page view. """ import ddt +from collections import OrderedDict 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, @@ -18,6 +20,12 @@ from xmodule.modulestore.tests.factories import CourseFactory +FEATURES_WITH_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy() +FEATURES_WITH_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = True +FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy() +FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = False + + @ddt.ddt class HomePageViewTest(CourseTestCase): """ @@ -74,6 +82,7 @@ def test_taxonomy_list_link(self): ) +@override_settings(FEATURES=FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API) @ddt.ddt class HomePageCoursesViewTest(CourseTestCase): """ @@ -83,6 +92,12 @@ class HomePageCoursesViewTest(CourseTestCase): def setUp(self): super().setUp() self.url = reverse("cms.djangoapps.contentstore:v1:courses") + CourseOverviewFactory.create( + id=self.course.id, + org=self.course.org, + display_name=self.course.display_name, + display_number_with_default=self.course.number, + ) def test_home_page_response(self): """Check successful response content""" @@ -108,6 +123,36 @@ def test_home_page_response(self): print(response.data) self.assertDictEqual(expected_response, response.data) + def test_home_page_response_with_api_v2(self): + """Check successful response content with api v2 modifications. + + When the feature flag is enabled, the courses are exclusively fetched from the CourseOverview model, so + the values in the courses' list are OrderedDicts instead of the default dictionaries. + """ + course_id = str(self.course.id) + expected_response = { + "archived_courses": [], + "courses": [ + OrderedDict([ + ("course_key", course_id), + ("display_name", self.course.display_name), + ("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'), + ("number", self.course.number), + ("org", self.course.org), + ("rerun_link", f'/course_rerun/{course_id}'), + ("run", self.course.id.run), + ("url", f'/course/{course_id}'), + ]), + ], + "in_process_course_actions": [], + } + + with override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API): + response = self.client.get(self.url) + + 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): """Test home page when org filter passed as a query param""" diff --git a/cms/djangoapps/contentstore/rest_api/v2/__init__.py b/cms/djangoapps/contentstore/rest_api/v2/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py new file mode 100644 index 000000000000..6e102bab44a1 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/__init__.py @@ -0,0 +1,3 @@ +"""Module for v2 serializers.""" + +from cms.djangoapps.contentstore.rest_api.v2.serializers.home import CourseHomeTabSerializerV2 diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/home.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/home.py new file mode 100644 index 000000000000..857291fe838b --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/home.py @@ -0,0 +1,68 @@ +""" +API Serializers for course home V2 API. +""" +from django.conf import settings +from rest_framework import serializers + +from cms.djangoapps.contentstore.utils import get_lms_link_for_item, reverse_course_url +from cms.djangoapps.contentstore.views.course import _get_rerun_link_for_item +from openedx.core.lib.api.serializers import CourseKeyField + + +class UnsucceededCourseSerializerV2(serializers.Serializer): + """Serializer for unsucceeded course.""" + + display_name = serializers.CharField() + course_key = CourseKeyField() + org = serializers.CharField() + number = serializers.CharField() + run = serializers.CharField() + is_failed = serializers.BooleanField() + is_in_progress = serializers.BooleanField() + dismiss_link = serializers.CharField() + + +class CourseCommonSerializerV2(serializers.Serializer): + """Serializer for course common fields V2.""" + + course_key = CourseKeyField(source='id') + display_name = serializers.CharField() + lms_link = serializers.SerializerMethodField() + cms_link = serializers.SerializerMethodField() + number = serializers.CharField() + org = serializers.CharField() + rerun_link = serializers.SerializerMethodField() + run = serializers.CharField(source='id.run') + url = serializers.SerializerMethodField() + is_active = serializers.SerializerMethodField() + + def get_lms_link(self, obj): + """Get LMS link for course.""" + return get_lms_link_for_item(obj.location) + + def get_cms_link(self, obj): + """Get CMS link for course.""" + return f"//{settings.CMS_BASE}{reverse_course_url('course_handler', obj.id)}" + + def get_rerun_link(self, obj): + """Get rerun link for course.""" + return _get_rerun_link_for_item(obj.id) + + def get_url(self, obj): + """Get URL from the course handler.""" + return reverse_course_url('course_handler', obj.id) + + def get_is_active(self, obj): + """Get whether the course is active or not.""" + return not obj.has_ended() + + +class CourseHomeTabSerializerV2(serializers.Serializer): + """Serializer for course home tab V2 with unsucceeded courses and in process course actions.""" + + courses = CourseCommonSerializerV2(required=False, many=True) + in_process_course_actions = UnsucceededCourseSerializerV2( + many=True, + required=False, + allow_null=True + ) diff --git a/cms/djangoapps/contentstore/rest_api/v2/urls.py b/cms/djangoapps/contentstore/rest_api/v2/urls.py new file mode 100644 index 000000000000..ad61cc937015 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/urls.py @@ -0,0 +1,15 @@ +"""Contenstore API v2 URLs.""" + +from django.urls import path + +from cms.djangoapps.contentstore.rest_api.v2.views import HomePageCoursesViewV2 + +app_name = "v2" + +urlpatterns = [ + path( + "home/courses", + HomePageCoursesViewV2.as_view(), + name="courses", + ), +] diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py new file mode 100644 index 000000000000..73ddde98440c --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py @@ -0,0 +1,3 @@ +"""Module for v2 views.""" + +from cms.djangoapps.contentstore.rest_api.v2.views.home import HomePageCoursesViewV2 diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py new file mode 100644 index 000000000000..c32741510157 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -0,0 +1,147 @@ +"""HomePageCoursesViewV2 APIView for getting content available to the logged in user.""" +import edx_api_doc_tools as apidocs +from collections import OrderedDict +from django.conf import settings +from django.http import HttpResponseNotFound +from rest_framework.response import Response +from rest_framework.request import Request +from rest_framework.views import APIView +from rest_framework.pagination import PageNumberPagination + +from openedx.core.lib.api.view_utils import view_auth_classes + +from cms.djangoapps.contentstore.utils import get_course_context_v2 +from cms.djangoapps.contentstore.rest_api.v2.serializers import CourseHomeTabSerializerV2 + + +class HomePageCoursesPaginator(PageNumberPagination): + """Custom paginator for the home page courses view version 2.""" + + def get_paginated_response(self, data): + """Return a paginated style `Response` object for the given output data.""" + return Response(OrderedDict([ + ('count', self.page.paginator.count), + ('num_pages', self.page.paginator.num_pages), + ('next', self.get_next_link()), + ('previous', self.get_previous_link()), + ('results', data), + ])) + + def paginate_queryset(self, queryset, request, view=None): + """ + Paginate a queryset if required, either returning a page object, + or `None` if pagination is not configured for this view. + + This method is a modified version of the original `paginate_queryset` method + from the `PageNumberPagination` class. The original method was modified to + handle the case where the `queryset` is a `filter` object. + """ + if isinstance(queryset, filter): + queryset = list(queryset) + + return super().paginate_queryset(queryset, request, view) + + +@view_auth_classes(is_authenticated=True) +class HomePageCoursesViewV2(APIView): + """View for getting all courses available to the logged in user.""" + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "org", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course org", + ), + apidocs.string_parameter( + "search", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course name, org, or number", + ), + apidocs.string_parameter( + "order", + apidocs.ParameterLocation.QUERY, + description="Query param to order by course name, org, or number", + ), + apidocs.string_parameter( + "active_only", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by active courses only", + ), + apidocs.string_parameter( + "archived_only", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by archived courses only", + ), + apidocs.string_parameter( + "page", + apidocs.ParameterLocation.QUERY, + description="Query param to paginate the courses", + ), + ], + responses={ + 200: CourseHomeTabSerializerV2, + 401: "The requester is not authenticated.", + }, + ) + def get(self, request: Request): + """ + Get an object containing all courses. + + **Example Request** + + GET /api/contentstore/v2/home/courses + GET /api/contentstore/v2/home/courses?org=edX + GET /api/contentstore/v2/home/courses?search=E2E + GET /api/contentstore/v2/home/courses?order=-org + GET /api/contentstore/v2/home/courses?active_only=true + GET /api/contentstore/v2/home/courses?archived_only=true + GET /api/contentstore/v2/home/courses?page=2 + + **Response Values** + + If the request is successful, an HTTP 200 "OK" response is returned. + + The HTTP 200 response contains a single dict that contains keys that + are the course's home. + + **Example Response** + + ```json + { + "courses": [ + { + "course_key": "course-v1:edX+E2E-101+course", + "display_name": "E2E Test Course", + "lms_link": "//localhost:18000/courses/course-v1:edX+E2E-101+course", + "cms_link": "//localhost:18010/course/course-v1:edX+E2E-101+course", + "number": "E2E-101", + "org": "edX", + "rerun_link": "/course_rerun/course-v1:edX+E2E-101+course", + "run": "course", + "url": "/course/course-v1:edX+E2E-101+course", + "is_active": true + }, + ], + "in_process_course_actions": [], + } + ``` + + if the `ENABLE_HOME_PAGE_COURSE_API_V2` feature flag is not enabled, an HTTP 404 "Not Found" response + is returned. + """ + if not settings.FEATURES.get('ENABLE_HOME_PAGE_COURSE_API_V2', False): + return HttpResponseNotFound() + + courses, in_process_course_actions = get_course_context_v2(request) + paginator = HomePageCoursesPaginator() + courses_page = paginator.paginate_queryset( + courses, + self.request, + view=self + ) + serializer = CourseHomeTabSerializerV2({ + 'courses': courses_page, + 'in_process_course_actions': in_process_course_actions, + }) + return paginator.get_paginated_response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/__init__.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 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 new file mode 100644 index 000000000000..c0ffa50903cd --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -0,0 +1,241 @@ +""" +Unit tests for home page view. +""" +from collections import OrderedDict +from datetime import datetime, timedelta +from unittest.mock import patch + +import ddt +import pytz +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.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() +FEATURES_WITH_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = True + + +@override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API) +@ddt.ddt +class HomePageCoursesViewV2Test(CourseTestCase): + """ + Tests for HomePageView view version 2. + """ + + def setUp(self): + super().setUp() + self.api_v2_url = reverse("cms.djangoapps.contentstore:v2:courses") + self.api_v1_url = reverse("cms.djangoapps.contentstore:v1:courses") + self.active_course = CourseOverviewFactory.create( + id=self.course.id, + org=self.course.org, + display_name=self.course.display_name, + ) + archived_course_key = self.store.make_course_key('demo-org', 'demo-number', 'demo-run') + self.archived_course = CourseOverviewFactory.create( + display_name="Demo Course (Sample)", + id=archived_course_key, + org=archived_course_key.org, + end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC), + ) + + def test_home_page_response(self): + """Get list of courses available to the logged in user. + + Expected result: + - A paginated response. + - A list of courses available to the logged in user. + """ + response = self.client.get(self.api_v2_url) + course_id = str(self.course.id) + archived_course_id = str(self.archived_course.id) + + expected_data = { + "courses": [ + OrderedDict([ + ("course_key", course_id), + ("display_name", self.course.display_name), + ("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'), + ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'), + ("number", self.course.number), + ("org", self.course.org), + ("rerun_link", f'/course_rerun/{course_id}'), + ("run", self.course.id.run), + ("url", f'/course/{course_id}'), + ("is_active", True), + ]), + OrderedDict([ + ("course_key", str(self.archived_course.id)), + ("display_name", self.archived_course.display_name), + ( + "lms_link", + f'//{settings.LMS_BASE}/courses/{archived_course_id}/jump_to/{self.archived_course.location}' + ), + ( + "cms_link", + f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}', + ), + ("number", self.archived_course.number), + ("org", self.archived_course.org), + ("rerun_link", f'/course_rerun/{str(self.archived_course.id)}'), + ("run", self.archived_course.id.run), + ("url", f'/course/{str(self.archived_course.id)}'), + ("is_active", False), + ]), + ], + "in_process_course_actions": [], + } + expected_response = OrderedDict([ + ('count', 2), + ('num_pages', 1), + ('next', None), + ('previous', None), + ('results', expected_data), + ]) + + 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. + + Expected result: + - A list of active courses available to the logged in user. + """ + response = self.client.get(self.api_v2_url, {"active_only": "true"}) + + self.assertEqual(len(response.data["results"]["courses"]), 1) + self.assertEqual(response.data["results"]["courses"], [OrderedDict([ + ("course_key", str(self.course.id)), + ("display_name", self.course.display_name), + ("lms_link", f'//{settings.LMS_BASE}/courses/{str(self.course.id)}/jump_to/{self.course.location}'), + ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'), + ("number", self.course.number), + ("org", self.course.org), + ("rerun_link", f'/course_rerun/{str(self.course.id)}'), + ("run", self.course.id.run), + ("url", f'/course/{str(self.course.id)}'), + ("is_active", True), + ])]) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_archived_only_query_if_passed(self): + """Get list of archived courses only. + + Expected result: + - A list of archived courses available to the logged in user. + """ + response = self.client.get(self.api_v2_url, {"archived_only": "true"}) + + self.assertEqual(len(response.data["results"]["courses"]), 1) + self.assertEqual(response.data["results"]["courses"], [OrderedDict([ + ("course_key", str(self.archived_course.id)), + ("display_name", self.archived_course.display_name), + ( + "lms_link", + f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}', + ), + ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'), + ("number", self.archived_course.number), + ("org", self.archived_course.org), + ("rerun_link", f'/course_rerun/{str(self.archived_course.id)}'), + ("run", self.archived_course.id.run), + ("url", f'/course/{str(self.archived_course.id)}'), + ("is_active", False), + ])]) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_search_query_if_passed(self): + """Get list of courses when search filter passed as a query param. + + Expected result: + - A list of courses (active or inactive) available to the logged in user for the specified search. + """ + response = self.client.get(self.api_v2_url, {"search": "sample"}) + + self.assertEqual(len(response.data["results"]["courses"]), 1) + self.assertEqual(response.data["results"]["courses"], [OrderedDict([ + ("course_key", str(self.archived_course.id)), + ("display_name", self.archived_course.display_name), + ( + "lms_link", + f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}', + ), + ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'), + ("number", self.archived_course.number), + ("org", self.archived_course.org), + ("rerun_link", f'/course_rerun/{str(self.archived_course.id)}'), + ("run", self.archived_course.id.run), + ("url", f'/course/{str(self.archived_course.id)}'), + ("is_active", False), + ])]) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_order_query_if_passed(self): + """Get list of courses when order filter passed as a query param. + + Expected result: + - A list of courses (active or inactive) available to the logged in user for the specified order. + """ + response = self.client.get(self.api_v2_url, {"order": "org"}) + + self.assertEqual(len(response.data["results"]["courses"]), 2) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["results"]["courses"][0]["org"], "demo-org") + + def test_page_query_if_passed(self): + """Get list of courses when page filter passed as a query param. + + Expected result: + - A list of courses (active or inactive) available to the logged in user for the specified page. + """ + response = self.client.get(self.api_v2_url, {"page": 1}) + + self.assertEqual(response.data["count"], 2) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + @patch("cms.djangoapps.contentstore.views.course.CourseOverview") + @patch("cms.djangoapps.contentstore.views.course.modulestore") + def test_api_v2_is_disabled(self, mock_modulestore, mock_course_overview): + """Get list of courses when home page course v2 API is disabled. + + Expected result: + - Courses are read from the modulestore. + """ + with override_settings(FEATURES={'ENABLE_HOME_PAGE_COURSE_API_V2': False}): + response = self.client.get(self.api_v1_url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_modulestore().get_course_summaries.assert_called_once() + mock_course_overview.get_all_courses.assert_not_called() diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index 66c16dc6dd8c..44084e3595b8 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -10,7 +10,7 @@ import ddt from ccx_keys.locator import CCXLocator from django.conf import settings -from django.test import RequestFactory +from django.test import RequestFactory, override_settings from opaque_keys.edx.locations import CourseLocator from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient @@ -42,6 +42,10 @@ TOTAL_COURSES_COUNT = 10 USER_COURSES_COUNT = 1 +FEATURES_WITH_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy() +FEATURES_WITH_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = True +FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy() +FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = False @ddt.ddt @@ -182,12 +186,18 @@ def test_staff_course_listing(self): self.assertEqual(len(list(courses_list_by_staff)), TOTAL_COURSES_COUNT) - # Verify fetched accessible courses list is a list of CourseSummery instances - self.assertTrue(all(isinstance(course, CourseSummary) for course in courses_list_by_staff)) + with override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API): + # Verify fetched accessible courses list is a list of CourseOverview instances when home page course v2 + # api is enabled. + self.assertTrue(all(isinstance(course, CourseOverview) for course in courses_list_by_staff)) - # Now count the db queries for staff - with check_mongo_calls(2): - list(_accessible_courses_summary_iter(self.request)) + with override_settings(FEATURES=FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API): + # Verify fetched accessible courses list is a list of CourseSummery instances + self.assertTrue(all(isinstance(course, CourseSummary) for course in courses_list_by_staff)) + + # Now count the db queries for staff + with check_mongo_calls(2): + list(_accessible_courses_summary_iter(self.request)) def test_get_course_list_with_invalid_course_location(self): """ @@ -202,13 +212,21 @@ def test_get_course_list_with_invalid_course_location(self): courses_list = list(courses_iter) self.assertEqual(len(courses_list), 1) - courses_summary_iter, __ = _accessible_courses_summary_iter(self.request) - courses_summary_list = list(courses_summary_iter) - - # Verify fetched accessible courses list is a list of CourseSummery instances and only one course - # is returned - self.assertTrue(all(isinstance(course, CourseSummary) for course in courses_summary_list)) - self.assertEqual(len(courses_summary_list), 1) + with override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API): + # Verify fetched accessible courses list is a list of CourseOverview instances when home page course v2 + # api is enabled. + courses_summary_iter, __ = _accessible_courses_summary_iter(self.request) + courses_summary_list = list(courses_summary_iter) + self.assertTrue(all(isinstance(course, CourseOverview) for course in courses_summary_list)) + self.assertEqual(len(courses_summary_list), 1) + + with override_settings(FEATURES=FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API): + # Verify fetched accessible courses list is a list of CourseSummery instances and only one course + # is returned + courses_summary_iter, __ = _accessible_courses_summary_iter(self.request) + courses_summary_list = list(courses_summary_iter) + self.assertTrue(all(isinstance(course, CourseSummary) for course in courses_summary_list)) + self.assertEqual(len(courses_summary_list), 1) # get courses by reversing group name formats courses_list_by_groups, __ = _accessible_courses_list_from_groups(self.request) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index cc2c7b4f330e..9d90b1c7eda3 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1580,6 +1580,49 @@ def format_in_process_course_view(uca): return active_courses, archived_courses, in_process_course_actions +def get_course_context_v2(request): + """Get context of the homepage course tab from the Studio Home.""" + + # Importing here to avoid circular imports: + # ImportError: cannot import name 'reverse_course_url' from partially initialized module + # '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): + """ + Return a dict of the data which the view requires for each unsucceeded course. + + Args: + uca: CourseRerunUIStateManager object. + """ + return { + 'display_name': uca.display_name, + 'course_key': str(uca.course_key), + 'org': uca.course_key.org, + 'number': uca.course_key.course, + 'run': uca.course_key.run, + 'is_failed': uca.state == CourseRerunUIStateManager.State.FAILED, + 'is_in_progress': uca.state == CourseRerunUIStateManager.State.IN_PROGRESS, + 'dismiss_link': reverse_course_url( + 'course_notifications_handler', + uca.course_key, + kwargs={ + 'action_state_id': uca.id, + }, + ) 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) + in_process_course_actions = [format_in_process_course_view(uca) for uca in in_process_course_actions] + return courses_iter, in_process_course_actions + + def get_home_context(request, no_course=False): """ Utils is used to get context of course home. diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 053514123825..efb5b58bde3f 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -15,7 +15,7 @@ from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.decorators import login_required -from django.core.exceptions import PermissionDenied, ValidationError as DjangoValidationError +from django.core.exceptions import FieldError, PermissionDenied, ValidationError as DjangoValidationError from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound from django.shortcuts import redirect from django.urls import reverse @@ -37,6 +37,7 @@ from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.djangoapps.models.settings.course_metadata import CourseMetadata from cms.djangoapps.models.settings.encoder import CourseSettingsEncoder +from cms.djangoapps.contentstore.api.views.utils import get_bool_param from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError from common.djangoapps.course_action_state.models import CourseRerunState, CourseRerunUIStateManager from common.djangoapps.edxmako.shortcuts import render_to_response @@ -410,15 +411,80 @@ def course_filter(course_summary): return False return has_studio_read_access(request.user, course_summary.id) + + 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 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: courses_summary = modulestore().get_course_summaries() + + if enable_home_page_api_v2: + search_query, order, active_only, archived_only = get_query_params_if_present(request) + courses_summary = get_filtered_and_ordered_courses( + courses_summary, + active_only, + archived_only, + search_query, + order, + ) + courses_summary = filter(course_filter, courses_summary) in_process_course_actions = get_in_process_course_actions(request) + return courses_summary, in_process_course_actions +def get_query_params_if_present(request): + """ + Returns the query params from request if present. + + Arguments: + request: the request object + + Returns: + search_query (str): any string used to filter Course Overviews based on visible fields. + order (str): any string used to order Course Overviews. + active_only (str): if not None, this value will limit the courses returned to active courses. + The default value is None. + archived_only (str): if not None, this value will limit the courses returned to archived courses. + The default value is None. + """ + allowed_query_params = ['search', 'order', 'active_only', 'archived_only'] + if not any(param in request.GET for param in allowed_query_params): + return None, None, None, None + search_query = request.GET.get('search') + order = request.GET.get('order') + active_only = get_bool_param(request, 'active_only', None) + archived_only = get_bool_param(request, 'archived_only', None) + return search_query, order, active_only, archived_only + + +def get_filtered_and_ordered_courses(course_overviews, active_only, archived_only, search_query, order): + """ + Returns the filtered and ordered courses based on the query params. + + Arguments: + courses_summary (Course Overview objects): course overview queryset to be filtered. + active_only (str): if not None, this value will limit the courses returned to active courses. + The default value is None. + archived_only (str): if not None, this value will limit the courses returned to archived courses. + The default value is None. + search_query (str): any string used to filter Course Overviews based on visible fields. + order (str): any string used to order Course Overviews. + + Returns: + Course Overview objects: queryset filtered and ordered based on the query params. + """ + course_overviews = get_courses_by_status(active_only, archived_only, course_overviews) + course_overviews = get_courses_by_search_query(search_query, course_overviews) + course_overviews = get_courses_order_by(order, course_overviews) + return course_overviews + + def _accessible_courses_iter(request): """ List all courses available to the logged in user by iterating through all the courses. @@ -509,9 +575,60 @@ def filter_ccx(course_access): if course_keys: courses_list = CourseOverview.get_all_courses(filter_={'id__in': course_keys}) + search_query, order, active_only, archived_only = get_query_params_if_present(request) + courses_list = get_filtered_and_ordered_courses( + courses_list, + active_only, + archived_only, + search_query, + order, + ) + return courses_list, [] +def get_courses_by_status(active_only, archived_only, course_overviews): + """ + Return course overviews based on a base queryset filtered by a status. + + Args: + active_only (str): if not None, this value will limit the courses returned to active courses. + The default value is None. + archived_only (str): if not None, this value will limit the courses returned to archived courses. + The default value is None. + course_overviews (Course Overview objects): course overview queryset to be filtered. + """ + return CourseOverview.get_courses_by_status(active_only, archived_only, course_overviews) + + +def get_courses_by_search_query(search_query, course_overviews): + """Return course overviews based on a base queryset filtered by a search query. + + Args: + search_query (str): any string used to filter Course Overviews based on visible fields. + course_overviews (Course Overview objects): course overview queryset to be filtered. + """ + if not search_query: + return course_overviews + return CourseOverview.get_courses_matching_query(search_query, course_overviews=course_overviews) + + +def get_courses_order_by(order_query, course_overviews): + """Return course overviews based on a base queryset ordered by a query. + + Args: + order_query (str): any string used to order Course Overviews. + course_overviews (Course Overview objects): queryset to be ordered. + """ + if not order_query: + return course_overviews + try: + return course_overviews.order_by(order_query) + except FieldError as e: + log.exception(f"Error ordering courses by {order_query}: {e}") + return course_overviews + + @function_trace('_accessible_libraries_iter') def _accessible_libraries_iter(user, org=None): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index b30f8c95a631..952483893a74 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -40,6 +40,11 @@ from ..course import _deprecated_blocks_info, course_outline_initial_state, reindex_course_and_check_access from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import VisibilityState, create_xblock_info +FEATURES_WITH_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy() +FEATURES_WITH_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = True +FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API = settings.FEATURES.copy() +FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API['ENABLE_HOME_PAGE_COURSE_API_V2'] = False + class TestCourseIndex(CourseTestCase): """ @@ -59,6 +64,11 @@ def setUp(self): number='test-2.3_course', display_name='dotted.course.name-2', ) + CourseOverviewFactory.create( + id=self.odd_course.id, + org=self.odd_course.org, + display_name=self.odd_course.display_name, + ) def check_courses_on_index(self, authed_client, expected_course_tab_len): """ @@ -420,6 +430,7 @@ def check_index_page(self, separate_archived_courses, org): archived_course_tab = parsed_html.find_class('archived-courses') self.assertEqual(len(archived_course_tab), 1 if separate_archived_courses else 0) + @override_settings(FEATURES=FEATURES_WITHOUT_HOME_PAGE_COURSE_V2_API) @ddt.data( # Staff user has course staff access (True, 'staff', None, 0, 21), @@ -450,6 +461,44 @@ def test_separate_archived_courses(self, separate_archived_courses, username, or mongo_queries=mongo_queries, sql_queries=sql_queries) + @override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API) + @ddt.data( + # Staff user has course staff access + (True, 'staff', None, 0, 21), + (False, 'staff', None, 0, 21), + # Base user has global staff access + (True, 'user', ORG, 0, 21), + (False, 'user', ORG, 0, 21), + (True, 'user', None, 0, 21), + (False, 'user', None, 0, 21), + ) + @ddt.unpack + def test_separate_archived_courses_with_home_page_course_v2_api( + self, + separate_archived_courses, + username, + org, + mongo_queries, + sql_queries + ): + """ + Ensure that archived courses are shown as expected for all user types, when the feature is enabled/disabled. + Also ensure that enabling the feature does not adversely affect the database query count. + """ + # Authenticate the requested user + user = getattr(self, username) + password = getattr(self, username + '_password') + self.client.login(username=user, password=password) + + # Enable/disable the feature before viewing the index page. + features = settings.FEATURES.copy() + features['ENABLE_SEPARATE_ARCHIVED_COURSES'] = separate_archived_courses + with override_settings(FEATURES=features): + self.check_index_page_with_query_count(separate_archived_courses=separate_archived_courses, + org=org, + mongo_queries=mongo_queries, + sql_queries=sql_queries) + @ddt.ddt class TestCourseOutline(CourseTestCase): diff --git a/cms/envs/common.py b/cms/envs/common.py index 15e1c39d1951..cf02df9093d1 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -561,6 +561,16 @@ # .. toggle_creation_date: 2024-02-29 # .. toggle_tickets: https://github.com/openedx/edx-platform/pull/33952 'ENABLE_HIDE_FROM_TOC_UI': False, + + # .. toggle_name: FEATURES['ENABLE_HOME_PAGE_COURSE_API_V2'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: True + # .. toggle_description: Enables the new home page course v2 API, which is a new version of the home page course + # API with pagination, filter and ordering capabilities. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2024-03-14 + # .. toggle_tickets: https://github.com/openedx/edx-platform/pull/34173 + 'ENABLE_HOME_PAGE_COURSE_API_V2': True, } # .. toggle_name: ENABLE_COPPA_COMPLIANCE diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 7684615f0fcb..10a56f0868fb 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -700,6 +700,41 @@ def get_all_courses(cls, orgs=None, filter_=None, active_only=False, course_keys return course_overviews + @classmethod + def get_courses_matching_query(cls, query, course_overviews): + """ + Return a queryset of CourseOverview objects filtered bythe given query. + + Args: + query: required parameter that allows filtering based on the CourseOverview. + course_overviews: queryset of CourseOverview objects to filter on. + """ + return course_overviews.filter( + Q(display_name__icontains=query) | + Q(org__icontains=query) | + Q(id__icontains=query) + ) + + @classmethod + def get_courses_by_status(cls, active_only, archived_only, course_overviews): + """ + Return a queryset of CourseOverview objects based on the given status. + + Args: + active_only: when True, only active courses will be returned. + archived_only: when True, only archived courses will be returned. + course_overviews: queryset of CourseOverview objects to filter on. + """ + if active_only: + return course_overviews.filter( + Q(end__isnull=True) | Q(end__gte=datetime.now().replace(tzinfo=pytz.UTC)) + ) + if archived_only: + return course_overviews.filter( + end__lt=datetime.now().replace(tzinfo=pytz.UTC) + ) + return course_overviews + @classmethod def get_all_course_keys(cls): """