Skip to content

feat: standardize the user directory, preference and time-zone APIs as /api/user/v2/ - #39237

Draft
Abdul-Muqadim-Arbisoft wants to merge 2 commits into
openedx:masterfrom
edly-io:feat/api-adr-user-v2-phase-a
Draft

Abdul-Muqadim-Arbisoft wants to merge 2 commits into
openedx:masterfrom
edly-io:feat/api-adr-user-v2-phase-a

Conversation

@Abdul-Muqadim-Arbisoft

Copy link
Copy Markdown
Contributor

Summary

This is phase A of five for the [API] User functional area
(#39058), under the API standardization
umbrella #38137. The area is split A → E
on that issue's plan; this PR carries A only — seven read-only operations — and B, C, D and E
are separate PRs
. B (accounts and profile images), C (preferences and email opt-in) and D (the
retirement pipeline and username replacement) follow this one; E (registration, login session,
validation and password reset) is not approved to build
and is listed here only so the phase count
is not mistaken for four.

Phase A is deliberately the smallest blast radius in the area — the user directory, the
stored-preference listings and the time-zone reference list — and it carries the shared wiring
(the mount, the schema filter, the deprecation hook, the pagination and error building blocks) that
the later phases reuse.

Decisions applied here come from
0025,
0026,
0027,
0028,
0029,
0030,
0031,
0032,
0033,
0034,
0036,
0037,
0038, OEP-66 and OEP-69.

Why a new version rather than an in-place fix. Every one of these seven operations changes
contract: the error body moves from {developer_message, field_errors} to the RFC 9457 envelope,
the time-zone list grows a pagination envelope where it returned a bare unbounded array, the stored
preference row stops embedding the owner's entire record, the member addresses stop using database
ids, and the gate changes from the deprecated X-Edx-Api-Key header to an authenticated staff
caller. A client parsing any of those would break. /api/user/v1/ is untouched and keeps serving
every one of them for its deprecation window.

Who has to do something. Only operators calling the four API-key-gated directory endpoints
server to server: /api/user/v2/ does not accept X-Edx-Api-Key and expects a JWT or session for a
staff user instead. v1 continues to accept the key. No front end and no mobile app calls any of
these seven addresses (checked by grepping the master tarballs of frontend-platform,
frontend-app-account, frontend-app-profile, frontend-app-authn and openedx-app-android;
openedx-app-ios and openedx-platform-sdk were not readable and are named as unchecked).

Two of the six gates are red and are reconciled rather than argued away — see Gate report.
Neither is described anywhere in this body as passing.

Reference detail

Three reference sections live in the first comment below, because the complete body exceeds GitHub's 65,536-character limit for a pull request description:

  • What else a caller or a client generator sees — every caller-visible change, enumerated
  • Backward compatibility — the evidence that no existing address, name or component moved
  • Follow-ups — the library-first candidates this pass deliberately did not fix locally

Nothing was cut; the split is purely a length limit.

Change table

Commit Endpoint(s) Legacy behaviour kept New behaviour
feat: add /api/user/v2/ users, user_preferences and time_zones GET /api/user/v2/users/ ← GET /api/user/v1/users/ same row fields (id, url, email, name, username, preferences), same pagination envelope member address keyed by username; username/email/is_active filters; ordering on username and date_joined with a deterministic default; ?view=minimal and ?fields=; RFC 9457 errors; staff auth
GET /api/user/v2/users/{username}/ ← GET /api/user/v1/users/{id}/ same fields addressed by username; a missing user is a typed 404 instead of an untyped one
GET /api/user/v2/users/?preference_key= ← GET /api/user/v1/preferences/{pref_key}/users/ same rows the parent key moves into the query string, on the one users collection
GET /api/user/v2/users/?forum_role=&course_key= ← GET /api/user/v1/forum_roles/{name}/users/ same rows for an existing role the GET no longer creates the role; an unknown role returns an empty page; an unparsable course_key is a 400 envelope where v1 let InvalidKeyError escape and answered 500; a missing one keeps the 400 it already had at v1, with the body moving from {"detail": "course_id must be specified"} to the envelope's errors.course_key
GET /api/user/v2/user_preferences/ ← GET /api/user/v1/user_prefs/ same key/value, same pagination envelope flat row (username instead of the embedded user object, and a user member that is null unless ?view=full restores the embedding); key/username filters, and the legacy ?user=<id> filter refused with a 400 rather than ignored; constant query count
GET /api/user/v2/user_preferences/{username},{key}/ ← GET /api/user/v1/user_prefs/{id}/ same key/value addressed by the user and key that identify it, not a database id
GET /api/user/v2/time_zones/ ← GET /api/user/v1/preferences/time_zones/ identical rows, identical order, same fallback for a well-formed but unknown country_code paginated instead of an unbounded array; a malformed country_code is a 400 envelope
feat: publish the user API in the LMS schema — enrollment schema unchanged: every enrollment operationId and tag is byte-identical the LMS document publishes this version's five addresses and the seven they supersede; SCHEMA_PATH_PREFIX covers both areas; title becomes "LMS API"; the seven superseded addresses are marked deprecated: true; user operations are grouped under a user tag wherever the generator would otherwise group them by version number, so the two areas never share a tag group. Every operationId and every tag the service-wide /api-docs document already published is unchanged, measured document against document

ADRs applied

  • 0025 serializers — serializer_class on all three viewsets, help_text on every field, method fields typed, query parameters validated by a serializer where the data path is not a queryset
  • 0026 permissions — permission_classes on every view; (IsAuthenticated, IsAdminUser) on the directory, (AllowAny,) declared deliberately on the public reference list; the deprecated ApiKeyHeaderPermission is not carried into v2
  • 0027 schema / docs — @extend_schema per operation with summary, description, parameters and a response per status code; the schema filter extended so the mount publishes
  • 0028 viewsets — ReadOnlyModelViewSet for the two ORM resources, plain ViewSet for the service-backed one, all three registered on a DRF router, with prefetching matched to what the serializers read. One of the six generated routes is replaced by a hand-declared one, because a router emits a single lookup parameter and the preference member is identified by the {username},{key} pair; the matrix row below carries the detail
  • 0029 errors — StandardizedErrorMixin first in the MRO, DRF exceptions only, no internal text in any response
  • 0030 GET idempotence — no writes on any GET path; the legacy get_or_create on the forum-role listing is dropped
  • 0031 merged endpoints — the three user listings become one users collection with filters
  • 0032 pagination — the seven-member envelope on all three lists, including the previously unbounded time-zone list
  • 0033 filtering — FilterSet + OrderingFilter on the ORM lists, serializer-validated parameters on the non-ORM list, OEP-68 parameter names
  • 0034 authentication — the platform default pair everywhere; no authentication_classes line anywhere in the diff
  • 0035 MFE config — not applicable: no endpoint in this phase serves site or front-end configuration. The nearest candidate, the time-zone list, is static reference data with no site settings and no user context attached
  • 0036 nested JSON — ?view=minimal on the directory, ?view=full on the preference rows, ?fields= on both; the time-zone row has two scalar fields and needs neither
  • 0037 versioning — not ticked because one clause is deliberately unmet: all new code is in a new v2/ package, no superseded file is in the diff, and the superseded addresses are flagged in the schema — but they carry no docstring marker, which decision 4 also asks for. The reason (FC-0118 review, OEP-21 timing) and the follow-up form are in the matrix row and in Deprecations
  • 0038 URL structure — full-prefix mount, trailing slashes, snake_case version-free names, no database ids in addresses, every v1 address still resolving. Read with the Gate report: the urls gate is red on the version-level JSON-404 fallback, which is not an address
  • OEP-66 queryset scoping — ScopedQuerysetMixin with FullScopePolicy() on both ORM lists, with permission, scoping and filtering as three separate layers
  • OEP-69 conventions — resource-shaped nouns, one collection per resource, the parent key in the query string, actions expressed as filters rather than as URL verbs
  • SDK tag — deliberately not applied in this phase: no operation here carries tags=["openedx-platform-sdk"], and SDK tagging starts at phase B. The learner-facing account and preference endpoints are the SDK candidates; an admin-only directory is not, and tagging it would put it in a generated client for no caller

Compliance matrix

Decision Status How it is met, or the concrete reason it is excluded Evidence (test / gate)
0025 serializers met UserDirectorySerializer, UserPreferenceRecordSerializer, TimeZoneSerializer declared on their views; help_text on every field; SerializerMethodFields annotated with extend_schema_field so the published types are real; TimeZoneQuerySerializer validates country_code. Component names checked against the base schema — UserPreference and CountryTimeZone were already taken by the v1 serializers, so the new ones are UserPreferenceRecord and TimeZone v2/tests/test_schema.py::test_user_list_publishes_its_filters, ::test_pagination_envelope_is_published_in_full; v2/tests/test_views.py::test_row_shape_matches_the_declared_fields
0026 permissions met (IsAuthenticated, IsActiveUser, IsAdminUser) on UserViewSet and UserPreferenceViewSet; (AllowAny,) declared explicitly on CountryTimeZoneViewSet because the platform sets no default permission and the list is public on purpose. IsActiveUser is a four-line class in v2/permissions.py, written because neither DRF's permissions nor edx-drf-extensions 10.9.0 ship one that reads is_active, and a token is accepted until it expires without the account being re-read: without it a staff account deactivated after its token was issued would keep reading the directory. No inline permission logic in any handler the deny-path tests listed in the Authorization section below, one per refusal, including DeactivatedStaffCallerTests (7 tests: both halves of the default authentication pair, every staff-gated address, and the still-active account admitted)
0027 schema / docs met every operation carries summary, description, its parameters and one response per status code; tags=["user"]; the LMS pre-processing filter widened so /api/user/v2/ and the seven addresses it supersedes reach the published document. LmsAutoSchema puts every user operation in the user tag group of the document that trims the area off the path, where the derived tag would otherwise be the version number; it renames nothing. The service-wide /api-docs document, which trims nothing, keeps the operationId and the tag it derived for each of these addresses before this change v2/tests/test_schema.py (23 tests); lms/lib/tests/test_spectacular.py (12 tests, including PublishedDocumentTest over the generated document and UserOperationIdTest); schema_diff.py compare → PASS (0 BREAKING, 18 WARN, 12 INFO), read with the warning note under Gate report
0028 viewsets met data path decides the base class: direct ORM → ReadOnlyModelViewSet, get_country_time_zones() → plain ViewSet. All three are registered on SnakeCaseRouter, a four-line SimpleRouter subclass that generates <basename>_<action> names so the published names are the ones reverse() and resolve() use. One deviation, deliberate: the preference member address is declared by hand with re_path and the router's own member route for that viewset is dropped by name (without_route(), which raises ImproperlyConfigured at import when the router generates no route under that name, so a renamed basename cannot quietly restore the database-id address), because a DRF router generates exactly one lookup parameter per registered viewset and this resource is identified by two — {username},{key} — with the key free-form enough to contain a comma itself. The hand-declared route serves the same viewset's retrieve action, so the resource is one viewset with one set of permissions, and no address keyed by a database id is served at all. prefetch_related("preferences").select_related("profile") kept on the directory; select_related("user", "user__profile") added to the preference list, with prefetch_related("user__preferences") only when the owner is embedded v2/tests/test_urls.py::test_addresses_resolve_to_the_version_two_viewsets (each address against the viewset and action it routes to), ::test_preference_member_address_splits_into_username_and_key, ::test_a_key_containing_a_comma_belongs_to_the_key and ::test_no_database_id_member_address_is_served for the hand-declared route and the dropped one, ::test_the_router_route_that_is_replaced_has_to_exist for the replacement being refused when it matches nothing; v2/tests/test_query_counts.py (22 tests: the five database-backed lists and the two member addresses over 1/5/25 rows, and the time-zone list over page sizes 1/10/100, since its rows are not database rows), of which ::test_the_earlier_preference_list_pays_per_row_where_this_one_does_not is the before-and-after: the same rows read through both versions in one test, 15 / 35 / 135 queries at version 1 for 1 / 5 / 25 owners against 15, constant, here
0029 errors met StandardizedErrorMixin first in the MRO on all three viewsets; validation errors raised with constant messages; Http404 from the generic member lookup translated to NotFound so the envelope is typed not-found and not internal; a refused method and an unanswerable Accept header named through two subclasses this version registers for itself, leaving every other API's classification untouched; no exception text or upstream string reaches a response every assert_error_envelope(...) call in v2/tests/test_views.py, including RefusedRequestTests (5 tests: the 405 and 406 envelopes, and another API's 405 asserted unchanged); v2/tests/test_parity.py::test_unauthenticated_bodies_differ_as_declared and ::test_forbidden_bodies_differ_as_declared record the legacy body beside the new one
0030 GET idempotence met no domain write on any GET path. The legacy forum-role listing calls Role.objects.get_or_create on every GET; the v2 filter does not, so an unknown role returns an empty page instead of creating one twelve CaptureQueriesContext captures — GET and HEAD on each of the five read addresses, plus the forum-role filter and the ?view=full embedding — asserting no INSERT/UPDATE/DELETE, plus ::test_unknown_forum_role_returns_no_rows_and_creates_nothing, which also asserts the Role row count. Hygiene H8 is a grep over get/list/retrieve bodies and cannot see a write in a service module, so a green hygiene gate is not evidence for this row — the SQL captures are
0031 merged endpoints met users, preferences/{key}/users and forum_roles/{name}/users are one resource addressed three ways, so they merge onto /api/user/v2/users/ with filters. No other merge was taken: the partner-report and retirement groups are different resources and belong to later phases v2/tests/test_parity.py::test_preference_key_filter_parity, ::test_forum_role_filter_parity
0032 pagination met UserApiPagination, a DefaultPagination subclass that overrides only get_paginated_response_schema, because the library publishes four of the seven members it returns. The time-zone list paginates through IterablePaginationMixin where it was previously unbounded (433 rows) v2/tests/test_schema.py::test_pagination_envelope_is_published_in_full and ::test_published_pagination_envelope_matches_the_response_body, which compares the published component against a real response body; v2/tests/test_views.py asserts the seven members and the page positions; ::PageSizeCapTests asserts that ?page_size=1000 returns one maximum page of 100 on all three lists, measured against 104, 102 and 433 live rows, so a later max_page_size override cannot unbound the anonymous time-zone list unnoticed
0033 filtering met filter_backends = (DjangoFilterBackend, OrderingFilter) plus a FilterSet on both ORM lists, ordering_fields and a deterministic default ordering (the legacy lists have no ORDER BY at all). The time-zone list is not a queryset, so its one parameter is serializer-validated. course_key is the OEP-68 spelling and owes no alias: the legacy listing carried the role and the course in the path, so no query parameter was renamed. The preference list is the one place a legacy query parameter is dropped — /api/user/v1/user_prefs/?user=<id> — and it is dropped rather than aliased: this list names owners by username, and ?user= is refused with a 400 naming the parameters it does read, so the client that ported by changing the version segment is told rather than handed every user's rows. No parameter of this version is deprecated. Each view's accepted names are assembled from its own declarations (filter set, sort backend, paginator, query serializer), so a filter that is added or renamed cannot fall out of step with the check. The published course_key example is percent-encoded, because an unencoded + arrives as a space and is refused v2/tests/test_views.py filter and ordering tests (14, including the three that pin is_active: each side narrows the page, an empty value narrows nothing, and ?is_active=maybe is a 400 envelope rather than a silently unfiltered 200); v2/tests/test_schema.py::test_user_list_publishes_its_filters, which also asserts is_active is still published as a boolean, and ::test_preference_list_publishes_its_filters, which pins the published parameter set and that none of them is deprecated; v2/tests/test_views.py::UnknownQueryParameterTests (16 tests: each of the five addresses refuses a name it does not read, names every unknown one at once, and answers every name it does read), ::test_the_owner_id_the_earlier_list_took_is_refused_rather_than_ignored, ::test_an_answered_page_announces_no_deprecated_parameter, ::test_the_published_course_key_example_can_be_pasted_into_a_query_string and ::test_a_course_key_whose_plus_signs_are_not_encoded_is_refused
0034 authentication met none of the three views declares authentication_classes; all use the platform default JWT + session pair. The legacy views' ApiKeyHeaderPermission is a permission class, not an authenticator, and is not carried forward. The two halves of that pair disagree about a deactivated account: the session half refuses one, and a token issued for one is still accepted, because only the session half reads the flag. That disagreement is closed above the authenticators rather than inside them, by the IsActiveUser permission both staff-gated viewsets declare, so such an account is refused on both paths — 401 authn through its session, 403 authz through its token. The operations also publish no security requirement and the document carries no securitySchemes, because the generator has no authentication extension for the platform's default classes; the same is true of all 25 enrollment operations in this document, and giving them one is a platform change rather than a change to this package hygiene H4 clean; the anonymous, learner and admin tests in v2/tests/test_views.py exercise both default authenticators; ::DeactivatedStaffCallerTests (7 tests) pins each half's answer to a deactivated staff account, at every staff-gated address
0035 MFE config excluded no endpoint in this phase serves site or front-end configuration, and none attaches user data to a cached or unauthenticated response. The time-zone list is reference data derived from the time-zone database —
0036 nested JSON met the directory row embeds the user's whole preference mapping, so ?view=minimal reduces it to (id, username, email); the legacy preference row embeds the owner's entire record including that owner's preferences, so v2 returns username by default and ?view=full restores the embedding; ?fields= projects both v2/tests/test_views.py::test_minimal_view_returns_only_identifying_fields (×2), ::test_full_view_embeds_the_owning_user (×2), ::test_fields_parameter_* (×3); v2/tests/test_parity.py::test_full_preset_reproduces_the_legacy_embedded_user; ::test_an_unknown_response_preset_is_refused on all four shaped addresses and UserListViewTests::test_the_preset_that_belongs_to_the_preference_list_is_refused_here, with ::test_an_empty_response_preset_returns_the_default_shape pinning the empty value
0037 versioning met, with one recorded deviation: no docstring marker — FC-0118 review, OEP-21 timing (box left unticked) every line of new code is in openedx/core/djangoapps/user_api/v2/. No superseded module is in the diff — in particular none carries a deprecation notice, a warnings.warn or a comment. ADR 0037's decision 4 asks for the superseded version to be marked "in the OpenAPI schema and in the endpoint's docstring"; the schema flag is set and the docstring marker is not, because the FC-0118 reviewer asked for the depr notes to be dropped from the replacement PRs (#39100, #39102) and OEP-21's own marker is a one-line issue link added once the DEPR ticket reaches Plan Accepted, not a notice. If a reviewer wants a marker, the answer is that one line, in a follow-up check_old_versions_untouched.sh with 14 --protect paths → PASS, with the superseded modules absent from the diff — read with the allowed: 0 caveat under Backward compatibility; v2/tests/test_schema.py::test_only_superseded_addresses_are_marked_deprecated pins the schema flag on exactly the seven addresses
0038 URL structure met path('api/user/v2/', ...) with the full prefix; trailing slash on every address; names user_list, user_detail, user_preference_list, user_preference_detail, time_zone_list — snake_case, version-free, unique; member addresses use the username, and the preference member uses the {username},{key} pair rather than a row id. The namespace is user_v2 — the domain and the version, never the Django app that implements it — so phases B to E extend one namespace for /api/user/. Two collection names were weighed rather than inherited: user_preferences/ repeats the api_name, which the stutter clause would otherwise rule out, and is kept because phase C publishes the caller's own preference document at accounts/{username}/preferences/ and a bare preferences/ here would read as that resource rather than as the cross-user index of stored rows; time_zones/ stays under this api_name because the list exists to populate the time_zone preference, and an area that needs the same reference data links to this canonical address instead of publishing a second one. Both namings are open questions rather than settled ones — Open decisions, item 2 v2/tests/test_urls.py (24 tests: reverse literals, resolution with routed_actions(), v1 untouched, no database-id address, JSON refusals, and RouteShapeTests, which pins that every pattern in the URLconf but the trailing fallback is named and slash-terminated); v2/tests/test_url_mount.py (4 tests: the two authn addresses under the same prefix still reach user_authn); url_snapshot.py check → FAIL (2 FAIL, 1 WARN), every finding on the fallback pattern and none on an address, reconciled under Gate report
OEP-66 queryset scoping met both ORM lists use ScopedQuerysetMixin with FullScopePolicy(): a documented pass-through, because the permission classes already restrict the endpoint to platform administrators, who may see every row. The three layers stay separate — permission classes, then the policy, then filter_queryset the deny tests cover layer 1 and the filter tests cover layer 3; swapping in a narrower policy requires no change to either
OEP-69 conventions met one collection per resource, plural resource nouns, the parent key in the query string rather than in the path, and no verb in any address v2/tests/test_urls.py

