Skip to content

Commit 4fe0c8e

Browse files
[FC-0118] docs: add ADR for merging similar endpoints (#38262)
1 parent 73ae76c commit 4fe0c8e

1 file changed

Lines changed: 184 additions & 0 deletions

File tree

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
Merge Similar Endpoints
2+
=======================
3+
4+
:Status: Accepted
5+
:Date: 2026-03-31
6+
:Deciders: Open edX Platform / API Working Group
7+
:Technical Story: Open edX REST API Standards - Consolidation of fragmented same-resource endpoints into unified parameterised views
8+
9+
Context
10+
-------
11+
12+
Open edX APIs currently expose multiple endpoints that perform closely related operations with only
13+
minor variations in behaviour. Rather than consolidating these into a single parameterised resource,
14+
the platform has grown a proliferation of narrow, action-scoped URLs — each duplicating validation,
15+
permission-checking, and business logic from its siblings.
16+
17+
A prominent cluster illustrate the problem:
18+
19+
**Certificate endpoints** (``lms/djangoapps/instructor/views/api_urls.py``):
20+
21+
* ``enable_certificate_generation`` — enables or disables self-generated certificates for students
22+
* ``start_certificate_generation`` — triggers bulk certificate generation for all enrolled students
23+
* ``start_certificate_regeneration`` — regenerates certificates based on provided
24+
``certificate_statuses``
25+
26+
All three are registered in ``api_urls.py`` as separate ``path()`` entries and each independently
27+
validates ``course_id``, checks instructor permissions, and dispatches a background Celery task —
28+
with near-identical boilerplate in each view.
29+
30+
The impact of this fragmentation is felt across several dimensions:
31+
32+
* **Redundant code**: Permission checks, serializer logic, and audit-logging are re-implemented
33+
independently across views, making fixes and feature additions error-prone.
34+
* **Client complexity**: External systems and AI agents must discover, call, and handle errors for
35+
multiple endpoints to complete a single logical workflow.
36+
* **Inconsistent contracts**: Divergent request/response shapes between sibling endpoints create
37+
subtle integration bugs and complicate contract testing.
38+
39+
Decision
40+
--------
41+
42+
We will consolidate groups of closely related endpoints into **single, parameterised DRF views**
43+
(or shared service layers), using an ``action`` (or equivalent) request parameter to distinguish
44+
the operation being performed.
45+
46+
Implementation requirements:
47+
48+
* Identify endpoint groups that share the same resource domain and differ only in the operation
49+
applied to that resource.
50+
* Expose a single URL per resource group accepting an ``action`` or ``mode`` field (or using HTTP
51+
verbs semantically where REST conventions apply cleanly).
52+
* Move shared infrastructure, input validation, audit logging, response shaping, and the
53+
enforcement machinery for permissions, into a common service layer or mixin that all operations
54+
invoke. The distinct authorization requirements of the legacy endpoints must be preserved: the
55+
view performs a coarse access check, and each mode handler in the service layer enforces its
56+
own specific permission. Consolidation removes duplicated boilerplate; it does not flatten the
57+
authorization model.
58+
* Preserve backward compatibility via URL aliases or deprecation redirects for a defined transition
59+
window.
60+
* Document the unified endpoint schema in drf-spectacular / OpenAPI, including the enumerated set
61+
of valid ``action`` / ``mode`` values and their respective request/response shapes.
62+
63+
Relevance in edx-platform
64+
--------------------------
65+
66+
Confirmed fragmentation in the codebase:
67+
68+
* **Certificate views** (``lms/djangoapps/instructor/views/api_urls.py``, lines confirmed in
69+
master): The following three entries exist as separate ``path()`` registrations::
70+
71+
path('enable_certificate_generation', api.enable_certificate_generation,
72+
name='enable_certificate_generation'),
73+
path('start_certificate_generation', api.StartCertificateGeneration.as_view(),
74+
name='start_certificate_generation'),
75+
path('start_certificate_regeneration', api.StartCertificateRegeneration.as_view(),
76+
name='start_certificate_regeneration'),
77+
78+
Code example (target unified endpoint)
79+
---------------------------------------
80+
81+
**Proposed unified certificate task endpoint**:
82+
83+
.. code-block:: http
84+
85+
POST /api/instructor/v1/certificate_task/{course_id}
86+
Content-Type: application/json
87+
88+
{
89+
"mode": "generate"
90+
}
91+
92+
Valid ``mode`` values: ``generate``, ``regenerate``, ``toggle``.
93+
94+
**Example DRF view skeleton:**
95+
96+
.. code-block:: python
97+
98+
# lms/djangoapps/instructor/views/api.py
99+
class CertificateTaskView(APIView):
100+
"""
101+
Unified entry point for certificate generation lifecycle operations.
102+
103+
Authorization is enforced in two layers:
104+
105+
1. A coarse view-level check confirms the caller has instructor-level
106+
access to the course at all.
107+
2. Per-mode permission checks live inside the corresponding
108+
``CertificateTaskService`` method, preserving the distinct
109+
authorization requirements of the legacy endpoints
110+
(``enable_certificate_generation``,
111+
``start_certificate_generation``,
112+
``start_certificate_regeneration``).
113+
"""
114+
115+
VALID_MODES = {"generate", "regenerate", "toggle"}
116+
117+
def post(self, request, course_id):
118+
course_key = CourseKey.from_string(course_id)
119+
# Coarse authorization: must be an instructor on this course.
120+
_check_instructor_permissions(request.user, course_key)
121+
122+
mode = request.data.get("mode")
123+
if mode not in self.VALID_MODES:
124+
raise ValidationError({"mode": f"Must be one of: {self.VALID_MODES}"})
125+
126+
service = CertificateTaskService(course_key, request.user)
127+
# Each service method enforces its own mode-specific permission
128+
# before dispatching to the underlying task.
129+
result = getattr(service, mode)(request.data)
130+
return Response(result, status=status.HTTP_200_OK)
131+
132+
Consequences
133+
------------
134+
135+
Positive
136+
~~~~~~~~
137+
138+
* Clients implement a single integration point per resource domain, reducing onboarding friction
139+
for external systems and AI agents.
140+
* Shared validation, permission, and audit logic lives in one place, eliminating divergence between
141+
sibling endpoints.
142+
* OpenAPI schemas become more compact — a single operation object per resource instead of three
143+
or more.
144+
* Contract tests cover one endpoint per resource group, cutting test surface area without reducing
145+
coverage.
146+
* The certificate consolidation aligns with an already-open upstream issue (#36961), increasing
147+
likelihood of community acceptance.
148+
149+
Negative / Trade-offs
150+
~~~~~~~~~~~~~~~~~~~~~
151+
152+
* Existing clients calling the legacy URLs require a migration period; deprecated aliases must be
153+
maintained until adoption drops sufficiently.
154+
* The ``mode`` / ``action`` parameter pattern diverges from strict REST conventions; teams must
155+
agree on a consistent naming standard across endpoint groups.
156+
* A poorly designed service layer could become a "god object"; care must be taken to keep each
157+
operation handler cohesive and independently testable.
158+
159+
Alternatives Considered
160+
-----------------------
161+
162+
* **Keep per-action endpoints**: Rejected. The duplication cost compounds with every new operation
163+
and makes consistent error handling and logging practically impossible to enforce.
164+
* **Use HTTP verbs exclusively (pure REST)**: Not applicable. This is already RESTful.
165+
The noun is ``certificate_task``, the ``POST`` indicates that we are creating a
166+
certificate task, and the payload indicates what the task is going to be.
167+
* **GraphQL mutations**: Considered but out of scope for this iteration; the platform's existing
168+
REST ecosystem makes a full GraphQL migration impractical in the near term.
169+
170+
Rollout Plan
171+
------------
172+
173+
1. Implement the unified ``CertificateTaskView``; register
174+
legacy paths as deprecated aliases emitting a ``Deprecation`` response header.
175+
2. Identify and document additional endpoint groups sharing a resource domain. Add them to the
176+
placeholder table below.
177+
3. Announce a deprecation timeline to known API consumers and update developer documentation.
178+
4. Remove legacy aliases after the deprecation window closes (target: two named Open edX releases).
179+
180+
References
181+
----------
182+
183+
* Django REST Framework – Class-Based Views:
184+
https://www.django-rest-framework.org/api-guide/views/

0 commit comments

Comments
 (0)