[FC-0118] docs: add ADR for merging similar endpoints#38262
[FC-0118] docs: add ADR for merging similar endpoints#38262Abdul-Muqadim-Arbisoft wants to merge 1 commit intoopenedx:docs/ADRs-axim_api_improvementsfrom
Conversation
- Propose consolidation of narrow action-scoped URLs into unified parameterised views - Document certificate generation endpoints as primary consolidation candidate - Define action/mode parameter pattern and shared service layer approach
Abdul-Muqadim-Arbisoft
changed the title
deborahgu
left a comment
deborahgu
left a comment
There was a problem hiding this comment.
this makes some of the PR's I saw go through last year make so much more sense. 😀
| * **Use HTTP verbs exclusively (pure REST)**: Partially applicable — ``POST`` for create, | ||
| ``DELETE`` for unenroll — but breaks down for operations that do not map cleanly to HTTP verbs | ||
| (e.g., ``enable_certificate_generation``). A hybrid approach (HTTP verbs where natural, |
There was a problem hiding this comment.
This actually is so-called purely RESTful, honestly. 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. You could get very busy and add more of the task details into the URL (e.g. POST /api/instructor/v1/certificate_task/:taskname/:course_id) but it's not required to make purists required.
So I'd say something more like
* **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.
or suchlike.
| .. code-block:: python | ||
|
|
||
| # lms/djangoapps/instructor/views/api.py | ||
| class CertificateTaskView(APIView): |
There was a problem hiding this comment.
I assume we would keep this 3 distinct permissions, and the downstream methods would do the permission checks based on the payload. Am I correct about that?
| * Existing clients calling the legacy URLs require a migration period; deprecated aliases must be | ||
| maintained until adoption drops sufficiently. |
There was a problem hiding this comment.
at least in openedx code, I'm pretty sure that these are only called internally by the instructor djangoapp, right? Although come to think of it that's probably just because this work got done last year. 😆
|
approved but I added a suggested change. |
3 participants
Currently, Open edX APIs expose multiple narrow action-scoped endpoints for the same resource domain, duplicating permission checks, validation logic, and business logic across sibling views. This ADR proposes consolidating these fragmented endpoints into single parameterised DRF views backed by a shared service layer, using an action or mode parameter to distinguish operations
Issue: #38166