Skip to content

feat: read and partially update course grading through authoring v3 - #39227

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

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

Conversation

@taimoor-ahmed-1

@taimoor-ahmed-1 taimoor-ahmed-1 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR standardizes the Course Grading functional area (#39065, umbrella #38137) on the existing authoring v3 grading resource, /api/authoring/v3/courses/{course_key}/grading/, following the API standardization ADRs docs/decisions/0025 to 0038 and OEP-69.

Why extend v3 instead of adding a version. Grading is versioned per resource, and v3 is already its conforming version (#39078 mounted it at the address above). But v3 could only write, so every caller still read through v1. This PR:

  • adds a GET to v3. Every GET to v3 returned 405, the schema change only adds, and PATCH responses for bodies that already worked are unchanged.
  • makes PATCH a real partial update. Partial bodies all returned 500 before.
  • deprecates v1 GET/POST and v0 POST in favour of v3.

Change table

Endpoint Before After
GET /api/authoring/v3/courses/{course_key}/grading/, and the already-deprecated /api/contentstore/v3/authoring_grading/{course_key}/ 405 Default: the four-field policy, the same shape PATCH returns. ?view=full also returns v1's course_assignment_lists, default_grade_designations, is_credit_course, show_credit_eligibility and mfe_proctored_exam_settings_url, under their v1 names.
PATCH, both v3 addresses The body went to the model unvalidated. Any body without all four keys returned 500, and some malformed bodies wrote corrupt data first. Whole body (all four keys): unchanged, the same values accepted and stored, the same response, events and credit task. Partial body: changes only the fields sent; graders are never re-sent, so stored weights cannot drift. Every body that returned 500: 400 validation envelope, nothing written.
v1 GET/POST /api/contentstore/v1/course_grading/{course_id}, v0 POST /api/contentstore/v0/grading/{course_id} v1 not deprecated; v0 listed on #39128 but not marked in the schema Behaviour unchanged. deprecated: true in the schema, plus a docstring naming the successor.

ADRs applied

  • 0025 serializers - separate request and response serializers, help_text on every field, *.v3 ref names, nullability and required fields matching real responses
  • 0026 permissions - per-action permission classes (CanViewGradingSettings, CanEditGradingSettings) replace the inline check; legacy fallback and 404-before-403 kept
  • 0027 schema / docs - every action documented with an explicit JSON request body and examples of both read shapes; errors reference ErrorResponse; GET carries the openedx-platform-sdk tag
  • 0028 viewsets - retrieve on the existing ViewSet; logic in grading_service.py; query counts pinned against v1
  • 0029 errors - envelope on every 400/401/403/404 this view raises. Open, unchanged from master and pinned by tests to flip when fixed: (1) DRF's malformed-JSON 400, 405, 406 and 415 come back typed internal because edx-drf-extensions does not catalogue them; (2) a malformed course key gets the routing-level HTML 404 on the conforming address, and on the legacy address for keys with no separator. No catch-all exists for authoring v3 yet.
  • 0030 GET idempotence - GET, ?view=full and HEAD write no domain data and fire no signal (SQL and Mongo captured, credit and non-credit, flag off and on, course auditor on a fresh database). The only write is the openedx-authz policy-version cache row on a fresh database's first permission check, which v1 makes too.
  • 0031 merged endpoints - n/a: no action family; the three writes are versions of one operation, which 0037 governs
  • 0032 pagination - n/a: singleton resource, no list
  • 0033 filtering - n/a: no list; the one view parameter is serializer-validated and documented
  • 0034 authentication - the viewset keeps (JwtAuthentication, SessionAuthenticationAllowInactiveUser) with its reason commented at the declaration (narrowing it would refuse callers PATCH already admits). Bearer is refused, as before on v3.
  • 0035 MFE config - n/a: no Course Grading endpoint serves front-end configuration
  • 0036 nested JSON - thin default, ?view=full opt-in, both shapes published
  • 0037 versioning - in-place categories 1, 2 and 4 with evidence below; v1 and v0 deprecated in schema and docstrings; DEPR entry on [DEPR]: Contentstore and enrollment API versions superseded by FC-0118 #39128 (comment) with the window per [DEPR]: Contentstore and enrollment API versions superseded by FC-0118 #39128
  • 0038 URL structure - route map change only (GET added); both v3 addresses serve the same view and actions; no address or URL name changed
  • OEP-66 queryset scoping - n/a: not a list endpoint
  • OEP-69 conventions - no governance identifiers or history narration in code or schema text; no local copies of library primitives

Compliance matrix

Decision Status How Evidence
0025 met serializers/grading.py: response CourseGrading.v3, CourseGradingFull.v3, Grader.v3, GracePeriod.v3; request CourseGradingUpdate.v3, GraderUpdate.v3, GracePeriodUpdate.v3. A whole-policy body is validated by a private serializer that accepts exactly what the model accepted. GradingReadTest.test_reads_match_the_published_shapes; every whole-body response validated against the published schema in test_whole_policy_bodies_behave_as_the_v0_update; cms/lib/tests/test_spectacular.py schema pins
0026 met v3/permissions.py, chosen per action in get_permissions() GradingAccessLegacyPermissionsTest, GradingAccessAuthzTest: every cell also asserts v1 makes the same decision
0027 met @extend_schema on every action; JSON-only request body; GET 200 = anyOf of the default and full shapes test_spectacular.py (OverlappingShapesTest, body type, nullable/required pins); schema gate
0028 met ViewSet retrieve; thin views; service module GradingQueryCountTest, GradingQueryCountAuthzTest, test_default_read_costs_no_more_than_the_v1_read, test_full_read_costs_the_same_as_the_v1_read
0029 partial Envelope via StandardizedErrorMixin; validation before any write test_bodies_the_model_cannot_store_are_refused_before_anything_is_written, test_numbers_too_large_to_store_are_refused_before_anything_is_written, test_a_form_encoded_body_is_refused; open items pinned by test_malformed_json_is_refused_with_400, test_unsupported_body_type_is_refused_with_415, test_unsupported_accept_is_refused_with_406, test_other_methods_are_not_allowed, test_unparsable_or_slash_form_key_is_404_on_conforming_route, test_key_without_separators_is_404_on_legacy_route
0030 met No writes on read GradingReadSideEffectsLegacyPermissionsTest, GradingReadSideEffectsAuthzTest, test_the_first_v1_read_writes_the_same_row
0034 met (commented deviation) Auth tuple kept with reason test_inactive_user_with_a_session_may_read_and_change, test_jwt_caller_may_read, test_bearer_token_is_refused
0036 met Thin default, ?view=full test_full_view_equals_the_v1_read_flattened, test_full_view_credit_context_equals_v1, test_full_view_proctoring_link_equals_v1, test_full_view_without_a_grace_period_equals_v1, test_grace_period_seconds_are_shown_where_v1_drops_them, test_unknown_view_is_refused_with_400, test_empty_view_is_the_default_view
0037 met Categories 1, 2, 4 (below); DEPR entry on #39128 versions gate; base-vs-head replay; test_whole_policy_bodies_behave_as_the_v0_update; DEPR entry
0038 met Route map only GradingRoutingTest (test_conforming_and_legacy_routes_share_view_and_actions, test_legacy_format_suffix_reads_and_updates_like_the_plain_address); urls gate
0031, 0032, 0033, 0035, OEP-66 n/a Reasons in the checkbox list none

In-place edits

File Category (ADR 0037) What and why
v3/views/authoring_grading.py 2 opt-in addition (GET, view, partial bodies that returned 500); 1 like-for-like (inline check to permission classes, v0 to v3 serializers) Whole-body PATCH matches master: replayed below and pinned against frozen v0
v3/authoring_urls.py 2 {"patch": "partial_update"} becomes {"get": "retrieve", "patch": "partial_update"} and nothing else
v3/views/tests/test_authoring_grading.py current-version tests Mocks replaced by real courses and roles; traceability below
v3/grading_service.py, v3/permissions.py, v3/serializers/__init__.py, v3/serializers/grading.py added inside v3 Service, permission classes and v3-owned serializers. The v0 serializers lack help_text and are frozen; reusing their names with new classes would collide in the schema.
v1/views/grading.py, v0/views/authoring_grading.py 4 docstring-only deprecation markers AST with docstrings stripped is identical to master; v1 and v0 tests untouched
cms/lib/spectacular.py, cms/lib/tests/test_spectacular.py schema hook (not a version directory) SUPERSEDED_PATHS gains v1 and v0; the existing post-processing hook publishes the grading read as anyOf (its two shapes overlap, so oneOf would reject full responses). No new hook, no settings change.

Test traceability (master test_authoring_grading.py to this PR)

Master test Now covered by
test_unauthenticated_patch_returns_401 (x2), test_unauthenticated_patch_returns_standardized_401, test_unauthenticated_401_type_uri test_anonymous_caller_is_refused_with_401 (both permission classes, both mounts, GET and PATCH, envelope slug)
test_authenticated_without_grading_permission_returns_403, test_non_author_patch_returns_standardized_403, test_non_author_403_type_uri test_user_without_a_role_is_refused_with_403, test_limited_staff_is_refused, test_staff_of_another_course_is_refused, test_data_researcher_is_refused, test_legacy_course_staff_without_a_policy_role_is_refused
test_nonexistent_course_returns_standardized_404, test_not_found_type_uri test_unknown_course_is_404_before_permission_is_checked
test_patch_with_minimum_grade_credit_fires_credit_task, test_patch_without_minimum_grade_credit_skips_credit_task test_partial_bodies_change_only_the_fields_sent (task queued if and only if the key is sent); test_whole_policy_bodies_behave_as_the_v0_update (same task as v0)
test_patch_on_conforming_route_updates_grading test_partial_bodies_change_only_the_fields_sent, test_whole_policy_bodies_behave_as_the_v0_update (real course instead of a mocked update)
test_post_not_allowed test_other_methods_are_not_allowed (POST, PUT, DELETE; envelope)
test_error_body_has_no_developer_message, test_instance_field_is_request_path assert_error_envelope on every refusal
test_detail_url_resolves, test_conforming_url_reverses_to_expected_path, test_conforming_and_legacy_routes_share_view_and_actions, test_version_only_course_key_is_404_on_conforming_route, test_v0_endpoint_unaffected_by_v3_envelope kept under the same names in GradingRoutingTest
test_invalid_course_key_is_404_on_conforming_route test_unparsable_or_slash_form_key_is_404_on_conforming_route (now also pins the HTML content type)

Backward compatibility

  • Versions gate: PASS. v1 and v0 differ from master only in docstrings. Protected: the grading model, the legacy Studio handler, utils.py, cms/urls.py, the authz app, view_utils.py, views/permissions.py.

  • URLs gate: PASS. No address or URL name changed. The gate does not see HTTP methods, so the added GET was probed by request: the legacy .json suffix address now answers GET and PATCH like the plain address, where master answered 405 and 500.

  • v1, v0 and course-settings tests untouched and green.

  • PATCH replay against master. 520 distinct bodies were each sent to master and to this branch, on both v3 addresses and both sides of MATERIAL_RECOMPUTE_ONLY:

    • 215 bodies master accepted gave identical response bytes, stored policy, signals, events and credit task;
    • 294 bodies that made master return 500 now return a 400 validation envelope with nothing written;
    • 11 partial bodies that made master return 500 now succeed and change only the fields sent;
    • nothing else.

    227 of those whole-policy shapes are committed as a parity test against frozen v0, which makes the same model call master's v3 made.

  • Schema gate: FAIL, 4 BREAKING, each a correction. The PATCH 200 schema on both addresses now marks grade_cutoffs and graders[].type nullable. Master already returns null in exactly those places: the replay records 8 master 200 responses carrying grade_cutoffs: null, a null cutoff value and a null grader type. The wire is unchanged; master's schema was wrong, and a client generated from it fails on those courses. Otherwise: 0 other breaking, 5 INFO (2 GETs added, 3 operations newly deprecated), 2 WARN (the viewset's existing authenticator warnings, moved lines).

  • Request schema changes (PATCH, both addresses):

    • form and multipart media types removed (a form could never send a valid body);
    • graders no longer nullable (null was a 500);
    • each grader requires only type (the model defaults the rest);
    • grade_cutoffs gains minProperties: 1 for partial bodies;
    • grace_period gets its own request component;
    • description text added.

Error-format decision

Versioned path: the new GET and the validated PATCH use v3's existing error envelope; v1 and v0 keep their legacy bodies.

Deprecations

v1 GET/POST /api/contentstore/v1/course_grading/{course_id} and v0 POST /api/contentstore/v0/grading/{course_id} are deprecated: true in the CMS schema, and their docstrings name the v3 successor. No runtime header (frozen routes). The DEPR record is an entry on #39128, whose body says further areas are appended there; the entry is this comment. Window, per #39128: the marking ships in the next named release after Verawood, and v1 and v0 stay mounted until one named release after that.

Authorization

Legacy check v3
IsAuthenticated (v1, v0 view_auth_classes; v3 PATCH) IsAuthenticated, first
verify_course_exists (v1, v0); resolve_course_key (v3) resolve_course_key inside the action's permission class, before the role check: 404 before 403
authz_permission_required(COURSES_VIEW_GRADING_SETTINGS, READ) (v1 GET) CanViewGradingSettings
authz_permission_required(COURSES_EDIT_GRADING_SETTINGS, READ) (v1 POST, v0 POST); inline check (v3 PATCH) CanEditGradingSettings
Legacy fallback has_studio_read_access for both read and edit Kept for both, as on every grading write path today

Nobody gains read access. Bearer-token callers (v1 only) and slash-form course keys on the conforming address are refused, as on v3 before this PR.

Gate report

run_gates.sh --base origin/master --service cms --prefix /api/authoring/v3/, run in a container with the branch's pinned requirements (Python 3.12, edx-drf-extensions 10.9.0). No gate was skipped.

versions   PASS  v1/v0 docstring-only; protected legacy files untouched
hygiene    PASS  0 FAIL, 0 WARN
schema     FAIL  4 BREAKING = "response field became nullable" on PATCH 200 grade_cutoffs and
                 graders[].type, both addresses: corrections, see Backward compatibility.
                 2 WARN = the viewset's existing authenticator warnings (line moved). 5 INFO.
urls       PASS  0 FAIL, 0 WARN
old-tests  PASS  107 passed (v1, v0 grading; course settings), unmodified
tests      PASS  292 passed (v3 grading, CMS schema hooks), -Wd

The schema gate cannot tell a schema fix from a contract change: it reports any response field gaining nullable as breaking. The four lines are fixes; the replay above shows master already sends null there.

The two whole-policy replay tests are the slowest in the module (about 2.5 and 1.8 minutes).

Query counts (warm request, flag off; SQL / Mongo reads / Mongo writes): v1 GET 14 / 4 / 0; v3 GET 12 / 4 / 0; v3 GET ?view=full 14 / 4 / 0; PATCH 33 / 42 / 6 on both addresses, the same as master.

Mutation checks. Each guard was broken on purpose and the suite went red, with the files restored byte-for-byte afterwards:

  • implementation, 22 mutations;
  • validation, 18;
  • schema, 15;
  • tests and hook, 13.

SDK

The GET is tagged openedx-platform-sdk. Regenerating the SDK after merge brings:

  • the conforming GET and partial PATCH;
  • a GET on the deprecated /api/contentstore/v3/authoring_grading/... address, deprecated from the start;
  • model renames authoring_grading_*_v0 to *_v3;
  • a grader request model that requires only type (it required five fields, more than the server ever did);
  • grade_cutoffs and grader type typed nullable in responses;
  • separate default and full read models.

Follow-ups

  • frontend-app-authoring: read v3 ?view=full instead of v1 GET, and write to the conforming v3 address.
  • Grader weights are truncated on output in v0, v1 and v3: 0.29 is shown as 28, so an MFE save stores 0.28.
  • v0 and v1: an integer wider than 8 bytes in a grading body returns 500 and leaves the course unreadable until the process restarts. v3 now refuses these with 400.
  • edx-drf-extensions:
    • catalogue ParseError, MethodNotAllowed, NotAcceptable and UnsupportedMediaType;
    • flatten_detail puts a Python repr into detail for every field-keyed 400.
  • openedx-authz: the first permission check on a fresh database writes the policy-version row, including during reads.
  • Authoring v3: a JSON catch-all for unmatched keys (the HTML 404 above).

🤖 Generated with Claude Code

Taimoor Ahmed and others added 2 commits October 13, 2026 23:40
Authoring v3 could only write the grading policy, so every caller still
read it through v1. v3 now answers GET with the four-field policy, and
with ?view=full adds v1's five derived fields under their v1 names.

PATCH validates its body and applies partial updates. A body with all
four keys takes exactly the previous path. Other bodies change only the
fields sent and never re-send graders, so stored weights cannot drift.
Bodies that used to fail with 500, some after writing corrupt data, now
get 400 with nothing written. Permissions move into per-action classes
with the same checks, fallback and 404-before-403 order. v1 and v0
grading are marked deprecated in the CMS schema.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Docstring-only markers naming the v3 successor and DEPR openedx#39128. No
statement, decorator or import changes; with docstrings stripped the
modules match the base.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 13, 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.

3 participants