Skip to content

feat: serve course team members from authoring v2 - #39306

Draft
taimoor-ahmed-1 wants to merge 13 commits into
openedx:masterfrom
edly-io:feat/standardize-course-team-api
Draft

taimoor-ahmed-1 wants to merge 13 commits into
openedx:masterfrom
edly-io:feat/standardize-course-team-api

Conversation

@taimoor-ahmed-1

Copy link
Copy Markdown
Contributor

Stacked on #39305 (custom pages and tabs, which adds the shared paginator DocumentedDefaultPagination), which is itself stacked on #39303 (advanced settings, which adds IsExistingCourse and the shared test helpers). Until both merge this branch also carries their commits; only the last 3 commits belong to this PR.

Summary

Standardizes the course team resource of the Course Management area (#39059, part of #38137). This is PR H of the eight resource-group PRs for the area; #39303 (advanced settings) is PR A and #39305 (custom pages and tabs) is PR C.

Two conforming operations replace the one v1 course team endpoint:

v1 (unchanged, now deprecated: true) v2 URL name (authoring_v2:)
GET /api/contentstore/v1/course_team/{course_id} GET /api/authoring/v2/courses/{course_key}/team_members/ course_team_member_list
(same: allow_actions, show_transfer_ownership_hint) GET /api/authoring/v2/courses/{course_key}/team_members/me/ course_team_member_me

It applies ADRs 0025–0038 and OEP-69.

One resource, a list and the caller. v1 returns one object: users (every instructor and staff member of the course) next to two facts about the caller, allow_actions and show_transfer_ownership_hint. v2 separates them:

  • team_members/ lists the members, {id, username, email, role}, in the shared seven-key page envelope. The order is fixed: instructors first, then staff, each by username. A user holding both roles is listed once, as an instructor. v1 built each role's members from a Python set, so the order within a role was not defined.
  • team_members/me/ is the caller as the team sees them: {username, email, role, can_manage_team, is_only_instructor}. can_manage_team is v1's allow_actions and is_only_instructor is v1's show_transfer_ownership_hint, both computed by the same Studio helper. role is null when the caller holds no team role. me follows ADR 0038 rule 9; the caller's flags move there so the page envelope keeps exactly its seven keys (ADR 0032).

RISK, stated in the schema: me sits in the member slot but returns a different component (CourseTeamCaller.v2, not CourseTeamMember.v2). It answers 200 with role: null for callers who may see the team without being on it: global staff, and org staff or org instructors. ADR 0038's own example address for this resource is …/team_members/{username}/. If that member is added later, one person would then have two components (the list item and me), and the two must be reconciled. Today no team_members/{username}/ route exists; such an address gets the CMS HTML 404.

RISK, kept from v1: every member's email is returned to anyone who may see the team (course staff included), as v1 does. Studio's team page needs it, because its write handler addresses members by email.

Why a new version and not a fix in place. Each of these changes the contract for v1's callers (the Course Team page in frontend-app-authoring calls it), so they ride v2 (ADR 0037):

  • the list becomes paginated (one object holding every member becomes the seven-key envelope);
  • the caller's flags move from the list body to me, under new names;
  • errors use the standard envelope;
  • Bearer tokens are refused;
  • addresses gain a trailing slash and a checked course key.

Changes

Commit Endpoint(s) Legacy behaviour kept New behaviour
feat: add the course team permission class — v1's inline check unchanged CanViewCourseTeam in views/permissions.py: the same get_user_permissions(user, course_key) & STUDIO_VIEW_USERS call v1's view makes inline
feat: serve course team members from authoring v2 the two v2 operations above (new) v1 route, view and URL name unchanged View, service, serializers, routes and tests. The service reuses the legacy helper contentstore.utils.get_course_team, which is not edited
docs: mark the v1 course team endpoint deprecated in favour of authoring v2 the v1 operation Route, view and response unchanged Docstrings name both successors and #39128. (deprecated: true in the schema comes from the exact SUPERSEDED_PATHS entry, with its test, added in the feat commit.)

ADRs applied

  • 0025 serializers — separate output serializers for the member (CourseTeamMember.v2) and the caller (CourseTeamCaller.v2), help_text on every field, explicit .v2 component names; read-only, so no request serializer
  • 0026 permissions — IsAuthenticated, IsExistingCourse, then CanViewCourseTeam (the check v1 made inline); the viewset refuses any action it was not written for; deny matrix with policy permissions off and on
  • 0027 schema / docs — @extend_schema on both with summary, description, parameters, examples and every 4xx as ErrorResponse; both tagged openedx-platform-sdk
  • 0028 viewsets — applied except router registration: one ViewSet over a service module, mounted with path(...) like every route in v2/authoring_urls.py, because a router's regex routes cannot use the shared path converters
  • 0029 errors — partial: the envelope on every response these views produce; 405 and 406 are typed internal, and a malformed course key or an address no route matches gets the CMS HTML 404 (library and routing gaps, pinned by tests); the published ErrorResponse component's text comes from edx-drf-extensions
  • 0030 GET idempotence — met for this code (no write, signal or event on GET or HEAD), with one inherited write outside it: openedx-authz inserts its PolicyCacheControl row on the first permission check against a database that lacks it, as on v1
  • 0031 merged endpoints — n/a: nothing merged; v1's one endpoint becomes the list and the caller's me member of the same resource, because the caller's flags cannot ride the page envelope
  • 0032 pagination — the list returns the seven-key envelope (page, page_size 10 by default, at most 100) through the shared DocumentedDefaultPagination from feat: serve course custom pages and tabs from authoring v1 #39305; v1 was unbounded
  • 0033 filtering — n/a: no filter or sort parameter; the order is fixed and documented (instructors, then staff, each by username), and adding filters would be a new capability
  • 0034 authentication — (JwtAuthentication, SessionAuthenticationAllowInactiveUser) declared with its reason, as ADR 0034 requires for an endpoint that used SessionAuthenticationAllowInactiveUser; Bearer dropped
  • 0035 MFE config — n/a: per-course authoring data, not site or front-end configuration
  • 0036 nested JSON — n/a: a flat list of thin objects (four scalar members each) and a flat me of five scalars
  • 0037 versioning — partial: new v2 modules, v1 frozen (docstring-only markers) and deprecated: true in the schema; the DEPR entry for [DEPR]: Contentstore and enrollment API versions superseded by FC-0118 #39128 is to be posted with this PR, and no removal release is named yet
  • 0038 URL structure — courses/{course_key}/team_members/ (the ADR's own example noun) and team_members/me/ (rule 9); shared converter, trailing slash, version-free URL names; the v1 name kept
  • OEP-66 queryset scoping — n/a as a mechanism: the view owns no get_queryset(); the team is read through Studio's course role helpers; the layering is shown in the matrix
  • OEP-69 conventions — the decisions above as recorded, with the 0028, 0029, 0030 and 0037 departures stated; no governance identifiers in code or schema text

Compliance matrix

Decision Status How it is met, or the concrete reason it is excluded Evidence (test / gate)
0025 met CourseTeamMemberSerializer (CourseTeamMember.v2: id, username, email, role ∈ {instructor, staff}) gives the same item v1's users holds. CourseTeamCallerSerializer (CourseTeamCaller.v2: username, email, nullable role, can_manage_team, is_only_instructor). Explicit ref_names, because CourseTeam already collides platform-wide. can_manage_team's help text says it concerns other members: a caller without it may still remove themself or move themself from instructor to staff, as Studio allows TeamMembersSchemaTest (test_the_member_and_caller_components, test_every_member_is_described); TeamMembersListTest.test_the_body_matches_the_published_schema; TeamMembersMeTest.test_the_body_matches_the_published_schema; hygiene PASS
0026 met Every v1 check has a v2 home (Authorization below). Order 401, 404, 403. CanViewCourseTeam reads the parsed course_key and refuses a view without one. The viewset serves only list, retrieve, OPTIONS and unrouted methods; any other action raises ImproperlyConfigured, answered as a 500 envelope instead of being served unchecked TeamMembersAccessLegacyPermissionsTest, TeamMembersAccessAuthzTest (each access case asserts v1's answer too); TeamMembersPermissionWiringTest (test_both_actions_check_the_course_then_the_permission_to_see_the_team, test_any_other_action_is_refused, test_a_routed_action_the_viewset_does_not_check_is_never_served, test_the_team_permission_needs_studios_permission_to_see_users, test_the_team_permission_refuses_a_view_without_a_course)
0027 met Operation IDs v2_courses_team_members_list, v2_courses_team_members_me_retrieve. course_key is a path parameter with the converter's pattern; the list adds page and page_size. Examples TeamMember and OnlyInstructor validate against the published components. Both tagged openedx-platform-sdk; the deprecated v1 operation loses the tag through untag_deprecated_operations TeamMembersSchemaTest (test_both_operations_are_published_for_the_sdk, test_the_parameters, test_the_examples_are_valid_bodies, test_the_legacy_operation_is_deprecated_and_left_out_of_the_sdk)
0028 applied except router registration CourseTeamMembersViewSet (StandardizedErrorMixin first, then IterablePaginationMixin, viewsets.ViewSet) over v2/team_members_service.py. A plain ViewSet, because the team is read through Studio's role helpers, not a queryset this view owns. Deviation: mounted with path(), not a router TeamMembersRoutingTest.test_the_addresses; TeamMembersReadCost{LegacyPermissions,Authz}Test
0029 partial StandardizedErrorMixin first. 401 authn, 403 authz, 404 not-found (unknown, version-only or library course; a page past the last). Neither operation takes a body or a validated query member, so there is no 400. Gaps pinned by tests that flip when fixed: 405 and 406 are typed internal (edx-drf-extensions 10.9.0 does not catalogue them); a malformed or slash-form key, and any address under team_members/ other than me/, get the CMS HTML 404 at routing. The published ErrorResponse component's description is the library's own text, which names ADR 0029; changing it is a library change TeamMembersListTest.test_a_page_beyond_the_last_is_404; TeamMembersRoutingTest (test_other_methods_are_not_allowed, test_unsupported_accept_is_refused_with_406, test_an_unparsable_or_slash_form_key_is_an_html_404, test_only_the_caller_is_addressable_as_a_member); TeamMembersSchemaTest.test_both_operations_are_published_for_the_sdk
0030 met for this code; one inherited write GET and HEAD of the list (plain and paged) and of me read only. With policy permissions off and on, no read writes to SQL or Mongo, stores through the modulestore, or sends a Django signal or Open edX event. The exception is the first read on a database without openedx-authz's PolicyCacheControl row, which inserts it (one INSERT, its pre_save/post_save). That write belongs to the permission library, happens on v1 too, and is allow-listed by name TeamMembersReadSideEffects{LegacyPermissions,Authz}Test (test_reads_have_no_side_effects, test_the_legacy_read_makes_the_same_first_lookup_write)
0031 excluded Nothing merged. v1's single endpoint is split into the list and the caller's member of one resource, because the caller's flags cannot ride the seven-key envelope —
0032 met IterablePaginationMixin with the shared DocumentedDefaultPagination (#39305), which publishes all seven envelope keys. The service sorts the full team before paging, so pages are stable TeamMembersListTest (test_the_envelope_has_the_seven_keys, test_page_size_splits_the_pages, test_the_default_page_size_is_ten, test_the_page_size_is_at_most_one_hundred, test_a_page_beyond_the_last_is_404); TeamMembersSchemaTest (test_the_list_publishes_the_seven_envelope_keys, test_the_paginator_is_the_shared_one)
0033 excluded No filter or sort; fixed, documented order TeamMembersListTest.test_instructors_come_first_then_staff_each_by_username; TeamMembersResponseDifferencesTest.test_members_holding_the_same_role_are_ordered_by_username
0034 met with declared deviation (JwtAuthentication, SessionAuthenticationAllowInactiveUser), commented at the declaration: an inactive account's session stays authenticated, so the course permission decides; inactive global staff are admitted and inactive course staff are refused (403, not 401). Bearer is dropped. Same as #39303 and #39305 TeamMembersAuthenticationTest (test_a_jwt_caller_may_list_and_see_themself, test_a_session_caller_may_list_and_see_themself, test_an_inactive_session_is_authenticated, test_a_bearer_token_is_refused); TeamMembersAccessAuthzTest.test_an_inactive_session_with_a_role_is_refused
0035 excluded Not configuration; per-course, per-user —
0036 excluded Flat list of thin objects; a flat me —
0037 partial New modules in v2/. v1 frozen apart from docstring-only markers: the syntax tree with docstrings stripped equals the base. deprecated: true through one exact SUPERSEDED_PATHS entry; no runtime header (frozen route). DEPR: an entry for #39128, to be posted with this PR; no removal release is named versions gate; CmsMarkSupersededPathsTest.test_the_course_team_operation_is_marked_and_its_successors_are_not; TeamMembersSchemaTest (test_the_legacy_operation_is_deprecated_and_left_out_of_the_sdk, test_only_superseded_or_migrated_addresses_are_deprecated)
0038 met team_members under the course (the ADR's example noun); me per rule 9; shared course_key converter; trailing slash; version-free URL names TeamMembersRoutingTest (test_the_addresses, test_the_legacy_address_is_unchanged, test_the_url_names_are_unique_in_the_namespace, test_the_address_without_its_trailing_slash_redirects_to_it, test_the_legacy_address_still_takes_a_slash_separated_key_that_v2_does_not); the urls gate itself was skipped
OEP-66 mechanism excluded, layering met (1) endpoint access: the permission classes; (2) record visibility: the holders of the course's instructor and staff roles, read by get_course_team as on v1; (3) user selection: none beyond page/page_size, which cannot widen what is listed access tests above; TeamMembersListTest (test_members_of_another_course_are_not_listed, test_global_staff_without_a_role_are_not_listed)
OEP-69 met The decisions above, with the departures stated in their rows. No ADR/OEP/PR identifiers or narration in code or schema text hygiene gate PASS (0 FAIL, 0 WARN)

Backward compatibility

  • v1 is unchanged. v1/views/course_team.py gains docstring text only; its syntax tree with docstrings stripped equals the base. Its route, view and URL name are untouched, and v1's own tests are not edited.

  • Parity. v2 is compared with v1 through the Django test client, with policy permissions off and on:

    • the list holds v1's members, in listed order, with identical members (two instructors and three staff, seen by an instructor and by a staff member);
    • me says what v1's body says about the caller, for every member;
    • 401, 403 and 404 differ only in the error body.

    The only declared body differences, verbatim from the test module:

    INTENTIONAL_DIFFERENCES = [
        ("type", "errors use the standardized envelope"),
        ("title", "errors use the standardized envelope"),
        ("status", "errors use the standardized envelope"),
        ("detail", "errors use the standardized envelope"),
        ("developer_message", "the legacy error body carries developer_message; the envelope does not"),
        ("error_code", "the legacy not-found body carries error_code; the envelope does not"),
    ]

    The comparison fails both on an undeclared difference and on a declared difference that never occurs (_ParityMixin.test_every_declared_difference_occurs).

  • Status and request differences from v1. Each is listed in the test module and observed against v1's own answer:

    RESPONSE_DIFFERENCES = [
        ("listing", "one object holding every member and the caller's flags", "a page of members in the standard envelope",
         "test_the_list_is_paginated"),
        ("order of members holding the same role", "no particular order", "by username",
         "test_members_holding_the_same_role_are_ordered_by_username"),
        ("whether the caller may manage the team, and is its only instructor",
         "allow_actions and show_transfer_ownership_hint on the team", "can_manage_team and is_only_instructor on me",
         "test_the_caller_flags_are_on_me"),
        ("listing with a page beyond the last", "200 with every member (no paging)", "404",
         "test_a_page_beyond_the_last"),
        ("OPTIONS by a signed-in caller without access, or for an unknown course", "200", "403 or 404",
         "test_options"),
        ("any request with a Bearer token", "200", "401", "test_a_bearer_token"),
        ("slash-separated course key", "404 JSON", "404 HTML", "test_a_slash_separated_course_key"),
    ]

    As a table, with the request-shape changes that carry no status difference:

    Request v1 v2 Test
    list GET v1/course_team/{course_id}, one object: users, allow_actions, show_transfer_ownership_hint GET v2/courses/{course_key}/team_members/, 7-key envelope, 10 per page by default; items unchanged TeamMembersResponseDifferencesTest.test_the_list_is_paginated
    order within a role no particular order by username (instructors first, then staff, as on v1) …test_members_holding_the_same_role_are_ordered_by_username
    the caller's flags allow_actions, show_transfer_ownership_hint on the list body can_manage_team, is_only_instructor on GET …/team_members/me/, with the caller's role …test_the_caller_flags_are_on_me
    page past the last 200, every member (page ignored) 404 …test_a_page_beyond_the_last
    OPTIONS by an outsider, or for an unknown course 200 403 / 404 …test_options
    Bearer token 200 401 …test_a_bearer_token
    slash-form course key 404 JSON (course_does_not_exist) 404 HTML (no route) …test_a_slash_separated_course_key
    address without the trailing slash served 301 to the slashed address TeamMembersRoutingTest.test_the_address_without_its_trailing_slash_redirects_to_it
  • Query counts, relative to v1. No literal baseline was measured, because no platform environment was available. Instead, in the same test, the list and me each run no more SQL queries and no more Mongo reads than v1's endpoint, and no Mongo writes, with policy permissions off and on. Their SQL count stays the same from 1 to 20 members (measured at page_size=100). The growth check counts SQL only.

  • Schema. Two new operations, and deprecated: true on the v1 operation. docs/cms-openapi.yaml is not edited; the generate_openapi_schemas workflow regenerates it.

In-place edits

File Category Evidence
cms/djangoapps/contentstore/rest_api/v1/views/course_team.py docstring-only deprecation marker (class and get) syntax tree with docstrings stripped equals the base; versions gate ALLOWED
cms/djangoapps/contentstore/rest_api/v2/authoring_urls.py strictly opt-in addition: one import, two routes no line removed; TeamMembersRoutingTest
cms/djangoapps/contentstore/views/permissions.py (shared, unversioned) one class added (CanViewCourseTeam); the existing common.djangoapps.student.auth import gains two names; existing classes unchanged TeamMembersPermissionWiringTest and the access tests
cms/lib/spectacular.py, cms/lib/tests/test_spectacular.py one exact SUPERSEDED_PATHS entry; the exact-set test updated and a test for the course team path added CmsMarkSupersededPathsTest

New files inside the pre-existing v2/: team_members_service.py, serializers/team_members.py, views/team_members.py, views/tests/test_team_members.py. The production modules import nothing from rest_api/v0/ or rest_api/v1/.

Test-only imports from v1. The test module imports these shared helpers from rest_api/v1/views/tests/helpers.py (added by #39303): FIRST_LOOKUP_SIGNALS, FIRST_LOOKUP_WRITES, UNKNOWN_COURSE_KEY, VERSION_ONLY_KEY, api_client, assert_matches_published_schema, assert_refused, changed_paths, measure, published_schema, record_effects, session_client, undeclared_paths. They are the area's shared test helpers (clients, the side-effect recorder, the parity diff, the cost measure, the schema check); importing them keeps one copy rather than a second one in v2/. resolve_ref comes from the existing v2/views/tests/helpers.py.

Error-format decision

Versioned. v1 keeps its developer_message/error_code bodies unchanged. v2 uses the envelope. The Course Team page in frontend-app-authoring calls the v1 endpoint (src/course-team/data/api.ts); the list's pagination and the move of the caller's flags already force a new version, and the error format rides with it.

Deprecations

The v1 operation is superseded by the two v2 operations in the Summary. It is deprecated: true in the CMS schema, and the docstrings name both successors. The view is otherwise unchanged: no runtime header. Tracked on the FC-0118 umbrella DEPR ticket, #39128; this resource's entry is posted there with the PR. Window per #39128: removed one named release after the release this ships in.

Migration notes:

  • Members: call GET /api/authoring/v2/courses/{course_key}/team_members/. It is paginated: read results, and follow next or pass page/page_size (at most 100) to get every member. Items are unchanged (id, username, email, role). Instructors come first, then staff, each by username.
  • The caller's flags: call GET /api/authoring/v2/courses/{course_key}/team_members/me/. allow_actions is can_manage_team; show_transfer_ownership_hint is is_only_instructor. role is the caller's team role, or null when they hold none.
  • Errors use the standard envelope.
  • Bearer tokens are refused; use JWT or a session.
  • Every address ends with a slash. Slash-separated (deprecated) course keys are not accepted.

Authorization

Legacy check Where it lives in v2 Deny-path tests
@view_auth_classes(is_authenticated=True) IsAuthenticated test_anonymous_caller_is_refused_with_401 (both permission modes); test_anonymous_caller_of_an_unknown_course_is_refused_with_401
@verify_course_exists() IsExistingCourse (also 404 for version-only and library keys) unknown course, version-only key and library key → 404, before any role check (test_unknown_course_is_404_before_permission_is_checked, test_a_version_only_course_key_is_404, test_a_library_is_not_a_course)
get_user_permissions(user, course_key) & STUDIO_VIEW_USERS, else permission_denied, inline in get CanViewCourseTeam (new), same call no role; staff or instructor of another course; limited staff; with policy permissions off, the course editor and course auditor policy roles; with them on, auditor, editor, limited staff, data researcher, beta tester, and an admin role on another course; inactive course staff in both modes

Order: 401, then 404 for a missing course, then 403. Allowed: course staff and instructor, org staff and org instructor, global staff (also when the account is inactive), and holders of the course admin or course staff policy role. can_manage_team is true for instructors, org instructors and global staff, as v1's allow_actions is.

Corrections to the area plan, found in review and checked against the code. The tests follow the platform's behaviour, which v2 keeps:

  • Course admin and course staff policy roles are admitted, whatever the flag. Studio's role cache converts these openedx-authz assignments into the instructor and staff course roles whether or not policy-based permissions are on for the course (common/djangoapps/student/roles.py, RoleCache). Their holders get 200 on v1 and v2; the plan said 403. Every other policy role (editor, auditor, limited staff, data researcher, beta tester) is refused, even though the policy lets editor and auditor view the team, because they stand for no Studio course role. Course team access still does not consult courses.view_course_team (test_a_policy_course_staff_role_opens_the_team_without_listing_its_holder, test_a_policy_role_without_a_studio_course_role_is_refused, test_the_course_admin_and_course_staff_roles_are_allowed, test_other_roles_are_refused).
  • Inactive accounts. Inactive course staff are refused (403): a course role does not count while the account is inactive. Inactive global staff are admitted (200). The plan said an inactive session gets 200 (TeamMembersAuthenticationTest.test_an_inactive_session_is_authenticated, TeamMembersAccessAuthzTest.test_an_inactive_session_with_a_role_is_refused).
  • Superusers are admitted because saving a superuser sets is_staff. The pre_save receiver grant_staff_access_to_superusers (common/djangoapps/student/signals/receivers.py) makes every saved superuser global staff. The policy engine is not the reason, contrary to what feat: serve course advanced settings from authoring v1 #39303 and feat: serve course custom pages and tabs from authoring v1 #39305 say (test_a_superuser_is_allowed_as_global_staff, test_staff_and_superuser_are_allowed_without_a_role).

Who may see the team and who is listed come from different code paths, as on v1. With policy permissions off, a course staff policy-role holder may see the team but is not listed, because the list is read from Studio's role records, and their me.role is null. With policy permissions on, a holder of Studio's own staff role may see the team but is not listed, because the list is read from the policy assignments (test_a_studio_staff_role_still_opens_the_team_without_listing_its_holder).

Gate report

Run on the host against 8721326a74 (the head of #39305, which this branch stacks on). No platform environment was available, so four gates were skipped. The test module has not been run locally, and CI is its first run.

Gate Result
versions PASS: 2 ALLOWED (v1/views/course_team.py docstring marker, v2/authoring_urls.py), 4 ADDED in the existing v2 directories, nothing protected touched (23 shared legacy modules protected, including contentstore/utils.py, whose get_course_team v2 reuses)
hygiene PASS (0 FAIL, 0 WARN) over 9 files
ruff clean; py_compile clean; no line over 120 characters
pylint (4.1.3, repo pylintrc) only C1803 (== [] comparisons), the same message #39303 carries with its CI pylint passing
supplementary, not a gate: offline harness with fakes 63/63 checks; 22/22 mutants killed
schema SKIPPED (no platform environment)
urls SKIPPED (no platform environment)
old-tests SKIPPED (no platform environment)
tests SKIPPED (no platform environment)

Follow-ups

  • SDK: both operations carry openedx-platform-sdk; the next openedx-platform-sdk regeneration gains them.
  • frontend-app-authoring: the Course Team page (src/course-team/data/api.ts) can move to the v2 list (request page_size=100 or follow next) and read its flags from me. Its writes still go to Studio's course_team/{course}/{email} handler, which has no REST successor in this area.
  • A team_members/{username}/ member (the ADR's example address), if added, must be reconciled with me: one component for a person, or the difference documented.
  • edx-drf-extensions:
    • Catalogue MethodNotAllowed and NotAcceptable.
    • The shared ErrorResponse component's description is ErrorResponseSerializer's docstring, which names ADR 0029 and carries a usage example; it should describe the error body instead. Issue to be filed.
    • L3: DefaultPagination.get_paginated_response_schema with all seven keys (from feat: serve course custom pages and tabs from authoring v1 #39305), after which the shared paginator can be deleted.
  • Platform: a JSON 404 for addresses under /api/ that match no route, instead of the CMS HTML page. Issue to be filed.
  • openedx-authz PolicyCacheControl: the first permission check on a fresh database inserts the cache row on a read; seeding it by migration would make reads write-free. Issue to be filed.
  • Authentication across /api/authoring/v2/: v2 here and the feat: apply FC-0118 ADRs to course content APIs #39157 v2 views keep SessionAuthenticationAllowInactiveUser; feat: serve the course outline from authoring v2 #39229 and feat: serve course containers from authoring v2 #39274 use the default on the same mount. One policy for new versions of such endpoints should be agreed.
  • feat: serve course advanced settings from authoring v1 #39303 and feat: serve course custom pages and tabs from authoring v1 #39305 explain superuser admission with the wrong cause, in their descriptions and test docstrings; to be corrected in those PRs.
  • Sibling PRs F (course settings) and G (group configurations) edit the same shared files: views/permissions.py, v2/authoring_urls.py, cms/lib/spectacular.py and its test. The conflicts are additive, resolved by rebasing in merge order.

Taimoor Ahmed added 13 commits October 25, 2026 15:09
The v0 advanced settings view checks course existence with the
@verify_course_exists decorator and access with an inline
check_course_advanced_settings_access call in each handler. The
conforming views need these as permission classes.

IsExistingCourse answers 404 for a course key that names no course run:
an unknown course, a key that names only a version, or a library key.
CanViewAdvancedSettings and CanManageAdvancedSettings make the same
check_course_advanced_settings_access(read|write) call the v0 view
makes, so the policy-permission and legacy Studio-access branches are
unchanged. Existing classes in the module are untouched.
GET/PATCH /api/contentstore/v0/advanced_settings/{course_id} answers
errors with developer_message bodies and some failures with a 500,
accepts Bearer tokens, returns every settings field by default, and
reports a refused value with the field parser's own text, which can be
Python exception text.

GET/PATCH /api/authoring/v1/courses/{course_key}/advanced_settings/ is
its successor. Errors use the standard envelope; a refused value is
reported under its setting name with one fixed message and the original
text is logged. The default read is the set of settings course authors
edit (v0 with fetch_all=0); view=full returns every field and fields
narrows either view. Authentication keeps the session class that admits
inactive users, as the legacy endpoint did; Bearer is dropped. Access is
IsAuthenticated, IsExistingCourse and a per-action class making v0's
read or write check, and an action without a declared permission is
never served. Parity tests compare v1 with v0 through the test client;
the declared differences are the error envelope and the fixed message.
GET/PATCH /api/contentstore/v0/advanced_settings/{course_id} now has a
conforming successor at /api/authoring/v1/courses/{course_key}/advanced_settings/.
Both v0 operations are marked deprecated in the CMS schema through an
exact SUPERSEDED_PATHS entry, and the view's docstrings name the
successor, how each v0 read maps to a v1 read, and the deprecation
ticket. The view itself is unchanged. The schema hook test that used
this path as its example of an unrelated contentstore path now uses the
heartbeat, which is not deprecated.
Importing cms.djangoapps.contentstore.rest_api.v1.authoring_urls at
module level builds its routes before cms/urls.py registers the
course_key path converter, so collecting the module failed with an
ImproperlyConfigured error. The URL-name test now imports it when it
runs, as the youtube transcript tests do. Also wraps a line pylint
reported as too long.
The first CI run of the module showed three expectations that did not
match the platform:

- The legacy role checks consult the policy engine, which admits
  superusers, so a superuser who is not global staff is served by v0 and
  v1 alike. The test now expects that.
- The proctoring provider field keeps its default unless proctored exams
  are enabled, so the proctoring tests enable them.
- The JWT middleware appends NotJwtRestrictedApplication to the
  permissions of views that accept JWTs, so the wiring test compares only
  the classes the view declares.
The side-effect recorder, the body comparison helpers, the schema
helpers and the client helpers in the advanced settings tests are not
specific to advanced settings. They move unchanged, under public names,
into v1/views/tests/helpers.py so the other authoring v1 endpoint tests
reuse one copy instead of each carrying its own.
edx-drf-extensions' DefaultPagination returns seven envelope keys but
publishes a schema with four, so generated clients drop num_pages,
current_page and start. DocumentedDefaultPagination keeps the library's
pagination and publishes all seven keys, each with an example. It lives
in an unversioned module for every authoring list to reuse, and is to
be deleted once the library documents the envelope itself.
The v0 tab views check courses.manage_pages_and_resources inline with a
legacy write-access fallback before reordering custom pages or changing
a tab. CanManagePagesAndResources makes the same call, beside the
existing CanViewPagesAndResources. Existing classes are unchanged.
The v0 tabs endpoints return the custom pages as a bare array, reorder
them through a POST verb that answers a missing page with 500 and stores
a repeated page twice, change a tab's visibility through a POST verb that
echoes its input in errors, and accept Bearer tokens.

GET /api/authoring/v1/custom_pages/?course_key=... lists a course's
custom pages a page at a time in course order; a custom page is
addressed by its own usage key, so the list is top level with the course
in the query. PUT /api/authoring/v1/courses/{course_key}/custom_page_order/
replaces their order, with pages left out kept after the listed ones.
PATCH /api/authoring/v1/courses/{course_key}/tabs/{tab_id}/ shows or
hides a built-in tab and returns it. Errors use the standard envelope
with fixed messages. Access makes v0's view and manage checks. Parity
tests compare the stored tabs and each refusal with v0.
The three v0 tab operations now have conforming successors under
/api/authoring/v1/. They are marked deprecated in the CMS schema through
exact SUPERSEDED_PATHS entries, and the views' docstrings name the
successors and the deprecation ticket. The views themselves are
unchanged.
CanViewCourseTeam makes the same check the v1 course team endpoint makes
inline (Studio permissions on the course include viewing users), so the
authoring v2 team members endpoint can declare it as a permission class.
Policy-assigned course admin and course staff roles count as the Studio
roles they stand for; other policy roles are refused, as on v1.
Adds GET /api/authoring/v2/courses/{course_key}/team_members/ (a paginated
list in a fixed order: instructors, then staff, each by username) and
GET .../team_members/me/ (the caller's role and whether they may manage
the team), replacing GET /api/contentstore/v1/course_team/{course_id}.

The new endpoints use the standard error envelope, drop Bearer
authentication, answer 404 for an unknown course before checking roles,
and are tagged for the SDK. The legacy path is marked deprecated in the
published schema.
…ing v2

Docstring-only: names the successor addresses and the deprecation ticket.
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 25, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @taimoor-ahmed-1!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform-oncall.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Update the status of your PR

Your PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate.


Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

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

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

2 participants