From 9132e436df372ba296fc6a05a42492aea6596098 Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Tue, 17 Mar 2026 23:12:12 +0500 Subject: [PATCH 01/18] 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 cb313a37c03523cefb7767092e705d0fe1102296 Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Wed, 18 Mar 2026 19:14:36 +0500 Subject: [PATCH 02/18] 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 33d1fc21d2ce835529295fa95d74c8b0af30e5a0 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed <68893403+taimoor-ahmed-1@users.noreply.github.com> Date: Wed, 8 Apr 2026 19:01:08 +0500 Subject: [PATCH 03/18] docs: add ADR for standardizing permissions usage (#38187) Currently, authorization logic is implemented inconsistently across views, serializers, and custom access checks. This ADR will define a consistent approach using DRF permission classes, object-level permissions, and queryset scoping where appropriate. Co-authored-by: Taimoor Ahmed --- .../0026-standardize-permission-classes.rst | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 docs/decisions/0026-standardize-permission-classes.rst diff --git a/docs/decisions/0026-standardize-permission-classes.rst b/docs/decisions/0026-standardize-permission-classes.rst new file mode 100644 index 000000000000..659ddf5175e4 --- /dev/null +++ b/docs/decisions/0026-standardize-permission-classes.rst @@ -0,0 +1,122 @@ +Open edX ADR 0026: Standardize Permission Classes Across APIs +============================================================= + +:Status: Proposed +:Date: 2026-03-18 +:Deciders: API Working Group +:Technical Story: Open edX REST API Standards - Permission standardization for security consistency + +Context +------- + +Permissions are inconsistently applied across Open edX apps using custom decorators, inline role-based checks, and embedded authorization logic within views. This creates security gaps, makes it difficult for external systems to reliably determine access, and leads to duplicate authorization logic across multiple views. + +Decision +-------- + +We will standardize all Open edX REST APIs to use **DRF permission_classes** as the primary authorization mechanism. + +This ADR standardizes the **DRF integration surface** for authorization, not the underlying policy engine. DRF +permission classes may delegate to legacy authorization checks or newer policy engines (such as Casbin) during +phased migrations. + +Implementation requirements: + +* Use DRF permission_classes for all authorization logic instead of custom decorators. +* Create reusable permission classes for common authorization patterns (course staff, global staff, etc.). +* Replace inline role-based checks with explicit permission classes. +* Ensure permission classes are properly documented and tested. +* Maintain consistent permission patterns across similar endpoint types. +* Keep the permission backend pluggable so DRF endpoints can migrate from legacy checks to newer policy engines + without changing endpoint-level authorization structure. + +Relevance in edx-platform +------------------------- + +Current patterns that should be migrated: + +* **Enrollment API** (``/api/enrollment/v1/enrollment/{username},{course_id}``) uses custom inline role checks. +* **User Tours API** (``/api/user_tours/v1/{username}``) mixes inline checks and permission_classes. +* **Course orphan endpoints** (``^orphan/{settings.COURSE_KEY_PATTERN}$``) use functional views with inline permission logic. + +Code example (target permission usage) +-------------------------------------- + +**Example permission classes and APIView using DRF best practices:** + +.. code-block:: python + + # permissions.py + from rest_framework.permissions import BasePermission + + class IsCourseStaff(BasePermission): + """ + Allows access only to course staff members. + """ + def has_permission(self, request, view): + return request.user.is_authenticated and request.user.is_staff + + class IsEnrollmentOwnerOrStaff(BasePermission): + """ + Allows access to enrollment data for the user themselves or course staff. + """ + def has_object_permission(self, request, view, obj): + return ( + obj.user == request.user or + request.user.is_staff or + request.user.has_perm('course_staff', obj.course) + ) + + # views.py + from rest_framework.views import APIView + from rest_framework.response import Response + from rest_framework.permissions import IsAuthenticated + from .permissions import IsCourseStaff + + class EnrollmentAPIView(APIView): + permission_classes = [IsAuthenticated, IsCourseStaff] + + def get(self, request): + return Response({"detail": "Access granted"}) + +Consequences +------------ + +Positive +~~~~~~~~ + +* Improves security consistency across all APIs. +* Enhances predictability for external integrations. +* Ensures reusable, testable authorization logic. +* Simplifies security audits and permission reviews. +* Enables centralized permission management. +* Supports backend evolution (legacy checks to Casbin or other engines) without changing DRF endpoint contracts. + +Negative / Trade-offs +~~~~~~~~~~~~~~~~~~~~~ + +* Requires refactoring existing views with inline permission logic. +* May need to create custom permission classes for complex authorization scenarios. +* Initial development effort to identify and standardize permission patterns. +* Requires careful bridging during migration so permission classes and underlying engines stay behaviorally + compatible while feature flags are in use. + +Alternatives Considered +----------------------- + +* **Keep mixed permission approaches**: rejected due to security inconsistencies and maintenance burden. +* **Use only decorators**: rejected because DRF permission_classes provide better integration with the framework. + +Rollout Plan +------------ + +1. Audit existing endpoints to identify inconsistent permission patterns. +2. Create a library of standard permission classes for common use cases. +3. Migrate high-security endpoints first (enrollment, user data, course management). +4. Add comprehensive tests for permission classes and their usage. +5. Update API documentation to clearly specify permission requirements. + +References +---------- + +* Open edX REST API Standards: "Permissions" recommendations for security consistency. From fb89ac3e0046dafcd6754f29719d298911168aae Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 19 Mar 2026 15:47:45 -0400 Subject: [PATCH 04/18] docs: minor change in ADR language --- 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 780e7ed22ecc..37aad7535577 100644 --- a/docs/decisions/0025-standardize-serializer-usage.rst +++ b/docs/decisions/0025-standardize-serializer-usage.rst @@ -89,7 +89,7 @@ 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. +* Rare backward incompatibilities may require updates to existing client code that expect legacy formats. Alternatives Considered ----------------------- From 4c086e9ffa33ee5b8e6b3c91a61fc2319c343dc8 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 19 Mar 2026 15:49:40 -0400 Subject: [PATCH 05/18] fix: add s back --- 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 37aad7535577..7f3e81d35123 100644 --- a/docs/decisions/0025-standardize-serializer-usage.rst +++ b/docs/decisions/0025-standardize-serializer-usage.rst @@ -89,7 +89,7 @@ Negative / Trade-offs * Requires refactoring existing endpoints that manually construct JSON. * Initial development overhead for creating comprehensive serializers. -* Rare backward incompatibilities may require updates to existing client code that expect legacy formats. +* Rare backward incompatibilities may require updates to existing client code that expects legacy formats. Alternatives Considered ----------------------- From d7636a8b3cc2bb44e7f098cd981ca98eca39dc66 Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Tue, 21 Apr 2026 14:38:08 +0500 Subject: [PATCH 06/18] docs: migrate restful & legacy django api endpoints to standard drf viewsets (#38191) --- ...-endpoints-using-standard-drf-viewsets.rst | 200 ++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 docs/decisions/0028-migrate-restful-and-legacy-api-endpoints-using-standard-drf-viewsets.rst diff --git a/docs/decisions/0028-migrate-restful-and-legacy-api-endpoints-using-standard-drf-viewsets.rst b/docs/decisions/0028-migrate-restful-and-legacy-api-endpoints-using-standard-drf-viewsets.rst new file mode 100644 index 000000000000..102a2c374b40 --- /dev/null +++ b/docs/decisions/0028-migrate-restful-and-legacy-api-endpoints-using-standard-drf-viewsets.rst @@ -0,0 +1,200 @@ +Migrating RESTful & Legacy Django API Endpoints to Standard DRF ViewSets +======================================================================== + +:Status: Proposed +:Date: 2026-03-19 +:Deciders: API Working Group +:Technical Story: Open edX REST API Standards - RESTful & Legacy Django API endpoint structure standardization using DRF ViewSets + +Context +------- + +Many Open edX platform API endpoints are currently implemented as separate, individual +class-based or function-based (legacy) views for each HTTP action. Instead of using Django REST +Framework (DRF) ViewSets to group related operations into a single, cohesive class, each +action (list, retrieve, create, update, delete) is handled by its own standalone view. +This fragmented approach leads to significant code duplication, inconsistent behavior +across related endpoints, and an API layer that is difficult to extend or maintain. + +Decision +-------- + +We will refactor all fragmented Open edX REST API endpoints to use DRF ViewSets, +consolidating related actions into unified, well-structured view classes. + +Implementation requirements: + +* All related API actions (list, retrieve, create, update, delete) **MUST** be + consolidated into a single DRF ViewSet per resource. +* ViewSets **MUST** be registered using DRF Routers to ensure consistent, predictable + URL patterns. +* All ViewSets **MUST** use explicit serializers for both request validation and response + formatting (per ADR 0025). +* All ViewSets **MUST** optimize their ``get_queryset`` method using ``select_related`` + and ``prefetch_related`` to match the fields required by their serializers, preventing + N+1 query regressions. +* Multi-method handler functions (e.g., a single method handling DELETE, POST, and PUT) + **MUST** be refactored into properly documented, action-specific methods within a + ViewSet. +* Legacy views should be migrated to ViewSets. And **APIView** should be used for + non-resource endpoints where ViewSets are not a good fit. +* Backward compatibility **MUST** be maintained during migration, using + versioned endpoints or deprecation notices. If a backwards incompatible change is + required, 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 +------------------------- + +Current patterns that should be migrated: + +* **Enrollment API** (``/api/enrollment/v1/``) - currently split across three + independent ``APIView`` classes, each handling a distinct part of the enrollment + resource: + + * ``EnrollmentListView(APIView, ApiKeyPermissionMixIn)`` - handles + ``GET /api/enrollment/v1/enrollment`` (list all enrollments for the current user) + and ``POST /api/enrollment/v1/enrollment`` (enroll a user in a course, with support + for mode, enrollment attributes, and enterprise consent). + * ``UnenrollmentView(APIView)`` - handles + ``POST /api/enrollment/v1/unenrollment``, a privileged service-only endpoint that + unenrolls a single user from all courses as part of the user retirement pipeline. + * ``EnrollmentAllowedView(APIView)`` - handles retrieval and creation of + ``CourseEnrollmentAllowed`` records for a given user email and course ID; restricted + to admin users via ``permissions.IsAdminUser``. + * These three views operate on the same enrollment resource and should be unified into + a single ``EnrollmentViewSet``. + +* **Assets Handling Endpoints** - currently exhibit a distinct issue: + + * A single handler function mixes DELETE, POST, and PUT operations without clear + separation of concerns. + * **Resolution:** Refactor into a properly documented ``AssetsViewSet`` with distinct + action methods. + +* **Legacy Django Views** - Many endpoints still use plain Django views instead of DRF: + * Hard-coded JSON responses using ``HttpResponse(json.dumps(...))`` instead of serializers + * Manual method dispatch instead of DRF mixins and ViewSets + * Missing DRF features like automatic authentication, permission classes, and schema generation + * Inconsistent error handling and response formats + +Illustrative Example +-------------------- + +The following shows the structural pattern being replaced and the target pattern. +Full implementation details will be addressed during the migration of each endpoint. + +**Before - Enrollment API (current fragmented pattern):** + +.. code-block:: python + + # Three separate APIView classes for one logical resource + class EnrollmentListView(APIView, ApiKeyPermissionMixIn): ... + class UnenrollmentView(APIView): ... # privileged, retirement pipeline only + class EnrollmentAllowedView(APIView): ... # admin-only, CourseEnrollmentAllowed records + +**After - Consolidated into a single ViewSet:** + +.. code-block:: python + + class EnrollmentViewSet(viewsets.ViewSet): + # viewsets.ViewSet used intentionally — enrollment logic routes through + # the api module, not direct ORM, so ModelViewSet is not appropriate. + + def list(self, request): ... + def create(self, request): ... + + @action(detail=False, methods=["post"], url_path="unenrollment", + permission_classes=[ApiKeyHeaderPermission]) + def unenroll(self, request): ... # preserves privileged-only restriction + + @action(detail=False, methods=["get", "post"], url_path="allowed", + permission_classes=[permissions.IsAdminUser], + throttle_classes=[EnrollmentUserThrottle]) + def allowed(self, request): ... # reuses existing CourseEnrollmentAllowedSerializer + + router = DefaultRouter() + router.register(r"enrollment", EnrollmentViewSet, basename="enrollment") + +**Before - Assets handler (current pattern):** + +.. code-block:: python + + # Single function-based view with GET, POST, PUT, and DELETE all dispatched + # inside handle_assets(). + @login_required + @ensure_csrf_cookie + def assets_handler(request, course_key_string=None, asset_key_string=None): + return handle_assets(request, course_key_string, asset_key_string) + +**After - Dedicated ViewSet:** + +.. code-block:: python + + class AssetsViewSet(viewsets.ViewSet): + # Reuses existing asset_storage_handlers service functions. + def list(self, request, course_key_string): ... # GET - paginated asset list + def create(self, request, course_key_string): ... # POST - upload asset + def update(self, request, course_key_string, pk): ... # PUT - update lock state + def destroy(self, request, course_key_string, pk): ...# DELETE - remove asset + +Consequences +------------ + +Positive +~~~~~~~~ + +* Consistent, discoverable API structure that is easier for developers and third-party + integrators to understand and consume. +* Significant reduction in code duplication by consolidating related operations into a + single ViewSet class. +* Improved maintainability - changes to a resource's API logic are localized to one + ViewSet rather than scattered across multiple view files. +* DRF Routers automatically generate standard URL patterns, reducing manual URL + configuration and human error. +* Improved compatibility with AI systems, automated testing frameworks, and third-party + integrations that expect predictable, standardized responses. +* Enables automatic API schema generation and documentation via ``drf-spectacular``. + +Negative / Trade-offs +~~~~~~~~~~~~~~~~~~~~~ + +* Refactoring existing fragmented views into ViewSets requires non-trivial upfront + development effort. +* Existing URL patterns may change during migration, requiring updates to client-side + code, documentation, and any hardcoded references. +* Teams unfamiliar with DRF ViewSets and Routers will require onboarding before + contributing to migrated endpoints. + +Alternatives Considered +----------------------- + +* **Keep fragmented APIView classes:** Rejected. Maintains code duplication, + inconsistency, and high maintenance burden across related operations. +* **Use DRF GenericAPIView with mixins:** Partially considered but rejected as the + primary approach. While mixins reduce some duplication, they do not provide the same + level of structural consolidation or URL standardization as ViewSets with routers. + +Rollout Plan +------------ + +1. Audit existing API endpoints to identify all fragmented view patterns and legacy Django + views. +2. Prioritize high-impact resources for migration: Enrollment API, Assets endpoints, and + any other endpoints identified in the audit. +3. Refactor identified endpoints into DRF ViewSets, registered via DRF Routers. Ensure + all ViewSets use explicit serializers per ADR 0025. +4. Include a comparison of SQL query counts to ensure no performance degradation. +5. Update and expand test coverage to validate correct behavior of all refactored + ViewSet actions. +6. Publish deprecation notices for any legacy URL patterns that will be replaced, + providing clear migration guidance to internal and external API consumers. +7. Update API documentation to reflect the new ViewSet-based structure and URL patterns. + +References +---------- + +* Django REST Framework documentation - ViewSets: + https://www.django-rest-framework.org/api-guide/viewsets/ +* Django REST Framework documentation - Routers: + https://www.django-rest-framework.org/api-guide/routers/ From 4fe247848be7ef04ff3bb5cfabc1afaa3441ea55 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed Date: Tue, 31 Mar 2026 11:37:54 +0500 Subject: [PATCH 07/18] docs: Add ADR for ensuring GET requests are idempotent Add edx-platform/docs/decisions/0030-ensure-get-requests-are-idempotent.rst as an accepted ADR. Define policy that GET endpoints must be strictly read-only, with side effects moved to explicit write endpoints or async event pipelines. Include edx-platform relevance, anti-pattern vs preferred code examples, and rollout guidance for testing and migration. --- ...030-ensure-get-requests-are-idempotent.rst | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 docs/decisions/0030-ensure-get-requests-are-idempotent.rst diff --git a/docs/decisions/0030-ensure-get-requests-are-idempotent.rst b/docs/decisions/0030-ensure-get-requests-are-idempotent.rst new file mode 100644 index 000000000000..d030dab2dbb5 --- /dev/null +++ b/docs/decisions/0030-ensure-get-requests-are-idempotent.rst @@ -0,0 +1,142 @@ +Ensure GET is Idempotent +======================== + +:Status: Accepted +:Date: 2026-03-31 +:Deciders: API Working Group + +Context +======= + +Some Open edX endpoints use ``GET`` requests that mutate **domain state** as a +side-effect — for example, firing openedx-events, triggering Django signals, or +directly creating/updating transactional records (e.g. enrollments, first-access +markers). This violates REST safety/idempotency expectations and can break +caching/proxy behavior and automated clients/agents. + +Note that not every write that occurs during a ``GET`` handler is a violation. Pure +analytics writes — such as emitting a ``tracker.emit`` event, recording a Segment +event, or incrementing a read counter in a dedicated analytics store — do *not* +mutate transactional domain state and are treated differently in this ADR (see +Decision below). + +Decision +======== + +**Domain state** is defined as any data that lives in the transactional domain model +and drives application behavior — for example: enrollments, grades, user profile +fields, course access records, or any other database records that affect what a user +can see or do. Writes to domain state from a ``GET`` handler make the request +non-idempotent in ways that are difficult to audit and can cause unpredictable +behavior when responses are cached or requests are retried. + +**Non-domain writes** — such as emitting pure analytics events (``tracker.emit``, +Segment), writing to a dedicated read-analytics store, or updating a read-count +counter — do *not* modify domain state. These are explicitly **permitted** inside +``GET`` handlers, subject to the constraint that the response content must not depend +on them and no openedx-events or Django signals are involved. + +1. Treat ``GET`` as strictly read-only with respect to **domain state**: a ``GET`` handler + must not create, update, or delete records that are part of the transactional domain model + (e.g. enrollments, grades, user profile fields). +2. Move domain-state-mutating side-effects out of ``GET`` handlers: + + * **openedx-events and Django signals must not be fired from ``GET`` handlers.** + These are the primary concern: signal receivers may perform writes, trigger + downstream workflows, or update domain state in ways that are invisible to the + ``GET`` handler itself. + * Create explicit write endpoints (``POST``, ``PUT``, ``PATCH``) for state changes, + including any side-effects that need to emit openedx-events or Django signals. + * Simple telemetry writes to a **separate analytics store** (e.g. ``tracker.emit``, + Segment events, read-count increments) are acceptable inside a ``GET`` handler + **provided** the response content does not depend on them and no openedx-events + or Django signals are involved. These writes do not need to be moved to + async pipelines unless there is a specific performance or reliability reason to do so. + +3. Add regression tests to ensure ``GET`` handlers do not modify domain state. +4. Document exceptions (if any) and provide migration notes for clients. + +Relevance in edx-platform +========================= + +* **openedx-events and Django signals on read**: The primary concern is ``GET`` + handlers that fire openedx-events (e.g. ``COURSE_ENROLLMENT_CREATED``, + ``STUDENT_REGISTRATION_COMPLETED``) or Django signals (e.g. ``post_save``, + ``m2m_changed``) as a side-effect. Receivers of these events/signals can trigger + domain-state mutations that are invisible to the ``GET`` handler, making the + request non-idempotent in ways that are difficult to audit. These must be moved + to explicit write endpoints. +* **GET used with side-effects**: Various views use ``@require_GET`` while + triggering writes (e.g. tracking, first-access, or logging). Discussion views + (``lms/djangoapps/discussion/views.py``) use ``@require_GET`` for thread/topic + listing; any implicit domain-state mutation on read should be moved to separate + endpoints or async events. +* **Legacy analytics on read**: ``common/djangoapps/student`` and courseware code + sometimes emit pure analytics events (e.g. ``tracker.emit``, streak updates) in + code paths triggered by GET. Pure telemetry that does not affect domain state and + does not involve openedx-events or Django signals may remain, but anything that + can cause downstream domain writes must be decoupled. + +Code example +============ + +**Anti-pattern (GET that fires an openedx-event):** + +.. code-block:: python + + @require_GET + def get_enrollment(request, course_id): + # BAD: firing an openedx-event from a GET handler; receivers may + # perform domain-state writes invisible to this handler. + COURSE_ENROLLMENT_CHANGED.send_event( + enrollment=EnrollmentData(user=request.user, course_key=course_id) + ) + return JsonResponse(fetch_enrollment_data(...)) + +**Preferred: read-only GET + explicit write endpoint for state-changing events** + +.. code-block:: python + + @require_GET + def get_enrollment(request, course_id): + # GOOD: pure read, no signals or openedx-events fired + return Response(EnrollmentSerializer(fetch_enrollment_data(...)).data) + + @require_POST + def track_enrollment_event(request, course_id): + # Explicit write endpoint; openedx-event fired safely on POST + COURSE_ENROLLMENT_CHANGED.send_event( + enrollment=EnrollmentData(user=request.user, course_key=course_id) + ) + return Response(status=204) + +Consequences +============ + +* Pros + + * REST-compliant behavior; safer automated consumption (AI agents, integrations). + * Predictable caching/proxy semantics. + * Prevents unintended downstream side-effects from read operations (e.g. duplicate + event emissions when a response is served from cache without hitting the handler). + +* Cons / Costs + + * Requires refactoring legacy courseware/analytics endpoints that currently fire + openedx-events or Django signals on read. + * Potential behavior changes for internal systems that relied on implicit GET-triggered + events or signals. + +Implementation Notes +==================== + +* Inventory endpoints with GET side-effects, paying particular attention to those + that fire openedx-events or Django signals. +* For each, define a read-only GET representation and a separate write/track endpoint + (or async event emission) if needed. + +References +========== + +* "Non-Idempotent GET Requests" recommendation in the Open edX REST API standardization notes. +* `openedx-events `_ — Open edX architectural events. From c4ee4e0138a84f256468887057b030ac1699f85f Mon Sep 17 00:00:00 2001 From: Abdul Muqadim Date: Wed, 18 Mar 2026 17:59:18 +0500 Subject: [PATCH 08/18] docs: add ADR for standardizing API documentation and schema coverage - Propose adoption of drf-spectacular across Open edX services - Require @extend_schema decorators for all API endpoints - Document request/response schemas, status codes, and error conditions --- ...-api-documentation-and-schema-coverage.rst | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst diff --git a/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst new file mode 100644 index 000000000000..1ea2528e814b --- /dev/null +++ b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst @@ -0,0 +1,145 @@ +Open edX ADR 003: Standardize API Documentation & Schema Coverage +================================================================= + +:Status: Proposed +:Date: 2026-03-18 +:Deciders: API Working Group +:Technical Story: Open edX REST API Standards - Documentation standardization for discoverability + +Context +------- + +Many Open edX views lack proper OpenAPI schema decorators and machine-readable documentation. This makes it difficult for AI and external tools to auto-discover endpoints, creates integration challenges for external developers, and leads to the emergence of duplicate or overlapping endpoints. + +Decision +-------- + +We will standardize all Open edX REST APIs to use **drf-spectacular** with **@extend_schema decorators** for complete machine-readable documentation. + +Implementation requirements: + +* Use drf-spectacular for all API endpoints with @extend_schema decorators. +* Document request/response schemas, status codes, and error conditions. +* Include comprehensive descriptions and examples for complex endpoints. +* Ensure all endpoints have machine-readable OpenAPI coverage. +* Maintain consistent documentation patterns across services. + +Relevance in edx-platform +------------------------- + +Current patterns that should be migrated: + +* **Discussion topics API** (``^v0/course/{settings.COURSE_KEY_PATTERN}/sync_discussion_topics$``) lacks OpenAPI-compliant schema and has incomplete documentation. +* **Course content APIs** have missing or incomplete schema definitions. +* **User management endpoints** lack proper request/response documentation. + +Code example (target documentation usage) +----------------------------------------- + +**Example APIView with comprehensive drf-spectacular documentation:** + +.. code-block:: python + + # views.py + from drf_spectacular.utils import extend_schema, OpenApiRequest, OpenApiResponse + from rest_framework.views import APIView + from rest_framework.response import Response + from .serializers import TopicSerializer + + class ExampleTopicAPIView(APIView): + """ + API endpoint for managing discussion topics. + """ + + @extend_schema( + summary="List discussion topics", + description="Returns a paginated list of discussion topics for the specified course.", + responses={ + 200: OpenApiResponse( + response=TopicSerializer(many=True), + description="List of discussion topics retrieved successfully" + ), + 400: OpenApiResponse( + description="Bad request - invalid parameters" + ), + 403: OpenApiResponse( + description="Permission denied - user lacks access" + ), + 404: OpenApiResponse( + description="Course not found" + ), + }, + parameters=[ + { + "name": "course_id", + "in": "path", + "required": True, + "schema": {"type": "string"}, + "description": "Course identifier" + } + ] + ) + def get(self, request): + """Retrieve discussion topics for the current course.""" + return Response({"results": []}) + + @extend_schema( + summary="Create discussion topic", + description="Creates a new discussion topic in the specified course.", + request=OpenApiRequest(request=TopicSerializer), + responses={ + 201: OpenApiResponse( + response=TopicSerializer, + description="Discussion topic created successfully" + ), + 400: OpenApiResponse( + description="Bad request - invalid data" + ), + 403: OpenApiResponse( + description="Permission denied" + ), + } + ) + def post(self, request): + """Create a new discussion topic.""" + return Response({"detail": "Topic created"}, status=201) + +Consequences +------------ + +Positive +~~~~~~~~ + +* Provides machine-readable schemas for external systems integrations. +* Improves developer experience through standardized OpenAPI documentation. +* Enables automatic client SDK generation. +* Reduces duplicate endpoints through better discoverability. +* Facilitates API testing and validation. + +Negative / Trade-offs +~~~~~~~~~~~~~~~~~~~~~ + +* Requires initial effort to document existing endpoints. +* Ongoing maintenance to keep documentation in sync with code changes. +* Learning curve for teams unfamiliar with drf-spectacular decorators. + +Alternatives Considered +----------------------- + +* **Keep minimal documentation**: rejected due to poor discoverability and integration challenges. +* **Use separate documentation files**: rejected because inline decorators provide better maintainability. + +Rollout Plan +------------ + +1. Configure drf-spectacular across all Open edX services. +2. Create documentation templates and guidelines for common endpoint patterns. +3. Audit existing endpoints and prioritize high-impact APIs for documentation. +4. Implement automated testing to ensure schema completeness. +5. Set up continuous integration to validate documentation quality. +6. Publish and maintain OpenAPI specifications for all services. + +References +---------- + +* Open edX REST API Standards: "API Documentation & Schema Coverage" recommendations for discoverability. From f906fcac2563c002bc4c514fc3afbd4dd2f6f10a Mon Sep 17 00:00:00 2001 From: Abdul Muqadim Date: Sat, 21 Mar 2026 02:07:08 +0500 Subject: [PATCH 09/18] docs: remove incorrect ADR number --- .../0027-standardize-api-documentation-and-schema-coverage.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst index 1ea2528e814b..496158190497 100644 --- a/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst +++ b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst @@ -1,4 +1,4 @@ -Open edX ADR 003: Standardize API Documentation & Schema Coverage +Standardize API Documentation & Schema Coverage ================================================================= :Status: Proposed From 547c6254b4185be954f12f72d6a8fdcdb4e4e1de Mon Sep 17 00:00:00 2001 From: Abdul Muqadim Date: Mon, 20 Apr 2026 00:34:48 +0500 Subject: [PATCH 10/18] docs: address api-doc-tools deprecation in ADR per review feedback - Add context explaining what api-doc-tools is and its relationship to drf-yasg - Document deprecation and archival of api-doc-tools as a consequence - Add migration guide mapping api-doc-tools decorators and URL helpers to their drf-spectacular equivalents - Add rejected alternative for updating api-doc-tools internals - Add rollout step for final archival cutover Closes review comment by @feanil --- ...-api-documentation-and-schema-coverage.rst | 61 ++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst index 496158190497..9fc686f6c940 100644 --- a/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst +++ b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst @@ -9,7 +9,23 @@ Standardize API Documentation & Schema Coverage Context ------- -Many Open edX views lack proper OpenAPI schema decorators and machine-readable documentation. This makes it difficult for AI and external tools to auto-discover endpoints, creates integration challenges for external developers, and leads to the emergence of duplicate or overlapping endpoints. +Many Open edX views lack proper OpenAPI schema decorators and machine-readable +documentation. This makes it difficult for AI and external tools to +auto-discover endpoints, creates integration challenges for external developers, +and leads to the emergence of duplicate or overlapping endpoints. + +The platform currently uses `api-doc-tools`_ (``edx-api-doc-tools``), an +Open edX-maintained library that wraps `drf-yasg`_ and exposes the ``@schema``, +``@schema_for``, ``parameter``, ``query_parameter``, and ``path_parameter`` +decorators, along with URL helpers (``make_api_info`` / ``make_docs_urls``) that +serve a Swagger UI at ``/api-docs``. drf-yasg only targets OpenAPI 2.0 and is +largely unmaintained upstream. Replacing drf-yasg inside api-doc-tools with +drf-spectacular was investigated and found infeasible due to the significant +divergence in their decorator contracts. Direct adoption of drf-spectacular is +therefore the right path forward. + +.. _api-doc-tools: https://github.com/openedx/api-doc-tools/ +.. _drf-yasg: https://github.com/axnsan12/drf-yasg Decision -------- @@ -122,12 +138,51 @@ Negative / Trade-offs * Requires initial effort to document existing endpoints. * Ongoing maintenance to keep documentation in sync with code changes. * Learning curve for teams unfamiliar with drf-spectacular decorators. +* **api-doc-tools and drf-yasg will be deprecated and archived.** Existing + usages across edx-platform and downstream services must be migrated. See + the migration guide below. Alternatives Considered ----------------------- * **Keep minimal documentation**: rejected due to poor discoverability and integration challenges. * **Use separate documentation files**: rejected because inline decorators provide better maintainability. +* **Update api-doc-tools to use drf-spectacular internally**: investigated and + found infeasible — the decorator contracts of the two libraries diverge + significantly, making a compatibility shim as costly as a direct migration. + +Migration: api-doc-tools to drf-spectacular +-------------------------------------------- + +**What is api-doc-tools?** +``api-doc-tools`` is the current Open edX documentation library, built on top of +``drf-yasg``. It provides the ``@schema`` / ``@schema_for`` decorators and the +``make_docs_urls`` URL helper used across edx-platform today. + +**Decorator mapping:** + ++----------------------------------------------+-------------------------------------------------------+ +| api-doc-tools (old) | drf-spectacular (new) | ++==============================================+=======================================================+ +| ``@schema(parameters=[...], responses={...})``| ``@extend_schema(parameters=[...], responses={...})`` | ++----------------------------------------------+-------------------------------------------------------+ +| ``@schema_for("list", "docstring", ...)`` | ``@extend_schema_view(list=extend_schema(...))`` | ++----------------------------------------------+-------------------------------------------------------+ +| ``parameter(name, type, desc)`` | ``OpenApiParameter(name, type, desc)`` | ++----------------------------------------------+-------------------------------------------------------+ +| ``query_parameter(name, type, desc)`` | ``OpenApiParameter(name, type, desc,`` | +| | ``location=OpenApiParameter.QUERY)`` | ++----------------------------------------------+-------------------------------------------------------+ +| ``path_parameter(name, type, desc)`` | ``OpenApiParameter(name, type, desc,`` | +| | ``location=OpenApiParameter.PATH)`` | ++----------------------------------------------+-------------------------------------------------------+ +| ``make_docs_urls(make_api_info(...))`` | ``SpectacularAPIView`` + ``SpectacularSwaggerView`` | ++----------------------------------------------+-------------------------------------------------------+ + +Endpoints should be migrated incrementally. New endpoints must use +drf-spectacular directly. Once all usages are migrated, ``edx-api-doc-tools`` +and ``drf-yasg`` will be removed from requirements and the ``api-doc-tools`` +repository will be archived. Rollout Plan ------------ @@ -138,8 +193,12 @@ Rollout Plan 4. Implement automated testing to ensure schema completeness. 5. Set up continuous integration to validate documentation quality. 6. Publish and maintain OpenAPI specifications for all services. +7. Once migration is complete, archive ``api-doc-tools`` and remove ``drf-yasg`` + from service requirements. References ---------- * Open edX REST API Standards: "API Documentation & Schema Coverage" recommendations for discoverability. +* `api-doc-tools repository `_ +* `drf-spectacular documentation `_ \ No newline at end of file From 2f47231437accaf0ff85eb176667064b081c63d8 Mon Sep 17 00:00:00 2001 From: Abdul Muqadim Date: Sun, 26 Apr 2026 13:36:38 +0500 Subject: [PATCH 11/18] docs: expand ADR-0027 with api-doc-tools deprecation and drf-yasg incompatibility analysis Address review feedback on FC-0118 ADR 0027: - Add context paragraph explaining what api-doc-tools is (drf-yasg shim, decorators it provides, schema view, OpenAPI 2.0 output) - Document deprecation of api-doc-tools and drf-yasg as a consequence, including transition-window behavior - Add detailed 8-point incompatibility analysis explaining why drf-yasg cannot be replaced with drf-spectacular inside api-doc-tools (recorded in the ADR itself for future reference) - Add migration plan for existing api-doc-tools consumers with concrete decorator/import/setting mapping - Update Rollout Plan to track api-doc-tools removal - Add references to drf-spectacular migration guide, drf-yasg upstream status, and api-doc-tools repository --- ...-api-documentation-and-schema-coverage.rst | 331 +++++++++++------- 1 file changed, 209 insertions(+), 122 deletions(-) diff --git a/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst index 9fc686f6c940..285b848ef23d 100644 --- a/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst +++ b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst @@ -11,26 +11,31 @@ Context Many Open edX views lack proper OpenAPI schema decorators and machine-readable documentation. This makes it difficult for AI and external tools to -auto-discover endpoints, creates integration challenges for external developers, -and leads to the emergence of duplicate or overlapping endpoints. - -The platform currently uses `api-doc-tools`_ (``edx-api-doc-tools``), an -Open edX-maintained library that wraps `drf-yasg`_ and exposes the ``@schema``, -``@schema_for``, ``parameter``, ``query_parameter``, and ``path_parameter`` -decorators, along with URL helpers (``make_api_info`` / ``make_docs_urls``) that -serve a Swagger UI at ``/api-docs``. drf-yasg only targets OpenAPI 2.0 and is -largely unmaintained upstream. Replacing drf-yasg inside api-doc-tools with -drf-spectacular was investigated and found infeasible due to the significant -divergence in their decorator contracts. Direct adoption of drf-spectacular is -therefore the right path forward. - -.. _api-doc-tools: https://github.com/openedx/api-doc-tools/ -.. _drf-yasg: https://github.com/axnsan12/drf-yasg +auto-discover endpoints, creates integration challenges for external +developers, and leads to the emergence of duplicate or overlapping endpoints. + +Today, the documentation that does exist in the platform is largely produced +through `api-doc-tools `_, an Open +edX-maintained shim over +`drf-yasg `_. ``api-doc-tools`` provides +simplified decorators (``@schema``, ``@schema_for``) that wrap +``@swagger_auto_schema``, helper functions such as ``parameter()`` for +declaring query/path parameters, and a configured schema view (typically +served at ``/api-docs.yaml`` and ``/api-docs``). Internally it emits OpenAPI +2.0 (Swagger 2.0). + +``drf-yasg`` itself is in light maintenance mode and explicitly will not gain +OpenAPI 3.x support (per its upstream README). As a result, neither +``api-doc-tools`` nor ``drf-yasg`` is a viable long-term home for the +platform's API documentation. Decision -------- -We will standardize all Open edX REST APIs to use **drf-spectacular** with **@extend_schema decorators** for complete machine-readable documentation. +We will standardize all Open edX REST APIs to use **drf-spectacular** with +**@extend_schema decorators** for complete machine-readable documentation, and +we will deprecate ``api-doc-tools`` (and the platform's transitive dependency +on ``drf-yasg``) as part of the same effort. Implementation requirements: @@ -39,6 +44,8 @@ Implementation requirements: * Include comprehensive descriptions and examples for complex endpoints. * Ensure all endpoints have machine-readable OpenAPI coverage. * Maintain consistent documentation patterns across services. +* Migrate endpoints currently documented via ``api-doc-tools`` directly to + ``@extend_schema`` rather than through a wrapper. Relevance in edx-platform ------------------------- @@ -56,69 +63,69 @@ Code example (target documentation usage) .. code-block:: python - # views.py - from drf_spectacular.utils import extend_schema, OpenApiRequest, OpenApiResponse - from rest_framework.views import APIView - from rest_framework.response import Response - from .serializers import TopicSerializer - - class ExampleTopicAPIView(APIView): - """ - API endpoint for managing discussion topics. - """ - - @extend_schema( - summary="List discussion topics", - description="Returns a paginated list of discussion topics for the specified course.", - responses={ - 200: OpenApiResponse( - response=TopicSerializer(many=True), - description="List of discussion topics retrieved successfully" - ), - 400: OpenApiResponse( - description="Bad request - invalid parameters" - ), - 403: OpenApiResponse( - description="Permission denied - user lacks access" - ), - 404: OpenApiResponse( - description="Course not found" - ), - }, - parameters=[ - { - "name": "course_id", - "in": "path", - "required": True, - "schema": {"type": "string"}, - "description": "Course identifier" - } - ] - ) - def get(self, request): - """Retrieve discussion topics for the current course.""" - return Response({"results": []}) - - @extend_schema( - summary="Create discussion topic", - description="Creates a new discussion topic in the specified course.", - request=OpenApiRequest(request=TopicSerializer), - responses={ - 201: OpenApiResponse( - response=TopicSerializer, - description="Discussion topic created successfully" - ), - 400: OpenApiResponse( - description="Bad request - invalid data" - ), - 403: OpenApiResponse( - description="Permission denied" - ), - } - ) - def post(self, request): - """Create a new discussion topic.""" - return Response({"detail": "Topic created"}, status=201) + # views.py + from drf_spectacular.utils import extend_schema, OpenApiRequest, OpenApiResponse + from rest_framework.views import APIView + from rest_framework.response import Response + from .serializers import TopicSerializer + + class ExampleTopicAPIView(APIView): + """ + API endpoint for managing discussion topics. + """ + + @extend_schema( + summary="List discussion topics", + description="Returns a paginated list of discussion topics for the specified course.", + responses={ + 200: OpenApiResponse( + response=TopicSerializer(many=True), + description="List of discussion topics retrieved successfully" + ), + 400: OpenApiResponse( + description="Bad request - invalid parameters" + ), + 403: OpenApiResponse( + description="Permission denied - user lacks access" + ), + 404: OpenApiResponse( + description="Course not found" + ), + }, + parameters=[ + { + "name": "course_id", + "in": "path", + "required": True, + "schema": {"type": "string"}, + "description": "Course identifier" + } + ] + ) + def get(self, request): + """Retrieve discussion topics for the current course.""" + return Response({"results": []}) + + @extend_schema( + summary="Create discussion topic", + description="Creates a new discussion topic in the specified course.", + request=OpenApiRequest(request=TopicSerializer), + responses={ + 201: OpenApiResponse( + response=TopicSerializer, + description="Discussion topic created successfully" + ), + 400: OpenApiResponse( + description="Bad request - invalid data" + ), + 403: OpenApiResponse( + description="Permission denied" + ), + } + ) + def post(self, request): + """Create a new discussion topic.""" + return Response({"detail": "Topic created"}, status=201) Consequences ------------ @@ -131,6 +138,7 @@ Positive * Enables automatic client SDK generation. * Reduces duplicate endpoints through better discoverability. * Facilitates API testing and validation. +* Removes the platform's dependency on an unmaintained OpenAPI 2.0 library (``drf-yasg``). Negative / Trade-offs ~~~~~~~~~~~~~~~~~~~~~ @@ -138,51 +146,128 @@ Negative / Trade-offs * Requires initial effort to document existing endpoints. * Ongoing maintenance to keep documentation in sync with code changes. * Learning curve for teams unfamiliar with drf-spectacular decorators. -* **api-doc-tools and drf-yasg will be deprecated and archived.** Existing - usages across edx-platform and downstream services must be migrated. See - the migration guide below. +* Existing consumers of ``api-doc-tools`` decorators (``@schema``, ``@schema_for``, ``parameter()``) will need to be migrated to ``@extend_schema``. + +Deprecation of api-doc-tools and drf-yasg +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +As a direct consequence of this decision: + +* ``api-doc-tools`` will be deprecated and, once it has no remaining consumers in the Open edX platform or community, archived. +* ``drf-yasg`` will be removed from the platform's dependency set as part of the same migration. +* During the transition window, ``api-doc-tools`` will be kept functional so that already-documented endpoints continue to render. New endpoints, however, must use ``drf-spectacular`` directly. Alternatives Considered ----------------------- * **Keep minimal documentation**: rejected due to poor discoverability and integration challenges. * **Use separate documentation files**: rejected because inline decorators provide better maintainability. -* **Update api-doc-tools to use drf-spectacular internally**: investigated and - found infeasible — the decorator contracts of the two libraries diverge - significantly, making a compatibility shim as costly as a direct migration. - -Migration: api-doc-tools to drf-spectacular --------------------------------------------- - -**What is api-doc-tools?** -``api-doc-tools`` is the current Open edX documentation library, built on top of -``drf-yasg``. It provides the ``@schema`` / ``@schema_for`` decorators and the -``make_docs_urls`` URL helper used across edx-platform today. - -**Decorator mapping:** - -+----------------------------------------------+-------------------------------------------------------+ -| api-doc-tools (old) | drf-spectacular (new) | -+==============================================+=======================================================+ -| ``@schema(parameters=[...], responses={...})``| ``@extend_schema(parameters=[...], responses={...})`` | -+----------------------------------------------+-------------------------------------------------------+ -| ``@schema_for("list", "docstring", ...)`` | ``@extend_schema_view(list=extend_schema(...))`` | -+----------------------------------------------+-------------------------------------------------------+ -| ``parameter(name, type, desc)`` | ``OpenApiParameter(name, type, desc)`` | -+----------------------------------------------+-------------------------------------------------------+ -| ``query_parameter(name, type, desc)`` | ``OpenApiParameter(name, type, desc,`` | -| | ``location=OpenApiParameter.QUERY)`` | -+----------------------------------------------+-------------------------------------------------------+ -| ``path_parameter(name, type, desc)`` | ``OpenApiParameter(name, type, desc,`` | -| | ``location=OpenApiParameter.PATH)`` | -+----------------------------------------------+-------------------------------------------------------+ -| ``make_docs_urls(make_api_info(...))`` | ``SpectacularAPIView`` + ``SpectacularSwaggerView`` | -+----------------------------------------------+-------------------------------------------------------+ - -Endpoints should be migrated incrementally. New endpoints must use -drf-spectacular directly. Once all usages are migrated, ``edx-api-doc-tools`` -and ``drf-yasg`` will be removed from requirements and the ``api-doc-tools`` -repository will be archived. +* **Replace drf-yasg with drf-spectacular inside api-doc-tools**: rejected. The two libraries are not drop-in compatible at any meaningful layer; the wrapper would effectively need to be rewritten, and existing consumers would still require migration. See "Why we are not replacing drf-yasg inside api-doc-tools" below for the detailed analysis. + +Why we are not replacing drf-yasg inside api-doc-tools +------------------------------------------------------ + +The most attractive alternative on the surface was to update +``api-doc-tools`` to use ``drf-spectacular`` under the hood instead of +``drf-yasg``, leaving existing ``@schema`` / ``@schema_for`` / ``parameter()`` +call sites untouched. After investigation this path is not viable. The +specific incompatibilities, recorded here for future reference, are: + +1. **Different OpenAPI specification versions.** ``drf-yasg`` only emits + OpenAPI 2.0 (Swagger 2.0); ``drf-spectacular`` emits OpenAPI 3.0/3.1. The + two output documents are structurally different, so any downstream consumer + of the generated ``/api-docs.yaml`` (codegen tools, AI tooling, external + integrators) would need to be updated regardless of whether the wrapper + survives. ``drf-yasg``'s upstream documentation explicitly states that + OpenAPI 3.x support will not be added. + +2. **Different decorator APIs.** ``api-doc-tools``'s ``@schema`` and + ``@schema_for`` wrap ``@swagger_auto_schema``. The ``drf-spectacular`` + equivalent is ``@extend_schema``, with different argument names (e.g. + ``operation_description`` → ``description``), different ``summary`` vs. + ``description`` semantics, and different supported keyword arguments + (``exclude``, ``versions``, ``examples``, etc.). This is not a 1:1 rename — + every wrapper call site would need to be re-translated. + +3. **Different type and parameter primitives.** ``parameter()`` in + ``api-doc-tools`` relies on ``drf_yasg.openapi.TYPE_*`` / ``FORMAT_*`` + constants and the ``drf_yasg.openapi.Schema`` class. ``drf-spectacular`` + uses the ``OpenApiTypes`` enum and plain Python types/``dict`` s, and its + ``OpenApiParameter`` has a different shape (``format`` is folded into + ``type``, ``many=True`` replaces drf-yasg's ``Items`` class, and + ``IN_BODY`` / ``IN_FORM`` location constants have no direct equivalent — + request bodies are handled via + ``@extend_schema(request={"": ...})``). + +4. **Different docstring conventions.** ``drf-yasg`` treats the first line of + a docstring as the operation ``summary`` and the remainder as the + ``description``. ``drf-spectacular`` uses the entire docstring as the + ``description`` and requires ``summary`` to be passed explicitly. Every + docstring-documented endpoint in the platform would render differently + after a silent backend swap. + +5. **Different schema view, settings, and UI integration.** ``drf-yasg`` + exposes the schema via ``drf_yasg.views.get_schema_view`` and is + configured through ``SWAGGER_SETTINGS`` / ``REDOC_SETTINGS``. + ``drf-spectacular`` uses ``SpectacularAPIView`` / ``SpectacularSwaggerView`` + / ``SpectacularRedocView`` configured through ``SPECTACULAR_SETTINGS``. UI + assets are also handled differently: ``drf-yasg`` ships Swagger UI and + Redoc internally, whereas ``drf-spectacular`` serves them from a CDN or + via the optional ``drf-spectacular-sidecar`` package. + +6. **Different authentication scheme handling.** ``drf-yasg`` requires manual + security scheme definitions. ``drf-spectacular`` auto-generates security + definitions for built-in DRF authenticators and popular third-party + packages, with ``OpenApiAuthenticationExtension`` as the hook for custom + classes. Auth-related schema configuration in ``api-doc-tools`` consumers + would need to be re-expressed. + +7. **Different extension and customization architectures.** Custom schema + generation in ``drf-yasg`` is done by subclassing + ``OpenAPISchemaGenerator`` and ``SwaggerAutoSchema``. In + ``drf-spectacular`` it is done via ``OpenApiSerializerExtension``, + ``OpenApiSerializerFieldExtension``, ``OpenApiAuthenticationExtension``, + etc. — different inheritance hierarchies and different hook signatures, so + no custom generator code can be carried over unchanged. + +8. **AutoSchema generation differs even on identical inputs.** Given the + same DRF serializers and viewsets, the two libraries produce materially + different schemas in practice — common discrepancies include handling of + ``read_only`` / ``write_only`` fields, nullable fields, nested + serializers, and custom ``@action`` endpoints. A drop-in engine swap would + silently change generated documentation for every endpoint without any + corresponding code change, which is unacceptable as an "invisible" + upgrade. + +Because of the above, swapping the engine inside ``api-doc-tools`` would not +save existing endpoints from migration, every consumer of ``@schema``, +``@schema_for``, ``parameter()``, and ``get_schema_view`` would still need to +change, while also locking the project into indefinitely maintaining a +wrapper aligned with ``drf-spectacular``'s evolving API. A direct migration +to ``drf-spectacular`` is therefore preferred. + +Migration Plan for api-doc-tools consumers +------------------------------------------ + +Existing usages of ``api-doc-tools`` will be migrated to ``drf-spectacular`` +directly, using the following mapping as a starting point: + +* ``@schema(...)`` and ``@schema_for(...)`` → ``@extend_schema(...)`` +* ``parameter(name, type, description)`` → ``OpenApiParameter(name=..., type=..., description=..., location=OpenApiParameter.QUERY|PATH|HEADER|COOKIE)`` +* ``responses={status: SerializerOrString}`` → ``responses={status: OpenApiResponse(response=Serializer, description=...)}`` +* ``get_schema_view`` from ``drf_yasg.views`` → ``SpectacularAPIView`` / ``SpectacularSwaggerView`` / ``SpectacularRedocView`` from ``drf_spectacular.views`` +* ``SWAGGER_SETTINGS`` / ``REDOC_SETTINGS`` → ``SPECTACULAR_SETTINGS`` + +Approach: + +1. Stand up ``drf-spectacular`` alongside the existing ``api-doc-tools`` + setup so both can co-exist during the transition. +2. Migrate endpoints incrementally, prioritizing high-traffic and + externally-consumed APIs. +3. Once a service has no remaining ``api-doc-tools`` imports, remove the + dependency from that service. +4. When no service in the platform depends on ``api-doc-tools``, deprecate + the package on its own repository and schedule archival. Rollout Plan ------------ @@ -193,12 +278,14 @@ Rollout Plan 4. Implement automated testing to ensure schema completeness. 5. Set up continuous integration to validate documentation quality. 6. Publish and maintain OpenAPI specifications for all services. -7. Once migration is complete, archive ``api-doc-tools`` and remove ``drf-yasg`` - from service requirements. +7. Track and complete migration of ``api-doc-tools`` consumers per the + migration plan above; remove ``api-doc-tools`` and ``drf-yasg`` from the + dependency set once migration is complete. References ---------- * Open edX REST API Standards: "API Documentation & Schema Coverage" recommendations for discoverability. -* `api-doc-tools repository `_ -* `drf-spectacular documentation `_ \ No newline at end of file +* drf-spectacular migration guide: https://drf-spectacular.readthedocs.io/en/stable/drf_yasg.html +* drf-yasg OpenAPI 3.0 status: https://drf-yasg.readthedocs.io/en/stable/readme.html +* api-doc-tools repository: https://github.com/openedx/api-doc-tools From fd71d6321282712f84eae633c4766db72aa5675e Mon Sep 17 00:00:00 2001 From: Abdul Muqadim Date: Mon, 27 Apr 2026 21:08:19 +0500 Subject: [PATCH 12/18] chore: fix edx-mantained to edX-platform --- ...0027-standardize-api-documentation-and-schema-coverage.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst index 285b848ef23d..5739cec98c34 100644 --- a/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst +++ b/docs/decisions/0027-standardize-api-documentation-and-schema-coverage.rst @@ -1,7 +1,7 @@ Standardize API Documentation & Schema Coverage ================================================================= -:Status: Proposed +:Status: Accepted :Date: 2026-03-18 :Deciders: API Working Group :Technical Story: Open edX REST API Standards - Documentation standardization for discoverability @@ -16,7 +16,7 @@ developers, and leads to the emergence of duplicate or overlapping endpoints. Today, the documentation that does exist in the platform is largely produced through `api-doc-tools `_, an Open -edX-maintained shim over +edX platform shim over `drf-yasg `_. ``api-doc-tools`` provides simplified decorators (``@schema``, ``@schema_for``) that wrap ``@swagger_auto_schema``, helper functions such as ``parameter()`` for From 8a53e888dadc7990811c2da343faa063118c2cea Mon Sep 17 00:00:00 2001 From: Abdul-Muqadim-Arbisoft <139064778+Abdul-Muqadim-Arbisoft@users.noreply.github.com> Date: Wed, 29 Apr 2026 14:09:07 +0500 Subject: [PATCH 13/18] docs: add ADR for standardizing pagination across APIs (#38300) --- .../0032-standardize-pagination-usage.rst | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 docs/decisions/0032-standardize-pagination-usage.rst diff --git a/docs/decisions/0032-standardize-pagination-usage.rst b/docs/decisions/0032-standardize-pagination-usage.rst new file mode 100644 index 000000000000..dcccb263f73b --- /dev/null +++ b/docs/decisions/0032-standardize-pagination-usage.rst @@ -0,0 +1,176 @@ +Standardize Pagination Across APIs +=================================== + +:Status: Proposed +:Date: 2026-04-08 +:Deciders: API Working Group +:Technical Story: Open edX REST API Standards - Pagination standardization for consistency and scalability + +Context +------- + +Open edX platform API endpoints use multiple, inconsistent pagination strategies. Some endpoints use ``limit``/``offset`` query parameters, others use ``page``/``page_size``, and several return complete result sets with no pagination at all. This inconsistency forces every API consumer — whether a frontend micro-frontend (MFE), a mobile client, an AI agent, or a third-party integration — to implement custom data-loading logic per endpoint. + +The ``edx-drf-extensions`` library already provides a ``DefaultPagination`` class (a subclass of DRF's ``PageNumberPagination``) that standardizes on ``page``/``page_size`` parameters with a default page size of 10 and a maximum of 100. However, many endpoints either override this with ad-hoc pagination classes, use ``LimitOffsetPagination``, or bypass pagination entirely by returning raw lists or manually constructed JSON arrays. + +Decision +-------- + +We will standardize all Open edX REST APIs to use the existing ``DefaultPagination`` class from ``edx-drf-extensions`` as the platform-wide pagination standard. + +Implementation requirements: + +* All list-type API endpoints MUST use ``DefaultPagination`` (or a subclass of it) from ``edx-drf-extensions``. +* Endpoints currently using ``LimitOffsetPagination`` MUST be migrated to ``DefaultPagination`` with appropriate versioning. +* Endpoints returning unpaginated result sets MUST be updated to return paginated responses. +* All paginated responses MUST include the standard envelope: ``count``, ``next``, ``previous``, ``num_pages``, ``current_page``, ``start``, and ``results``. +* Views that subclass ``APIView`` directly (rather than ``GenericAPIView`` or ``ListAPIView``) MUST manually invoke the pagination API to return paginated responses. +* Custom ``page_size`` overrides per endpoint are acceptable when justified (e.g., mobile APIs may use a smaller default), but MUST be implemented by subclassing ``DefaultPagination`` rather than using an unrelated pagination class. +* Maintain backward compatibility for all APIs during migration. If a fully compatible migration is not possible, a new API version MUST be created and the old version deprecated following the standard deprecation process. + +Scope and Tree-Shaped Endpoints +------------------------------- + +This ADR applies to **flat list endpoints** — endpoints whose response is a collection of sibling items with no hierarchical nesting between items. + +**Tree-shaped endpoints** — where each item may contain an arbitrary subtree of child items (Course Blocks, Taxonomy, OLX structure, progress trees) — are out of scope for the standard item-count pagination envelope described here. Applying ``DefaultPagination`` to such endpoints is ill-defined: a "page size of 10" has no consistent meaning when items may contain hundreds of descendants, and paginating over a flat node set risks splitting parents from their children across page boundaries. + +Tree-shaped endpoints MUST instead follow one of these patterns: + +1. Return the complete structural representation (IDs, types, parent/child relationships, display names) unpaginated at a controlled depth, and paginate separately over node *content* via follow-up endpoints. The Course Blocks API's ``requested_fields`` behavior is the reference implementation of this pattern. +2. Return the tree to a fixed maximum depth, and provide explicit child-fetch URLs for any subtrees beyond that depth. + +Response-shape conventions for these endpoints — minimal vs full views, field selection (``?fields=...``), and flattening of deeply nested JSON — are specified in ADR-0036 (*Reduce Deeply Nested JSON via Minimal/Flattened Views*), which is the canonical place for those decisions. + +Where a tree endpoint exposes a flat list of node IDs alongside its structural representation (for example via ``?fields=id,type``), standard ``DefaultPagination`` over that flat ID list is appropriate and in scope. + +Relevance in edx-platform +-------------------------- + +Current example patterns that should be migrated: + +* **Completion API** (``/api/completion/v1/completion/``) — uses inconsistent pagination formats depending on request parameters; some paths return unpaginated results. +* **User Accounts API** (``/api/user/v1/accounts/``) — pagination behavior differs from other user-related APIs, making it difficult for consumers to use a single data-loading pattern. +* **Course Members API** (``/api/courses/v1/.../members/``) — returns all enrollments without pagination, relying on a ``COURSE_MEMBER_API_ENROLLMENT_LIMIT`` setting (default 1000) to cap results and raising ``OverEnrollmentLimitException`` instead of paginating. +* **Enrollment API** (``/api/enrollment/v1/``) — some list endpoints return full result sets without pagination support. +* **Course Blocks API** (``/api/courses/v2/blocks/``) — a tree-shaped endpoint. Out of scope for standard item-count pagination per the *Scope and Tree-Shaped Endpoints* section above; its ``requested_fields`` behavior is the reference pattern for structural queries over trees. Response-shape conventions for such endpoints are specified in ADR-0036. + +Code example (target pagination usage) +--------------------------------------- + +**Example using DefaultPagination with a ListAPIView:** + +.. code-block:: python + + # views.py + from rest_framework.generics import ListAPIView + from edx_rest_framework_extensions.paginators import DefaultPagination + from .serializers import EnrollmentSerializer + + class EnrollmentListView(ListAPIView): + """ + Returns a paginated list of enrollments for the authenticated user. + + Pagination parameters: + - page (int): The page number to retrieve. Default is 1. + - page_size (int): Number of results per page. Default is 10, max is 100. + + Response envelope: + - count (int): Total number of results. + - num_pages (int): Total number of pages. + - current_page (int): The current page number. + - next (str|null): URL for the next page, or null. + - previous (str|null): URL for the previous page, or null. + - start (int): The starting index of the current page. + - results (list): The list of enrollment objects. + """ + serializer_class = EnrollmentSerializer + pagination_class = DefaultPagination + + def get_queryset(self): + return CourseEnrollment.objects.filter( + user=self.request.user, + is_active=True, + ).order_by('-created') + +**Example subclassing DefaultPagination for a mobile endpoint with a smaller page size:** + +.. code-block:: python + + # paginators.py + from edx_rest_framework_extensions.paginators import DefaultPagination + + class MobileDefaultPagination(DefaultPagination): + """ + Pagination tuned for mobile clients with smaller payloads. + """ + page_size = 5 + max_page_size = 50 + +**Example using DefaultPagination with a plain APIView (manual invocation):** + +.. code-block:: python + + # views.py + from rest_framework.views import APIView + from rest_framework.response import Response + from edx_rest_framework_extensions.paginators import DefaultPagination + + class CompletionListView(APIView): + pagination_class = DefaultPagination + + def get(self, request): + completions = BlockCompletion.objects.filter( + user=request.user + ).order_by('-modified') + paginator = self.pagination_class() + page = paginator.paginate_queryset(completions, request) + serializer = CompletionSerializer(page, many=True) + return paginator.get_paginated_response(serializer.data) + +Consequences +------------ + +Positive +~~~~~~~~ + +* External systems and AI agents can implement a single, reusable data loader for all Open edX list endpoints. +* Consumers can reliably pre-calculate batch sizes using the ``count`` and ``num_pages`` fields in every paginated response. +* Eliminates unbounded response sizes that currently risk overloading clients and timing out requests (e.g., large enrollment or discussion lists). +* Enables consistent OpenAPI schema generation for all list endpoints. +* Leverages the already-existing ``DefaultPagination`` class, minimizing new code. + +Negative / Trade-offs +~~~~~~~~~~~~~~~~~~~~~ + +* Endpoints that currently return full result sets (e.g., Course Members, Completion) will require consumers to implement pagination loops where they previously did not need to. +* Requires refactoring views that use ``APIView`` directly without DRF's generic pagination machinery. +* Migrating ``limit``/``offset`` endpoints to ``page``/``page_size`` is a breaking change for existing consumers of those specific endpoints and must be versioned. +* Some internal consumers (e.g., modulestore aggregation) may need to be updated to handle paginated results instead of full lists. + +Alternatives Considered +----------------------- + +* **Standardize on LimitOffsetPagination instead of PageNumberPagination**: Rejected because ``edx-drf-extensions`` already ships ``DefaultPagination`` based on ``PageNumberPagination``, and a significant portion of the platform already uses it — standardizing on it minimizes migration churn. Numbered pages are also easier for humans to reason about, bookmark, and share, and map directly onto existing MFE numbered-page UI controls. Note that the two styles have equivalent database query characteristics by default (both emit ``LIMIT ... OFFSET ...`` SQL via Django's core Paginator); the choice here is about ecosystem fit, not query cost. +* **Adopt CursorPagination as the platform standard**: Rejected because cursor-based pagination does not support random page access (jumping directly to page N), which would break existing MFE numbered-page controls and bookmarkable deep links. The ``CursorPagination`` response envelope (opaque ``next`` / ``previous`` cursors, no ``count``) also differs substantially from what existing Open edX consumers expect, so adoption would require coordinated client-side changes across MFEs and mobile rather than a gradual per-endpoint rollout. ``CursorPagination`` remains a reasonable per-endpoint choice for very large, append-only, or high-churn datasets where numbered pages are not needed. +* **Allow each API app to choose its own pagination style**: Rejected because this is the current state, and it is the root cause of the inconsistency this ADR aims to resolve. +* **Do nothing and document the differences**: Rejected because documentation alone does not reduce the integration burden on consumers or prevent future inconsistencies. + +Rollout Plan +------------ + +1. Audit all list-type API endpoints in ``edx-platform`` to categorize them as: already using ``DefaultPagination``, using a different pagination class, or unpaginated. +2. Add a custom ``pylint`` or ``edx-lint`` check that warns when a ``ListAPIView`` or list-returning ``APIView`` does not specify ``DefaultPagination`` (or a subclass). +3. Migrate high-impact unpaginated endpoints first (Course Members, Completion, Enrollment). +4. Migrate ``limit``/``offset`` endpoints by introducing new API versions that use ``DefaultPagination``, and deprecating the old versions. +5. Update MFEs and known external consumers to adopt the new pagination parameters where versions change. +6. Update API documentation and OpenAPI specs to reflect the standardized pagination envelope. + +References +---------- + +* ``edx-drf-extensions`` ``DefaultPagination`` class: https://github.com/openedx/edx-drf-extensions/blob/master/edx_rest_framework_extensions/paginators.py +* Django REST Framework Pagination documentation: https://www.django-rest-framework.org/api-guide/pagination/ +* ADR-0036 — Reduce Deeply Nested JSON via Minimal/Flattened Views (docs/decisions/0036-normalize-deeply-nested-json-apis.rst). +* Open edX REST API Standards: "Pagination" recommendations for API consistency. +* Open edX API Thoughts wiki: https://openedx.atlassian.net/wiki/spaces/AC/pages/16646635/API+Thoughts From 085301eb4f8f82837b94d0889060ee32356cad16 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed <68893403+taimoor-ahmed-1@users.noreply.github.com> Date: Tue, 5 May 2026 11:42:04 +0500 Subject: [PATCH 14/18] docs: add ADR for api versioning strategy (#38304) --- .../0037-api-versioning-strategy.rst | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 docs/decisions/0037-api-versioning-strategy.rst diff --git a/docs/decisions/0037-api-versioning-strategy.rst b/docs/decisions/0037-api-versioning-strategy.rst new file mode 100644 index 000000000000..4d1c4d845f1e --- /dev/null +++ b/docs/decisions/0037-api-versioning-strategy.rst @@ -0,0 +1,113 @@ +API Versioning Strategy — Versioned Endpoints with CI-Enforced Compatibility +============================================================================= + +:Status: Accepted +:Date: 2026-04-08 +:Deciders: API Working Group + +Context +======= + +Open edX has multiple API versions in parallel (e.g., v0/v1/v2/v3) which creates confusion about +which version is stable or deprecated and increases the risk that external systems rely on outdated +contracts. The platform currently mixes versioned endpoints (``/api/enrollment/v1/``, +``/api/user/v2/``) with unversioned ones (``/api/course_experience/``), making it unclear which +endpoint clients should use and what the deprecation timeline looks like. + +Decision +======== + +1. **All new APIs must be versioned.** Every new API endpoint must include an explicit version in + its URL path (e.g., ``/api/foo/v1/``). Unversioned paths are not permitted for new APIs. + +2. **The highest version number is the one clients should prefer.** When multiple versions of an + endpoint exist simultaneously, clients must target the highest available version. Older versions + are kept only during an active deprecation window. + +3. **Automated CI tooling must detect backwards-incompatible OpenAPI changes.** Any PR that + introduces a backwards-incompatible change to the OpenAPI spec (e.g., removed fields, changed + types, removed endpoints) must bump the API version. CI checks enforce this automatically by + diffing the OpenAPI schema against the base branch and failing the build if a breaking change is + detected without a corresponding version increment. + +4. **Follow the OEP-0021 deprecation process when removing old versions:** + + * File a DEPR issue in the ``openedx/public-engineering`` project to track the deprecation. + * Mark old versions as deprecated in the OpenAPI schema (using the ``deprecated: true`` flag) + and in the endpoint's docstring. + * Provide a migration guide pointing clients to the new version. + * Set and communicate a removal timeline aligned with the Open edX release cycle (minimum one + named release, typically ~6 months). + * Complete the deprecation by removing the old version's URL route and implementation code once + the timeline has elapsed. + +Relevance in edx-platform +========================= + +* **Current mix**: LMS and CMS use both versioned and non-versioned API paths. + Examples: ``api/enrollment/v1/``, ``api/val/v0/``, ``api/instructor/v1/`` and + ``v2/``, ``api/user/v1/`` and ``api/user/v2/``, ``api/mfe_config/v1``, + ``api/course_experience/`` (no version in path), ``api/xblock/v2/``, + ``api/libraries/v2/`` (see ``lms/urls.py``, + ``openedx/core/djangoapps/user_authn/urls_common.py``). +* **Confusion**: Multiple versions (v0, v1, v2, v3) without a single "default" + make it unclear which endpoint clients should use. Unversioned paths provide no deprecation + contract at all. +* **Existing unversioned endpoints**: These are out of scope for an immediate migration, but new + work on those services should add versioned paths following this ADR. + +Code example (routing pattern) +============================= + +**Standard versioned endpoint:** + +.. code-block:: python + + # urls.py + urlpatterns = [ + path("api/courses/v1/", include("course_api.v1.urls")), # current stable + ] + +**When introducing a breaking change:** + +.. code-block:: text + + 1. Increment the version: add /api/courses/v2/ with the new contract. + 2. Register v2 in OpenAPI. Mark v1 as deprecated (deprecated: true) with a removal date. + 3. File a DEPR issue to track the deprecation timeline (openedx/public-engineering). + 4. After the deprecation period has elapsed, remove the v1 URL route and implementation. + +Consequences +============ + +* Pros + + * Clear, explicit versioning contract — clients always know what version they are targeting. + * The highest version number is unambiguous: clients can always upgrade to the latest. + * Automated CI enforcement prevents accidental breaking changes from reaching clients without + a corresponding version bump. + * Formal DEPR issue tracking provides accountability and visibility for all active deprecations. + +* Cons / Costs + + * Existing unversioned endpoints (e.g., ``api/course_experience/``) require a future migration + plan to add versioning. + * CI tooling for OpenAPI diff checks requires initial setup investment. + * Teams must increment version numbers and update URL routing when making breaking changes. + +Rejected Alternatives +===================== + +* **Non-versioned endpoints as the default "stable" surface**: An earlier draft proposed treating + unversioned paths (e.g., ``/api/courses/``) as the default stable entry point, with versioned + paths created only for breaking changes and the unversioned URL kept as a forwarding alias to + the latest version. This was rejected because unversioned URL aliases pointing to the latest + implementation create ambiguity for clients and tooling, do not provide a clear deprecation + contract, and make it difficult to reason about backwards compatibility. + +References +========== + +* `OEP-0021: Deprecation and Removal `_ +* "Versioning confusion / deprecated versions" recommendation in the Open edX REST API + standardization notes. From 17256ce8453f75a9b2b89c4caf8ad69fd70da46d Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Tue, 5 May 2026 15:57:40 +0500 Subject: [PATCH 15/18] docs: add ADR for standardizing filtering/sorting parameters (#38303) --- .../0025-standardize-serializer-usage.rst | 2 +- .../0026-standardize-permission-classes.rst | 2 +- ...-endpoints-using-standard-drf-viewsets.rst | 2 +- .../0032-standardize-pagination-usage.rst | 2 +- ...ardize-filter-sort-using-django-filter.rst | 139 ++++++++++++++++++ 5 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 docs/decisions/0033-standardize-filter-sort-using-django-filter.rst diff --git a/docs/decisions/0025-standardize-serializer-usage.rst b/docs/decisions/0025-standardize-serializer-usage.rst index 7f3e81d35123..9a8f40d3f326 100644 --- a/docs/decisions/0025-standardize-serializer-usage.rst +++ b/docs/decisions/0025-standardize-serializer-usage.rst @@ -1,7 +1,7 @@ Standardize Serializer Usage Across APIs ======================================== -:Status: Proposed +:Status: Accepted :Date: 2026-03-09 :Deciders: API Working Group :Technical Story: Open edX REST API Standards - Serializer standardization for consistency diff --git a/docs/decisions/0026-standardize-permission-classes.rst b/docs/decisions/0026-standardize-permission-classes.rst index 659ddf5175e4..a4190334ab6e 100644 --- a/docs/decisions/0026-standardize-permission-classes.rst +++ b/docs/decisions/0026-standardize-permission-classes.rst @@ -1,7 +1,7 @@ Open edX ADR 0026: Standardize Permission Classes Across APIs ============================================================= -:Status: Proposed +:Status: Accepted :Date: 2026-03-18 :Deciders: API Working Group :Technical Story: Open edX REST API Standards - Permission standardization for security consistency diff --git a/docs/decisions/0028-migrate-restful-and-legacy-api-endpoints-using-standard-drf-viewsets.rst b/docs/decisions/0028-migrate-restful-and-legacy-api-endpoints-using-standard-drf-viewsets.rst index 102a2c374b40..02365607a1f7 100644 --- a/docs/decisions/0028-migrate-restful-and-legacy-api-endpoints-using-standard-drf-viewsets.rst +++ b/docs/decisions/0028-migrate-restful-and-legacy-api-endpoints-using-standard-drf-viewsets.rst @@ -1,7 +1,7 @@ Migrating RESTful & Legacy Django API Endpoints to Standard DRF ViewSets ======================================================================== -:Status: Proposed +:Status: Accepted :Date: 2026-03-19 :Deciders: API Working Group :Technical Story: Open edX REST API Standards - RESTful & Legacy Django API endpoint structure standardization using DRF ViewSets diff --git a/docs/decisions/0032-standardize-pagination-usage.rst b/docs/decisions/0032-standardize-pagination-usage.rst index dcccb263f73b..a305c4b59b86 100644 --- a/docs/decisions/0032-standardize-pagination-usage.rst +++ b/docs/decisions/0032-standardize-pagination-usage.rst @@ -1,7 +1,7 @@ Standardize Pagination Across APIs =================================== -:Status: Proposed +:Status: Accepted :Date: 2026-04-08 :Deciders: API Working Group :Technical Story: Open edX REST API Standards - Pagination standardization for consistency and scalability diff --git a/docs/decisions/0033-standardize-filter-sort-using-django-filter.rst b/docs/decisions/0033-standardize-filter-sort-using-django-filter.rst new file mode 100644 index 000000000000..fbc898763fad --- /dev/null +++ b/docs/decisions/0033-standardize-filter-sort-using-django-filter.rst @@ -0,0 +1,139 @@ +======================================== +Standardize Filtering/Sorting Parameters +======================================== + +:Status: Accepted +:Date: 2026-04-08 +:Deciders: API Working Group +:Technical Story: Open edX REST API Standards - Filtering/Sorting parameters standardization for consistency + +Context +======= + +Filtering and sorting syntax varies across Open edX APIs (e.g., inconsistent parameter names such as +``course_key`` vs ``course``). This forces clients to hardcode endpoint-specific logic and prevents +tooling/agents from reliably inferring query patterns. + +Decision +======== + +1. Adopt ``django-filter`` for list endpoints requiring filtering. +2. Standardize parameter naming conventions (e.g., use ``course_key`` consistently) and document them. +3. Provide consistent sorting conventions: + + * Use a standard ``ordering`` parameter (DRF convention), with documented allowed fields. + +4. Update schemas so filters and ordering are discoverable via OpenAPI. + +Relevance in edx-platform +========================= + +* **Existing usage**: ``django_filters`` is already used in several places: + ``openedx/core/djangoapps/user_api/views.py`` (``DjangoFilterBackend``), + ``lms/djangoapps/experiments/views.py`` and ``experiments/filters.py`` + (``ExperimentDataFilter``, ``ExperimentKeyValueFilter``), + ``common/djangoapps/entitlements/rest_api/v1/views.py`` and + ``entitlements/rest_api/v1/filters.py`` (``CourseEntitlementFilter`` with + ``uuid``, ``user__username``, ``course_uuid``, ``expired_at__isnull``). +* **Inconsistency**: Parameter names and filter semantics vary across APIs + (e.g. ``course_key`` vs ``course``); standardizing on ``course_key`` for course identifier strings (per `OEP-68`_) and + a single ``ordering`` parameter aligns with this ADR. + +Code examples +============= + +**FilterSet (entitlements pattern in edx-platform):** + +.. code-block:: python + + # common/djangoapps/entitlements/rest_api/v1/filters.py + from django_filters import rest_framework as filters + + class CourseEntitlementFilter(filters.FilterSet): + user = filters.CharFilter(field_name='user__username') + course_uuid = UUIDListFilter(field_name='course_uuid') + expired_at__isnull = filters.BooleanFilter(field_name='expired_at', lookup_expr='isnull') + + class Meta: + model = CourseEntitlement + fields = ('uuid', 'user', 'course_uuid') + +**ViewSet with filter and ordering:** + +.. code-block:: python + + from django_filters.rest_framework import DjangoFilterBackend + from rest_framework.filters import OrderingFilter + + class CourseEntitlementViewSet(viewsets.ReadOnlyModelViewSet): + queryset = CourseEntitlement.objects.all() + filter_backends = (DjangoFilterBackend, OrderingFilter) + filterset_class = CourseEntitlementFilter + ordering_fields = ["created", "modified", "expired_at"] + ordering = ["-created"] # default + + # GET /api/entitlements/v1/?user=john&course_uuid=...&ordering=-modified + +Consequences +============ + +* Pros + + * Predictable client implementation; easier SDK generation and AI discovery. + * Reduced duplication across apps. + +* Cons / Costs + + * Requires coordinated changes and backward-compatible aliases (temporary) for existing params. + +Implementation Notes +==================== + +* Add filtersets per endpoint; expose allowed fields via schema generation. +* Provide deprecation warnings for old parameter names and remove after a defined window. +* Create migration guide for teams updating existing API clients. + +Backward Compatibility Strategy +=============================== + +To ensure smooth transition for existing API consumers: + +1. **Parameter Aliases**: Support old parameter names alongside new standardized names: + +.. code-block:: python + + class CourseEntitlementFilter(filters.FilterSet): + course = filters.CharFilter(field_name='course_uuid') # old param + course_key = filters.CharFilter(field_name='course_uuid') # new param + +2. **Deprecation Warnings**: Return HTTP headers warning about deprecated parameters: + + * Deprecation: Parameter 'course' is deprecated. Use 'course_key' instead. + * Support will be removed in release ''. + +3. **Gradual Migration**: + + * Phase 1: Support both old and new parameters with warnings + * Phase 2: Remove old parameters after 2 release cycles + * Phase 3: Enforce new parameter names only + +4. **Documentation Updates**: Clearly mark deprecated parameters in OpenAPI schemas with `deprecated: true`. + +**Why django-filter was chosen:** + + * **Mature ecosystem**: Well-maintained with extensive documentation + * **DRF integration**: Seamless integration with Django REST Framework + * **OpenAPI support**: Automatic schema generation for filters + * **Feature completeness**: Supports complex filtering scenarios (lookups, OR conditions, etc.) + * **Community adoption**: Widely used in Django ecosystem with good community support + +References +========== + +* `Open edX REST API Standardization Notes`_ — “Missing Filter/Sort Consistency” recommendation +* `OEP-68`_ — Content Identifiers (defines ``course_key`` naming convention) +* `django-filter documentation`_ + +.. _Open edX REST API Standardization Notes: https://openedx.atlassian.net/wiki/spaces/AC/pages/18350757/Open+edX+REST+API+Conventions#Conventions +.. _OEP-68: https://docs.openedx.org/projects/openedx-proposals/en/latest/best-practices/oep-0068-bp-content-identifiers.html#summary +.. _django-filter documentation: https://django-filter.readthedocs.io/ From c3513c61493a3573b26061cc9917718c2453d899 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed <68893403+taimoor-ahmed-1@users.noreply.github.com> Date: Thu, 7 May 2026 18:13:55 +0500 Subject: [PATCH 16/18] docs: add ADR-0029 standardized error responses decision (#38246) --- .../0029-standardize-error-responses.rst | 298 ++++++++++++++++++ 1 file changed, 298 insertions(+) create mode 100644 docs/decisions/0029-standardize-error-responses.rst diff --git a/docs/decisions/0029-standardize-error-responses.rst b/docs/decisions/0029-standardize-error-responses.rst new file mode 100644 index 000000000000..6eb31e8c7dbe --- /dev/null +++ b/docs/decisions/0029-standardize-error-responses.rst @@ -0,0 +1,298 @@ +Standardize Error Responses +============================ + +:Status: Accepted +:Date: 2026-03-31 +:Deciders: API Working Group +:Technical Story: Open edX REST API Standards – Error response interoperability + +Context +------- + +Open edX APIs currently return errors in multiple incompatible shapes (e.g., ``{"error": ...}``, +``{"detail": ...}``, nested field errors, and even HTTP 200 responses containing ``"success": false``). This +inconsistency makes it difficult for external clients and AI systems to reliably detect and map error +states across services. + +Objectives +---------- + +We want error responses that: + +* Use **correct HTTP status codes** (4xx/5xx) for failures, and avoid masking errors behind HTTP 200. +* Provide a **single, predictable JSON shape** so clients can implement one parsing path across services. +* Include **machine-readable identifiers** (e.g. a URI for the error class) so tools and integrations can + classify failures without scraping free-form text. +* Carry a **short human-readable summary** plus a **specific explanation** for this request when helpful. +* Tie errors to the **request** when useful (e.g. request path or URL) for support and logging. +* Represent **validation failures** in a consistent way (e.g. field/path to messages) instead of ad-hoc nesting. +* Are **documented and enforced** in DRF (central exception handling + schema generation). + +Decision +-------- + +We will standardize all Open edX REST APIs to return errors using a **structured JSON error object** for +non-2xx responses that meets the objectives above. + +Implementation requirements: + +* Use appropriate HTTP status codes (4xx/5xx). Avoid returning HTTP 200 for error conditions. +* Return a consistent payload with these core fields: + + * ``type`` (URI identifying the problem type) + * ``title`` (short, developer/operator-facing summary of the error class; not intended for display to end users) + * ``status`` (HTTP status code) + * ``detail`` (stable, developer-facing explanation specific to this occurrence; safe for log + aggregators and APM tools — see *Note on RFC 9457 deviation* below) + * ``instance`` (the URI of the request that produced this error, e.g. the request path; see + *Note on ``instance``* below) + * ``user_message`` *(optional)* — a human-readable, translatable string intended for + display in MFEs or end-user UIs. MFE clients should prefer mapping the ``type`` URI to a + locally-translated string; use ``user_message`` when the server must supply context that cannot + be expressed by ``type`` alone. + +* For validation errors, include a predictable extension member ``errors``: a dict mapping each + invalid field/path to a list of error message strings. This maps directly onto DRF's native + ``ValidationError.detail`` dict, so the central exception handler can populate it without + per-view changes. Example:: + + "errors": { + "course_id": ["This field is required."], + "display_name": ["Ensure this field has no more than 255 characters."] + } + +* Define a small catalog of common ``type`` URIs for shared errors. Initial entries: + + .. list-table:: + :header-rows: 1 + :widths: 50 10 40 + + * - URI + - Status + - When to use + * - ``https://docs.openedx.org/errors/not-found`` + - 404 + - Resource does not exist + * - ``https://docs.openedx.org/errors/authz`` + - 403 + - Authenticated but not authorized + * - ``https://docs.openedx.org/errors/authn`` + - 401 + - Not authenticated + * - ``https://docs.openedx.org/errors/validation`` + - 400 + - Request body / query-param validation failure + * - ``https://docs.openedx.org/errors/rate-limited`` + - 429 + - Rate limit exceeded + * - ``https://docs.openedx.org/errors/internal`` + - 500 + - Unexpected server error + + App-specific types may extend this catalog; they must still be absolute URIs. + + While many catalog entries map 1-to-1 with an HTTP status code, ``type`` provides + sub-category granularity that HTTP status alone cannot express (e.g. ``authn`` vs + ``authz`` vs ``validation`` vs ``not-found`` are all 4xx but represent distinct failure + classes). App-specific ``type`` extensions add even finer-grained identifiers (e.g. + ``https://docs.openedx.org/errors/enrollment/already-enrolled``). The ``status`` field is + a convenience duplicate for clients that triage responses by status code without + inspecting the body further. + + These URIs serve as **opaque, stable identifiers** first. They *should* eventually resolve to + human-readable documentation pages on ``docs.openedx.org`` describing the error class, its + causes, and remediation steps — but dereference-ability is not a requirement for the initial + rollout. Clients must treat ``type`` as an opaque string and never rely on HTTP-fetching it at + runtime. +* Error responses must respect the content type signalled by the request. The platform must not + produce HTML error pages when the request used JSON (i.e. when ``Content-Type: application/json`` + or ``Accept: application/json`` was sent). The platform-level DRF exception handler must catch + exceptions that would otherwise produce Django's default HTML error page and return a JSON body + in the standardized format instead. Endpoints not using DRF's ``APIView`` must be identified and + wrapped accordingly. +* For **5xx / unhandled exceptions** in **production** (``DEBUG=False``), the handler must return + a **generic error body** — no stack traces, no internal exception messages, and no sensitive + system details must be included in the response. Only the ``https://docs.openedx.org/errors/internal`` + ``type`` and a fixed ``"Internal Server Error"`` title are safe to return. Detailed diagnostics + belong in server-side logs and APM tooling, not in API responses. + + In **development** (``DEBUG=True``), the handler MAY include additional diagnostic information + (e.g. the exception class and message) in an extension field (e.g. ``debug_detail``) to ease + local debugging. Stack traces should still be written to the server log regardless of mode. +* Preserve **CORS headers** on error responses. When the exception handler short-circuits the + normal response cycle, ``Access-Control-*`` headers set by ``django-cors-headers`` can be + dropped, causing browsers to surface a misleading CORS error rather than the actual error + body. The platform-level exception handler must ensure CORS headers are not stripped from + error responses. +* Ensure the schema is **documented in drf-spectacular** by registering the standardized error + shape as a reusable component (``#/components/schemas/ErrorResponse``), so all API endpoint + docs automatically reference it for 4xx/5xx response types. + +Note on RFC 9457 deviation +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +`RFC 9457 `_ (Problem Details for HTTP APIs) defines +``detail`` as a "human-readable explanation" intended for the client/end-user. This ADR +intentionally deviates from that definition: we use ``detail`` for a **stable, developer-facing, +English-language** string that is safe to forward to APM systems and log aggregators. User-facing +copy is carried in the separate ``user_message`` field instead. This separation keeps localizable, +UI-bound strings out of the machine-readable layer while still providing a meaningful explanation +for developers and on-call engineers. + +Note on ``instance`` +~~~~~~~~~~~~~~~~~~~~ + +The ``instance`` field in this ADR is the **path of the request that produced the error** (e.g. +``request.path``, yielding ``/api/courses/v1/``). A path-only value is preferred over a full +absolute URL (``request.build_absolute_uri()``) because it is useful for correlation and support +without embedding the server hostname or protocol, which can vary across environments. RFC 9457 +permits ``instance`` to be either relative or absolute and does not require it to be +dereferenceable; using the request path is a valid application of the field. + +Relevance in edx-platform +------------------------- + +Current error shapes in the codebase are inconsistent: + +* **DeveloperErrorViewMixin** (``openedx/core/lib/api/view_utils.py``) returns + ``{"developer_message": "...", "error_code": "..."}`` and for validation + ``{"developer_message": "...", "field_errors": {field: {"developer_message": "..."}}}``. +* **Instructor API** (``lms/djangoapps/instructor/views/api.py``) uses + ``JsonResponse({"error": msg}, 400)``. +* **Registration** (``openedx/core/djangoapps/user_authn/views/register.py``) returns + HTTP 200 with ``success: true/false`` and ``error_code`` for some failures. +* **ORA Staff Grader** (``lms/djangoapps/ora_staff_grader/errors.py``) uses a custom + ``ErrorSerializer`` with an ``error`` field. +* **Enrollment API** (``openedx/core/djangoapps/enrollments/``) returns + ``{"message": "..."}`` or ``{"message": "...", "localizedMessage": "..."}`` for errors. + +Code example (target shape) +--------------------------- + +**Example structured error response (4xx):** + +.. code-block:: json + + { + "type": "https://docs.openedx.org/errors/validation", + "title": "Validation Error", + "status": 400, + "detail": "The request body failed validation.", + "user_message": "Some required fields are missing or invalid.", + "instance": "/api/courses/v1/", + "errors": { + "course_id": ["This field is required."], + "display_name": ["Ensure this field has no more than 255 characters."] + } + } + +**Attaching a** ``user_message`` **to an exception:** + +Because ``user_message`` is detected via ``hasattr``, it can be set on any ``APIException`` +instance before raising — no subclass required: + +.. code-block:: python + + from django.utils.translation import gettext_lazy as _ + from rest_framework.exceptions import APIException + + exc = APIException("Enrollment limit reached for course-v1:edX+DemoX+Demo_Course.") + exc.user_message = _("This course is currently full. Please try again later.") + raise exc + +The central exception handler's ``hasattr(exc, 'user_message')`` check picks this up +automatically, requiring no per-view changes. + +**Example DRF exception handler emitting the standard shape:** + +.. code-block:: python + + # Central exception handler (e.g. in openedx/core/lib/api/exceptions.py) + def standardized_error_exception_handler(exc, context): + from rest_framework.views import exception_handler + response = exception_handler(exc, context) + if response is None: + # DRF returned None — unhandled exception (e.g. IntegrityError, unexpected 5xx). + # Always return a generic body; never include stack traces or exception details. + return Response( + { + "type": "https://docs.openedx.org/errors/internal", + "title": "Internal Server Error", + "status": 500, + "detail": "An unexpected error occurred. Please try again later.", + }, + status=500, + ) + request = context.get("request") + body = { + "type": f"https://docs.openedx.org/errors/{_error_type(exc)}", + "title": _error_title(exc), + "status": response.status_code, + "detail": _flatten_detail(response.data), + } + if request: + body["instance"] = request.path + if hasattr(exc, "user_message") and exc.user_message: + body["user_message"] = exc.user_message + if isinstance(exc, ValidationError) and hasattr(exc, "detail"): + body["errors"] = _normalize_validation_errors(exc.detail) + response.data = body + response["Content-Type"] = "application/json" + return response + +Consequences +------------ + +Positive +~~~~~~~~ + +* Clients can implement a single error-handling path across services. +* AI agents and external integrations can programmatically detect and classify error states. +* Removes "hidden failures" caused by HTTP 200 + ``success: false`` patterns. + +Negative / Trade-offs +~~~~~~~~~~~~~~~~~~~~~ + +* Requires refactoring of existing endpoints and tests that currently depend on ad-hoc error shapes. +* Some clients may need a migration period if they parse legacy error formats. + +Alternatives Considered +----------------------- + +* **Keep per-app formats**: rejected due to interoperability and client complexity. +* **Use DRF defaults only**: rejected because DRF defaults still vary across validation/auth exceptions + unless centrally handled and documented. +* **`drf-standardized-errors `_**: a well-maintained + third-party library that implements RFC 9457-style responses for DRF. Considered but not adopted + because: (a) it would add a new dependency to platform core, (b) we need custom behavior for CORS + header preservation and the non-``APIView`` 500 path that would require overriding most of the + library anyway, and (c) the contract defined here is lightweight enough to implement directly in + the platform exception handler without a library. + +Rollout Plan +------------ + +Error response format changes are considered backwards-compatible: well-behaved clients should +handle unexpected JSON fields gracefully (robustness principle). The default migration path is +therefore **in-place** — update the exception handler and, where needed, individual views without +bumping the URL version. Teams with clients that are tightly coupled to a legacy error shape MAY +version their endpoint following ADR-0037 (API Versioning Strategy) and maintain both shapes +during a deprecation window. + +1. Introduce a shared DRF exception handler (platform-level) that emits the standardized error shape, + including catching unhandled exceptions that would otherwise produce Django's HTML 500 page. +2. Verify CORS headers (``Access-Control-*``) are preserved on all error responses; update the + exception handler if ``django-cors-headers`` does not run before it. +3. Update existing endpoint unit tests to assert the standardized error shape. Contract tests + across services are optional but encouraged for endpoints consumed by external clients. +4. Audit and fix endpoints that still return HTML errors on 500 (e.g. non-``APIView`` entry points). +5. Migrate apps module-by-module; keep a short deprecation window for legacy shapes where feasible. +6. Update API documentation to specify the standard error schema. + +References +---------- + +* Open edX REST API Standards: "Inconsistent Error Response Structure" and alignment with structured, + interoperable error payloads across services. +* `RFC 9457 – Problem Details for HTTP APIs `_ +* `drf-standardized-errors `_ From 8fea7a68ccc3b61b82c0c8bf84efa11a14192b06 Mon Sep 17 00:00:00 2001 From: Abdul-Muqadim-Arbisoft <139064778+Abdul-Muqadim-Arbisoft@users.noreply.github.com> Date: Tue, 12 May 2026 11:17:03 +0500 Subject: [PATCH 17/18] docs: add ADR for merging similar endpoints (#38262) --- .../0031-merge-similar-endpoints.rst | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 docs/decisions/0031-merge-similar-endpoints.rst diff --git a/docs/decisions/0031-merge-similar-endpoints.rst b/docs/decisions/0031-merge-similar-endpoints.rst new file mode 100644 index 000000000000..3a041287b895 --- /dev/null +++ b/docs/decisions/0031-merge-similar-endpoints.rst @@ -0,0 +1,184 @@ +Merge Similar Endpoints +======================= + +:Status: Accepted +:Date: 2026-03-31 +:Deciders: Open edX Platform / API Working Group +:Technical Story: Open edX REST API Standards - Consolidation of fragmented same-resource endpoints into unified parameterised views + +Context +------- + +Open edX APIs currently expose multiple endpoints that perform closely related operations with only +minor variations in behaviour. Rather than consolidating these into a single parameterised resource, +the platform has grown a proliferation of narrow, action-scoped URLs — each duplicating validation, +permission-checking, and business logic from its siblings. + +A prominent cluster illustrate the problem: + +**Certificate endpoints** (``lms/djangoapps/instructor/views/api_urls.py``): + +* ``enable_certificate_generation`` — enables or disables self-generated certificates for students +* ``start_certificate_generation`` — triggers bulk certificate generation for all enrolled students +* ``start_certificate_regeneration`` — regenerates certificates based on provided + ``certificate_statuses`` + +All three are registered in ``api_urls.py`` as separate ``path()`` entries and each independently +validates ``course_id``, checks instructor permissions, and dispatches a background Celery task — +with near-identical boilerplate in each view. + +The impact of this fragmentation is felt across several dimensions: + +* **Redundant code**: Permission checks, serializer logic, and audit-logging are re-implemented + independently across views, making fixes and feature additions error-prone. +* **Client complexity**: External systems and AI agents must discover, call, and handle errors for + multiple endpoints to complete a single logical workflow. +* **Inconsistent contracts**: Divergent request/response shapes between sibling endpoints create + subtle integration bugs and complicate contract testing. + +Decision +-------- + +We will consolidate groups of closely related endpoints into **single, parameterised DRF views** +(or shared service layers), using an ``action`` (or equivalent) request parameter to distinguish +the operation being performed. + +Implementation requirements: + +* Identify endpoint groups that share the same resource domain and differ only in the operation + applied to that resource. +* Expose a single URL per resource group accepting an ``action`` or ``mode`` field (or using HTTP + verbs semantically where REST conventions apply cleanly). +* Move shared infrastructure, input validation, audit logging, response shaping, and the + enforcement machinery for permissions, into a common service layer or mixin that all operations + invoke. The distinct authorization requirements of the legacy endpoints must be preserved: the + view performs a coarse access check, and each mode handler in the service layer enforces its + own specific permission. Consolidation removes duplicated boilerplate; it does not flatten the + authorization model. +* Preserve backward compatibility via URL aliases or deprecation redirects for a defined transition + window. +* Document the unified endpoint schema in drf-spectacular / OpenAPI, including the enumerated set + of valid ``action`` / ``mode`` values and their respective request/response shapes. + +Relevance in edx-platform +-------------------------- + +Confirmed fragmentation in the codebase: + +* **Certificate views** (``lms/djangoapps/instructor/views/api_urls.py``, lines confirmed in + master): The following three entries exist as separate ``path()`` registrations:: + + path('enable_certificate_generation', api.enable_certificate_generation, + name='enable_certificate_generation'), + path('start_certificate_generation', api.StartCertificateGeneration.as_view(), + name='start_certificate_generation'), + path('start_certificate_regeneration', api.StartCertificateRegeneration.as_view(), + name='start_certificate_regeneration'), + +Code example (target unified endpoint) +--------------------------------------- + +**Proposed unified certificate task endpoint**: + +.. code-block:: http + + POST /api/instructor/v1/certificate_task/{course_id} + Content-Type: application/json + + { + "mode": "generate" + } + +Valid ``mode`` values: ``generate``, ``regenerate``, ``toggle``. + +**Example DRF view skeleton:** + +.. code-block:: python + + # lms/djangoapps/instructor/views/api.py + class CertificateTaskView(APIView): + """ + Unified entry point for certificate generation lifecycle operations. + + Authorization is enforced in two layers: + + 1. A coarse view-level check confirms the caller has instructor-level + access to the course at all. + 2. Per-mode permission checks live inside the corresponding + ``CertificateTaskService`` method, preserving the distinct + authorization requirements of the legacy endpoints + (``enable_certificate_generation``, + ``start_certificate_generation``, + ``start_certificate_regeneration``). + """ + + VALID_MODES = {"generate", "regenerate", "toggle"} + + def post(self, request, course_id): + course_key = CourseKey.from_string(course_id) + # Coarse authorization: must be an instructor on this course. + _check_instructor_permissions(request.user, course_key) + + mode = request.data.get("mode") + if mode not in self.VALID_MODES: + raise ValidationError({"mode": f"Must be one of: {self.VALID_MODES}"}) + + service = CertificateTaskService(course_key, request.user) + # Each service method enforces its own mode-specific permission + # before dispatching to the underlying task. + result = getattr(service, mode)(request.data) + return Response(result, status=status.HTTP_200_OK) + +Consequences +------------ + +Positive +~~~~~~~~ + +* Clients implement a single integration point per resource domain, reducing onboarding friction + for external systems and AI agents. +* Shared validation, permission, and audit logic lives in one place, eliminating divergence between + sibling endpoints. +* OpenAPI schemas become more compact — a single operation object per resource instead of three + or more. +* Contract tests cover one endpoint per resource group, cutting test surface area without reducing + coverage. +* The certificate consolidation aligns with an already-open upstream issue (#36961), increasing + likelihood of community acceptance. + +Negative / Trade-offs +~~~~~~~~~~~~~~~~~~~~~ + +* Existing clients calling the legacy URLs require a migration period; deprecated aliases must be + maintained until adoption drops sufficiently. +* The ``mode`` / ``action`` parameter pattern diverges from strict REST conventions; teams must + agree on a consistent naming standard across endpoint groups. +* A poorly designed service layer could become a "god object"; care must be taken to keep each + operation handler cohesive and independently testable. + +Alternatives Considered +----------------------- + +* **Keep per-action endpoints**: Rejected. The duplication cost compounds with every new operation + and makes consistent error handling and logging practically impossible to enforce. +* **Use HTTP verbs exclusively (pure REST)**: Not applicable. This is already RESTful. + The noun is ``certificate_task``, the ``POST`` indicates that we are creating a + certificate task, and the payload indicates what the task is going to be. +* **GraphQL mutations**: Considered but out of scope for this iteration; the platform's existing + REST ecosystem makes a full GraphQL migration impractical in the near term. + +Rollout Plan +------------ + +1. Implement the unified ``CertificateTaskView``; register + legacy paths as deprecated aliases emitting a ``Deprecation`` response header. +2. Identify and document additional endpoint groups sharing a resource domain. Add them to the + placeholder table below. +3. Announce a deprecation timeline to known API consumers and update developer documentation. +4. Remove legacy aliases after the deprecation window closes (target: two named Open edX releases). + +References +---------- + +* Django REST Framework – Class-Based Views: + https://www.django-rest-framework.org/api-guide/views/ From ad4b7bae125f66528bb983226fb7edcad82bbc05 Mon Sep 17 00:00:00 2001 From: Taimoor Ahmed <68893403+taimoor-ahmed-1@users.noreply.github.com> Date: Wed, 10 Jun 2026 14:40:42 +0500 Subject: [PATCH 18/18] docs: ADR for normalizing nested json apis (#38305) --- ...0036-normalize-deeply-nested-json-apis.rst | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 docs/decisions/0036-normalize-deeply-nested-json-apis.rst diff --git a/docs/decisions/0036-normalize-deeply-nested-json-apis.rst b/docs/decisions/0036-normalize-deeply-nested-json-apis.rst new file mode 100644 index 000000000000..af6c2835c207 --- /dev/null +++ b/docs/decisions/0036-normalize-deeply-nested-json-apis.rst @@ -0,0 +1,182 @@ + +Reduce Deeply Nested JSON via Minimal/Flattened Views +===================================================== + +:Status: Accepted +:Date: 2026-04-08 +:Deciders: API Working Group + +Context +======= + +Some APIs return deeply nested JSON payloads (course structures, block trees, progress views). +This makes payloads hard to parse, increases response size, and slows clients and automated agents. + +Decision +======== + +1. Provide a "minimal" representation option for complex resources: + + * Query param example: ``?view=minimal`` or ``?fields=...``. + +2. Normalize/flatten overly nested structures where possible: + + * Prefer references/IDs with follow-up endpoints over embedding entire trees by + default. This is the recommended pattern for server-to-server integrations and + automated clients (e.g., AI agents), where minimizing coupling to a specific + nested shape matters more than minimizing request count. + + **Exception — frontend / MFE clients:** when a client needs to render a complete + view in a single page load and the nested data is *always* needed together, an + embedded full representation is acceptable to avoid costly sequential requests. + In these cases, use an explicit opt-in such as ``?view=full`` or + ``?fields=`` so the heavy payload is never the default and clients + can still request only what they need. + +3. Document response shapes in OpenAPI, including minimal vs full variants. + +.. note:: + Endpoints that expose a flat list of nodes (e.g. a search result set or an enrollment + list) are also subject to :doc:`0032-standardize-pagination-usage`. For *tree-shaped* + responses, see the `Interaction with Pagination (ADR 0032)`_ section below. + +Relevance in edx-platform +========================= + +* **Deeply nested payloads**: The course blocks API (``lms/djangoapps/course_api/blocks/views.py``) + returns a tree of blocks with ``root`` and ``blocks`` (dict keyed by usage ID), each block + containing ``id``, ``type``, ``display_name``, ``children``, ``student_view_data``, etc. + Full trees can be large and hard to parse. +* **Existing flexibility**: Blocks API already supports ``requested_fields`` and + ``block_types_filter`` to reduce payload; a ``view=minimal`` or ``fields=...`` + convention would align with this ADR. +* **Modulestore/OLX**: ``openedx/core/djangoapps/olx_rest_api/views.py`` returns + nested block OLX; providing a minimal (e.g. IDs + types only) view would help + clients that only need structure. + +Interaction with Pagination (ADR 0032) +====================================== + +Tree-shaped endpoints are **out of scope** for ADR 0032's flat-list ``DefaultPagination`` +requirement, because "page N of a tree" is not well-defined when the tree structure itself +provides the navigation context. However, unbounded child lists within a tree still pose a +performance risk. When a node may have a large number of direct children (e.g. a course +chapter with hundreds of units, or a library with thousands of components), one of the +following two patterns MUST be used: + +**Pattern A — Depth cap + child-fetch URLs (recommended for tree fidelity)** + +Return the full structural representation up to a fixed maximum depth (default ``depth=2``). +For subtrees beyond that depth, return the child usage ID and a follow-up URL instead of +embedding the full subtree: + +.. code-block:: json + + { + "id": "block-v1:edX+Demo+2026+type@chapter+block@week1", + "type": "chapter", + "display_name": "Week 1", + "children": [ + { + "id": "block-v1:...", + "type": "sequential", + "display_name": "Lesson 1", + "children_url": "/api/courses/v1/blocks/block-v1:.../?depth=1" + } + ] + } + +This preserves tree semantics while bounding response size. The depth default and maximum +MUST be documented and honoured via a ``?depth=N`` query parameter. + +**Pattern B — Flat paginated child list (recommended when order/count matters more than +structure)** + +When clients primarily need to enumerate children (e.g. a library component picker or a +block-type filter), expose a flat paginated sub-resource following ADR 0032: + +.. code-block:: text + + GET /api/courses/v1/blocks//children/?page=1&page_size=50 + +This separates structural navigation (tree) from bulk enumeration (paginated flat list). + +**Guidance for** ``?fields=...`` **with large child sets** + +If a client requests a field that would expand a large child collection (e.g. +``?fields=children.student_view_data``), the server MUST NOT silently return all children +unbounded. Apply whichever pattern above is appropriate for the endpoint and document the +behaviour in OpenAPI. Returning HTTP 400 with a descriptive message is preferable to +silently truncating or returning an oversized payload. + +The Course Blocks API (``/api/courses/v1/blocks/``) is the canonical reference case: it +already supports ``?depth=N`` and ``requested_fields``. Any extension of that API MUST +continue to honour depth caps and MUST NOT add unbounded field expansions. + +Code example +============ + +**Query params for minimal representation:** + +New endpoints should use ``fields`` (explicit field selection) and/or ``view`` +(named preset) consistently. The existing Blocks API uses ``requested_fields`` +for backwards compatibility; new work should follow the ``fields`` convention. + +.. code-block:: text + + GET /api/courses/v1/blocks//?depth=1&fields=id,type,display_name + GET /api/course_structure/v1/?view=minimal + +**Response shape (minimal vs full):** + +.. code-block:: json + + // minimal (?view=minimal or ?fields=id,type,display_name,children) + { + "root": "block-v1:...", + "blocks": { + "block-v1:...": { "id": "...", "type": "chapter", "display_name": "Week 1", "children": ["..."] } + } + } + + // full: same structure but with student_view_data, completion, block_counts, etc. + +**Prefer IDs + follow-up over embedding:** + +.. code-block:: text + + GET /api/courses/v1/blocks/ → returns block IDs and types + GET /api/courses/v1/blocks// → returns full block when needed + +Consequences +============ + +* Pros + + * Improved performance and developer ergonomics. + * Easier integration for external services and AI agents. + +* Cons / Costs + + * Must maintain multiple representations: each ``view`` preset or ``fields`` + variant requires a separate serializer code path, its own test coverage, and a + documented schema entry in OpenAPI. All variants must be kept in sync whenever + the underlying data model changes (new fields, renamed keys, deprecated sub-objects). + Without versioning discipline this becomes a source of subtle divergence bugs + and undocumented breaking changes. + +Implementation Notes +==================== + +* Start with endpoints called out in the standardization notes (course structure, contentstore index, + xblock, progress). +* Measure payload size reduction and client performance improvements. + +References +========== + +* :doc:`0032-standardize-pagination-usage` — Pagination standardization ADR. Tree-shaped + endpoints are explicitly out of scope for ADR 0032's flat-list pagination requirement; + this ADR (0036) governs how deep-structure responses should bound child list sizes at + those same endpoints. +* `Open edX REST API Conventions — “Multiple Formats” `_