Skip to content

feat: serve the course optimizer from authoring v1 - #39304

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

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

Conversation

@taimoor-ahmed-1

Copy link
Copy Markdown
Contributor

Summary

Stacked on #39303 (advanced settings, which adds IsExistingCourse and the shared test helpers). Until #39303 merges this branch also carries its commits; only the last 3 commits belong to this PR.

Standardizes the course optimizer resources of the Course Management area (#39059, part of #38137). This is PR B of the eight resource-group PRs for the area. #39303 (advanced settings) is the first; custom pages and tabs, file assets, video transcripts and images, course settings, group configurations and team members follow separately.

Four action-style v0 routes become two resources, each with a verb per operation:

v0 (frozen, deprecated) v1 URL name
POST /api/contentstore/v0/link_check/{course_id} POST /api/authoring/v1/courses/{course_key}/link_check/ authoring_v1:course_link_check
GET /api/contentstore/v0/link_check_status/{course_id} GET /api/authoring/v1/courses/{course_key}/link_check/ same
POST /api/contentstore/v0/rerun_link_update/{course_id} POST /api/authoring/v1/courses/{course_key}/previous_run_link_update/ authoring_v1:course_previous_run_link_update
GET /api/contentstore/v0/rerun_link_update_status/{course_id} GET /api/authoring/v1/courses/{course_key}/previous_run_link_update/ same

v0 stays mounted and unchanged. It is marked deprecated: true in the CMS schema and in its docstrings.

It applies ADRs 0025–0038 and OEP-69.

Behaviour.

  • Starting a run. A POST queues the same Celery task with the same arguments as v0 (check_broken_links, update_course_rerun_links). It answers 202 Accepted with {"status": "Pending"}.
  • Reading a run. A GET returns the latest run of the course that the caller may see: their own, or anyone's for a superuser, through django-user-tasks' StatusFilterBackend as on v0.
    • Without view (or with view=minimal) it returns the state of the run. For a link check that is {status, created_at, error}; for a link update, {status}.
    • view=full adds the link check's report (output) or the link update's per-link results.
    • The default read never reads or builds the report.
  • Run states are django-user-tasks' strings verbatim, published as one enum. That enum adds Uninitiated (no run) and the link-update task's own stages, Scanning and Updating.
  • Member names are snake_case.
  • Starting a link update takes {"action": "all" | "single", "data": [...]}. data is validated (each link needs url, type and id) when action is single. When action is all it is not read, as on v0.
  • Feature flag. Where updating links to a previous run is not enabled for the course, both link-update operations answer 404 once the caller's permission is established.

Why a new version and not a fix in place. Each of these changes the contract for v0's callers, so they ride v1 (ADR 0037):

  • errors use the standard envelope;
  • a started run answers 202 instead of 200;
  • member names change from LinkCheckStatus/displayName to status/display_name;
  • the report moves behind view=full;
  • "feature off" answers 404 instead of 400;
  • Bearer tokens are refused;
  • the addresses gain a trailing slash and a converter-checked course key.

The Course Optimizer page in frontend-app-authoring calls all four v0 routes (src/optimizer-page/data/api.ts).

Changes

Commit Endpoint(s) Legacy behaviour kept New behaviour
feat: add course content editing permission classes — user_has_course_permission and the legacy READ/WRITE fallback unchanged CanStartLinkCheck (the edit-course-content permission, with Studio read access as the legacy fallback, as v0's link check POST) and CanEditCourseContent (the same permission, with Studio write access as the fallback, as v0's other three operations)
feat: serve the course optimizer from authoring v1 POST/GET /api/authoring/v1/courses/{course_key}/link_check/, POST/GET /api/authoring/v1/courses/{course_key}/previous_run_link_update/ (new) v0 routes, views and URL names unchanged; same tasks, same task arguments, same run-visibility filter The endpoints above, their service and serializers, two routes, and their tests
docs: mark the v0 course optimizer deprecated in favour of authoring v1 the four v0 routes Routes, views and responses unchanged deprecated: true in the schema (four exact SUPERSEDED_PATHS entries); the docstrings name each successor, what changes, and #39128

ADRs applied

  • 0025 serializers — separate request, response and query serializers with help_text on every field and .v1 component names; the queued-run, status, report and result shapes are all published
  • 0026 permissions — IsAuthenticated, IsExistingCourse, then a per-action class making the same check v0 makes for that operation; v0's READ-for-starting/WRITE-for-the-rest split is kept; unmapped actions are refused by construction; deny matrix with policy permissions off and on
  • 0027 schema / docs — @extend_schema on all four operations with summary, description, the view parameter, request and response examples and every 4xx as ErrorResponse; all four tagged openedx-platform-sdk
  • 0028 viewsets — applied except router registration: two ViewSets backed by a service module, mounted with path(...) and the shared course_key converter, as every route in authoring_urls.py is
  • 0029 errors — partial: the envelope on every response these views produce, with fixed messages in place of recorded failure text. Library gaps are pinned by tests: malformed JSON, 405, 406 and 415 are typed internal, and per-link errors under data are rendered as Python repr strings. A malformed key gets the CMS HTML 404 at routing
  • 0030 GET idempotence — met for this code (no write, signal, event or modulestore change on GET or HEAD, either view), with two inherited exceptions outside it. openedx-authz inserts its PolicyCacheControl row on the first permission check against a database that lacks it, as on v0. And the report builder that view=full reuses would create a course_info block if a reported one had been deleted since the check ran; that path is currently unreachable in practice and also exists on v0
  • 0031 merged endpoints — two merges: v0's link-check start and status routes, and its rerun-link-update start and status routes, each become one resource with POST (start) and GET (latest run). The link_update start's two operations are an enumerated action (all, single), each with its request shape. Each legacy permission check survives per action
  • 0032 pagination — n/a: each read returns one status object; the report is opt-in with view=full and bounded by the links the run flagged
  • 0033 filtering — n/a: no list and no filter; view selects a representation and is validated by a serializer
  • 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, per-user authoring data, not site or front-end configuration
  • 0036 nested JSON — the state of the run by default (view=minimal); the four-level report and the per-link results only with view=full, and the default read never builds them
  • 0037 versioning — partial: new v1 modules, v0 frozen (docstring-only markers) and deprecated: true in the schema; the DEPR entry is a comment on [DEPR]: Contentstore and enrollment API versions superseded by FC-0118 #39128 until it is folded into the issue body
  • 0038 URL structure — /api/authoring/v1/courses/{course_key}/link_check/ and .../previous_run_link_update/: domain API name, one-per-course children of courses, no verbs, shared converter, trailing slash, version-free URL names; the v0 names are kept
  • OEP-66 queryset scoping — n/a as a mechanism: no get_queryset(); record visibility is django-user-tasks' StatusFilterBackend, reused unchanged through the provider
  • 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 Output: QueuedRunSerializer (CourseOptimizerQueuedRun.v1), LinkCheckSerializer (LinkCheck.v1) with the report serializers (LinkCheckReport*.v1, LinkCheckPreviousRunLink.v1), and PreviousRunLinkUpdateSerializer with LinkUpdateResult.v1. Request: PreviousRunLinkUpdateRequestSerializer with LinkToUpdate.v1. Query: one view serializer per resource. The report serializers map the provider's camelCase members to snake_case with source=. The link type help text documents its four values (course_content, course_updates, handouts, custom_pages) CourseOptimizerSchemaTest (test_every_member_is_described, test_the_minimal_and_full_members, test_starting_a_link_update_takes_an_action_and_links, test_the_link_types_are_documented), CourseOptimizerReadTest.test_the_bodies_match_the_published_schema; hygiene H9
0026 met Every v0 check has a v1 home (Authorization below). The per-action classes make the same user_has_course_permission(user, "courses.edit_course_content", course_key, READ|WRITE) call v0 makes inline, so the policy branch and the legacy Studio-access branch are inherited unchanged. Starting a link check keeps v0's READ fallback; reading it and both link-update operations keep WRITE; the two are separate classes, not one. An action missing from the per-action map raises ImproperlyConfigured rather than running with login only CourseOptimizerAccessLegacyPermissionsTest (incl. test_starting_a_check_needs_only_read_access_and_the_rest_write_access, test_starting_a_check_is_refused_without_read_access), CourseOptimizerAccessAuthzTest, CourseOptimizerPermissionWiringTest; each access case asserts v0's answer and that a refused start queues nothing
0027 met Summary, description, view enum (minimal, full), 202/200/400/401/403/404 with ErrorResponse, request examples for both action values, and a view=full report example. Operation IDs v1_courses_link_check_create / _retrieve and v1_courses_previous_run_link_update_create / _retrieve. All four are tagged openedx-platform-sdk; the deprecated v0 operations lose the tag through untag_deprecated_operations CourseOptimizerSchemaTest (test_the_operations_are_published_for_the_sdk, test_the_read_parameters, test_starting_a_link_check_takes_no_body, test_the_examples_are_valid_bodies, test_the_states_are_one_published_enum, test_the_v0_operations_are_deprecated_and_left_out_of_the_sdk, test_only_superseded_or_migrated_addresses_are_deprecated)
0028 applied except router registration CourseLinkCheckViewSet and CoursePreviousRunLinkUpdateViewSet (StandardizedErrorMixin, viewsets.ViewSet) with create and retrieve over v1/course_optimizer_service.py. They are plain ViewSets because a run is a queued task read back from its recorded status, not a row. Deviation: mounted with path("courses/<course_key:course_key>/.../", ...as_view({...})), because a router's regex routes cannot use the shared path converters CourseOptimizerRoutingTest.test_the_addresses; CourseOptimizerQueryCount{LegacyPermissions,Authz}Test
0029 partial StandardizedErrorMixin first. 401 authn, 403 authz, 404 not-found (unknown, version-only or library course key, or the feature off for link updates, each with a fixed detail), 400 validation for the body and view. A failed link check reports a fixed error message, and a link that could not be updated a fixed error_message. The text the tasks record can be an exception's, so it is never returned. Gaps pinned by tests that flip when fixed: (1) malformed JSON (400), 405, 406 and 415 are typed internal, because edx-drf-extensions 10.9.0 does not catalogue them; (2) the errors of each link under data are rendered as Python repr strings (e.g. "{'url': [ErrorDetail(string='This field is required.', code='required')]}"), because the library does not normalize nested errors; (3) a malformed course key gets the CMS HTML 404 at routing CourseOptimizerReadTest (test_a_failed_link_check_reports_a_fixed_message_not_the_recorded_text, test_the_full_view_reports_each_links_outcome_with_a_fixed_failure_message); CourseOptimizerStartTest (test_each_link_needs_a_url_a_type_and_an_id, test_the_links_must_be_a_list); CourseOptimizerRoutingTest (test_a_malformed_json_body_is_a_400_typed_internal, test_an_unsupported_media_type_is_a_415_typed_internal, test_other_methods_are_not_allowed, test_unsupported_accept_is_refused_with_406, test_an_unparsable_or_slash_form_key_is_an_html_404)
0030 met for this code; two inherited exceptions GET and HEAD read the recorded run status and, with view=full, the run's artifact and the course outline. On every read, with policy permissions off and on, nothing is written to SQL or Mongo, no modulestore item is created or updated, and no Django signal or Open edX event is sent, including on commit. Exceptions, both outside this code and both present on v0: (a) the first read on a database without openedx-authz's PolicyCacheControl row inserts it (one INSERT, its pre_save/post_save), allow-listed by name; (b) the provider's report builder fetches each reported component with get_xblock, which creates a course_info block that is not found. A reported course_info block can only be missing if it was deleted after the check ran, so the path is currently unreachable, and no test exercises it CourseOptimizerReadSideEffects{LegacyPermissions,Authz}Test (test_reads_have_no_side_effects, test_the_first_v0_read_writes_what_the_first_v1_read_writes, test_the_creation_recorder_sees_an_item_created)
0031 met Two RPC-style families, each one resource: link_check + link_check_status → link_check/, and rerun_link_update + rerun_link_update_status → previous_run_link_update/, with POST to start and GET for the latest run. Not merged with each other (different resources, gates and states). The link update's modes stay an enumerated action (all, single) with single requiring data, documented with an example each. Permissions are not flattened: per action, starting a check keeps READ and the rest keep WRITE. No deprecated aliases are written: the legacy routes stay on their own frozen views CourseOptimizerPermissionWiringTest.test_each_action_checks_the_course_then_its_own_permission; CourseOptimizerStartTest (test_every_link_is_queued_as_v0_queues_it, test_the_named_links_are_queued_as_v0_queues_them, test_single_needs_links); CourseOptimizerSchemaTest.test_the_examples_are_valid_bodies
0032 excluded Each read returns one status object; the report is opt-in and bounded by the links the run flagged —
0033 excluded No list. view is enum-validated by a serializer. v0 had no query parameters CourseOptimizerReadTest.test_an_unknown_view_is_refused
0034 met with declared deviation (JwtAuthentication, SessionAuthenticationAllowInactiveUser), commented at the declaration: kept from the legacy endpoints, so an inactive account's browser session stays authenticated and the course permission decides. Bearer is dropped. This matches #39303, #39100, #39157 and #39227; #39229/#39274 use the platform default on /api/authoring/v2/, so the authoring mounts are not yet uniform CourseOptimizerAuthenticationTest (test_a_jwt_caller_may_start_and_read, test_a_session_caller_may_start_and_read, test_an_inactive_session_is_authenticated, test_a_bearer_token_is_refused); hygiene H4
0035 excluded Not configuration; per-course, per-user —
0036 met view=minimal (default): the run's state only, projected with edx_rest_framework_extensions.shaping.project; the report builder and the results artifact are not touched. view=full: the four-level report, or the per-link results. Both shapes are published (output and results optional) CourseOptimizerReadTest (test_the_default_view_does_not_build_the_report, test_the_default_view_of_a_link_update_does_not_read_its_results, test_the_full_view_reports_the_flagged_links_in_the_course, test_the_default_view_of_a_link_update_is_its_state); CourseOptimizerSchemaTest.test_the_minimal_and_full_members
0037 partial New modules in v1/. v0 frozen apart from docstring-only markers: the syntax tree with docstrings stripped equals the base. deprecated: true through four exact SUPERSEDED_PATHS entries; no runtime header (frozen routes). DEPR: an entry comment on #39128, to be folded into the issue body versions gate; CmsMarkSupersededPathsTest.test_the_course_optimizer_operations_are_marked_and_their_successors_are_not; CourseOptimizerSchemaTest.test_the_v0_operations_are_deprecated_and_left_out_of_the_sdk
0038 met authoring API name, courses/{course_key}/link_check/ and courses/{course_key}/previous_run_link_update/ (one-per-course children; rerun_link_update would have read as a verb), course_key converter, trailing slash, course_link_check and course_previous_run_link_update CourseOptimizerRoutingTest (test_the_addresses, test_the_v0_addresses_are_unchanged, test_v0_still_takes_a_slash_separated_key_that_v1_does_not, test_the_url_names_are_unique_in_the_namespace, test_the_address_without_its_trailing_slash_redirects_to_it); url_snapshot rules check on a hand-built snapshot (the urls gate itself was skipped)
OEP-66 mechanism excluded, layering met (1) Endpoint access: the permission classes. (2) Record visibility: the run lookup applies django-user-tasks' USER_TASKS_STATUS_FILTERS (StatusFilterBackend: own runs, everyone's for a superuser), through the provider exactly as v0. (3) User selection: view only selects a representation CourseOptimizerVisibility{LegacyPermissions,Authz}Test (test_each_caller_sees_only_their_own_link_checks, test_each_caller_sees_only_their_own_link_updates, test_a_run_of_another_course_is_not_seen, test_global_staff_who_is_not_a_superuser_sees_only_their_own_runs)
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

  • v0 is unchanged. v0/views/course_optimizer.py gains docstring text only; its syntax tree with docstrings stripped equals the base. Its routes, views and URL names are untouched, and v0's own tests are not edited.

  • Same tasks, same arguments. Each accepted start queues the task v0 queues, with the arguments v0 passes. The tests assert the two calls are equal for the same caller and body. The one exception: when action is all, v0 hands any data it was sent to the task, which ignores it, while v1 hands [].

  • Parity. v1 is compared with v0 through the Django test client, with policy permissions off and on, after translating v0's member names to v1's. A successful link check's view=full report is v0's report member for member. The only declared body differences, verbatim from the test module:

    ENVELOPE_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"),
        ("errors", "a refused body maps each refused member to its messages"),
        ("developer_message", "v0 error bodies carry developer_message; the envelope does not"),
        ("error_code", "the v0 not-found body carries error_code; the envelope does not"),
        ("error", "v0 refuses a link update body with {'error': ...}; the envelope replaces it"),
    ]
    # Members of a successful body, in v1's names, that differ from v0's, each with
    # the test asserting exactly where it occurs.
    BODY_DIFFERENCES = [
        ("created_at", "always present, null without a run; v0 omits it", "test_reads_without_a_run"),
        ("error", "always present: null, or a fixed message where v0 returns the recorded text",
         "test_a_failed_link_check"),
        ("output", "with view=full, null for a run that did not succeed; v0 omits it", "test_reads_without_a_run"),
        ("status", "no link update yet is Uninitiated; v0 spells it uninitiated", "test_reads_without_a_run"),
        ("results", "only with view=full; v0 always returns it", "test_reads_without_a_run"),
        ("results[1].error_message", "a fixed message where v0 returns the recorded text",
         "test_a_link_update_with_a_failed_link"),
        ("results[0].error_message", "null for a link that was changed; v0 omits it",
         "test_a_link_update_with_a_failed_link"),
    ]

    Each successful case asserts the exact set of changed members, so a declared path cannot hide a difference elsewhere in the report. test_every_declared_difference_occurs fails if a declared difference never occurs.

  • Status and request differences from v0. Each is pinned against v0's own answer. Verbatim from the test module:

    RESPONSE_DIFFERENCES = [
        ("start a link check or a link update", "200", "202", "test_starting_a_run_is_accepted"),
        ("read a link check of an unknown course, as global staff", "500", "404", "test_reads_of_an_unknown_course"),
        ("read a link update of an unknown course, as global staff", "200, uninitiated", "404",
         "test_reads_of_an_unknown_course"),
        ("read either of an unknown course, without a role", "403", "404", "test_reads_of_an_unknown_course"),
        ("start or read a link update where the feature is off", "400", "404", "test_the_feature_off"),
        ("start a link update naming a link with a missing or blank url, type or id", "200, queued", "400",
         "test_a_link_missing_a_member"),
        ("start a link update whose body is not a JSON object", "500", "400", "test_a_body_that_is_not_an_object"),
        ("read with a view other than minimal or full", "200, view ignored", "400", "test_an_unknown_view"),
        ("read a link check after a status was stored in the session", "the stored status", "Uninitiated",
         "test_a_status_stored_in_the_session"),
        ("read a link update with no run", "uninitiated, results []", "Uninitiated", "test_no_link_update_yet"),
        ("start or read with a Bearer token", "200", "401", "test_a_bearer_token"),
        ("slash-separated course key", "404 JSON", "404 HTML", "test_a_slash_separated_course_key"),
    ]

    The full table, with what else changed:

    Request v0 v1 Test
    start a link check or link update 200 202 CourseOptimizerResponseDifferencesTest.test_starting_a_run_is_accepted
    read a link check of an unknown course, global staff 500 404 …test_reads_of_an_unknown_course
    read a link update of an unknown course, global staff 200, uninitiated 404 …test_reads_of_an_unknown_course
    read either of an unknown course, no role 403 404 …test_reads_of_an_unknown_course
    link update with the feature off 400 {"error"} 404, fixed detail …test_the_feature_off; CourseOptimizerFeatureOff{LegacyPermissions,Authz}Test
    link with a missing or blank url, type or id (single) 200, queued 400 …test_a_link_missing_a_member
    body that is not a JSON object 500 400 …test_a_body_that_is_not_an_object
    view other than minimal/full 200, ignored 400 …test_an_unknown_view
    status stored in the browser session (nothing in the platform stores one) returned Uninitiated …test_a_status_stored_in_the_session
    no link update yet uninitiated, results: [] Uninitiated …test_no_link_update_yet
    Bearer token 200 401 …test_a_bearer_token
    slash-separated course key 404 JSON HTML 404 (no route) …test_a_slash_separated_course_key
    address without the trailing slash served 301 to the slashed address (GET) CourseOptimizerRoutingTest.test_the_address_without_its_trailing_slash_redirects_to_it
    member names LinkCheckStatus, displayName, … status, display_name, … CourseOptimizerReadTest.test_the_full_report_is_v0s_report_in_snake_case
    report and results always only with view=full _ParityMixin.test_reads_without_a_run, CourseOptimizerReadTest.test_the_default_view_does_not_build_the_report
    recorded failure text returned fixed message _ParityMixin.test_a_failed_link_check, _ParityMixin.test_a_link_update_with_a_failed_link
    data sent with action: all queued, unread queued, not validated or passed on CourseOptimizerStartTest.test_links_sent_with_action_all_are_not_read

    The 401 → 404 → 403 order matches v0 wherever v0 looked the course up. For link updates, a caller without permission is still refused with 403 when the feature is off, as on v0.

  • Known inherited gap (both versions). A flagged link in a component nested below a unit's child, such as one inside a library_content or split_test block, gives the provider's report a fifth level that the four-level shape cannot hold. The full read then answers 500, on v1 and on v0 alike; the default view is unaffected. The provider is not changed here. Pinned with a library_content block by CourseOptimizerReadTest.test_a_flagged_link_below_a_unit_child_fails_the_full_read_on_both_versions; split_test children take the same path in the provider and are not separately tested.

  • Provider reuse. Run visibility comes from the provider's _latest_task_status and _latest_course_link_update_task_status, imported by their private names. They apply USER_TASKS_STATUS_FILTERS exactly as v0 does, and the provider is a frozen legacy module, so this PR does not rename them. The report comes from its public generate_broken_links_descriptor and sort_course_sections, and view=full results from get_course_link_update_data.

  • Query counts, measured against v0 in the same test (CourseOptimizerQueryCount{LegacyPermissions,Authz}Test.test_reads_cost_no_more_than_v0):

    • the default view of each read runs no more SQL queries than v0, and for a link check fewer Mongo reads, since v0 always builds the report;
    • view=full runs at most one more SQL query than v0: the course lookup, which v0's reads skip.

    No literal baseline was measured, because no platform environment was available.

  • Schema. Four new operations and deprecated: true on the four v0 operations. The run states are one enum component, StatusEnum, shared by both reads and both starts, so no new enum-naming warning is emitted. Risk: drf-spectacular names it from the field name, so if another CMS serializer later publishes a status enum with different values, both get hash-suffixed names and the generator warns. Pinning the name needs an ENUM_NAME_OVERRIDES settings change, which this PR does not make (test_the_states_are_one_published_enum will flag it). 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/v0/views/course_optimizer.py docstring-only deprecation marker syntax tree with docstrings stripped equals the base; versions gate ALLOWED