In-place edits

Seven pre-existing files are in the diff. Plan §8 originally proposed three (lms/urls.py,
lms/lib/spectacular.py, lms/envs/common.py); a repair pass added the fourth after ruling on it,
and the last three were found during implementation, not predicted by the plan. Each is named here
with its own justification and its sanctioned ADR 0037 category, because the PR — not the versions
gate, which never consults --allow for any of them — is what has to justify them.

File Sanctioned category Justification and evidence
lms/urls.py 3 — URL conformance Mounting the new version under its full prefix is the one edit a new version cannot avoid. One added line (path('api/user/v2/', include('openedx.core.djangoapps.user_api.v2.urls'))), placed before the api/user/ include so resolution does not depend on user_api/urls.py never growing a v2/ pattern. Evidence: url_snapshot.py check reports no address-preservation finding on any base route; test_urls.py::LegacyAddressTests proves the v1 surface is unaffected; test_url_mount.py (4 tests) proves the two authn addresses under the same prefix still reach user_authn, and goes red when a catch-all is inserted into the v2 URLconf
lms/lib/spectacular.py 3 — URL conformance (schema corollary) The deprecated: true flag and the published names must live outside the frozen view modules; this hook is where the LMS schema already does that for enrollment. The pre-processing filter now admits, besides enrollment vN, (a) /api/user/v2/ addresses whose view comes from the user_api.v2 package — not the bare prefix, since older apps are mounted under the same prefix — and (b) the seven exact superseded paths, listed one at a time in a new SUPERSEDED_USER_PATHS tuple, never a /api/user/v1/ prefix, so an address whose replacement nobody has written cannot be marked. LmsAutoSchema gains a tag rule for /api/user/ paths, applied only where the tag the generator derives would be a version number, which leaves the service-wide document's tags exactly as they were; it gains no operationId rule, so no published name changes. COLLIDING_LEGACY_PATH and LEGACY_V2_PREFIX — #39078's enrollment-only _legacy machinery — keep their contents and no parallel hook was added. Evidence: the twelve tests in lms/lib/tests/test_spectacular.py (the filter, the deprecation hook with foreign paths in the fixture, the enrollment _legacy suffix, the user ids and tags, and three over the generated document)
lms/envs/common.py 3 — URL conformance (schema corollary) The filter and prefix the mount needs in order to publish at all. TITLE → LMS API (the document now carries two areas), SCHEMA_PATH_PREFIX → r'/api/(enrollment|user)', which strips the area from both mounts so no enrollment operationId moves. POSTPROCESSING_HOOKS is unchanged in content and postprocess_schema_enums is still first, pinned by test_schema.py::test_enum_post_processing_runs_before_the_deprecation_hook. Evidence: every enrollment operationId, tag and component compared base-to-head and byte-identical
lms/lib/tests/test_spectacular.py 3 — URL conformance (schema corollary), the test of the row two above Its filter test asserted the old allowlist and must fail once the allowlist widens; leaving it would mean shipping a red suite. This is the only pre-existing test file in the diff, it holds no API contract, and the rewrite is stricter, not looser: it now also pins that /api/courses/, /api/user_tours/ and /user_api/ stay out, and a new test asserts that only the seven listed user addresses are deprecated while /api/user/v1/accounts, /api/user/v1/preferences/{username}, /api/user/v2/users/ and /user_api/v1/users/ are not. Five tests are added for the names the document publishes: two on the schema class (the service-wide document keeps every id and tag the generator derives, spelled out address by address; the area tag replaces a derived version tag and nothing else) and three over the generated document (every operationId unique; the only numeral-suffixed id is the enrollment pair that predates this change; no tag group holds two areas)
lms/envs/devstack.py 3 — URL conformance (schema corollary), comment only The section banner heading the block that sets SERVERS for this document still read drf-spectacular (LMS enrollment schema) after the document was retitled LMS API and grew a second area. It now reads drf-spectacular (LMS API schema), matching lms/envs/common.py. One line; no setting, no value and no behaviour changed. Consequent on the retitle in the row above, not an independent edit
lms/envs/production.py 3 — URL conformance (schema corollary), comment only The same stale banner over the same SERVERS block, corrected the same way. One line; nothing executable changed
openedx/core/apidocs.py 3 — URL conformance (schema corollary), comment only The comment explaining why the service-wide /api-docs document carries its own settings described the narrow document that claims SPECTACULAR_SETTINGS in LMS as the Enrollment API; after the retitle that sentence is wrong. It now names it the LMS API. One comment line; no code changed

