Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,77 @@ def test_error_body_has_no_developer_message(self):
def test_instance_field_is_request_path(self):
response = self.client.get(_detail_url())
assert response.json()["instance"] == _detail_url()


# ---------------------------------------------------------------------------
# ADR 0036 — minimal-view regression tests
# ---------------------------------------------------------------------------
class TestXblockViewSetMinimalView(ModuleStoreTestCase, APITestCase):
"""
ADR 0036 — verify ``?view=minimal`` strips the xblock response down to
the structural fields enumerated in ``_MINIMAL_VIEW_FIELDS`` and leaves
the default (full) response untouched.
"""

_FULL_PAYLOAD = {
"id": TEST_LOCATOR,
"display_name": "Problem 1",
"category": "problem",
"children": [],
"has_children": False,
"studio_url": "/studio/...",
# Heavy / contextual fields that ``?view=minimal`` MUST drop:
"data": "<problem>...</problem>",
"metadata": {"weight": 1.0},
"fields": {"showanswer": "always"},
"student_view_data": {"...": "..."},
"edited_on": "2026-06-17T00:00:00Z",
"published": True,
}

def setUp(self):
super().setUp()
self.author = GlobalStaffFactory.create()
self.client.force_authenticate(user=self.author)

@patch(f"{_VIEW_MODULE}.retrieve_xblock_response")
def test_default_response_is_unchanged(self, mock_retrieve):
"""Without ``?view=minimal`` the response is the full handler payload."""
mock_retrieve.return_value = JsonResponse(self._FULL_PAYLOAD)
response = self.client.get(_detail_url())
assert response.status_code == status.HTTP_200_OK
body = response.json()
# Heavy fields must still be present in the default response.
assert "data" in body
assert "metadata" in body
assert "student_view_data" in body

@patch(f"{_VIEW_MODULE}.retrieve_xblock_response")
def test_minimal_view_strips_heavy_fields(self, mock_retrieve):
"""``?view=minimal`` drops data, metadata, fields, student_view_data, edited_on, published."""
mock_retrieve.return_value = JsonResponse(self._FULL_PAYLOAD)
response = self.client.get(_detail_url(), {"view": "minimal"})
assert response.status_code == status.HTTP_200_OK
body = response.json()
# Heavy fields MUST be dropped.
for dropped in ("data", "metadata", "fields", "student_view_data", "edited_on", "published"):
assert dropped not in body, f"ADR 0036: ?view=minimal must drop '{dropped}'"

@patch(f"{_VIEW_MODULE}.retrieve_xblock_response")
def test_minimal_view_keeps_structural_fields(self, mock_retrieve):
"""``?view=minimal`` keeps id, display_name, category, children, has_children, studio_url."""
mock_retrieve.return_value = JsonResponse(self._FULL_PAYLOAD)
response = self.client.get(_detail_url(), {"view": "minimal"})
body = response.json()
for kept in ("id", "display_name", "category", "children", "has_children", "studio_url"):
assert kept in body, f"ADR 0036: ?view=minimal must keep '{kept}'"
assert body["id"] == TEST_LOCATOR
assert body["category"] == "problem"

@patch(f"{_VIEW_MODULE}.retrieve_xblock_response")
def test_minimal_view_is_noop_for_non_json_payload(self, mock_retrieve):
"""Legacy ``?fields=graderType`` returns a non-dict body — minimal must be a no-op."""
mock_retrieve.return_value = JsonResponse("notgraded", safe=False)
response = self.client.get(_detail_url(), {"view": "minimal", "fields": "graderType"})
assert response.status_code == status.HTTP_200_OK
assert response.json() == "notgraded"
66 changes: 64 additions & 2 deletions cms/djangoapps/contentstore/rest_api/v1/views/xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,24 @@
* ADR 0026 - explicit authentication_classes + permission_classes
* ADR 0028 - consolidated into XblockViewSet via DefaultRouter
* ADR 0029 - standardized error envelope via StandardizedErrorMixin
* ADR 0036 - minimal/flattened views. ``retrieve`` accepts a ``?view=minimal``
query parameter that strips the (tree-shaped) xblock response to a small
set of structural fields. The full xblock response is kept as the default
for backwards compatibility with the existing Studio frontend; new clients
SHOULD opt into ``?view=minimal`` whenever the full nested payload is not
required.