cms/djangoapps/contentstore/rest_api/v1/authoring_urls.py strictly opt-in addition: one import, two routes no line removed; CourseOptimizerRoutingTest
cms/djangoapps/contentstore/views/permissions.py (shared) two classes added; the openedx_authz.constants.permissions import widened by one name; existing classes unchanged CourseOptimizerPermissionWiringTest and the access tests
cms/lib/spectacular.py, cms/lib/tests/test_spectacular.py four exact SUPERSEDED_PATHS entries; the exact-set test updated, plus a test that the four v0 operations are marked and their successors are not CmsMarkSupersededPathsTest

New files inside the pre-existing v1/: course_optimizer_service.py, serializers/course_optimizer.py, views/course_optimizer.py, views/tests/test_course_optimizer.py. They import nothing from rest_api/v0/; the tests use the shared v1/views/tests/helpers.py added by #39303.

Error-format decision

Versioned. v0 keeps its developer_message / {"error": ...} bodies, its 400 for the feature flag, its 500s and the recorded failure text unchanged. v1 uses the envelope. The Course Optimizer page calls v0 today (frontend-app-authoring src/optimizer-page/data/api.ts), so the format change rides the new version along with the member-name and status-code changes.

Deprecations

The four v0 operations are superseded as in the Summary table. They are deprecated: true in the CMS schema, and the docstrings name the successors. The views are otherwise unchanged: no runtime header. Tracked on the FC-0118 umbrella DEPR ticket, #39128; this PR's entry is posted there with the PR. Window per #39128: removed one named release after the release this ships in.

