diff --git a/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py b/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py new file mode 100644 index 000000000000..17be6d10a3ca --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py @@ -0,0 +1,28 @@ +"""Authoring API v1 URLs (ADR 0038 conforming mount for Contentstore v1).""" + +from django.urls import path + +from cms.djangoapps.contentstore.rest_api.v1.views import XblockViewSet + +app_name = "authoring_v1" + +urlpatterns = [ + # The viewset has no ``list`` action, so the collection accepts POST only. + path( + "xblocks/", + XblockViewSet.as_view({"post": "create"}), + name="xblock_list", + ), + path( + "xblocks//", + XblockViewSet.as_view( + { + "get": "retrieve", + "put": "update", + "patch": "partial_update", + "delete": "destroy", + } + ), + name="xblock_detail", + ), +] 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 index 638b0ce2eb35..e212bf016932 100644 --- 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 @@ -10,7 +10,7 @@ from unittest.mock import patch from django.http import JsonResponse -from django.urls import reverse +from django.urls import resolve, reverse from rest_framework import status from rest_framework.test import APITestCase @@ -216,3 +216,66 @@ def test_minimal_view_is_noop_for_non_json_payload(self, mock_retrieve): response = self.client.get(_detail_url(), {"view": "minimal", "fields": "graderType"}) assert response.status_code == status.HTTP_200_OK assert response.json() == "notgraded" + + +# --------------------------------------------------------------------------- +# ADR 0038 — URL-structure tests +# --------------------------------------------------------------------------- + + +def _authoring_list_url(): + return reverse("authoring_v1:xblock_list") + + +def _authoring_detail_url(): + return reverse( + "authoring_v1:xblock_detail", + kwargs={"usage_key_string": TEST_LOCATOR}, + ) + + +class XblockViewSetUrlStructureTest(ModuleStoreTestCase, APITestCase): + """The conforming /api/authoring/v1/xblocks/ routes serve the same view as the legacy ones.""" + + def setUp(self): + super().setUp() + self.staff = GlobalStaffFactory(password='password') + self.client.force_authenticate(user=self.staff) + + def test_conforming_urls_reverse_to_expected_paths(self): + assert _authoring_list_url() == "/api/authoring/v1/xblocks/" + assert _authoring_detail_url() == f"/api/authoring/v1/xblocks/{TEST_LOCATOR}/" + + def test_conforming_and_legacy_routes_share_view(self): + legacy_cls = resolve(_detail_url()).func.cls + conforming_cls = resolve(_authoring_detail_url()).func.cls + assert conforming_cls is legacy_cls + + def test_invalid_usage_key_is_404_on_conforming_route(self): + # The shared usage_key converter rejects unparseable keys with a + # routing-level 404. + response = self.client.get("/api/authoring/v1/xblocks/not-a-usage-key/") + assert response.status_code == status.HTTP_404_NOT_FOUND + + @patch(f"{_VIEW_MODULE}.retrieve_xblock_response", return_value=_MOCK_RESPONSE) + def test_get_on_conforming_route_calls_retrieve(self, mock_fn): + # Also exercises the UsageKey→str coercion in XblockViewSet.initial(). + response = self.client.get(_authoring_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}.create_xblock_response", return_value=_MOCK_RESPONSE) + def test_post_on_conforming_route_calls_create(self, mock_fn): + data = {"parent_locator": PARENT_LOCATOR, "category": "html"} + response = self.client.post(_authoring_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}.delete_xblock_response", return_value=_MOCK_RESPONSE) + def test_delete_on_conforming_route_calls_destroy(self, mock_fn): + response = self.client.delete(_authoring_detail_url()) + assert response.status_code == status.HTTP_200_OK + mock_fn.assert_called_once() + assert mock_fn.call_args[0][0].method == "DELETE" diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py b/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py index 2d8d94ceb1de..02c0434ced2a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/xblock.py @@ -198,7 +198,12 @@ def initial(self, request, *args, **kwargs): 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") + # The conforming route passes a parsed UsageKey; the legacy route + # passes the raw string. Coerce to the string the actions expect — + # ``self.kwargs`` is the dict ``dispatch()`` unpacks into the handler. + if isinstance(self.kwargs.get("usage_key_string"), UsageKey): + self.kwargs["usage_key_string"] = str(self.kwargs["usage_key_string"]) + usage_key_string = self.kwargs.get("usage_key_string") if usage_key_string: try: self.course_key = UsageKey.from_string(usage_key_string).course_key diff --git a/cms/djangoapps/contentstore/rest_api/v3/authoring_urls.py b/cms/djangoapps/contentstore/rest_api/v3/authoring_urls.py new file mode 100644 index 000000000000..56faf73c811c --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v3/authoring_urls.py @@ -0,0 +1,35 @@ +"""Authoring API v3 URLs (ADR 0038 conforming mount for Contentstore v3).""" + +from django.urls import path + +from cms.djangoapps.contentstore.rest_api.v3.views import AuthoringGradingViewSet, CourseDetailsViewSet, HomeViewSet + +app_name = "authoring_v3" + +urlpatterns = [ + path( + "home/", + HomeViewSet.as_view({"get": "list"}), + name="home", + ), + path( + "home/courses/", + HomeViewSet.as_view({"get": "courses"}), + name="home_courses", + ), + path( + "home/libraries/", + HomeViewSet.as_view({"get": "libraries"}), + name="home_libraries", + ), + path( + "courses//details/", + CourseDetailsViewSet.as_view({"get": "retrieve", "put": "update"}), + name="course_details", + ), + path( + "courses//grading/", + AuthoringGradingViewSet.as_view({"patch": "partial_update"}), + name="course_grading", + ), +] diff --git a/cms/djangoapps/contentstore/rest_api/v3/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v3/tests/test_home.py index f2b4a4744d65..e48ae26ac47a 100644 --- a/cms/djangoapps/contentstore/rest_api/v3/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v3/tests/test_home.py @@ -10,7 +10,7 @@ ``EXCEPTION_HANDLER`` setting is unchanged, so v0/v1/v2 endpoints continue to return the legacy error shape. """ -from django.urls import reverse +from django.urls import resolve, reverse from rest_framework import status from rest_framework.test import APIClient, APITestCase @@ -84,3 +84,30 @@ def test_v1_endpoint_unaffected_by_v3_envelope(self): # v1 still uses the project-default handler → ADR 0029 fields absent. assert "type" not in response.data assert "instance" not in response.data + + +class TestHomeViewSetUrlStructure(APITestCase): + """The conforming /api/authoring/v3/home/ routes serve the same view as the legacy ones.""" + + def test_conforming_urls_reverse_to_expected_paths(self): + assert reverse("authoring_v3:home") == "/api/authoring/v3/home/" + assert reverse("authoring_v3:home_courses") == "/api/authoring/v3/home/courses/" + assert reverse("authoring_v3:home_libraries") == "/api/authoring/v3/home/libraries/" + + def test_conforming_and_legacy_routes_share_view(self): + pairs = ( + ("cms.djangoapps.contentstore:v3:home-list", "authoring_v3:home"), + ("cms.djangoapps.contentstore:v3:home-courses", "authoring_v3:home_courses"), + ("cms.djangoapps.contentstore:v3:home-libraries", "authoring_v3:home_libraries"), + ) + for legacy_name, conforming_name in pairs: + legacy_cls = resolve(reverse(legacy_name)).func.cls + conforming_cls = resolve(reverse(conforming_name)).func.cls + assert conforming_cls is legacy_cls, f"{conforming_name} must serve the same view as {legacy_name}" + + def test_unauthenticated_conforming_route_returns_standardized_401(self): + """The conforming mount carries the same contract — ADR 0029 envelope included.""" + response = APIClient().get(reverse("authoring_v3:home")) + 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}'" diff --git a/cms/djangoapps/contentstore/rest_api/v3/utils.py b/cms/djangoapps/contentstore/rest_api/v3/utils.py index 79524acb8c53..4fed4e44f7d8 100644 --- a/cms/djangoapps/contentstore/rest_api/v3/utils.py +++ b/cms/djangoapps/contentstore/rest_api/v3/utils.py @@ -27,10 +27,15 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -def resolve_course_key(course_key: str) -> CourseKey: +def resolve_course_key(course_key: str | CourseKey) -> CourseKey: """ - Parse ``course_key`` (string) into a :class:`CourseKey` and verify the - course exists. + Parse ``course_key`` into a :class:`CourseKey` and verify the course + exists. + + Accepts either the raw string (the legacy ``/api/contentstore/v3/`` + routes) or an already-parsed :class:`CourseKey` (the conforming + ``/api/authoring/v3/`` routes, whose ``course_key`` path converter — + ADR 0038 rule 9 — hands views a parsed key). Raises: rest_framework.exceptions.NotFound: if the string is unparseable @@ -44,7 +49,7 @@ def resolve_course_key(course_key: str) -> CourseKey: positional argument. """ try: - parsed = CourseKey.from_string(course_key) + parsed = course_key if isinstance(course_key, CourseKey) else 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): 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 index 33f9efd69ff4..c81cd2641eb3 100644 --- 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 @@ -18,7 +18,7 @@ from unittest.mock import patch from django.test import TestCase -from django.urls import reverse +from django.urls import resolve, reverse from rest_framework import status from rest_framework.test import APIClient, APITestCase @@ -291,3 +291,58 @@ def test_v0_endpoint_unaffected_by_v3_envelope(self): assert response.status_code == status.HTTP_401_UNAUTHORIZED assert "type" not in response.data assert "instance" not in response.data + + +class TestAuthoringGradingViewSetUrlStructure(APITestCase): + """The conforming courses/{course_key}/grading/ route serves the same view as the legacy one.""" + + def setUp(self): + super().setUp() + self.client = APIClient() + self.conforming_url = reverse( + "authoring_v3:course_grading", + kwargs={"course_key": COURSE_ID}, + ) + self.legacy_url = reverse( + "cms.djangoapps.contentstore:v3:authoring_grading-detail", + kwargs={"course_key": COURSE_ID}, + ) + + def test_conforming_url_reverses_to_expected_path(self): + assert self.conforming_url == f"/api/authoring/v3/courses/{COURSE_ID}/grading/" + + def test_conforming_and_legacy_routes_share_view(self): + legacy_cls = resolve(self.legacy_url).func.cls + conforming_cls = resolve(self.conforming_url).func.cls + assert conforming_cls is legacy_cls + + def test_invalid_course_key_is_404_on_conforming_route(self): + # The shared course_key converter rejects unparseable keys with a + # routing-level 404. + response = self.client.patch( + "/api/authoring/v3/courses/not-a-course-key/grading/", + data={}, format="json", + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_unauthenticated_patch_returns_401(self): + response = self.client.patch(self.conforming_url, data={}, format="json") + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + @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_on_conforming_route_updates_grading( + self, mock_exists, mock_perm, mock_update, mock_credit, # noqa: ARG002 + ): + """Same contract on the conforming mount as on the legacy one.""" + user = UserFactory.create() + self.client.force_authenticate(user=user) + response = self.client.patch( + self.conforming_url, + data={"graders": _GRADERS_PAYLOAD}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + mock_update.assert_called_once() 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 e25df0188d18..c57b9123456b 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 @@ -19,7 +19,7 @@ """ from unittest.mock import MagicMock, patch -from django.urls import reverse +from django.urls import resolve, reverse from rest_framework import status from rest_framework.test import APIClient, APITestCase @@ -378,3 +378,51 @@ def test_fields_csv_restricts_top_level_keys( assert response.status_code == status.HTTP_200_OK assert set(response.data.keys()) == {"course_id", "title"} + + +# --------------------------------------------------------------------------- +# ADR 0038 — URL-structure tests +# --------------------------------------------------------------------------- +class TestCourseDetailsViewSetUrlStructure(APITestCase): + """The conforming courses/{course_key}/details/ route serves the same view as the legacy one.""" + + def _conforming_url(self): + return reverse( + "authoring_v3:course_details", + kwargs={"course_id": TEST_COURSE_ID}, + ) + + def _legacy_url(self): + return reverse( + "cms.djangoapps.contentstore:v3:course_details-detail", + kwargs={"course_id": TEST_COURSE_ID}, + ) + + def test_conforming_url_reverses_to_expected_path(self): + assert self._conforming_url() == ( + f"/api/authoring/v3/courses/{TEST_COURSE_ID}/details/" + ) + + def test_conforming_and_legacy_routes_share_view(self): + legacy_cls = resolve(self._legacy_url()).func.cls + conforming_cls = resolve(self._conforming_url()).func.cls + assert conforming_cls is legacy_cls + + def test_invalid_course_key_is_404_on_conforming_route(self): + # The shared course_key converter rejects unparseable keys with a + # routing-level 404. + response = self.client.get("/api/authoring/v3/courses/not-a-course-key/details/") + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_unauthenticated_get_returns_401(self): + response = self.client.get(self._conforming_url()) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + @patch(MOCK_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 + """The conforming mount enforces the same authorization as the legacy one.""" + user = UserFactory.create() + self.client.force_authenticate(user=user) + response = self.client.get(self._conforming_url()) + assert response.status_code == status.HTTP_403_FORBIDDEN diff --git a/cms/djangoapps/contentstore/rest_api/v4/authoring_urls.py b/cms/djangoapps/contentstore/rest_api/v4/authoring_urls.py new file mode 100644 index 000000000000..27e3ec45533a --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v4/authoring_urls.py @@ -0,0 +1,15 @@ +"""Authoring API v4 URLs (ADR 0038 conforming mount for Contentstore v4).""" + +from django.urls import path + +from cms.djangoapps.contentstore.rest_api.v4.views import home + +app_name = "authoring_v4" + +urlpatterns = [ + path( + "courses/", + home.HomeCoursesViewSet.as_view({"get": "list"}), + name="course_list", + ), +] 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 index 4b6b89c92d6c..f9868c2788d5 100644 --- a/cms/djangoapps/contentstore/rest_api/v4/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v4/views/tests/test_home.py @@ -8,7 +8,7 @@ import ddt from django.conf import settings -from django.urls import reverse +from django.urls import resolve, reverse from rest_framework import status from rest_framework.test import APIClient, APITestCase @@ -277,3 +277,34 @@ def test_no_ordering_param_no_deprecation_header(self): response = self.client.get(self.list_url) self.assertNotIn("Deprecation", response) # noqa: PT009 + + +class TestHomeCoursesViewSetUrlStructure(APITestCase): + """The conforming /api/authoring/v4/courses/ route serves the same view as the legacy one.""" + + def test_conforming_url_reverses_to_expected_path(self): + assert reverse("authoring_v4:course_list") == "/api/authoring/v4/courses/" + + def test_conforming_and_legacy_routes_share_view(self): + legacy_cls = resolve( + reverse("cms.djangoapps.contentstore:v4:home-courses-list") + ).func.cls + conforming_cls = resolve(reverse("authoring_v4:course_list")).func.cls + assert conforming_cls is legacy_cls + + def test_unauthenticated_returns_401(self): + response = APIClient().get(reverse("authoring_v4:course_list")) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) # noqa: PT009 + + def test_authenticated_staff_gets_200(self): + """Same contract on the conforming mount as on the legacy one.""" + from django.contrib.auth import get_user_model + + User = get_user_model() + user = User.objects.create_user( + username="teststaff-authoring", password="pass", is_staff=True + ) + self.client.force_authenticate(user=user) + with patch(_MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(reverse("authoring_v4:course_list")) + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 0576a35272af..595e04884974 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -356,6 +356,13 @@ def should_show_debug_toolbar(request): # pylint: disable=missing-function-docs 'SERVE_INCLUDE_SCHEMA': False, # restrict spectacular to CMS API endpoints (cms/lib/spectacular.py): 'PREPROCESSING_HOOKS': ['cms.lib.spectacular.cms_api_filter'], + # Mark migrated legacy addresses deprecated and BFF surfaces x-internal. + # The enum hook is drf-spectacular's default, restated because setting + # this key replaces the default list. + 'POSTPROCESSING_HOOKS': [ + 'drf_spectacular.hooks.postprocess_schema_enums', + 'cms.lib.spectacular.cms_mark_migrated_paths', + ], # remove the default schema path prefix to replace it with server-specific base paths: 'SCHEMA_PATH_PREFIX': '/api/contentstore', 'SCHEMA_PATH_PREFIX_TRIM': '/api/contentstore', diff --git a/cms/envs/production.py b/cms/envs/production.py index 604d2753bccd..c3bd9d94b8d9 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -416,6 +416,13 @@ def get_env_setting(setting): 'SERVE_INCLUDE_SCHEMA': False, # restrict spectacular to CMS API endpoints (cms/lib/spectacular.py): 'PREPROCESSING_HOOKS': ['cms.lib.spectacular.cms_api_filter'], + # Mark migrated legacy addresses deprecated and BFF surfaces x-internal. + # The enum hook is drf-spectacular's default, restated because setting + # this key replaces the default list. + 'POSTPROCESSING_HOOKS': [ + 'drf_spectacular.hooks.postprocess_schema_enums', + 'cms.lib.spectacular.cms_mark_migrated_paths', + ], # remove the default schema path prefix to replace it with server-specific base paths: 'SCHEMA_PATH_PREFIX': '/api/contentstore', 'SCHEMA_PATH_PREFIX_TRIM': '/api/contentstore', diff --git a/cms/lib/spectacular.py b/cms/lib/spectacular.py index 90bce5668fec..f8b04efac2d6 100644 --- a/cms/lib/spectacular.py +++ b/cms/lib/spectacular.py @@ -2,14 +2,31 @@ import re +# Legacy addresses of APIs migrated to /api/authoring/, marked deprecated for +# their OEP-21 window. Paths are post-SCHEMA_PATH_PREFIX_TRIM. +LEGACY_MIGRATED_PATH_PREFIXES = ( + "/v1/xblock/", # → /api/authoring/v1/xblocks/ + "/v3/home/", # → /api/authoring/v3/home/ + "/v3/course_details/", # → /api/authoring/v3/courses/{course_key}/details/ + "/v3/authoring_grading/", # → /api/authoring/v3/courses/{course_key}/grading/ + "/v4/home/courses/", # → /api/authoring/v4/courses/ +) + +# BFF surfaces, marked x-internal so clients can tell them from a stable +# resource contract. Both the legacy and conforming mounts. +INTERNAL_BFF_PATH_PREFIXES = ( + "/v3/home/", + "/api/authoring/v3/home/", +) + def cms_api_filter(endpoints): """ - Pre-processing hook: keep only contentstore versioned endpoints and select - course-level endpoints. + Pre-processing hook: keep only contentstore + authoring versioned + endpoints and select course-level endpoints. """ filtered = [] - CMS_PATH_PATTERN = re.compile(r"^/api/contentstore/v\d+/") + CMS_PATH_PATTERN = re.compile(r"^/api/(contentstore|authoring)/v\d+/") for path, path_regex, method, callback in endpoints: if ( @@ -22,3 +39,23 @@ def cms_api_filter(endpoints): filtered.append((path, path_regex, method, callback)) return filtered + + +def cms_mark_migrated_paths(result, generator, request, public): # pylint: disable=unused-argument + """ + Post-processing hook (ADR 0038 / OEP-21): mark the legacy addresses of + migrated APIs ``deprecated: true`` and BFF surfaces ``x-internal``. + """ + for path, path_item in result.get("paths", {}).items(): + legacy = path.startswith(LEGACY_MIGRATED_PATH_PREFIXES) + internal = path.startswith(INTERNAL_BFF_PATH_PREFIXES) + if not (legacy or internal): + continue + for operation in path_item.values(): + if not isinstance(operation, dict): + continue + if legacy: + operation["deprecated"] = True + if internal: + operation["x-internal"] = True + return result diff --git a/cms/urls.py b/cms/urls.py index c0f96f489bb8..c52f52659155 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -13,6 +13,7 @@ from django.views.generic import RedirectView from drf_spectacular.views import SpectacularAPIView, SpectacularSwaggerView from edx_api_doc_tools import make_docs_urls +from edx_rest_framework_extensions.url_converters import register_url_converters import openedx.core.djangoapps.common_views.xblock import openedx.core.djangoapps.debug.views @@ -26,6 +27,9 @@ from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance from openedx.core.djangoapps.password_policy.forms import PasswordPolicyAwareAdminAuthForm +# Shared opaque-key path converters, registered before any pattern using them. +register_url_converters() + django_autodiscover() admin.site.site_header = _('Studio Administration') admin.site.site_title = admin.site.site_header @@ -356,6 +360,14 @@ path('api/contentstore/', include('cms.djangoapps.contentstore.rest_api.urls')) ] +# Authoring REST APIs — conforming addresses (ADR 0038), dual-mounted beside +# their legacy /api/contentstore/ routes for the OEP-21 deprecation window. +urlpatterns += [ + path('api/authoring/v1/', include('cms.djangoapps.contentstore.rest_api.v1.authoring_urls')), + path('api/authoring/v3/', include('cms.djangoapps.contentstore.rest_api.v3.authoring_urls')), + path('api/authoring/v4/', include('cms.djangoapps.contentstore.rest_api.v4.authoring_urls')), +] + # Content tagging urlpatterns += [ path('api/content_tagging/', include(('openedx.core.djangoapps.content_tagging.urls', 'content_tagging'))), diff --git a/lms/envs/common.py b/lms/envs/common.py index f7a6f15558cb..8bc7a258be8a 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2161,6 +2161,13 @@ 'VERSION': '0.1.0', 'SERVE_INCLUDE_SCHEMA': False, 'PREPROCESSING_HOOKS': ['lms.lib.spectacular.lms_api_filter'], + # Mark legacy slashless enrollment addresses deprecated. The enum hook is + # drf-spectacular's default, restated because setting this key replaces + # the default list. + 'POSTPROCESSING_HOOKS': [ + 'drf_spectacular.hooks.postprocess_schema_enums', + 'lms.lib.spectacular.lms_mark_legacy_paths_deprecated', + ], 'SCHEMA_PATH_PREFIX': '/api/enrollment', 'SCHEMA_PATH_PREFIX_TRIM': '/api/enrollment', # SERVERS is environment-specific (LMS_ROOT_URL differs per env) and is diff --git a/lms/lib/spectacular.py b/lms/lib/spectacular.py index 433b05f6db11..aea3f8c4dd0b 100644 --- a/lms/lib/spectacular.py +++ b/lms/lib/spectacular.py @@ -15,3 +15,19 @@ def lms_api_filter(endpoints): filtered.append((path, path_regex, method, callback)) return filtered + + +def lms_mark_legacy_paths_deprecated(result, generator, request, public): # pylint: disable=unused-argument + """ + Mark the legacy slashless Enrollment v2 addresses ``deprecated: true``. + + Conforming routes always end in a slash, so a slashless /v2/ path is by + construction a legacy address. Paths are post-SCHEMA_PATH_PREFIX_TRIM. + """ + for path, path_item in result.get("paths", {}).items(): + if not path.startswith("/v2/") or path.endswith("/"): + continue + for operation in path_item.values(): + if isinstance(operation, dict): + operation["deprecated"] = True + return result diff --git a/lms/urls.py b/lms/urls.py index 0765504d4080..5b6fdefa47b1 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -13,6 +13,7 @@ from drf_spectacular.views import SpectacularAPIView from edx_api_doc_tools import make_docs_urls from edx_django_utils.plugins import get_plugin_url_patterns +from edx_rest_framework_extensions.url_converters import register_url_converters from submissions import urls as submissions_urls from common.djangoapps.student import views as student_views @@ -53,6 +54,9 @@ from openedx.core.djangoapps.user_authn.views.login import redirect_to_lms_login from openedx.features.enterprise_support.api import enterprise_enabled +# Shared opaque-key path converters, registered before any pattern using them. +register_url_converters() + RESET_COURSE_DEADLINES_NAME = 'reset_course_deadlines' RENDER_XBLOCK_NAME = 'render_xblock' RENDER_VIDEO_XBLOCK_NAME = 'render_public_video_xblock' diff --git a/openedx/core/djangoapps/enrollments/v2/tests/test_views.py b/openedx/core/djangoapps/enrollments/v2/tests/test_views.py index 414aa8075926..fe54ce2dd69f 100644 --- a/openedx/core/djangoapps/enrollments/v2/tests/test_views.py +++ b/openedx/core/djangoapps/enrollments/v2/tests/test_views.py @@ -13,7 +13,7 @@ from unittest.mock import patch from django.test import override_settings -from django.urls import reverse +from django.urls import resolve, reverse from rest_framework import status from rest_framework.test import APITestCase @@ -213,7 +213,9 @@ class TestUserRolesViewAliases(APITestCase): def setUp(self): super().setUp() self.user = UserFactory.create(password="test") - self.url = reverse("v2:enrollment-v2-roles") + # Renamed from the versioned kebab-case ``enrollment-v2-roles`` + # (ADR 0038; the path is unchanged). + self.url = reverse("v2:user_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 @@ -287,3 +289,81 @@ def test_minimal_view_collapses_course_details_to_course_id(self, mock_list, moc assert {r["course_id"] for r in response.data["results"]} == { "course-v1:org+a+r", "course-v1:org+b+r", } + + +# --------------------------------------------------------------------------- +# ADR 0038 — URL-structure tests +# --------------------------------------------------------------------------- + +@skip_unless_lms +class TestEnrollmentUrlStructure(APITestCase): + """Conforming trailing-slash routes with snake_case names, beside the legacy slashless ones.""" + + USERNAME = "someone" + COURSE_ID = "course-v1:org+course+run" + + def test_conforming_urls_reverse_to_expected_paths(self): + assert reverse("v2:enrollment_admin_list") == "/api/enrollment/v2/enrollments/" + assert reverse( + "v2:enrollment_detail", + kwargs={"username": self.USERNAME, "course_id": self.COURSE_ID}, + ) == f"/api/enrollment/v2/enrollments/{self.USERNAME},{self.COURSE_ID}/" + assert reverse( + "v2:course_enrollment_detail", kwargs={"course_id": self.COURSE_ID}, + ) == f"/api/enrollment/v2/courses/{self.COURSE_ID}/" + assert reverse("v2:user_roles") == "/api/enrollment/v2/roles/" + + def test_conforming_and_legacy_routes_share_views(self): + pairs = ( + # (conforming path, legacy path) + ("/api/enrollment/v2/enrollments/", "/api/enrollment/v2/enrollments"), + ( + f"/api/enrollment/v2/enrollments/{self.USERNAME},{self.COURSE_ID}/", + f"/api/enrollment/v2/enrollment/{self.USERNAME},{self.COURSE_ID}", + ), + ( + f"/api/enrollment/v2/courses/{self.COURSE_ID}/", + f"/api/enrollment/v2/course/{self.COURSE_ID}", + ), + ) + for conforming, legacy in pairs: + assert resolve(conforming).func.cls is resolve(legacy).func.cls, ( + f"{conforming} must serve the same view as {legacy}" + ) + + def test_legacy_admin_list_optional_slash_coverage_is_preserved(self): + """ + The legacy ``^enrollments/?$`` optional-slash pattern is split into a + conforming slashed route plus a slashless legacy route: both + addresses still resolve, one route each. + """ + slashless = resolve("/api/enrollment/v2/enrollments") + slashed = resolve("/api/enrollment/v2/enrollments/") + assert slashless.func.cls is slashed.func.cls + assert slashless.url_name == "enrollment-v2-admin-list" + assert slashed.url_name == "enrollment_admin_list" + + def test_legacy_retrieve_routes_have_unique_names(self): + """ + The two legacy retrieve forms no longer share one URL name (Django + disambiguated them only by argument signature). + """ + composite = resolve( + f"/api/enrollment/v2/enrollment/{self.USERNAME},{self.COURSE_ID}" + ) + course_only = resolve(f"/api/enrollment/v2/enrollment/{self.COURSE_ID}") + assert composite.func.cls is course_only.func.cls + assert composite.url_name == "enrollment-v2-retrieve" + assert course_only.url_name == "enrollment-v2-retrieve-own" + + def test_invalid_course_key_is_404_on_conforming_route(self): + # The shared course_key converter rejects unparseable keys with a + # routing-level 404. + response = self.client.get("/api/enrollment/v2/courses/not-a-course-key/") + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_admin_list_contract_is_identical_on_both_addresses(self): + """Unauthenticated callers get the same 401 on legacy and conforming.""" + legacy = self.client.get("/api/enrollment/v2/enrollments") + conforming = self.client.get("/api/enrollment/v2/enrollments/") + assert legacy.status_code == conforming.status_code == status.HTTP_401_UNAUTHORIZED diff --git a/openedx/core/djangoapps/enrollments/v2/urls.py b/openedx/core/djangoapps/enrollments/v2/urls.py index cda839fd4319..24729b9e0b45 100644 --- a/openedx/core/djangoapps/enrollments/v2/urls.py +++ b/openedx/core/djangoapps/enrollments/v2/urls.py @@ -3,12 +3,11 @@ 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()``. +Conforming routes (ADR 0038) are dual-mounted beside the legacy slashless +routes, which keep their original names and are marked ``deprecated: true`` +in the OpenAPI schema (``lms/lib/spectacular.py``). Collapsing ``enrollment/`` +into ``enrollments/``, replacing ``unenroll`` with ``DELETE``, and addressing +the caller as ``me`` are contract changes deferred to a future version. URL surface ----------- @@ -21,12 +20,17 @@ POST /enrollment/enrollment_allowed/ DELETE /enrollment/enrollment_allowed/ -Explicit paths: +Conforming explicit paths: + GET /enrollments/ (name: enrollment_admin_list) + GET /enrollments/{username},{course_key}/ (name: enrollment_detail) + GET /courses/{course_key}/ (name: course_enrollment_detail) + GET /roles/ (name: user_roles) + +Legacy paths (deprecated, kept for their OEP-21 window): 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 /enrollment/{course_key} (name: enrollment-v2-retrieve-own) + 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 @@ -46,7 +50,33 @@ router = DefaultRouter() router.register(r"enrollment", EnrollmentViewSet, basename="enrollment") -urlpatterns = router.urls + [ +urlpatterns = [ + *router.urls, + # Conforming routes (ADR 0038). + path( + "enrollments/", + EnrollmentsAdminListView.as_view(), + name="enrollment_admin_list", + ), + path( + "enrollments/,/", + EnrollmentRetrieveView.as_view(), + name="enrollment_detail", + ), + path( + "courses//", + CourseEnrollmentDetailView.as_view(), + name="course_enrollment_detail", + ), + path("roles/", UserRolesView.as_view(), name="user_roles"), + # Legacy routes, kept for their OEP-21 window. The admin list's + # optional-slash pattern is narrowed to slashless only, since the slashed + # address is now served by the conforming route above. + re_path( + r"^enrollments$", + EnrollmentsAdminListView.as_view(), + name="enrollment-v2-admin-list", + ), re_path( r"^enrollment/{username},{course_key}$".format( # noqa: UP032 username=settings.USERNAME_PATTERN, course_key=settings.COURSE_ID_PATTERN, @@ -57,17 +87,13 @@ 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", + # Was sharing ``enrollment-v2-retrieve`` with the composite-key form + # above; nothing reverses it, so it gets its own name. + name="enrollment-v2-retrieve-own", ), 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/views.py b/openedx/core/djangoapps/enrollments/v2/views.py index 5979b0aa92f7..180089e89bee 100644 --- a/openedx/core/djangoapps/enrollments/v2/views.py +++ b/openedx/core/djangoapps/enrollments/v2/views.py @@ -468,6 +468,11 @@ def get(self, request, course_id=None, username=None): ``has_api_key`` or staff privileges raises ``NotFound`` (so the caller cannot probe for the existence of other users' enrollments). """ + # The conforming route passes a parsed CourseKey; the legacy route + # passes the raw string. Coerce to the string form used below. + if course_id is not None and not isinstance(course_id, str): + course_id = str(course_id) + if username is None: username = request.user.username @@ -610,6 +615,10 @@ def get(self, request, course_id=None): course schedule and supported enrollment modes; pass ``?include_expired=1`` to include expired enrollment modes. """ + # The conforming route passes a parsed CourseKey; the legacy route + # passes the raw string. + if course_id is not None and not isinstance(course_id, str): + course_id = str(course_id) try: course_key = CourseKey.from_string(course_id) except InvalidKeyError as exc: diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 74dcae4d3d8c..8fd1a45ca988 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -467,7 +467,7 @@ edx-django-utils==8.0.2 # ora2 # super-csv # xblocks-contrib -edx-drf-extensions==10.7.0 +edx-drf-extensions==10.8.0 # via # edx-completion # edx-enterprise diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 37e0d79208db..352848bce529 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -520,7 +520,7 @@ edx-django-utils==8.0.2 # ora2 # super-csv # xblocks-contrib -edx-drf-extensions==10.7.0 +edx-drf-extensions==10.8.0 # via # edx-completion # edx-enterprise diff --git a/uv.lock b/uv.lock index 310e9c56fb20..7bdde241397c 100644 --- a/uv.lock +++ b/uv.lock @@ -2023,7 +2023,7 @@ wheels = [ [[package]] name = "edx-drf-extensions" -version = "10.7.0" +version = "10.8.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "django", version = "4.2.30", source = { registry = "https://pypi.org/simple" }, marker = "extra == 'group-16-openedx-platform-django42'" }, @@ -2037,9 +2037,9 @@ dependencies = [ { name = "requests" }, { name = "semantic-version" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/50/06/b32d6d48415d9278c188a70da786796715ce2c4e73178f114fd498108052/edx_drf_extensions-10.7.0.tar.gz", hash = "sha256:784710bf9dc77e4234d201295963c20fd15b4e27595f1c1587b180a79e0914d4", size = 80429, upload-time = "2026-08-18T15:32:48.976Z" } +sdist = { url = "https://files.pythonhosted.org/packages/03/ce/7b348f25bb9a975171166904abe740a026ce7c6dca10e3827c927aab36cb/edx_drf_extensions-10.8.0.tar.gz", hash = "sha256:f7a6d1d0a4cfdec7c95635b0f3eb427cc473f28428bfabe3c4e3db0095feb6da", size = 99711, upload-time = "2026-09-02T20:47:28.28Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/e8/e6/03aa9fc1de473702887657c0e165798c99dee32c77a6eca5a4b2f4d8809d/edx_drf_extensions-10.7.0-py2.py3-none-any.whl", hash = "sha256:c1931816a88ac60908051e28ecb6fd18ba97cc3d6f18d61360d8eb22d3203886", size = 79474, upload-time = "2026-08-18T15:32:47.815Z" }, + { url = "https://files.pythonhosted.org/packages/ff/cd/7db09d26bb1c01ecef32d0762207930c900caf689d9bdc635c31406d4488/edx_drf_extensions-10.8.0-py2.py3-none-any.whl", hash = "sha256:853892aaba931315e82a3d3cf3cab57c5fe105f8d1760d70a45455b6e995e33b", size = 102718, upload-time = "2026-09-02T20:47:26.99Z" }, ] [[package]]