None of these seven files is inside a vN/ package or under one of the fourteen frozen legacy
roots, which is why the versions gate counts all seven as other and its --allow list is never
consulted (see Backward compatibility).

No superseded view module is in the diff. They carry no deprecation notice, no comment and no
warnings.warn, which is why they do not appear here.

Error-format decision

Versioned, not changed in place. frontend-app-account parses the legacy body directly —
src/account-settings/data/utils/serviceUtils.js reads error.response.data.field_errors and each
entry's user_message. Changing the shape under the live v1 addresses would break every field-level
validation message in account settings, so the RFC 9457 envelope ships only on /api/user/v2/.
The two bodies are recorded side by side in
v2/tests/test_parity.py::test_unauthenticated_bodies_differ_as_declared and
::test_forbidden_bodies_differ_as_declared.

Error types registered by this PR: two, and both are its own. Every refusal these seven
read-only operations raise themselves is already in the shared catalog (authn, authz,
not-found, validation). Two failures the framework raises are not: MethodNotAllowed and
NotAcceptable are absent from edx-drf-extensions 10.9.0's catalog, so a refused method or an
Accept header the API cannot answer is enveloped as type: …/errors/internal,
title: "Internal Server Error" at status 405 or 406 — the caller is told its own bad request was a
server fault, and on the public /api/user/v2/time_zones/ that is reachable anonymously.

The catalog is process-global, so this PR does not register DRF's own classes, which would
reclassify those failures for every standardized API in the process. It registers two subclasses of
its own (v2/errors.py: RefusedMethod, UnsatisfiableAccept) and raises them from its own views
through CatalogedRequestErrorsMixin, so the two errors are named method-not-allowed and
not-acceptable here and classified exactly as before everywhere else. That isolation is
asserted, not argued: RefusedRequestTests::test_another_api_is_classified_exactly_as_the_shared_catalog_classifies_it
sends a refused method to another standardized API (POST /api/enrollment/v2/courses/{course_id}/)
and asserts it still answers internal / "Internal Server Error". The other four tests in that class
assert the 405 envelope for an anonymous caller and for the two staff-gated collections and members,
and the 406 envelope for Accept: application/xml, each with its title, its detail and
Content-Type: application/json.

