Remove integration-specific options from contracts#3704
Merged
Conversation
OpenAPI ChangesShow/hide ## Changes for v0.yaml:Unexpected changes? Ensure your branch is up-to-date with |
dsubak
reviewed
Jun 26, 2026
| contract = factories.ContractPageFactory( | ||
| integration_type=CONTRACT_MEMBERSHIP_SSO | ||
| membership_type=CONTRACT_MEMBERSHIP_MANAGED | ||
| if is_sso |
Contributor
There was a problem hiding this comment.
This is an ultra-nit, but we could switch the arg names referencing sso at this point to use the term managed instead for consistency if we wanted to (is_sso -> is_managed and update_sso -> update_managed)
dsubak
reviewed
Jun 26, 2026
dsubak
left a comment
dsubak
left a comment
Contributor
There was a problem hiding this comment.
Still owe you some manual testing before I can approve, but it looks pretty straightforward mechanically. Big fan of making the contract models a bit less crufty!
dsubak
reviewed
Jun 26, 2026
| "membership_type", | ||
| type=str, | ||
| help="The membership type for this contract.", | ||
| choices=[value[0] for value in CONTRACT_MEMBERSHIP_CHOICES], |
Contributor
There was a problem hiding this comment.
Think this constant got dropped - we'll need to fix the import and this choice valueset
jkachel
force-pushed
the
jkachel/drop-sso-contract-opts
branch
from
e784563 to
54fe51d
Compare
dsubak
approved these changes
Jun 29, 2026
dsubak
left a comment
dsubak
left a comment
Contributor
There was a problem hiding this comment.
I think this is looking good! After a bit of hullabaloo fighting keycloak, I was able to step through the manual testing and get my test user added to the contract on login while in the KC Org and removed from it while removed.
options This is non-working as is - lots of dependencies to fix up still.
…onsso types appropriately
jkachel
force-pushed
the
jkachel/drop-sso-contract-opts
branch
from
54fe51d to
55ba8c4
Compare
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
2 participants
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?
Closes https://github.com/mitodl/hq/issues/11657
Description (What does it do?)
This PR:
membership_typeset tossoornon-ssotomanagedandcode, respectively.integration_typefield fromContractPage.ssoandnon-ssotype choices.How can this be tested?
Automated tests should pass, and the system should work in general. The B2B system specifically should work as expected - you should be able to create contracts, add courseware, etc. and everything that is expected to happen should happen.
Additionally, the auto-attach functionality should be tested:
sync_keycloak_orgscommand may be helpful here.)managed, no price, no seat limit.code.If you wish you can also add them to the contract with an enrollment code, and then test adding and removing the user from the Keycloak org again; they should remain in the contract. (This functionality didn't change, so it should work as expected - if there's questions about what the behavior should be, double-check against main.)
The migration in this PR is reversible. I didn't add any new options so it does not set anything back to use
ssoornon-sso, but it does make sure theintegration_typefield is re-populated frommembership_type.Additional Context
As noted elsewhere, the contract doesn't specify whether or not users are to log in via SSO or not. It does specify whether or not the user gets access to the contract based solely on their membership in the org, though.