Note on ``?fields=`` — the underlying ``retrieve_xblock_response`` already
interprets ``?fields=`` with **legacy semantics** as a "type of response"
selector (``?fields=graderType``, ``?fields=ancestorInfo``,
``?fields=customReadToken``). To avoid breaking existing callers, v1 does
NOT repurpose ``?fields=`` as the ADR 0036 CSV-subset selector — use
``?view=minimal`` instead. A future v2 may reconcile these names.
"""
import json
import logging

from django.http import JsonResponse
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
from opaque_keys import InvalidKeyError
Expand All @@ -33,6 +47,39 @@

log = logging.getLogger(__name__)

# ADR 0036 — top-level keys kept when ``?view=minimal`` is requested. Chosen so
# the response is structurally complete (callers can navigate the tree by id
# and fetch full nodes on demand) without any heavy/contextual fields
# (student_view_data, completion, OLX metadata, etc.).
_MINIMAL_VIEW_FIELDS = frozenset({
"id",
"display_name",
"category",
"children",
"has_children",
"studio_url",
})


def _apply_minimal_view(response):
"""
ADR 0036 — when ``?view=minimal`` was requested, drop every top-level key
not in :data:`_MINIMAL_VIEW_FIELDS` from ``response``. No-op for non-JSON
or non-2xx responses.
"""
if not isinstance(response, JsonResponse) or response.status_code >= 300:
return response
try:
body = json.loads(response.content.decode("utf-8") or "{}")
except (ValueError, AttributeError):
return response
if not isinstance(body, dict):
# If the handler returned a non-object payload (e.g. `?fields=graderType`
# which returns the grader-type value directly), there's nothing to
# filter — return the response untouched.
return response
return JsonResponse({k: v for k, v in body.items() if k in _MINIMAL_VIEW_FIELDS})


class XblockViewSet(StandardizedErrorMixin, viewsets.ViewSet):
"""
Expand All @@ -44,6 +91,12 @@ class XblockViewSet(StandardizedErrorMixin, viewsets.ViewSet):
PUT /api/contentstore/v1/xblock/{usage_key_string}/ → update
PATCH /api/contentstore/v1/xblock/{usage_key_string}/ → partial_update
DELETE /api/contentstore/v1/xblock/{usage_key_string}/ → destroy

Query parameters (ADR 0036, GET only):
?view=minimal Drop heavy / contextual fields from the response,
keeping only structural fields (id, display_name,
category, children, has_children, studio_url).
Default response is the full xblock payload.
"""

authentication_classes = (
Expand Down Expand Up @@ -98,8 +151,17 @@ def create(self, request):

@expect_json_in_class_view
def retrieve(self, request, usage_key_string=None):
"""Retrieve an xblock by its usage key."""
return retrieve_xblock_response(request, usage_key_string)
"""
Retrieve an xblock by its usage key.

ADR 0036 — honours ``?view=minimal``; everything else is delegated to
``retrieve_xblock_response`` (which keeps its legacy ``?fields=`` /
``?fields=ancestorInfo`` / ``?fields=customReadToken`` semantics).
"""
response = retrieve_xblock_response(request, usage_key_string)
if request.GET.get("view") == "minimal":
response = _apply_minimal_view(response)
return response

@expect_json_in_class_view
@validate_request_with_serializer
Expand Down
32 changes: 32 additions & 0 deletions cms/djangoapps/contentstore/rest_api/v3/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
* :data:`COMMON_ERROR_RESPONSES` – the shared ``@extend_schema(responses=...)``
fragment for the 401 / 403 / 404 cases every v3 course-scoped viewset
can raise.
* :func:`apply_field_selection` – ADR 0036 helper. Drops every top-level
key not listed in the caller's ``?fields=`` CSV. No-op when ``?fields=``
is absent. Use this when an action returns a wide flat object and clients
want to request a subset (e.g. ``?fields=id,display_name,courses``).
"""