The durable fix belongs in the library: the four uncataloged DRF exceptions — MethodNotAllowed,
NotAcceptable, UnsupportedMediaType and ParseError — belong in _BUILTIN_ERROR_TYPES, where
they would be named for every standardized API at once. It is recorded under Follow-ups; when it
ships, these two subclasses and the mixin give way to it.

Deprecations

Exactly seven legacy paths are marked by this PR — the seven Phase-A addresses below, and no
others:

Superseded Replacement
GET /api/user/v1/users/ GET /api/user/v2/users/
GET /api/user/v1/users/{id}/ GET /api/user/v2/users/{username}/
GET /api/user/v1/user_prefs/ GET /api/user/v2/user_preferences/
GET /api/user/v1/user_prefs/{id}/ GET /api/user/v2/user_preferences/{username},{key}/
GET /api/user/v1/preferences/{pref_key}/users/ GET /api/user/v2/users/?preference_key={pref_key}
GET /api/user/v1/forum_roles/{name}/users/ GET /api/user/v2/users/?forum_role={name}&course_key={course_key}
GET /api/user/v1/preferences/time_zones/ GET /api/user/v2/time_zones/

They are marked only by deprecated: true in the published schema, set by the LMS
post-processing hook (lms/lib/spectacular.py) from a tuple of those seven exact paths. There is
no response header and no deprecation note in any superseded code, docstring or comment — the
superseded modules are not in the diff at all. That is a deliberate departure from ADR 0037's
decision 4, which marks the superseded version in the schema and in the endpoint's docstring: the
FC-0118 reviewer asked for the depr notes to be dropped from the replacement PRs (#39100, #39102),
and OEP-21's marker for a REST API is a single Deprecated <issue link> line added after the DEPR
ticket reaches Plan Accepted, not a notice. If a reviewer wants the marker, that one line in a
follow-up is the form it should take.

The /user_api/v1/… twins of these addresses are not deprecated here: they are called by
frontend-app-account, frontend-app-profile and the Android app, and this PR mounts no successor
at those addresses.

No query parameter is deprecated alongside them. GET /api/user/v1/user_prefs/ accepts
?user=<id>; its successor names owners by username and publishes no identifier filter, so
GET /api/user/v2/user_preferences/?user=<id> is a 400 validation envelope naming the parameters
that list does read. Nothing in this version is published with deprecated: true and no response
carries a Deprecation header. The removal note for the seven addresses carries the replacement
filter name, so a caller reading the ticket has the whole move in one place.

Tracked on the FC-0118 umbrella DEPR ticket,
#39128, which removes them one named
release after the release this ships in. This area's entry is posted on #39128 once the PR is
open.

Five behaviour notes the removal must carry:

  1. GET /forum_roles/{name}/users/ runs Role.objects.get_or_create on every call today. The v2
    filter does not create the role; an unknown role returns an empty page. No receiver is attached
    to Role, so nothing else fans out.
    Where that write still lives, for an operator who relied on reading the list to bring a role
    into existence: seed_permissions_roles(course_key)
    (openedx/core/djangoapps/django_comment_common/utils.py:49) creates the five standard roles
    with their permissions, and already runs when a course is created or imported in Studio
    (cms/djangoapps/contentstore/utils.py:126, contentstore/management/commands/import.py:82) and
    when a CCX course is set up (lms/djangoapps/ccx/utils.py:517); the LMS management command
    seed_permissions_roles <course_id> runs it for a course that predates that; and
    assign_role(course_id, user, rolename) (django_comment_common/models.py:62) creates a role
    when granting it to a user. A role created by the legacy GET carried no permissions and no
    members
    , so none of these is a like-for-like replacement for it — they are what should have
    created the role in the first place.
    What has to exist before the v1 address can be retired: every course whose roles were
    brought into existence by calling it must have them seeded, because the instructor endpoint that
    grants a role (POST …/instructor/api/update_forum_role_membership) reads the role with
    Role.objects.get and raises Role.DoesNotExist when it is missing.
    are_permissions_roles_seeded(course_id) reports whether a course is in that state and the
    management command puts it right; the check belongs in the removal ticket, not in this PR, since
    the legacy address keeps its behaviour until then.
  2. GET /api/user/v2/time_zones/ validates country_code as two letters. A well-formed but unknown
    code still falls back to every time zone, exactly as before; a malformed one (FRA) is now a 400
    envelope where v1 returned the full list. An empty value (?country_code=) narrows nothing and
    returns every zone, as it does at v1; that branch has a test on both counts.
  3. /api/user/v2/ does not accept X-Edx-Api-Key. Server-to-server callers of the v1 directory
    endpoints must issue a staff JWT before moving.
  4. A user with no profile row makes GET /api/user/v1/users/{id}/ raise; the v2 entry returns
    name: null. A caller moving to v2 gets a row where it used to get a server fault.
  5. The superseded member address is keyed by a database identifier and its replacement is keyed by
    username, so an integration holding identifiers has a step to take before it can address an
    entry: GET /api/user/v1/accounts?lms_user_id={id} — staff-gated, untouched by this PR and
    itself standardized in phase B — answers with the account whose username keys the v2 entry.
    The v2 directory publishes no identifier filter, and this version refuses a parameter it does
    not read, so GET /api/user/v2/users/?id={id} is a 400 validation envelope naming id rather
    than a page that looks narrowed, and the resolution step is not optional.
    v2/tests/test_parity.py::test_a_database_identifier_is_refused_instead_of_narrowing_nothing
    records the refusal and
    ::test_an_identifier_held_by_a_caller_resolves_to_the_username_the_entry_is_keyed_by walks the
    two calls end to end. This step belongs in this area's entry on the DEPR ticket. Whether a v2
    id filter should exist instead is Open decisions, item 3.

Authorization

Endpoint Legacy check v2 permission classes
GET /users/, GET /users/{username}/ ApiKeyHeaderPermission — ignores request.user entirely; a valid X-Edx-Api-Key reads every user's email address and full preference mapping (IsAuthenticated, IsActiveUser, IsAdminUser) + FullScopePolicy()
GET /user_preferences/, GET /user_preferences/{username},{key}/ ApiKeyHeaderPermission (IsAuthenticated, IsActiveUser, IsAdminUser) + FullScopePolicy()
GET /users/?preference_key=, ?forum_role=&course_key= ApiKeyHeaderPermission as above — the merge does not widen who may call
GET /time_zones/ none declared (public by omission) (AllowAny,), declared on purpose

Both staff-gated viewsets require the caller's account to be active, not only authenticated and
staff. The platform's two default authenticators disagree about a deactivated account: the session
half stops accepting one, while a token issued before the account was deactivated is still accepted,
because only the session half reads the flag. The requirement is therefore declared where it applies
to both, as the IsActiveUser permission class in v2/permissions.py, and such an account is
refused on either path. edx-drf-extensions 10.9.0 ships no equivalent permission; if it gains one,
this class gives way to it.

Deny-path tests, one per refusal:

Caller Endpoint Expected Test
anonymous /users/ 401 authn UserListViewTests::test_anonymous_caller_is_unauthenticated
authenticated non-admin /users/ 403 authz ::test_non_admin_caller_is_forbidden
authenticated non-admin /users/?preference_key= 403 authz ::test_non_admin_cannot_list_another_users_preference_holders
anonymous / non-admin / the named user /users/{username}/ 401 / 403 / 403 UserDetailViewTests (three tests)
admin, unknown username /users/{username}/ 404 not-found UserDetailViewTests::test_unknown_username_is_not_found
anonymous / non-admin / the owner /user_preferences/ 401 / 403 / 403 UserPreferenceListViewTests (three tests)
anonymous / non-admin / the owner /user_preferences/{username},{key}/ 401 / 403 / 403 UserPreferenceDetailViewTests (three tests)
admin, unknown key or another user's key /user_preferences/{username},{key}/ 404 not-found (×2) ::test_unknown_key_is_not_found, ::test_key_belonging_to_another_user_is_not_found
anonymous /time_zones/ 200 — public on purpose TimeZoneListViewTests::test_anonymous_caller_receives_the_list
anyone, country_code=FRA /time_zones/ 400 validation ::test_malformed_country_code_is_rejected
anyone, a page past the end or a page that is not a number /users/, /user_preferences/, /time_zones/ 404 not-found ::test_a_page_beyond_the_last_one_is_not_found (×3), UserListViewTests::test_a_page_number_that_is_not_a_number_is_not_found
admin, fields= naming a property the response cannot carry the four shaped addresses 400 validation, exact body asserted ::test_a_field_name_the_row_does_not_carry_is_refused and its three siblings, UserListViewTests::test_a_field_the_minimal_preset_drops_is_refused, ::test_every_unknown_field_name_is_named_at_once
a staff account deactivated after it signed in, through its session every staff-gated address 401 authn — the session authenticator stops accepting it DeactivatedStaffCallerTests::test_a_session_held_by_a_deactivated_staff_account_is_refused, ::test_a_deactivated_staff_session_is_refused_at_every_staff_gated_address
a staff account deactivated after a token was issued for it every staff-gated address 403 authz — the token is still valid and the active-account permission refuses it ::test_a_token_held_by_a_deactivated_staff_account_is_refused, ::test_a_deactivated_staff_token_is_refused_at_every_staff_gated_address, ::test_the_directory_does_not_list_the_account_that_was_refused (no page body reaches the caller)
a still active staff account, by token /users/ 200 — the requirement refuses deactivated accounts and nothing else ::test_an_active_staff_token_still_reaches_the_directory
a deactivated non-staff account holding a token /users/ 403 authz — the permission layer still applies on that path DeactivatedStaffCallerTests::test_a_token_held_by_a_deactivated_learner_is_still_forbidden
admin, forum_role without course_key and the reverse /users/ 400 validation, exact body asserted two tests
admin, unparsable course_key /users/ 400 validation ::test_malformed_course_key_is_rejected
admin, course_key whose + is not encoded /users/ 400 validation, exact body asserted ::test_a_course_key_whose_plus_signs_are_not_encoded_is_refused
admin, ?user=<id> (the filter the v1 list took) /user_preferences/ 400 validation, exact body asserted UserPreferenceListViewTests::test_the_owner_id_the_earlier_list_took_is_refused_rather_than_ignored
admin, any name an endpoint does not read (?lms_user_id=, ?expand=, a mistyped filter, ?sort= and ?limit= together) all five addresses 400 validation, exact body asserted, every unknown name reported at once UnknownQueryParameterTests (8 refusal tests)
anyone, ?country= or ?ordering= /time_zones/ 400 validation — this list reads country_code and the page parameters only UnknownQueryParameterTests::test_the_time_zone_list_refuses_a_parameter_it_does_not_read, ::test_the_time_zone_list_refuses_a_sort_it_does_not_offer
anonymous / non-admin, an unknown parameter /users/, /user_preferences/ 401 / 403 — the caller is refused before the query string is read UnknownQueryParameterTests::test_an_unknown_parameter_does_not_answer_an_anonymous_caller, ::test_an_unknown_parameter_does_not_answer_a_non_admin_caller
anonymous / non-admin / a holder of the role /users/?forum_role=&course_key= 401 / 403 / 403 UserListViewTests::test_anonymous_caller_cannot_list_a_courses_forum_role_holders, ::test_non_admin_cannot_list_a_courses_forum_role_holders, ::test_holding_the_role_does_not_let_a_learner_list_its_holders
anyone, ?view= naming a preset the address does not answer to /users/, /users/{username}/, /user_preferences/, /user_preferences/{username},{key}/ 400 validation, exact body asserted on the two collections ::test_an_unknown_response_preset_is_refused (×4), UserListViewTests::test_the_preset_that_belongs_to_the_preference_list_is_refused_here
anonymous, ?view=nonsense /users/ 401 authn — the caller is refused before the parameter is read ::test_an_unknown_preset_does_not_answer_an_anonymous_caller
learner's JWT every staff-gated address 403 authz TokenCallerTests::test_a_learners_token_is_forbidden_on_every_staff_gated_address
the owner of the record, by token /user_preferences/… 403 authz — owning the row is not being staff TokenCallerTests::test_the_owner_of_the_record_is_still_not_staff

Gate report

run_gates.sh, this branch against origin/master @ ce074d1155, final run, after ten repair passes and the two
decisions taken at review. Six gates ran; none was skipped; two are red.

Gate Result What it reports
versions PASS files in new version dirs: 16 other files: 7 added in old dirs: 0 allowed: 0 — all fourteen protected legacy paths unchanged and nothing added inside a pre-existing version directory
hygiene PASS (0 FAIL, 1 WARN) WARN H3 openedx/core/djangoapps/user_api/v2/paginators.py:6 — the local DefaultPagination subclass, kept because it adds behaviour (the full envelope schema the library does not publish) and the class docstring says so. The two WARN H11 findings of the earlier run were on the Deprecation wording of the ?user= alias and are gone with it. No superseded route carries any such text
schema PASS (0 BREAKING, 18 WARN, 12 INFO) measured document against document on both refs: no path, operationId, component or tag removed or renamed in either schema. The 12 INFO are operations added to this narrow document — the five Phase-A addresses and the seven superseded v1 addresses it now publishes beside them (those seven are not new endpoints; they were already in the service-wide /api-docs document on master). The 18 WARN are all new generation warnings: twelve on the legacy v1 user views and serializers the document now admits, six on this PR's own three viewsets — in both cases could not resolve authenticator for the platform's default classes, plus two type-hint warnings on the v1 serializer. None is an error, and the effect is named below
urls FAIL (2 FAIL, 1 WARN) — deliberate 3036 routes on the base ref, 3042 on the branch; no address-preservation finding at all. All three findings are on the one added pattern that is not an address — the version-level JSON-404 fallback. Left red on purpose; reconciled immediately below. Not to be read as passing
old-tests FAIL — red and blind red for an environment reason, identically on both refs; see below. Not to be read as passing, and not evidence that the legacy suite is green
tests PASS (302 passed) 237 in openedx/core/djangoapps/user_api/v2/tests/, 12 in lms/lib/tests/test_spectacular.py, 53 in openedx/core/djangoapps/enrollments/v2/tests/ (the regression test for the schema settings change). This is the new suite only. The legacy openedx/core/djangoapps/user_api/tests/ suite is unchanged against its baseline on this branch: 7 failed, 100 passed, 16 errors, the same counts before and after these changes

What the four green gates do not prove. Each is green for what it measures, and no more:

  • versions proves the fourteen protected paths are untouched. It proves nothing about the seven
    pre-existing files in the diff: the --allow arguments are mechanically inert (allowed: 0), and
    the evidence for those seven is the per-file review in In-place edits.
  • hygiene passing H8 is not evidence for ADR 0030: H8 greps get/list/retrieve bodies
    and cannot see a write in a service module. The twelve SQL captures are the evidence.
  • schema proves nothing was removed or renamed in either document. It does not say the 18 new
    generation warnings are harmless; their concrete effect is the missing security block, named
    below and under Follow-ups.
  • tests covers the new version's suite and the enrollment regression. It says nothing about the
    19 legacy tests that cannot run in this environment.

The security block, since the schema gate cannot flag it. Six of the 18 warnings are on this
PR's own views: could not resolve authenticator DefaultJwtAuthentication /
DefaultSessionAuthentication; there was no OpenApiAuthenticationExtension registered for that
class.
They are the platform's default pair, which ADR 0034 requires and which no view here
overrides; the same warning is emitted for the enrollment views on the base ref. The visible effect
is that the four staff-gated operations publish no security requirement at all
(security: [{}] on the public time-zone list, absent on the other four), so a generated client
cannot see that a JWT or a session is required. Registering the extension is a shared-library
change, recorded under Follow-ups, not something to work around per view.

Why urls is red, and the rule that cannot see the difference. The gate rule-checks every route
added under api/. Six were added: the five Phase-A addresses and one fallback,
re_path(r"^(?!account/).*$", UnknownAddressView.as_view()), which answers every address under
/api/user/v2/ that no route serves with the same JSON error envelope as the rest of the API
instead of the service's HTML 404 page. The gate normalises that pattern to the address
api/user/v2/{_}. and reports it three times:

Finding Subject What the rule is for
FAIL R6 missing trailing slash api/user/v2/{_}. one address, one spelling
FAIL R8 identifier directly under the version api/user/v2/{_}. a resource is addressed collection/{id}, not {id}
WARN R11 route has no URL name api/user/v2/{_}. reverse() must work for an address

None of the three applies to the subject: a fallback is not a resource, has no identifier, and is
deliberately unnamed because nothing should ever reverse to it. Six candidate shapes were run
through the checker itself rather than guessed at:

Shape Normalised Result
re_path(r"^(?!account/).*$", …) (shipped, nameless) api/user/v2/{_}. R6, R8, R11
the same with a URL name api/user/v2/{_}. R6, R8 — and a name that must never be used
path("<path:unknown_address>", …) api/user/v2/{unknown_address} R6, R8
path("<path:unknown_address>/", …) api/user/v2/{unknown_address}/ R8, and slashless addresses lose the JSON 404
re_path(r"^.*/$", …) api/user/v2/./ R4
path("users/<path:unknown_address>/", …) per collection api/user/v2/users/{unknown_address}/ clean — but cannot answer /api/user/v2/not_a_collection/, which is half the behaviour

A version-level fallback cannot be spelled as a conforming address, because the thing that makes it
a fallback — matching anything — is exactly what the normaliser reads as an identifier in the
collection slot. url_snapshot.py check does take an --allowlist, matched against the normalised
path and not, as is sometimes assumed, the route name (url_snapshot.py:253), so the route's
namelessness was never the obstacle; with the single entry api/user/v2/{_}. the run reports
new api routes checked: 5, PASS (0 FAIL, 0 WARN) (saved as
artifacts/urls-fallback-allowlist.txt). That figure is quoted only for the five real addresses
and never as the gate result
, for two reasons: run_gates.sh:134 passes no allowlist to the
checker, so the composite gate could not go green that way without editing the gate harness; and a
green line produced by exempting the subject of the finding would read as "the rules were met" when
the honest statement is "the rules do not apply here". So the gate stays red, this is reported as a
checker limitation and not a checker changed to suit the diff, and the fallback's shape is pinned in
the suite instead — by v2/tests/test_urls.py::RouteShapeTests (every other pattern in the URLconf
is named and slash-terminated; the fallback is the single unnamed one and is matched last) together
with MalformedAddressTests, which asserts the JSON 404 — content type, no <html, and the error
envelope — for an unknown collection, an unknown member, a malformed username and a write, anonymous
and authenticated.

Why old-tests is red, and what that does and does not show. The gate runs
openedx/core/djangoapps/user_api/tests/ and reports 7 failed, 100 passed, 16 errors (381.78s).
The same command on origin/master @ ce074d1155, in a clean worktree with the same image, the same
interpreter and the same settings, reports 7 failed, 100 passed, 16 errors (381.02s). The two
FAILED/ERROR sets were compared, not just the counts: 23 lines over 19 distinct tests on each side
(four tests report both a FAILED and an ERROR line, the ERROR coming from teardown), and the
difference between the sets is empty. Every failure and error is a pymongo
ServerSelectionTimeoutError — the test container has no modulestore, so UserPreferenceModelTest
and UpdateEmailOptInTestCase cannot run in it at all.

  • It proves: no legacy test that passes on master fails on this branch.
  • It does not prove: that those 19 tests pass at all. They are unrunnable in this environment, on
    master as much as on this branch. CI, which has Mongo, is the only place that can answer it.
  • It is also narrow: only 2 of the 11 legacy suites this area owns were exercised at all. The
    100 passing legacy tests are a floor, not legacy coverage; Backward compatibility lists the
    nine suites (~420 tests) that CI will be the first to run on this branch.

Open decisions — not settled by this PR

Four things on this branch are decisions rather than findings. Each is implemented one way, tested
and reversible, and each is cheap to change now and expensive later. None should be read as closed.

  1. Attribution trailers. Both commits currently carry
    Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>, and the repair-pass work still to be
    committed will carry whatever is decided. The convention is that the trailer is the author's
    call, never a default: keeping it needs no action, dropping it is a rebase of two commits. This
    PR body carries no attribution line either way.
  2. F53 and F54 — two published addresses a repair pass declined to change. user_preferences/
    repeats the api_name (the stutter clause), kept because phase C will publish the caller's own
    preference document at accounts/{username}/preferences/ and a bare preferences/ here would
    read as that resource. time_zones/ stays under the user api_name because the list exists to
    populate the time_zone preference and one canonical address beats two. Both reasons are
    recorded in the 0038 matrix row. This is the last moment either can change for free: once the
    addresses are published, a rename is a second address.
  3. F71 — id-keyed callers migrate by a documented step, not by a new filter. An integration
    holding database ids resolves them with GET /api/user/v1/accounts?lms_user_id={id} (staff-gated,
    standardized in phase B) and then addresses users/{username}/; the v2 directory publishes no
    id filter, and since this version refuses a parameter it does not read, ?id= is a 400 naming
    the parameter rather than a page that looks narrowed. Two tests pin both halves, and Deprecations
    note 5 carries the step. The alternative — an id filter on the v2 directory — is about four
    lines plus a published parameter plus tests, and it would reintroduce a database identifier into
    the v2 contract.
  4. Whether the red urls gate reconciliation reads honestly enough to ship. The gate is red on
    three findings, all on the JSON-404 fallback, and the argument above is that the rules do not
    apply to a route that addresses nothing. The alternatives were measured and each costs behaviour
    or honesty: making it conforming drops the "unknown collection" case; allowlisting it needs a
    change to run_gates.sh and produces a green line that would misrepresent what was checked. If a
    reviewer would rather have a green gate than this paragraph, the honest green requires teaching
    url_snapshot.py the difference between an address and a fallback — a change to the checker, in
    its own PR.

Three decisions taken at review and now settled in the body above, recorded here so a reader of an
earlier draft can find them: the deactivated-account gap is closed on the v2 endpoints by the
IsActiveUser permission (Authorization, and the 0026 and 0034 matrix rows); the ?user=<id>
alias and its Deprecation header are removed, and an unrecognised query parameter is now a
400 instead (What else a caller or a client generator sees, and the 0033 matrix row).

The user directory, the stored-preference listings and the time-zone
reference list were the least conforming endpoints of the User API: the
only gate was the deprecated X-Edx-Api-Key header, which ignores
request.user entirely, so any holder of the key could read every user's
email address and full preference mapping. The listings were addressed by
database id, two of them put the parent key in the path instead of the
query string, the time-zone list returned an unbounded array, the stored
preference row embedded its owner's entire record (one extra query per
row), and the forum-role listing created the role it was asked about on
every GET.

