diff --git a/cms/djangoapps/contentstore/rest_api/urls.py b/cms/djangoapps/contentstore/rest_api/urls.py index af2694bfbbc5..9696cdf9b73c 100644 --- a/cms/djangoapps/contentstore/rest_api/urls.py +++ b/cms/djangoapps/contentstore/rest_api/urls.py @@ -7,11 +7,15 @@ from .v0 import urls as v0_urls from .v1 import urls as v1_urls from .v2 import urls as v2_urls +from .v3 import urls as v3_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('v3/', include(v3_urls)), + path('v4/', include(v4_urls)), ] diff --git a/cms/djangoapps/contentstore/rest_api/v0/views/xblock.py b/cms/djangoapps/contentstore/rest_api/v0/views/xblock.py index 79217971bb67..1cab4a390570 100644 --- a/cms/djangoapps/contentstore/rest_api/v0/views/xblock.py +++ b/cms/djangoapps/contentstore/rest_api/v0/views/xblock.py @@ -1,7 +1,14 @@ """ -Public rest API endpoints for the CMS API. +Public rest API endpoints for the CMS API — v0 xblock (DEPRECATED). + +.. deprecated:: + These views are superseded by ``XblockViewSet`` in + ``cms.djangoapps.contentstore.rest_api.v1.views.xblock``. + Use ``/api/contentstore/v1/xblock/`` going forward. + These v0 endpoints will be removed in a future release. """ import logging +import warnings from django.views.decorators.csrf import csrf_exempt from rest_framework.generics import CreateAPIView, RetrieveUpdateDestroyAPIView @@ -17,10 +24,17 @@ log = logging.getLogger(__name__) handle_xblock = view_handlers.handle_xblock +_DEPRECATION_MSG = ( + "The v0 xblock API (/api/contentstore/v0/xblock/) is deprecated. " + "Use /api/contentstore/v1/xblock/ instead." +) + @view_auth_classes() class XblockView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView): """ + **DEPRECATED** — use ``/api/contentstore/v1/xblock/{usage_key_string}/`` instead. + Public rest API endpoints for the CMS API. course_key: required argument, needed to authorize course authors. usage_key_string (optional): @@ -32,29 +46,35 @@ class XblockView(DeveloperErrorViewMixin, RetrieveUpdateDestroyAPIView): @course_author_access_required @expect_json_in_class_view def retrieve(self, request, course_key, usage_key_string=None): + warnings.warn(_DEPRECATION_MSG, DeprecationWarning, stacklevel=2) return handle_xblock(request, usage_key_string) @course_author_access_required @expect_json_in_class_view @validate_request_with_serializer def update(self, request, course_key, usage_key_string=None): + warnings.warn(_DEPRECATION_MSG, DeprecationWarning, stacklevel=2) return handle_xblock(request, usage_key_string) @course_author_access_required @expect_json_in_class_view @validate_request_with_serializer def partial_update(self, request, course_key, usage_key_string=None): + warnings.warn(_DEPRECATION_MSG, DeprecationWarning, stacklevel=2) return handle_xblock(request, usage_key_string) @course_author_access_required @expect_json_in_class_view def destroy(self, request, course_key, usage_key_string=None): + warnings.warn(_DEPRECATION_MSG, DeprecationWarning, stacklevel=2) return handle_xblock(request, usage_key_string) @view_auth_classes() class XblockCreateView(DeveloperErrorViewMixin, CreateAPIView): """ + **DEPRECATED** — use ``POST /api/contentstore/v1/xblock/`` instead. + Public rest API endpoints for the CMS API. course_key: required argument, needed to authorize course authors. usage_key_string (optional): @@ -68,4 +88,5 @@ class XblockCreateView(DeveloperErrorViewMixin, CreateAPIView): @expect_json_in_class_view @validate_request_with_serializer def create(self, request, course_key, usage_key_string=None): + warnings.warn(_DEPRECATION_MSG, DeprecationWarning, stacklevel=2) return handle_xblock(request, usage_key_string) diff --git a/cms/djangoapps/contentstore/rest_api/v1/urls.py b/cms/djangoapps/contentstore/rest_api/v1/urls.py index 685a81d778ce..48cd5118cc40 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/urls.py @@ -2,6 +2,7 @@ from django.conf import settings from django.urls import path, re_path +from rest_framework.routers import DefaultRouter from openedx.core.constants import COURSE_ID_PATTERN @@ -27,6 +28,7 @@ ProctoringErrorsView, VideoDownloadView, VideoUsageView, + XblockViewSet, vertical_container_children_redirect_view, ) @@ -34,7 +36,10 @@ VIDEO_ID_PATTERN = r'(?P[-\w]+)' -urlpatterns = [ +_router = DefaultRouter() +_router.register(r'xblock', XblockViewSet, basename='xblock') + +urlpatterns = _router.urls + [ path( 'home', HomePageView.as_view(), diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py index 7654c9e0befc..f60e186f9f5c 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py @@ -16,3 +16,4 @@ from .textbooks import CourseTextbooksView # noqa: F401 from .vertical_block import ContainerHandlerView, vertical_container_children_redirect_view # noqa: F401 from .videos import CourseVideosView, VideoDownloadView, VideoUsageView # noqa: F401 +from .xblock import XblockViewSet # noqa: F401 diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/permissions.py b/cms/djangoapps/contentstore/rest_api/v1/views/permissions.py new file mode 100644 index 000000000000..3b60607eebec --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/permissions.py @@ -0,0 +1,27 @@ +""" +Permission classes for v1 contentstore API views (ADR 0026). +""" +import logging + +from rest_framework.permissions import BasePermission + +from common.djangoapps.student.auth import has_course_author_access + +log = logging.getLogger(__name__) + + +class HasCourseAuthorAccess(BasePermission): + """ + ADR 0026: replaces the @course_author_access_required decorator. + + Reads ``view.kwargs["course_key"]`` (a CourseKey instance) that is + injected by XblockViewSet.initial() before DRF runs permission checks. + Returns 403 if the authenticated user lacks authoring rights on that + course, or if no course key could be derived. + """ + + def has_permission(self, request, view): + course_key = getattr(view, "course_key", None) + if not course_key: + return False + return has_course_author_access(request.user, course_key) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_xblock_viewset.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_xblock_viewset.py new file mode 100644 index 000000000000..8694f7023683 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_xblock_viewset.py @@ -0,0 +1,144 @@ +""" +Tests for XblockViewSet (v1 — ADR 0028, 0026, 0029). + +Verifies: + * Each HTTP method routes to the correct per-verb handler (ADR 0028) + * Unauthenticated requests return standardized 401 (ADR 0029) + * Authenticated non-authors return standardized 403 (ADR 0029) + * ADR 0029 error envelope fields are present and correctly typed +""" +from unittest.mock import patch + +from django.http import JsonResponse +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APITestCase + +from common.djangoapps.student.tests.factories import GlobalStaffFactory, UserFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + +TEST_LOCATOR = "block-v1:edX+ToyX+Toy_Course+type@problem+block@ba6327f840da49289fb27a9243913478" +PARENT_LOCATOR = "block-v1:edX+ToyX+Toy_Course+type@vertical+block@vert1" + +_REQUIRED_ERROR_FIELDS = ("type", "title", "status", "detail", "instance") + +_MOCK_RESPONSE = JsonResponse({"locator": TEST_LOCATOR}) + +_VIEW_MODULE = "cms.djangoapps.contentstore.rest_api.v1.views.xblock" + + +def _list_url(): + return reverse("cms.djangoapps.contentstore:v1:xblock-list") + + +def _detail_url(): + return reverse( + "cms.djangoapps.contentstore:v1:xblock-detail", + kwargs={"usage_key_string": TEST_LOCATOR}, + ) + + +# --------------------------------------------------------------------------- +# Routing tests +# --------------------------------------------------------------------------- + + +class XblockViewSetRoutingTest(ModuleStoreTestCase, APITestCase): + """Verify each HTTP method routes to the correct per-verb handler (ADR 0028).""" + + def setUp(self): + super().setUp() + self.staff = GlobalStaffFactory(password='password') + self.client.force_authenticate(user=self.staff) + + @patch(f"{_VIEW_MODULE}.create_xblock_response", return_value=_MOCK_RESPONSE) + def test_post_calls_create_xblock_response(self, mock_fn): + data = {"parent_locator": PARENT_LOCATOR, "category": "html"} + response = self.client.post(_list_url(), data=data, format="json") + assert response.status_code == status.HTTP_200_OK + mock_fn.assert_called_once() + assert mock_fn.call_args[0][0].method == "POST" + + @patch(f"{_VIEW_MODULE}.retrieve_xblock_response", return_value=_MOCK_RESPONSE) + def test_get_calls_retrieve_xblock_response(self, mock_fn): + response = self.client.get(_detail_url()) + assert response.status_code == status.HTTP_200_OK + mock_fn.assert_called_once() + assert mock_fn.call_args[0][0].method == "GET" + + @patch(f"{_VIEW_MODULE}.update_xblock_response", return_value=_MOCK_RESPONSE) + def test_put_calls_update_xblock_response(self, mock_fn): + data = {"id": TEST_LOCATOR, "data": "

Updated

"} + response = self.client.put(_detail_url(), data=data, format="json") + assert response.status_code == status.HTTP_200_OK + mock_fn.assert_called_once() + assert mock_fn.call_args[0][0].method == "PUT" + + @patch(f"{_VIEW_MODULE}.update_xblock_response", return_value=_MOCK_RESPONSE) + def test_patch_calls_update_xblock_response(self, mock_fn): + data = {"id": TEST_LOCATOR, "display_name": "New Name"} + response = self.client.patch(_detail_url(), data=data, format="json") + assert response.status_code == status.HTTP_200_OK + mock_fn.assert_called_once() + assert mock_fn.call_args[0][0].method == "PATCH" + + @patch(f"{_VIEW_MODULE}.delete_xblock_response", return_value=_MOCK_RESPONSE) + def test_delete_calls_delete_xblock_response(self, mock_fn): + response = self.client.delete(_detail_url()) + assert response.status_code == status.HTTP_200_OK + mock_fn.assert_called_once() + assert mock_fn.call_args[0][0].method == "DELETE" + + +# --------------------------------------------------------------------------- +# ADR 0029 error-shape tests +# --------------------------------------------------------------------------- + + +class XblockViewSetErrorShapeTest(ModuleStoreTestCase, APITestCase): + """Verify ADR 0029 standardized error envelope for auth failures.""" + + def setUp(self): + super().setUp() + self.non_author = UserFactory.create(password='password') + + def test_unauthenticated_returns_401(self): + response = self.client.get(_detail_url()) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_unauthenticated_401_has_required_fields(self): + response = self.client.get(_detail_url()) + data = response.json() + for field in _REQUIRED_ERROR_FIELDS: + assert field in data, f"Missing ADR 0029 field: {field}" + + def test_unauthenticated_401_type_uri(self): + response = self.client.get(_detail_url()) + assert response.json()["type"] == "https://docs.openedx.org/errors/authn" + + def test_non_author_returns_403(self): + self.client.force_authenticate(user=self.non_author) + response = self.client.get(_detail_url()) + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_non_author_403_has_required_fields(self): + self.client.force_authenticate(user=self.non_author) + response = self.client.get(_detail_url()) + data = response.json() + for field in _REQUIRED_ERROR_FIELDS: + assert field in data, f"Missing ADR 0029 field: {field}" + + def test_non_author_403_type_uri(self): + self.client.force_authenticate(user=self.non_author) + response = self.client.get(_detail_url()) + assert response.json()["type"] == "https://docs.openedx.org/errors/authz" + + def test_error_body_has_no_developer_message(self): + response = self.client.get(_detail_url()) + data = response.json() + assert "developer_message" not in data + assert "error_code" not in data + + def test_instance_field_is_request_path(self): + response = self.client.get(_detail_url()) + assert response.json()["instance"] == _detail_url() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py b/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py new file mode 100644 index 000000000000..acd4a8192ad9 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py @@ -0,0 +1,119 @@ +""" +API Views for Studio xblock CRUD — v1. + +Standardizes the v0 XblockView + XblockCreateView pair into a single +XblockViewSet applying the FC-0118 ADRs: + + * ADR 0025 - serializer_class + * ADR 0026 - explicit authentication_classes + permission_classes + * ADR 0028 - consolidated into XblockViewSet via DefaultRouter + * ADR 0029 - standardized error envelope via StandardizedErrorMixin +""" +import json +import logging + +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey +from rest_framework import viewsets +from rest_framework.permissions import IsAuthenticated + +from cms.djangoapps.contentstore.rest_api.v0.serializers import XblockSerializer +from cms.djangoapps.contentstore.rest_api.v0.views.utils import validate_request_with_serializer +from cms.djangoapps.contentstore.rest_api.v1.views.permissions import HasCourseAuthorAccess +from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import ( + create_xblock_response, + delete_xblock_response, + retrieve_xblock_response, + update_xblock_response, +) +from common.djangoapps.util.json_request import expect_json_in_class_view +from openedx.core.lib.api.mixins import StandardizedErrorMixin + +log = logging.getLogger(__name__) + + +class XblockViewSet(StandardizedErrorMixin, viewsets.ViewSet): + """ + ViewSet for xblock CRUD operations (v1 — ADR 0028). + + Router-generated URLs: + POST /api/contentstore/v1/xblock/ → create + GET /api/contentstore/v1/xblock/{usage_key_string}/ → retrieve + PUT /api/contentstore/v1/xblock/{usage_key_string}/ → update + PATCH /api/contentstore/v1/xblock/{usage_key_string}/ → partial_update + DELETE /api/contentstore/v1/xblock/{usage_key_string}/ → destroy + """ + + authentication_classes = ( + JwtAuthentication, + SessionAuthenticationAllowInactiveUser, + ) + permission_classes = (IsAuthenticated, HasCourseAuthorAccess) + serializer_class = XblockSerializer + lookup_field = "usage_key_string" + lookup_value_regex = r'(?:i4x://?[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+)' + + def __init__(self, **kwargs): + super().__init__(**kwargs) + self.course_key = None + + def initial(self, request, *args, **kwargs): + """ + Derive course_key and store it as self.course_key before DRF runs + permission checks. + + Detail actions (GET/PUT/PATCH/DELETE): course_key is extracted from + the usage key embedded in the URL. + Create action (POST): course_key is extracted from parent_locator in + the raw request body. We read request._request.body (Django's cached + bytes) rather than request.data to avoid consuming the WSGI stream + before @expect_json_in_class_view runs. + """ + usage_key_string = kwargs.get("usage_key_string") + if usage_key_string: + try: + self.course_key = UsageKey.from_string(usage_key_string).course_key + except InvalidKeyError: + self.course_key = None + else: + try: + # pylint: disable=protected-access + body = json.loads(request._request.body or b'{}') + parent_locator = body.get("parent_locator", "") + self.course_key = ( + UsageKey.from_string(parent_locator).course_key + if parent_locator else None + ) + except (ValueError, InvalidKeyError): + self.course_key = None + super().initial(request, *args, **kwargs) + + @expect_json_in_class_view + @validate_request_with_serializer + def create(self, request): + """Create a new xblock under the given parent.""" + return create_xblock_response(request) + + @expect_json_in_class_view + def retrieve(self, request, usage_key_string=None): + """Retrieve an xblock by its usage key.""" + return retrieve_xblock_response(request, usage_key_string) + + @expect_json_in_class_view + @validate_request_with_serializer + def update(self, request, usage_key_string=None): + """Fully update an xblock.""" + return update_xblock_response(request, usage_key_string) + + @expect_json_in_class_view + @validate_request_with_serializer + def partial_update(self, request, usage_key_string=None): + """Partially update an xblock.""" + return update_xblock_response(request, usage_key_string) + + @expect_json_in_class_view + def destroy(self, request, usage_key_string=None): + """Delete an xblock.""" + return delete_xblock_response(request, usage_key_string) diff --git a/cms/djangoapps/contentstore/rest_api/v3/__init__.py b/cms/djangoapps/contentstore/rest_api/v3/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cms/djangoapps/contentstore/rest_api/v3/tests/__init__.py b/cms/djangoapps/contentstore/rest_api/v3/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cms/djangoapps/contentstore/rest_api/v3/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v3/tests/test_home.py new file mode 100644 index 000000000000..54c08b0a3894 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/tests/test_home.py @@ -0,0 +1,86 @@ +""" +ADR 0029 – Standardized error-response regression tests for HomeViewSet (v3). + +The ADR 0029 envelope is wired into the v3 viewset via +:class:`openedx.core.lib.api.mixins.StandardizedErrorMixin`, +which overrides DRF's per-view ``get_exception_handler`` to point at +``openedx.core.lib.api.exceptions.standardized_error_exception_handler``. + +This is intentionally *scoped to v3* — the project-wide DRF +``EXCEPTION_HANDLER`` setting is unchanged, so v0/v1/v2 endpoints continue +to return the legacy error shape. +""" +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 TestHomeViewSetErrorShape(APITestCase): + """ + ADR 0029 – error response shape regression tests for HomeViewSet (v3). + + Verifies that 401 responses on all three actions conform to the + standardized JSON envelope. + """ + + def setUp(self): + super().setUp() + self.client = APIClient() + self.list_url = reverse("cms.djangoapps.contentstore:v3:home-list") + self.courses_url = reverse("cms.djangoapps.contentstore:v3:home-courses") + self.libraries_url = reverse("cms.djangoapps.contentstore:v3:home-libraries") + + def test_unauthenticated_list_returns_standardized_401(self): + """Unauthenticated GET /home/ must return 401 with the ADR 0029 envelope.""" + response = self.client.get(self.list_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + def test_unauthenticated_list_401_type_uri(self): + """The ``type`` field for 401 must be the ADR 0029 authn URI.""" + response = self.client.get(self.list_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response.data.get("type") == "https://docs.openedx.org/errors/authn" + + def test_unauthenticated_courses_returns_standardized_401(self): + """Unauthenticated GET /home/courses/ must return 401 with the ADR 0029 envelope.""" + response = self.client.get(self.courses_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + def test_unauthenticated_libraries_returns_standardized_401(self): + """Unauthenticated GET /home/libraries/ must return 401 with the ADR 0029 envelope.""" + response = self.client.get(self.libraries_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + def test_error_body_has_no_developer_message(self): + """Error responses must NOT contain old DeveloperErrorViewMixin fields.""" + response = self.client.get(self.list_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert "developer_message" not in response.data + assert "error_code" not in response.data + + def test_instance_field_is_request_path(self): + """The ``instance`` field must equal the request path.""" + response = self.client.get(self.list_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response.data.get("instance") == self.list_url + + def test_v1_endpoint_unaffected_by_v3_envelope(self): + """ + The ADR 0029 envelope must be scoped to v3 — hitting the legacy v1 + ``home/courses`` endpoint unauthenticated must NOT return the v3 envelope + (it has no ``type`` / ``instance`` keys). + """ + v1_url = reverse("cms.djangoapps.contentstore:v1:courses") + response = self.client.get(v1_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + # v1 still uses the project-default handler → ADR 0029 fields absent. + assert "type" not in response.data + assert "instance" not in response.data diff --git a/cms/djangoapps/contentstore/rest_api/v3/urls.py b/cms/djangoapps/contentstore/rest_api/v3/urls.py new file mode 100644 index 000000000000..36eb245396b9 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/urls.py @@ -0,0 +1,14 @@ +"""Contentstore API v3 URLs.""" + +from rest_framework.routers import DefaultRouter + +from cms.djangoapps.contentstore.rest_api.v3.views import AuthoringGradingViewSet, CourseDetailsViewSet, HomeViewSet + +app_name = "v3" + +router = DefaultRouter() +router.register(r'home', HomeViewSet, basename='home') +router.register(r'course_details', CourseDetailsViewSet, basename='course_details') +router.register(r'authoring_grading', AuthoringGradingViewSet, basename='authoring_grading') + +urlpatterns = router.urls diff --git a/cms/djangoapps/contentstore/rest_api/v3/utils.py b/cms/djangoapps/contentstore/rest_api/v3/utils.py new file mode 100644 index 000000000000..3db96963b764 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/utils.py @@ -0,0 +1,55 @@ +""" +Shared utilities for v3 contentstore API viewsets. + +Houses the small helpers and OpenAPI constants that more than one v3 viewset +needs, so the per-viewset modules stay focused on action bodies and don't +drift apart over time. + +Currently provides: + * :func:`resolve_course_key` – parse-and-verify a course key string, + raising ``NotFound`` for unparseable keys or missing courses (replaces + the legacy ``@verify_course_exists()`` decorator from v1 and avoids + relying on ``DeveloperErrorViewMixin``). + * :data:`COMMON_ERROR_RESPONSES` – the shared ``@extend_schema(responses=...)`` + fragment for the 401 / 403 / 404 cases every v3 course-scoped viewset + can raise. +""" + +from drf_spectacular.utils import OpenApiResponse +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from rest_framework.exceptions import NotFound + +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + + +def resolve_course_key(course_key: str) -> CourseKey: + """ + Parse ``course_key`` (string) into a :class:`CourseKey` and verify the + course exists. + + Raises: + rest_framework.exceptions.NotFound: if the string is unparseable + *or* the course does not exist. The ADR 0029 envelope (wired in + by :class:`openedx.core.lib.api.mixins.StandardizedErrorMixin`) + renders both as a structured 404. + + OEP-68: the parameter name is ``course_key`` rather than the legacy + ``course_id``. The function is intentionally agnostic to which URL kwarg + name the caller used — callers may pass the value of either kwarg as a + positional argument. + """ + try: + parsed = CourseKey.from_string(course_key) + except InvalidKeyError as exc: + raise NotFound("The provided course key cannot be parsed.") from exc + if not CourseOverview.course_exists(parsed): + raise NotFound(f"Course {course_key} not found.") + return parsed + + +COMMON_ERROR_RESPONSES = { + 401: OpenApiResponse(description="The requester is not authenticated."), + 403: OpenApiResponse(description="The requester cannot access the specified course."), + 404: OpenApiResponse(description="The requested course does not exist."), +} diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v3/views/__init__.py new file mode 100644 index 000000000000..d3d8670950a3 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/views/__init__.py @@ -0,0 +1,5 @@ +"""Views for v3 contentstore API.""" + +from .authoring_grading import AuthoringGradingViewSet # noqa: F401 +from .course_details import CourseDetailsViewSet # noqa: F401 +from .home import HomeViewSet # noqa: F401 diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/authoring_grading.py b/cms/djangoapps/contentstore/rest_api/v3/views/authoring_grading.py new file mode 100644 index 000000000000..49dd882b4f07 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/views/authoring_grading.py @@ -0,0 +1,163 @@ +""" +API Views for course grading settings — v3. + +This module is the v3 incarnation of the v0 ``AuthoringGradingView`` endpoint, +restructured to apply the FC-0118 ADRs from the start: + + * ADR 0025 – ``serializer_class`` on the viewset + * ADR 0026 – explicit ``authentication_classes`` + ``permission_classes`` + * ADR 0027 – ``drf_spectacular`` for OpenAPI schema generation + * ADR 0028 – consolidated into a single DRF ``ViewSet`` registered via + ``DefaultRouter`` (replaces ``AuthoringGradingView`` ``APIView``) + * ADR 0029 – standardized error envelope via :class:`StandardizedErrorMixin` + (v3-scoped — does not change the project-wide DRF ``EXCEPTION_HANDLER`` + setting) + * ADR 0033 / OEP-68 – the URL kwarg, action parameter, and OpenAPI parameter + are named ``course_key`` (the OEP-68-standardized name) rather than the + legacy ``course_id``. Since this is a brand-new versioned API, no + deprecated alias is needed — clients on the v0 endpoint continue to use + ``course_id`` there. + +Permission model note: + PR #38363 proposed a class-level ``HasStudioReadAccess`` permission. The + current v0 view has since evolved to use the ``openedx_authz`` permission + framework (``COURSES_EDIT_GRADING_SETTINGS``), which is more specific to + grading and aligns with the platform-wide authz direction. + + The v3 viewset preserves the openedx_authz model via an *inline* + ``user_has_course_permission`` check inside the action body (rather than + the ``@authz_permission_required`` decorator). The decorator raises + ``DeveloperErrorResponseException`` — a plain ``Exception`` subclass that + does not flow through DRF's exception handler, so it would bypass + :class:`StandardizedErrorMixin` and surface as an unstructured 500. + Raising ``rest_framework.exceptions.PermissionDenied`` directly keeps the + ADR 0029 envelope intact. +""" + +from drf_spectacular.utils import OpenApiParameter, OpenApiRequest, OpenApiResponse, extend_schema +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from openedx_authz.constants.permissions import COURSES_EDIT_GRADING_SETTINGS +from rest_framework import viewsets +from rest_framework.exceptions import PermissionDenied +from rest_framework.permissions import IsAuthenticated +from rest_framework.request import Request +from rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.v0.serializers import CourseGradingModelSerializer +from cms.djangoapps.contentstore.rest_api.v3.utils import COMMON_ERROR_RESPONSES, resolve_course_key +from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission +from openedx.core.djangoapps.authz.decorators import user_has_course_permission +from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements +from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser +from openedx.core.lib.api.mixins import StandardizedErrorMixin + +_COURSE_KEY_PARAMETER = OpenApiParameter( + name="course_key", + description="OEP-68 course key (e.g. course-v1:org+course+run).", + required=True, + type=str, + location=OpenApiParameter.PATH, +) + + +class AuthoringGradingViewSet(StandardizedErrorMixin, viewsets.ViewSet): + """ + ViewSet for course grading settings (v3). Registered via DefaultRouter + (basename ``authoring_grading``). + + Router-generated URL:: + + PATCH /api/contentstore/v3/authoring_grading/{course_key}/ → partial_update + + Supersedes ``AuthoringGradingView`` at ``POST /api/contentstore/v0/grading/{course_id}``. + """ + + authentication_classes = ( + JwtAuthentication, + BearerAuthenticationAllowInactiveUser, + SessionAuthenticationAllowInactiveUser, + ) + permission_classes = (IsAuthenticated,) + serializer_class = CourseGradingModelSerializer + + # DefaultRouter lookup: matches course-v1:org+course+run (+ or / separators). + # OEP-68: the kwarg name is ``course_key`` (not the legacy ``course_id``). + lookup_field = "course_key" + lookup_value_regex = r"[^/+]+(?:/|\+)[^/+]+(?:/|\+)[^/?]+" + + def get_serializer(self, *args, **kwargs): + """Instantiate and return the configured serializer class.""" + return self.serializer_class(*args, **kwargs) + + @extend_schema( + summary="Update a course's grading settings", + description="Partially update the grading settings for the specified course.", + request=OpenApiRequest(request=CourseGradingModelSerializer), + parameters=[_COURSE_KEY_PARAMETER], + responses={ + 200: OpenApiResponse( + response=CourseGradingModelSerializer, + description="Grading settings updated successfully.", + ), + **COMMON_ERROR_RESPONSES, + }, + ) + def partial_update(self, request: Request, course_key: str): + """ + Update a course's grading settings. + + **Example Request** + + PATCH /api/contentstore/v3/authoring_grading/{course_key}/ + + **PATCH Parameters** + + The request body should follow the ``CourseGradingModelSerializer`` + schema. Example:: + + { + "graders": [ + { + "type": "Homework", + "min_count": 1, + "drop_count": 0, + "short_label": "", + "weight": 100, + "id": 0 + } + ], + "grade_cutoffs": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5}, + "grace_period": {"hours": 12, "minutes": 0}, + "minimum_grade_credit": 0.7, + "is_credit_course": true + } + + **Response Values** + + If the request is successful, an HTTP 200 "OK" response is returned + with the updated grading data serialized via + :class:`CourseGradingModelSerializer`. + """ + parsed_course_key = resolve_course_key(course_key) + + # Per-action authorization (ADR 0026): kept inline rather than + # behind ``@authz_permission_required`` because that decorator + # raises ``DeveloperErrorResponseException`` (not a DRF exception), + # which bypasses :class:`StandardizedErrorMixin`. Raising + # ``PermissionDenied`` directly flows through the ADR 0029 envelope. + if not user_has_course_permission( + request.user, + COURSES_EDIT_GRADING_SETTINGS.identifier, + parsed_course_key, + LegacyAuthoringPermission.READ, + ): + raise PermissionDenied("You do not have permission to perform this action.") + + if "minimum_grade_credit" in request.data: + update_credit_course_requirements.delay(str(parsed_course_key)) + + updated_data = CourseGradingModel.update_from_json(parsed_course_key, request.data, request.user) + serializer = self.get_serializer(updated_data) + return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/course_details.py b/cms/djangoapps/contentstore/rest_api/v3/views/course_details.py new file mode 100644 index 000000000000..47a4743854b0 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/views/course_details.py @@ -0,0 +1,178 @@ +""" +API Views for course details — v3. + +This module is the v3 incarnation of the v1 ``course_details`` endpoint, +restructured to apply the FC-0118 ADRs: + + * ADR 0025 – ``serializer_class`` on the viewset + * ADR 0026 – explicit ``authentication_classes`` + ``permission_classes`` + * ADR 0027 – ``drf_spectacular`` for OpenAPI schema generation + * ADR 0028 – consolidated into a single DRF ``ViewSet`` registered via + ``DefaultRouter`` (replaces ``CourseDetailsView`` ``APIView``) + * ADR 0029 – standardized error envelope via :class:`StandardizedErrorMixin` + (v3-scoped — does not change the project-wide DRF ``EXCEPTION_HANDLER`` + setting) + +Permission model note: + PR #38365 proposed a class-level ``HasStudioReadAccess`` permission. The + current v1 view has since evolved to use the ``openedx_authz`` permission + framework with a schedule-vs-details classification that gates updates on + *different* permissions depending on the payload. That granularity cannot + be hoisted to a single class-level permission, so the per-action checks + remain inline (gated by ``IsAuthenticated`` at the class level). +""" + +from django.core.exceptions import ValidationError as DjangoValidationError +from drf_spectacular.utils import OpenApiParameter, OpenApiRequest, OpenApiResponse, extend_schema +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from openedx_authz.constants.permissions import ( + COURSES_EDIT_DETAILS, + COURSES_EDIT_SCHEDULE, + COURSES_VIEW_SCHEDULE_AND_DETAILS, +) +from rest_framework import viewsets +from rest_framework.exceptions import ValidationError as DRFValidationError +from rest_framework.permissions import IsAuthenticated +from rest_framework.request import Request +from rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.v1.serializers import CourseDetailsSerializer +from cms.djangoapps.contentstore.rest_api.v1.views.course_details import _classify_update +from cms.djangoapps.contentstore.rest_api.v3.utils import COMMON_ERROR_RESPONSES, resolve_course_key +from cms.djangoapps.contentstore.utils import update_course_details +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission +from openedx.core.djangoapps.authz.decorators import user_has_course_permission +from openedx.core.djangoapps.models.course_details import CourseDetails +from openedx.core.lib.api.mixins import StandardizedErrorMixin +from xmodule.modulestore.django import modulestore + +_COURSE_ID_PARAMETER = OpenApiParameter( + name="course_id", + description="Course ID", + required=True, + type=str, + location=OpenApiParameter.PATH, +) + + +class CourseDetailsViewSet(StandardizedErrorMixin, viewsets.ViewSet): + """ + ViewSet for course details (v3). Registered via DefaultRouter (basename ``course_details``). + + Router-generated URLs:: + + GET /api/contentstore/v3/course_details/{course_id}/ → retrieve + PUT /api/contentstore/v3/course_details/{course_id}/ → update + + Supersedes ``CourseDetailsView`` at ``/api/contentstore/v1/course_details/{course_id}``. + """ + + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated,) + serializer_class = CourseDetailsSerializer + + # Matches both slash-separated (org/course/run) and plus-separated (course-v1:org+course+run) IDs + lookup_field = "course_id" + lookup_value_regex = r"[^/+]+(?:/|\+)[^/+]+(?:/|\+)[^/?]+" + + @extend_schema( + summary="Retrieve a course's details", + description="Get an object containing all the course details for the specified course.", + parameters=[_COURSE_ID_PARAMETER], + responses={ + 200: OpenApiResponse( + response=CourseDetailsSerializer, + description="Course details retrieved successfully.", + ), + **COMMON_ERROR_RESPONSES, + }, + ) + def retrieve(self, request: Request, course_id: str): + """ + Get an object containing all the course details. + + **Example Request** + + GET /api/contentstore/v3/course_details/{course_id}/ + """ + course_key = resolve_course_key(course_id) + if not user_has_course_permission( + request.user, + COURSES_VIEW_SCHEDULE_AND_DETAILS.identifier, + course_key, + LegacyAuthoringPermission.READ, + ): + self.permission_denied(request) + + course_details = CourseDetails.fetch(course_key) + serializer = self.serializer_class(course_details) + return Response(serializer.data) + + @extend_schema( + summary="Update a course's details", + description="Update the details for the specified course.", + request=OpenApiRequest(request=CourseDetailsSerializer), + parameters=[_COURSE_ID_PARAMETER], + responses={ + 200: OpenApiResponse( + response=CourseDetailsSerializer, + description="Course details updated successfully.", + ), + 400: OpenApiResponse(description="Bad request — invalid data."), + **COMMON_ERROR_RESPONSES, + }, + ) + def update(self, request: Request, course_id: str): + """ + Update a course's details. + + **Example Request** + + PUT /api/contentstore/v3/course_details/{course_id}/ + + **PUT Parameters** + + The data sent for a put request should follow a similar format as + is returned by a ``GET`` request. Multiple details can be updated in + a single request, however only the ``value`` field can be updated; + any other fields, if included, will be ignored. + + **Response Values** + + If the request is successful, an HTTP 200 "OK" response is returned, + along with all the course's details similar to a ``GET`` request. + """ + course_key = resolve_course_key(course_id) + is_schedule_update, is_details_update = _classify_update(request.data, course_key) + + if not is_schedule_update and not is_details_update: + # No updatable fields provided — fall through to a details-permission check + # so the caller gets 403 if they lack edit-details rights. + is_details_update = True + + if is_schedule_update and not user_has_course_permission( + request.user, + COURSES_EDIT_SCHEDULE.identifier, + course_key, + LegacyAuthoringPermission.READ, + ): + self.permission_denied(request) + + if is_details_update and not user_has_course_permission( + request.user, + COURSES_EDIT_DETAILS.identifier, + course_key, + LegacyAuthoringPermission.READ, + ): + self.permission_denied(request) + + course_block = modulestore().get_course(course_key) + + try: + updated_data = update_course_details(request, course_key, request.data, course_block) + except DjangoValidationError as err: + raise DRFValidationError(err.message) from err + + serializer = self.serializer_class(updated_data) + return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/home.py b/cms/djangoapps/contentstore/rest_api/v3/views/home.py new file mode 100644 index 000000000000..2e11b191806e --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/views/home.py @@ -0,0 +1,164 @@ +""" +API Views for Studio course home — v3. + +This module is the v3 incarnation of the v1 ``home`` endpoints, restructured +to apply the FC-0118 ADRs: + + * ADR 0025 – ``serializer_class`` (with per-action ``get_serializer_class``) + * ADR 0026 – explicit ``authentication_classes`` + ``permission_classes`` + * ADR 0028 – consolidated into a single DRF ``ViewSet`` registered via + ``DefaultRouter`` (replaces the three legacy ``APIView`` classes + ``HomePageView`` / ``HomePageCoursesView`` / ``HomePageLibrariesView``) + * ADR 0029 – standardized error envelope, opted in via + :class:`StandardizedErrorMixin` (v3-scoped — does not change the + project-wide DRF ``EXCEPTION_HANDLER`` setting) +""" + +import edx_api_doc_tools as apidocs +from django.conf import settings +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from organizations import api as org_api +from rest_framework import viewsets +from rest_framework.decorators import action +from rest_framework.permissions import IsAuthenticated +from rest_framework.request import Request +from rest_framework.response import Response + +from cms.djangoapps.contentstore.rest_api.v1.serializers import ( + CourseHomeTabSerializer, + LibraryTabSerializer, + StudioHomeSerializer, +) +from cms.djangoapps.contentstore.utils import get_course_context, get_home_context, get_library_context +from openedx.core.lib.api.mixins import StandardizedErrorMixin + + +class HomeViewSet(StandardizedErrorMixin, viewsets.ViewSet): + """ + ViewSet for the Studio home page. Registered via DefaultRouter (basename ``home``). + + Router-generated URLs: + GET /api/contentstore/v3/home/ → list (aggregated home context) + GET /api/contentstore/v3/home/courses/ → courses (course list only) + GET /api/contentstore/v3/home/libraries/ → libraries (library list only) + """ + + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated,) + serializer_class = StudioHomeSerializer + + def get_serializer_class(self): + """Return the appropriate serializer class for the current action.""" + if self.action == 'courses': + return CourseHomeTabSerializer + if self.action == 'libraries': + return LibraryTabSerializer + return StudioHomeSerializer + + def get_serializer(self, *args, **kwargs): + """Return a serializer instance using the action-appropriate class.""" + return self.get_serializer_class()(*args, **kwargs) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "org", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course org", + )], + responses={ + 200: StudioHomeSerializer, + 401: "The requester is not authenticated.", + }, + ) + def list(self, request: Request): + """ + Get an object containing all courses and libraries on home page. + + **Example Request** + + GET /api/contentstore/v3/home/ + """ + home_context = get_home_context(request, True) + home_context.update({ + # 'allow_to_create_new_org' is actually about auto-creating organizations + # (e.g. when creating a course or library), so we add an additional test. + 'allow_to_create_new_org': ( + home_context['can_create_organizations'] and + org_api.is_autocreate_enabled() + ), + 'studio_name': settings.STUDIO_NAME, + 'studio_short_name': settings.STUDIO_SHORT_NAME, + 'studio_request_email': settings.FEATURES.get('STUDIO_REQUEST_EMAIL', ''), + 'tech_support_email': settings.TECH_SUPPORT_EMAIL, + 'platform_name': settings.PLATFORM_NAME, + 'user_is_active': request.user.is_active, + }) + serializer = self.get_serializer(home_context) + return Response(serializer.data) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "org", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course org", + )], + responses={ + 200: CourseHomeTabSerializer, + 401: "The requester is not authenticated.", + }, + ) + @action(detail=False, methods=['get'], url_path='courses', url_name='courses') + def courses(self, request: Request): + """ + Get an object containing all courses. + + **Example Request** + + GET /api/contentstore/v3/home/courses/ + """ + active_courses, archived_courses, in_process_course_actions = get_course_context(request) + courses_context = { + "courses": active_courses, + "archived_courses": archived_courses, + "in_process_course_actions": in_process_course_actions, + } + serializer = self.get_serializer(courses_context) + return Response(serializer.data) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "org", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course org", + ), + apidocs.query_parameter( + "is_migrated", + bool, + description=( + "Query param to filter by migrated status of library." + " If present (true or false), it will filter by migration status" + " else it will return all legacy libraries." + ), + ) + ], + responses={ + 200: LibraryTabSerializer, + 401: "The requester is not authenticated.", + }, + ) + @action(detail=False, methods=['get'], url_path='libraries', url_name='libraries') + def libraries(self, request: Request): + """ + Get an object containing all libraries on home page. + + **Example Request** + + GET /api/contentstore/v3/home/libraries/ + """ + library_context = get_library_context(request) + serializer = self.get_serializer(library_context) + return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/tests/__init__.py b/cms/djangoapps/contentstore/rest_api/v3/views/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_authoring_grading.py b/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_authoring_grading.py new file mode 100644 index 000000000000..33f9efd69ff4 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_authoring_grading.py @@ -0,0 +1,293 @@ +""" +Unit tests for AuthoringGradingViewSet (v3). + +Single test module covering every ADR applied to the viewset: + * ADR 0025 / 0026 / 0027 / 0028 — action + permission + routing tests + (``TestAuthoringGradingViewSetPermissions``, ``TestAuthoringGradingViewSetUpdate``, + ``TestAuthoringGradingViewSetRouting``) + * ADR 0029 — standardized error envelope shape tests + (``TestAuthoringGradingViewSetErrorShape``) + +MongoDB-free: every service-layer call (``CourseGradingModel.update_from_json``, +``CourseOverview.course_exists``, ``update_credit_course_requirements.delay``, +and the ``openedx_authz`` permission lookup) is mocked, so these tests run +without a live modulestore or course-overview row. +""" +import json +from types import SimpleNamespace +from unittest.mock import patch + +from django.test import TestCase +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient, APITestCase + +from common.djangoapps.student.tests.factories import UserFactory + +COURSE_ID = "course-v1:edX+ToyX+Toy_Course" + +# Minimal graders payload accepted by CourseGradingModelSerializer. +_GRADERS_PAYLOAD = [ + { + "type": "Homework", + "min_count": 1, + "drop_count": 0, + "short_label": "", + "weight": 100, + "id": 0, + }, +] + +# Fake CourseGradingModel return value: only the field the serializer reads. +_MOCK_GRADING_MODEL = SimpleNamespace(graders=_GRADERS_PAYLOAD) + +# CourseOverview.course_exists is called inside ``resolve_course_key()`` (now in +# v3/utils.py), not directly in the view module — so the patch must target the +# utils module's binding, not the view module's. +MOCK_COURSE_EXISTS = ( + "cms.djangoapps.contentstore.rest_api.v3.utils.CourseOverview.course_exists" +) +MOCK_UPDATE_FROM_JSON = ( + "cms.djangoapps.contentstore.rest_api.v3.views.authoring_grading.CourseGradingModel.update_from_json" +) +MOCK_CREDIT_TASK = ( + "cms.djangoapps.contentstore.rest_api.v3.views.authoring_grading." + "update_credit_course_requirements.delay" +) +# Patch the local module-level binding (imported from openedx.core.djangoapps.authz.decorators) +# — patching the source module would not replace the already-bound reference inside the view. +MOCK_HAS_PERMISSION = ( + "cms.djangoapps.contentstore.rest_api.v3.views.authoring_grading.user_has_course_permission" +) + +_REQUIRED_ERROR_FIELDS = ("type", "title", "status", "detail", "instance") + + +# =========================================================================== +# ADR 0026 — permission boundary tests +# =========================================================================== +class TestAuthoringGradingViewSetPermissions(APITestCase): + """ + ADR 0026 — permission boundary tests for the partial_update action. + """ + + def setUp(self): + super().setUp() + self.client = APIClient() + self.url = reverse( + "cms.djangoapps.contentstore:v3:authoring_grading-detail", + kwargs={"course_key": COURSE_ID}, + ) + + def test_unauthenticated_patch_returns_401(self): + """Unauthenticated PATCH must return 401 (IsAuthenticated).""" + response = self.client.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + @patch(MOCK_HAS_PERMISSION, return_value=False) + @patch(MOCK_COURSE_EXISTS, return_value=True) + def test_authenticated_without_grading_permission_returns_403(self, mock_exists, mock_perm): # noqa: ARG002 + """Authenticated user without grading permission must receive 403.""" + user = UserFactory.create() + self.client.force_authenticate(user=user) + response = self.client.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_403_FORBIDDEN + + +# =========================================================================== +# ADR 0025 / 0028 — action body tests +# =========================================================================== +class TestAuthoringGradingViewSetUpdate(APITestCase): + """ + Action tests for the partial_update flow. + + All service-layer interactions are mocked so the test exercises the + routing, serialization, and credit-task wiring without touching MongoDB + or modulestore. + """ + + def setUp(self): + super().setUp() + self.client = APIClient() + self.user = UserFactory.create(is_staff=True) + self.client.force_authenticate(user=self.user) + self.url = reverse( + "cms.djangoapps.contentstore:v3:authoring_grading-detail", + kwargs={"course_key": COURSE_ID}, + ) + + @patch(MOCK_CREDIT_TASK) + @patch(MOCK_UPDATE_FROM_JSON, return_value=_MOCK_GRADING_MODEL) + @patch(MOCK_HAS_PERMISSION, return_value=True) + @patch(MOCK_COURSE_EXISTS, return_value=True) + def test_patch_with_minimum_grade_credit_fires_credit_task( + self, mock_exists, mock_perm, mock_update, mock_credit_task, # noqa: ARG002 + ): + """``minimum_grade_credit`` in the payload triggers the credit-requirements Celery task.""" + body = { + "graders": _GRADERS_PAYLOAD, + "grade_cutoffs": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5}, + "grace_period": {"hours": 12, "minutes": 0}, + "minimum_grade_credit": 0.7, + "is_credit_course": True, + } + response = self.client.patch(self.url, data=json.dumps(body), content_type="application/json") + assert response.status_code == status.HTTP_200_OK + mock_update.assert_called_once() + mock_credit_task.assert_called_once() + + @patch(MOCK_CREDIT_TASK) + @patch(MOCK_UPDATE_FROM_JSON, return_value=_MOCK_GRADING_MODEL) + @patch(MOCK_HAS_PERMISSION, return_value=True) + @patch(MOCK_COURSE_EXISTS, return_value=True) + def test_patch_without_minimum_grade_credit_skips_credit_task( + self, mock_exists, mock_perm, mock_update, mock_credit_task, # noqa: ARG002 + ): + """Absent ``minimum_grade_credit`` keeps the credit-requirements task unscheduled.""" + body = { + "graders": _GRADERS_PAYLOAD, + "grade_cutoffs": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5}, + "grace_period": {"hours": 12, "minutes": 0}, + } + response = self.client.patch(self.url, data=json.dumps(body), content_type="application/json") + assert response.status_code == status.HTTP_200_OK + mock_update.assert_called_once() + mock_credit_task.assert_not_called() + + +# =========================================================================== +# ADR 0028 — routing checks +# =========================================================================== +class TestAuthoringGradingViewSetRouting(TestCase): + """ + Routing checks — confirm the URL namespace and HTTP-method mapping are wired correctly. + """ + + def test_detail_url_resolves(self): + """v3 router exposes the viewset under ``v3:authoring_grading-detail``.""" + url = reverse( + "cms.djangoapps.contentstore:v3:authoring_grading-detail", + kwargs={"course_key": COURSE_ID}, + ) + assert "/api/contentstore/v3/authoring_grading/" in url + assert COURSE_ID in url + + def test_post_not_allowed(self): + """The viewset only exposes PATCH (partial_update); POST must return 405.""" + client = APIClient() + client.force_authenticate(user=UserFactory.create(is_staff=True)) + url = reverse( + "cms.djangoapps.contentstore:v3:authoring_grading-detail", + kwargs={"course_key": COURSE_ID}, + ) + response = client.post(url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + + +# =========================================================================== +# ADR 0029 — standardized error-response envelope tests +# =========================================================================== +class TestAuthoringGradingViewSetErrorShape(APITestCase): + """ + ADR 0029 — error response shape regression tests for AuthoringGradingViewSet. + + The envelope is wired in via + :class:`openedx.core.lib.api.mixins.StandardizedErrorMixin`, which overrides + DRF's per-view ``get_exception_handler`` to point at + ``openedx.core.lib.api.exceptions.standardized_error_exception_handler``. + + Scoped to v3 — the project-wide DRF ``EXCEPTION_HANDLER`` setting is + unchanged, so v0 / v1 / v2 / v4 endpoints continue to return the legacy + error shape (locked in by ``test_v0_endpoint_unaffected_by_v3_envelope``). + """ + + def setUp(self): + super().setUp() + self.client = APIClient() + self.url = reverse( + "cms.djangoapps.contentstore:v3:authoring_grading-detail", + kwargs={"course_key": COURSE_ID}, + ) + + def test_unauthenticated_patch_returns_standardized_401(self): + """Unauthenticated PATCH must return 401 with the ADR 0029 envelope.""" + response = self.client.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + for field in _REQUIRED_ERROR_FIELDS: + assert field in 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.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response.data.get("type") == "https://docs.openedx.org/errors/authn" + + @patch(MOCK_COURSE_EXISTS, return_value=True) + @patch(MOCK_HAS_PERMISSION, return_value=False) + def test_non_author_patch_returns_standardized_403(self, mock_perm, mock_exists): # noqa: ARG002 + """Authenticated non-author PATCH must return 403 with the ADR 0029 envelope.""" + non_author = UserFactory.create() + self.client.force_authenticate(user=non_author) + response = self.client.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_403_FORBIDDEN + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + @patch(MOCK_COURSE_EXISTS, return_value=True) + @patch(MOCK_HAS_PERMISSION, return_value=False) + def test_non_author_403_type_uri(self, mock_perm, mock_exists): # noqa: ARG002 + """The ``type`` field for 403 must be the ADR 0029 authz URI.""" + non_author = UserFactory.create() + self.client.force_authenticate(user=non_author) + response = self.client.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.data.get("type") == "https://docs.openedx.org/errors/authz" + + @patch(MOCK_COURSE_EXISTS, return_value=False) + def test_nonexistent_course_returns_standardized_404(self, mock_exists): # noqa: ARG002 + """PATCH for a non-existent course must return 404 with the ADR 0029 envelope.""" + staff = UserFactory.create(is_staff=True) + self.client.force_authenticate(user=staff) + response = self.client.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_404_NOT_FOUND + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + @patch(MOCK_COURSE_EXISTS, return_value=False) + def test_not_found_type_uri(self, mock_exists): # noqa: ARG002 + """The ``type`` field for 404 must be the ADR 0029 not-found URI.""" + staff = UserFactory.create(is_staff=True) + self.client.force_authenticate(user=staff) + response = self.client.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.data.get("type") == "https://docs.openedx.org/errors/not-found" + + def test_error_body_has_no_developer_message(self): + """Error responses must NOT contain old DeveloperErrorViewMixin fields.""" + response = self.client.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert "developer_message" not in response.data + assert "error_code" not in response.data + + def test_instance_field_is_request_path(self): + """The ``instance`` field must equal the request path.""" + response = self.client.patch(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response.data.get("instance") == self.url + + def test_v0_endpoint_unaffected_by_v3_envelope(self): + """ + The ADR 0029 envelope must be scoped to v3 — hitting the legacy v0 + ``grading`` endpoint unauthenticated must NOT return the v3 envelope + (no ``type`` / ``instance`` keys). + """ + v0_url = reverse( + "cms.djangoapps.contentstore:v0:cms_api_update_grading", + # v0 URL uses the legacy ``course_id`` named group; only v3 was renamed + # to ``course_key`` per OEP-68. + kwargs={"course_id": COURSE_ID}, + ) + response = self.client.post(v0_url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert "type" not in response.data + assert "instance" not in response.data diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_course_details.py b/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_course_details.py new file mode 100644 index 000000000000..a7d0165fb32d --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_course_details.py @@ -0,0 +1,272 @@ +""" +Unit tests for CourseDetailsViewSet (v3). + +Single test module covering every ADR applied to the viewset: + * ADR 0025 / 0026 / 0027 / 0028 — action + permission + routing tests + (``TestCourseDetailsViewSetPermissions``, ``TestCourseDetailsViewSetActions``) + * ADR 0029 — standardized error envelope shape tests + (``TestCourseDetailsViewSetErrorShape``) + +MongoDB-free: every service-layer call (``CourseDetails.fetch``, +``modulestore``, ``update_course_details``, +``openedx_authz.user_has_course_permission``, and +``CourseOverview.course_exists``) is mocked so the suite runs without a live +modulestore or course-overview row. + +``patch.object`` is used for ``serializer_class`` because the attribute is +resolved at class-definition time — string-based ``patch()`` of the module +attribute does not replace the live ViewSet attribute. +""" +from unittest.mock import MagicMock, patch + +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient, APITestCase + +from cms.djangoapps.contentstore.rest_api.v3.views.course_details import CourseDetailsViewSet +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + +# --------------------------------------------------------------------------- +# Mock target paths +# --------------------------------------------------------------------------- +MOCK_FETCH = "openedx.core.djangoapps.models.course_details.CourseDetails.fetch" +MOCK_MODULESTORE = "cms.djangoapps.contentstore.rest_api.v3.views.course_details.modulestore" +MOCK_UPDATE = "cms.djangoapps.contentstore.rest_api.v3.views.course_details.update_course_details" +MOCK_HAS_PERMISSION = ( + "cms.djangoapps.contentstore.rest_api.v3.views.course_details.user_has_course_permission" +) +MOCK_CLASSIFY = "cms.djangoapps.contentstore.rest_api.v3.views.course_details._classify_update" +# CourseOverview.course_exists is called inside ``resolve_course_key()`` (now in +# v3/utils.py), not directly in the view module — so the patch must target the +# utils module's binding, not the view module's. +MOCK_COURSE_EXISTS = ( + "cms.djangoapps.contentstore.rest_api.v3.utils.CourseOverview.course_exists" +) + +# Syntactically valid course key reused across action / permission / envelope tests. +TEST_COURSE_ID = "course-v1:org+course+run" + +_REQUIRED_ERROR_FIELDS = ("type", "title", "status", "detail", "instance") + + +# =========================================================================== +# ADR 0026 — permission boundary tests +# =========================================================================== +class TestCourseDetailsViewSetPermissions(APITestCase): + """ + ADR 0026 – permission regression tests for CourseDetailsViewSet (v3). + + The v3 viewset enforces ``IsAuthenticated`` at the class level and uses + inline ``user_has_course_permission`` checks for course-level authorization + (necessary because the schedule-vs-details split depends on the payload). + """ + + def setUp(self): + super().setUp() + self.url = reverse( + "cms.djangoapps.contentstore:v3:course_details-detail", + kwargs={"course_id": TEST_COURSE_ID}, + ) + + # --- Unauthenticated --- + + def test_unauthenticated_get_returns_401(self): + """Unauthenticated GET must return 401 (IsAuthenticated).""" + response = self.client.get(self.url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_unauthenticated_put_returns_401(self): + """Unauthenticated PUT must return 401 (IsAuthenticated).""" + response = self.client.put(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + # --- Authenticated but no course access --- + + @patch.object(CourseOverview, "course_exists", return_value=True) + @patch(MOCK_HAS_PERMISSION, return_value=False) + def test_non_author_get_returns_403(self, mock_perm, mock_exists): # noqa: ARG002 + """Authenticated user without view permission must receive 403 on GET.""" + user = UserFactory.create() + self.client.force_authenticate(user=user) + response = self.client.get(self.url) + assert response.status_code == status.HTTP_403_FORBIDDEN + + @patch.object(CourseOverview, "course_exists", return_value=True) + @patch(MOCK_HAS_PERMISSION, return_value=False) + @patch(MOCK_CLASSIFY, return_value=(False, True)) # details-only update + def test_non_author_put_returns_403(self, mock_classify, mock_perm, mock_exists): # noqa: ARG002 + """Authenticated user without edit permission must receive 403 on PUT.""" + user = UserFactory.create() + self.client.force_authenticate(user=user) + response = self.client.put(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_403_FORBIDDEN + + +# =========================================================================== +# ADR 0025 / 0028 — action body tests +# =========================================================================== +class TestCourseDetailsViewSetActions(APITestCase): + """ + Action tests for CourseDetailsViewSet (retrieve and update). + + Service-layer calls are mocked, and ``user_has_course_permission`` returns + True so authorization passes through to the action body. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory.create() + self.client.force_authenticate(user=self.user) + self.url = reverse( + "cms.djangoapps.contentstore:v3:course_details-detail", + kwargs={"course_id": TEST_COURSE_ID}, + ) + + @patch.object(CourseDetailsViewSet, "serializer_class") + @patch.object(CourseOverview, "course_exists", return_value=True) + @patch(MOCK_HAS_PERMISSION, return_value=True) + @patch(MOCK_FETCH) + def test_retrieve_calls_course_details_fetch( + self, mock_fetch, mock_perm, mock_exists, mock_ser_cls, # noqa: ARG002 + ): + """GET calls CourseDetails.fetch() and returns 200.""" + mock_fetch.return_value = MagicMock() + mock_ser_cls.return_value.data = {"course_id": "run"} + + response = self.client.get(self.url) + + assert response.status_code == status.HTTP_200_OK + mock_fetch.assert_called_once() + + @patch.object(CourseDetailsViewSet, "serializer_class") + @patch.object(CourseOverview, "course_exists", return_value=True) + @patch(MOCK_HAS_PERMISSION, return_value=True) + @patch(MOCK_UPDATE) + @patch(MOCK_MODULESTORE) + @patch(MOCK_CLASSIFY, return_value=(False, True)) + def test_update_calls_update_course_details( # noqa: PLR0913 + self, + mock_classify, # noqa: ARG002 + mock_store, + mock_update, + mock_perm, # noqa: ARG002 + mock_exists, # noqa: ARG002 + mock_ser_cls, + ): + """PUT calls update_course_details() and returns 200.""" + mock_store.return_value.get_course.return_value = MagicMock() + mock_update.return_value = MagicMock() + mock_ser_cls.return_value.data = {"course_id": "run"} + + response = self.client.put(self.url, data={}, content_type="application/json") + + assert response.status_code == status.HTTP_200_OK + mock_update.assert_called_once() + + +# =========================================================================== +# ADR 0029 — standardized error-response envelope tests +# =========================================================================== +class TestCourseDetailsViewSetErrorShape(APITestCase): + """ + ADR 0029 – error response shape regression tests for CourseDetailsViewSet (v3). + + The envelope is wired in via + :class:`openedx.core.lib.api.mixins.StandardizedErrorMixin`, which overrides + DRF's per-view ``get_exception_handler`` to point at + ``openedx.core.lib.api.exceptions.standardized_error_exception_handler``. + + Scoped to v3 — the project-wide DRF ``EXCEPTION_HANDLER`` setting is + unchanged, so v0 / v1 / v2 / v4 endpoints continue to return the legacy + error shape (locked in by ``test_v1_endpoint_unaffected_by_v3_envelope``). + """ + + def setUp(self): + super().setUp() + self.client = APIClient() + self.detail_url = reverse( + "cms.djangoapps.contentstore:v3:course_details-detail", + kwargs={"course_id": TEST_COURSE_ID}, + ) + + def test_unauthenticated_get_returns_standardized_401(self): + """Unauthenticated GET must return 401 with the ADR 0029 envelope.""" + response = self.client.get(self.detail_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + for field in _REQUIRED_ERROR_FIELDS: + assert field in 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.detail_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response.data.get("type") == "https://docs.openedx.org/errors/authn" + + @patch(MOCK_COURSE_EXISTS, return_value=True) + @patch(MOCK_HAS_PERMISSION, return_value=False) + def test_non_author_get_returns_standardized_403(self, mock_perm, mock_exists): # noqa: ARG002 + """Authenticated non-author GET must return 403 with the ADR 0029 envelope.""" + non_author = UserFactory.create() + self.client.force_authenticate(user=non_author) + response = self.client.get(self.detail_url) + assert response.status_code == status.HTTP_403_FORBIDDEN + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + @patch(MOCK_COURSE_EXISTS, return_value=True) + @patch(MOCK_HAS_PERMISSION, return_value=False) + def test_non_author_403_type_uri(self, mock_perm, mock_exists): # noqa: ARG002 + """The ``type`` field for 403 must be the ADR 0029 authz URI.""" + non_author = UserFactory.create() + self.client.force_authenticate(user=non_author) + response = self.client.get(self.detail_url) + assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.data.get("type") == "https://docs.openedx.org/errors/authz" + + @patch(MOCK_COURSE_EXISTS, return_value=False) + def test_nonexistent_course_returns_standardized_404(self, mock_exists): # noqa: ARG002 + """GET for a non-existent course must return 404 with the ADR 0029 envelope.""" + staff = UserFactory.create(is_staff=True) + self.client.force_authenticate(user=staff) + response = self.client.get(self.detail_url) + assert response.status_code == status.HTTP_404_NOT_FOUND + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + @patch(MOCK_COURSE_EXISTS, return_value=False) + def test_not_found_type_uri(self, mock_exists): # noqa: ARG002 + """The ``type`` field for 404 must be the ADR 0029 not-found URI.""" + staff = UserFactory.create(is_staff=True) + self.client.force_authenticate(user=staff) + response = self.client.get(self.detail_url) + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.data.get("type") == "https://docs.openedx.org/errors/not-found" + + def test_error_body_has_no_developer_message(self): + """Error responses must NOT contain old DeveloperErrorViewMixin fields.""" + response = self.client.get(self.detail_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert "developer_message" not in response.data + assert "error_code" not in response.data + + def test_instance_field_is_request_path(self): + """The ``instance`` field must equal the request path.""" + response = self.client.get(self.detail_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response.data.get("instance") == self.detail_url + + def test_v1_endpoint_unaffected_by_v3_envelope(self): + """ + The ADR 0029 envelope must be scoped to v3 — hitting the legacy v1 + ``course_details`` endpoint unauthenticated must NOT return the v3 + envelope (no ``type`` / ``instance`` keys). + """ + v1_url = reverse( + "cms.djangoapps.contentstore:v1:course_details", + kwargs={"course_id": TEST_COURSE_ID}, + ) + response = self.client.get(v1_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert "type" not in response.data + assert "instance" not in response.data diff --git a/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_home.py new file mode 100644 index 000000000000..a688a488e63c --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/views/tests/test_home.py @@ -0,0 +1,118 @@ +""" +Unit tests for HomeViewSet — v3 (ADR 0025 / 0026 / 0028). + +MongoDB-free: all service-layer calls are mocked. + +patch.object is used for the ViewSet's get_serializer() method because: + - get_serializer_class() returns a *different* serializer per action. + - Each serializer (StudioHomeSerializer, CourseHomeTabSerializer, + LibraryTabSerializer) has many required fields that would be painful to + satisfy with synthetic data. + - Patching get_serializer() lets us focus on routing + service-call + assertions without re-testing serializer logic (covered in serializer + unit tests). +""" +from unittest.mock import patch + +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APITestCase + +from cms.djangoapps.contentstore.rest_api.v3.views.home import HomeViewSet +from common.djangoapps.student.tests.factories import UserFactory + +MOCK_GET_HOME_CONTEXT = ( + 'cms.djangoapps.contentstore.rest_api.v3.views.home.get_home_context' +) +MOCK_GET_COURSE_CONTEXT = ( + 'cms.djangoapps.contentstore.rest_api.v3.views.home.get_course_context' +) +MOCK_GET_LIBRARY_CONTEXT = ( + 'cms.djangoapps.contentstore.rest_api.v3.views.home.get_library_context' +) +MOCK_ORG_API = ( + 'cms.djangoapps.contentstore.rest_api.v3.views.home.org_api' +) + + +class TestHomeViewSetPermissions(APITestCase): + """ + ADR 0026 – permission regression tests for HomeViewSet (v3). + + Verifies that ``permission_classes = (IsAuthenticated,)`` enforces the + access rules expected of the consolidated viewset. + """ + + def test_unauthenticated_list_returns_401(self): + """Unauthenticated GET /home/ must return 401.""" + url = reverse('cms.djangoapps.contentstore:v3:home-list') + response = self.client.get(url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_unauthenticated_courses_returns_401(self): + """Unauthenticated GET /home/courses/ must return 401.""" + url = reverse('cms.djangoapps.contentstore:v3:home-courses') + response = self.client.get(url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_unauthenticated_libraries_returns_401(self): + """Unauthenticated GET /home/libraries/ must return 401.""" + url = reverse('cms.djangoapps.contentstore:v3:home-libraries') + response = self.client.get(url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + +class TestHomeViewSetActions(APITestCase): + """ + Action tests for HomeViewSet (list, courses, libraries). + + Any authenticated user can access these endpoints — no course-staff role + is required — so a plain (non-staff) factory user is sufficient. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory.create() + self.client.force_authenticate(user=self.user) + + @patch.object(HomeViewSet, 'get_serializer') + @patch(MOCK_ORG_API) + @patch(MOCK_GET_HOME_CONTEXT) + def test_list_calls_get_home_context(self, mock_home, mock_org, mock_get_ser): + """GET /home/ calls get_home_context() and returns 200.""" + mock_home.return_value = {'can_create_organizations': True} + mock_org.is_autocreate_enabled.return_value = True + mock_get_ser.return_value.data = {'studio_name': 'Studio'} + + response = self.client.get(reverse('cms.djangoapps.contentstore:v3:home-list')) + + assert response.status_code == status.HTTP_200_OK + mock_home.assert_called_once() + + @patch.object(HomeViewSet, 'get_serializer') + @patch(MOCK_GET_COURSE_CONTEXT) + def test_courses_calls_get_course_context(self, mock_courses, mock_get_ser): + """GET /home/courses/ calls get_course_context() and returns 200.""" + mock_courses.return_value = ([], [], []) + mock_get_ser.return_value.data = { + 'courses': [], + 'archived_courses': [], + 'in_process_course_actions': [], + } + + response = self.client.get(reverse('cms.djangoapps.contentstore:v3:home-courses')) + + assert response.status_code == status.HTTP_200_OK + mock_courses.assert_called_once() + + @patch.object(HomeViewSet, 'get_serializer') + @patch(MOCK_GET_LIBRARY_CONTEXT) + def test_libraries_calls_get_library_context(self, mock_libs, mock_get_ser): + """GET /home/libraries/ calls get_library_context() and returns 200.""" + mock_libs.return_value = {'libraries': []} + mock_get_ser.return_value.data = {'libraries': []} + + response = self.client.get(reverse('cms.djangoapps.contentstore:v3:home-libraries')) + + assert response.status_code == status.HTTP_200_OK + mock_libs.assert_called_once() 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..8f0fedc1cfed --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v4/views/home.py @@ -0,0 +1,217 @@ +"""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 +from openedx.core.lib.api.mixins import StandardizedErrorMixin + + +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(StandardizedErrorMixin, 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 + - 0029: standardized error envelope via ``StandardizedErrorMixin`` + - 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_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/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 4f5e1ccb4244..e558c42735d1 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -305,46 +305,7 @@ def handle_xblock(request, usage_key_string=None): elif request.method in ("PUT", "POST"): if "duplicate_source_locator" in request.json: - parent_usage_key = usage_key_with_run(request.json["parent_locator"]) - duplicate_source_usage_key = usage_key_with_run( - request.json["duplicate_source_locator"] - ) - source_course = duplicate_source_usage_key.course_key - dest_course = parent_usage_key.course_key # noqa: F841 - - # Check authz permission for destination - permission = _check_xblock_permission(request, parent_usage_key) - # Legacy path also requires read access on the source course - if permission is None: - if not has_studio_read_access(request.user, source_course): - raise PermissionDenied() - - # Libraries have a maximum component limit enforced on them - if isinstance( - parent_usage_key, LibraryUsageLocator - ) and _is_library_component_limit_reached(parent_usage_key): - return JsonResponse( - { - "error": _( - "Libraries cannot have more than {limit} components" - ).format(limit=settings.MAX_BLOCKS_PER_CONTENT_LIBRARY) - }, - status=400, - ) - - dest_usage_key = duplicate_block( - parent_usage_key, - duplicate_source_usage_key, - request.user, - display_name=request.json.get('display_name'), - ) - - return JsonResponse( - { - "locator": str(dest_usage_key), - "courseKey": str(dest_usage_key.course_key), - } - ) + return _duplicate_xblock(request) else: return _create_block(request) elif request.method == "PATCH": @@ -378,6 +339,122 @@ def handle_xblock(request, usage_key_string=None): ) +# --------------------------------------------------------------------------- +# Public per-verb helpers for XblockViewSet (v1 — ADR 0028) +# These replace handle_xblock's internal method-dispatch so each ViewSet action +# calls the correct logic directly, satisfying REST semantics. +# --------------------------------------------------------------------------- + +def _duplicate_xblock(request): + """ + Shared duplicate-block logic used by both handle_xblock and create_xblock_response. + + Expects request.json to contain ``parent_locator`` and ``duplicate_source_locator``. + """ + parent_usage_key = usage_key_with_run(request.json["parent_locator"]) + duplicate_source_usage_key = usage_key_with_run( + request.json["duplicate_source_locator"] + ) + source_course = duplicate_source_usage_key.course_key + + # Check authz permission for destination + permission = _check_xblock_permission(request, parent_usage_key) + # Legacy path also requires read access on the source course + if permission is None: + if not has_studio_read_access(request.user, source_course): + raise PermissionDenied() + + # Libraries have a maximum component limit enforced on them + if isinstance( + parent_usage_key, LibraryUsageLocator + ) and _is_library_component_limit_reached(parent_usage_key): + return JsonResponse( + { + "error": _( + "Libraries cannot have more than {limit} components" + ).format(limit=settings.MAX_BLOCKS_PER_CONTENT_LIBRARY) + }, + status=400, + ) + + dest_usage_key = duplicate_block( + parent_usage_key, + duplicate_source_usage_key, + request.user, + display_name=request.json.get('display_name'), + ) + return JsonResponse( + { + "locator": str(dest_usage_key), + "courseKey": str(dest_usage_key.course_key), + } + ) + + +def create_xblock_response(request): + """ + Public entry point for POST (create). Called by XblockViewSet.create. + + Handles both duplication (duplicate_source_locator present) and normal creation. + """ + if "duplicate_source_locator" in request.json: + return _duplicate_xblock(request) + return _create_block_core(request) + + +def retrieve_xblock_response(request, usage_key_string): + """ + Public entry point for GET on a specific xblock. Called by XblockViewSet.retrieve. + """ + usage_key = usage_key_with_run(usage_key_string) + _check_xblock_permission(request, usage_key) + + accept_header = request.META.get("HTTP_ACCEPT", "application/json") + if "application/json" not in accept_header: + return HttpResponse(status=406) + + fields = request.GET.get("fields", "").split(",") + if "graderType" in fields: + return JsonResponse(CourseGradingModel.get_section_grader_type(usage_key)) + if "ancestorInfo" in fields: + xblock = get_xblock(usage_key, request.user) + return JsonResponse(_create_xblock_ancestor_info(xblock, is_concise=True)) + with modulestore().bulk_operations(usage_key.course_key): + response = get_block_info(get_xblock(usage_key, request.user)) + if "customReadToken" in fields: + parent_children = _get_block_parent_children(get_xblock(usage_key, request.user)) + response.update(parent_children) + return JsonResponse(response) + + +def update_xblock_response(request, usage_key_string): + """ + Public entry point for PUT/PATCH on a specific xblock. Called by XblockViewSet.update/partial_update. + + For PATCH with ``move_source_locator``, routes to the move-item path (permission check + is against the target parent, not the URL's usage key). + """ + if request.method == "PATCH" and "move_source_locator" in request.json: + move_source_usage_key = usage_key_with_run(request.json.get("move_source_locator")) + target_parent_usage_key = usage_key_with_run(request.json.get("parent_locator")) + target_index = request.json.get("target_index") + _check_xblock_permission(request, target_parent_usage_key) + return _move_item(move_source_usage_key, target_parent_usage_key, request.user, target_index) + usage_key = usage_key_with_run(usage_key_string) + _check_xblock_permission(request, usage_key) + return modify_xblock(usage_key, request) + + +def delete_xblock_response(request, usage_key_string): + """ + Public entry point for DELETE on a specific xblock. Called by XblockViewSet.destroy. + """ + usage_key = usage_key_with_run(usage_key_string) + _check_xblock_permission(request, usage_key) + _delete_item(usage_key, request.user) + return JsonResponse() + + def modify_xblock(usage_key, request): request_data = request.json return _save_xblock( @@ -746,10 +823,12 @@ def sync_library_content( return static_file_notices -@login_required -@expect_json -def _create_block(request): - """View for create blocks.""" +def _create_block_core(request): + """ + Core xblock creation logic, usable without @login_required / @expect_json decorators. + + Called by both _create_block (legacy view) and create_xblock_response (v1 ViewSet). + """ parent_locator = request.json["parent_locator"] usage_key = usage_key_with_run(parent_locator) category = request.json.get("category") @@ -836,6 +915,13 @@ def _create_block(request): return JsonResponse(response) +@login_required +@expect_json +def _create_block(request): + """View for create blocks.""" + return _create_block_core(request) + + def _get_source_index(source_usage_key, source_parent): """ Get source index position of the XBlock. diff --git a/lms/urls.py b/lms/urls.py index caa95569f3e0..280c739c9bce 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -117,6 +117,7 @@ # Enrollment API RESTful endpoints path('api/enrollment/v1/', include('openedx.core.djangoapps.enrollments.urls')), + path('api/enrollment/v2/', include('openedx.core.djangoapps.enrollments.v2.urls')), # Agreements API RESTful endpoints path('api/agreements/v1/', include('openedx.core.djangoapps.agreements.urls')), diff --git a/openedx/core/djangoapps/enrollments/v2/__init__.py b/openedx/core/djangoapps/enrollments/v2/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/djangoapps/enrollments/v2/forms.py b/openedx/core/djangoapps/enrollments/v2/forms.py new file mode 100644 index 000000000000..413bb870bb87 --- /dev/null +++ b/openedx/core/djangoapps/enrollments/v2/forms.py @@ -0,0 +1,128 @@ +""" +Forms for validating user input to the Course Enrollment v2 views. + +ADR 0033 (OEP-68 parameter naming standardization) — accepts both the +preferred parameter names (``course_key``, ``course_keys``) and the legacy +aliases (``course_id``, ``course_ids``). When both are present, the +preferred name wins. Use :meth:`legacy_param_aliases_used` from the view +layer to emit the ADR 0033 ``Deprecation`` HTTP header when a legacy alias +was sent. + +Internally the cleaned_data continues to expose ``course_id`` / +``course_ids`` (the names the queryset code reads) — the form coalesces +the preferred values onto those fields before the rest of validation runs. +""" + +from django.core.exceptions import ValidationError +from django.forms import CharField, Form +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey + +from openedx.core.djangoapps.user_authn.views.registration_form import validate_username + + +class EnrollmentsAdminListForm(Form): + """ + Validates the query string parameters for the v2 admin enrollments list + endpoint (``GET /api/enrollment/v2/enrollments/``). + """ + + MAX_INPUT_COUNT = 100 + # Legacy / OEP-68 alias pairs: (legacy, preferred). + _LEGACY_PARAM_ALIASES = ( + ("course_id", "course_key"), + ("course_ids", "course_keys"), + ) + + username = CharField(required=False) + course_id = CharField(required=False) + course_key = CharField(required=False) + course_ids = CharField(required=False) + course_keys = CharField(required=False) + email = CharField(required=False) + + def __init__(self, query_params, *args, **kwargs): + # Capture the raw param names supplied on the wire (before Django's + # form layer resolves aliases) so :meth:`legacy_param_aliases_used` + # can later report exactly which legacy names were used. + try: + raw_keys = set(query_params.keys()) + except AttributeError: + raw_keys = set() + self._raw_param_names = raw_keys + + # Coalesce OEP-68 preferred names onto the legacy field names so the + # downstream queryset code keeps reading ``course_id`` / ``course_ids`` + # without changes. The preferred name wins when both are sent. + if hasattr(query_params, "copy"): + data = query_params.copy() + else: + data = dict(query_params) + for legacy_name, preferred_name in self._LEGACY_PARAM_ALIASES: + preferred_value = data.get(preferred_name) + if preferred_value: + data[legacy_name] = preferred_value + + super().__init__(data, *args, **kwargs) + + def legacy_param_aliases_used(self): + """ + Return the list of legacy parameter names that were actually present + in the request, in declaration order. The view layer uses this to + emit the ADR 0033 ``Deprecation`` header. + """ + return [ + legacy for legacy, _preferred in self._LEGACY_PARAM_ALIASES + if legacy in self._raw_param_names + ] + + def clean_course_id(self): + """Parse and validate the ``course_id`` (or aliased ``course_key``) parameter.""" + course_id = self.cleaned_data.get("course_id") + if course_id: + try: + return CourseKey.from_string(course_id) + except InvalidKeyError as exc: + raise ValidationError(f"'{course_id}' is not a valid course id.") from exc + return course_id + + def clean_course_ids(self): + """Split the ``course_ids`` CSV (or aliased ``course_keys``) and enforce MAX_INPUT_COUNT.""" + course_ids_csv = self.cleaned_data.get("course_ids") + if course_ids_csv: + course_ids = course_ids_csv.split(",") + if len(course_ids) > self.MAX_INPUT_COUNT: + raise ValidationError( + f"Too many course_ids in a single request - {len(course_ids)}. " + f"A maximum of {self.MAX_INPUT_COUNT} is allowed" + ) + return course_ids + return course_ids_csv + + def clean_username(self): + """Split the ``username`` CSV, validate each entry, and enforce MAX_INPUT_COUNT.""" + usernames_csv = self.cleaned_data.get("username") + if usernames_csv: + usernames = usernames_csv.split(",") + if len(usernames) > self.MAX_INPUT_COUNT: + raise ValidationError( + f"Too many usernames in a single request - {len(usernames)}. " + f"A maximum of {self.MAX_INPUT_COUNT} is allowed" + ) + for username in usernames: + validate_username(username) + return usernames + return usernames_csv + + def clean_email(self): + """Split the ``email`` CSV and enforce MAX_INPUT_COUNT.""" + emails_csv = self.cleaned_data.get("email") + if emails_csv: + emails = emails_csv.split(",") + if len(emails) > self.MAX_INPUT_COUNT: + raise ValidationError( + f"Too many emails in a single request - {len(emails)}. " + f"A maximum of {self.MAX_INPUT_COUNT} is allowed" + ) + return emails + return emails_csv diff --git a/openedx/core/djangoapps/enrollments/v2/paginators.py b/openedx/core/djangoapps/enrollments/v2/paginators.py new file mode 100644 index 000000000000..d8d9eed96c77 --- /dev/null +++ b/openedx/core/djangoapps/enrollments/v2/paginators.py @@ -0,0 +1,29 @@ +""" +Pagination for the Enrollment API — v2. + +ADR 0032 — uses :class:`DefaultPagination` from +``edx-rest-framework-extensions``, which provides the standard 7-field +envelope: ``count``, ``num_pages``, ``current_page``, ``start``, ``next``, +``previous``, ``results``. + +Distinct from v1's :class:`openedx.core.djangoapps.enrollments.paginators.CourseEnrollmentsApiListPagination` +(which is a :class:`CursorPagination` subclass with a 3-field envelope). +v2 introduces the new shape — clients that need the legacy shape stay on +``/api/enrollment/v1/`` until they migrate. +""" + +from edx_rest_framework_extensions.paginators import DefaultPagination + + +class EnrollmentsAdminListPagination(DefaultPagination): + """ + ADR 0032 — standard pagination for the admin enrollments list API + (GET /api/enrollment/v2/enrollments/). + + Defaults sized for an admin-facing bulk-query endpoint: + page_size 100, max 100. + """ + + page_size = 100 + page_size_query_param = "page_size" + max_page_size = 100 diff --git a/openedx/core/djangoapps/enrollments/v2/serializers.py b/openedx/core/djangoapps/enrollments/v2/serializers.py new file mode 100644 index 000000000000..2c7220b398f1 --- /dev/null +++ b/openedx/core/djangoapps/enrollments/v2/serializers.py @@ -0,0 +1,34 @@ +""" +Serializers for the Enrollment API — v2. + +Only contains the serializers introduced by ADR 0025 (replacing inline +dict construction in role-listing endpoints). The other v1 serializers +(:class:`CourseEnrollmentSerializer`, :class:`CourseSerializer`, +:class:`CourseEnrollmentAllowedSerializer`, :class:`CourseEnrollmentsApiListSerializer`) +are unchanged in shape between v1 and v2 — v2 view code imports them +directly from :mod:`openedx.core.djangoapps.enrollments.serializers`. + +If a future v3 needs to break any of those response shapes, fork them +into a new v3/serializers.py at that time. +""" + +from rest_framework import serializers + + +class UserRoleSerializer(serializers.Serializer): # pylint: disable=abstract-method + """Serializes a single course-level role entry for a user (ADR 0025).""" + + org = serializers.CharField() + course_id = serializers.SerializerMethodField() + role = serializers.CharField() + + def get_course_id(self, obj): + """Return course_id as a string.""" + return str(obj.course_id) + + +class UserRolesResponseSerializer(serializers.Serializer): # pylint: disable=abstract-method + """Serializes the full response payload for UserRolesViewSet (ADR 0025).""" + + roles = UserRoleSerializer(many=True) + is_staff = serializers.BooleanField() diff --git a/openedx/core/djangoapps/enrollments/v2/tests/__init__.py b/openedx/core/djangoapps/enrollments/v2/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/djangoapps/enrollments/v2/tests/test_envelope.py b/openedx/core/djangoapps/enrollments/v2/tests/test_envelope.py new file mode 100644 index 000000000000..3c87e8c8008d --- /dev/null +++ b/openedx/core/djangoapps/enrollments/v2/tests/test_envelope.py @@ -0,0 +1,129 @@ +""" +ADR 0029 — Standardized error-response envelope regression tests for the +v2 Enrollment API. + +The envelope is wired into every v2 viewset via +:class:`openedx.core.lib.api.mixins.StandardizedErrorMixin`, which overrides +DRF's per-view ``get_exception_handler`` to point at the project-wide +``standardized_error_exception_handler``. + +The envelope shape is:: + + { + "type": "https://docs.openedx.org/errors/", + "title": "", + "status": , + "detail": "", + "instance": "", + } + +These tests confirm the envelope reaches every v2 endpoint that can produce +a 401 / 403 / 404 / 400. The last test (``test_v1_endpoint_unaffected``) +locks in the scoping — v1 must NOT carry the envelope. +""" +from unittest.mock import patch + +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient, APITestCase + +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms + +_REQUIRED_ERROR_FIELDS = ("type", "title", "status", "detail", "instance") + + +@skip_unless_lms +class TestEnrollmentViewSetEnvelope(APITestCase): + """ADR 0029 — envelope on the EnrollmentViewSet 401s.""" + + def setUp(self): + super().setUp() + self.client = APIClient() + self.list_url = reverse("v2:enrollment-list") + self.unenroll_url = reverse("v2:enrollment-unenroll") + self.allowed_url = reverse("v2:enrollment-allowed") + + def test_list_unauthenticated_envelope(self): + response = self.client.get(self.list_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + def test_list_unauthenticated_type_uri(self): + response = self.client.get(self.list_url) + assert response.data.get("type") == "https://docs.openedx.org/errors/authn" + + def test_unenroll_unauthenticated_envelope(self): + response = self.client.post(self.unenroll_url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + def test_allowed_unauthenticated_envelope(self): + response = self.client.get(self.allowed_url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + + def test_non_admin_get_allowed_envelope(self): + """ADR 0029 — 403 also carries the envelope.""" + self.client.force_authenticate(user=UserFactory.create()) + response = self.client.get(self.allowed_url) + assert response.status_code == status.HTTP_403_FORBIDDEN + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + assert response.data.get("type") == "https://docs.openedx.org/errors/authz" + + def test_create_missing_course_id_envelope(self): + """ADR 0029 — inline ValidationError surfaces with the envelope.""" + self.client.force_authenticate(user=UserFactory.create()) + response = self.client.post(self.list_url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + assert response.data.get("type") == "https://docs.openedx.org/errors/validation" + + def test_instance_field_is_request_path(self): + response = self.client.get(self.list_url) + assert response.data.get("instance") == self.list_url + + def test_error_body_has_no_developer_message(self): + """Legacy DeveloperErrorViewMixin fields must not leak through.""" + response = self.client.get(self.list_url) + assert "developer_message" not in response.data + assert "error_code" not in response.data + + +@skip_unless_lms +class TestCourseEnrollmentDetailViewEnvelope(APITestCase): + """ADR 0029 — envelope on the public course-detail endpoint.""" + + def test_invalid_course_key_envelope(self): + url = reverse("v2:enrollment-v2-course-detail", kwargs={"course_id": "course-v1:org+course+run"}) + with patch( + "openedx.core.djangoapps.enrollments.v2.views.CourseOverview.get_from_id", + ) as mock_get: + from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + mock_get.side_effect = CourseOverview.DoesNotExist() + response = self.client.get(url) + assert response.status_code == status.HTTP_404_NOT_FOUND + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing field '{field}'" + assert response.data.get("type") == "https://docs.openedx.org/errors/not-found" + + +@skip_unless_lms +class TestV1EndpointUnaffected(APITestCase): + """ + The ADR 0029 envelope must be scoped to v2 — v1 endpoints continue to + use whichever handler the project-wide ``EXCEPTION_HANDLER`` setting + points at. Hitting v1 unauthenticated must NOT return the v2 envelope. + """ + + def test_v1_enrollment_list_does_not_carry_envelope(self): + response = self.client.get(reverse("courseenrollments")) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + # v1 still uses the project-default handler; ADR 0029 fields absent. + assert "type" not in response.data + assert "instance" not in response.data diff --git a/openedx/core/djangoapps/enrollments/v2/tests/test_view_services.py b/openedx/core/djangoapps/enrollments/v2/tests/test_view_services.py new file mode 100644 index 000000000000..0bb8fd1ecbfe --- /dev/null +++ b/openedx/core/djangoapps/enrollments/v2/tests/test_view_services.py @@ -0,0 +1,87 @@ +""" +Unit tests for EnrollmentOperationsService (v2). + +Exercises the service methods directly — no HTTP layer involved. Tests the +two-layer authorization model (ADR 0031) and the modern ADR 0029 raise-DRF- +exceptions pattern. +""" +from unittest.mock import MagicMock, patch + +import pytest +from django.test import TestCase +from rest_framework.exceptions import NotFound, ValidationError + +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.enrollments.v2.view_services import EnrollmentOperationsService + + +class TestUnenrollUserForRetirement(TestCase): + """ADR 0029 — error handling for the retirement unenroll flow.""" + + def setUp(self): + super().setUp() + self.service = EnrollmentOperationsService() + + def test_missing_username_raises_validation_error(self): + with pytest.raises(ValidationError): + self.service.unenroll_user_for_retirement(None) + + def test_blank_username_raises_validation_error(self): + with pytest.raises(ValidationError): + self.service.unenroll_user_for_retirement("") + + @patch( + "openedx.core.djangoapps.enrollments.v2.view_services.UserRetirementStatus.get_retirement_for_retirement_action" + ) + def test_unknown_retirement_status_raises_not_found(self, mock_get): + from openedx.core.djangoapps.user_api.models import UserRetirementStatus + mock_get.side_effect = UserRetirementStatus.DoesNotExist() + with pytest.raises(NotFound): + self.service.unenroll_user_for_retirement("ghost-user") + + +class TestListEnrollmentsForUser(TestCase): + """ADR 0031 — per-operation permission filter in the listing helper.""" + + def setUp(self): + super().setUp() + self.service = EnrollmentOperationsService() + self.user = UserFactory.create() + self.other = UserFactory.create() + + @patch("openedx.core.djangoapps.enrollments.v2.view_services.CourseEnrollment.objects") + def test_self_lookup_returns_full_list_unfiltered(self, mock_objects): + """Requesting your own enrollments bypasses the course-staff filter.""" + mock_qs = MagicMock() + mock_qs.__iter__ = lambda self: iter([]) + mock_objects.filter.return_value.select_related.return_value = mock_qs + result = self.service.list_enrollments_for_user( + request_user=self.user, target_username=self.user.username, has_api_key=False, + ) + assert isinstance(result, list) + + @patch("openedx.core.djangoapps.enrollments.v2.view_services.CourseEnrollment.objects") + def test_api_key_bypasses_per_course_filter(self, mock_objects): + """has_api_key=True returns the full list even across user boundaries.""" + mock_qs = MagicMock() + mock_qs.__iter__ = lambda self: iter([]) + mock_objects.filter.return_value.select_related.return_value = mock_qs + result = self.service.list_enrollments_for_user( + request_user=self.user, target_username=self.other.username, has_api_key=True, + ) + assert isinstance(result, list) + + +class TestDeleteAllowedEnrollment(TestCase): + """ADR 0029 — delete raises NotFound when the row is missing.""" + + def setUp(self): + super().setUp() + self.service = EnrollmentOperationsService() + + @patch("openedx.core.djangoapps.enrollments.v2.view_services.CourseEnrollmentAllowed.objects") + def test_delete_missing_row_raises_not_found(self, mock_objects): + from django.core.exceptions import ObjectDoesNotExist + mock_objects.get.side_effect = ObjectDoesNotExist() + with pytest.raises(NotFound): + self.service.delete_allowed_enrollment("ghost@example.com", "course-v1:org+course+run") diff --git a/openedx/core/djangoapps/enrollments/v2/tests/test_views.py b/openedx/core/djangoapps/enrollments/v2/tests/test_views.py new file mode 100644 index 000000000000..ce8f958a1d79 --- /dev/null +++ b/openedx/core/djangoapps/enrollments/v2/tests/test_views.py @@ -0,0 +1,236 @@ +""" +Action + permission regression tests for the v2 Enrollment ViewSet. + +MongoDB-free: every service-layer call is mocked, so these tests run +without a live modulestore or course-overview row. + +Covers: + - ADR 0026: permission enforcement on every action (list/create/unenroll/allowed) + - ADR 0028: router-generated URL reverse names work + - ADR 0032: list action returns the 7-field DefaultPagination envelope + - ADR 0033: ordering whitelist + Deprecation header on the admin list +""" +from unittest.mock import patch + +from django.test import override_settings +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APITestCase + +from common.djangoapps.student.tests.factories import AdminFactory, UserFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms + +API_KEY = "test-enrollment-v2-api-key" + +# Mock targets — all keyed off the v2 module to avoid leaking into v1. +MOCK_OPS_LIST = "openedx.core.djangoapps.enrollments.v2.views._OPS.list_enrollments_for_user" +MOCK_OPS_CREATE = "openedx.core.djangoapps.enrollments.v2.views._OPS.create_or_update_enrollment" +MOCK_OPS_UNENROLL = "openedx.core.djangoapps.enrollments.v2.views._OPS.unenroll_user_for_retirement" +MOCK_OPS_LIST_ALLOWED = "openedx.core.djangoapps.enrollments.v2.views._OPS.list_allowed_for_email" +MOCK_OPS_CREATE_ALLOWED = "openedx.core.djangoapps.enrollments.v2.views._OPS.create_allowed_enrollment" +MOCK_OPS_DELETE_ALLOWED = "openedx.core.djangoapps.enrollments.v2.views._OPS.delete_allowed_enrollment" + + +# --------------------------------------------------------------------------- +# EnrollmentViewSet.list (GET /enrollment/) +# --------------------------------------------------------------------------- + +@skip_unless_lms +class TestEnrollmentViewSetList(APITestCase): + """ADR 0026 + 0028 — permission + reverse-name tests for the list action.""" + + def setUp(self): + super().setUp() + self.user = UserFactory.create(password="test") + self.url = reverse("v2:enrollment-list") + + def test_unauthenticated_gets_401(self): + response = self.client.get(self.url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + @patch(MOCK_OPS_LIST, return_value=[]) + def test_authenticated_user_gets_200(self, mock_list): # noqa: ARG002 + self.client.force_authenticate(user=self.user) + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + + @patch(MOCK_OPS_LIST, return_value=[]) + def test_valid_api_key_gets_200(self, mock_list): # noqa: ARG002 + with override_settings(EDX_API_KEY=API_KEY): + response = self.client.get(self.url, HTTP_X_EDX_API_KEY=API_KEY) + assert response.status_code == status.HTTP_200_OK + + def test_invalid_api_key_without_session_gets_401(self): + response = self.client.get(self.url, HTTP_X_EDX_API_KEY="wrong-key") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + @patch(MOCK_OPS_LIST, return_value=[]) + def test_list_returns_pagination_envelope(self, mock_list): # noqa: ARG002 + """ADR 0032 — every response carries the 7-field DefaultPagination envelope.""" + self.client.force_authenticate(user=self.user) + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + for field in ("count", "num_pages", "current_page", "start", "next", "previous", "results"): + assert field in response.data, f"ADR 0032: missing envelope field '{field}'" + + +# --------------------------------------------------------------------------- +# EnrollmentViewSet.create (POST /enrollment/) +# --------------------------------------------------------------------------- + +@skip_unless_lms +class TestEnrollmentViewSetCreate(APITestCase): + """ADR 0026 + 0028 — permission tests for the create action.""" + + def setUp(self): + super().setUp() + self.user = UserFactory.create(password="test") + self.url = reverse("v2:enrollment-list") + + def test_unauthenticated_post_gets_401(self): + response = self.client.post(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_authenticated_post_missing_course_id_gets_400(self): + """ADR 0029 — missing course_id raises ValidationError → 400.""" + self.client.force_authenticate(user=self.user) + response = self.client.post(self.url, data={}, content_type="application/json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_authenticated_post_invalid_course_id_gets_400(self): + """ADR 0029 — unparseable course_id raises ValidationError → 400.""" + self.client.force_authenticate(user=self.user) + response = self.client.post( + self.url, + data={"course_details": {"course_id": "not-a-course-key"}}, + content_type="application/json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @patch(MOCK_OPS_CREATE, return_value={"mode": "audit", "is_active": True}) + def test_authenticated_post_valid_returns_200(self, mock_create): # noqa: ARG002 + self.client.force_authenticate(user=self.user) + response = self.client.post( + self.url, + data={"course_details": {"course_id": "course-v1:org+course+run"}}, + content_type="application/json", + ) + assert response.status_code == status.HTTP_200_OK + + +# --------------------------------------------------------------------------- +# EnrollmentViewSet.unenroll (POST /enrollment/unenroll/) +# --------------------------------------------------------------------------- + +@skip_unless_lms +class TestEnrollmentViewSetUnenroll(APITestCase): + """ADR 0026 — IsAuthenticated + CanRetireUser permission.""" + + def setUp(self): + super().setUp() + self.user = UserFactory.create(password="test") + self.url = reverse("v2:enrollment-unenroll") + + def test_unauthenticated_gets_401(self): + response = self.client.post( + self.url, data={"username": self.user.username}, content_type="application/json", + ) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_authenticated_non_retirement_user_gets_403(self): + """A plain authenticated user lacks CanRetireUser → 403.""" + self.client.force_authenticate(user=self.user) + response = self.client.post( + self.url, data={"username": self.user.username}, content_type="application/json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +# --------------------------------------------------------------------------- +# EnrollmentViewSet.allowed (GET/POST/DELETE /enrollment/enrollment_allowed/) +# --------------------------------------------------------------------------- + +@skip_unless_lms +class TestEnrollmentViewSetAllowed(APITestCase): + """ADR 0026 — IsAdminUser permission on the allowed action.""" + + def setUp(self): + super().setUp() + self.user = UserFactory.create(password="test") + self.admin = AdminFactory.create(password="test") + self.url = reverse("v2:enrollment-allowed") + + def test_unauthenticated_get_gets_401(self): + response = self.client.get(self.url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_non_admin_get_gets_403(self): + self.client.force_authenticate(user=self.user) + response = self.client.get(self.url) + assert response.status_code == status.HTTP_403_FORBIDDEN + + @patch(MOCK_OPS_LIST_ALLOWED, return_value=[]) + def test_admin_get_gets_200(self, mock_list): # noqa: ARG002 + self.client.force_authenticate(user=self.admin) + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + assert response.data == [] + + def test_non_admin_post_gets_403(self): + self.client.force_authenticate(user=self.user) + response = self.client.post( + self.url, + data={"email": "test@example.com", "course_id": "course-v1:edX+DemoX+Demo_Course"}, + content_type="application/json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_non_admin_delete_gets_403(self): + self.client.force_authenticate(user=self.user) + response = self.client.delete( + self.url, + data={"email": "test@example.com", "course_id": "course-v1:edX+DemoX+Demo_Course"}, + content_type="application/json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +# --------------------------------------------------------------------------- +# UserRolesView (GET /roles/) — ADR 0033 OEP-68 aliasing +# --------------------------------------------------------------------------- + +_ADR_0033_HEADER_COURSE_ID = ( + "Parameter 'course_id' is deprecated. Use 'course_key' instead. " + "Support will be removed in release ''." +) + + +@skip_unless_lms +class TestUserRolesViewAliases(APITestCase): + """ADR 0033 — OEP-68 parameter alias + Deprecation header tests.""" + + def setUp(self): + super().setUp() + self.user = UserFactory.create(password="test") + self.url = reverse("v2:enrollment-v2-roles") + + @patch("openedx.core.djangoapps.enrollments.v2.views.api.get_user_roles", return_value=[]) + def test_new_course_key_param_no_header(self, mock_get): # noqa: ARG002 + self.client.force_authenticate(user=self.user) + response = self.client.get(self.url, {"course_key": "course-v1:org+course+run"}) + assert response.status_code == status.HTTP_200_OK + assert "Deprecation" not in response.headers + + @patch("openedx.core.djangoapps.enrollments.v2.views.api.get_user_roles", return_value=[]) + def test_legacy_course_id_param_emits_header(self, mock_get): # noqa: ARG002 + self.client.force_authenticate(user=self.user) + response = self.client.get(self.url, {"course_id": "course-v1:org+course+run"}) + assert response.status_code == status.HTTP_200_OK + assert response.headers.get("Deprecation") == _ADR_0033_HEADER_COURSE_ID + + @patch("openedx.core.djangoapps.enrollments.v2.views.api.get_user_roles", return_value=[]) + def test_no_filter_no_header(self, mock_get): # noqa: ARG002 + self.client.force_authenticate(user=self.user) + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + assert "Deprecation" not in response.headers diff --git a/openedx/core/djangoapps/enrollments/v2/urls.py b/openedx/core/djangoapps/enrollments/v2/urls.py new file mode 100644 index 000000000000..cda839fd4319 --- /dev/null +++ b/openedx/core/djangoapps/enrollments/v2/urls.py @@ -0,0 +1,73 @@ +""" +URLs for the Enrollment API — v2. + +Mounted at ``/api/enrollment/v2/`` (see ``lms/urls.py``). + +ADR 0028 — :class:`EnrollmentViewSet` is registered via ``DefaultRouter`` +(actions: ``list``, ``create``, ``unenroll``, ``allowed``). The other v2 +endpoints (singleton retrieve by URL form, roles, course-detail-by-id, +admin enrollments list) cannot be expressed as router-generated URLs, so +they remain as standalone ``APIView`` classes routed via ``path()`` / +``re_path()``. + +URL surface +----------- + +Router-generated (basename ``enrollment``): + GET /enrollment/ + POST /enrollment/ + POST /enrollment/unenroll/ + GET /enrollment/enrollment_allowed/ + POST /enrollment/enrollment_allowed/ + DELETE /enrollment/enrollment_allowed/ + +Explicit paths: + GET /enrollment/{username},{course_key} (name: enrollment-v2-retrieve) + GET /enrollment/{course_key} (name: enrollment-v2-retrieve) + GET /enrollments/ (name: enrollment-v2-admin-list) + GET /course/{course_key} (name: enrollment-v2-course-detail) + GET /roles/ (name: enrollment-v2-roles) +""" + +from django.conf import settings +from django.urls import path, re_path +from rest_framework.routers import DefaultRouter + +from .views import ( + CourseEnrollmentDetailView, + EnrollmentRetrieveView, + EnrollmentsAdminListView, + EnrollmentViewSet, + UserRolesView, +) + +app_name = "v2" + +router = DefaultRouter() +router.register(r"enrollment", EnrollmentViewSet, basename="enrollment") + +urlpatterns = router.urls + [ + re_path( + r"^enrollment/{username},{course_key}$".format( # noqa: UP032 + username=settings.USERNAME_PATTERN, course_key=settings.COURSE_ID_PATTERN, + ), + EnrollmentRetrieveView.as_view(), + name="enrollment-v2-retrieve", + ), + re_path( + rf"^enrollment/{settings.COURSE_ID_PATTERN}$", + EnrollmentRetrieveView.as_view(), + name="enrollment-v2-retrieve", + ), + re_path( + r"^enrollments/?$", + EnrollmentsAdminListView.as_view(), + name="enrollment-v2-admin-list", + ), + re_path( + rf"^course/{settings.COURSE_ID_PATTERN}$", + CourseEnrollmentDetailView.as_view(), + name="enrollment-v2-course-detail", + ), + path("roles/", UserRolesView.as_view(), name="enrollment-v2-roles"), +] diff --git a/openedx/core/djangoapps/enrollments/v2/view_services.py b/openedx/core/djangoapps/enrollments/v2/view_services.py new file mode 100644 index 000000000000..75c7de0dcec3 --- /dev/null +++ b/openedx/core/djangoapps/enrollments/v2/view_services.py @@ -0,0 +1,376 @@ +""" +Shared service layer for enrollment v2 HTTP operations. + +ADR 0031 (Merge Similar Endpoints) — consolidates the business logic +behind the three v2 viewset actions that previously had partially duplicated +implementations in v1's ``EnrollmentListView`` / ``UnenrollmentView`` / +``EnrollmentAllowedView``. + +Authorization model +------------------- +Each operation is enforced in two layers: + +1. The viewset declares a coarse permission class (``IsAuthenticated``, + ``IsAdminUser``, ``CanRetireUser``, ``ApiKeyHeaderPermissionIsAuthenticated``) + on the action. +2. The service method enforces the per-operation rules — e.g. only API-key + callers or global staff may deactivate enrollments, downgrade modes, or + force-enroll a user. + +ADR 0029 — service methods raise DRF exceptions (``NotFound``, +``ValidationError``, ``PermissionDenied``, ``Conflict``) instead of returning +``Response`` objects with non-2xx status. The exceptions flow through the +viewset's :class:`StandardizedErrorMixin` to produce the standardized +envelope. +""" + +import logging + +from django.contrib.auth import get_user_model +from django.core.exceptions import ObjectDoesNotExist +from rest_framework.exceptions import ( + APIException, + NotFound, + PermissionDenied, + ValidationError, +) + +from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.auth import user_has_role +from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed, EnrollmentNotAllowed +from common.djangoapps.student.roles import CourseStaffRole, GlobalStaff +from openedx.core.djangoapps.course_groups.cohorts import CourseUserGroup, add_user_to_cohort, get_cohort_by_name +from openedx.core.djangoapps.embargo import api as embargo_api +from openedx.core.djangoapps.enrollments import api +from openedx.core.djangoapps.enrollments.errors import ( + CourseEnrollmentError, + CourseEnrollmentExistsError, + CourseModeNotFoundError, + InvalidEnrollmentAttribute, +) +from openedx.core.djangoapps.user_api.models import UserRetirementStatus +from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in +from openedx.core.lib.api.exceptions import Conflict +from openedx.core.lib.exceptions import CourseNotFoundError +from openedx.core.lib.log_utils import audit_log +from openedx.features.enterprise_support.api import ( + ConsentApiServiceClient, + EnterpriseApiException, + EnterpriseApiServiceClient, + enterprise_enabled, +) + +log = logging.getLogger(__name__) + +User = get_user_model() + +REQUIRED_ATTRIBUTES = { + "credit": ["credit:provider_id"], +} + + +class EnrollmentOperationsService: + """ + Operation handlers for the v2 EnrollmentViewSet. + + All methods raise DRF exceptions on error paths so the viewset's + :class:`StandardizedErrorMixin` can produce the ADR 0029 envelope. + """ + + # ------------------------------------------------------------------ + # Listing + # ------------------------------------------------------------------ + def list_enrollments_for_user(self, request_user, target_username, has_api_key): + """ + Return enrollments visible to ``request_user`` for ``target_username``. + + - Self / global staff / api-key requests → full list. + - Otherwise filtered to courses ``request_user`` staffs. + """ + enrollments = CourseEnrollment.objects.filter( + user__username=target_username + ).select_related("user", "course") + if ( + target_username == request_user.username + or GlobalStaff().has_user(request_user) + or has_api_key + ): + return list(enrollments) + return [ + enrollment for enrollment in enrollments + if user_has_role(request_user, CourseStaffRole(enrollment.course_id)) + ] + + # ------------------------------------------------------------------ + # Create / update + # ------------------------------------------------------------------ + def create_or_update_enrollment(self, request, has_api_key, course_id): + """ + Handle the POST /enrollment/ create-or-update flow. + + ``course_id`` is a parsed :class:`CourseKey`. The viewset is + responsible for the up-front ``InvalidKeyError → ValidationError`` + translation before calling this method. + + Returns the enrollment dict on success. Raises DRF exceptions on + any error path. + """ + # pylint: disable=too-many-statements,too-many-branches + username = request.data.get("user") + mode = request.data.get("mode") + is_active = None + user = None + cohort_name = None + + # Per-operation authz layer 1: only admin/api-key callers may enroll + # other users. Non-staff callers can only enroll themselves. + if ( + username + and username != request.user.username + and not has_api_key + and not GlobalStaff().has_user(request.user) + ): + raise NotFound() + + if not username: + email = request.data.get("email") + if email: + if not has_api_key and not GlobalStaff().has_user(request.user): + raise NotFound() + try: + username = User.objects.get(email=email).username + except ObjectDoesNotExist as exc: + raise NotFound( + f"The user with the email address {email} does not exist." + ) from exc + else: + username = request.user.username + + # Per-operation authz layer 2: non-default modes require api-key or + # global-staff privileges. + if ( + mode not in (CourseMode.AUDIT, CourseMode.HONOR, None) + and not has_api_key + and not GlobalStaff().has_user(request.user) + ): + raise PermissionDenied( + f"User does not have permission to create enrollment with mode [{mode}]." + ) + + try: + user = User.objects.get(username=username) + except ObjectDoesNotExist as exc: + raise NotFound(f"The user {username} does not exist.") from exc + + embargo_response = embargo_api.get_embargo_response(request, course_id, user) + if embargo_response: + # Embargo returns a fully-formed Response; surface its body as a + # PermissionDenied so the standardized envelope wraps it. + raise PermissionDenied(detail=getattr(embargo_response, "data", "Embargoed.")) + + try: + is_active = request.data.get("is_active") + if is_active is not None and not isinstance(is_active, bool): + raise ValidationError(f"'{is_active}' is an invalid enrollment activation status.") + + explicit_linked_enterprise = request.data.get("linked_enterprise_customer") + if explicit_linked_enterprise and has_api_key and enterprise_enabled(): + enterprise_api_client = EnterpriseApiServiceClient() + consent_client = ConsentApiServiceClient() + try: + enterprise_api_client.post_enterprise_course_enrollment(username, str(course_id)) + except EnterpriseApiException as error: + log.exception( + "An unexpected error occurred while creating the new EnterpriseCourseEnrollment " + "for user [%s] in course run [%s]", username, course_id, + ) + raise CourseEnrollmentError(str(error)) from error + consent_client.provide_consent( + username=username, + course_id=str(course_id), + enterprise_customer_uuid=explicit_linked_enterprise, + ) + + enrollment_attributes = request.data.get("enrollment_attributes") + force_enrollment = request.data.get("force_enrollment") + if force_enrollment is not None and not isinstance(force_enrollment, bool): + raise ValidationError(f"'{force_enrollment}' is an invalid force enrollment status.") + force_enrollment = force_enrollment and GlobalStaff().has_user(request.user) + + enrollment = api.get_enrollment(username, str(course_id)) + mode_changed = enrollment and mode is not None and enrollment["mode"] != mode + active_changed = enrollment and is_active is not None and enrollment["is_active"] != is_active + missing_attrs = [] + if enrollment_attributes: + actual_attrs = ["{namespace}:{name}".format(**attr) for attr in enrollment_attributes] + missing_attrs = set(REQUIRED_ATTRIBUTES.get(mode, [])) - set(actual_attrs) + + if (GlobalStaff().has_user(request.user) or has_api_key) and (mode_changed or active_changed): + if mode_changed and active_changed and not is_active: + msg = ( + f"Enrollment mode mismatch: active mode={enrollment['mode']}, " + f"requested mode={mode}. Won't deactivate." + ) + log.warning(msg) + raise ValidationError(msg) + + if missing_attrs: + msg = ( + f"Missing enrollment attributes: requested mode={mode} " + f"required attributes={REQUIRED_ATTRIBUTES.get(mode)}" + ) + log.warning(msg) + raise ValidationError(msg) + + response_data = api.update_enrollment( + username, + str(course_id), + mode=mode, + is_active=is_active, + enrollment_attributes=enrollment_attributes, + include_expired=has_api_key, + ) + else: + response_data = api.add_enrollment( + username, + str(course_id), + mode=mode, + is_active=is_active, + enrollment_attributes=enrollment_attributes, + enterprise_uuid=request.data.get("enterprise_uuid"), + force_enrollment=force_enrollment, + include_expired=force_enrollment, + ) + + cohort_name = request.data.get("cohort") + if cohort_name is not None: + cohort = get_cohort_by_name(course_id, cohort_name) + try: + add_user_to_cohort(cohort, user) + except ValueError: + log.exception("Cohort re-addition") + + email_opt_in = request.data.get("email_opt_in", None) + if email_opt_in is not None: + update_email_opt_in(request.user, course_id.org, email_opt_in) + + log.info("The user [%s] has already been enrolled in course run [%s].", username, course_id) + return response_data + + except InvalidEnrollmentAttribute as error: + raise ValidationError(str(error)) from error + except EnrollmentNotAllowed as error: + raise PermissionDenied(str(error)) from error + except CourseModeNotFoundError as error: + raise ValidationError( + f"The [{mode}] course mode is expired or otherwise unavailable for course run [{course_id}]." + ) from error + except CourseNotFoundError as error: + raise ValidationError(f"No course '{course_id}' found for enrollment") from error + except CourseEnrollmentExistsError as error: + log.warning("An enrollment already exists for user [%s] in course run [%s].", username, course_id) + # Caller-visible signal that the enrollment already exists. Use 200 + existing enrollment body + # (matches v1 semantics) — surface as a successful return, not an exception. + return error.enrollment + except CourseEnrollmentError as error: + log.exception( + "An error occurred while creating the new course enrollment for user [%s] in course run [%s]", + username, course_id, + ) + raise ValidationError( + f"An error occurred while creating the new course enrollment " + f"for user '{username}' in course '{course_id}'" + ) from error + except CourseUserGroup.DoesNotExist as error: + log.exception("Missing cohort [%s] in course run [%s]", cohort_name, course_id) + raise ValidationError( + f"An error occured while adding to cohort [{cohort_name}]" + ) from error + finally: + # Audit-log every API-key-driven enrollment change. + if has_api_key and user is not None: + try: + current = CourseEnrollment.objects.get(user__username=username, course_id=course_id) + actual_mode = current.mode + actual_activation = current.is_active + except CourseEnrollment.DoesNotExist: + actual_mode = None + actual_activation = None + audit_log( + "enrollment_change_requested", + course_id=str(course_id), + requested_mode=mode, + actual_mode=actual_mode, + requested_activation=is_active, + actual_activation=actual_activation, + user_id=user.id, + ) + + # ------------------------------------------------------------------ + # Unenroll (retirement pipeline) + # ------------------------------------------------------------------ + def unenroll_user_for_retirement(self, username): + """ + Handle the retirement-pipeline /enrollment/unenroll/ flow. + + Returns: + - ``None`` if the user has no active enrollments (caller should + return 204 No Content). + - A dict (the unenroll-result payload) on success (caller returns 200). + + Raises: + ValidationError: if ``username`` is missing. + NotFound: if no retirement-status row exists for the user. + APIException: on any other unexpected error (mapped to 500). + """ + if not username: + raise ValidationError("Username not specified.") + try: + UserRetirementStatus.get_retirement_for_retirement_action(username) + except UserRetirementStatus.DoesNotExist as exc: + raise NotFound("No retirement request status for username.") from exc + try: + if not CourseEnrollment.objects.filter(user__username=username, is_active=True).exists(): + return None + return api.unenroll_user_from_all_courses(username) + except Exception as exc: # pylint: disable=broad-except + log.exception("Unexpected error during unenrollment for user %s", username) + raise APIException("An unexpected error occurred during unenrollment.") from exc + + # ------------------------------------------------------------------ + # Allowed enrollments + # ------------------------------------------------------------------ + def list_allowed_for_email(self, email): + """Return the ``CourseEnrollmentAllowed`` queryset for ``email``.""" + return CourseEnrollmentAllowed.objects.filter(email=email) + + def create_allowed_enrollment(self, serializer): + """ + Persist the allowed-enrollment described by ``serializer``. + + Raises: + Conflict: if a row already exists for the (email, course_id) pair. + """ + from django.db import IntegrityError # local import — avoid heavy startup cost + try: + return serializer.save() + except IntegrityError as exc: + email = serializer.validated_data.get("email") + course_id = serializer.validated_data.get("course_id") + raise Conflict( + f"An enrollment allowed with email {email} and course {course_id} already exists." + ) from exc + + def delete_allowed_enrollment(self, email, course_id): + """ + Delete the allowed-enrollment row identified by (email, course_id). + + Raises: + NotFound: if no such row exists. + """ + try: + CourseEnrollmentAllowed.objects.get(email=email, course_id=course_id).delete() + except ObjectDoesNotExist as exc: + raise NotFound( + f"An enrollment allowed with email {email} and course {course_id} doesn't exists." + ) from exc diff --git a/openedx/core/djangoapps/enrollments/v2/views.py b/openedx/core/djangoapps/enrollments/v2/views.py new file mode 100644 index 000000000000..cb940244c401 --- /dev/null +++ b/openedx/core/djangoapps/enrollments/v2/views.py @@ -0,0 +1,626 @@ +""" +API Views for the Enrollment API — v2. + +This module is the v2 incarnation of the v1 enrollment views, restructured +to apply the FC-0118 ADRs from the start: + + * ADR 0025 – ``serializer_class`` on every viewset/view + * ADR 0026 – explicit ``authentication_classes`` + ``permission_classes`` + * ADR 0027 – ``drf_spectacular`` for OpenAPI schema generation + * ADR 0028 – consolidated into ``ViewSet`` classes registered via + ``DefaultRouter`` where the URL shape allows it + * ADR 0029 – standardized error envelope via :class:`StandardizedErrorMixin` + * ADR 0031 – business logic centralized in + :class:`EnrollmentOperationsService` (``v2.view_services``) + * ADR 0032 – ``DefaultPagination`` 7-field envelope on list endpoints + * ADR 0033 – OEP-68 parameter naming (``course_key`` preferred, + ``course_id`` as deprecated alias) plus standard ``ordering`` whitelist + +Existing v1 endpoints at ``/api/enrollment/v1/`` are unchanged — v2 is a +parallel new version mounted at ``/api/enrollment/v2/``. +""" + +import logging + +from django.contrib.auth import get_user_model +from django.utils.decorators import method_decorator +from drf_spectacular.utils import ( + OpenApiParameter, + OpenApiRequest, + OpenApiResponse, + extend_schema, +) +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.paginators import DefaultPagination +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from rest_framework import permissions, status, viewsets +from rest_framework.decorators import action +from rest_framework.exceptions import NotFound, ValidationError +from rest_framework.generics import ListAPIView +from rest_framework.response import Response +from rest_framework.views import APIView + +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.util.disable_rate_limit import can_disable_rate_limit +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.cors_csrf.decorators import ensure_csrf_cookie_cross_domain +from openedx.core.djangoapps.enrollments import api +from openedx.core.djangoapps.enrollments.errors import CourseEnrollmentError +from openedx.core.djangoapps.enrollments.serializers import ( + CourseEnrollmentAllowedSerializer, + CourseEnrollmentsApiListSerializer, + CourseEnrollmentSerializer, + CourseSerializer, +) +from openedx.core.djangoapps.enrollments.v2.forms import EnrollmentsAdminListForm +from openedx.core.djangoapps.enrollments.v2.paginators import EnrollmentsAdminListPagination +from openedx.core.djangoapps.enrollments.v2.serializers import UserRolesResponseSerializer +from openedx.core.djangoapps.enrollments.v2.view_services import EnrollmentOperationsService +from openedx.core.djangoapps.enrollments.views import ( + ApiKeyPermissionMixIn, + EnrollmentCrossDomainSessionAuth, + EnrollmentUserThrottle, +) +from openedx.core.djangoapps.user_api.accounts.permissions import CanRetireUser +from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser +from openedx.core.lib.api.mixins import StandardizedErrorMixin +from openedx.core.lib.api.permissions import ApiKeyHeaderPermissionIsAuthenticated + +log = logging.getLogger(__name__) +User = get_user_model() + +# ADR 0031 — single shared service instance for the v2 enrollment operations. +_OPS = EnrollmentOperationsService() + + +# --------------------------------------------------------------------------- +# ADR 0027 — shared OpenAPI parameter and response building blocks +# --------------------------------------------------------------------------- +def _path_param(name: str, description: str) -> OpenApiParameter: + return OpenApiParameter( + name=name, description=description, required=True, type=str, location=OpenApiParameter.PATH, + ) + + +def _query_param(name: str, description: str, *, required: bool = False, type_=str, + deprecated: bool = False) -> OpenApiParameter: + return OpenApiParameter( + name=name, description=description, required=required, type=type_, + location=OpenApiParameter.QUERY, deprecated=deprecated, + ) + + +_COURSE_ID_PATH_PARAM = _path_param("course_id", "Course ID (e.g. course-v1:org+course+run).") +_USERNAME_PATH_PARAM = _path_param("username", "Username of the user.") +_USER_QUERY_PARAM = _query_param("user", "Username of the user whose enrollments to list.") +_INCLUDE_EXPIRED_QUERY_PARAM = _query_param( + "include_expired", "If '1', include expired enrollment modes in the response.", +) +_PAGE_QUERY_PARAM = _query_param("page", "Page number to retrieve. Default 1.") +_PAGE_SIZE_QUERY_PARAM = _query_param("page_size", "Items per page (default 10, max 100).") + +_RESP_UNAUTHENTICATED = OpenApiResponse(description="The requester is not authenticated.") +_RESP_FORBIDDEN = OpenApiResponse(description="The requester does not have permission for this operation.") +_RESP_NOT_FOUND = OpenApiResponse(description="The requested resource does not exist.") +_RESP_BAD_REQUEST = OpenApiResponse(description="Invalid request data or parameters.") + + +# --------------------------------------------------------------------------- +# ADR 0033 — Deprecation-header helpers (OEP-68 parameter naming) +# --------------------------------------------------------------------------- +def _build_legacy_param_deprecation_header(legacy_to_preferred): + """ + Build the ADR 0033 ``Deprecation`` HTTP header value for one or more + legacy parameter names, each paired with its OEP-68-compliant + replacement. + + Example: ``[('course_id', 'course_key')]`` → + ``"Parameter 'course_id' is deprecated. Use 'course_key' instead. ..."`` + """ + parts = [ + f"Parameter '{legacy}' is deprecated. Use '{preferred}' instead." + for legacy, preferred in legacy_to_preferred + ] + parts.append("Support will be removed in release ''.") + return " ".join(parts) + + +def _maybe_set_legacy_param_deprecation_header(request, response, alias_pairs): + """Set the ADR 0033 ``Deprecation`` HTTP header on the response when any + legacy parameter name from ``alias_pairs`` is present in the request.""" + used = [(legacy, preferred) for legacy, preferred in alias_pairs if legacy in request.query_params] + if used: + response["Deprecation"] = _build_legacy_param_deprecation_header(used) + return response + + +# =========================================================================== +# EnrollmentViewSet — consolidates list / create / unenroll / allowed +# =========================================================================== +@can_disable_rate_limit +class EnrollmentViewSet(StandardizedErrorMixin, viewsets.ViewSet, ApiKeyPermissionMixIn): + """ + Canonical ViewSet for the v2 Enrollment API. + + Consolidates the v1 ``EnrollmentListView`` + ``UnenrollmentView`` + + ``EnrollmentAllowedView`` into a single router-registered ViewSet + (ADR 0028). Per-action permissions are declared via the ``@action`` + decorator's ``permission_classes`` kwarg. + + Router URLs (registered at ``basename="enrollment"``):: + + GET /api/enrollment/v2/enrollment/ → list + POST /api/enrollment/v2/enrollment/ → create + POST /api/enrollment/v2/enrollment/unenroll/ → unenroll + GET /api/enrollment/v2/enrollment/enrollment_allowed/ → allowed (GET) + POST /api/enrollment/v2/enrollment/enrollment_allowed/ → allowed (POST) + DELETE /api/enrollment/v2/enrollment/enrollment_allowed/ → allowed (DELETE) + """ + + authentication_classes = ( + JwtAuthentication, + BearerAuthenticationAllowInactiveUser, + EnrollmentCrossDomainSessionAuth, + ) + permission_classes = (ApiKeyHeaderPermissionIsAuthenticated,) + throttle_classes = (EnrollmentUserThrottle,) + serializer_class = CourseEnrollmentSerializer + pagination_class = DefaultPagination # ADR 0032 + + def get_serializer_class(self): + if self.action == "allowed": + return CourseEnrollmentAllowedSerializer + return self.serializer_class + + def get_serializer(self, *args, **kwargs): + return self.get_serializer_class()(*args, **kwargs) + + # ------------------------------------------------------------------ + # list — GET /enrollment/ + # ------------------------------------------------------------------ + @extend_schema( + summary="List enrollments for a user (paginated)", + description=( + "Returns a paginated list of enrollments for the currently logged-in user, or for " + "the user named by the 'user' query parameter. Staff/admin/api-key access is required " + "to view another user's enrollments — otherwise the list is filtered to courses the " + "requester staffs." + ), + parameters=[_USER_QUERY_PARAM, _PAGE_QUERY_PARAM, _PAGE_SIZE_QUERY_PARAM], + responses={ + 200: OpenApiResponse( + response=CourseEnrollmentSerializer(many=True), + description="Paginated enrollment list.", + ), + 401: _RESP_UNAUTHENTICATED, + }, + ) + @method_decorator(ensure_csrf_cookie_cross_domain) + def list(self, request): + """List enrollments for the currently logged-in user (paginated).""" + username = request.GET.get("user", request.user.username) + enrollments = _OPS.list_enrollments_for_user( + request_user=request.user, + target_username=username, + has_api_key=self.has_api_key_permissions(request), + ) + paginator = self.pagination_class() + page = paginator.paginate_queryset(enrollments, request, view=self) + return paginator.get_paginated_response(self.get_serializer(page, many=True).data) + + # ------------------------------------------------------------------ + # create — POST /enrollment/ + # ------------------------------------------------------------------ + @extend_schema( + summary="Create or update an enrollment", + description=( + "Enrolls a user in a course. Server-to-server calls may deactivate or modify the " + "mode of existing enrollments; all other requests create or reactivate enrollments. " + "The request body must include course_details.course_id." + ), + request=OpenApiRequest(request=CourseEnrollmentSerializer), + responses={ + 200: OpenApiResponse( + response=CourseEnrollmentSerializer, + description="Enrollment created, reactivated, or updated successfully.", + ), + 400: _RESP_BAD_REQUEST, + 403: _RESP_FORBIDDEN, + 404: _RESP_NOT_FOUND, + }, + ) + @method_decorator(ensure_csrf_cookie_cross_domain) + def create(self, request): + """Enroll a user in a course (or update an existing enrollment).""" + course_id = request.data.get("course_details", {}).get("course_id") + if not course_id: + raise ValidationError("Course ID must be specified to create a new enrollment.") + try: + course_key = CourseKey.from_string(course_id) + except InvalidKeyError as exc: + raise ValidationError(f"No course '{course_id}' found for enrollment") from exc + return Response(_OPS.create_or_update_enrollment( + request=request, + has_api_key=self.has_api_key_permissions(request), + course_id=course_key, + )) + + # ------------------------------------------------------------------ + # unenroll — @action POST /enrollment/unenroll/ + # ------------------------------------------------------------------ + @extend_schema( + summary="Unenroll a user from all courses (retirement)", + description=( + "Privileged retirement-pipeline use only. Unenrolls the named user from every active " + "enrollment. The request must be made by a service user with CanRetireUser permission, " + "not the user being unenrolled." + ), + request=OpenApiRequest( + request={"type": "object", "properties": {"username": {"type": "string"}}, "required": ["username"]}, + ), + responses={ + 200: OpenApiResponse(description="List of courses from which the user was unenrolled."), + 204: OpenApiResponse(description="User has no active enrollments."), + 400: _RESP_BAD_REQUEST, + 404: _RESP_NOT_FOUND, + }, + ) + @action( + detail=False, + methods=["post"], + url_path="unenroll", + permission_classes=[permissions.IsAuthenticated, CanRetireUser], + ) + def unenroll(self, request): + """Unenroll the specified user from all courses (retirement pipeline).""" + result = _OPS.unenroll_user_for_retirement(request.data.get("username")) + if result is None: + return Response(status=status.HTTP_204_NO_CONTENT) + return Response(result) + + # ------------------------------------------------------------------ + # allowed — @action GET/POST/DELETE /enrollment/enrollment_allowed/ + # ------------------------------------------------------------------ + @extend_schema( + summary="Manage CourseEnrollmentAllowed records (admin-only)", + description=( + "GET lists allowed enrollments for an email; POST creates a new one; DELETE removes " + "an existing one by email + course_id. Admin-only." + ), + request=OpenApiRequest(request=CourseEnrollmentAllowedSerializer), + parameters=[_query_param("email", "Email to query (GET only). Defaults to the requester's email.")], + responses={ + 200: OpenApiResponse( + response=CourseEnrollmentAllowedSerializer(many=True), + description="GET success — list of allowed enrollments for the email.", + ), + 201: OpenApiResponse( + response=CourseEnrollmentAllowedSerializer, + description="POST success — allowed enrollment created.", + ), + 204: OpenApiResponse(description="DELETE success — allowed enrollment deleted."), + 400: _RESP_BAD_REQUEST, + 403: _RESP_FORBIDDEN, + 404: OpenApiResponse(description="DELETE: allowed enrollment not found."), + 409: OpenApiResponse(description="POST: allowed enrollment already exists."), + }, + ) + @action( + detail=False, + methods=["get", "post", "delete"], + url_path="enrollment_allowed", + permission_classes=[permissions.IsAdminUser], + throttle_classes=[EnrollmentUserThrottle], + ) + def allowed(self, request): + """Retrieve, create, or delete CourseEnrollmentAllowed records. Admin-only.""" + if request.method == "GET": + user_email = request.query_params.get("email") or request.user.email + enrollments_allowed = _OPS.list_allowed_for_email(user_email) + return Response( + status=status.HTTP_200_OK, + data=self.get_serializer(enrollments_allowed, many=True).data, + ) + + serializer = self.get_serializer(data=request.data) + if not serializer.is_valid(): + raise ValidationError(serializer.errors) + + if request.method == "POST": + enrollment_allowed = _OPS.create_allowed_enrollment(serializer) + return Response( + status=status.HTTP_201_CREATED, + data=self.get_serializer(enrollment_allowed).data, + ) + + # DELETE + _OPS.delete_allowed_enrollment( + email=serializer.validated_data.get("email"), + course_id=serializer.validated_data.get("course_id"), + ) + return Response(status=status.HTTP_204_NO_CONTENT) + + +# =========================================================================== +# EnrollmentRetrieveView — singleton GET /enrollment/{course_id} +# =========================================================================== +# Kept as a standalone APIView because the {username},{course_id} URL form +# (comma-separated, both optional) is not expressible via DefaultRouter. +class EnrollmentRetrieveView(StandardizedErrorMixin, ApiKeyPermissionMixIn, APIView): + """GET enrollment for a course (and optionally a named user).""" + + authentication_classes = ( + JwtAuthentication, + BearerAuthenticationAllowInactiveUser, + EnrollmentCrossDomainSessionAuth, + ) + permission_classes = (ApiKeyHeaderPermissionIsAuthenticated,) + throttle_classes = (EnrollmentUserThrottle,) + serializer_class = CourseEnrollmentSerializer + + @extend_schema( + summary="Retrieve a user's enrollment in a course", + description=( + "Returns the current user's enrollment for the specified course, or the named user's " + "enrollment when invoked with the {username},{course_id} URL form (server-to-server or " + "staff only)." + ), + parameters=[_USERNAME_PATH_PARAM, _COURSE_ID_PATH_PARAM], + responses={ + 200: OpenApiResponse( + response=CourseEnrollmentSerializer, + description="Enrollment retrieved successfully (or empty body if no enrollment).", + ), + 400: _RESP_BAD_REQUEST, + 404: _RESP_NOT_FOUND, + }, + ) + @method_decorator(ensure_csrf_cookie_cross_domain) + def get(self, request, course_id=None, username=None): + """ + Return the enrollment for ``(username, course_id)``. + + When ``username`` is omitted (the ``GET /enrollment/{course_id}`` + URL form), the request user is used. Non-staff callers may only + look up their own enrollment; any cross-user lookup without + ``has_api_key`` or staff privileges raises ``NotFound`` (so the + caller cannot probe for the existence of other users' enrollments). + """ + if username is None: + username = request.user.username + + if ( + username != request.user.username + and not self.has_api_key_permissions(request) + and not request.user.is_staff + ): + # Hide existence of other users' enrollments. + raise NotFound() + + try: + course_key = CourseKey.from_string(course_id) + except InvalidKeyError as exc: + raise ValidationError(f"No course '{course_id}' found for enrollment") from exc + + try: + enrollment = CourseEnrollment.objects.get(user__username=username, course_id=course_key) + except CourseEnrollment.DoesNotExist: + return Response(None) + except CourseEnrollmentError as exc: + raise ValidationError( + f"An error occurred while retrieving enrollments for user " + f"'{username}' in course '{course_id}'" + ) from exc + + return Response(self.serializer_class(enrollment).data) + + +# =========================================================================== +# UserRolesView — GET /roles/ (singleton list endpoint for the current user) +# =========================================================================== +class UserRolesView(StandardizedErrorMixin, APIView): + """List the current user's course-level roles.""" + + authentication_classes = ( + JwtAuthentication, + BearerAuthenticationAllowInactiveUser, + EnrollmentCrossDomainSessionAuth, + ) + permission_classes = (ApiKeyHeaderPermissionIsAuthenticated,) + throttle_classes = (EnrollmentUserThrottle,) + serializer_class = UserRolesResponseSerializer + + # ADR 0033: ``course_key`` is the preferred filter name (OEP-68); + # ``course_id`` is retained as a deprecated alias. + _LEGACY_PARAM_ALIASES = (("course_id", "course_key"),) + + @extend_schema( + summary="List the current user's course roles", + description=( + "Returns the list of course-level roles held by the currently logged-in user, plus " + "an is_staff flag. Optionally filters by course_key (or course_id, deprecated)." + ), + parameters=[ + _query_param("course_key", "If provided, only roles for this course are returned (OEP-68)."), + _query_param( + "course_id", "Deprecated alias for 'course_key' (ADR 0033). Use 'course_key' instead.", + deprecated=True, + ), + ], + responses={ + 200: OpenApiResponse( + response=UserRolesResponseSerializer, + description="Roles retrieved successfully.", + ), + 400: _RESP_BAD_REQUEST, + }, + ) + @method_decorator(ensure_csrf_cookie_cross_domain) + def get(self, request): + """ + List the current user's course-level roles. + + Optionally filtered by ``course_key`` (preferred, OEP-68) or + ``course_id`` (deprecated alias). When both are present, + ``course_key`` wins and the response carries the ADR 0033 + ``Deprecation`` HTTP header. + """ + try: + course_key = request.GET.get("course_key") or request.GET.get("course_id") + roles_data = api.get_user_roles(request.user.username) + if course_key: + roles_data = [role for role in roles_data if str(role.course_id) == course_key] + except Exception as exc: # pylint: disable=broad-except + raise ValidationError( + f"An error occurred while retrieving roles for user '{request.user.username}'" + ) from exc + + serializer = self.serializer_class({ + "roles": list(roles_data), + "is_staff": request.user.is_staff, + }) + response = Response(serializer.data) + return _maybe_set_legacy_param_deprecation_header( + request, response, self._LEGACY_PARAM_ALIASES, + ) + + +# =========================================================================== +# CourseEnrollmentDetailView — GET /course/{course_id} (public, no auth) +# =========================================================================== +class CourseEnrollmentDetailView(StandardizedErrorMixin, APIView): + """Get enrollment information about a particular course.""" + + authentication_classes = () + permission_classes = () + throttle_classes = (EnrollmentUserThrottle,) + serializer_class = CourseSerializer + + @extend_schema( + summary="Get enrollment details for a course", + description=( + "Returns the course schedule and supported enrollment modes. No authentication " + "required. Use ?include_expired=1 to include expired enrollment modes." + ), + parameters=[_COURSE_ID_PATH_PARAM, _INCLUDE_EXPIRED_QUERY_PARAM], + responses={ + 200: OpenApiResponse( + response=CourseSerializer, + description="Course enrollment details retrieved successfully.", + ), + 400: _RESP_BAD_REQUEST, + 404: _RESP_NOT_FOUND, + }, + ) + def get(self, request, course_id=None): + """ + Return enrollment-related details for the specified course. + + Public (no authentication required). The response includes the + course schedule and supported enrollment modes; pass + ``?include_expired=1`` to include expired enrollment modes. + """ + try: + course_key = CourseKey.from_string(course_id) + except InvalidKeyError as exc: + raise ValidationError(f"No course found for course ID '{course_id}'") from exc + try: + course_overview = CourseOverview.get_from_id(course_key) + except CourseOverview.DoesNotExist as exc: + raise NotFound(f"No course found for course ID '{course_id}'") from exc + + include_expired = bool(request.GET.get("include_expired", "")) + serializer = self.serializer_class(course_overview, include_expired=include_expired) + return Response(serializer.data) + + +# =========================================================================== +# EnrollmentsAdminListView — GET /enrollments/ (admin paginated list) +# =========================================================================== +@extend_schema( + summary="List all course enrollments (admin-only, paginated)", + description=( + "Admin-only paginated list of CourseEnrollment records, optionally filtered by " + "course_key, course_keys, username, or email, and optionally ordered." + ), + parameters=[ + _query_param("course_key", "Filter to enrollments for this course (OEP-68)."), + _query_param("course_keys", "Comma-separated list of course keys (OEP-68)."), + _query_param( + "course_id", "Deprecated alias for 'course_key' (ADR 0033). Use 'course_key' instead.", + deprecated=True, + ), + _query_param( + "course_ids", "Deprecated alias for 'course_keys' (ADR 0033). Use 'course_keys' instead.", + deprecated=True, + ), + _query_param("username", "Comma-separated list of usernames."), + _query_param("email", "Comma-separated list of emails."), + _query_param("ordering", "Order results by one of: created, -created, id, -id (ADR 0033 §3)."), + _PAGE_QUERY_PARAM, + _PAGE_SIZE_QUERY_PARAM, + ], + responses={ + 200: OpenApiResponse( + response=CourseEnrollmentsApiListSerializer(many=True), + description="Paginated list of course enrollments.", + ), + 400: _RESP_BAD_REQUEST, + 401: _RESP_UNAUTHENTICATED, + 403: _RESP_FORBIDDEN, + }, +) +class EnrollmentsAdminListView(StandardizedErrorMixin, ListAPIView): + """Admin-only paginated enrollment list with OEP-68 filter aliases.""" + + authentication_classes = ( + JwtAuthentication, + BearerAuthenticationAllowInactiveUser, + EnrollmentCrossDomainSessionAuth, + ) + permission_classes = (permissions.IsAdminUser,) + throttle_classes = (EnrollmentUserThrottle,) + serializer_class = CourseEnrollmentsApiListSerializer + pagination_class = EnrollmentsAdminListPagination + + # ADR 0033 §3 — whitelist of allowed values for the ``ordering`` param. + ALLOWED_ORDERING_FIELDS = frozenset({"created", "-created", "id", "-id"}) + + # ADR 0033 §2 / OEP-68 alias pairs accepted by this endpoint. + _LEGACY_PARAM_ALIASES = ( + ("course_id", "course_key"), + ("course_ids", "course_keys"), + ) + + def get_queryset(self): + form = EnrollmentsAdminListForm(self.request.query_params) + if not form.is_valid(): + raise ValidationError(form.errors) + + queryset = CourseEnrollment.objects.all().select_related("user", "course") + course_id = form.cleaned_data.get("course_id") + course_ids = form.cleaned_data.get("course_ids") + usernames = form.cleaned_data.get("username") + emails = form.cleaned_data.get("email") + + if course_id: + queryset = queryset.filter(course_id=course_id) + if course_ids: + queryset = queryset.filter(course_id__in=course_ids) + if usernames: + queryset = queryset.filter(user__username__in=usernames) + if emails: + queryset = queryset.filter(user__email__in=emails) + + ordering = self.request.query_params.get("ordering") + if ordering in self.ALLOWED_ORDERING_FIELDS: + queryset = queryset.order_by(ordering) + return queryset + + def list(self, request, *args, **kwargs): + """Override to emit the ADR 0033 Deprecation header when legacy params used.""" + response = super().list(request, *args, **kwargs) + return _maybe_set_legacy_param_deprecation_header( + request, response, self._LEGACY_PARAM_ALIASES, + ) 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)]} diff --git a/openedx/core/lib/api/mixins.py b/openedx/core/lib/api/mixins.py index f0af9072e101..693a02ecf155 100644 --- a/openedx/core/lib/api/mixins.py +++ b/openedx/core/lib/api/mixins.py @@ -8,6 +8,29 @@ from rest_framework.mixins import CreateModelMixin from rest_framework.response import Response +from openedx.core.lib.api.exceptions import standardized_error_exception_handler + + +class StandardizedErrorMixin: + """ + Opt-in mixin that routes DRF exceptions on this view through the ADR 0029 + standardized error-response handler (see + ``openedx.core.lib.api.exceptions.standardized_error_exception_handler``). + + DRF's :class:`rest_framework.views.APIView` calls ``self.get_exception_handler`` + inside ``handle_exception``; overriding that method here lets the view + return the standardized envelope while other endpoints continue to use + whichever handler the project-wide ``EXCEPTION_HANDLER`` setting points at. + + Usage:: + + class MyViewSet(StandardizedErrorMixin, viewsets.ViewSet): + ... + """ + + def get_exception_handler(self): + return standardized_error_exception_handler + class PutAsCreateMixin(CreateModelMixin): """