Add the staff contract provisioning API (C3 3/3) - #3965
Merged
Merged
Conversation
OpenAPI ChangesShow/hide changesUnexpected changes? Ensure your branch is up-to-date with |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
blarghmatey
added this pull request to stack #3966
September 15, 2026 14:44
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
blarghmatey
force-pushed
the
b2b-c3-contract-routes
branch
from
September 15, 2026 14:53
0cb6ced to
c399bf7
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical correctness and concurrency issues remain in expiration, forced moves, contract creation, and code assignment.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a staff-facing B2B contract provisioning API for contract CRUD, courseware setup, status tracking, retries, and enrollment-code management.
Changes:
- Adds nested provisioning endpoints, serializers, routes, and tests.
- Shares enrollment-code assignment and expiration logic across API, dashboard, and commands.
- Updates status constants, enum naming, and OpenAPI specifications.
File summaries
| File | Reviewed changes |
|---|---|
openapi/specs/v2.yaml |
Regenerated provisioning API specification. |
openapi/specs/v1.yaml |
Regenerated provisioning API specification. |
openapi/specs/v0.yaml |
Regenerated provisioning API specification. |
openapi/settings_spectacular.py |
Pins provisioning status enum names. |
b2b/views/v0/urls.py |
Registers nested contract provisioning routes. |
b2b/views/v0/provisioning.py |
Adds provisioning endpoints. Critical (1 vote): missing membership_type causes TypeError; moderate (1 vote): assignment prefetch is not contract-scoped; nit (1 vote): code-contract queueing lacks endpoint coverage. |
b2b/views/v0/provisioning_contracts_test.py |
Covers staff access, scoping, lifecycle, setup/retry, removal, and code routes. |
b2b/views/v0/manager.py |
Centralizes bulk assignment. Critical (2 votes): concurrent requests can assign one code twice; moderate (1 vote): duplicate products can duplicate candidate codes. |
b2b/serializers/v0/provisioning.py |
Defines provisioning serializers. Critical (1 vote): forced course-run moves leave prior M2M associations attached. |
b2b/management/commands/b2b_codes.py |
Reuses shared enrollment-code expiration logic. |
b2b/contracts.py |
Implements shared setup and expiration helpers. Critical (1 vote): shared products can cause codes and discounts for another contract to be deleted; moderate (1 vote): unused discounts are not distinct. |
b2b/contracts_test.py |
Tests shared contract helpers. |
b2b/constants.py |
Adds setup status constants. |
Review details
Suppressed comments (4)
b2b/contracts.py:414
get_unused_discounts()is also not distinct, so a code attached to multiple products is processed once per product. Dry runs will return duplicate code rows, while real runs repeat detach/delete work for the same discount. Iterate over a distinct queryset here.
for discount in list(contract.get_unused_discounts()):
b2b/views/v0/manager.py:202
get_discounts()joins through the contract's products and is not distinct. If a discount applies to more than one product, it appears multiple times here, so the same free code can be assigned to multiple people in one request. Adddistinct()before the slice.
available_discounts = list(
contract.get_discounts()
.filter(contract_redemptions__isnull=True)
.order_by("id")[: len(email_assignees)]
)
b2b/views/v0/provisioning.py:546
- The new code-contract branch is not covered by the endpoint tests:
test_add_courseware_and_follow_setupuses a managed, free contract, soqueue_enrollment_code_check_if_requiredis never asserted for this action. Add a code-contract case that verifies adding a run queues the enrollment-code check; this is a key asynchronous behavior promised by the API and can otherwise regress independently of the PATCH test.
if added.runs_added:
queue_enrollment_code_check_if_required(contract)
b2b/views/v0/provisioning.py:639
- This prefetch is not scoped to the contract being viewed. A course run can be attached to multiple contracts, so the same discount can have redemption rows whose
contractvalues differ; the endpoint can then show the latest assignment from another contract. Filter the prefetch queryset by this contract before selecting the latest row.
Prefetch(
"contract_redemptions",
queryset=DiscountContractAttachmentRedemption.objects.select_related(
"user"
).order_by("-created_on")[:1],
- Files reviewed: 13/13 changed files
- Comments generated: 4
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
blarghmatey
force-pushed
the
b2b-c3-contract-routes
branch
from
September 17, 2026 14:48
33657d0 to
cf9625a
Compare
blarghmatey
force-pushed
the
b2b-c3-contract-routes
branch
2 times, most recently
from
September 17, 2026 18:14
d14414d to
f42acac
Compare
blarghmatey
force-pushed
the
b2b-c3-contract-routes
branch
from
September 22, 2026 13:51
f42acac to
e39d010
Compare
jkachel
approved these changes
Sep 22, 2026
jkachel
left a comment
jkachel
left a comment
Contributor
There was a problem hiding this comment.
Functionality works as described. 👍
I do have one concern. It's fine as-is for MVP, I think, but something that will need to be added somewhat soon: this will only add in the default variants for the contract. There's no provisioning for variant types into the contract, and thus the courseware added will only ever be the default variants. This is not a deal-breaker - we can add the variants in later via Django Admin and re-provision the courseware (which should not re-run the existing variants, unless that is requested specifically). But a lot of contracts will have some amount of variants beyond the default so there should be an API to handle the supported variant sets. (Courseware provisioning should respect the configured variant sets for the contract and course on its own.) Approving this since I don't think it's strictly necessary for this stage of things, but should definitely be part of the next phase of development.
Contract setup still meant running b2b_contract, b2b_courseware,
b2b_codes and check_contract_variant by hand. These routes, nested under
/api/v0/b2b/provisioning/organizations/{org_key}/contracts/, give the
staff UI the same operations: create, retrieve, PATCH, courseware add and
remove, setup status, retry, and enrollment codes list, expire and assign.
A write returns once MITx Online's rows exist. edX clones and enrollment
codes follow in Celery, and setup-status reports them from the
CourseRunClone records and the expected-versus-existing code count. Nothing
here sets OrganizationOnboarding to contract_ready: for an SSO org the
contract is often ready before the IdP is validated, and the single
ordered state would then claim SSO is done.
The whole viewset is IsAdminUser, reads included, unlike the organization
routes' IsAdminOrReadOnly. The codes routes return redeemable codes.
The code check is queued after PATCH and after courseware is added, and
only for contracts that use codes. For the rest, the check strips codes,
which should stay a deliberate b2b_codes validate.
The manager dashboard's bulk-assign body moves into
bulk_assign_enrollment_codes so both surfaces assign codes the same way.
b2b_codes expire calls expire_unused_enrollment_codes, whose dry run no
longer assumes a code applies to exactly one product.
The setup status enums are pinned in ENUM_NAME_OVERRIDES. Unpinned they
published as StatusEnum and CloneStatusEnum. The specs for v1 and v2
change along with v0 because every version file carries every route.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NUA1EjC95LZV4J5swhZgxy
CodeQL flagged the detail field built from str(exc) as information exposure through an exception. The 400 bodies for a course with no source run and for an invalid course key are now built from the requested courseware ID, and the exception is logged instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUA1EjC95LZV4J5swhZgxy
CreateContractSerializer made membership_type optional because the model has a default, while create_contract takes it as a required keyword. A POST with only a name passed validation and then raised TypeError, a 500. The field is now required, as it is for b2b_contract create, and the specs regenerate with it in CreateContractRequest's required list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUA1EjC95LZV4J5swhZgxy
…r PATCH Follows the removal of --force from b2b_courseware and the shared service: the API's ContractCoursewareSerializer no longer takes force, and the courseware endpoint no longer passes it. A run already in another contract is reported as skipped. The specs regenerate without the field. Sentry flagged that partial_update returned the contract still holding the queryset's contract_programs prefetch, so the response could show the programs as they were before the save. Cleared the same way DRF's UpdateModelMixin does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUA1EjC95LZV4J5swhZgxy
blarghmatey
force-pushed
the
b2b-c3-contract-routes
branch
from
September 25, 2026 16:38
e39d010 to
b80e8ff
Compare
Member
Author
|
@jkachel agreed, this goes in the next phase. Re-adding a course through |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
Capability C3 of the B2B onboarding RFC, https://github.com/mitodl/hq/discussions/12784. Design and decisions: https://github.com/mitodl/hq/discussions/12784#discussioncomment-18437846.
Stack (3 of 3). Based on #3964. The consumer is the contract section of the staff UI.
Description (What does it do?)
Routes nested under
/api/v0/b2b/provisioning/organizations/{org_key}/contracts/, so contracts can be set up without runningb2b_contract,b2b_courseware,b2b_codesandcheck_contract_variantby hand:GET /,POST /GET /{id}/,PATCH /{id}/POST /{id}/courseware/POST /{id}/courseware/remove/GET /{id}/setup-status/in_progress/complete/failedPOST /{id}/retry-setup/GET /{id}/codes/POST /{id}/codes/expire/POST /{id}/codes/assign/Per the design comment:
CourseRunClone(1/3).max_learnerscodes per run. It's queued after PATCH and after courseware is added, and only for contracts that use codes. For the rest the check strips codes, which should stay a deliberateb2b_codes validate.OrganizationOnboardingtocontract_ready. For an SSO org the contract is often ready before the IdP is validated, and the single ordered state would then say SSO is done.The whole viewset is
IsAdminUser, reads included, unlike the organization routes'IsAdminOrReadOnly. The codes routes return redeemable codes.Also:
bulk_assign_enrollment_codes, so both surfaces assign codes the same way. The manager route's behaviour doesn't change.b2b_codes expirecallsexpire_unused_enrollment_codes. Its dry run no longer assumes each code applies to exactly one product.ContractSetupStatusEnumandCourseRunCloneStatusEnumare pinned inENUM_NAME_OVERRIDES. Unpinned they published asStatusEnumandCloneStatusEnum.v1.yamlandv2.yamlchange along withv0.yamlbecause every version file carries every route (Fix OpenAPI spec partitioning #3824).How can this be tested?
Run in the web container:
pytest b2b openedx, 814 passed and 7 skipped.generate_openapi_spec --fail-on-warnexits 0 and the committed specs are its output. pre-commit passes.b2b/views/v0/provisioning_contracts_test.pycovers: staff-only access, scoping to the organization, create, the code check after PATCH, add then setup-status through tocomplete, a failed clone and retry, remove, and the three codes routes.Not run by hand. Logged in as a staff user on a local stack, in the browsable API:
POST /api/v0/b2b/provisioning/organizations/<org_key>/contracts/with{"name": "Test", "membership_type": "code", "max_learners": 2}POST .../contracts/<id>/courseware/with{"courseware_id": "<course readable_id with a source run>"}GET .../contracts/<id>/setup-status/shows the runpendingand the contractin_progressuntil the clone and code check finishAdditional Context
organizations/andcontracts/viewsets toIsAdminUser. It editsb2b/views/v0/__init__.py, which this stack doesn't touch.🤖 Generated with Claude Code
https://claude.ai/code/session_01NUA1EjC95LZV4J5swhZgxy