[FC-0118] Contentstore Course Detail API ADR Implementation pt 2

cms/djangoapps/contentstore/rest_api/v1/tests/test_course_details.py

@@ -0,0 +1,100 @@
+"""
+ADR 0029 – Standardized error-response tests for CourseDetailsViewSet.
+
+Tests that auth/permission/not-found error responses conform to the ADR 0029
+JSON envelope after removing DeveloperErrorViewMixin and @verify_course_exists().
+"""
+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
+
+# A syntactically valid course key that does not exist in the DB.
+TEST_COURSE_ID = "course-v1:TestOrg+TestCourse+2026"
+MOCK_COURSE_EXISTS = (
+    "cms.djangoapps.contentstore.rest_api.v1.views.course_details.CourseOverview.course_exists"
+)
+
+_REQUIRED_ERROR_FIELDS = ("type", "title", "status", "detail", "instance")
+
+
+class TestCourseDetailsViewSetErrorShape(APITestCase):
+    """
+    ADR 0029 – error response shape regression tests for CourseDetailsViewSet.
+
+    Verifies that 401, 403, and 404 responses use the standardized envelope
+    after removing DeveloperErrorViewMixin and @verify_course_exists().
+    """
+
+    def setUp(self):
+        super().setUp()
+        self.client = APIClient()
+        self.detail_url = reverse(
+            "cms.djangoapps.contentstore:v1: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)
+        self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
+        for field in _REQUIRED_ERROR_FIELDS:
+            self.assertIn(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.detail_url)
+        self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
+        self.assertEqual(response.data.get("type"), "https://docs.openedx.org/errors/authn")
+
+    def test_non_author_get_returns_standardized_403(self):
+        """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)
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        for field in _REQUIRED_ERROR_FIELDS:
+            self.assertIn(field, response.data, f"ADR 0029: missing field '{field}'")
+
+    def test_non_author_403_type_uri(self):
+        """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)
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(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):
+        """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)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        for field in _REQUIRED_ERROR_FIELDS:
+            self.assertIn(field, response.data, f"ADR 0029: missing field '{field}'")
+
+    @patch(MOCK_COURSE_EXISTS, return_value=False)
+    def test_not_found_type_uri(self, _mock):
+        """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)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        self.assertEqual(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 the old DeveloperErrorViewMixin fields."""
+        response = self.client.get(self.detail_url)
+        self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
+        self.assertNotIn("developer_message", response.data)
+        self.assertNotIn("error_code", 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)
+        self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
+        self.assertEqual(response.data.get("instance"), self.detail_url)

cms/djangoapps/contentstore/rest_api/v1/views/course_details.py

@@ -1,18 +1,20 @@
 """ API Views for course details """
 
 import edx_api_doc_tools as apidocs
-from django.core.exceptions import ValidationError
-from common.djangoapps.util.json_request import JsonResponseBadRequest
+from django.core.exceptions import ValidationError as DjangoValidationError
+from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
+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
 from rest_framework.response import Response
 from rest_framework import viewsets
 from rest_framework.views import APIView
 from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
 from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.models.course_details import CourseDetails
-from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists
 from xmodule.modulestore.django import modulestore
 
 from cms.djangoapps.contentstore.views.permissions import HasStudioReadAccess
@@ -21,7 +23,7 @@
 
 
 # ADR 0028 – consolidated from CourseDetailsView
-class CourseDetailsViewSet(DeveloperErrorViewMixin, viewsets.ViewSet):
+class CourseDetailsViewSet(viewsets.ViewSet):
     """
     ViewSet for course details.  Registered via DefaultRouter (basename ``course_details``).
 
@@ -31,8 +33,7 @@ class CourseDetailsViewSet(DeveloperErrorViewMixin, viewsets.ViewSet):
 
     ADR 0025 compliance notes:
     - ``serializer_class`` declared; used for both response serialization and apidocs schema.
-    - Request validation is handled by ``update_course_details()`` (service layer) and
-      the ``@verify_course_exists()`` decorator.
+    - Request validation is handled by ``update_course_details()`` (service layer).
     """
 
     authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser)
@@ -54,7 +55,6 @@ class CourseDetailsViewSet(DeveloperErrorViewMixin, viewsets.ViewSet):
             404: "The requested course does not exist.",
         },
     )
-    @verify_course_exists()
     def retrieve(self, request: Request, course_id: str):
         """
         Get an object containing all the course details.
@@ -70,7 +70,12 @@ def retrieve(self, request: Request, course_id: str):
         The HTTP 200 response contains a single dict that contains keys that
         are the course's details.
         """
-        course_key = CourseKey.from_string(course_id)
+        try:
+            course_key = CourseKey.from_string(course_id)
+        except InvalidKeyError:
+            raise NotFound("The provided course key cannot be parsed.")  # noqa: B904
+        if not CourseOverview.course_exists(course_key):
+            raise NotFound(f"Course {course_id} not found.")
         course_details = CourseDetails.fetch(course_key)
         serializer = self.serializer_class(course_details)
         return Response(serializer.data)
@@ -87,7 +92,6 @@ def retrieve(self, request: Request, course_id: str):
             404: "The requested course does not exist.",
         },
     )
-    @verify_course_exists()
     def update(self, request: Request, course_id: str):
         """
         Update a course's details.
@@ -108,27 +112,33 @@ 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 = CourseKey.from_string(course_id)
+        try:
+            course_key = CourseKey.from_string(course_id)
+        except InvalidKeyError:
+            raise NotFound("The provided course key cannot be parsed.")  # noqa: B904
+        if not CourseOverview.course_exists(course_key):
+            raise NotFound(f"Course {course_id} not found.")
         course_block = modulestore().get_course(course_key)
 
         try:
             updated_data = update_course_details(request, course_key, request.data, course_block)
-        except ValidationError as err:
-            return JsonResponseBadRequest({"error": err.message})
+        except DjangoValidationError as err:
+            raise DRFValidationError(err.message) from err
 
         serializer = self.serializer_class(updated_data)
         return Response(serializer.data)
 
 
 # DEPRECATED (ADR 0028): Use CourseDetailsViewSet instead.
 # Will be removed after one named release. Use GET/PUT course_details/{course_id}/ instead.
-class CourseDetailsView(DeveloperErrorViewMixin, APIView):
+class CourseDetailsView(APIView):
     """
     View for getting and setting the course details.
     """
     authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser)
     permission_classes = (IsAuthenticated, HasStudioReadAccess)
     serializer_class = CourseDetailsSerializer