Each of those is a contract change, so they land on a new version and
/api/user/v1/ is untouched and keeps serving them.

What callers get instead:

  GET /api/user/v2/users/                              (also ?preference_key=,
                                                        ?forum_role=&course_key=)
  GET /api/user/v2/users/{username}/
  GET /api/user/v2/user_preferences/
  GET /api/user/v2/user_preferences/{username},{key}/
  GET /api/user/v2/time_zones/

Authenticated staff callers rather than an API key, the standard error
envelope, the standard pagination envelope on all three lists, filter
sets and a deterministic ordering, and the ?view= and ?fields= response
presets. The forum-role filter no longer creates a role, and an
unparsable course key is a validation error rather than a 500.

Being refused is part of the contract, so the refusals are explicit. A
deactivated staff account is turned away on both paths rather than only
one: the session authenticator already rejected it, but IsAdminUser reads
only is_staff and the JWT authenticator does not read is_active, so such
an account could read the whole directory until its token expired. These
addresses require an active user.

An unrecognised query parameter is a 400 rather than silence. A client
porting from a superseded address by changing the version segment alone
would otherwise pass a filter this version does not define, have it
ignored, and receive every row believing the page was filtered. The
allowlist is derived from the paginator, the ordering backend, the filter
set and the query serializer, so it cannot drift away from the filters it
describes.

