From 1567a3529e4c5acbd1260e9c83d623588c2af791 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 28 Feb 2024 10:01:56 -0400 Subject: [PATCH 1/8] temp --- .../rest_api/v2/views/tests/__init__.py | 0 .../rest_api/v2/views/tests/test_home.py | 69 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 cms/djangoapps/contentstore/rest_api/v2/views/tests/__init__.py create mode 100644 cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py 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..3e58b6d92491 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -0,0 +1,69 @@ +""" +Unit tests for home page view. +""" +import ddt +from django.conf import 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.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 + + +@ddt.ddt +class HomePageCoursesViewTest(CourseTestCase): + """ + Tests for HomePageView. + """ + + def setUp(self): + super().setUp() + self.url = reverse("cms.djangoapps.contentstore:v2:courses") + + def test_home_page_response(self): + """Check successful response content""" + response = self.client.get(self.url) + course_id = str(self.course.id) + + expected_response = { + "courses": [{ + "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": [], + } + + 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""" + 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) From 546a74e770aa30be3d441714de7cae6224632a43 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Fri, 2 Feb 2024 13:42:46 -0400 Subject: [PATCH 2/8] feat: add pagination to the HomePageCoursesV2 API --- .../contentstore/rest_api/v2/views/home.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 8f2759c537b8..3e0a5a42ff51 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -3,6 +3,8 @@ from rest_framework.request import Request from rest_framework.response import Response 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 @@ -88,11 +90,11 @@ def get(self, request: Request): } ``` """ - courses, in_process_course_actions = get_course_context_v2(request) - courses_context = { - "courses": courses, - "in_process_course_actions": in_process_course_actions, - } - serializer = CourseHomeTabSerializerV2(courses_context) - return Response(serializer.data) + paginator = PageNumberPagination() + 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) From d87c42ac3e55ba7d8bac6b6a8e7bc0efb1ab8f99 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 6 Feb 2024 09:13:24 -0400 Subject: [PATCH 3/8] refactor: address PR reviews --- cms/djangoapps/contentstore/rest_api/v2/views/home.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 3e0a5a42ff51..001622250bfc 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -42,6 +42,11 @@ class HomePageCoursesViewV2(APIView): apidocs.ParameterLocation.QUERY, description="Query param to filter by archived courses only", ), + apidocs.integer_parameter( + "page", + apidocs.ParameterLocation.QUERY, + description="Query param to paginate the courses", + ), ], responses={ 200: CourseHomeTabSerializerV2, @@ -60,6 +65,7 @@ def get(self, request: Request): 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** @@ -90,6 +96,7 @@ def get(self, request: Request): } ``` """ + courses, in_process_course_actions = get_course_context_v2(request) paginator = PageNumberPagination() courses_page = paginator.paginate_queryset(courses, self.request, view=self) From ee1c0d266b8e761a8ed7655ae9047510a29363e1 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 6 Feb 2024 09:25:11 -0400 Subject: [PATCH 4/8] fix: use string_parameter instead of integer_parameter --- cms/djangoapps/contentstore/rest_api/v2/views/home.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 001622250bfc..0d9ef5d1f6a6 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -42,7 +42,7 @@ class HomePageCoursesViewV2(APIView): apidocs.ParameterLocation.QUERY, description="Query param to filter by archived courses only", ), - apidocs.integer_parameter( + apidocs.string_parameter( "page", apidocs.ParameterLocation.QUERY, description="Query param to paginate the courses", From 9cb10e7b0e31513e8fd9f80521bf5d99b447f6bc Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 6 Feb 2024 10:15:30 -0400 Subject: [PATCH 5/8] fix: address quality issues --- cms/djangoapps/contentstore/rest_api/v2/views/home.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 0d9ef5d1f6a6..5c771038cd37 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -1,7 +1,6 @@ """HomePageCoursesViewV2 APIView for getting content available to the logged in user.""" import edx_api_doc_tools as apidocs from rest_framework.request import Request -from rest_framework.response import Response from rest_framework.views import APIView from rest_framework.pagination import PageNumberPagination From d127b36d2c692e1e7dd11553b9bbe3a9550b3703 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 6 Feb 2024 14:21:13 -0400 Subject: [PATCH 6/8] refactor: return number of pages --- cms/djangoapps/contentstore/rest_api/v2/views/home.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 5c771038cd37..d8584ad3af20 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -1,5 +1,7 @@ """HomePageCoursesViewV2 APIView for getting content available to the logged in user.""" import edx_api_doc_tools as apidocs +from collections import OrderedDict +from rest_framework.response import Response from rest_framework.request import Request from rest_framework.views import APIView from rest_framework.pagination import PageNumberPagination @@ -103,4 +105,10 @@ def get(self, request: Request): 'courses': courses_page, 'in_process_course_actions': in_process_course_actions, }) - return paginator.get_paginated_response(serializer.data) + return Response(OrderedDict([ + ('count', paginator.page.paginator.count), + ('num_pages', paginator.page.paginator.num_pages), + ('next', paginator.get_next_link()), + ('previous', paginator.get_previous_link()), + ('results', serializer.data), + ])) From e817f562d018dd8cd63c15f0c21d5627a69c16e6 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Fri, 16 Feb 2024 10:13:16 -0400 Subject: [PATCH 7/8] refactor: use custom paginator for managing course filter --- .../contentstore/rest_api/v2/views/home.py | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index d8584ad3af20..2432c300efdc 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -12,6 +12,32 @@ from cms.djangoapps.contentstore.rest_api.v2.serializers import CourseHomeTabSerializerV2 +class HomePageCoursesPaginator(PageNumberPagination): + + 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), + ('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.""" @@ -97,18 +123,15 @@ def get(self, request: Request): } ``` """ - courses, in_process_course_actions = get_course_context_v2(request) - paginator = PageNumberPagination() - courses_page = paginator.paginate_queryset(courses, self.request, view=self) + 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 Response(OrderedDict([ - ('count', paginator.page.paginator.count), - ('num_pages', paginator.page.paginator.num_pages), - ('next', paginator.get_next_link()), - ('previous', paginator.get_previous_link()), - ('results', serializer.data), - ])) + return paginator.get_paginated_response(serializer.data) From e59900e014b870896dfe7727393518b82f2e10f7 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Fri, 16 Feb 2024 17:23:07 -0400 Subject: [PATCH 8/8] fix: add missing num_pages for paginated response --- cms/djangoapps/contentstore/rest_api/v2/views/home.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 2432c300efdc..4c7ee4151535 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -18,6 +18,7 @@ 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),