|
1 | 1 | Merge Similar Endpoints |
2 | 2 | ======================= |
3 | 3 |
|
4 | | -:Status: Proposed |
| 4 | +:Status: Accepted |
5 | 5 | :Date: 2026-03-31 |
6 | 6 | :Deciders: Open edX Platform / API Working Group |
7 | 7 | :Technical Story: Open edX REST API Standards - Consolidation of fragmented same-resource endpoints into unified parameterised views |
@@ -49,8 +49,12 @@ Implementation requirements: |
49 | 49 | applied to that resource. |
50 | 50 | * Expose a single URL per resource group accepting an ``action`` or ``mode`` field (or using HTTP |
51 | 51 | verbs semantically where REST conventions apply cleanly). |
52 | | -* Move shared logic — permission checking, input validation, audit logging — into a common service |
53 | | - layer or mixin that all operations invoke. |
| 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. |
54 | 58 | * Preserve backward compatibility via URL aliases or deprecation redirects for a defined transition |
55 | 59 | window. |
56 | 60 | * Document the unified endpoint schema in drf-spectacular / OpenAPI, including the enumerated set |
@@ -93,19 +97,35 @@ Valid ``mode`` values: ``generate``, ``regenerate``, ``toggle``. |
93 | 97 |
|
94 | 98 | # lms/djangoapps/instructor/views/api.py |
95 | 99 | class CertificateTaskView(APIView): |
96 | | - """Unified entry point for certificate generation lifecycle operations.""" |
| 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 | + """ |
97 | 114 |
|
98 | 115 | VALID_MODES = {"generate", "regenerate", "toggle"} |
99 | 116 |
|
100 | 117 | def post(self, request, course_id): |
101 | 118 | course_key = CourseKey.from_string(course_id) |
| 119 | + # Coarse authorization: must be an instructor on this course. |
102 | 120 | _check_instructor_permissions(request.user, course_key) |
103 | 121 |
|
104 | 122 | mode = request.data.get("mode") |
105 | 123 | if mode not in self.VALID_MODES: |
106 | 124 | raise ValidationError({"mode": f"Must be one of: {self.VALID_MODES}"}) |
107 | 125 |
|
108 | | - service = CertificateTaskService(course_key) |
| 126 | + service = CertificateTaskService(course_key, request.user) |
| 127 | + # Each service method enforces its own mode-specific permission |
| 128 | + # before dispatching to the underlying task. |
109 | 129 | result = getattr(service, mode)(request.data) |
110 | 130 | return Response(result, status=status.HTTP_200_OK) |
111 | 131 |
|
@@ -141,10 +161,9 @@ Alternatives Considered |
141 | 161 |
|
142 | 162 | * **Keep per-action endpoints**: Rejected. The duplication cost compounds with every new operation |
143 | 163 | and makes consistent error handling and logging practically impossible to enforce. |
144 | | -* **Use HTTP verbs exclusively (pure REST)**: Partially applicable — ``POST`` for create, |
145 | | - ``DELETE`` for unenroll — but breaks down for operations that do not map cleanly to HTTP verbs |
146 | | - (e.g., ``enable_certificate_generation``). A hybrid approach (HTTP verbs where natural, |
147 | | - ``action`` / ``mode`` parameter otherwise) is acceptable. |
| 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. |
148 | 167 | * **GraphQL mutations**: Considered but out of scope for this iteration; the platform's existing |
149 | 168 | REST ecosystem makes a full GraphQL migration impractical in the near term. |
150 | 169 |
|
|
0 commit comments