| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Standardizes the v0 ``YoutubeTranscriptCheckView`` + ``YoutubeTranscriptUploadView``
pair (``cms/djangoapps/contentstore/rest_api/v0/views/transcripts.py``) into a
single ``YoutubeTranscriptsViewSet`` applying the FC-0118 ADRs. The v0 views,
serializers, and URLs are untouched (ADR 0037) — this is a new, additive v1
surface for the same resource. Only the two YouTube transcript endpoints are
migrated here; the sibling ``TranscriptView`` (``/video_transcripts/...``) is
a different resource and is out of scope for this change.
ADR compliance:
* ADR 0025 - ``serializer_class`` is declared as a class attribute (the
ADR 0025 checklist requirement — schema generation and any
``getattr(view, 'serializer_class')`` caller depend on it existing even
when a view overrides per-action selection), and additionally
per-action via ``get_serializer_class`` (the two actions have different
response shapes), plus a ``get_serializer`` helper since plain
``viewsets.ViewSet`` has none. Response bodies are now actually built
through ``YoutubeTranscriptCheckSerializer`` / ``YoutubeTranscriptUploadSerializer``
instead of being declared-but-unused as in v0. Request bodies are also now
actually validated: both actions parse the ``data`` query parameter
themselves and run it through ``YoutubeTranscriptCheckRequestSerializer`` /
``YoutubeTranscriptUploadRequestSerializer`` via ``is_valid(raise_exception=True)``
*before* calling the legacy function, so a malformed ``data`` payload now
gets a real structured 400 instead of only ever validating a response
body. (A prior version of this view declared the request serializers only
inside ``@extend_schema`` and never instantiated them — that was a real
ADR 0025 violation, caught in review; see the ``check``/``upload``
docstrings for the fix and reasoning.)
* ADR 0026 - explicit ``authentication_classes`` + ``permission_classes``
declared on the viewset (no reliance on project defaults).
* ADR 0027 - ``drf_spectacular`` ``@extend_schema`` on both actions
(v0 had no schema annotation at all).
* ADR 0028 - both endpoints act on the same resource (a course's YouTube
transcript state) and neither is a real ORM-backed model, so this is a
plain ``viewsets.ViewSet`` (not ``ModelViewSet``), with ``check`` and
``upload`` as its two action methods. Routing is wired via explicit
``re_path`` entries calling ``YoutubeTranscriptsViewSet.as_view({'get':
'check'})`` / ``as_view({'post': 'upload'})`` in ``v1/urls.py`` rather
than ``DefaultRouter`` dynamic ``@action`` discovery — the course_id path
parameter here is not a router "detail" lookup on this resource's own
identity (the viewset has no ``list``/``retrieve``/collection of its
own), and explicit registration keeps the URL shape unambiguous and
independently reviewable while the view class itself remains a standard
DRF ``ViewSet``, satisfying the ADR's "migrate away from ad-hoc
``APIView``/legacy dispatch" intent. Query-count discipline: both actions
delegate to the existing ``check_transcripts`` / ``replace_transcripts``
legacy functions unchanged, so this migration introduces no new N+1s. No
``select_related``/``prefetch_related`` opportunity exists here — the
data path is modulestore/contentstore/VAL/YouTube-API calls, not a
Django ORM queryset, so that MUST doesn't apply to this resource (see
Enrollment v2's ``viewsets.ViewSet`` precedent for a non-ORM data path).
* ADR 0029 - ``StandardizedErrorMixin`` provides the standardized error
envelope. The legacy functions return a raw ``JsonResponse`` with a
``status`` field carrying the error message on failure (not the DRF
standard ``developer_message`` shape) — this view parses that JsonResponse
and re-raises as a DRF ``ValidationError`` so error responses go through
the standardized envelope. This is a deliberate shape change on the error
path only; the success-path response body is unchanged (see docstring on
each action). The ``ValidationError`` is raised with the *full* legacy
error body merged with an explicit ``error_code`` key (e.g.
``youtube_transcript_check_failed`` / ``youtube_transcript_upload_failed``)
— an earlier version of this view raised ``ValidationError`` with only
the truncated ``status`` string, silently discarding the rest of the
legacy ``transcripts_presence`` dict (``html5_local``, ``youtube_diff``,
etc., which are also present on the error path since ``error_response()``
only overwrites the ``status`` key on the full dict) and had no
``error_code`` at all, inconsistent with the 403 path's
``error_code='user_permissions'``. Fixed here as a low-risk change (it
only affects what is included in an already-thrown exception's detail).
* ADR 0030 - ``check`` remains ``GET`` (already idempotent - read-only
status probe, no writes). ``upload`` is changed from ``GET`` to ``POST``:
the v0 endpoint used GET for an operation that downloads transcripts from
YouTube and writes to VAL + modulestore, which is exactly the GET-mutates
violation this ADR targets. v1 fixes it: the operation is now a POST. The
``/upload`` URL segment is kept (arguably a verb - see ADR 0038 note
below) because this is a real, non-resource-shaped operation ("perform an
upload/replace"), not a CRUD create of an addressable sub-resource; POST
to a stable noun path is the ADR 0038 rule 10 escape hatch for exactly
this case, and it must be flagged as such in the OpenAPI description,
which is done below.
* ADR 0031 - considered merging check (read) and upload (write) into one
action selected by a ``mode``/``action`` field, per the ADR's merge test:
"share the same resource domain and differ only in the operation
applied". Decision: kept as **two separate actions** on one viewset
rather than fused into a single endpoint. Reasoning: the ADR's merge
target is endpoints that differ only in *operation* on an otherwise
identical request/response contract (e.g. generate/regenerate/toggle a
certificate, all POST, all returning a task handle). Check and upload
differ in HTTP semantics (GET vs POST), side effects (none vs
YouTube-download + VAL-write + modulestore-write), and response shape
(``html5_local``/``youtube_diff``/... vs ``edx_video_id``/``status``) -
merging them behind one ``mode`` field would force a GET-shaped read and
a POST-shaped write through one verb-agnostic entry point, which is a
worse fit than the ADR's own certificate-task example (three POSTs that
already shared one shape). They *do* still get the boilerplate-sharing
benefit of ADR 0031 by living on the same ``YoutubeTranscriptsViewSet``
with one class-level ``authentication_classes``/``permission_classes``
declaration, without forcing an artificial shared contract. Both actions
keep their own coarse (``@course_author_access_required`` on the URL's
``course_id``) plus specific (``has_course_author_access`` inside the
legacy ``_get_item`` against the *item's actual* course_key - relevant
for library content) permission layers, per the ADR's "do not flatten
authorization" requirement.
* ADR 0032 - out of scope. Neither action returns a list/collection.
* ADR 0033 - out of scope. Neither action takes filter/sort parameters.
* ADR 0034 - already compliant. ``authentication_classes`` is
``(JwtAuthentication, SessionAuthenticationAllowInactiveUser)`` - no
``BearerAuthentication``/``BearerAuthenticationAllowInactiveUser`` to
remove (v0 carried ``BearerAuthenticationAllowInactiveUser`` via
``@view_auth_classes()``; that is dropped here per the deprecation
policy). ``SessionAuthenticationAllowInactiveUser`` is kept explicitly so
inactive Studio authors can still reach the endpoint.
* ADR 0035 - out of scope. Not an MFE configuration endpoint.
* ADR 0036 - out of scope. Both response bodies are flat, small, fixed-key
objects (9 and 2 top-level fields respectively) with no nested
sub-objects or tree shape to collapse.
* ADR 0037 - this is a new v1 surface. The v0
``YoutubeTranscriptCheckView``/``YoutubeTranscriptUploadView``, their
serializers, and their URL entries are untouched and continue to serve
``GET`` on the old paths exactly as before.
* ADR 0038 - URLs are
``/api/contentstore/v1/youtube_transcripts/{course_id}/check/`` (GET) and
``/api/contentstore/v1/youtube_transcripts/{course_id}/upload/`` (POST),
replacing v0's ``.../youtube_transcripts/{course_id}/check?`` (optional
trailing ``?`` regex anti-pattern flagged by the survey). One level of
nesting (course -> check|upload) is used because neither ``check`` nor
``upload`` is an independently addressable resource with its own opaque
key - both only make sense scoped to a course, satisfying rule 8. Trailing
slash is now mandatory (rule 6). ``check``/``upload`` are technically verb
segments (rule 10); they are kept because this is the ADR-0031-considered
"genuine non-resource operation" case the rule explicitly carves out, and
each is marked as such via its ``@extend_schema`` description. ``api_name``
stays ``contentstore`` here (not renamed to ``authoring``) to stay
consistent with the rest of this same v1 mount (``xblock``, etc.) - a
platform-wide ``contentstore`` -> ``authoring`` rename is a bigger,
cross-viewset migration out of scope for this issue.
| Back | FazBrowse Home | New Git URL |
Related Issue: #39061
Standardizes the v0 YoutubeTranscriptCheckView + YoutubeTranscriptUploadView pair (cms/djangoapps/contentstore/rest_api/v0/views/transcripts.py) into a single YoutubeTranscriptsViewSet applying the FC-0118 ADRs. The v0 views, serializers, and URLs are untouched (ADR 0037) — this is a new, additive v1 surface for the same resource. Only the two YouTube transcript endpoints are migrated here; the sibling TranscriptView (/video_transcripts/...) is a different resource and is out of scope for this change.
ADR compliance: