From 87f31559ba0d4a5925dc5bec653bca0beea1dbf6 Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Tue, 17 Mar 2026 23:12:12 +0500 Subject: [PATCH 1/8] docs: add ADR for standardizing serializer usage (#38139) --- .../0025-standardize-serializer-usage.rst | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 docs/decisions/0025-standardize-serializer-usage.rst diff --git a/docs/decisions/0025-standardize-serializer-usage.rst b/docs/decisions/0025-standardize-serializer-usage.rst new file mode 100644 index 000000000000..61491acc3e80 --- /dev/null +++ b/docs/decisions/0025-standardize-serializer-usage.rst @@ -0,0 +1,113 @@ +Standardize Serializer Usage Across APIs +======================================== + +:Status: Proposed +:Date: 2026-03-09 +:Deciders: API Working Group +:Technical Story: Open edX REST API Standards - Serializer standardization for consistency + +Context +------- + +Many Open edX platform API endpoints manually construct JSON responses using Python dictionaries instead of Django REST Framework (DRF) serializers. This leads to inconsistent schema responses, makes validation errors harder to manage, and creates unpredictable formats that AI and third-party systems struggle with. + +Decision +-------- + +We will standardize all Open edX REST APIs to use **DRF serializers** for request and response handling. + +Implementation requirements: + +* All API views MUST define explicit serializers for request and response handling. +* Replace manual JSON construction with serializer-based responses. +* Use serializers for both input validation and output formatting. +* Ensure serializers are properly documented with field descriptions and validation rules. +* Maintain backward compatibility for all APIs during migration. + +Relevance in edx-platform +------------------------- + +Current patterns that should be migrated: + +* **Certificates API** (``/api/certificates/v0/``) constructs JSON manually with nested dictionaries. +* **Enrollment API** endpoints manually build response objects without serializers. +* **Course API** views use hand-coded JSON responses instead of structured serializers. + +Code example (target serializer usage) +-------------------------------------- + +**Example serializer and APIView using DRF best practices:** + +.. code-block:: python + + # serializers.py + from rest_framework import serializers + + class CertificateSerializer(serializers.Serializer): + username = serializers.CharField( + help_text="The username of the certificate holder" + ) + course_id = serializers.CharField( + help_text="The course identifier" + ) + status = serializers.CharField( + help_text="The certificate status (e.g., downloadable, generating)" + ) + grade = serializers.FloatField( + help_text="The final grade achieved" + ) + + # views.py + from rest_framework.views import APIView + from rest_framework.response import Response + from rest_framework import status + + class CertificateAPIView(APIView): + def get(self, request): + data = { + "username": "john_doe", + "course_id": "course-v1:edX+DemoX+1T2024", + "status": "downloadable", + "grade": 0.95, + } + serializer = CertificateSerializer(data) + return Response(serializer.data, status=status.HTTP_200_OK) + +Consequences +------------ + +Positive +~~~~~~~~ + +* Simplifies validation and ensures consistent response contracts. +* Improves AI compatibility through predictable data structures. +* Enables automatic schema generation and documentation. +* Reduces code duplication and maintenance overhead. + +Negative / Trade-offs +~~~~~~~~~~~~~~~~~~~~~ + +* Requires refactoring existing endpoints that manually construct JSON. +* Initial development overhead for creating comprehensive serializers. +* May require updates to existing client code that expects legacy formats. + +Alternatives Considered +----------------------- + +* **Keep manual JSON construction**: rejected due to inconsistency and maintenance burden. +* **Use DRF defaults only**: rejected because explicit serializers provide better validation and documentation. +* **Use newer ways of managing API responses such as dataclasses or pydantic**: rejected due to complexity and unknowns in transitioning from two existing patterns (manual JSON and DRF serializers) to a third approach. While these python libraries offer better ergonomics, migration would require checking nested serializers, complex validation, and ModelSerializer-heavy endpoints. To move to some new format, we would want to prevent using the basic DRF Serializers any more than we do right now, but preventing new DRF serializers via linting is more complex than anticipated. This work can be revisited in the future once the platform is a bit more consistent. + +Rollout Plan +------------ + +1. Audit existing endpoints to identify those using manual JSON construction. +2. Create a library of common serializers for shared data structures. +3. Migrate high-impact endpoints first (certificates, enrollment, courses). +4. Update tests to validate serializer-based responses. +5. Update API documentation to reflect new serializer-based contracts. + +References +---------- + +* Open edX REST API Standards: "Serializer Usage" recommendations for API consistency. From 2e2ebb9900ff81fb6561d6ff760f40b61a41b15e Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Wed, 18 Mar 2026 19:14:36 +0500 Subject: [PATCH 2/8] docs: explicitly mention API versioning incase of backwards incompatible change (#38188) Co-authored-by: Muhammad Faraz Maqsood --- docs/decisions/0025-standardize-serializer-usage.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/decisions/0025-standardize-serializer-usage.rst b/docs/decisions/0025-standardize-serializer-usage.rst index 61491acc3e80..780e7ed22ecc 100644 --- a/docs/decisions/0025-standardize-serializer-usage.rst +++ b/docs/decisions/0025-standardize-serializer-usage.rst @@ -22,7 +22,7 @@ Implementation requirements: * Replace manual JSON construction with serializer-based responses. * Use serializers for both input validation and output formatting. * Ensure serializers are properly documented with field descriptions and validation rules. -* Maintain backward compatibility for all APIs during migration. +* Maintain backward compatibility for all APIs during migration. While the goal is fully compatible DRF serializers, if that is not possible and we must make a backwards incompatible change, that change MUST be handled by creating a new version of the API and transitioning to that API using the deprecation process. Relevance in edx-platform ------------------------- From 2f6ce18f0ea04b46db29c8faad2b647a36a5498e Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed Date: Mon, 13 Apr 2026 15:14:31 +0500 Subject: [PATCH 3/8] feat: add serializer_class to HomePageCoursesViewV2 (ADR 0025) Declare serializer_class = CourseHomeTabSerializerV2 on HomePageCoursesViewV2 so the serializer contract is explicit and discoverable by drf-spectacular, per ADR 0025. Co-Authored-By: Claude Sonnet 4.6 --- cms/djangoapps/contentstore/rest_api/v2/views/home.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 9d37684bfd81..188579024e5c 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -45,6 +45,7 @@ def paginate_queryset(self, queryset, request, view=None): @view_auth_classes(is_authenticated=True) class HomePageCoursesViewV2(APIView): """View for getting all courses available to the logged in user.""" + serializer_class = CourseHomeTabSerializerV2 @apidocs.schema( parameters=[ @@ -140,7 +141,7 @@ def get(self, request: Request): self.request, view=self ) - serializer = CourseHomeTabSerializerV2({ + serializer = self.serializer_class({ 'courses': courses_page, 'in_process_course_actions': in_process_course_actions, }) From 9d79272b91726daefded1787191d148393973008 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed Date: Mon, 20 Apr 2026 10:57:32 +0500 Subject: [PATCH 4/8] feat: standardize permission classes usage according to ADR 0026 --- .../contentstore/rest_api/v2/views/home.py | 8 +-- .../rest_api/v2/views/tests/test_home.py | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 188579024e5c..4f875215baa1 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -2,12 +2,13 @@ import edx_api_doc_tools as apidocs from collections import OrderedDict +from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.request import Request from rest_framework.views import APIView from rest_framework.pagination import PageNumberPagination - -from openedx.core.lib.api.view_utils import view_auth_classes +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from cms.djangoapps.contentstore.utils import get_course_context_v2 from cms.djangoapps.contentstore.rest_api.v2.serializers import CourseHomeTabSerializerV2 @@ -42,9 +43,10 @@ def paginate_queryset(self, queryset, request, view=None): return super().paginate_queryset(queryset, request, view) -@view_auth_classes(is_authenticated=True) class HomePageCoursesViewV2(APIView): """View for getting all courses available to the logged in user.""" + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated,) serializer_class = CourseHomeTabSerializerV2 @apidocs.schema( diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py index 6a51610ac9f2..44631f74e934 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -8,11 +8,14 @@ import ddt import pytz from django.conf import settings +from django.test import TestCase from django.urls import reverse from rest_framework import status +from rest_framework.test import APIClient from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.utils import reverse_course_url +from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory @@ -298,3 +301,52 @@ def test_if_empty_list_of_courses_non_staff(self, query_param, value): self.assertEqual(len(response.data["results"]["courses"]), 0) self.assertEqual(response.status_code, status.HTTP_200_OK) + + +class HomePageCoursesViewV2PermissionsTest(TestCase): + """ + ADR 0026 – permission regression tests for HomePageCoursesViewV2. + + 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:v2: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) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_authenticated_user_gets_200(self): + """ + Any authenticated user (not necessarily staff) must receive 200. + + HomePageCoursesViewV2 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) + self.assertEqual(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) + self.assertEqual(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={}) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) From 7f83e219704a441e2f36dfb3cc4ef75861eeacc2 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed Date: Wed, 29 Apr 2026 12:47:28 +0500 Subject: [PATCH 5/8] feat: apply ADR 0028 to HomePageCoursesViewV2 --- .../contentstore/rest_api/v2/urls.py | 11 ++- .../contentstore/rest_api/v2/views/home.py | 96 +++++++++++++++++++ .../v2/views/tests/test_home_viewset.py | 85 ++++++++++++++++ 3 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home_viewset.py diff --git a/cms/djangoapps/contentstore/rest_api/v2/urls.py b/cms/djangoapps/contentstore/rest_api/v2/urls.py index 893069f89aec..044e68432ade 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v2/urls.py @@ -2,12 +2,21 @@ from django.conf import settings from django.urls import path, re_path +from rest_framework.routers import DefaultRouter from cms.djangoapps.contentstore.rest_api.v2.views import downstreams, home, utils app_name = "v2" -urlpatterns = [ +# ADR 0028: HomeCoursesViewSetV2 registered via DefaultRouter. +# Generates: GET home/courses/ → home-courses-list +router = DefaultRouter() +router.register(r'home/courses', home.HomeCoursesViewSetV2, basename='home-courses') + +urlpatterns = router.urls + [ + # DEPRECATED (ADR 0028): kept for backward compatibility. + # Will be removed after one named release. + # Use GET home/courses/ (router URL name: home-courses-list) instead. path( "home/courses", home.HomePageCoursesViewV2.as_view(), diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 4f875215baa1..6735481617bb 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -2,6 +2,7 @@ import edx_api_doc_tools as apidocs from collections import OrderedDict +from rest_framework import viewsets from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.request import Request @@ -43,6 +44,101 @@ def paginate_queryset(self, queryset, request, view=None): return super().paginate_queryset(queryset, request, view) +# ADR 0028 – consolidated from HomePageCoursesViewV2 +class HomeCoursesViewSetV2(viewsets.ViewSet): + """ + ViewSet for course listing (v2). Registered via DefaultRouter (basename ``home-courses``). + + Router-generated URLs: + GET /api/contentstore/v2/home/courses/ → list + """ + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated,) + serializer_class = CourseHomeTabSerializerV2 + + def get_serializer(self, *args, **kwargs): + """Instantiate and return the configured serializer class.""" + return self.serializer_class(*args, **kwargs) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "org", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course org", + ), + apidocs.string_parameter( + "search", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course name, org, or number", + ), + apidocs.string_parameter( + "order", + apidocs.ParameterLocation.QUERY, + description="Query param to order by course name, org, or number", + ), + apidocs.string_parameter( + "active_only", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by active courses only", + ), + apidocs.string_parameter( + "archived_only", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by archived courses only", + ), + apidocs.string_parameter( + "page", + apidocs.ParameterLocation.QUERY, + description="Query param to paginate the courses", + ), + apidocs.string_parameter( + "page_size", + apidocs.ParameterLocation.QUERY, + description="Query param to set page size", + ), + ], + responses={ + 200: CourseHomeTabSerializerV2, + 401: "The requester is not authenticated.", + }, + ) + def list(self, request: Request): + """ + Get a paginated list of all courses available to the logged-in user. + + **Example Request** + + GET /api/contentstore/v2/home/courses/ + GET /api/contentstore/v2/home/courses/?org=edX + GET /api/contentstore/v2/home/courses/?search=E2E + GET /api/contentstore/v2/home/courses/?order=-org + GET /api/contentstore/v2/home/courses/?active_only=true + GET /api/contentstore/v2/home/courses/?archived_only=true + GET /api/contentstore/v2/home/courses/?page=2 + GET /api/contentstore/v2/home/courses/?page_size=20 + + **Response Values** + + If the request is successful, an HTTP 200 \"OK\" response is returned. + + The HTTP 200 response is paginated and contains ``count``, ``num_pages``, + ``next``, ``previous`` and ``results`` keys. ``results`` contains the + serialized course data. + """ + courses, in_process_course_actions = get_course_context_v2(request) + paginator = HomePageCoursesPaginator() + courses_page = paginator.paginate_queryset(courses, request, view=self) + serializer = self.get_serializer({ + 'courses': courses_page, + 'in_process_course_actions': in_process_course_actions, + }) + return paginator.get_paginated_response(serializer.data) + + +# DEPRECATED (ADR 0028): Use HomeCoursesViewSetV2 instead. +# Will be removed after one named release. +# Use GET home/courses/ (router URL name: home-courses-list) instead. class HomePageCoursesViewV2(APIView): """View for getting all courses available to the logged in user.""" authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home_viewset.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home_viewset.py new file mode 100644 index 000000000000..2101d568f385 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home_viewset.py @@ -0,0 +1,85 @@ +""" +Unit tests for HomeCoursesViewSetV2 (ADR 0028 ViewSet migration). + +All service-layer calls are mocked so these tests run without MongoDB. +""" +from unittest.mock import MagicMock, patch + +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient, APITestCase + +from cms.djangoapps.contentstore.rest_api.v2.views.home import HomeCoursesViewSetV2 +from common.djangoapps.student.tests.factories import UserFactory + +MOCK_GET_COURSE_CONTEXT_V2 = ( + 'cms.djangoapps.contentstore.rest_api.v2.views.home.get_course_context_v2' +) + + +class TestHomeCoursesViewSetV2Permissions(APITestCase): + """ + ADR 0028 – permission regression tests for HomeCoursesViewSetV2. + + URL: GET /api/contentstore/v2/home/courses/ (router name: home-courses-list) + """ + + def setUp(self): + super().setUp() + self.client = APIClient() + self.url = reverse("cms.djangoapps.contentstore:v2:home-courses-list") + + def test_unauthenticated_list_returns_401(self): + """Unauthenticated request must be rejected with 401.""" + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_authenticated_user_gets_200(self): + """Any authenticated user gets 200 — only IsAuthenticated is required.""" + user = UserFactory.create() + self.client.force_authenticate(user=user) + with patch(MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_staff_user_gets_200(self): + """Staff user also gets 200 (superset of authenticated).""" + user = UserFactory.create(is_staff=True) + self.client.force_authenticate(user=user) + with patch(MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + +class TestHomeCoursesViewSetV2Actions(APITestCase): + """ + ADR 0028 – action tests for HomeCoursesViewSetV2.list. + + Service layer (get_course_context_v2) and serializer are mocked to keep + these tests free of MongoDB and CourseOverview DB queries. + """ + + 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:v2:home-courses-list") + + @patch.object(HomeCoursesViewSetV2, 'get_serializer') + @patch(MOCK_GET_COURSE_CONTEXT_V2) + def test_list_calls_get_course_context_v2(self, mock_context, mock_get_ser): + """GET /home/courses/ calls get_course_context_v2 exactly once and returns 200.""" + mock_context.return_value = ([], []) + mock_get_ser.return_value.data = { + 'courses': [], + 'in_process_course_actions': [], + } + + response = self.client.get(self.url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + mock_context.assert_called_once() + # Response must be paginated (count / num_pages / next / previous / results) + self.assertIn('count', response.data) + self.assertIn('results', response.data) From c495b29857eda41c7704c3cf7cc29d590ed32a65 Mon Sep 17 00:00:00 2001 From: Abdul Muqadim Date: Thu, 30 Apr 2026 01:07:19 +0500 Subject: [PATCH 6/8] feat: standardize API documentation and schema coverage according to ADR 0027 --- .../contentstore/rest_api/v2/views/home.py | 135 +++++++----------- 1 file changed, 54 insertions(+), 81 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 6735481617bb..23afea990fec 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -1,7 +1,12 @@ """HomePageCoursesViewV2 APIView for getting content available to the logged in user.""" -import edx_api_doc_tools as apidocs from collections import OrderedDict + +from drf_spectacular.utils import ( + extend_schema, + OpenApiParameter, + OpenApiResponse, +) from rest_framework import viewsets from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -15,6 +20,29 @@ from cms.djangoapps.contentstore.rest_api.v2.serializers import CourseHomeTabSerializerV2 +def _query_param(name: str, description: str) -> OpenApiParameter: + """Build a string-typed, optional query parameter (preserves api-doc-tools behavior).""" + return OpenApiParameter( + name=name, + description=description, + required=False, + type=str, + location=OpenApiParameter.QUERY, + ) + + +_HOME_COURSES_QUERY_PARAMETERS = [ + _query_param("org", "Query param to filter by course org"), + _query_param("search", "Query param to filter by course name, org, or number"), + _query_param("order", "Query param to order by course name, org, or number"), + _query_param("active_only", "Query param to filter by active courses only"), + _query_param("archived_only", "Query param to filter by archived courses only"), + _query_param("page", "Query param to paginate the courses"), + _query_param("page_size", "Query param to set page size"), +] +_UNAUTHENTICATED_RESPONSE = OpenApiResponse(description="The requester is not authenticated.") + + class HomePageCoursesPaginator(PageNumberPagination): """Custom paginator for the home page courses view version 2.""" page_size_query_param = 'page_size' @@ -60,47 +88,19 @@ def get_serializer(self, *args, **kwargs): """Instantiate and return the configured serializer class.""" return self.serializer_class(*args, **kwargs) - @apidocs.schema( - parameters=[ - apidocs.string_parameter( - "org", - apidocs.ParameterLocation.QUERY, - description="Query param to filter by course org", - ), - apidocs.string_parameter( - "search", - apidocs.ParameterLocation.QUERY, - description="Query param to filter by course name, org, or number", - ), - apidocs.string_parameter( - "order", - apidocs.ParameterLocation.QUERY, - description="Query param to order by course name, org, or number", - ), - apidocs.string_parameter( - "active_only", - apidocs.ParameterLocation.QUERY, - description="Query param to filter by active courses only", - ), - apidocs.string_parameter( - "archived_only", - apidocs.ParameterLocation.QUERY, - description="Query param to filter by archived courses only", - ), - apidocs.string_parameter( - "page", - apidocs.ParameterLocation.QUERY, - description="Query param to paginate the courses", - ), - apidocs.string_parameter( - "page_size", - apidocs.ParameterLocation.QUERY, - description="Query param to set page size", - ), - ], + @extend_schema( + summary="List courses for the Studio home page (paginated)", + description=( + "Returns a paginated list of all courses available to the logged-in user, " + "with optional filtering and ordering." + ), + parameters=_HOME_COURSES_QUERY_PARAMETERS, responses={ - 200: CourseHomeTabSerializerV2, - 401: "The requester is not authenticated.", + 200: OpenApiResponse( + response=CourseHomeTabSerializerV2, + description="Paginated course list retrieved successfully.", + ), + 401: _UNAUTHENTICATED_RESPONSE, }, ) def list(self, request: Request): @@ -145,48 +145,21 @@ class HomePageCoursesViewV2(APIView): permission_classes = (IsAuthenticated,) serializer_class = CourseHomeTabSerializerV2 - @apidocs.schema( - parameters=[ - apidocs.string_parameter( - "org", - apidocs.ParameterLocation.QUERY, - description="Query param to filter by course org", - ), - apidocs.string_parameter( - "search", - apidocs.ParameterLocation.QUERY, - description="Query param to filter by course name, org, or number", - ), - apidocs.string_parameter( - "order", - apidocs.ParameterLocation.QUERY, - description="Query param to order by course name, org, or number", - ), - apidocs.string_parameter( - "active_only", - apidocs.ParameterLocation.QUERY, - description="Query param to filter by active courses only", - ), - apidocs.string_parameter( - "archived_only", - apidocs.ParameterLocation.QUERY, - description="Query param to filter by archived courses only", - ), - apidocs.string_parameter( - "page", - apidocs.ParameterLocation.QUERY, - description="Query param to paginate the courses", - ), - apidocs.string_parameter( - "page_size", - apidocs.ParameterLocation.QUERY, - description="Query param to set page size", - ), - ], + @extend_schema( + operation_id="v2_home_courses_retrieve_deprecated", + summary="List courses for the Studio home page (deprecated)", + description=( + "Deprecated. Use GET /api/contentstore/v2/home/courses/ instead." + ), + parameters=_HOME_COURSES_QUERY_PARAMETERS, responses={ - 200: CourseHomeTabSerializerV2, - 401: "The requester is not authenticated.", + 200: OpenApiResponse( + response=CourseHomeTabSerializerV2, + description="Paginated course list retrieved successfully.", + ), + 401: _UNAUTHENTICATED_RESPONSE, }, + deprecated=True, ) def get(self, request: Request): """ From 88e287bd7ed4bc28fac3a2f3baf94f4f308d799e Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed Date: Tue, 5 May 2026 11:37:01 +0500 Subject: [PATCH 7/8] apply pagination standardization to contentstore apis --- .../contentstore/rest_api/v2/views/home.py | 83 +++++++++++-------- .../rest_api/v2/views/tests/test_home.py | 82 ++++++++++++++++-- 2 files changed, 123 insertions(+), 42 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 23afea990fec..356154ae1b61 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -9,12 +9,11 @@ ) from rest_framework import viewsets from rest_framework.permissions import IsAuthenticated -from rest_framework.response import Response from rest_framework.request import Request from rest_framework.views import APIView -from rest_framework.pagination import PageNumberPagination from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser +from edx_rest_framework_extensions.paginators import DefaultPagination from cms.djangoapps.contentstore.utils import get_course_context_v2 from cms.djangoapps.contentstore.rest_api.v2.serializers import CourseHomeTabSerializerV2 @@ -43,19 +42,15 @@ def _query_param(name: str, description: str) -> OpenApiParameter: _UNAUTHENTICATED_RESPONSE = OpenApiResponse(description="The requester is not authenticated.") -class HomePageCoursesPaginator(PageNumberPagination): - """Custom paginator for the home page courses view version 2.""" - page_size_query_param = 'page_size' +class HomePageCoursesPaginator(DefaultPagination): + """ + ADR 0032 – standard pagination for the Studio home courses list (v2). - def get_paginated_response(self, data): - """Return a paginated style `Response` object for the given output data.""" - return Response(OrderedDict([ - ('count', self.page.paginator.count), - ('num_pages', self.page.paginator.num_pages), - ('next', self.get_next_link()), - ('previous', self.get_previous_link()), - ('results', data), - ])) + Extends DefaultPagination with the full 7-field response envelope: + count, num_pages, current_page, start, next, previous, results. + Handles Python ``filter`` objects returned by get_course_context_v2. + """ + page_size_query_param = 'page_size' def paginate_queryset(self, queryset, request, view=None): """ @@ -72,7 +67,6 @@ def paginate_queryset(self, queryset, request, view=None): return super().paginate_queryset(queryset, request, view) -# ADR 0028 – consolidated from HomePageCoursesViewV2 class HomeCoursesViewSetV2(viewsets.ViewSet): """ ViewSet for course listing (v2). Registered via DefaultRouter (basename ``home-courses``). @@ -136,9 +130,6 @@ def list(self, request: Request): return paginator.get_paginated_response(serializer.data) -# DEPRECATED (ADR 0028): Use HomeCoursesViewSetV2 instead. -# Will be removed after one named release. -# Use GET home/courses/ (router URL name: home-courses-list) instead. class HomePageCoursesViewV2(APIView): """View for getting all courses available to the logged in user.""" authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) @@ -176,32 +167,54 @@ def get(self, request: Request): GET /api/contentstore/v2/home/courses?page=2 GET /api/contentstore/v2/home/courses?page_size=20 + **Pagination Parameters** + + - ``page`` (int): Page number to retrieve. Default is 1. + - ``page_size`` (int): Items per page. Default is 10, max is 100. + **Response Values** If the request is successful, an HTTP 200 "OK" response is returned. - The HTTP 200 response contains a single dict that contains keys that - are the course's home. + The HTTP 200 response contains the ADR 0032 standard pagination envelope. + + **Response Envelope (ADR 0032)** + + - ``count`` (int): Total number of courses matching the filters. + - ``num_pages`` (int): Total number of pages. + - ``current_page`` (int): The current page number. + - ``start`` (int): The 0-based index of the first course on this page. + - ``next`` (str|null): URL for the next page, or null if this is the last page. + - ``previous`` (str|null): URL for the previous page, or null if this is the first page. + - ``results`` (dict): Course data for the current page. **Example Response** ```json { - "courses": [ - { - "course_key": "course-v1:edX+E2E-101+course", - "display_name": "E2E Test Course", - "lms_link": "//localhost:18000/courses/course-v1:edX+E2E-101+course", - "cms_link": "//localhost:18010/course/course-v1:edX+E2E-101+course", - "number": "E2E-101", - "org": "edX", - "rerun_link": "/course_rerun/course-v1:edX+E2E-101+course", - "run": "course", - "url": "/course/course-v1:edX+E2E-101+course", - "is_active": true - }, - ], - "in_process_course_actions": [], + "count": 2, + "num_pages": 1, + "current_page": 1, + "start": 0, + "next": null, + "previous": null, + "results": { + "courses": [ + { + "course_key": "course-v1:edX+E2E-101+course", + "display_name": "E2E Test Course", + "lms_link": "//localhost:18000/courses/course-v1:edX+E2E-101+course", + "cms_link": "//localhost:18010/course/course-v1:edX+E2E-101+course", + "number": "E2E-101", + "org": "edX", + "rerun_link": "/course_rerun/course-v1:edX+E2E-101+course", + "run": "course", + "url": "/course/course-v1:edX+E2E-101+course", + "is_active": true + } + ], + "in_process_course_actions": [] + } } ``` """ diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py index 44631f74e934..08c04c8dcc74 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -12,6 +12,7 @@ from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient +from rest_framework.test import APITestCase as DRFAPITestCase # noqa: E402 from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.contentstore.utils import reverse_course_url @@ -89,13 +90,15 @@ def test_home_page_response(self): ], "in_process_course_actions": [], } - expected_response = OrderedDict([ - ('count', 2), - ('num_pages', 1), - ('next', None), - ('previous', None), - ('results', expected_data), - ]) + expected_response = { + 'count': 2, + 'num_pages': 1, + 'current_page': 1, + 'start': 0, + 'next': None, + 'previous': None, + 'results': expected_data, + } self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertDictEqual(expected_response, response.data) @@ -303,6 +306,71 @@ def test_if_empty_list_of_courses_non_staff(self, query_param, value): self.assertEqual(response.status_code, status.HTTP_200_OK) +class TestHomePageCoursesPaginatorStructure(TestCase): + """ + ADR 0032 – structural checks for HomePageCoursesPaginator. + + Pure import-level check: no course data or MongoDB required. + Verifies the paginator inherits from DefaultPagination, satisfying the + ADR 0032 requirement that all list endpoints use the standard paginator. + """ + + def test_paginator_is_defaultpagination_subclass(self): + """HomePageCoursesPaginator must subclass DefaultPagination (not PageNumberPagination directly).""" + from edx_rest_framework_extensions.paginators import DefaultPagination + from cms.djangoapps.contentstore.rest_api.v2.views.home import HomePageCoursesPaginator + self.assertTrue( + issubclass(HomePageCoursesPaginator, DefaultPagination), + "ADR 0032: HomePageCoursesPaginator must subclass DefaultPagination", + ) + + +class TestHomePageCoursesViewV2PaginationEnvelope(DRFAPITestCase): + """ + ADR 0032 – pagination envelope regression tests for HomePageCoursesViewV2. + + Uses a plain staff user (no CourseFactory / MongoDB) so that the test can + run in any environment. The endpoint returns an empty course list for a + user with no courses, which is sufficient to verify the 7-field envelope. + + Verifies that the GET /api/contentstore/v2/home/courses endpoint returns + the full 7-field ADR 0032 response envelope: count, num_pages, current_page, + start, next, previous, results. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory.create(is_staff=True) + self.client.force_authenticate(user=self.user) + self.url = reverse("cms.djangoapps.contentstore:v2:courses") + + def test_response_includes_full_envelope(self): + """All 7 ADR 0032 envelope fields must be present in every paginated response.""" + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + for field in ('count', 'num_pages', 'current_page', 'start', 'next', 'previous', 'results'): + self.assertIn(field, response.data, f"ADR 0032: missing envelope field '{field}'") + + def test_current_page_is_one_on_first_page(self): + """current_page must equal 1 when requesting the first page.""" + response = self.client.get(self.url, {'page': 1}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['current_page'], 1) + + def test_start_is_zero_on_first_page(self): + """start must be 0 on the first page (0-based index of the first item).""" + response = self.client.get(self.url, {'page': 1}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['start'], 0) + + def test_results_contains_courses_key(self): + """results must be a dict containing the 'courses' key.""" + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIsInstance(response.data['results'], dict) + self.assertIn('courses', response.data['results']) + + class HomePageCoursesViewV2PermissionsTest(TestCase): """ ADR 0026 – permission regression tests for HomePageCoursesViewV2. From 618354aa71c7198168405e12c7d67c3acbf5084b Mon Sep 17 00:00:00 2001 From: Abdul Muqadim Date: Tue, 5 May 2026 22:53:41 +0500 Subject: [PATCH 8/8] feat: standardize filtering/sorting parameters according to ADR 0033 --- .../contentstore/rest_api/v2/views/home.py | 60 ++++++++++++-- .../v2/views/tests/test_home_viewset.py | 81 +++++++++++++++++++ cms/djangoapps/contentstore/views/course.py | 11 ++- 3 files changed, 143 insertions(+), 9 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/home.py b/cms/djangoapps/contentstore/rest_api/v2/views/home.py index 356154ae1b61..a19fda86ae3b 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/home.py @@ -10,6 +10,7 @@ from rest_framework import viewsets from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request +from rest_framework.response import Response 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 @@ -19,7 +20,7 @@ from cms.djangoapps.contentstore.rest_api.v2.serializers import CourseHomeTabSerializerV2 -def _query_param(name: str, description: str) -> OpenApiParameter: +def _query_param(name: str, description: str, deprecated: bool = False) -> OpenApiParameter: """Build a string-typed, optional query parameter (preserves api-doc-tools behavior).""" return OpenApiParameter( name=name, @@ -27,13 +28,24 @@ def _query_param(name: str, description: str) -> OpenApiParameter: required=False, type=str, location=OpenApiParameter.QUERY, + deprecated=deprecated, ) +# ADR 0033 – sorting standardization: +# ``ordering`` is the DRF-standard parameter name and is the preferred form. +# ``order`` is kept as a deprecated alias; requests using it receive a +# ``Deprecation`` HTTP header. Removal is scheduled for the named release +# tracked in ADR 0033's deprecation window (see _LEGACY_ORDER_DEPRECATION_HEADER). _HOME_COURSES_QUERY_PARAMETERS = [ _query_param("org", "Query param to filter by course org"), _query_param("search", "Query param to filter by course name, org, or number"), - _query_param("order", "Query param to order by course name, org, or number"), + _query_param("ordering", "Query param to order by course name, org, or number (DRF standard, ADR 0033)"), + _query_param( + "order", + "Deprecated alias for 'ordering' (ADR 0033). Use 'ordering' instead.", + deprecated=True, + ), _query_param("active_only", "Query param to filter by active courses only"), _query_param("archived_only", "Query param to filter by archived courses only"), _query_param("page", "Query param to paginate the courses"), @@ -41,6 +53,27 @@ def _query_param(name: str, description: str) -> OpenApiParameter: ] _UNAUTHENTICATED_RESPONSE = OpenApiResponse(description="The requester is not authenticated.") +# ADR 0033 BC strategy §2: deprecation warning header emitted when the legacy +# ``order`` query parameter is used in place of the standardized ``ordering``. +_LEGACY_ORDER_DEPRECATION_HEADER = ( + "Parameter 'order' is deprecated. Use 'ordering' instead. " + "Support will be removed in release ''." +) + + +def _maybe_set_legacy_order_deprecation_header(request: Request, response: Response) -> Response: + """ + Set the ADR 0033 ``Deprecation`` header on ``response`` when the request + used the legacy ``order`` query parameter. + + The header is emitted whenever ``order`` appears in the query string, even + if ``ordering`` was also supplied (in which case ``ordering`` wins, but the + caller should still be told that ``order`` is deprecated). + """ + if 'order' in request.query_params: + response['Deprecation'] = _LEGACY_ORDER_DEPRECATION_HEADER + return response + class HomePageCoursesPaginator(DefaultPagination): """ @@ -73,6 +106,17 @@ class HomeCoursesViewSetV2(viewsets.ViewSet): Router-generated URLs: GET /api/contentstore/v2/home/courses/ → list + + ADR 0033 compliance notes: + - Sorting uses the DRF-standard ``ordering`` parameter; ``order`` is + retained as a deprecated alias (Phase 1 of the BC strategy) and + requests using it receive a ``Deprecation`` HTTP header. + - Full migration to ``django-filter``/``DjangoFilterBackend`` is tracked + as a follow-up: the underlying data source returned by + ``get_courses_accessible_to_user`` is a Python ``filter()`` wrapper + around a hybrid queryset, so attaching a ``FilterSet`` requires + reshaping ``_accessible_courses_summary_iter`` / + ``_accessible_courses_list_from_groups`` first. """ authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) permission_classes = (IsAuthenticated,) @@ -106,7 +150,8 @@ def list(self, request: Request): GET /api/contentstore/v2/home/courses/ GET /api/contentstore/v2/home/courses/?org=edX GET /api/contentstore/v2/home/courses/?search=E2E - GET /api/contentstore/v2/home/courses/?order=-org + GET /api/contentstore/v2/home/courses/?ordering=-org + GET /api/contentstore/v2/home/courses/?order=-org # deprecated, use ?ordering= GET /api/contentstore/v2/home/courses/?active_only=true GET /api/contentstore/v2/home/courses/?archived_only=true GET /api/contentstore/v2/home/courses/?page=2 @@ -127,7 +172,8 @@ def list(self, request: Request): 'courses': courses_page, 'in_process_course_actions': in_process_course_actions, }) - return paginator.get_paginated_response(serializer.data) + response = paginator.get_paginated_response(serializer.data) + return _maybe_set_legacy_order_deprecation_header(request, response) class HomePageCoursesViewV2(APIView): @@ -161,7 +207,8 @@ def get(self, request: Request): GET /api/contentstore/v2/home/courses GET /api/contentstore/v2/home/courses?org=edX GET /api/contentstore/v2/home/courses?search=E2E - GET /api/contentstore/v2/home/courses?order=-org + GET /api/contentstore/v2/home/courses?ordering=-org + GET /api/contentstore/v2/home/courses?order=-org # deprecated, use ?ordering= GET /api/contentstore/v2/home/courses?active_only=true GET /api/contentstore/v2/home/courses?archived_only=true GET /api/contentstore/v2/home/courses?page=2 @@ -229,4 +276,5 @@ def get(self, request: Request): 'courses': courses_page, 'in_process_course_actions': in_process_course_actions, }) - return paginator.get_paginated_response(serializer.data) + response = paginator.get_paginated_response(serializer.data) + return _maybe_set_legacy_order_deprecation_header(request, response) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home_viewset.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home_viewset.py index 2101d568f385..3a94be855c55 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home_viewset.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home_viewset.py @@ -83,3 +83,84 @@ def test_list_calls_get_course_context_v2(self, mock_context, mock_get_ser): # Response must be paginated (count / num_pages / next / previous / results) self.assertIn('count', response.data) self.assertIn('results', response.data) + + +class TestHomeCoursesViewSetV2OrderingDeprecation(APITestCase): + """ + ADR 0033 – sorting standardization tests. + + Verify that: + * The new ``ordering`` parameter works and does NOT trigger the + ``Deprecation`` header. + * The legacy ``order`` parameter is still accepted (backward compat, + ADR 0033 BC strategy §1) but DOES trigger the ``Deprecation`` header + (BC strategy §2). + * When both are sent, ``ordering`` wins (its value is forwarded into + ``get_query_params_if_present``) and the ``Deprecation`` header is + still emitted because ``order`` was present in the query string. + * When neither is sent, no ``Deprecation`` header is emitted. + """ + + EXPECTED_DEPRECATION_HEADER = ( + "Parameter 'order' is deprecated. Use 'ordering' instead. " + "Support will be removed in release ''." + ) + + 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:v2:home-courses-list") + + def _patched_get_course_context(self, captured): + """ + Returns a MOCK_GET_COURSE_CONTEXT_V2 patch whose side_effect captures + the request so the test can assert which ordering value reached the + service layer via ``get_query_params_if_present``. + """ + def _capture(request): + captured['request'] = request + return ([], []) + return patch(MOCK_GET_COURSE_CONTEXT_V2, side_effect=_capture) + + def test_new_ordering_param_does_not_emit_deprecation_header(self): + """``?ordering=display_name`` returns 200 and no ``Deprecation`` header.""" + with patch(MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(self.url, {'ordering': 'display_name'}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertNotIn('Deprecation', response.headers) + + def test_legacy_order_param_emits_deprecation_header(self): + """``?order=display_name`` returns 200 AND emits the ADR 0033 header.""" + with patch(MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(self.url, {'order': 'display_name'}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.headers.get('Deprecation'), self.EXPECTED_DEPRECATION_HEADER) + + def test_ordering_wins_when_both_sent_but_header_still_emitted(self): + """ + When both params are present, ``get_query_params_if_present`` must + forward the value of ``ordering`` (not ``order``) to the service + layer, but the ``Deprecation`` header must still be emitted because + the request *contained* the deprecated param. + """ + captured = {} + with self._patched_get_course_context(captured): + response = self.client.get( + self.url, + {'ordering': 'display_name', 'order': 'org'}, + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.headers.get('Deprecation'), self.EXPECTED_DEPRECATION_HEADER) + # Confirm ordering wins inside get_query_params_if_present + from cms.djangoapps.contentstore.views.course import get_query_params_if_present + _search, order_resolved, _active, _archived = get_query_params_if_present(captured['request']) + self.assertEqual(order_resolved, 'display_name') + + def test_no_ordering_param_no_deprecation_header(self): + """Plain ``GET /home/courses/`` does not emit the ``Deprecation`` header.""" + with patch(MOCK_GET_COURSE_CONTEXT_V2, return_value=([], [])): + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertNotIn('Deprecation', response.headers) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index bf2cd9cb83d6..a8f1ae758231 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -426,22 +426,27 @@ def get_query_params_if_present(request): """ Returns the query params from request if present. + ADR 0033 – sorting standardization: prefer the DRF-standard ``ordering`` + parameter; ``order`` is accepted as a deprecated alias for backward + compatibility. When both are present, ``ordering`` wins. + Arguments: request: the request object Returns: search_query (str): any string used to filter Course Overviews based on visible fields. - order (str): any string used to order Course Overviews. + order (str): any string used to order Course Overviews. Sourced from + ``ordering`` (preferred) or ``order`` (deprecated alias). active_only (str): if not None, this value will limit the courses returned to active courses. The default value is None. archived_only (str): if not None, this value will limit the courses returned to archived courses. The default value is None. """ - allowed_query_params = ['search', 'order', 'active_only', 'archived_only'] + allowed_query_params = ['search', 'ordering', 'order', 'active_only', 'archived_only'] if not any(param in request.GET for param in allowed_query_params): return None, None, None, None search_query = request.GET.get('search') - order = request.GET.get('order') + order = request.GET.get('ordering') or request.GET.get('order') active_only = get_bool_param(request, 'active_only', None) archived_only = get_bool_param(request, 'archived_only', None) return search_query, order, active_only, archived_only