from drf_spectacular.utils import OpenApiResponse
Expand Down Expand Up @@ -53,3 +57,31 @@ def resolve_course_key(course_key: str) -> CourseKey:
403: OpenApiResponse(description="The requester cannot access the specified course."),
404: OpenApiResponse(description="The requested course does not exist."),
}


def apply_field_selection(data, fields_csv):
"""
ADR 0036 — drop every top-level key not listed in ``fields_csv``.

Args:
data: a ``dict`` (typically ``serializer.data``). Anything else is
returned untouched.
fields_csv: the raw value of the ``?fields=`` query parameter. ``None``
or empty string → no filtering (the full ``data`` is returned).

Returns:
A new ``dict`` containing only the requested top-level keys, or the
original ``data`` if filtering is not applicable.

Note:
Only top-level keys are honoured. Dotted paths (``?fields=children.x``)
are stripped to their first segment (``children``) — full dotted-path
traversal is intentionally left to a future implementation per the
ADR 0036 guidance to "reject silent over-fetching" via that syntax.
"""
if not fields_csv or not isinstance(data, dict):
return data
wanted = {name.strip().split(".", 1)[0] for name in fields_csv.split(",") if name.strip()}
if not wanted:
return data
return {key: value for key, value in data.items() if key in wanted}
13 changes: 13 additions & 0 deletions cms/djangoapps/contentstore/rest_api/v3/views/authoring_grading.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@
legacy ``course_id``. Since this is a brand-new versioned API, no
deprecated alias is needed — clients on the v0 endpoint continue to use
``course_id`` there.
* ADR 0036 – **largely out of scope.** The ``CourseGradingModelSerializer``
response is a single top-level ``graders`` list of small fixed-shape
objects (type, min_count, drop_count, short_label, weight, id) — no
tree nesting, no embedded sub-objects, no ``children`` field, no wide
flat object that would benefit from ``?view=minimal`` / ``?fields=``.

The one ADR 0036 concern is anti-pattern #3 (unbounded child list): the
``graders`` array has no upper bound in the serializer. In practice each
course has typically ≤8 graders (Homework, Lab, Exam, etc.) and the
update flow is exercised only by course-authoring staff, so the
real-world payload is always small. A hard cap is enforced upstream of
this endpoint by :func:`CourseGradingModel.update_from_json`; we surface
that as a documentation note rather than re-implement the bound here.

Permission model note:
PR #38363 proposed a class-level ``HasStudioReadAccess`` permission. The
Expand Down
100 changes: 94 additions & 6 deletions cms/djangoapps/contentstore/rest_api/v3/views/course_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@

from cms.djangoapps.contentstore.rest_api.v1.serializers import CourseDetailsSerializer
from cms.djangoapps.contentstore.rest_api.v1.views.course_details import _classify_update
from cms.djangoapps.contentstore.rest_api.v3.utils import COMMON_ERROR_RESPONSES, resolve_course_key
from cms.djangoapps.contentstore.rest_api.v3.utils import (
COMMON_ERROR_RESPONSES,
apply_field_selection,
resolve_course_key,
)
from cms.djangoapps.contentstore.utils import update_course_details
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
from openedx.core.djangoapps.authz.decorators import user_has_course_permission
Expand All @@ -55,6 +59,68 @@
location=OpenApiParameter.PATH,
)

# ADR 0036 — document the minimal/full response variants in OpenAPI (decision #3).
# Declaring these as query parameters is what makes the presets discoverable by
# OpenAPI consumers (Swagger UI, generated SDK clients, etc.). The 200 response
# schema below points at the full ``CourseDetailsSerializer``; ``?view=minimal``
# returns the subset of top-level keys listed in :data:`_MINIMAL_VIEW_FIELDS`.
_VIEW_QUERY_PARAMETER = OpenApiParameter(
name="view",
description=(
"ADR 0036 response preset. ``minimal`` drops heavy fields (overview, "
"syllabus, description, instructor_info, learning_info, banner/video "
"assets, license) leaving only identification, schedule, and flags. "
"Omit the parameter to receive the full response."
),
required=False,
type=str,
location=OpenApiParameter.QUERY,
enum=["minimal"],
)
_FIELDS_QUERY_PARAMETER = OpenApiParameter(
name="fields",
description=(
"ADR 0036 explicit field selection. Comma-separated list of top-level "
"keys to include in the response (e.g. ``course_id,title,start_date``). "
"When combined with ``?view=``, the preset is applied first and "
"``?fields=`` is applied to the result. Unknown keys are silently "
"skipped."
),
required=False,
type=str,
location=OpenApiParameter.QUERY,
)