+
     @apidocs.schema(
         parameters=[
             apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID"),
@@ -140,7 +150,6 @@ class CourseDetailsView(DeveloperErrorViewMixin, APIView):
             404: "The requested course does not exist.",
         },
     )
-    @verify_course_exists()
     def get(self, request: Request, course_id: str):
         """
         Get an object containing all the course details.
@@ -205,7 +214,12 @@ def get(self, request: Request, course_id: str):
         }
         ```
         """
-        course_key = CourseKey.from_string(course_id)
+        try:
+            course_key = CourseKey.from_string(course_id)
+        except InvalidKeyError:
+            raise NotFound("The provided course key cannot be parsed.")  # noqa: B904
+        if not CourseOverview.course_exists(course_key):
+            raise NotFound(f"Course {course_id} not found.")
         course_details = CourseDetails.fetch(course_key)
         serializer = self.serializer_class(course_details)
         return Response(serializer.data)
@@ -222,7 +236,6 @@ def get(self, request: Request, course_id: str):
             404: "The requested course does not exist.",
         },
     )
-    @verify_course_exists()
     def put(self, request: Request, course_id: str):
         """
         Update a course's details.
@@ -245,13 +258,18 @@ def put(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 = CourseKey.from_string(course_id)
+        try:
+            course_key = CourseKey.from_string(course_id)
+        except InvalidKeyError:
+            raise NotFound("The provided course key cannot be parsed.")  # noqa: B904
+        if not CourseOverview.course_exists(course_key):
+            raise NotFound(f"Course {course_id} not found.")
         course_block = modulestore().get_course(course_key)
 
         try:
             updated_data = update_course_details(request, course_key, request.data, course_block)
-        except ValidationError as err:
-            return JsonResponseBadRequest({"error": err.message})
+        except DjangoValidationError as err:
+            raise DRFValidationError(err.message) from err
 
         serializer = self.serializer_class(updated_data)
         return Response(serializer.data)