diff --git a/cms/djangoapps/contentstore/rest_api/urls.py b/cms/djangoapps/contentstore/rest_api/urls.py index af2694bfbbc5..e6337cb11b7c 100644 --- a/cms/djangoapps/contentstore/rest_api/urls.py +++ b/cms/djangoapps/contentstore/rest_api/urls.py @@ -7,11 +7,13 @@ from .v0 import urls as v0_urls from .v1 import urls as v1_urls from .v2 import urls as v2_urls +from .v4 import urls as v4_urls app_name = 'cms.djangoapps.contentstore' urlpatterns = [ path('v0/', include(v0_urls)), path('v1/', include(v1_urls)), - path('v2/', include(v2_urls)) + path('v2/', include(v2_urls)), + path('v4/', include(v4_urls)), ] diff --git a/cms/djangoapps/contentstore/rest_api/v4/__init__.py b/cms/djangoapps/contentstore/rest_api/v4/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cms/djangoapps/contentstore/rest_api/v4/serializers/__init__.py b/cms/djangoapps/contentstore/rest_api/v4/serializers/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cms/djangoapps/contentstore/rest_api/v4/serializers/home.py b/cms/djangoapps/contentstore/rest_api/v4/serializers/home.py new file mode 100644 index 000000000000..f3b6bf0f4ee3 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v4/serializers/home.py @@ -0,0 +1,16 @@ +"""API serializers for course home V4. Re-exports V2 serializers under V4 names.""" +from cms.djangoapps.contentstore.rest_api.v2.serializers.home import ( + CourseCommonSerializerV2, + CourseHomeTabSerializerV2, + UnsucceededCourseSerializerV2, +) + +CourseCommonSerializerV4 = CourseCommonSerializerV2 +CourseHomeTabSerializerV4 = CourseHomeTabSerializerV2 +UnsucceededCourseSerializerV4 = UnsucceededCourseSerializerV2 + +__all__ = [ + "CourseCommonSerializerV4", + "CourseHomeTabSerializerV4", + "UnsucceededCourseSerializerV4", +] diff --git a/cms/djangoapps/contentstore/rest_api/v4/tests/__init__.py b/cms/djangoapps/contentstore/rest_api/v4/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cms/djangoapps/contentstore/rest_api/v4/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v4/tests/test_home.py new file mode 100644 index 000000000000..aa58305a605f --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v4/tests/test_home.py @@ -0,0 +1,55 @@ +""" +ADR 0029 - Standardized error-response tests for HomeCoursesViewSet (v4). + +Verifies that the central exception handler produces the correct ADR 0029 +envelope for auth errors on the v4 home courses endpoint. +""" + +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient, APITestCase + +_REQUIRED_ERROR_FIELDS = ("type", "title", "status", "detail", "instance") + + +class TestHomeCoursesViewSetErrorShape(APITestCase): + """ + ADR 0029 - error response shape regression tests for HomeCoursesViewSet (v4). + """ + + def setUp(self): + super().setUp() + self.client = APIClient() + self.list_url = reverse("cms.djangoapps.contentstore:v4:home-courses-list") + + def test_unauthenticated_returns_standardized_401(self): + """Unauthenticated GET must return 401 with the ADR 0029 envelope.""" + response = self.client.get(self.list_url) + + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) # noqa: PT009 + for field in _REQUIRED_ERROR_FIELDS: + self.assertIn( # noqa: PT009 + field, response.data, f"ADR 0029: missing field '{field}'" + ) + + def test_unauthenticated_401_type_uri(self): + """The ``type`` field for 401 must be the ADR 0029 authn URI.""" + response = self.client.get(self.list_url) + + self.assertEqual( # noqa: PT009 + response.data.get("type"), + "https://docs.openedx.org/errors/authn", + ) + + def test_error_body_has_no_legacy_fields(self): + """Error responses must NOT contain old DeveloperErrorViewMixin fields.""" + response = self.client.get(self.list_url) + + self.assertNotIn("developer_message", response.data) # noqa: PT009 + self.assertNotIn("error_code", response.data) # noqa: PT009 + + def test_instance_field_is_request_path(self): + """The ``instance`` field must equal the request path.""" + response = self.client.get(self.list_url) + + self.assertEqual(response.data.get("instance"), self.list_url) # noqa: PT009 diff --git a/cms/djangoapps/contentstore/rest_api/v4/urls.py b/cms/djangoapps/contentstore/rest_api/v4/urls.py new file mode 100644 index 000000000000..c75a113ef53f --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v4/urls.py @@ -0,0 +1,14 @@ +"""Contentstore API v4 URLs.""" + +from rest_framework.routers import DefaultRouter + +from cms.djangoapps.contentstore.rest_api.v4.views import home + +app_name = "v4" + +# ADR 0028: HomeCoursesViewSet registered via DefaultRouter. +# Generates: GET home/courses/ → name: home-courses-list +router = DefaultRouter() +router.register(r'home/courses', home.HomeCoursesViewSet, basename='home-courses') + +urlpatterns = router.urls diff --git a/cms/djangoapps/contentstore/rest_api/v4/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v4/views/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cms/djangoapps/contentstore/rest_api/v4/views/home.py b/cms/djangoapps/contentstore/rest_api/v4/views/home.py new file mode 100644 index 000000000000..d6c927b328c7 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v4/views/home.py @@ -0,0 +1,220 @@ +"""HomeCoursesViewSet for getting courses available to the logged-in user (v4).""" + +from drf_spectacular.utils import OpenApiParameter, OpenApiResponse, extend_schema +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import ( + SessionAuthenticationAllowInactiveUser, +) +from edx_rest_framework_extensions.paginators import DefaultPagination +from rest_framework import viewsets +from rest_framework.permissions import IsAuthenticated +from rest_framework.request import Request +from rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.v4.serializers.home import ( + CourseHomeTabSerializerV4, +) +from cms.djangoapps.contentstore.utils import get_course_context_v2 + + +class HomePageCoursesPaginator(DefaultPagination): + """ + ADR 0032 - standard pagination for the Studio home courses list (v4). + + Extends ``DefaultPagination`` (edx-rest-framework-extensions) which + provides the 7-field response envelope: + ``count``, ``num_pages``, ``current_page``, ``start``, + ``next``, ``previous``, ``results``. + + Overrides ``paginate_queryset`` to handle ``filter`` objects returned + by ``get_course_context_v2``. + """ + + page_size_query_param = "page_size" + + def paginate_queryset(self, queryset, request, view=None): + """ + Paginate a queryset, converting ``filter`` objects to lists first. + + ``get_course_context_v2`` may return a ``filter`` object; the base + ``PageNumberPagination`` cannot measure its length without materialising + it first, so we do that here. + """ + if isinstance(queryset, filter): + queryset = list(queryset) + return super().paginate_queryset(queryset, request, view) + + +def _query_param( + name: str, description: str, deprecated: bool = False +) -> OpenApiParameter: + """Build a string-typed, optional query parameter for OpenAPI docs.""" + return OpenApiParameter( + name=name, + description=description, + required=False, + type=str, + location=OpenApiParameter.QUERY, + deprecated=deprecated, + ) + + +_HOME_COURSES_QUERY_PARAMETERS = [ + _query_param("org", "Filter by course org"), + _query_param("search", "Filter by course name, org, or number"), + _query_param( + "ordering", + "Order by course field: display_name, org, number, or run (ADR 0033 standard parameter).", + ), + _query_param( + "order", + "Deprecated alias for 'ordering' (ADR 0033). Use 'ordering' instead.", + deprecated=True, + ), + _query_param("active_only", "Filter to active courses only"), + _query_param("archived_only", "Filter to archived courses only"), + _query_param("page", "Page number for pagination"), + _query_param("page_size", "Number of courses per page (default 10, max 100)"), +] + +_UNAUTHENTICATED_RESPONSE = OpenApiResponse( + description="The requester is not authenticated." +) + +# ADR 0033: emitted as an HTTP ``Deprecation`` header when the legacy ``order`` +# parameter is used instead of the DRF-standard ``ordering``. +_LEGACY_ORDER_DEPRECATION_HEADER = ( + "Parameter 'order' is deprecated. Use 'ordering' instead. " + "Support will be removed in release ''." +) + + +def _maybe_set_legacy_order_deprecation_header( + request: Request, response: Response +) -> Response: + """Set the ADR 0033 Deprecation header when the legacy ``order`` parameter is used.""" + if "order" in request.query_params: + response["Deprecation"] = _LEGACY_ORDER_DEPRECATION_HEADER + return response + + +class HomeCoursesViewSet(viewsets.ViewSet): + """ + ViewSet for course listing (v4). Registered via DefaultRouter (basename ``home-courses``). + + Router-generated URLs:: + + GET /api/contentstore/v4/home/courses/ → list + + Supersedes ``HomePageCoursesViewV2`` at ``/api/contentstore/v2/home/courses``. + + ADR compliance: + - 0025: ``serializer_class`` attribute for schema generation + - 0026: explicit ``authentication_classes`` and ``permission_classes`` + - 0027: ``drf_spectacular`` for OpenAPI documentation + - 0028: ViewSet with DefaultRouter registration + - 0032: 7-field pagination envelope via ``DefaultPagination`` + - 0033: ``ordering`` parameter; ``order`` kept as deprecated alias + """ + + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated,) + serializer_class = CourseHomeTabSerializerV4 + + def get_exception_handler(self): + """Return the ADR 0029 standardized error handler for this viewset.""" + from openedx.core.lib.api.exceptions import standardized_error_exception_handler + return standardized_error_exception_handler + + def get_serializer(self, *args, **kwargs): + """Instantiate and return the configured serializer class.""" + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="List courses for the Studio home page (paginated)", + description=( + "Returns a paginated list of all courses available to the logged-in user, " + "with optional filtering and ordering. " + "Supersedes ``GET /api/contentstore/v2/home/courses``." + ), + parameters=_HOME_COURSES_QUERY_PARAMETERS, + responses={ + 200: OpenApiResponse( + response=CourseHomeTabSerializerV4, + description="Paginated course list retrieved successfully.", + ), + 401: _UNAUTHENTICATED_RESPONSE, + }, + ) + def list(self, request: Request): + """ + Get a paginated list of all courses available to the logged-in user. + + **Example Request** + + GET /api/contentstore/v4/home/courses/ + GET /api/contentstore/v4/home/courses/?org=edX + GET /api/contentstore/v4/home/courses/?search=E2E + GET /api/contentstore/v4/home/courses/?ordering=-org + GET /api/contentstore/v4/home/courses/?order=-org + GET /api/contentstore/v4/home/courses/?active_only=true + GET /api/contentstore/v4/home/courses/?archived_only=true + GET /api/contentstore/v4/home/courses/?page=2 + GET /api/contentstore/v4/home/courses/?page_size=20 + + **Pagination Parameters** + + - ``page`` (int): Page number to retrieve. Default is 1. + - ``page_size`` (int): Items per page. Default is 10, max is 100. + + **Response Envelope (ADR 0032)** + + - ``count`` (int): Total number of courses matching the filters. + - ``num_pages`` (int): Total number of pages. + - ``current_page`` (int): The current page number. + - ``start`` (int): The 0-based index of the first course on this page. + - ``next`` (str|null): URL for the next page, or null on the last page. + - ``previous`` (str|null): URL for the previous page, or null on the first page. + - ``results`` (dict): Course data for the current page. + + **Example Response** + + ```json + { + "count": 1, + "num_pages": 1, + "current_page": 1, + "start": 0, + "next": null, + "previous": null, + "results": { + "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": [] + } + } + ``` + """ + courses, in_process_course_actions = get_course_context_v2(request) + paginator = HomePageCoursesPaginator() + courses_page = paginator.paginate_queryset(courses, request, view=self) + serializer = self.get_serializer( + { + "courses": courses_page, + "in_process_course_actions": in_process_course_actions, + } + ) + response = paginator.get_paginated_response(serializer.data) + return _maybe_set_legacy_order_deprecation_header(request, response) diff --git a/cms/djangoapps/contentstore/rest_api/v4/views/tests/__init__.py b/cms/djangoapps/contentstore/rest_api/v4/views/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cms/djangoapps/contentstore/rest_api/v4/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v4/views/tests/test_home.py new file mode 100644 index 000000000000..45b6ab472709 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v4/views/tests/test_home.py @@ -0,0 +1,275 @@ +""" +Unit tests for HomeCoursesViewSet (v4). +""" + +from collections import OrderedDict +from datetime import datetime, timedelta, timezone +from unittest.mock import patch + +import ddt +from django.conf import settings +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient, APITestCase + +from cms.djangoapps.contentstore.rest_api.v4.views.home import ( + _LEGACY_ORDER_DEPRECATION_HEADER, +) +from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from cms.djangoapps.contentstore.utils import reverse_course_url +from openedx.core.djangoapps.content.course_overviews.tests.factories import ( + CourseOverviewFactory, +) + +_MOCK_GET_COURSE_CONTEXT_V2 = ( + "cms.djangoapps.contentstore.rest_api.v4.views.home.get_course_context_v2" +) + + +class TestHomeCoursesViewSetPermissions(APITestCase): + """ADR 0026 - permission regression tests for HomeCoursesViewSet.""" + + def setUp(self): + super().setUp() + self.list_url = reverse("cms.djangoapps.contentstore:v4:home-courses-list") + + def test_unauthenticated_returns_401(self): + """Unauthenticated GET /v4/home/courses/ must return 401.""" + client = APIClient() + response = client.get(self.list_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) # noqa: PT009 + + def test_authenticated_staff_gets_200(self): + """Authenticated staff user must receive 200.""" + from django.contrib.auth import get_user_model + + User = get_user_model() + user = User.objects.create_user( + username="teststaff", password="pass", is_staff=True + ) + self.client.force_authenticate(user=user) + with patch(_MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(self.list_url) + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + + +@ddt.ddt +class TestHomeCoursesViewSet(CourseTestCase): + """Functional tests for HomeCoursesViewSet list action.""" + + def setUp(self): + super().setUp() + self.api_v4_url = reverse("cms.djangoapps.contentstore:v4:home-courses-list") + 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=timezone.utc # noqa: UP017 + ), + ) + self.non_staff_client, _ = self.create_non_staff_authed_user_client() + + def test_home_page_response(self): + """GET /v4/home/courses/ must return the 7-field ADR 0032 pagination envelope.""" + response = self.client.get(self.api_v4_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_ROOT_URL}/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_ROOT_URL}/courses/{archived_course_id}" + f"/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 = { + "count": 2, + "num_pages": 1, + "current_page": 1, + "start": 0, + "next": None, + "previous": None, + "results": expected_data, + } + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertDictEqual(expected_response, response.data) # noqa: PT009 + + def test_active_only_query_if_passed(self): + """?active_only=true must return only active courses.""" + response = self.client.get(self.api_v4_url, {"active_only": "true"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(len(response.data["results"]["courses"]), 1) # noqa: PT009 + self.assertTrue( # noqa: PT009 + response.data["results"]["courses"][0]["is_active"] + ) + + def test_archived_only_query_if_passed(self): + """?archived_only=true must return only archived courses.""" + response = self.client.get(self.api_v4_url, {"archived_only": "true"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(len(response.data["results"]["courses"]), 1) # noqa: PT009 + self.assertFalse( # noqa: PT009 + response.data["results"]["courses"][0]["is_active"] + ) + + def test_search_query_if_passed(self): + """?search=sample must filter courses by name.""" + response = self.client.get(self.api_v4_url, {"search": "sample"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(len(response.data["results"]["courses"]), 1) # noqa: PT009 + + def test_ordering_query_if_passed(self): + """?ordering=org must order courses by org (ADR 0033 standard parameter).""" + response = self.client.get(self.api_v4_url, {"ordering": "org"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(len(response.data["results"]["courses"]), 2) # noqa: PT009 + self.assertEqual( # noqa: PT009 + response.data["results"]["courses"][0]["org"], "demo-org" + ) + + def test_legacy_order_query_still_works(self): + """?order=org must still work (deprecated alias, ADR 0033).""" + response = self.client.get(self.api_v4_url, {"order": "org"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(len(response.data["results"]["courses"]), 2) # noqa: PT009 + + def test_page_query_if_passed(self): + """?page=1 must return paginated result with count.""" + response = self.client.get(self.api_v4_url, {"page": 1}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(response.data["count"], 2) # noqa: PT009 + + @ddt.data( + ("active_only", "true"), + ("archived_only", "true"), + ("search", "sample"), + ("ordering", "org"), + ("page", 1), + ) + @ddt.unpack + def test_if_empty_list_of_courses(self, query_param, value): + """Empty course list returns empty results, not an error.""" + self.active_course.delete() + self.archived_course.delete() + + response = self.client.get(self.api_v4_url, {query_param: value}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(len(response.data["results"]["courses"]), 0) # noqa: PT009 + + @ddt.data( + ("active_only", "true"), + ("archived_only", "true"), + ("search", "sample"), + ("ordering", "org"), + ("page", 1), + ) + @ddt.unpack + def test_if_empty_list_of_courses_non_staff(self, query_param, value): + """Non-staff users with no courses get an empty result.""" + self.active_course.delete() + self.archived_course.delete() + + response = self.non_staff_client.get(self.api_v4_url, {query_param: value}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertEqual(len(response.data["results"]["courses"]), 0) # noqa: PT009 + + +class TestHomeCoursesViewSetOrderingDeprecation(CourseTestCase): + """ADR 0033 – Deprecation header tests for the legacy ``order`` parameter.""" + + def setUp(self): + super().setUp() + self.list_url = reverse("cms.djangoapps.contentstore:v4:home-courses-list") + + def test_ordering_param_no_deprecation_header(self): + """``?ordering=display_name`` must not emit a Deprecation header.""" + with patch(_MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(self.list_url, {"ordering": "display_name"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertNotIn("Deprecation", response) # noqa: PT009 + + def test_legacy_order_param_emits_deprecation_header(self): + """``?order=display_name`` must emit the ADR 0033 Deprecation header.""" + with patch(_MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(self.list_url, {"order": "display_name"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertIn("Deprecation", response) # noqa: PT009 + self.assertEqual( # noqa: PT009 + response["Deprecation"], _LEGACY_ORDER_DEPRECATION_HEADER + ) + + def test_ordering_wins_when_both_present(self): + """When both params sent, ``ordering`` wins and Deprecation header is still emitted.""" + with patch(_MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get( + self.list_url, {"ordering": "org", "order": "display_name"} + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertIn("Deprecation", response) # noqa: PT009 + + def test_no_ordering_param_no_deprecation_header(self): + """Plain GET /v4/home/courses/ must not emit a Deprecation header.""" + with patch(_MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(self.list_url) + + self.assertNotIn("Deprecation", response) # noqa: PT009 diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 569871b95bea..72df46d33c61 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -459,19 +459,23 @@ def get_query_params_if_present(request): Arguments: request: the request object + ADR 0033 - ``ordering`` is the preferred parameter (DRF standard); ``order`` + is kept as a deprecated alias. When both are present, ``ordering`` wins. + Returns: search_query (str): any string used to filter Course Overviews based on visible fields. - order (str): any string used to order Course Overviews. + order (str): any string used to order Course Overviews. Sourced from + ``ordering`` (preferred) or ``order`` (deprecated alias). 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'] + allowed_query_params = ['search', 'ordering', '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') + order = request.GET.get('ordering') or 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 diff --git a/openedx/core/lib/api/exceptions.py b/openedx/core/lib/api/exceptions.py new file mode 100644 index 000000000000..13f6cb744c5c --- /dev/null +++ b/openedx/core/lib/api/exceptions.py @@ -0,0 +1,135 @@ +"""ADR 0029 - Standardized error-response exception handler and helpers.""" + +from rest_framework.exceptions import APIException, ValidationError +from rest_framework.response import Response + + +class Conflict(APIException): + """HTTP 409 Conflict — ADR 0029.""" + + status_code = 409 + default_detail = "A conflict occurred." + default_code = "conflict" + + +def standardized_error_exception_handler(exc, context): + """ + ADR 0029 - platform-level DRF exception handler. + + Wraps the existing ``ignored_error_exception_handler`` and reformats its + response into the standardized JSON error envelope:: + + { + "type": "https://docs.openedx.org/errors/{category}", + "title": "", + "status": , + "detail": "", + "instance": "" + } + + For ``ValidationError``, an additional ``errors`` key is included with + per-field error details. + """ + from openedx.core.lib.request_utils import ( + ignored_error_exception_handler, + ) # avoid circular import + + response = ignored_error_exception_handler(exc, context) + + if response is None: + return Response( + { + "type": "https://docs.openedx.org/errors/internal", + "title": "Internal Server Error", + "status": 500, + "detail": "An unexpected error occurred. Please try again later.", + }, + status=500, + ) + + request = context.get("request") + body = { + "type": f"https://docs.openedx.org/errors/{_error_type(exc)}", + "title": _error_title(exc), + "status": response.status_code, + "detail": _flatten_detail(response.data), + } + if request: + body["instance"] = request.path + if hasattr(exc, "user_message") and exc.user_message: + body["user_message"] = exc.user_message + if isinstance(exc, ValidationError) and hasattr(exc, "detail"): + body["errors"] = _normalize_validation_errors(exc.detail) + + response.data = body + response["Content-Type"] = "application/json" + return response + + +def _error_type(exc): + """Map a DRF exception to an ADR 0029 error category slug.""" + from rest_framework.exceptions import ( # avoid circular import at module level + AuthenticationFailed, + NotAuthenticated, + NotFound, + PermissionDenied, + Throttled, + ) + + if isinstance(exc, (NotAuthenticated, AuthenticationFailed)): + return "authn" + if isinstance(exc, PermissionDenied): + return "authz" + if isinstance(exc, NotFound): + return "not-found" + if isinstance(exc, ValidationError): + return "validation" + if isinstance(exc, Throttled): + return "rate-limited" + if isinstance(exc, Conflict): + return "conflict" + return "internal" + + +def _error_title(exc): + """Return a human-readable title for the given DRF exception.""" + from rest_framework.exceptions import ( # avoid circular import at module level + AuthenticationFailed, + NotAuthenticated, + NotFound, + PermissionDenied, + Throttled, + ) + + return { + NotAuthenticated: "Authentication Required", + AuthenticationFailed: "Authentication Failed", + PermissionDenied: "Permission Denied", + NotFound: "Not Found", + ValidationError: "Validation Error", + Throttled: "Too Many Requests", + Conflict: "Conflict", + }.get(type(exc), "Internal Server Error") + + +def _flatten_detail(data): + """Extract a single string detail message from a DRF response data payload.""" + if isinstance(data, str): + return data + if isinstance(data, dict) and "detail" in data: + return str(data["detail"]) + if isinstance(data, list) and data: + return str(data[0]) + return str(data) + + +def _normalize_validation_errors(detail): + """Convert DRF validation error detail into a consistent per-field dict.""" + if isinstance(detail, dict): + return { + field: [str(e) for e in (errs if isinstance(errs, list) else [errs])] + for field, errs in detail.items() + } + if isinstance(detail, list): + return {"non_field_errors": [str(e) for e in detail]} + return {"non_field_errors": [str(detail)]}