Migration notes:

  • Call /api/authoring/v1/courses/{course_key}/link_check/ and .../previous_run_link_update/, with the trailing slash: POST to start a run, GET for the latest one.
  • Starting a run answers 202 (was 200) with {"status": "Pending"}.
  • Member names are snake_case: LinkCheckStatus → status, LinkCheckCreatedAt → created_at, LinkCheckError → error, LinkCheckOutput → output, and in the report displayName → display_name, brokenLinks → broken_links, lockedLinks → locked_links, externalForbiddenLinks → external_forbidden_links, previousRunLinks → previous_run_links (original_link, is_updated, updated_link).
  • The link check report and the link update results need view=full. Without it, the reads return the run's state only.
  • "No run" is Uninitiated on both resources (the link update read spelled it uninitiated). error and error_message carry fixed messages, not the recorded failure text.
  • Where updating links to a previous run is not enabled, the link-update operations answer 404 (was 400).
  • With action: all, data is not read and may be omitted. With action: single, every link needs a non-blank url, type and id.
  • Errors use the standard envelope. Bearer tokens are refused; use JWT or a session.

Authorization

Legacy check Where it lives in v1 Deny-path tests
@view_auth_classes() (IsAuthenticated) on all four views 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() on the two v0 POSTs; none on the two GETs IsExistingCourse on all four (also 404 for version-only and library keys) test_unknown_course_is_404_before_permission_is_checked, test_a_version_only_course_key_is_404, test_a_library_is_not_a_course; v0 did not look the course up on its two reads (see the unknown-course rows of the difference table)
link check POST: user_has_course_permission(edit_course_content, READ) inline CanStartLinkCheck, same call test_starting_a_check_is_refused_without_read_access; no role, other-course staff/instructor, limited staff (legacy); auditor, limited staff, data researcher, beta tester, a role on another course, legacy staff without a policy role (policy permissions on)
link check GET, rerun link update POST and GET: user_has_course_permission(edit_course_content, WRITE) inline CanEditCourseContent, same call test_starting_a_check_needs_only_read_access_and_the_rest_write_access (Studio read access without write → only the start is allowed, on v0 and v1); the same deny callers as above
enable_course_optimizer_check_prev_run_links(course_key) after the permission check, 400 on v0 checked first in each link-update action, after the permission classes; 404 test_link_updates_are_404_for_a_caller_who_may_use_them, test_a_caller_who_may_not_use_them_is_still_refused_with_403, test_the_feature_is_checked_before_the_body, test_instructors_and_global_staff_get_404_too
run visibility: USER_TASKS_STATUS_FILTERS (StatusFilterBackend) in the provider unchanged, same provider functions test_each_caller_sees_only_their_own_link_checks, test_each_caller_sees_only_their_own_link_updates, test_a_run_of_another_course_is_not_seen, test_global_staff_who_is_not_a_superuser_sees_only_their_own_runs