Shared building blocks are used rather than rewritten:
StandardizedErrorMixin, DefaultPagination, IterablePaginationMixin,
ScopedQuerysetMixin with FullScopePolicy, MinimalViewMixin and project.
The two local classes each supply one predicate the library does not,
and each docstring says which.

The tests pin behaviour rather than structure: every allowed and every
refused caller with the error type asserted, value-level parity with each
superseded address, query counts that stay constant across 1, 5 and 25
rows where the superseded listing pays per row, SQL captures proving no
GET writes, and the URL literals each name reverses to.
The LMS drf-spectacular pre-processing hook is an allowlist and admitted
only /api/enrollment/vN/, so everything under /api/user/ generated,
routed and tested while being invisible in the published document. The
new v2 addresses would have shipped undocumented and the schema gate
would have reported the prefix missing with no clue as to why.

The filter now admits exactly what this change owns: the v2 addresses,
matched by their view package rather than by path prefix, plus the seven
legacy paths they supersede, so each superseded address appears beside
its replacement. It is deliberately not a /api/user/vN/ prefix. Admitting
the whole prefix pulled thirty-eight further v1 operations into the
document -- undocumented request bodies and internal docstrings published
as client-facing text -- none of which this change owns or documents.

SCHEMA_PATH_PREFIX becomes a two-branch pattern, stripped only to derive
tags and operationIds. Every existing enrollment operationId was compared
before and after and is unchanged, and so is every operationId, path and
component of the service-wide /api-docs document: a generated client
derives a module per operationId and a model per component, so a rename
there deletes a module as surely as a removal does. The title follows the
contents, and the three comments that named the old title follow it.

Deprecation is marked where it belongs, outside the frozen modules: the
existing post-processing hook gains a tuple of the seven exact legacy
paths this change supersedes. Exact paths, not a prefix, so an address
whose replacement nobody has written yet can never be marked -- the hook
test includes neighbouring user addresses, the unversioned twins and
another area's paths and asserts they are left alone.

POSTPROCESSING_HOOKS is unchanged: setting it replaces drf-spectacular's
defaults, so the enum hook stays restated and stays first.
@Abdul-Muqadim-Arbisoft

Copy link
Copy Markdown
Contributor Author

Reference detail for this pull request

Moved out of the description: GitHub caps a PR body at 65,536 characters. Nothing is omitted.

What else a caller or a client generator sees

