feat: standardize the Grades API as v2 at /api/grade/v2/ - #39172
taimoor-ahmed-1 wants to merge 14 commits into
Conversation
…ons (ADR 0038) Register the CourseKeyConverter / UsageKeyConverter path converters — added to edx-drf-extensions 10.8.0 (openedx/edx-drf-extensions#573) per ADR 0038's 'Code examples' section — once per service in lms/urls.py and cms/urls.py, as <course_key:...> / <usage_key:...>. ADR 0038 rule 9: conforming routes resolve opaque keys in the URLconf, views receive parsed keys, and malformed or deprecated (Org/Course/Run, i4x://) keys become routing-level 404s. Bumps edx-drf-extensions 10.7.0 -> 10.8.0, the release that adds the converters (plus the ADR 0029/0032/0036 building blocks this API series already consumes). Converter unit tests live in the library; the per-API URL tests in the following commits cover resolve/reverse integration through the real routes.
Mount the conforming routes beside the legacy /api/contentstore/v1/xblock/ ones (OEP-21), serving the same XblockViewSet: the collection becomes plural (rule 2), the API name describes the domain rather than the implementing Django app (rule 3), the usage key is resolved by the shared usage_key converter, which turns malformed and deprecated i4x:// keys into routing-level 404s (rule 9), and URL names are snake_case, version-free, and unique (rule 11). The viewset's initial() coerces a parsed UsageKey back to the string form the action methods expect, so both mounts share one contract. The legacy routes stay live for their deprecation window and are marked deprecated: true in the OpenAPI schema via the new cms_mark_migrated_paths post-processing hook; cms_api_filter now also admits /api/authoring/ paths. Tests pin reverse() literals, same-view resolution for both mounts, the routing-level 404, and handler parity on the conforming routes. ADR 0038 (implementation note 4) asks that /api/authoring/v1/xblocks/ be reconciled with the Learning Core /api/xblock/v2/xblocks/ rather than leaving two names for what looks like one API; that reconciliation is an API-owner decision tracked with the DEPR work, not part of this mechanical migration.
…ernal BFF) Mount the conforming home/, home/courses/, and home/libraries/ routes at /api/authoring/v3/ beside the legacy /api/contentstore/v3/home/ ones (OEP-21), serving the same HomeViewSet, with snake_case version-free URL names (rule 11) and the domain-named api_name (rule 3). home is a BFF aggregate for the Studio home screen. Rule 4 disfavors screen names as resources, but the ADR's BFF provision applies: the surface keeps the /api/ prefix and one canonical conforming mount, and is marked x-internal in the OpenAPI schema — on both mounts — so clients can tell it apart from a stable resource contract. The legacy routes are additionally marked deprecated: true. Tests pin reverse() literals, same-view resolution for all three action pairs, and the ADR 0029 envelope on the conforming mount.
Mount the conforming courses/ collection at /api/authoring/v4/ beside the legacy /api/contentstore/v4/home/courses/ route (OEP-21), serving the same HomeCoursesViewSet: the screen-shaped home/courses/ address becomes the concrete plural collection of authorable courses (rule 4), filtered, sorted, and paginated in the query string, under the domain-named api_name (rule 3) with a snake_case version-free URL name (rule 11). The legacy route stays live for its deprecation window and is marked deprecated: true in the OpenAPI schema. Tests pin the reverse() literal, same-view resolution for both mounts, and 401/200 contract parity on the conforming mount.
Mount the conforming /api/authoring/v3/courses/{course_key}/details/ route beside the legacy /api/contentstore/v3/course_details/{course_id}/ one (OEP-21), serving the same CourseDetailsViewSet: the screen-shaped collection becomes a sub-resource of the plural courses/ collection, one level deep — the ADR's own target for these endpoints (rules 4 and 8) — with the course key resolved by the shared course_key converter, which turns malformed and deprecated Org/Course/Run keys into routing-level 404s (rule 9).
resolve_course_key() now also accepts an already-parsed CourseKey, so both mounts funnel through one code path and share one contract. The legacy route stays live for its deprecation window and is marked deprecated: true in the OpenAPI schema. Tests pin the reverse() literal, same-view resolution, the routing-level 404, and 401/403 parity on the conforming mount.
Mount the conforming /api/authoring/v3/courses/{course_key}/grading/ route beside the legacy /api/contentstore/v3/authoring_grading/{course_key}/ one (OEP-21), serving the same AuthoringGradingViewSet: the app-flavored authoring_grading collection becomes the grading sub-resource of the plural courses/ collection, one level deep (rules 3, 4 and 8) — the authoring_ prefix is dropped because the namespace already says it — with the course key resolved by the shared course_key converter (rule 9). Both mounts funnel through resolve_course_key(), which already accepts parsed keys, so they share one contract.
The legacy route stays live for its deprecation window and is marked deprecated: true in the OpenAPI schema. Tests pin the reverse() literal, same-view resolution, the routing-level 404, and 401/200 PATCH parity on the conforming mount.
…nforming URL names)
/api/enrollment/v2/ already conforms in API name and version position; this fixes the remaining rule 6 and rule 11 violations. Conforming routes are dual-mounted (OEP-21) beside the legacy slashless ones, serving the same views: GET /enrollments/ (the admin list's optional-slash pattern — the ADR's own rule 6 example — is split into an exact slashed route plus a slashless legacy route, so every address that resolved before still resolves), GET /enrollments/{username},{course_key}/ under the plural collection (rule 2), GET /courses/{course_key}/ (plural, slashed), and roles/ renamed from the versioned kebab-case enrollment-v2-roles to user_roles (rule 11; path unchanged). Conforming member routes resolve course keys with the shared course_key converter (rule 9); the views coerce a parsed CourseKey back to the string form their bodies expect, so both mounts share one contract.
The two legacy retrieve forms no longer share one URL name — Django resolved that only by argument signature, the fragility rule 11 calls out — and the slashless legacy addresses are marked deprecated: true in the OpenAPI schema via a post-processing hook scoped to /v2/ (deprecating v1 is its own DEPR decision). Deeper ADR 0038 targets — collapsing the singular enrollment/ collection into enrollments/, replacing the unenroll verb (rule 10) with DELETE on the member address, and addressing the requesting user as me (rule 9) — are contract changes and belong to a future v3 per ADR 0037.
Tests pin the reverse() literals, same-view resolution for every legacy/conforming pair, the optional-slash coverage split, the unique legacy names, the routing-level 404, and 401 parity on both admin-list addresses.
…igration Review feedback: the new authoring_urls.py module docstrings restated ADR 0038's rules rather than describing the module, and several comments repeated what the commits already say. The three urls.py modules now carry a one-line docstring matching their siblings in the same package, the per-rule conformance lists and the "same view, same contract" notes are gone, and the test-class docstrings, Enrollment v2 docstring, and spectacular helpers are trimmed; comments that prevent a mistake are kept but shortened (conforming routes pass a parsed CourseKey/UsageKey where legacy routes pass the raw string; POSTPROCESSING_HOOKS replaces rather than extends drf-spectacular's default list). Prose only — with docstrings stripped, all 19 files parse to ASTs identical to the previous revision, ruff passes, and no view or serializer docstring is touched.
Review feedback on openedx#39037: the code should follow the standards without naming the documents. The three authoring urls.py modules now read "Authoring API vN URLs.", the URL-structure test banners and the Enrollment v2 module docstring lose their rule references, and the remaining comments say what the code does instead — deprecation window rather than OEP-21 window, "whose course_key path converter hands views a parsed key" rather than a rule number. Only lines this branch adds are touched; the pre-existing ADR references elsewhere in these files are left alone, the one exception being the ADR 0028 line in the Enrollment v2 module docstring, which this branch was already rewriting. Prose only apart from one assert message in the new URL-structure test, which now reads "missing error-envelope field". ruff passes.
…refix SCHEMA_PATH_PREFIX_TRIM is a boolean that we had set to a string in three places; it only worked because the string was truthy. More importantly the trimming itself produced wrong URLs: the LMS spec advertised /v2/enrollments/ against a bare LMS_ROOT_URL server, and adding /api/authoring/ to the CMS spec left those paths untrimmed beside trimmed contentstore ones. Widening the prefix regex would collapse /api/contentstore/v3/home/ and /api/authoring/v3/home/ onto one key. Drop the trim, drop the CMS-contentstore server that existed only to compensate, keep SCHEMA_PATH_PREFIX for tag extraction, and update both post-processing hooks to full paths. Adds tests for the hooks, which had none.
…iew actions Review feedback on the dual mounts. A legacy mount and its conforming mount serve the same view and tokenize to the same operationId, so drf-spectacular was breaking the tie with a numeral suffix in registration order: the CMS schema put _2 on the legacy path and the LMS schema put it on the conforming one, so regenerating the SDK would have renamed modules by accident. Each service now has an AutoSchema subclass wired in as DEFAULT_SCHEMA_CLASS that suffixes the legacy address with _legacy and leaves the conforming address with the clean id, so a regenerated SDK follows the same function name onto the non-deprecated address. The two Studio home viewsets carry their own schema instances, which bypass the default, so those are rebased onto the CMS class as well; tests pin the ids for each pair, assert the default wiring, and assert the per-view schemas chain the CMS class. The LMS legacy predicate is shared between the schema class and the deprecation hook. Two URL names had been dropped although every address was kept: enrollment-v2-roles and the course-only form of enrollment-v2-retrieve. Both are registered again on their paths after the new names, so reverse() keeps working for out-of-tree callers and resolution stays on the first entry. The route-sharing tests compared .func.cls, which is the same object for a ViewSet whatever actions a mount wires up; they now compare .func.actions as well, and the v1 test covers the list route too.
Admit /api/grade/v2/ and /api/grades/v1/ to the LMS OpenAPI document, and mark every /api/grades/v1/ operation deprecated through the existing post-processing hook instead of adding a second one.
Grades v1 had no standardized endpoint: bare error bodies, cursor
pagination, Bearer auth, numeric-id addresses, verb paths and
200 {"success": false}. Fixing any of it changes the contract, so v2
is a new version and v1 stays mounted as it is.
Nine operations replace the nine v1 routes: course grades (with the
staff-only section breakdown as ?view=full), gradebook entries,
all-or-nothing subsection grade overrides, subsection grades and their
override history, the grading policy, and submission histories. Every
v1 permission check maps to a permission class, and tests compare v2
with v1 caller by caller, CCX included. Parity tests compare v1 and v2
responses and fail on any undeclared difference.
Docstring-only markers on the nine v1 views, naming each successor. No statement, decorator or import changes; the syntax trees match the base with docstrings blanked.
|
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 Grades functional area (#39057, umbrella #38137) as a new version,
/api/grade/v2/, following the API standardization ADRsdocs/decisions/0025to0038, OEP-66 and OEP-69./api/grades/v1/stays mounted, unchanged in behaviour, and marked deprecated.Why a new version. Nothing in v1 was standardized, and every endpoint needs contract changes that existing callers would notice:
usernameand opaque keys instead of numeric ids, and no verbs in paths;200 {"success": false}becomes a real 404;Changes like these go in a new version, not the current one.
Stacked on #39078 (URL structure). This branch uses its
register_url_converters(), its schema path settings and its LMS deprecation hook, and adds no parallel ones. Until #39078 merges, this branch also carries #39078's commits (rebased onto current master). Only the last three commits belong to this PR:feat: publish the Grades API in the LMS schema,feat: add the Grades API v2 at /api/grade/v2/anddocs: mark Grades API v1 deprecated in favour of v2. The branch rebases onto master once #39078 merges, and this PR stays a draft until then.Change table
/api/grade/v2/and/api/grades/v1/are admitted to the LMS schema; all nine v1 operations are marked deprecatedGET course_grades/,GET course_grades/{username},{course_key}/courses/,courses/{course_id}/,section_grades_breakdown/?view=full, for global staff only (as E8). Rows are scoped: staff see all, a restricted token sees its org and user filters, everyone else sees their ownGET courses/{course_key}/gradebook_entries/[{username}/]gradebook/{course_id}/?view=minimal. Invalid filter values give 400 (v1 gave 500)POST courses/{course_key}/subsection_grade_overrides/gradebook/{course_id}/bulk-updateoverrides[1].usernameGET subsection_grades/{username},{usage_key}/,.../override_history_records/subsection/{subsection_id}/GET courses/{course_key}/grading_policy/policy/courses/{course_id}/,gradebook/{course_id}/grading-info?view=minimalkeeps the policy endpoint's access and content. A read never creates a course overviewGET courses/{course_key}/submission_histories/submission_history/{course_id}/?view=full; an unknown course gives 404ADRs applied
help_texton every field, nullable and view-dependent fields marked, and agrade_v2.*ref_nameon each@extend_schemaand an explicit operation id on all nine operations, with examples for the batch override. The only v2 generator warning is "could not resolve authenticator" for the platform's default authentication classes, the same as Enrollment v2 today (library follow-up)path()routes, but not router-registered. A DRF router names routescourse_grade-list, cannot carry the{username},{course_key}composite or the key converters, and publishes an extra{id}operation. The per-row query growth is pinned below v1's200 success:falsebecomes 404; no exception text in any body. Until the library catalogues them, a malformed JSON body (400), 405, 406 and 415 report typeinternal(library follow-up); tests pin this and flip when the pin moves?view=full) can assign an uncohorted learner to a cohort, writing two cohort tables and sendingCOHORT_MEMBERSHIP_CHANGEDand two tracking events. The tracking issue is not filed yetbulk-updatebecomes a POST to a noun. The two consolidations (breakdown into course grades, grading-info with policy) are view variants that keep each legacy check for the data it guarded, as caller-by-caller comparison tests showorderingallow-list with an id tie-break, and no parameter aliases in v2?view=fullor?view=minimalwhere a representation is heavy; view-dependent fields are optional in the schema; other values give 400ScopingPolicyprotocol;FullScopePolicyon the other lists; filters only narrowCompliance matrix
help_texton every field;allow_nullwhere values can be null (for exampledisplay_nameon an unnamed subsection); fields dropped by a view arerequired=Falsetest_schema.py::test_override_request_body_differs_from_its_response,test_gradebook_list_is_the_page_envelope(required{username, percent}),test_grading_policy_operation,GradingPolicyUnnamedSubsectionTestCourseGradeFullViewAccessTest,CourseGradeFullViewParityTest,RestrictedTokenUserFilterParityTest,GradingPolicyAccessTest,GradingPolicyCcxAccessTest,HasGradebookAccess*intest_infrastructure.py, deny tests in every endpoint moduleextend_schema; explicit operation ids; request and error examples on N5; the default-authenticator warning is the only v2 warningtest_every_operation_id_is_the_declared_one,test_override_examples,test_v2_generates_no_warning_but_the_default_authentication_one, schema gatetest_full_list_grows_by_a_fixed_count_per_row,test_list_grows_by_one_query_per_row,*QueryCountTestgrades/writable-gradebook-disabled,grades/grades-frozen,grades/subsection-unavailable; no exception textassert_error_envelopeacross modules;test_failure_while_storing_records_nothing;test_failure_while_recomputing_keeps_every_override_and_answers_500;UnmatchedAddressTest; the 400/405/406/415internalpins*ReadOnlyTest,GradebookColdCohortSettingsTest,ComputedGradeAnonymousIdTest,CourseGradeCohortAssignmentTest, the cohort no-write tests on N3/N4/N7/N8/N9,test_course_without_an_overview_is_not_created_oneCourseGradeFullViewParityTest,assert_views_follow_v1(N8, with fields per view)GradePagination(DefaultPagination)adds only the response schemaCourseGradePagingTest,GradebookRowTest.test_paging, schema testsexcluded_course_rolespublished as a repeatable array andcohort_idas an integer; unknownorderinggives 400CourseGradeFilterTest,GradebookFilterTest,test_gradebook_filter_parameter_typesauthentication_classeson any v2 viewtest_deactivated_account_*, restricted-token tests on N5 and N9, hygiene H4 clean?view=fullon N1/N2 (staff) and N9;?view=minimalon N3/N4 and N8*RepresentationTest,test_minimal_view_reads_the_course_one_level_deepdeprecated: true; DEPR issue, release and guide owedtest_every_legacy_operation_is_deprecatedGradeV2UrlNameTest,GradeV2ResolutionTest,SupersededAddressTest,test_no_route_is_shadowed_by_a_not_found_route,UnmatchedAddressTest,reverse()literal tests, urls gateCourseGradeScopingPolicy(staff all, restricted token by org and first user filter as v1, else own rows);FullScopePolicyelsewhereCourseGradeVisibilityTest,RestrictedTokenListTest,test_filters_do_not_widen_a_learners_viewCourseGradeScopingPolicyby name, but it implements the library's protocol rather than copying a library classBackward compatibility
rest_api/urls.py,rest_api/serializers.py,apps.py, the shared view utils, and the enrollment form and throttle) are protected.api_grades_v1_courses_retrieve,_2), listed under Gate report. So the schema gate is not evidence that v1 is unchanged; the syntax-tree comparison and the untouched v1 tests are.Migration guide (v1 → v2)
Everywhere
count,num_pages,current_page,start,next,previous,results;page,page_sizeup to 100) instead of the cursor.user_id,id,grade_id,history_user_id); learners are named byusername.course_id→course_key,module_id/subsection_id→usage_key,assignment→assignment_usage_key,history_record_limit→page_size.Course grades (E1, E2, E8 → N1, N2)
current_grade(0 to 100) becomespercent(0 to 1). This is a unit change, not only a rename.section_breakdown[].sequential_idbecomesusage_key, and isnullon entries that name no subsection.?view=full, for global staff only. Rows carryemailandletter_gradewith or without it.grades:readand an org filter to list rows; a restricted token for a staff service user no longer gets E8's breakdown.resultsbut still counted incount, as in v1.Gradebook entries (E4 → N3, N4)
total_users_countandfiltered_users_countare removed;countis the filtered total. For "N of M learners", make a second call without filters andpage_size=1.cohort_idnaming another course's cohort, or no cohort, gives an empty page (v1: 404).cohort_id=1.5and other invalid filter values give 400 (v1: 500 or ignored).Overrides (E5 → N5)
{"overrides": [{username, usage_key, earned_all_override, possible_all_override, earned_graded_override, possible_graded_override, comment}]}, JSON only, at most 100 items.errorskeyed likeoverrides[1].username. v1 applied the valid items and returned 202 or 422.commentis at most 300 characters.Subsection grades (E7 → N6, N7)
override_history_records/, paginated, newest first.grades/subsection-unavailable(v1: 200success:false).Grading policy (E3, E6 → N8)
can_see_bulk_managementbecomesbulk_management_enabled, andassignment_typesis a list, not a dict.?view=minimal:assignment_type/count/droppedbecometype/min_count/drop_count, and it addsgrade_cutoffsand each type'sshort_label.allparameter is dropped: it changed load depth, not output.Submission histories (E9 → N9)
user→username,location→usage_key,name→display_name,submission_history→submissions.course_idandcourse_nameare removed; the course is the address.dataonly with?view=full.problems[]andsubmissions[]are not bounded, as in v1; the page bounds learners only.In-place edits
lms/djangoapps/grades/rest_api/v1/views.pylms/djangoapps/grades/rest_api/v1/gradebook_views.pyShared service files, not frozen:
lms/urls.py: the mount.lms/lib/spectacular.py: the filter admits grades, and the existing hook marks v1 deprecated.lms/lib/tests/test_spectacular.py.drf-spectacular prefers method docstrings, so the successor text reaches the schema description of one v1 operation; all nine carry
deprecated: true.Error-format decision
Versioned. v1 keeps its bodies, and v2 uses the envelope. v1's main caller is frontend-app-gradebook, which migrates on its own schedule.
Deprecations
All nine
/api/grades/v1/operations aredeprecated: truein the LMS schema, and each v1 view's docstring names its successor. The DEPR issue is not filed yet; the v1 docstring markers link a placeholder that is replaced once it exists. Both mounts stay for at least one named release, to be named in the DEPR issue.Authorization
JWT_RESTRICTED_APPLICATION_OR_USER_ACCESS, scopegrades:readCOURSE_GRADE_READ_ACCESS(endpoint), plusCourseGradeScopingPolicy(rows: staff all, restricted token by org and first user filter, else own)IsStaff, plus the middleware'sNotJwtRestrictedApplicationHasGradeBreakdownAccesson?view=full:IsAdminUser & NotJwtRestrictedApplicationcourse_author_access_required(with #39145's CCX branch)HasGradebookAccess(imports v1's CCX check)verify_course_exists,verify_writable_gradebook_enabledCourseExists,WritableGradebookEnabled, after accessare_grades_frozenGradesNotFrozen, lasthas_course_author_access(no CCX branch)HasGradebookAccess, which gains CCX, plus enrollment of the named learnerhas_access('staff')COURSE_GRADING_POLICY_READ_ACCESS = IsAuthenticated & (HasCourseStaffAccess | HasGradebookAccess), plusHasGradebookAccessUnlessMinimalso the default view keeps E6's accessIsStaff, throttle,can_disable_rate_limitIsAdminUser, same throttle and switchDeny-path tests cover anonymous callers, learners, staff of another course, CCX outsiders, restricted tokens with and without scope, deactivated sessions, and, for callers without access, whether a course exists or which flags are set.
Gate report
run_gates.sh --base <#39078 rebased onto master> --service lms --prefix /api/grade/v2/, run in a Tutor dev LMS container (Python 3.12, edx-drf-extensions 10.9.0). No gate was skipped.The schema gate's 46 warnings:
Hygiene notes.
CourseGradeScopingPolicyby name. It implements the library'sScopingPolicyprotocol throughScopedQuerysetMixin, which is the intended use.GradePagination, the documented stopgap.envs/tutorsettings. Those are untracked and not in this diff.Query counts (v1 → v2, same fixture)
?view=fullat 5 / 10 rowsv1 cached the gradebook's unfiltered user count for an hour. v2 counts the filtered page on every request, and the staff cross-course N1 list counts all active enrollments.
Follow-ups
ATOMIC_REQUESTSwhen it converts an exception to a 500;bulk-update/history/, which matches no route.openedx-platform-sdktag, so the SDK needs regenerating.