Order: 401, then 404 for a missing course, then 403, then 404 for the feature off, then 400 for the body. Allowed callers:

  • course staff and instructor, org staff and instructor, and global staff;
  • with policy permissions on, the course editor, staff and admin roles (the roles granting edit-course-content);
  • superusers, admitted on v0 and v1 alike, with policy permissions off too: the course role lookup consults openedx-authz, which admits them (test_a_superuser_who_is_not_global_staff_is_allowed_as_on_v0). A superuser also sees everyone's runs, as on v0.

Each access case asserts v0's status for the same caller. Every refused start asserts that no task was queued.

Gate report

Run on the host against 9b4d60ea0d (the head of #39303, on which this branch is stacked). 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 (v0/views/course_optimizer.py docstrings, v1/authoring_urls.py), 4 ADDED, nothing protected touched (23 shared legacy modules protected)
hygiene PASS (0 FAIL, 0 WARN), 9 files
ruff clean; no line over 120 characters
supplementary, not a gate: URL rules (url_snapshot.py check) on a hand-built snapshot of the new routes PASS (0 FAIL, 0 WARN); the real urls gate above is SKIPPED
schema SKIPPED (no platform environment)
urls SKIPPED (no platform environment)
old-tests SKIPPED (no platform environment)
tests SKIPPED (no platform environment)

Follow-ups

  • SDK: the four operations carry openedx-platform-sdk; the next openedx-platform-sdk regeneration gains them.
  • frontend-app-authoring: src/optimizer-page/data/api.ts can move to the v1 addresses. It needs: view=full for the report and the results; the new member names (the page camel-cases responses with camelCaseObject, so it reads status, createdAt and output where it read linkCheckStatus, linkCheckCreatedAt and linkCheckOutput); 202 on start; 404 for the feature off.
  • edx-drf-extensions: catalogue ParseError, MethodNotAllowed, NotAcceptable and UnsupportedMediaType, and normalize nested validation errors (per-item objects instead of Python repr strings; detail without the repr of the error dict). Issue to be filed; the pinned tests flip when it lands.
  • 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.
  • Course optimizer report shape: links inside components nested below a unit's child (library_content, split_test) break the four-level report on v0 and v1. A future version could report such components at the unit level. Recorded as a future-version candidate; the provider is not changed here.
  • Shared existence check. feat: serve the course outline from authoring v2 #39229 (open) adds the same COURSE_NOT_FOUND_MESSAGE and an inline course-existence check in CanViewCourse. Whichever of feat: serve the course outline from authoring v2 #39229 and feat: serve course advanced settings from authoring v1 #39303 lands second should build on IsExistingCourse, which this PR reuses.
  • Authentication across /api/authoring/: v1 here, feat: serve course advanced settings from authoring v1 #39303 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. One policy for new versions of such endpoints should be agreed.
  • Naming. link_check/ and previous_run_link_update/ are singular, one-per-course children of courses/{course_key}/. They follow ADR 0038's own courses/{course_key}/details/ singleton precedent: each course has one current check and one current update. rerun_link_update was renamed because "rerun" reads as a verb and the resource is the update of links to the previous run.

Taimoor Ahmed added 9 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.
The v0 course optimizer views check courses.edit_course_content inline,
falling back to legacy read access to start a link check and to legacy
write access for everything else. The conforming views need these as
permission classes: CanStartLinkCheck makes the read-fallback call and
CanEditCourseContent the write-fallback call, so the two stay separate
when the endpoints are merged. Existing classes are unchanged.
The v0 course optimizer spreads two resources over four verb-style
addresses (link_check, link_check_status, rerun_link_update,
rerun_link_update_status), answers starts with 200, returns PascalCase
and camelCase reports, passes recorded failure text to clients, answers
a disabled feature with 400 and accepts Bearer tokens.

/api/authoring/v1/courses/{course_key}/link_check/ and
.../previous_run_link_update/ each take POST to start a run (202 with
its status) and GET to read the latest run the caller may see. The
default read is the status; view=full adds the report or the results.
Keys are snake_case, failures carry fixed messages, errors use the
standard envelope, a disabled feature answers 404, and links sent with
action=all are not read, as on v0. Starting a link check keeps v0's
read-level check and every other operation its write-level check, and
per-user run visibility is reused unchanged. Parity tests compare each
read and each queued task with v0.
The four v0 course optimizer operations now have conforming successors
under /api/authoring/v1/courses/{course_key}/. 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.
@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