Every caller-visible change this branch makes, including the ones no row above carries. Nothing
caller-visible ships undeclared.

  • An unsupported ordering value is refused (400). ?ordering= takes only the published
    allow-list — username and date_joined on the directory, key and username on the preference
    list, each optionally prefixed with - — and anything else is a 400 validation envelope whose
    message names the allow-list without echoing the input. One unknown term refuses the whole sort
    rather than silently sorting by the rest. No v1 caller is affected: v1 has no ordering parameter
    at all.
  • ?fields= naming a property the row cannot carry is refused (400). The projection parameter
    takes only field names the response actually has; an unknown one is a 400 validation envelope
    that names every unrecognised name at once, and asking for a field the active preset drops
    (?view=minimal&fields=email) is refused for the same reason rather than silently returning
    nothing. The library's shaping helper ignores what it does not recognise, which would hand a
    caller an empty projection and no way to tell. v1 has no fields parameter, so no v1 caller is
    affected. (Dotted paths such as ?fields=preferences.time_zone pass validation on their first
    segment and are not traversed — the helper's documented behaviour, recorded under Follow-ups
    rather than changed here.)
  • ?is_active= takes a boolean and nothing else. true, false, 1, 0 and their
    capitalised spellings narrow the directory; an empty value narrows nothing, as an absent parameter
    does; anything else — ?is_active=maybe — is a 400 validation envelope naming the parameter.
    The framework default reads an unrecognised value as "no preference" and answers 200 with the
    filter dropped, which hands a caller that mistyped true exactly the inactive accounts it asked
    to leave out. v1 has no is_active filter, so no v1 caller is affected.
  • A response preset this API does not publish is refused (400). ?view= takes minimal on the
    directory and full or minimal on the preference rows; any other value is a 400 validation
    envelope naming the presets that address answers to, while an empty value returns the default
    shape, as an absent parameter does. Both member addresses now publish a 400 beside their other
    error responses. v1 has no view parameter, so no v1 caller is affected.
  • A malformed course_key on the forum-role filter is a 400, not a 404, and not the 500 it is
    today.
    GET /api/user/v1/forum_roles/{name}/users/?course_id=not-a-key lets InvalidKeyError
    escape and answers 500; GET /api/user/v2/users/?forum_role=…&course_key=not-a-key answers a 400
    validation envelope. This is a declared departure from the plan as first written: plan §10.5
    asked for a 404. course_key here is a query parameter, not a path segment — there is no
    converter to fail and no resource being addressed — so the conforming answer is validation at
    400, which is also what the plan's own error column for these rows says. The plan was corrected on
    a repair pass so phases B–E inherit one rule: 400 for a malformed key in a query parameter, 404 for
    one in a path segment.
  • country_code on the time-zone list is validated as two letters. A well-formed but unknown
    code still falls back to every time zone, exactly as before, and an empty value narrows nothing;
    a malformed one (FRA) is now a 400 validation envelope where v1 returned the full list, with a
    message naming the form expected (Enter a two-letter ISO 3166-1 alpha-2 country code, for example FR.) rather than the framework's pattern text. This is the one refusal on the anonymous endpoint.
  • A mistyped address under /api/user/v2/ answers JSON. Anything under the mount that matches no
    route is a 404 error envelope with Content-Type: application/json, where the service would
    otherwise return the LMS HTML error page that a client asking for JSON cannot read. The two
    addresses the authentication app serves under the same prefix (account/registration/,
    account/login_session/) are matched before it and still reach that app.
  • A refused method (405) and an unanswerable Accept (406) now render the error envelope
    correctly.
    Both are classified as internal / "Internal Server Error" at a 4xx status by the
    shared catalog; on these endpoints they are named method-not-allowed and not-acceptable. The
    405 is reachable anonymously on the public time-zone list. See Error-format decision — the fix
    is scoped to this version's views and changes nothing for any other API.
  • A query parameter an endpoint does not read is a 400 validation envelope. Each address
    answers to a fixed set of names — its filters, ordering, page, page_size, the response
    presets view and fields, and the framework's format — and anything else is refused, with the
    offending name as the key of the errors member and the accepted names in the message. v1 dropped
    an unknown parameter in silence and answered 200. The case this exists for: the v1 preference list
    filters by owner id (?user=<id>) and this version names owners by username, so a client that
    ported by changing only the version segment is told its filter was dropped instead of receiving
    every user's preference rows and reading the page as narrowed. ?id= on the directory and any
    mistyped filter name behave the same way. The time-zone list reads only country_code and the
    page parameters, so a sort it does not offer (?ordering=) is refused there too.
  • No query parameter of this version is deprecated and no response carries a Deprecation
    header.
    The one v1 parameter without a successor, ?user=<id>, is refused rather than aliased.
  • Every preference row carries a user member, present and null by default. It holds the
    owner's whole directory entry only under ?view=full; the flat username string beside it is how
    the default row names its owner. The legacy row carried the owner under the same name and never
    carried it empty, so a client that validates the default row's shape strictly sees one member it
    did not see before, always present and always null.
  • A user with no profile row is a row here, not a server fault. The v1 serializer reads
    user.profile.name unguarded, so such a user makes GET /api/user/v1/users/{id}/ raise; v2
    returns the entry with name: null, and name is published as nullable. No fixture on v1 covers
    this, which is why it is stated here rather than in the parity table: there is no v1 body to diff
    against.
  • Nineteen component schemas are added to the LMS document and none is renamed or removed.
    Thirteen are this version's own — ErrorResponse, UserDirectory, UserDirectoryResponse,
    UserDirectoryMinimal, UserDirectoryProjection, UserPreferenceRecord,
    UserPreferenceRecordResponse, UserPreferenceRecordMinimal, UserPreferenceRecordProjection,
    TimeZone, PaginatedTimeZoneList, PaginatedUserDirectoryResponseList and
    PaginatedUserPreferenceRecordResponseList. The other six (user_api.User, UserPreference,
    CountryTimeZone, Paginateduser_api.UserList, PaginatedUserPreferenceList,
    PaginatedCountryTimeZoneList) are the existing v1 serializers, carried into this document
    because the seven superseded operations are now published beside their replacements; all six were
    already published in the service-wide /api-docs document on master and none of them changed.
    No *Enum component is created, so no existing enum component can be renamed by a collision.
    The comparison is reproduced in the suite rather than living only in this PR's working notes:
    v2/tests/test_schema.py::test_this_version_only_adds_to_the_service_wide_document generates the
    service-wide document twice — with this version mounted and with its mount taken out of the
    URLconf — and asserts every component name and operationId of the second is still in the first.
    The second generation carries the same 856 names as the /api-docs document on origin/master,
    compared name by name; the first carries 874. Making one of this version's serializers declare a
    colliding status choice field turns the test red on StatusEnum, which is the failure it exists
    to catch. Two of the names changed within this PR while it was written
    (PaginatedUserDirectoryList → PaginatedUserDirectoryResponseList, likewise for preferences),
    as did one new operationId (v2_user_preferences_,_retrieve → v2_user_preferences_retrieve);
    all of them are Phase-A names that no published document carried before.

Backward compatibility

