From f9c34d312ad7392ef4f6f8d9c10a4373691635c8 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed Date: Tue, 9 Jun 2026 02:34:30 +0500 Subject: [PATCH] feat: apply ADR standardization to AuthorGrading apis --- .../contentstore/rest_api/v3/urls.py | 3 +- .../contentstore/rest_api/v3/utils.py | 55 ++++ .../rest_api/v3/views/__init__.py | 1 + .../rest_api/v3/views/authoring_grading.py | 163 ++++++++++ .../rest_api/v3/views/course_details.py | 36 +-- .../v3/views/tests/test_authoring_grading.py | 293 ++++++++++++++++++ .../v3/views/tests/test_course_details.py | 5 +- 7 files changed, 523 insertions(+), 33 deletions(-) create mode 100644 cms/djangoapps/contentstore/rest_api/v3/utils.py create mode 100644 cms/djangoapps/contentstore/rest_api/v3/views/authoring_grading.py create mode 100644 cms/djangoapps/contentstore/rest_api/v3/views/tests/test_authoring_grading.py diff --git a/cms/djangoapps/contentstore/rest_api/v3/urls.py b/cms/djangoapps/contentstore/rest_api/v3/urls.py index b0ba53997bae..36eb245396b9 100644 --- a/cms/djangoapps/contentstore/rest_api/v3/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v3/urls.py @@ -2,12 +2,13 @@ from rest_framework.routers import DefaultRouter -from cms.djangoapps.contentstore.rest_api.v3.views import CourseDetailsViewSet, HomeViewSet +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 index 3fd8759e2318..d3d8670950a3 100644 --- a/cms/djangoapps/contentstore/rest_api/v3/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v3/views/__init__.py @@ -1,4 +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 index 85bfa6d691bc..47a4743854b0 100644 --- a/cms/djangoapps/contentstore/rest_api/v3/views/course_details.py +++ b/cms/djangoapps/contentstore/rest_api/v3/views/course_details.py @@ -26,15 +26,12 @@ 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 opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey 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 NotFound from rest_framework.exceptions import ValidationError as DRFValidationError from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request @@ -42,10 +39,10 @@ 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.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.models.course_details import CourseDetails from openedx.core.lib.api.mixins import StandardizedErrorMixin from xmodule.modulestore.django import modulestore @@ -57,29 +54,6 @@ type=str, location=OpenApiParameter.PATH, ) -_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."), -} - - -def _resolve_course_key(course_id: str) -> CourseKey: - """ - Parse ``course_id`` into a ``CourseKey`` and verify the course exists. - - Raises ``NotFound`` for both unparseable keys and missing courses, which - the ADR 0029 envelope renders as a structured 404 response. This replaces - the legacy ``@verify_course_exists()`` decorator from v1 and avoids - relying on ``DeveloperErrorViewMixin``. - """ - try: - course_key = CourseKey.from_string(course_id) - except InvalidKeyError as exc: - raise NotFound("The provided course key cannot be parsed.") from exc - if not CourseOverview.course_exists(course_key): - raise NotFound(f"Course {course_id} not found.") - return course_key class CourseDetailsViewSet(StandardizedErrorMixin, viewsets.ViewSet): @@ -111,7 +85,7 @@ class CourseDetailsViewSet(StandardizedErrorMixin, viewsets.ViewSet): response=CourseDetailsSerializer, description="Course details retrieved successfully.", ), - **_COMMON_ERROR_RESPONSES, + **COMMON_ERROR_RESPONSES, }, ) def retrieve(self, request: Request, course_id: str): @@ -122,7 +96,7 @@ def retrieve(self, request: Request, course_id: str): GET /api/contentstore/v3/course_details/{course_id}/ """ - course_key = _resolve_course_key(course_id) + course_key = resolve_course_key(course_id) if not user_has_course_permission( request.user, COURSES_VIEW_SCHEDULE_AND_DETAILS.identifier, @@ -146,7 +120,7 @@ def retrieve(self, request: Request, course_id: str): description="Course details updated successfully.", ), 400: OpenApiResponse(description="Bad request — invalid data."), - **_COMMON_ERROR_RESPONSES, + **COMMON_ERROR_RESPONSES, }, ) def update(self, request: Request, course_id: str): @@ -169,7 +143,7 @@ def update(self, request: Request, course_id: str): 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) + 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: 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 index 3fe6854d1f35..a7d0165fb32d 100644 --- 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 @@ -37,8 +37,11 @@ "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.views.course_details.CourseOverview.course_exists" + "cms.djangoapps.contentstore.rest_api.v3.utils.CourseOverview.course_exists" ) # Syntactically valid course key reused across action / permission / envelope tests.