Repository navigation
feat: read and partially update course grading through authoring v3 - #39227
taimoor-ahmed-1 wants to merge 2 commits into
Conversation
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>
|
Thanks for the pull request, @taimoor-ahmed-1! This repository is currently maintained by 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 approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf 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 PRYour 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
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 ADRsdocs/decisions/0025to0038and 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:
Change table
GET /api/authoring/v3/courses/{course_key}/grading/, and the already-deprecated/api/contentstore/v3/authoring_grading/{course_key}/?view=fullalso returns v1'scourse_assignment_lists,default_grade_designations,is_credit_course,show_credit_eligibilityandmfe_proctored_exam_settings_url, under their v1 names.PATCH, both v3 addressesGET/POST /api/contentstore/v1/course_grading/{course_id}, v0POST /api/contentstore/v0/grading/{course_id}deprecated: truein the schema, plus a docstring naming the successor.ADRs applied
help_texton every field,*.v3ref names, nullability and required fields matching real responsesCanViewGradingSettings,CanEditGradingSettings) replace the inline check; legacy fallback and 404-before-403 keptErrorResponse; GET carries theopenedx-platform-sdktagretrieveon the existing ViewSet; logic ingrading_service.py; query counts pinned against v1internalbecause 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.?view=fulland 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.viewparameter is serializer-validated and documented(JwtAuthentication, SessionAuthenticationAllowInactiveUser)with its reason commented at the declaration (narrowing it would refuse callers PATCH already admits). Bearer is refused, as before on v3.?view=fullopt-in, both shapes publishedCompliance matrix
serializers/grading.py: responseCourseGrading.v3,CourseGradingFull.v3,Grader.v3,GracePeriod.v3; requestCourseGradingUpdate.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 intest_whole_policy_bodies_behave_as_the_v0_update;cms/lib/tests/test_spectacular.pyschema pinsv3/permissions.py, chosen per action inget_permissions()GradingAccessLegacyPermissionsTest,GradingAccessAuthzTest: every cell also asserts v1 makes the same decision@extend_schemaon every action; JSON-only request body; GET 200 =anyOfof the default and full shapestest_spectacular.py(OverlappingShapesTest, body type, nullable/required pins); schema gateretrieve; thin views; service moduleGradingQueryCountTest,GradingQueryCountAuthzTest,test_default_read_costs_no_more_than_the_v1_read,test_full_read_costs_the_same_as_the_v1_readStandardizedErrorMixin; validation before any writetest_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 bytest_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_routeGradingReadSideEffectsLegacyPermissionsTest,GradingReadSideEffectsAuthzTest,test_the_first_v1_read_writes_the_same_rowtest_inactive_user_with_a_session_may_read_and_change,test_jwt_caller_may_read,test_bearer_token_is_refused?view=fulltest_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_viewtest_whole_policy_bodies_behave_as_the_v0_update; DEPR entryGradingRoutingTest(test_conforming_and_legacy_routes_share_view_and_actions,test_legacy_format_suffix_reads_and_updates_like_the_plain_address); urls gateIn-place edits
v3/views/authoring_grading.pyview, partial bodies that returned 500); 1 like-for-like (inline check to permission classes, v0 to v3 serializers)v3/authoring_urls.py{"patch": "partial_update"}becomes{"get": "retrieve", "patch": "partial_update"}and nothing elsev3/views/tests/test_authoring_grading.pyv3/grading_service.py,v3/permissions.py,v3/serializers/__init__.py,v3/serializers/grading.pyhelp_textand are frozen; reusing their names with new classes would collide in the schema.v1/views/grading.py,v0/views/authoring_grading.pycms/lib/spectacular.py,cms/lib/tests/test_spectacular.pySUPERSEDED_PATHSgains v1 and v0; the existing post-processing hook publishes the grading read asanyOf(its two shapes overlap, sooneOfwould reject full responses). No new hook, no settings change.Test traceability (master
test_authoring_grading.pyto this PR)test_unauthenticated_patch_returns_401(x2),test_unauthenticated_patch_returns_standardized_401,test_unauthenticated_401_type_uritest_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_uritest_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_refusedtest_nonexistent_course_returns_standardized_404,test_not_found_type_uritest_unknown_course_is_404_before_permission_is_checkedtest_patch_with_minimum_grade_credit_fires_credit_task,test_patch_without_minimum_grade_credit_skips_credit_tasktest_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_gradingtest_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_allowedtest_other_methods_are_not_allowed(POST, PUT, DELETE; envelope)test_error_body_has_no_developer_message,test_instance_field_is_request_pathassert_error_envelopeon every refusaltest_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_envelopeGradingRoutingTesttest_invalid_course_key_is_404_on_conforming_routetest_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
.jsonsuffix 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: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_cutoffsandgraders[].typenullable. Master already returns null in exactly those places: the replay records 8 master 200 responses carryinggrade_cutoffs: null, a null cutoff value and a null gradertype. 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):
gradersno longer nullable (null was a 500);type(the model defaults the rest);grade_cutoffsgainsminProperties: 1for partial bodies;grace_periodgets its own request component;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 v0POST /api/contentstore/v0/grading/{course_id}aredeprecated: truein 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
IsAuthenticated(v1, v0view_auth_classes; v3 PATCH)IsAuthenticated, firstverify_course_exists(v1, v0);resolve_course_key(v3)resolve_course_keyinside the action's permission class, before the role check: 404 before 403authz_permission_required(COURSES_VIEW_GRADING_SETTINGS, READ)(v1 GET)CanViewGradingSettingsauthz_permission_required(COURSES_EDIT_GRADING_SETTINGS, READ)(v1 POST, v0 POST); inline check (v3 PATCH)CanEditGradingSettingshas_studio_read_accessfor both read and editNobody 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.The schema gate cannot tell a schema fix from a contract change: it reports any response field gaining
nullableas 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=full14 / 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:
SDK
The GET is tagged
openedx-platform-sdk. Regenerating the SDK after merge brings:/api/contentstore/v3/authoring_grading/...address, deprecated from the start;authoring_grading_*_v0to*_v3;type(it required five fields, more than the server ever did);grade_cutoffsand gradertypetyped nullable in responses;Follow-ups
?view=fullinstead of v1 GET, and write to the conforming v3 address.ParseError,MethodNotAllowed,NotAcceptableandUnsupportedMediaType;flatten_detailputs a Python repr intodetailfor every field-keyed 400.🤖 Generated with Claude Code