# ADR 0036 — the ``CourseDetailsSerializer`` has ~40 top-level fields plus a
# nested ``instructor_info`` sub-object with bios and image URLs and a
# ``learning_info`` long-form list. When ``?view=minimal`` is requested,
# everything outside :data:`_MINIMAL_VIEW_FIELDS` is dropped so server-to-server
# and AI-agent callers can fetch just the identification + schedule + flags
# without paying for the heavy text and embedded sub-objects.
_MINIMAL_VIEW_FIELDS = frozenset({
"course_id",
"org",
"run",
"title",
"subtitle",
"language",
"self_paced",
"start_date",
"end_date",
"enrollment_start",
"enrollment_end",
"certificate_available_date",
"certificates_display_behavior",
"has_changes",
})


def _apply_view_preset(data, view_preset):
"""ADR 0036 — drop everything outside ``_MINIMAL_VIEW_FIELDS`` when ``?view=minimal``."""
if view_preset != "minimal" or not isinstance(data, dict):
return data
return {key: value for key, value in data.items() if key in _MINIMAL_VIEW_FIELDS}


class CourseDetailsViewSet(StandardizedErrorMixin, viewsets.ViewSet):
"""
Expand All @@ -78,12 +144,21 @@ class CourseDetailsViewSet(StandardizedErrorMixin, viewsets.ViewSet):

@extend_schema(
summary="Retrieve a course's details",
description="Get an object containing all the course details for the specified course.",
parameters=[_COURSE_ID_PARAMETER],
description=(
"Get an object containing the course details for the specified course. "
"Supports the ADR 0036 ``?view=minimal`` preset and ``?fields=`` "
"explicit field selection (see the parameter descriptions for details)."
),
parameters=[_COURSE_ID_PARAMETER, _VIEW_QUERY_PARAMETER, _FIELDS_QUERY_PARAMETER],
responses={
200: OpenApiResponse(
response=CourseDetailsSerializer,
description="Course details retrieved successfully.",
description=(
"Course details retrieved successfully. The schema below is "
"the full default response; when ``?view=minimal`` and/or "
"``?fields=`` is supplied, the response contains a subset of "
"these top-level keys (see ADR 0036)."
),
),
**COMMON_ERROR_RESPONSES,
},
Expand All @@ -95,6 +170,16 @@ def retrieve(self, request: Request, course_id: str):
**Example Request**

GET /api/contentstore/v3/course_details/{course_id}/
GET /api/contentstore/v3/course_details/{course_id}/?view=minimal
GET /api/contentstore/v3/course_details/{course_id}/?fields=course_id,title

ADR 0036:
* ``?view=minimal`` drops heavy fields (overview, syllabus, description,
instructor_info, learning_info, banner/video assets, license, etc.)
leaving only identification, schedule, and flags.
* ``?fields=...`` keeps an arbitrary CSV subset of top-level keys.
* ``?fields=`` and ``?view=`` may be combined — ``?view=minimal``
is applied first, then ``?fields=`` is applied to the result.
"""
course_key = resolve_course_key(course_id)
if not user_has_course_permission(
Expand All @@ -106,8 +191,11 @@ def retrieve(self, request: Request, course_id: str):
self.permission_denied(request)

course_details = CourseDetails.fetch(course_key)
serializer = self.serializer_class(course_details)
return Response(serializer.data)
data = self.serializer_class(course_details).data
# ADR 0036 — preset first, then explicit CSV subset.
data = _apply_view_preset(data, request.query_params.get("view"))
data = apply_field_selection(data, request.query_params.get("fields"))
return Response(data)

@extend_schema(
summary="Update a course's details",
Expand Down
Loading
Loading