Nothing on master disappears. Measured, not asserted:

  • Versions gate — check_old_versions_untouched.sh --base origin/master with --protect on the
    fourteen frozen legacy surfaces, named one file or package at a time (user_api/urls.py,
    legacy_urls.py, views.py, serializers.py, accounts/, preferences/, verification_api/,
    user_authn/urls.py, urls_common.py, user_authn/views/, profile_images/views.py,
    profile_images/urls.py, profile_images/images.py, profile_images/exceptions.py) →
    files in new version dirs: 15 other files: 7 added in old dirs: 0 allowed: 0,
    RESULT: PASS. All fourteen protected paths are unchanged; none is in the diff.
    Read for exactly what it measures, because the gate proves less than its green line suggests:
    PASS means no path under any of the fourteen protected roots is in the diff, and that nothing was
    added inside a pre-existing version directory. It says nothing about the seven pre-existing
    files this PR does edit. Four --allow arguments were passed for them, and those arguments are
    mechanically inert
    — the gate reports allowed: 0, literally: the seven files sit outside every
    vN/ directory and outside every protected root, so the gate classifies them as other and never
    consults --allow. Running the same diff with no --protect and no --allow at all prints the
    identical three counters. The real evidence for those seven is the per-file diff review in the
    In-place edits table below
    , plus git diff --name-only origin/master, which lists exactly those
    seven files and the files under openedx/core/djangoapps/user_api/v2/, and nothing else.

  • URL gate — a route dump on the base ref (3036 routes) and on the branch (3042 routes), then
    url_snapshot.py check --base … --head … → FAIL (2 FAIL, 1 WARN). Address preservation is
    clean
    : not one ADDR finding, so every base route still resolves to the same view under the
    same name, and five of the six added routes are exactly the Phase-A addresses. All three findings
    name one subject, api/user/v2/{_}., which is the sixth: the fallback pattern that answers an
    address no route serves. It is not an addressable resource, the gate has no rule that distinguishes
    one from the other, and the finding is reconciled rather than silenced under Gate report.
    A dedicated test asserts /api/user/v1/users/, /api/user/v1/users/{id}/,
    /api/user/v1/user_prefs/, /api/user/v1/preferences/{key}/users/,
    /api/user/v1/forum_roles/{name}/users/, /api/user/v1/preferences/time_zones/ and the
    /user_api/v1/… twins still reach the v1 views, and that accounts_api, own_username_api and
    preferences_api still reverse to their exact literals. Because the new mount is matched before
    the api/user/ include, v2/tests/test_url_mount.py additionally pins that
    /api/user/v2/account/registration/ and /api/user/v2/account/login_session/ still resolve into
    user_authn; inserting a catch-all into the v2 URLconf turns both red.

  • Schema diff — measured document against document on both refs, for both schemas this service
    publishes. The narrow LMS document goes from 19 paths to 31 (the five new operations and the seven
    addresses they supersede); the service-wide /api-docs document goes from 495 paths / 658
    operations to 500 / 663 (counted from artifacts/base-api-docs.json and
    artifacts/head-api-docs.json; the final gate run compares only the narrow document, so these two
    figures come from the document pair generated during the repair passes, not from gates-final).
    Nothing is removed and nothing is renamed. Compared key by key: no path, no operationId, no
    component schema and no tag that either document carried on origin/master is missing or
    different. The 658 base operationIds are a subset of the 663; all 48 operations under /api/user/
    in /api-docs keep the tag they had; user_api.User — the ref_name on the v1 serializer — is
    still published; the LMS document adds 19 component schemas and removes none. Every enrollment
    operationId and tag is byte-identical after the SCHEMA_PATH_PREFIX change, compared operation by
    operation rather than inferred.
    Two things the gate's own counts do not tell a reviewer, so they are stated here:

    • The gate compares generation warnings only when a .warnings.txt sidecar sits beside both
      documents. Re-running compare over two saved documents without the sidecars reports no warning
      difference at all, which reads as "clean" and is not a measurement.
    • The 18 new generation warnings are named rather than counted under Gate report: twelve
      belong to the legacy v1 user views and serializers that this document now publishes beside their
      replacements, and six are on this PR's own three viewsets.
  • Parity tests — v2/tests/test_parity.py runs the same fixtures through both versions and
    diffs the JSON. The declared intentional differences, verbatim from the test module:

    Case Path Reason
    user list results[*].url the member address is keyed by username instead of database id
    user detail url the member address is keyed by username instead of database id
    preference list results[0].user, results[1].user the embedded owner is a member of every row and is null unless the full preset is asked for
    preference list results[0].username, results[1].username the owner's username is carried in place of the embedded user object
    preference list results[0].url, results[1].url the member address is keyed by username and preference key
    preference detail user, username, url as above
    ?preference_key= filter results[0].url as above
    ?forum_role= filter results[0].url as above

    The harness asserts these are the only differences and that each one actually occurs. The
    preference cases run with two rows, so the comparison pairs rows rather than reading a single one.
    It was mutation-checked five ways — dropping email from the serializer, emptying preferences,
    truncating the time-zone list, marking a foreign address deprecated, and reordering the preference
    page — and went red each time.
    Row order is a difference like any other here: v1 orders none of these lists, v2 orders the
    directory by username and the preference list by (username, key), and
    ::test_the_conforming_page_is_ordered_by_username_whatever_the_creation_order and
    ::test_the_conforming_preference_page_is_ordered_by_owner_and_key pin both sequences against the
    creation order the legacy page returns.
    One difference has no v1 body to diff against and is asserted on its own:
    ::test_a_user_without_a_profile_is_a_row_here_and_a_failure_at_version_one records that the v1
    member address raises for a user with no profile row where v2 answers 200 with name: null.
    The time-zone case asserts value equality: new["count"] == len(legacy) and
    new["results"] == legacy[:100], and the country-filtered rows compare equal outright.
    Two refusals are recorded the same way, because their status stays and only their body moves.
    ::test_a_missing_course_parameter_keeps_its_status_and_changes_its_body records that
    GET /api/user/v1/forum_roles/Moderator/users/ without course_id already answered
    400 {"detail": "course_id must be specified"}, against the v2 envelope's errors.course_key at
    the same 400. ::test_an_unparsable_course_parameter_is_answered_rather_than_raised records the
    one that really was a server fault: the v1 handler lets InvalidKeyError escape, which the test
    asserts with pytest.raises, where v2 answers 400. Both go red when the guard in the filter set is
    removed.

  • Query counts, measured through both versions over the same rows. The stored-preference list
    is where the saving is: its v1 row embeds its owner without a join and re-reads that owner's own
    preferences, so a page costs five queries per owner —
    15 / 35 / 135 queries for 1 / 5 / 25 owners (two preferences each, one page). The v2 list
    answers the same rows at 15, constant, and ?view=full, which restores the embedded owner,
    at 16, constant. The two measurements live in one test so they cannot drift apart:
    v2/tests/test_query_counts.py::test_the_earlier_preference_list_pays_per_row_where_this_one_does_not,
    parametrized over the same 1 / 5 / 25. The other six operations have no per-row cost at either
    version; their absolute v2 counts are higher than the v1 ones
    (directory 16 against 11, member 15 against 10, time zones 13 against 8) because the v1 caller
    authenticates with a header that touches no table and the v2 caller with a session that does.
    That fixed difference is the price of dropping X-Edx-Api-Key, and it does not grow with the page.

  • Legacy tests untouched — but not shown green, and the gate that runs them is red and blind.
    No test of any v1 endpoint was modified. The suite that was run,
    openedx/core/djangoapps/user_api/tests/, reports the identical 7 failed, 100 passed, 16 errors
    on origin/master @ ce074d1155 (381.02s) and on this branch (381.78s); the FAILED/ERROR sets
    were compared rather than the counts, and they are the same 23 lines over the same 19 distinct
    tests on both refs, with an empty difference. Every one of them is a pymongo
    ServerSelectionTimeoutError: the test container has no modulestore, so UserPreferenceModelTest
    and UpdateEmailOptInTestCase cannot run there at all. That proves no legacy test which passes
    on master fails on this branch
    , and it does not prove those 19 tests pass; only CI, which has
    Mongo, can answer that.
    Of the eleven legacy suites this area owns, two were run (user_api/tests/test_views.py and
    user_api/tests/test_legacy_urls.py, inside the directory above). The other nine were not run on
    this branch
    : accounts/tests/test_views.py (74), accounts/tests/test_retirement_views.py (71),
    preferences/tests/test_views.py (32), verification_api/tests/test_views.py (13),
    user_authn/views/tests/test_login.py (51), test_register.py (101), test_reset_password.py
    (43), test_password.py (14) and profile_images/tests/test_views.py (19) — about 420 tests.
    Their modules are byte-identical in this diff and the LMS files it touches cannot change their
    behaviour, but that is an argument, not a measurement; CI runs them.
    openedx/core/djangoapps/enrollments/v2/tests/ is green (53 tests), which is the regression test
    for the schema settings change.

  • No code moved, so no re-export shim is owed.

Follow-ups

  • Library-first candidates. Each is a gap in edx-drf-extensions; this PR ships a local minimum
    and should drop it when the library catches up. No PR numbers are given because none of these has
    been opened yet.
    1. get_paginated_response_schema is absent from edx-drf-extensions 10.9.0, so DefaultPagination
      publishes four of the seven envelope members it returns. UserApiPagination overrides only that
      method. This belongs on DefaultPagination itself.
    2. There is no snake_case router. DRF hard-codes {basename}-list / {basename}-detail, which
      ADR 0038 rule 11 forbids, so v2/urls.py carries SnakeCaseRouter, a four-line SimpleRouter
      subclass that generates the names in the published form rather than rewriting them afterwards.
      A router that names its routes from a configurable separator belongs in the library.
    3. edx_rest_framework_extensions.errors.classify_error does not know django.http.Http404, so a
      404 raised by DRF's own generic member lookup is enveloped as internal at status 404. A
      four-line view mixin translates it to NotFound here; the catalog is the right place to fix it.
    4. No OpenApiAuthenticationExtension is registered for DefaultJwtAuthentication or
      DefaultSessionAuthentication, so drf-spectacular cannot describe the platform's default
      authentication pair and the document carries no securitySchemes and no security
      requirement
      on any view that uses it — six of this PR's schema warnings, and the same warnings
      on the enrollment views today. The extensions belong beside the authentication classes, not in
      one API's schema module.
    5. flatten_detail renders the whole DRF detail object into the envelope's detail member with
      str(), so a rejected parameter publishes a Python literal —
      {'country_code': [ErrorDetail(string='…', code='invalid')]} — in the one member a client or an
      APM pipeline is most likely to log. The per-field text is correct in errors, which is what
      these tests assert and what clients should read. Joining the messages into a sentence belongs in
      the library, not in a local override of the envelope.
    6. The 405/406 catalog gap: _BUILTIN_ERROR_TYPES classifies none of MethodNotAllowed,
      NotAcceptable, UnsupportedMediaType or ParseError, so a 405, a 406, a 415 and a malformed
      body are all enveloped as internal / "Internal Server Error" at a 4xx status for every
      standardized API in the platform. This PR names the two its own endpoints can produce, through
      subclasses of its own that change nothing for anyone else (see Error-format decision); the
      catalog is where the four belong, and the subclasses are written to be deleted when they land
      there.
  • Phases B–E of [API] User #39058 build on the wiring added here, each as its own PR: accounts and profile
    images (12 operations), preferences and email opt-in (6), the retirement pipeline and username
    replacement (13), and registration, login session, validation and password reset (9) — E is not
    approved to build
    . SDK tagging (tags=["openedx-platform-sdk"]) starts at phase B; no operation
    in this PR carries it, so no SDK regeneration PR is owed by this one.
  • Future-version candidate, recorded rather than smuggled in: /api/user/v2/users/ and
    /api/user/v2/accounts/ (phase B) will publish two representations of one record for two
    audiences. Consolidating them is a contract change for a later version.
  • Second future-version candidate: a dotted ?fields= path (preferences.time_zone) passes
    validation on its first segment and is then not traversed, so a caller that asks for one nested
    property receives the whole property it sits in. The shaping helper strips the path by design;
    honouring it, or refusing it outright the way an unknown top-level name is refused, is a contract
    change rather than a tightening.
  • Clients not checked: openedx-app-ios (not fetched) and openedx/openedx-platform-sdk (not
    readable anonymously). Neither is expected to call these seven admin-only addresses, but neither
    was verified.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant