diff --git a/cms/djangoapps/contentstore/rest_api/v1/urls.py b/cms/djangoapps/contentstore/rest_api/v1/urls.py index da97bcab6a7a..685a81d778ce 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/urls.py @@ -2,7 +2,6 @@ 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 @@ -24,7 +23,6 @@ HomePageCoursesView, HomePageLibrariesView, HomePageView, - HomeViewSet, ProctoredExamSettingsView, ProctoringErrorsView, VideoDownloadView, @@ -36,13 +34,7 @@ VIDEO_ID_PATTERN = r'(?P[-\w]+)' -# ADR 0028: ViewSets registered via DefaultRouter. -router = DefaultRouter() -router.register(r'home', HomeViewSet, basename='home') - -urlpatterns = router.urls + [ - # DEPRECATED (ADR 0028): Use HomeViewSet instead (GET home/, home/courses/, home/libraries/). - # Kept as backward-compatible aliases. Remove after one named release. +urlpatterns = [ 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 4bc061c1f6a4..7654c9e0befc 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py @@ -1,7 +1,6 @@ """ Views for v1 contentstore API. """ - from .certificates import CourseCertificatesView # noqa: F401 from .course_details import CourseDetailsView # noqa: F401 from .course_index import ContainerChildrenView, CourseIndexView # noqa: F401 @@ -11,7 +10,7 @@ from .grading import CourseGradingView # noqa: F401 from .group_configurations import CourseGroupConfigurationsView # noqa: F401 from .help_urls import HelpUrlsView # noqa: F401 -from .home import HomePageCoursesView, HomePageLibrariesView, HomePageView, HomeViewSet # noqa: F401 +from .home import HomePageCoursesView, HomePageLibrariesView, HomePageView # noqa: F401 from .proctoring import ProctoredExamSettingsView, ProctoringErrorsView # noqa: F401 from .settings import CourseSettingsView # noqa: F401 from .textbooks import CourseTextbooksView # noqa: F401 diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/home.py b/cms/djangoapps/contentstore/rest_api/v1/views/home.py index 9dc34744a7fa..95723020c11f 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/home.py @@ -2,12 +2,7 @@ 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 rest_framework.views import APIView @@ -18,145 +13,6 @@ from ..serializers import CourseHomeTabSerializer, LibraryTabSerializer, StudioHomeSerializer -# ADR 0028 – consolidated from HomePageView, HomePageCoursesView, HomePageLibrariesView -class HomeViewSet(viewsets.ViewSet): - """ - ViewSet for the Studio home page. Registered via DefaultRouter (basename ``home``). - - Router-generated URLs: - GET /api/contentstore/v1/home/ → list (aggregated home context) - GET /api/contentstore/v1/home/courses/ → courses (course list only) - GET /api/contentstore/v1/home/libraries/→ libraries (library list only) - - ADR 0025 compliance notes: - - Three different serializers are returned by ``get_serializer_class()`` depending - on the action. ``serializer_class`` is set to the default (``StudioHomeSerializer``). - - All response formatting is handled by the respective serializer class. - """ - - authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) - permission_classes = (IsAuthenticated,) # ADR 0026 - serializer_class = StudioHomeSerializer # default; overridden by get_serializer_class() - - 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/v1/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/v1/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/v1/home/libraries/ - """ - library_context = get_library_context(request) - serializer = self.get_serializer(library_context) - return Response(serializer.data) - - -# DEPRECATED (ADR 0028): Use HomeViewSet instead. -# Will be removed after one named release. -# Use GET home/, home/courses/, home/libraries/ instead. @view_auth_classes(is_authenticated=True) class HomePageView(APIView): """ @@ -243,13 +99,11 @@ def get(self, request: Request): return Response(serializer.data) +@view_auth_classes(is_authenticated=True) class HomePageCoursesView(APIView): """ View for getting all courses and libraries available to the logged in user. """ - authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) - permission_classes = (IsAuthenticated,) - serializer_class = CourseHomeTabSerializer @apidocs.schema( parameters=[ apidocs.string_parameter( @@ -316,7 +170,7 @@ def get(self, request: Request): "archived_courses": archived_courses, "in_process_course_actions": in_process_course_actions, } - serializer = self.serializer_class(courses_context) + serializer = CourseHomeTabSerializer(courses_context) return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index d0a16c095eb2..ccdd906609a4 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -7,19 +7,17 @@ import ddt import pytz from django.conf import settings -from django.test import TestCase, override_settings +from django.test import override_settings from django.urls import reverse from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_content import api as content_api from organizations.tests.factories import OrganizationFactory from rest_framework import status -from rest_framework.test import APIClient from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.modulestore_migrator import api as migrator_api from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy -from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.content_libraries import api as lib_api @@ -402,54 +400,5 @@ def test_home_page_libraries_response(self): ], } - assert response.status_code == status.HTTP_200_OK - assert response.json() == expected_response - - -class HomePageCoursesViewPermissionsTest(TestCase): - """ - ADR 0026 – permission regression tests for HomePageCoursesView. - - Verifies that the explicit permission_classes = (IsAuthenticated,) enforces - the same access rules previously set by the @view_auth_classes(is_authenticated=True) - decorator. - """ - - def setUp(self): - super().setUp() - self.client = APIClient() - self.url = reverse("cms.djangoapps.contentstore:v1:courses") - self.user = UserFactory.create() - self.staff_user = UserFactory.create(is_staff=True) - - def test_unauthenticated_request_returns_401(self): - """ - Unauthenticated request (no credentials) must be rejected with 401. - - Before ADR 0026: enforced by @view_auth_classes(is_authenticated=True). - After ADR 0026: enforced by permission_classes = (IsAuthenticated,). - """ - response = self.client.get(self.url) - assert response.status_code == status.HTTP_401_UNAUTHORIZED - - def test_authenticated_user_gets_200(self): - """ - Any authenticated user (not necessarily staff) must receive 200. - - HomePageCoursesView only requires authentication — no staff role needed. - The view returns an empty course list for users with no assigned courses. - """ - self.client.force_authenticate(user=self.user) - response = self.client.get(self.url) - assert response.status_code == status.HTTP_200_OK - - def test_staff_user_gets_200(self): - """Staff user must also receive 200 (staff is a superset of authenticated).""" - self.client.force_authenticate(user=self.staff_user) - response = self.client.get(self.url) - assert response.status_code == status.HTTP_200_OK - - def test_post_by_unauthenticated_returns_401(self): - """Non-GET methods also enforce authentication — POST without credentials is 401.""" - response = self.client.post(self.url, data={}) - assert response.status_code == status.HTTP_401_UNAUTHORIZED + self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + self.assertDictEqual(expected_response, response.json()) # noqa: PT009 diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home_viewset.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home_viewset.py deleted file mode 100644 index d0e309f5377a..000000000000 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home_viewset.py +++ /dev/null @@ -1,128 +0,0 @@ -""" -Unit tests for HomeViewSet (ADR 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.v1.views.home import HomeViewSet -from common.djangoapps.student.tests.factories import UserFactory - -# ------------------------------------------------------------------ -# Mock target paths -# ------------------------------------------------------------------ -MOCK_GET_HOME_CONTEXT = ( - 'cms.djangoapps.contentstore.rest_api.v1.views.home.get_home_context' -) -MOCK_GET_COURSE_CONTEXT = ( - 'cms.djangoapps.contentstore.rest_api.v1.views.home.get_course_context' -) -MOCK_GET_LIBRARY_CONTEXT = ( - 'cms.djangoapps.contentstore.rest_api.v1.views.home.get_library_context' -) -MOCK_ORG_API = ( - 'cms.djangoapps.contentstore.rest_api.v1.views.home.org_api' -) - - -class TestHomeViewSetPermissions(APITestCase): - """ - ADR 0028 – permission regression tests for HomeViewSet. - - Verifies that IsAuthenticated enforces the same access rules previously - provided by @view_auth_classes(is_authenticated=True) on the deprecated views. - """ - - def test_unauthenticated_list_returns_401(self): - """Unauthenticated GET /home/ must return 401.""" - url = reverse('cms.djangoapps.contentstore:v1: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:v1: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:v1: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) - - # ------------------------------------------------------------------ - # list → GET /home/ - # ------------------------------------------------------------------ - - @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:v1:home-list')) - - assert response.status_code == status.HTTP_200_OK - mock_home.assert_called_once() - - # ------------------------------------------------------------------ - # courses → GET /home/courses/ - # ------------------------------------------------------------------ - - @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:v1:home-courses')) - - assert response.status_code == status.HTTP_200_OK - mock_courses.assert_called_once() - - # ------------------------------------------------------------------ - # libraries → GET /home/libraries/ - # ------------------------------------------------------------------ - - @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:v1:home-libraries')) - - assert response.status_code == status.HTTP_200_OK - mock_libs.assert_called_once()