From e4d03545c6a719212f8fa06f96b897dc8a37c680 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 1 Oct 2025 15:43:25 -0500 Subject: [PATCH 01/15] Update integration type field to membership type, update org/contract attachment points, update org membership in Keycloak where approrpiate --- b2b/admin.py | 6 +- b2b/api.py | 177 +++++++++++----- b2b/api_test.py | 148 +++++++++++--- b2b/constants.py | 40 +++- b2b/factories.py | 9 +- b2b/management/commands/b2b_contract.py | 28 ++- b2b/management/commands/b2b_list.py | 2 +- ...9_rename_integration_type_add_new_types.py | 63 ++++++ b2b/models.py | 69 ++++++- b2b/serializers/v0/__init__.py | 4 +- b2b/views/v0/__init__.py | 4 + b2b/views/v0/views_test.py | 12 +- courses/api.py | 6 +- courses/api_test.py | 4 +- courses/views/v2/__init__.py | 2 +- courses/views/v2/views_test.py | 7 + users/admin.py | 10 +- ...8_add_fk_from_users_to_organizationpage.py | 55 +++++ users/models.py | 90 +++++++- users/models_test.py | 192 ++++++++++++++++++ users/serializers.py | 72 ++++++- users/views_test.py | 3 +- 22 files changed, 857 insertions(+), 146 deletions(-) create mode 100644 b2b/migrations/0009_rename_integration_type_add_new_types.py create mode 100644 users/migrations/0038_add_fk_from_users_to_organizationpage.py diff --git a/b2b/admin.py b/b2b/admin.py index 3858c3e198..470ae508bc 100644 --- a/b2b/admin.py +++ b/b2b/admin.py @@ -105,11 +105,11 @@ class ContractPageAdmin(ReadOnlyModelAdmin): "slug", "title", "organization", - "integration_type", + "membership_type", "contract_start", "contract_end", ] - list_filter = ["integration_type", "organization", "contract_start", "contract_end"] + list_filter = ["membership_type", "organization", "contract_start", "contract_end"] date_hierarchy = "contract_start" fields = [ "id", @@ -118,7 +118,7 @@ class ContractPageAdmin(ReadOnlyModelAdmin): "organization", "title", "description", - "integration_type", + "membership_type", "contract_start", "contract_end", "max_learners", diff --git a/b2b/api.py b/b2b/api.py index a95f27d03f..082d77040c 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -19,7 +19,7 @@ from b2b.constants import ( B2B_RUN_TAG_FORMAT, - CONTRACT_INTEGRATION_SSO, + CONTRACT_MEMBERSHIP_AUTOS, ORG_KEY_MAX_LENGTH, ) from b2b.exceptions import SourceCourseIncompleteError, TargetCourseRunExistsError @@ -47,6 +47,7 @@ from main.utils import date_to_datetime from openedx.api import create_user from openedx.tasks import clone_courserun +from users.models import UserOrganization log = logging.getLogger(__name__) @@ -596,7 +597,10 @@ def ensure_enrollment_codes_exist(contract: ContractPage): """ log.info("Checking enrollment codes for contract %s", contract) - if contract.integration_type == "sso" and not contract.enrollment_fixed_price: + if ( + contract.membership_type in CONTRACT_MEMBERSHIP_AUTOS + and not contract.enrollment_fixed_price + ): # SSO contracts w/out price don't need discounts. return _handle_sso_free_contract(contract) @@ -762,19 +766,30 @@ def create_b2b_enrollment(request, product: Product): def reconcile_user_orgs(user, organizations): """ - Reconcile the specified users with the provided organization list. - - When we get a list of organizations from an authoritative source, we need to - be able to parse that list and make sure the user's org attachments match. - This will pull the contracts that the user belongs to that are also - SSO-enabled, and will remove the user from the contract if they're not - supposed to be in them. It will also add the user to any SSO-enabled contract - that the org has. - - This only considers contracts that are SSO-enabled and zero-cost. If the - contract is seat limited, we will only add the user if there's room. - (If there isn't, we will log an error.) Only SSO-enabled contracts are - considered; any that the user is in that aren't SSO-enabled will be left alone. + Reconcile the specified user with the provided organization list. + + When we get a list of organizations from a source (so, in the user payload + from APISIX) for a particular user, we need to ensure that the user's + organization membership in MITx Online matches up with what we're given. In + addition, once we've done that, we need to ensure they're also in the + contracts that are marked as "managed". If the user is in an organization + that isn't in the list we've received, we need to remove them; in addition, + they should be removed from any "managed" contracts for the org they're in + as well. + + There is a special case where the user may be in an organization that isn't + represented in the payload we're given. This happens when the user uses an + enrollment code. We update the org membership here and in Keycloak, but the + payload from APISIX won't include their updated org membership until their + APISIX session expires. We have a flag on the many-to-many table that + indicates that we should leave those memberships alone - otherwise, we'll + inadvertently add them to the org and then immediately remove them. (Once + the org _does_ show up in the list, we should clear the flag.) + + We cache the user's org membership in redis to save some hits to the + database. This gets hit on every authenticated request, so probably good to + try to keep the query count low. The cache is a list of tuples of (org_uuid, + not_expected_in_payload). If the user is enrolled in any courses that are in a contract they'll be removed from, they will be left there. Not real sure what we should do in @@ -791,61 +806,87 @@ def reconcile_user_orgs(user, organizations): user_org_cache_key = f"org-membership-cache-{user.id}" cached_org_membership = caches["redis"].get(user_org_cache_key, False) - if cached_org_membership and sorted(cached_org_membership) == sorted(organizations): - log.info("reconcile_user_orgs: skipping reconcilation for %s", user.id) - return ( - 0, - 0, - ) + if cached_org_membership: + cached_expected_org_membership = [ + str(org_id) + for org_id, not_expected_in_payload in cached_org_membership + if not_expected_in_payload + ] + + if sorted(cached_expected_org_membership) == sorted(organizations): + log.info( + "reconcile_user_orgs: everything OK, skipping reconcilation for %s", + user.id, + ) + return ( + 0, + 0, + ) log.info("reconcile_user_orgs: running reconcilation for %s", user.id) - user_contracts_qs = user.b2b_contracts.filter( - integration_type=CONTRACT_INTEGRATION_SSO + # we've checked the cached org membership, so now figure out what orgs + # we're in but aren't in the list, and vice versa + + orgs_to_add = ( + OrganizationPage.objects.filter( + Q(sso_organization_id__in=organizations) & ~Q(organization_users__user=user) + ) + .filter(sso_organization_id__isnull=False) + .all() ) - if len(organizations) == 0: - # User has no orgs, so we should clear them from all SSO contracts. - contracts_to_remove = user_contracts_qs.all() - [user.b2b_contracts.remove(contract) for contract in contracts_to_remove] - user.save() - return (0, len(contracts_to_remove)) + orgs_to_remove = ( + UserOrganization.objects.filter( + ~Q(organization__sso_organization_id__in=organizations) + & Q(user=user, keep_until_seen=False) + ) + .filter(organization__sso_organization_id__isnull=False) + .all() + ) - orgs = OrganizationPage.objects.filter(sso_organization_id__in=organizations).all() - no_orgs = OrganizationPage.objects.exclude( - sso_organization_id__in=organizations - ).all() + for add_org in orgs_to_add: + # add org, add contracts, clear flag if we need to + new_membership, created = UserOrganization.objects.get_or_create( + user=user, + organization=add_org, + defaults={"keep_until_seen": False}, + ) - contracts_to_remove = user_contracts_qs.filter(organization__in=no_orgs).all() + if not created and new_membership.keep_until_seen: + new_membership.keep_until_seen = False + new_membership.save() + log.info( + "reconcile_user_orgs: cleared keep_until_seen for user %s org %s", + user.id, + add_org, + ) - if contracts_to_remove.count() > 0: - [ - user.b2b_contracts.remove(contract_to_remove) - for contract_to_remove in contracts_to_remove - ] + add_org.add_user_contracts(user) + log.info("reconcile_user_orgs: added user %s to org %s", user.id, add_org) - contracts_to_add = ( - ContractPage.objects.filter( - integration_type=CONTRACT_INTEGRATION_SSO, organization__in=orgs + for remove_org in orgs_to_remove: + # remove org, remove contracts + remove_org.organization.remove_user_contracts(user) + log.info( + "reconcile_user_orgs: removed user %s from org %s", user.id, remove_org ) - .exclude(pk__in=user_contracts_qs.all().values_list("id", flat=True)) - .all() - ) + remove_org.delete() - if contracts_to_add.count() > 0: - [ - user.b2b_contracts.add(contract_to_add) - for contract_to_add in contracts_to_add - ] - - user.save() user.refresh_from_db() - orgs = [str(org_id) for org_id in user.b2b_organization_sso_ids] + orgs = [ + (str(org.organization.sso_organization_id), not org.keep_until_seen) + for org in user.b2b_organizations.all() + ] + + user.b2b_organizations.filter( + organization__sso_organization_id__in=organizations, keep_until_seen=True + ).update(keep_until_seen=False) user_org_cache_key = f"org-membership-cache-{user.id}" caches["redis"].set(user_org_cache_key, sorted(orgs)) - return (len(contracts_to_add), len(contracts_to_remove)) + return (len(orgs_to_add), len(orgs_to_remove)) def reconcile_single_keycloak_org(keycloak_org: OrganizationRepresentation): @@ -925,3 +966,31 @@ def reconcile_keycloak_orgs(): ) return (created_count, updated_count) + + +def add_user_org_membership(org, user): + """ + Add a given user to a Keycloak organization. + + If we're adding a user to a contract, and they're not in that contract's + organization, we need to do that and update Keycloak as well. Since the user + won't have the org in their user data list initially, we'll also need to + flag the membership so we don't remove it immediately later in the + middleware. + + Args: + - org (OrganizationPage): The organization to add the user to. + - user (User): The user to add to the organization. + Returns: + - bool: True if the user was added, False otherwise. + """ + + org_model = get_keycloak_model(OrganizationRepresentation, "organizations") + + kc_org = org_model.get(org.sso_organization_id) + + if not kc_org: + log.warning("No Keycloak organization found for %s", org.sso_organization_id) + return False + + return org_model.associate("members", org.sso_organization_id, user.global_id) diff --git a/b2b/api_test.py b/b2b/api_test.py index 55c87f31f7..926c2bff70 100644 --- a/b2b/api_test.py +++ b/b2b/api_test.py @@ -25,10 +25,11 @@ ) from b2b.constants import ( B2B_RUN_TAG_FORMAT, - CONTRACT_INTEGRATION_NONSSO, - CONTRACT_INTEGRATION_SSO, + CONTRACT_MEMBERSHIP_NONSSO, + CONTRACT_MEMBERSHIP_SSO, ) from b2b.exceptions import SourceCourseIncompleteError, TargetCourseRunExistsError +from b2b.factories import OrganizationPageFactory from b2b.models import OrganizationIndexPage, OrganizationPage from courses.constants import UAI_COURSEWARE_ID_PREFIX from courses.factories import CourseFactory, CourseRunFactory @@ -52,6 +53,7 @@ from main.utils import date_to_datetime from openedx.constants import EDX_ENROLLMENT_VERIFIED_MODE from users.factories import UserFactory +from users.models import UserOrganization FAKE = faker.Faker() pytestmark = [ @@ -260,9 +262,9 @@ def test_ensure_enrollment_codes( # noqa: PLR0913 assert_price = price if price else Decimal(0) contract = factories.ContractPageFactory( - integration_type=CONTRACT_INTEGRATION_SSO + membership_type=CONTRACT_MEMBERSHIP_SSO if is_sso - else CONTRACT_INTEGRATION_NONSSO, + else CONTRACT_MEMBERSHIP_NONSSO, enrollment_fixed_price=price, max_learners=max_learners, ) @@ -304,8 +306,8 @@ def test_ensure_enrollment_codes( # noqa: PLR0913 if update_no_price: contract.enrolment_fixed_price = None if update_sso: - contract.integration_type = ( - CONTRACT_INTEGRATION_NONSSO if is_sso else CONTRACT_INTEGRATION_SSO + contract.membership_type = ( + CONTRACT_MEMBERSHIP_NONSSO if is_sso else CONTRACT_MEMBERSHIP_SSO ) contract.save() @@ -361,7 +363,7 @@ def test_create_b2b_enrollment( # noqa: PLR0913, C901, PLR0915 settings.OPENEDX_SERVICE_WORKER_USERNAME = "a username" contract = factories.ContractPageFactory.create( - integration_type=CONTRACT_INTEGRATION_SSO, + membership_type=CONTRACT_MEMBERSHIP_SSO, enrollment_fixed_price=Decimal(0) if price_is_zero else FAKE.pydecimal(left_digits=2, right_digits=2, positive=True), @@ -520,44 +522,130 @@ def test_create_contract_run(mocker, source_run_exists, run_exists): mocked_clone_run.assert_called() -def test_b2b_reconcile_user_orgs(): +def test_b2b_reconcile_user_orgs(): # noqa: PLR0915 """Test that we can get a list of B2B orgs from somewhere and fix a user's associations.""" - contracts = factories.ContractPageFactory.create_batch( - 2, integration_type=CONTRACT_INTEGRATION_NONSSO - ) - sso_contracts = factories.ContractPageFactory.create_batch( - 2, integration_type=CONTRACT_INTEGRATION_SSO - ) user = UserFactory.create() + organization_to_add = OrganizationPageFactory.create() + organization_to_ignore = OrganizationPageFactory.create() + organization_to_remove = OrganizationPageFactory.create() + weird_organization = OrganizationPageFactory.create(sso_organization_id=None) assert user.b2b_contracts.count() == 0 + assert user.b2b_organizations.count() == 0 - user.b2b_contracts.add(contracts[0]) - user.b2b_contracts.add(contracts[1]) - user.b2b_contracts.add(sso_contracts[0]) - user.save() + # Step 1: pass in an org to a user that's not in anything + # We should get back one addition, which is the org we're adding - assert user.b2b_contracts.count() == 3 + added, removed = reconcile_user_orgs( + user, [organization_to_add.sso_organization_id] + ) - sso_required_org = sso_contracts[1].organization.sso_organization_id + assert added == 1 + assert removed == 0 - added, removed = reconcile_user_orgs(user, [sso_required_org]) + user.refresh_from_db() + assert user.b2b_organizations.count() == 1 + assert user.b2b_organizations.filter(organization=organization_to_add).exists() + assert not user.b2b_organizations.filter( + organization=organization_to_ignore + ).exists() + assert not user.b2b_organizations.filter( + organization=organization_to_remove + ).exists() + + # Step 2: Add an org through a back channel, and then reconcile + # The org should be removed + + UserOrganization.objects.create( + user=user, organization=organization_to_remove, keep_until_seen=False + ) - assert added == 1 + assert user.b2b_organizations.count() == 2 + + added, removed = reconcile_user_orgs( + user, [organization_to_add.sso_organization_id] + ) + + assert added == 0 assert removed == 1 user.refresh_from_db() - assert user.b2b_contracts.count() == 3 - assert ( - user.b2b_contracts.filter( - organization__sso_organization_id=sso_contracts[ - 0 - ].organization.sso_organization_id - ).count() - == 0 + assert user.b2b_organizations.count() == 1 + assert user.b2b_organizations.filter(organization=organization_to_add).exists() + assert not user.b2b_organizations.filter( + organization=organization_to_ignore + ).exists() + assert not user.b2b_organizations.filter( + organization=organization_to_remove + ).exists() + + # Step 3: Add the remove org, but set the flag so it should be kept now. + + UserOrganization.objects.create( + user=user, organization=organization_to_remove, keep_until_seen=True + ) + + assert user.b2b_organizations.count() == 2 + + added, removed = reconcile_user_orgs( + user, [organization_to_add.sso_organization_id] + ) + + assert added == 0 + assert removed == 0 + + user.refresh_from_db() + assert user.b2b_organizations.count() == 2 + assert user.b2b_organizations.filter(organization=organization_to_add).exists() + assert not user.b2b_organizations.filter( + organization=organization_to_ignore + ).exists() + assert user.b2b_organizations.filter(organization=organization_to_remove).exists() + + # Step 3.5: now reconcile with the remove org, we should clear the flag + + added, removed = reconcile_user_orgs( + user, + [ + organization_to_add.sso_organization_id, + organization_to_remove.sso_organization_id, + ], ) + assert added == 0 + assert removed == 0 + + user.refresh_from_db() + assert user.b2b_organizations.count() == 2 + assert user.b2b_organizations.filter(organization=organization_to_add).exists() + assert not user.b2b_organizations.filter( + organization=organization_to_ignore + ).exists() + assert user.b2b_organizations.filter(organization=organization_to_remove).exists() + + # Step 4: add the weird org that doesn't have a UUID + # Legacy non-manged orgs won't have a UUID, so we should leave them alone + + UserOrganization.objects.create( + user=user, organization=weird_organization, keep_until_seen=False + ) + + added, removed = reconcile_user_orgs( + user, + [ + organization_to_add.sso_organization_id, + organization_to_remove.sso_organization_id, + ], + ) + + assert added == 0 + assert removed == 0 + + user.refresh_from_db() + assert user.b2b_organizations.count() == 3 + assert user.b2b_organizations.filter(organization=weird_organization).exists() + @pytest.mark.parametrize( "update_an_org", diff --git a/b2b/constants.py b/b2b/constants.py index 92871c7993..dc5178e821 100644 --- a/b2b/constants.py +++ b/b2b/constants.py @@ -2,14 +2,40 @@ ORG_INDEX_SLUG = "organizations" -CONTRACT_INTEGRATION_SSO = "sso" -CONTRACT_INTEGRATION_SSO_NAME = "SSO" -CONTRACT_INTEGRATION_NONSSO = "non-sso" -CONTRACT_INTEGRATION_NONSSO_NAME = "Non-SSO" +# Old values which will be removed in a future PR +CONTRACT_MEMBERSHIP_SSO = "sso" +CONTRACT_MEMBERSHIP_SSO_NAME = "SSO" +CONTRACT_MEMBERSHIP_NONSSO = "non-sso" +CONTRACT_MEMBERSHIP_NONSSO_NAME = "Non-SSO" +# New values +CONTRACT_MEMBERSHIP_MANAGED = "managed" +CONTRACT_MEMBERSHIP_MANAGED_NAME = "Managed" +CONTRACT_MEMBERSHIP_CODE = "code" +CONTRACT_MEMBERSHIP_CODE_NAME = "Enrollment Code" +CONTRACT_MEMBERSHIP_AUTO = "auto" +CONTRACT_MEMBERSHIP_AUTO_NAME = "Auto Enrollment" -CONTRACT_INTEGRATION_CHOICES = zip( - [CONTRACT_INTEGRATION_SSO, CONTRACT_INTEGRATION_NONSSO], - [CONTRACT_INTEGRATION_SSO_NAME, CONTRACT_INTEGRATION_NONSSO_NAME], +CONTRACT_MEMBERSHIP_AUTOS = [ + CONTRACT_MEMBERSHIP_AUTO, + CONTRACT_MEMBERSHIP_MANAGED, + CONTRACT_MEMBERSHIP_SSO, +] + +CONTRACT_MEMBERSHIP_CHOICES = zip( + [ + CONTRACT_MEMBERSHIP_SSO, + CONTRACT_MEMBERSHIP_NONSSO, + CONTRACT_MEMBERSHIP_MANAGED, + CONTRACT_MEMBERSHIP_CODE, + CONTRACT_MEMBERSHIP_AUTO, + ], + [ + CONTRACT_MEMBERSHIP_SSO_NAME, + CONTRACT_MEMBERSHIP_NONSSO_NAME, + CONTRACT_MEMBERSHIP_MANAGED_NAME, + CONTRACT_MEMBERSHIP_CODE_NAME, + CONTRACT_MEMBERSHIP_AUTO_NAME, + ], ) B2B_RUN_TAG_FORMAT = "{year}_C{contract_id}" diff --git a/b2b/factories.py b/b2b/factories.py index 9541639127..1cf555facf 100644 --- a/b2b/factories.py +++ b/b2b/factories.py @@ -6,7 +6,7 @@ import wagtail_factories from factory import Factory, LazyAttribute, LazyFunction, SubFactory -from b2b.constants import CONTRACT_INTEGRATION_NONSSO, CONTRACT_INTEGRATION_SSO +from b2b.constants import CONTRACT_MEMBERSHIP_NONSSO, CONTRACT_MEMBERSHIP_SSO from b2b.keycloak_admin_dataclasses import ( OrganizationRepresentation, RealmRepresentation, @@ -55,12 +55,11 @@ class ContractPageFactory(wagtail_factories.PageFactory): description = LazyAttribute(lambda _: FAKE.unique.text()) organization = SubFactory(OrganizationPageFactory) parent = LazyAttribute(lambda o: o.organization) - integration_type = LazyFunction( - lambda: CONTRACT_INTEGRATION_NONSSO + membership_type = LazyFunction( + lambda: CONTRACT_MEMBERSHIP_NONSSO if FAKE.boolean() - else CONTRACT_INTEGRATION_SSO + else CONTRACT_MEMBERSHIP_SSO ) - slug = LazyAttribute(lambda _: FAKE.unique.slug()) class Meta: model = ContractPage diff --git a/b2b/management/commands/b2b_contract.py b/b2b/management/commands/b2b_contract.py index 31db33f073..b7aa7281d4 100644 --- a/b2b/management/commands/b2b_contract.py +++ b/b2b/management/commands/b2b_contract.py @@ -6,7 +6,18 @@ from django.core.management import BaseCommand, CommandError from django.db.models import Q +<<<<<<< HEAD from b2b.constants import CONTRACT_INTEGRATION_NONSSO, CONTRACT_INTEGRATION_SSO +======= +from b2b.api import create_contract_run +from b2b.constants import ( + CONTRACT_MEMBERSHIP_AUTO, + CONTRACT_MEMBERSHIP_CODE, + CONTRACT_MEMBERSHIP_MANAGED, + CONTRACT_MEMBERSHIP_NONSSO, + CONTRACT_MEMBERSHIP_SSO, +) +>>>>>>> 0528cf9f (Update integration type field to membership type, update org/contract attachment points, update org membership in Keycloak where approrpiate) from b2b.models import ContractPage, OrganizationIndexPage, OrganizationPage log = logging.getLogger(__name__) @@ -41,14 +52,17 @@ def add_arguments(self, parser): help="The name of the contract.", ) create_parser.add_argument( - "integration_type", + "membership_type", type=str, - help="The type of integration for this contract.", + help="The mmebership type for this contract.", choices=[ - CONTRACT_INTEGRATION_SSO, - CONTRACT_INTEGRATION_NONSSO, + CONTRACT_MEMBERSHIP_SSO, + CONTRACT_MEMBERSHIP_NONSSO, + CONTRACT_MEMBERSHIP_MANAGED, + CONTRACT_MEMBERSHIP_CODE, + CONTRACT_MEMBERSHIP_AUTO, ], - default=CONTRACT_INTEGRATION_NONSSO, + default=CONTRACT_MEMBERSHIP_MANAGED, ) create_parser.add_argument( "--description", @@ -158,7 +172,7 @@ def handle_create(self, *args, **kwargs): # noqa: ARG002 """Handle the create subcommand.""" organization_name = kwargs.pop("organization") contract_name = kwargs.pop("contract_name") - integration_type = kwargs.pop("integration_type") + membership_type = kwargs.pop("membership_type") description = kwargs.pop("description") start_date = kwargs.pop("start") end_date = kwargs.pop("end") @@ -196,7 +210,7 @@ def handle_create(self, *args, **kwargs): # noqa: ARG002 contract = ContractPage( name=contract_name, description=description or "", - integration_type=integration_type, + membership_type=membership_type, organization=org, contract_start=start_date, contract_end=end_date, diff --git a/b2b/management/commands/b2b_list.py b/b2b/management/commands/b2b_list.py index 4cfc01c4d5..1378cefa53 100644 --- a/b2b/management/commands/b2b_list.py +++ b/b2b/management/commands/b2b_list.py @@ -252,7 +252,7 @@ def handle_list_contracts(self, *args, **kwargs): # noqa: ARG002 contract.name, contract.slug, contract.organization.name, - contract.integration_type, + contract.membership_type, contract.contract_start.strftime("%Y-%m-%d\n%H:%M") if contract.contract_start else "", diff --git a/b2b/migrations/0009_rename_integration_type_add_new_types.py b/b2b/migrations/0009_rename_integration_type_add_new_types.py new file mode 100644 index 0000000000..9608243b1e --- /dev/null +++ b/b2b/migrations/0009_rename_integration_type_add_new_types.py @@ -0,0 +1,63 @@ +# Generated by Django 4.2.24 on 2025-09-24 20:45 + +import django.db.models.deletion +from django.db import migrations, models + + +def copy_integration_type_to_membership_type(apps, schema_editor): + """Copy existing values from integration_type to membership_type.""" + ContractPage = apps.get_model("b2b", "ContractPage") + for contract in ContractPage.objects.all(): + if contract.integration_type == "sso": + contract.membership_type = "sso" + elif contract.integration_type == "non-sso": + contract.membership_type = "non-sso" + else: + contract.membership_type = "managed" # Default to managed for other cases + contract.save() + + +def noop(apps, schema_editor): + """No operation (placeholder for reverse migration).""" + + +class Migration(migrations.Migration): + dependencies = [ + ("b2b", "0008_increase_length_on_org_key_field"), + ] + + operations = [ + migrations.AddField( + model_name="contractpage", + name="membership_type", + field=models.CharField( + choices=[ + ("sso", "SSO"), + ("non-sso", "Non-SSO"), + ("managed", "Managed"), + ("code", "Enrollment Code"), + ("auto", "Auto Enrollment"), + ], + default="managed", + help_text="The method to use to manage membership in the contract.", + max_length=255, + ), + ), + migrations.RunPython( + copy_integration_type_to_membership_type, reverse_code=noop + ), + migrations.RemoveField( + model_name="contractpage", + name="integration_type", + ), + migrations.AlterField( + model_name="contractpage", + name="organization", + field=models.ForeignKey( + help_text="The organization that owns this contract.", + on_delete=django.db.models.deletion.PROTECT, + related_name="contracts", + to="b2b.organizationpage", + ), + ), + ] diff --git a/b2b/models.py b/b2b/models.py index 01449ef64b..d98a1ae6a0 100644 --- a/b2b/models.py +++ b/b2b/models.py @@ -12,7 +12,12 @@ from wagtail.fields import RichTextField from wagtail.models import Page -from b2b.constants import CONTRACT_INTEGRATION_CHOICES, ORG_INDEX_SLUG +from b2b.constants import ( + CONTRACT_MEMBERSHIP_AUTOS, + CONTRACT_MEMBERSHIP_CHOICES, + CONTRACT_MEMBERSHIP_MANAGED, + ORG_INDEX_SLUG, +) from b2b.exceptions import TargetCourseRunExistsError from b2b.tasks import queue_enrollment_code_check @@ -109,6 +114,55 @@ def get_learners(self): .distinct() ) + def attach_user(self, user): + """ + Attach the given user to the org in Keycloak. + + Args: + - user (User): the user to add to the org + Returns: + - bool: success flag + """ + + from b2b.api import add_user_org_membership + + return add_user_org_membership(self, user) + + def add_user_contracts(self, user): + """ + Add contracts that the user should get automatically to the user. + + Args: + - user (User): the user to add contracts to + Returns: + - int: number of contracts added + """ + + contracts_qs = self.contracts.filter( + membership_type__in=CONTRACT_MEMBERSHIP_AUTOS, active=True + ) + + for contract in contracts_qs.all(): + user.b2b_contracts.add(contract) + + user.save() + + return contracts_qs.count() + + def remove_user_contracts(self, user): + """ + Remove managed contracts from the given user. + + Args: + - user (User): the user to remove contracts from + Returns: + - int: number of contracts removed + """ + + return user.b2b_contracts.through.objects.filter( + contractpage_id__in=self.contracts.all().values_list("id", flat=True) + ).delete() + def __str__(self): """Return a reasonable representation of the org as a string.""" @@ -135,16 +189,17 @@ class ContractPage(Page): description = RichTextField( blank=True, help_text="Any useful extra information about the contract." ) - integration_type = models.CharField( + membership_type = models.CharField( max_length=255, - choices=CONTRACT_INTEGRATION_CHOICES, - help_text="The type of integration for this contract.", + choices=CONTRACT_MEMBERSHIP_CHOICES, + help_text="The method to use to manage membership in the contract.", + default=CONTRACT_MEMBERSHIP_MANAGED, ) organization = models.ForeignKey( OrganizationPage, on_delete=models.PROTECT, related_name="contracts", - help_text="The organization this contract is with.", + help_text="The organization that owns this contract.", ) contract_start = models.DateField( blank=True, @@ -190,7 +245,7 @@ class ContractPage(Page): ), MultiFieldPanel( [ - FieldPanel("integration_type"), + FieldPanel("membership_type"), FieldPanel("max_learners"), FieldPanel("enrollment_fixed_price"), ], @@ -231,7 +286,7 @@ def save(self, clean=True, user=None, log_action=False, **kwargs): # noqa: FBT0 self.title = str(self.name) - self.slug = slugify(f"contract-{self.organization.id}-{self.id}") + self.slug = slugify(f"contract-{self.organization.id}-{self.title}") Page.save(self, clean=clean, user=user, log_action=log_action, **kwargs) queue_enrollment_code_check.delay(self.id) diff --git a/b2b/serializers/v0/__init__.py b/b2b/serializers/v0/__init__.py index e1b68c11d7..2865c5453a 100644 --- a/b2b/serializers/v0/__init__.py +++ b/b2b/serializers/v0/__init__.py @@ -17,7 +17,7 @@ class Meta: "id", "name", "description", - "integration_type", + "membership_type", "organization", "contract_start", "contract_end", @@ -29,7 +29,7 @@ class Meta: "id", "name", "description", - "integration_type", + "membership_type", "organization", "contract_start", "contract_end", diff --git a/b2b/views/v0/__init__.py b/b2b/views/v0/__init__.py index 62bd0d1b36..6478fae9c4 100644 --- a/b2b/views/v0/__init__.py +++ b/b2b/views/v0/__init__.py @@ -27,6 +27,7 @@ from main.authentication import CsrfExemptSessionAuthentication from main.constants import USER_MSG_TYPE_B2B_ENROLL_SUCCESS from main.permissions import IsAdminOrReadOnly +from users.models import UserOrganization class OrganizationPageViewSet(viewsets.ReadOnlyModelViewSet): @@ -141,6 +142,9 @@ def post(self, request, enrollment_code: str, format=None): # noqa: A002, ARG00 if contract.is_full(): continue + UserOrganization.process_add_membership( + request.user, contract.organization, keep_until_seen=True + ) request.user.b2b_contracts.add(contract) DiscountContractAttachmentRedemption.objects.create( user=request.user, discount=code, contract=contract diff --git a/b2b/views/v0/views_test.py b/b2b/views/v0/views_test.py index 3d11079408..8466e5f94a 100644 --- a/b2b/views/v0/views_test.py +++ b/b2b/views/v0/views_test.py @@ -11,7 +11,7 @@ from rest_framework.test import APIClient from b2b.api import create_contract_run, ensure_enrollment_codes_exist -from b2b.constants import CONTRACT_INTEGRATION_NONSSO, CONTRACT_INTEGRATION_SSO +from b2b.constants import CONTRACT_MEMBERSHIP_NONSSO, CONTRACT_MEMBERSHIP_SSO from b2b.factories import ContractPageFactory from b2b.models import DiscountContractAttachmentRedemption from courses.factories import CourseRunFactory @@ -43,7 +43,7 @@ def test_b2b_contract_attachment(user): """Ensure a supplied code results in attachment for the user.""" contract = ContractPageFactory.create( - integration_type=CONTRACT_INTEGRATION_NONSSO, + membership_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=10, ) @@ -82,7 +82,7 @@ def test_b2b_contract_attachment_invalid_code_dates(user, bad_start_or_end): """Test that the attachment fails properly if the code has invalid dates.""" contract = ContractPageFactory.create( - integration_type=CONTRACT_INTEGRATION_NONSSO, + membership_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=1, ) @@ -135,7 +135,7 @@ def test_b2b_contract_attachment_invalid_contract_dates(user, bad_start_or_end): """Test that the attachment fails properly if the contract has invalid dates.""" contract = ContractPageFactory.create( - integration_type=CONTRACT_INTEGRATION_NONSSO, + membership_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=1, ) @@ -181,7 +181,7 @@ def test_b2b_contract_attachment_full_contract(): """Test that the attachment fails properly if the contract is full.""" contract = ContractPageFactory.create( - integration_type=CONTRACT_INTEGRATION_NONSSO, + membership_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=1, ) @@ -234,7 +234,7 @@ def test_b2b_enroll(mocker, settings, user_has_edx_user, has_price): settings.OPENEDX_SERVICE_WORKER_API_TOKEN = "a token" # noqa: S105 contract = ContractPageFactory.create( - integration_type=CONTRACT_INTEGRATION_SSO, + membership_type=CONTRACT_MEMBERSHIP_SSO, enrollment_fixed_price=100 if has_price else 0, ) source_courserun = CourseRunFactory.create(is_source_run=True) diff --git a/courses/api.py b/courses/api.py index 56d7f664ce..bbdb6507d6 100644 --- a/courses/api.py +++ b/courses/api.py @@ -78,6 +78,7 @@ NoEdxApiAuthError, UnknownEdxApiEnrollException, ) +from users.models import UserOrganization if TYPE_CHECKING: from django.db.models.query import QuerySet @@ -214,8 +215,11 @@ def _enroll_learner_into_associated_programs(): _enroll_learner_into_associated_programs() # If the run is associated with a B2B contract, add the contract - # to the user's contract list + # to the user's contract list and update their org memberships if run.b2b_contract: + UserOrganization.process_add_membership( + user, run.b2b_contract.organization, keep_until_seen=True + ) user.b2b_contracts.add(run.b2b_contract) user.save() diff --git a/courses/api_test.py b/courses/api_test.py index 7f4b3d9b1c..b92234a378 100644 --- a/courses/api_test.py +++ b/courses/api_test.py @@ -17,7 +17,7 @@ from reversion.models import Version from b2b.api import create_b2b_enrollment -from b2b.constants import CONTRACT_INTEGRATION_NONSSO +from b2b.constants import CONTRACT_MEMBERSHIP_NONSSO from b2b.factories import ( ContractPageFactory, OrganizationIndexPageFactory, @@ -1807,7 +1807,7 @@ def test_b2b_re_enrollment_after_multiple_unenrollments(mocker, user): contract = ContractPageFactory.create( organization=org, enrollment_fixed_price=Decimal("0.00"), - integration_type=CONTRACT_INTEGRATION_NONSSO, + membership_type=CONTRACT_MEMBERSHIP_NONSSO, ) course_run = CourseRunFactory.create(b2b_contract=contract) with reversion.create_revision(): diff --git a/courses/views/v2/__init__.py b/courses/views/v2/__init__.py index d5d5ce90bd..efde9a1239 100644 --- a/courses/views/v2/__init__.py +++ b/courses/views/v2/__init__.py @@ -71,7 +71,7 @@ def user_has_org_access(user, org_id): user and user.is_authenticated and org_id - and user.b2b_organizations.filter(id=org_id).exists() + and user.b2b_organizations.filter(organization__id=org_id).exists() ) diff --git a/courses/views/v2/views_test.py b/courses/views/v2/views_test.py index 62f922492b..5d168ca4a2 100644 --- a/courses/views/v2/views_test.py +++ b/courses/views/v2/views_test.py @@ -310,7 +310,10 @@ def test_filter_with_org_id_returns_contracted_course( org = OrganizationPageFactory(name="Test Org") contract = ContractPageFactory(organization=org, active=True) user = UserFactory() + user.b2b_organizations.create(organization=org) user.b2b_contracts.add(contract) + user.refresh_from_db() + (course, _) = contract_ready_course create_contract_run(contract, course) @@ -498,7 +501,10 @@ def test_next_run_id_with_org_filter( # noqa: PLR0915 contract = ContractPageFactory.create(organization=orgs[0]) second_contract = ContractPageFactory.create(organization=orgs[1]) test_user = UserFactory() + test_user.b2b_organizations.create(organization=contract.organization) test_user.b2b_contracts.add(contract) + test_user.save() + test_user.refresh_from_db() auth_api_client = APIClient() auth_api_client.force_authenticate(user=test_user) @@ -637,6 +643,7 @@ def test_program_filter_for_b2b_org(user, mock_course_run_clone): contract.add_program_courses(b2b_program) contract.save() + user.b2b_organizations.create(organization=org) user.b2b_contracts.add(contract) user.save() diff --git a/users/admin.py b/users/admin.py index 352f0d0a38..0398c1f0d2 100644 --- a/users/admin.py +++ b/users/admin.py @@ -12,7 +12,7 @@ from mitol.common.admin import TimestampedModelAdmin from openedx.models import OpenEdxUser -from users.models import BlockList, LegalAddress, User, UserProfile +from users.models import BlockList, LegalAddress, User, UserOrganization, UserProfile class UserLegalAddressInline(admin.StackedInline): @@ -109,6 +109,13 @@ class UserContractPageInline(admin.TabularInline): extra = 0 +class UserOrganizationInline(admin.TabularInline): + """Inline to allow modifying the contracts associated with a user""" + + model = UserOrganization + extra = 0 + + @admin.register(User) class UserAdmin( DjangoObjectActions, ContribUserAdmin, HijackUserAdminMixin, TimestampedModelAdmin @@ -165,6 +172,7 @@ class UserAdmin( OpenEdxUserInline, UserLegalAddressInline, UserProfileInline, + UserOrganizationInline, UserContractPageInline, ] diff --git a/users/migrations/0038_add_fk_from_users_to_organizationpage.py b/users/migrations/0038_add_fk_from_users_to_organizationpage.py new file mode 100644 index 0000000000..c084577916 --- /dev/null +++ b/users/migrations/0038_add_fk_from_users_to_organizationpage.py @@ -0,0 +1,55 @@ +# Generated by Django 4.2.24 on 2025-09-24 19:44 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("b2b", "0008_increase_length_on_org_key_field"), + ("users", "0037_add_global_id_default"), + ] + + operations = [ + migrations.CreateModel( + name="UserOrganization", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "keep_until_seen", + models.BooleanField( + default=False, + help_text="If True, the user will be kept in the organization until the organization is seen in their SSO data.", + ), + ), + ( + "organization", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="organization_users", + to="b2b.organizationpage", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="b2b_organizations", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "unique_together": {("user", "organization")}, + }, + ), + ] diff --git a/users/models.py b/users/models.py index 8cb8a712d6..c93ecc1080 100644 --- a/users/models.py +++ b/users/models.py @@ -1,5 +1,6 @@ """User models""" +import logging import uuid from datetime import timedelta from functools import cached_property @@ -18,11 +19,12 @@ from mitol.common.utils import now_in_utc from mitol.common.utils.collections import chunks -from b2b.models import OrganizationPage from cms.constants import CMS_EDITORS_GROUP_NAME from openedx.constants import OPENEDX_REPAIR_GRACE_PERIOD_MINS, OPENEDX_USERNAME_MAX_LEN from openedx.models import OpenEdxUser +log = logging.getLogger(__name__) + MALE = "m" FEMALE = "f" OTHER = "o" @@ -362,20 +364,13 @@ def is_editor(self) -> bool: or self.groups.filter(name=CMS_EDITORS_GROUP_NAME).exists() ) - @cached_property - def b2b_organizations(self): - """Return the organizations the user is associated with.""" - return OrganizationPage.objects.filter( - pk__in=self.b2b_contracts.values_list("organization", flat=True).distinct() - ).all() - @cached_property def b2b_organization_sso_ids(self): """Similar to b2b_organizations, but returns just the UUIDs.""" return list( self.b2b_organizations.filter( sso_organization_id__isnull=False - ).values_list("sso_organization_id", flat=True) + ).values_list("organization__sso_organization_id", flat=True) ) @property @@ -601,3 +596,80 @@ class BlockList(TimestampedModel): """A user's blocklist model""" hashed_email = models.CharField(max_length=128) + + +class UserOrganization(models.Model): + """The user's organizations memberships.""" + + user = models.ForeignKey( + User, on_delete=models.CASCADE, related_name="b2b_organizations" + ) + organization = models.ForeignKey( + "b2b.OrganizationPage", + on_delete=models.CASCADE, + related_name="organization_users", + ) + keep_until_seen = models.BooleanField( + default=False, + help_text="If True, the user will be kept in the organization until the organization is seen in their SSO data.", + ) + + class Meta: + unique_together = ("user", "organization") + + def __str__(self): + """Return a reasonable representation of the object as a string.""" + + return f"UserOrganization: {self.user} in {self.organization}" + + @staticmethod + def process_add_membership(user, organization, *, keep_until_seen=False): + """ + Add a user to an org, and kick off contract processing. + + This allows us to manage UserOrganization records without necessarily + being forced to process contract memberships at the same time. + + Args: + - user (users.models.User): the user to add + - organization (b2b.models.OrganizationPage): the organization to add the user to + - keep_until_seen (bool): if True, the user will be kept in the org until the + organization is seen in their SSO data. + """ + + obj, created = UserOrganization.objects.get_or_create( + user=user, + organization=organization, + ) + if created: + obj.keep_until_seen = keep_until_seen + obj.save() + try: + organization.attach_user(user) + except ConnectionError: + log.exception( + "Could not attach %s to Keycloak org for %s", user, organization + ) + organization.add_user_contracts(user) + + return obj + + @staticmethod + def process_remove_membership(user, organization): + """ + Remove a user from an org, and kick off contract processing. + + Other side of the process_add_membership function - removes the membership + and associated managed contracts. + + Args: + - user (users.models.User): the user to remove + - organization (b2b.models.OrganizationPage): the organization to remove the user from + """ + + organization.remove_user_contracts(user) + + UserOrganization.objects.filter( + user=user, + organization=organization, + ).get().delete() diff --git a/users/models_test.py b/users/models_test.py index d6dcba0b29..602adbc8c5 100644 --- a/users/models_test.py +++ b/users/models_test.py @@ -9,7 +9,9 @@ from django.core.exceptions import ValidationError from django.db import transaction +from b2b.factories import ContractPageFactory, OrganizationPageFactory from cms.constants import CMS_EDITORS_GROUP_NAME +from courses.factories import CourseRunEnrollmentFactory, CourseRunFactory from openedx.factories import OpenEdxApiAuthFactory, OpenEdxUserFactory from openedx.models import OpenEdxUser from users.factories import UserFactory @@ -18,11 +20,19 @@ OPENEDX_HIGHEST_EDUCATION_MAPPINGS, LegalAddress, User, + UserOrganization, ) pytestmark = pytest.mark.django_db +@pytest.fixture +def mocked_b2b_org_attach(mocker): + """Mock the org attachment call.""" + + return mocker.patch("b2b.api.add_user_org_membership", return_value=True) + + @pytest.mark.parametrize( "create_func,exp_staff,exp_superuser,exp_is_active", # noqa: PT006 [ @@ -211,3 +221,185 @@ def test_user_profile_edx_properties(should_exist): if should_exist else user.legal_address.edx_us_state is None ) + + +def test_user_add_b2b_org(mocked_b2b_org_attach): + """Ensure adding a user to an organization works as expected.""" + + orgs = OrganizationPageFactory.create_batch(2) + user = UserFactory.create() + + # New-style ones + contract_auto = ContractPageFactory.create( + organization=orgs[0], + membership_type="auto", + title="Contract Auto", + name="Contract Auto", + ) + contract_managed = ContractPageFactory.create( + organization=orgs[0], + membership_type="managed", + title="Contract Managed", + name="Contract Managed", + ) + contract_code = ContractPageFactory.create( + organization=orgs[0], + membership_type="code", + title="Contract Enrollment Code", + name="Contract Enrollment Code", + ) + # Legacy ones - these will migrate to "managed" and "code" + contract_sso = ContractPageFactory.create( + organization=orgs[0], + membership_type="sso", + title="Contract SSO", + name="Contract SSO", + ) + contract_non_sso = ContractPageFactory.create( + organization=orgs[0], + membership_type="non-sso", + title="Contract NonSSO", + name="Contract NonSSO", + ) + + UserOrganization.process_add_membership(user, orgs[0]) + + # We should now be in the SSO, auto, and managed contracts + # but not the other two. + + user.refresh_from_db() + assert user.b2b_contracts.count() == 3 + assert user.b2b_organizations.filter(organization=orgs[0]).exists() + assert ( + user.b2b_contracts.filter( + pk__in=[ + contract_auto.id, + contract_sso.id, + contract_managed.id, + ] + ).count() + == 3 + ) + assert ( + user.b2b_contracts.filter( + pk__in=[ + contract_code.id, + contract_non_sso.id, + ] + ).count() + == 0 + ) + + +def test_user_remove_b2b_org(mocked_b2b_org_attach): + """Ensure removing a user from an org also clears the appropriate contracts.""" + + orgs = OrganizationPageFactory.create_batch(2) + user = UserFactory.create() + + # New-style ones + contract_auto = ContractPageFactory.create( + organization=orgs[0], + membership_type="auto", + title="Contract Auto", + name="Contract Auto", + ) + contract_managed = ContractPageFactory.create( + organization=orgs[0], + membership_type="managed", + title="Contract Managed", + name="Contract Managed", + ) + contract_code = ContractPageFactory.create( + organization=orgs[1], + membership_type="code", + title="Contract Enrollment Code", + name="Contract Enrollment Code", + ) + # Legacy ones - these will migrate to "managed" and "code" + contract_sso = ContractPageFactory.create( + organization=orgs[0], + membership_type="sso", + title="Contract SSO", + name="Contract SSO", + ) + contract_non_sso = ContractPageFactory.create( + organization=orgs[1], + membership_type="non-sso", + title="Contract NonSSO", + name="Contract NonSSO", + ) + + managed_ids = [ + contract_auto.id, + contract_managed.id, + contract_sso.id, + ] + unmanaged_ids = [ + contract_code.id, + contract_non_sso.id, + ] + + UserOrganization.process_add_membership(user, orgs[0]) + UserOrganization.process_add_membership(user, orgs[1]) + + user.b2b_contracts.add(contract_code) + user.b2b_contracts.add(contract_non_sso) + user.save() + + user.refresh_from_db() + + assert user.b2b_contracts.count() == 5 + + UserOrganization.process_remove_membership(user, orgs[1]) + + assert user.b2b_contracts.filter(id__in=managed_ids).count() == 3 + assert user.b2b_contracts.filter(id__in=unmanaged_ids).count() == 0 + + UserOrganization.process_remove_membership(user, orgs[0]) + + # we should have no contracts now since we're no longer in any orgs + + assert user.b2b_contracts.count() == 0 + + +def test_b2b_contract_removal_keeps_enrollments(mocked_b2b_org_attach): + """Ensure that removing a user from a B2B contract leaves their enrollments alone.""" + + org = OrganizationPageFactory.create() + user = UserFactory.create() + + contract_auto = ContractPageFactory.create( + organization=org, + membership_type="auto", + title="Contract Auto", + name="Contract Auto", + ) + + courserun = CourseRunFactory.create(b2b_contract=contract_auto) + + UserOrganization.process_add_membership(user, org) + + CourseRunEnrollmentFactory( + user=user, + run=courserun, + ) + + user.refresh_from_db() + + assert courserun.enrollments.filter(user=user).count() == 1 + + UserOrganization.process_remove_membership(user, org) + + assert courserun.enrollments.filter(user=user).count() == 1 + + +def test_b2b_org_attach_calls_keycloak(mocked_b2b_org_attach): + """Test that attaching a user to an org calls Keycloak successfully.""" + + org = OrganizationPageFactory.create() + user = UserFactory.create() + + UserOrganization.process_add_membership(user, org) + + mocked_b2b_org_attach.assert_called() diff --git a/users/serializers.py b/users/serializers.py index ace4f925b0..7f30620c9b 100644 --- a/users/serializers.py +++ b/users/serializers.py @@ -11,7 +11,7 @@ from rest_framework import serializers from social_django.models import UserSocialAuth -from b2b.serializers.v0 import ContractPageSerializer, OrganizationPageSerializer +from b2b.serializers.v0 import ContractPageSerializer from hubspot_sync.task_helpers import sync_hubspot_user # from ecommerce.api import fetch_and_serialize_unused_coupons # noqa: ERA001 @@ -22,7 +22,13 @@ from openedx.exceptions import EdxApiRegistrationValidationException from openedx.models import OpenEdxUser from openedx.tasks import change_edx_user_email_async -from users.models import ChangeEmailRequest, LegalAddress, User, UserProfile +from users.models import ( + ChangeEmailRequest, + LegalAddress, + User, + UserOrganization, + UserProfile, +) log = logging.getLogger() @@ -202,15 +208,22 @@ class Meta: ) -class UserOrganizationSerializer(OrganizationPageSerializer): +class UserOrganizationSerializer(serializers.ModelSerializer): """ Serializer for user organization data. - Slightly different from the OrganizationPageSerializer; we only need - the user's orgs and contracts. + Return the user's organizations in a manner that makes them look like + OrganizationPage objects. (Previously, the user organizations were a queryset + of OrganizationPages that related to the user, but now we have a through + table.) """ contracts = serializers.SerializerMethodField() + id = serializers.SerializerMethodField() + name = serializers.SerializerMethodField() + description = serializers.SerializerMethodField() + logo = serializers.SerializerMethodField() + slug = serializers.SerializerMethodField() @extend_schema_field(ContractPageSerializer(many=True)) def get_contracts(self, instance): @@ -218,15 +231,56 @@ def get_contracts(self, instance): contracts = ( self.context["user"] .b2b_contracts.filter( - organization=instance, + organization=instance.organization, ) .all() ) return ContractPageSerializer(contracts, many=True).data - class Meta(OrganizationPageSerializer.Meta): - fields = (*OrganizationPageSerializer.Meta.fields, "contracts") - read_only_fields = (*OrganizationPageSerializer.Meta.fields, "contracts") + @extend_schema_field(int) + def get_id(self, instance): + """Get id""" + return instance.organization.id + + @extend_schema_field(str) + def get_name(self, instance): + """Get name""" + return instance.organization.name + + @extend_schema_field(str) + def get_description(self, instance): + """Get description""" + return instance.organization.description + + def get_logo(self, instance): + """Get logo""" + return instance.organization.logo if instance.organization.logo else None + + @extend_schema_field(str) + def get_slug(self, instance): + """Get slug""" + return instance.organization.slug + + class Meta: + """Meta opts for the serializer.""" + + model = UserOrganization + fields = [ + "id", + "name", + "description", + "logo", + "slug", + "contracts", + ] + read_only_fields = [ + "id", + "name", + "description", + "logo", + "slug", + "contracts", + ] class UserSerializer(serializers.ModelSerializer): diff --git a/users/views_test.py b/users/views_test.py index 6565eab85d..cd52af411a 100644 --- a/users/views_test.py +++ b/users/views_test.py @@ -58,6 +58,7 @@ def test_get_user_by_me(mocker, client, user, is_anonymous, has_orgs): if has_orgs: contract = ContractPageFactory.create() + user.b2b_organizations.create(organization=contract.organization) user.b2b_contracts.add(contract) user.save() b2b_orgs = [ @@ -72,7 +73,7 @@ def test_get_user_by_me(mocker, client, user, is_anonymous, has_orgs): "id": contract.id, "name": contract.name, "description": contract.description, - "integration_type": contract.integration_type, + "membership_type": contract.membership_type, "contract_start": None, "contract_end": None, "active": True, From 888d29430fec6370816cb30a2b40e41de16f39ae Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 2 Oct 2025 08:14:52 -0500 Subject: [PATCH 02/15] Fix config for pytest --- pytest.ini | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pytest.ini b/pytest.ini index 599f8a5198..704d288773 100644 --- a/pytest.ini +++ b/pytest.ini @@ -15,6 +15,10 @@ env = KEYCLOAK_BASE_URL=http://keycloak/ KEYCLOAK_REALM_NAME=ol-test KEYCLOAK_CLIENT_ID=mitxonline + KEYCLOAK_DISCOVERY_URL=https://keycloak/realms/ol-local/.well-known/openid-configuration + KEYCLOAK_ADMIN_CLIENT_ID=apisix + KEYCLOAK_ADMIN_CLIENT_SCOPES="openid profile email" + KEYCLOAK_ADMIN_CLIENT_NO_VERIFY_SSL=True LOGOUT_REDIRECT_URL=https://openedx.odl.local/logout MAILGUN_KEY=invalid-key MAILGUN_SENDER_DOMAIN=localhost From 817747a420ee499fdf2d0a9a279d7dd0c28b3a39 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 2 Oct 2025 11:22:27 -0500 Subject: [PATCH 03/15] fix another configuration item, update two of these tests to use mocks and check for b2b attachment better --- b2b/views/v0/views_test.py | 21 +++++++++++++++++++-- pytest.ini | 1 + 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/b2b/views/v0/views_test.py b/b2b/views/v0/views_test.py index 8466e5f94a..2a4432f06a 100644 --- a/b2b/views/v0/views_test.py +++ b/b2b/views/v0/views_test.py @@ -39,9 +39,13 @@ def test_b2b_contract_attachment_bad_code(user): assert user.b2b_contracts.count() == 0 -def test_b2b_contract_attachment(user): +def test_b2b_contract_attachment(mocker, user): """Ensure a supplied code results in attachment for the user.""" + mocked_attach_user = mocker.patch( + "b2b.models.OrganizationPage.attach_user", return_value=True + ) + contract = ContractPageFactory.create( membership_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=10, @@ -62,8 +66,10 @@ def test_b2b_contract_attachment(user): resp = client.post(url) assert resp.status_code == 200 + mocked_attach_user.assert_called() user.refresh_from_db() + assert user.b2b_organizations.filter(organization=contract.organization).exists() assert user.b2b_contracts.filter(pk=contract.id).exists() assert DiscountContractAttachmentRedemption.objects.filter( @@ -177,9 +183,13 @@ def test_b2b_contract_attachment_invalid_contract_dates(user, bad_start_or_end): ).exists() -def test_b2b_contract_attachment_full_contract(): +def test_b2b_contract_attachment_full_contract(mocker): """Test that the attachment fails properly if the contract is full.""" + mocked_attach_user = mocker.patch( + "b2b.models.OrganizationPage.attach_user", return_value=True + ) + contract = ContractPageFactory.create( membership_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=1, @@ -201,6 +211,7 @@ def test_b2b_contract_attachment_full_contract(): resp = client.post(url) assert resp.status_code == 200 + mocked_attach_user.assert_called() user.refresh_from_db() assert user.b2b_contracts.filter(pk=contract.id).exists() @@ -209,14 +220,20 @@ def test_b2b_contract_attachment_full_contract(): client = APIClient() client.force_login(user) + mocked_attach_user.reset_mock() + url = reverse( "b2b:attach-user", kwargs={"enrollment_code": contract_code.discount_code} ) resp = client.post(url) assert resp.status_code == 200 + mocked_attach_user.assert_not_called() user.refresh_from_db() + assert not user.b2b_organizations.filter( + organization=contract.organization + ).exists() assert not user.b2b_contracts.filter(pk=contract.id).exists() assert not DiscountContractAttachmentRedemption.objects.filter( contract=contract, user=user, discount=contract_code diff --git a/pytest.ini b/pytest.ini index 704d288773..0611aed054 100644 --- a/pytest.ini +++ b/pytest.ini @@ -17,6 +17,7 @@ env = KEYCLOAK_CLIENT_ID=mitxonline KEYCLOAK_DISCOVERY_URL=https://keycloak/realms/ol-local/.well-known/openid-configuration KEYCLOAK_ADMIN_CLIENT_ID=apisix + KEYCLOAK_ADMIN_CLIENT_SECRET=fake_admin_secret KEYCLOAK_ADMIN_CLIENT_SCOPES="openid profile email" KEYCLOAK_ADMIN_CLIENT_NO_VERIFY_SSL=True LOGOUT_REDIRECT_URL=https://openedx.odl.local/logout From 46b91aa350356e08367a09a3ae2c93f5c89a8bd5 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 2 Oct 2025 11:36:26 -0500 Subject: [PATCH 04/15] forgot to flesh out the org logo field; regenerate the openapi schema since things have changed a bit --- openapi/specs/v0.yaml | 62 ++++++++++++++++++++++++++----------------- openapi/specs/v1.yaml | 62 ++++++++++++++++++++++++++----------------- openapi/specs/v2.yaml | 62 ++++++++++++++++++++++++++----------------- users/serializers.py | 11 +++++++- 4 files changed, 121 insertions(+), 76 deletions(-) diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 79bba4c18c..5ba87fcf31 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -1510,19 +1510,22 @@ components: type: string readOnly: true description: Any useful extra information about the contract. - integration_type: + membership_type: allOf: - - $ref: '#/components/schemas/IntegrationTypeEnum' + - $ref: '#/components/schemas/MembershipTypeEnum' readOnly: true description: |- - The type of integration for this contract. + The method to use to manage membership in the contract. * `sso` - SSO * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment organization: type: integer readOnly: true - description: The organization this contract is with. + description: The organization that owns this contract. contract_start: type: string format: date @@ -1550,7 +1553,7 @@ components: - contract_start - description - id - - integration_type + - membership_type - name - organization - slug @@ -2534,17 +2537,6 @@ components: - Elementary/primary school - No formal education - Other education - IntegrationTypeEnum: - enum: - - sso - - non-sso - type: string - description: |- - * `sso` - SSO - * `non-sso` - Non-SSO - x-enum-descriptions: - - SSO - - Non-SSO LearnerProgramRecordShare: type: object properties: @@ -2648,6 +2640,26 @@ components: maxLength: 10 required: - country + MembershipTypeEnum: + enum: + - sso + - non-sso + - managed + - code + - auto + type: string + description: |- + * `sso` - SSO + * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment + x-enum-descriptions: + - SSO + - Non-SSO + - Managed + - Enrollment Code + - Auto Enrollment NullEnum: enum: - null @@ -3602,31 +3614,31 @@ components: description: |- Serializer for user organization data. - Slightly different from the OrganizationPageSerializer; we only need - the user's orgs and contracts. + Return the user's organizations in a manner that makes them look like + OrganizationPage objects. (Previously, the user organizations were a queryset + of OrganizationPages that related to the user, but now we have a through + table.) properties: id: type: integer + description: Get id readOnly: true name: type: string + description: Get name readOnly: true - description: The name of the organization description: type: string + description: Get description readOnly: true - description: Any useful extra information about the organization logo: type: string - format: uri + description: Get logo readOnly: true - description: The organization's logo. Will be displayed in the app in various - places. slug: type: string + description: Get slug readOnly: true - description: The name of the page as it will appear in URLs e.g http://domain.com/blog/[my-slug]/ - pattern: ^[-\w]+$ contracts: type: array items: diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 74da736ec6..652f6739d9 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -1510,19 +1510,22 @@ components: type: string readOnly: true description: Any useful extra information about the contract. - integration_type: + membership_type: allOf: - - $ref: '#/components/schemas/IntegrationTypeEnum' + - $ref: '#/components/schemas/MembershipTypeEnum' readOnly: true description: |- - The type of integration for this contract. + The method to use to manage membership in the contract. * `sso` - SSO * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment organization: type: integer readOnly: true - description: The organization this contract is with. + description: The organization that owns this contract. contract_start: type: string format: date @@ -1550,7 +1553,7 @@ components: - contract_start - description - id - - integration_type + - membership_type - name - organization - slug @@ -2534,17 +2537,6 @@ components: - Elementary/primary school - No formal education - Other education - IntegrationTypeEnum: - enum: - - sso - - non-sso - type: string - description: |- - * `sso` - SSO - * `non-sso` - Non-SSO - x-enum-descriptions: - - SSO - - Non-SSO LearnerProgramRecordShare: type: object properties: @@ -2648,6 +2640,26 @@ components: maxLength: 10 required: - country + MembershipTypeEnum: + enum: + - sso + - non-sso + - managed + - code + - auto + type: string + description: |- + * `sso` - SSO + * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment + x-enum-descriptions: + - SSO + - Non-SSO + - Managed + - Enrollment Code + - Auto Enrollment NullEnum: enum: - null @@ -3602,31 +3614,31 @@ components: description: |- Serializer for user organization data. - Slightly different from the OrganizationPageSerializer; we only need - the user's orgs and contracts. + Return the user's organizations in a manner that makes them look like + OrganizationPage objects. (Previously, the user organizations were a queryset + of OrganizationPages that related to the user, but now we have a through + table.) properties: id: type: integer + description: Get id readOnly: true name: type: string + description: Get name readOnly: true - description: The name of the organization description: type: string + description: Get description readOnly: true - description: Any useful extra information about the organization logo: type: string - format: uri + description: Get logo readOnly: true - description: The organization's logo. Will be displayed in the app in various - places. slug: type: string + description: Get slug readOnly: true - description: The name of the page as it will appear in URLs e.g http://domain.com/blog/[my-slug]/ - pattern: ^[-\w]+$ contracts: type: array items: diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index 181b69c01d..24f65db009 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -1510,19 +1510,22 @@ components: type: string readOnly: true description: Any useful extra information about the contract. - integration_type: + membership_type: allOf: - - $ref: '#/components/schemas/IntegrationTypeEnum' + - $ref: '#/components/schemas/MembershipTypeEnum' readOnly: true description: |- - The type of integration for this contract. + The method to use to manage membership in the contract. * `sso` - SSO * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment organization: type: integer readOnly: true - description: The organization this contract is with. + description: The organization that owns this contract. contract_start: type: string format: date @@ -1550,7 +1553,7 @@ components: - contract_start - description - id - - integration_type + - membership_type - name - organization - slug @@ -2534,17 +2537,6 @@ components: - Elementary/primary school - No formal education - Other education - IntegrationTypeEnum: - enum: - - sso - - non-sso - type: string - description: |- - * `sso` - SSO - * `non-sso` - Non-SSO - x-enum-descriptions: - - SSO - - Non-SSO LearnerProgramRecordShare: type: object properties: @@ -2648,6 +2640,26 @@ components: maxLength: 10 required: - country + MembershipTypeEnum: + enum: + - sso + - non-sso + - managed + - code + - auto + type: string + description: |- + * `sso` - SSO + * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment + x-enum-descriptions: + - SSO + - Non-SSO + - Managed + - Enrollment Code + - Auto Enrollment NullEnum: enum: - null @@ -3602,31 +3614,31 @@ components: description: |- Serializer for user organization data. - Slightly different from the OrganizationPageSerializer; we only need - the user's orgs and contracts. + Return the user's organizations in a manner that makes them look like + OrganizationPage objects. (Previously, the user organizations were a queryset + of OrganizationPages that related to the user, but now we have a through + table.) properties: id: type: integer + description: Get id readOnly: true name: type: string + description: Get name readOnly: true - description: The name of the organization description: type: string + description: Get description readOnly: true - description: Any useful extra information about the organization logo: type: string - format: uri + description: Get logo readOnly: true - description: The organization's logo. Will be displayed in the app in various - places. slug: type: string + description: Get slug readOnly: true - description: The name of the page as it will appear in URLs e.g http://domain.com/blog/[my-slug]/ - pattern: ^[-\w]+$ contracts: type: array items: diff --git a/users/serializers.py b/users/serializers.py index 7f30620c9b..1a2a04e6e8 100644 --- a/users/serializers.py +++ b/users/serializers.py @@ -12,6 +12,7 @@ from social_django.models import UserSocialAuth from b2b.serializers.v0 import ContractPageSerializer +from cms.api import get_wagtail_img_src from hubspot_sync.task_helpers import sync_hubspot_user # from ecommerce.api import fetch_and_serialize_unused_coupons # noqa: ERA001 @@ -252,9 +253,17 @@ def get_description(self, instance): """Get description""" return instance.organization.description + @extend_schema_field(str) def get_logo(self, instance): """Get logo""" - return instance.organization.logo if instance.organization.logo else None + + if hasattr(instance.organization, "logo"): + try: + return get_wagtail_img_src(instance.organization.logo) + except AttributeError: + pass + + return None @extend_schema_field(str) def get_slug(self, instance): From cd92a67efc0447b561f528edd5458ce7e023a2e0 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 2 Oct 2025 11:58:35 -0500 Subject: [PATCH 05/15] fix user view test --- users/views_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/users/views_test.py b/users/views_test.py index cd52af411a..6009921d99 100644 --- a/users/views_test.py +++ b/users/views_test.py @@ -66,7 +66,7 @@ def test_get_user_by_me(mocker, client, user, is_anonymous, has_orgs): "id": contract.organization.id, "name": contract.organization.name, "description": contract.organization.description, - "logo": None, + "logo": "", "slug": contract.organization.slug, "contracts": [ { From cd2e0cd2a19c0f800b0e6f16b5bc097a7f1e9ca2 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 6 Oct 2025 10:47:58 -0500 Subject: [PATCH 06/15] update b2b_contract membership type handling, rework 0009 migration to AlterField rather than drop a column and create a new one, which seems more straightforward --- b2b/management/commands/b2b_contract.py | 15 ++------- ...9_rename_integration_type_add_new_types.py | 31 ++++--------------- 2 files changed, 8 insertions(+), 38 deletions(-) diff --git a/b2b/management/commands/b2b_contract.py b/b2b/management/commands/b2b_contract.py index b7aa7281d4..a69178f82d 100644 --- a/b2b/management/commands/b2b_contract.py +++ b/b2b/management/commands/b2b_contract.py @@ -11,11 +11,7 @@ ======= from b2b.api import create_contract_run from b2b.constants import ( - CONTRACT_MEMBERSHIP_AUTO, - CONTRACT_MEMBERSHIP_CODE, - CONTRACT_MEMBERSHIP_MANAGED, - CONTRACT_MEMBERSHIP_NONSSO, - CONTRACT_MEMBERSHIP_SSO, + CONTRACT_MEMBERSHIP_CHOICES, ) >>>>>>> 0528cf9f (Update integration type field to membership type, update org/contract attachment points, update org membership in Keycloak where approrpiate) from b2b.models import ContractPage, OrganizationIndexPage, OrganizationPage @@ -55,14 +51,7 @@ def add_arguments(self, parser): "membership_type", type=str, help="The mmebership type for this contract.", - choices=[ - CONTRACT_MEMBERSHIP_SSO, - CONTRACT_MEMBERSHIP_NONSSO, - CONTRACT_MEMBERSHIP_MANAGED, - CONTRACT_MEMBERSHIP_CODE, - CONTRACT_MEMBERSHIP_AUTO, - ], - default=CONTRACT_MEMBERSHIP_MANAGED, + choices=[value[0] for value in CONTRACT_MEMBERSHIP_CHOICES], ) create_parser.add_argument( "--description", diff --git a/b2b/migrations/0009_rename_integration_type_add_new_types.py b/b2b/migrations/0009_rename_integration_type_add_new_types.py index 9608243b1e..c8b606f383 100644 --- a/b2b/migrations/0009_rename_integration_type_add_new_types.py +++ b/b2b/migrations/0009_rename_integration_type_add_new_types.py @@ -4,30 +4,18 @@ from django.db import migrations, models -def copy_integration_type_to_membership_type(apps, schema_editor): - """Copy existing values from integration_type to membership_type.""" - ContractPage = apps.get_model("b2b", "ContractPage") - for contract in ContractPage.objects.all(): - if contract.integration_type == "sso": - contract.membership_type = "sso" - elif contract.integration_type == "non-sso": - contract.membership_type = "non-sso" - else: - contract.membership_type = "managed" # Default to managed for other cases - contract.save() - - -def noop(apps, schema_editor): - """No operation (placeholder for reverse migration).""" - - class Migration(migrations.Migration): dependencies = [ ("b2b", "0008_increase_length_on_org_key_field"), ] operations = [ - migrations.AddField( + migrations.RenameField( + model_name="contractpage", + old_name="integration_type", + new_name="membership_type", + ), + migrations.AlterField( model_name="contractpage", name="membership_type", field=models.CharField( @@ -43,13 +31,6 @@ class Migration(migrations.Migration): max_length=255, ), ), - migrations.RunPython( - copy_integration_type_to_membership_type, reverse_code=noop - ), - migrations.RemoveField( - model_name="contractpage", - name="integration_type", - ), migrations.AlterField( model_name="contractpage", name="organization", From 777bd38bef91f5b1f90092029704f6cf56cebcaa Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 6 Oct 2025 11:59:26 -0500 Subject: [PATCH 07/15] more comment addressing: move UserOrg model/serializer to b2b, update a bunch of stuff for that, cleanup elsewhere --- b2b/api.py | 41 ++-- b2b/api_test.py | 3 +- ...0010_move_userorganization_model_to_b2b.py | 8 +- b2b/models.py | 85 +++++++- b2b/models_test.py | 192 +++++++++++++++++- b2b/serializers/v0/__init__.py | 67 +++++- b2b/views/v0/__init__.py | 2 +- courses/api.py | 2 +- pytest.ini | 2 +- users/admin.py | 3 +- users/models.py | 77 ------- users/models_test.py | 185 ----------------- users/serializers.py | 87 +------- 13 files changed, 364 insertions(+), 390 deletions(-) rename users/migrations/0038_add_fk_from_users_to_organizationpage.py => b2b/migrations/0010_move_userorganization_model_to_b2b.py (87%) diff --git a/b2b/api.py b/b2b/api.py index 082d77040c..ec80116b85 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -25,7 +25,12 @@ from b2b.exceptions import SourceCourseIncompleteError, TargetCourseRunExistsError from b2b.keycloak_admin_api import KCAM_ORGANIZATIONS, get_keycloak_model from b2b.keycloak_admin_dataclasses import OrganizationRepresentation -from b2b.models import ContractPage, OrganizationIndexPage, OrganizationPage +from b2b.models import ( + ContractPage, + OrganizationIndexPage, + OrganizationPage, + UserOrganization, +) from cms.api import get_home_page from courses.constants import UAI_COURSEWARE_ID_PREFIX from courses.models import Course, CourseRun @@ -47,7 +52,6 @@ from main.utils import date_to_datetime from openedx.api import create_user from openedx.tasks import clone_courserun -from users.models import UserOrganization log = logging.getLogger(__name__) @@ -828,40 +832,23 @@ def reconcile_user_orgs(user, organizations): # we've checked the cached org membership, so now figure out what orgs # we're in but aren't in the list, and vice versa - orgs_to_add = ( - OrganizationPage.objects.filter( - Q(sso_organization_id__in=organizations) & ~Q(organization_users__user=user) - ) - .filter(sso_organization_id__isnull=False) - .all() - ) + orgs_to_add = OrganizationPage.objects.filter( + Q(sso_organization_id__in=organizations) & ~Q(organization_users__user=user) + ).filter(sso_organization_id__isnull=False) - orgs_to_remove = ( - UserOrganization.objects.filter( - ~Q(organization__sso_organization_id__in=organizations) - & Q(user=user, keep_until_seen=False) - ) - .filter(organization__sso_organization_id__isnull=False) - .all() - ) + orgs_to_remove = UserOrganization.objects.filter( + ~Q(organization__sso_organization_id__in=organizations) + & Q(user=user, keep_until_seen=False) + ).filter(organization__sso_organization_id__isnull=False) for add_org in orgs_to_add: # add org, add contracts, clear flag if we need to - new_membership, created = UserOrganization.objects.get_or_create( + UserOrganization.objects.update_or_create( user=user, organization=add_org, defaults={"keep_until_seen": False}, ) - if not created and new_membership.keep_until_seen: - new_membership.keep_until_seen = False - new_membership.save() - log.info( - "reconcile_user_orgs: cleared keep_until_seen for user %s org %s", - user.id, - add_org, - ) - add_org.add_user_contracts(user) log.info("reconcile_user_orgs: added user %s to org %s", user.id, add_org) diff --git a/b2b/api_test.py b/b2b/api_test.py index 926c2bff70..918fb90966 100644 --- a/b2b/api_test.py +++ b/b2b/api_test.py @@ -30,7 +30,7 @@ ) from b2b.exceptions import SourceCourseIncompleteError, TargetCourseRunExistsError from b2b.factories import OrganizationPageFactory -from b2b.models import OrganizationIndexPage, OrganizationPage +from b2b.models import OrganizationIndexPage, OrganizationPage, UserOrganization from courses.constants import UAI_COURSEWARE_ID_PREFIX from courses.factories import CourseFactory, CourseRunFactory from courses.models import CourseRunEnrollment @@ -53,7 +53,6 @@ from main.utils import date_to_datetime from openedx.constants import EDX_ENROLLMENT_VERIFIED_MODE from users.factories import UserFactory -from users.models import UserOrganization FAKE = faker.Faker() pytestmark = [ diff --git a/users/migrations/0038_add_fk_from_users_to_organizationpage.py b/b2b/migrations/0010_move_userorganization_model_to_b2b.py similarity index 87% rename from users/migrations/0038_add_fk_from_users_to_organizationpage.py rename to b2b/migrations/0010_move_userorganization_model_to_b2b.py index c084577916..87c05d58ba 100644 --- a/users/migrations/0038_add_fk_from_users_to_organizationpage.py +++ b/b2b/migrations/0010_move_userorganization_model_to_b2b.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.24 on 2025-09-24 19:44 +# Generated by Django 4.2.25 on 2025-10-06 16:53 import django.db.models.deletion from django.conf import settings @@ -7,8 +7,8 @@ class Migration(migrations.Migration): dependencies = [ - ("b2b", "0008_increase_length_on_org_key_field"), - ("users", "0037_add_global_id_default"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("b2b", "0009_rename_integration_type_add_new_types"), ] operations = [ @@ -17,7 +17,7 @@ class Migration(migrations.Migration): fields=[ ( "id", - models.AutoField( + models.BigAutoField( auto_created=True, primary_key=True, serialize=False, diff --git a/b2b/models.py b/b2b/models.py index d98a1ae6a0..c875fb6fd2 100644 --- a/b2b/models.py +++ b/b2b/models.py @@ -1,5 +1,7 @@ """Models for B2B data.""" +import logging + from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.contenttypes.models import ContentType @@ -21,6 +23,8 @@ from b2b.exceptions import TargetCourseRunExistsError from b2b.tasks import queue_enrollment_code_check +log = logging.getLogger(__name__) + class OrganizationObjectIndexPage(Page): """The index page for organizations - provides the root level folder.""" @@ -145,8 +149,6 @@ def add_user_contracts(self, user): for contract in contracts_qs.all(): user.b2b_contracts.add(contract) - user.save() - return contracts_qs.count() def remove_user_contracts(self, user): @@ -408,3 +410,82 @@ class DiscountContractAttachmentRedemption(TimestampedModel): on_delete=models.DO_NOTHING, help_text="The contract that the user was attached to.", ) + + +class UserOrganization(models.Model): + """The user's organizations memberships.""" + + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + related_name="b2b_organizations", + ) + organization = models.ForeignKey( + "b2b.OrganizationPage", + on_delete=models.CASCADE, + related_name="organization_users", + ) + keep_until_seen = models.BooleanField( + default=False, + help_text="If True, the user will be kept in the organization until the organization is seen in their SSO data.", + ) + + class Meta: + unique_together = ("user", "organization") + + def __str__(self): + """Return a reasonable representation of the object as a string.""" + + return f"UserOrganization: {self.user} in {self.organization}" + + @staticmethod + def process_add_membership(user, organization, *, keep_until_seen=False): + """ + Add a user to an org, and kick off contract processing. + + This allows us to manage UserOrganization records without necessarily + being forced to process contract memberships at the same time. + + Args: + - user (users.models.User): the user to add + - organization (b2b.models.OrganizationPage): the organization to add the user to + - keep_until_seen (bool): if True, the user will be kept in the org until the + organization is seen in their SSO data. + """ + + obj, created = UserOrganization.objects.get_or_create( + user=user, + organization=organization, + ) + if created: + obj.keep_until_seen = keep_until_seen + obj.save() + try: + organization.attach_user(user) + except ConnectionError: + log.exception( + "Could not attach %s to Keycloak org for %s", user, organization + ) + organization.add_user_contracts(user) + + return obj + + @staticmethod + def process_remove_membership(user, organization): + """ + Remove a user from an org, and kick off contract processing. + + Other side of the process_add_membership function - removes the membership + and associated managed contracts. + + Args: + - user (users.models.User): the user to remove + - organization (b2b.models.OrganizationPage): the organization to remove the user from + """ + + organization.remove_user_contracts(user) + + UserOrganization.objects.filter( + user=user, + organization=organization, + ).get().delete() diff --git a/b2b/models_test.py b/b2b/models_test.py index 8501c10734..db16a59062 100644 --- a/b2b/models_test.py +++ b/b2b/models_test.py @@ -3,8 +3,14 @@ import faker import pytest -from b2b.factories import ContractPageFactory -from courses.factories import CourseRunFactory, ProgramFactory +from b2b.factories import ContractPageFactory, OrganizationPageFactory +from b2b.models import UserOrganization +from courses.factories import ( + CourseRunEnrollmentFactory, + CourseRunFactory, + ProgramFactory, +) +from users.factories import UserFactory pytestmark = [pytest.mark.django_db] FAKE = faker.Faker() @@ -50,3 +56,185 @@ def test_add_program_courses_to_contract(mocker): assert contract.programs.count() == 1 assert contract.get_course_runs().count() == 4 + + +def test_user_add_b2b_org(mocked_b2b_org_attach): + """Ensure adding a user to an organization works as expected.""" + + orgs = OrganizationPageFactory.create_batch(2) + user = UserFactory.create() + + # New-style ones + contract_auto = ContractPageFactory.create( + organization=orgs[0], + membership_type="auto", + title="Contract Auto", + name="Contract Auto", + ) + contract_managed = ContractPageFactory.create( + organization=orgs[0], + membership_type="managed", + title="Contract Managed", + name="Contract Managed", + ) + contract_code = ContractPageFactory.create( + organization=orgs[0], + membership_type="code", + title="Contract Enrollment Code", + name="Contract Enrollment Code", + ) + # Legacy ones - these will migrate to "managed" and "code" + contract_sso = ContractPageFactory.create( + organization=orgs[0], + membership_type="sso", + title="Contract SSO", + name="Contract SSO", + ) + contract_non_sso = ContractPageFactory.create( + organization=orgs[0], + membership_type="non-sso", + title="Contract NonSSO", + name="Contract NonSSO", + ) + + UserOrganization.process_add_membership(user, orgs[0]) + + # We should now be in the SSO, auto, and managed contracts + # but not the other two. + + user.refresh_from_db() + assert user.b2b_contracts.count() == 3 + assert user.b2b_organizations.filter(organization=orgs[0]).exists() + assert ( + user.b2b_contracts.filter( + pk__in=[ + contract_auto.id, + contract_sso.id, + contract_managed.id, + ] + ).count() + == 3 + ) + assert ( + user.b2b_contracts.filter( + pk__in=[ + contract_code.id, + contract_non_sso.id, + ] + ).count() + == 0 + ) + + +def test_user_remove_b2b_org(mocked_b2b_org_attach): + """Ensure removing a user from an org also clears the appropriate contracts.""" + + orgs = OrganizationPageFactory.create_batch(2) + user = UserFactory.create() + + # New-style ones + contract_auto = ContractPageFactory.create( + organization=orgs[0], + membership_type="auto", + title="Contract Auto", + name="Contract Auto", + ) + contract_managed = ContractPageFactory.create( + organization=orgs[0], + membership_type="managed", + title="Contract Managed", + name="Contract Managed", + ) + contract_code = ContractPageFactory.create( + organization=orgs[1], + membership_type="code", + title="Contract Enrollment Code", + name="Contract Enrollment Code", + ) + # Legacy ones - these will migrate to "managed" and "code" + contract_sso = ContractPageFactory.create( + organization=orgs[0], + membership_type="sso", + title="Contract SSO", + name="Contract SSO", + ) + contract_non_sso = ContractPageFactory.create( + organization=orgs[1], + membership_type="non-sso", + title="Contract NonSSO", + name="Contract NonSSO", + ) + + managed_ids = [ + contract_auto.id, + contract_managed.id, + contract_sso.id, + ] + unmanaged_ids = [ + contract_code.id, + contract_non_sso.id, + ] + + UserOrganization.process_add_membership(user, orgs[0]) + UserOrganization.process_add_membership(user, orgs[1]) + + user.b2b_contracts.add(contract_code) + user.b2b_contracts.add(contract_non_sso) + user.save() + + user.refresh_from_db() + + assert user.b2b_contracts.count() == 5 + + UserOrganization.process_remove_membership(user, orgs[1]) + + assert user.b2b_contracts.filter(id__in=managed_ids).count() == 3 + assert user.b2b_contracts.filter(id__in=unmanaged_ids).count() == 0 + + UserOrganization.process_remove_membership(user, orgs[0]) + + # we should have no contracts now since we're no longer in any orgs + + assert user.b2b_contracts.count() == 0 + + +def test_b2b_contract_removal_keeps_enrollments(mocked_b2b_org_attach): + """Ensure that removing a user from a B2B contract leaves their enrollments alone.""" + + org = OrganizationPageFactory.create() + user = UserFactory.create() + + contract_auto = ContractPageFactory.create( + organization=org, + membership_type="auto", + title="Contract Auto", + name="Contract Auto", + ) + + courserun = CourseRunFactory.create(b2b_contract=contract_auto) + + UserOrganization.process_add_membership(user, org) + + CourseRunEnrollmentFactory( + user=user, + run=courserun, + ) + + user.refresh_from_db() + + assert courserun.enrollments.filter(user=user).count() == 1 + + UserOrganization.process_remove_membership(user, org) + + assert courserun.enrollments.filter(user=user).count() == 1 + + +def test_b2b_org_attach_calls_keycloak(mocked_b2b_org_attach): + """Test that attaching a user to an org calls Keycloak successfully.""" + + org = OrganizationPageFactory.create() + user = UserFactory.create() + + UserOrganization.process_add_membership(user, org) + + mocked_b2b_org_attach.assert_called() diff --git a/b2b/serializers/v0/__init__.py b/b2b/serializers/v0/__init__.py index 2865c5453a..e5c83c27a8 100644 --- a/b2b/serializers/v0/__init__.py +++ b/b2b/serializers/v0/__init__.py @@ -1,8 +1,10 @@ """Serializers for the B2B API (v0).""" +from drf_spectacular.utils import extend_schema_field from rest_framework import serializers -from b2b.models import ContractPage, OrganizationPage +from b2b.models import ContractPage, OrganizationPage, UserOrganization +from cms.api import get_wagtail_img_src from main.constants import USER_MSG_TYPE_B2B_CHOICES @@ -109,3 +111,66 @@ class CreateB2BEnrollmentSerializer(serializers.Serializer): max_digits=None, decimal_places=2, read_only=True, required=False ) checkout_result = GenerateCheckoutPayloadSerializer(required=False) + + +class UserOrganizationSerializer(serializers.ModelSerializer): + """ + Serializer for user organization data. + + Return the user's organizations in a manner that makes them look like + OrganizationPage objects. (Previously, the user organizations were a queryset + of OrganizationPages that related to the user, but now we have a through + table.) + """ + + contracts = serializers.SerializerMethodField() + id = serializers.IntegerField(source="organization.id") + name = serializers.CharField(source="organization.name") + description = serializers.CharField(source="organization.description") + logo = serializers.SerializerMethodField() + slug = serializers.CharField(source="organization.slug") + + @extend_schema_field(ContractPageSerializer(many=True)) + def get_contracts(self, instance): + """Get the contracts for the organization for the user""" + contracts = ( + self.context["user"] + .b2b_contracts.filter( + organization=instance.organization, + ) + .all() + ) + return ContractPageSerializer(contracts, many=True).data + + @extend_schema_field(str) + def get_logo(self, instance): + """Get logo""" + + if hasattr(instance.organization, "logo"): + try: + return get_wagtail_img_src(instance.organization.logo) + except AttributeError: + pass + + return None + + class Meta: + """Meta opts for the serializer.""" + + model = UserOrganization + fields = [ + "id", + "name", + "description", + "logo", + "slug", + "contracts", + ] + read_only_fields = [ + "id", + "name", + "description", + "logo", + "slug", + "contracts", + ] diff --git a/b2b/views/v0/__init__.py b/b2b/views/v0/__init__.py index 6478fae9c4..a061c1af31 100644 --- a/b2b/views/v0/__init__.py +++ b/b2b/views/v0/__init__.py @@ -16,6 +16,7 @@ ContractPage, DiscountContractAttachmentRedemption, OrganizationPage, + UserOrganization, ) from b2b.serializers.v0 import ( ContractPageSerializer, @@ -27,7 +28,6 @@ from main.authentication import CsrfExemptSessionAuthentication from main.constants import USER_MSG_TYPE_B2B_ENROLL_SUCCESS from main.permissions import IsAdminOrReadOnly -from users.models import UserOrganization class OrganizationPageViewSet(viewsets.ReadOnlyModelViewSet): diff --git a/courses/api.py b/courses/api.py index bbdb6507d6..b04e77c97d 100644 --- a/courses/api.py +++ b/courses/api.py @@ -28,6 +28,7 @@ from requests.exceptions import HTTPError from rest_framework.status import HTTP_404_NOT_FOUND +from b2b.models import UserOrganization from cms.api import create_default_courseware_page from courses import mail_api from courses.constants import ( @@ -78,7 +79,6 @@ NoEdxApiAuthError, UnknownEdxApiEnrollException, ) -from users.models import UserOrganization if TYPE_CHECKING: from django.db.models.query import QuerySet diff --git a/pytest.ini b/pytest.ini index 0611aed054..97cbb9b657 100644 --- a/pytest.ini +++ b/pytest.ini @@ -15,7 +15,7 @@ env = KEYCLOAK_BASE_URL=http://keycloak/ KEYCLOAK_REALM_NAME=ol-test KEYCLOAK_CLIENT_ID=mitxonline - KEYCLOAK_DISCOVERY_URL=https://keycloak/realms/ol-local/.well-known/openid-configuration + KEYCLOAK_DISCOVERY_URL=https://keycloak/ KEYCLOAK_ADMIN_CLIENT_ID=apisix KEYCLOAK_ADMIN_CLIENT_SECRET=fake_admin_secret KEYCLOAK_ADMIN_CLIENT_SCOPES="openid profile email" diff --git a/users/admin.py b/users/admin.py index 0398c1f0d2..c4841ef2a5 100644 --- a/users/admin.py +++ b/users/admin.py @@ -11,8 +11,9 @@ from hijack.contrib.admin import HijackUserAdminMixin from mitol.common.admin import TimestampedModelAdmin +from b2b.models import UserOrganization from openedx.models import OpenEdxUser -from users.models import BlockList, LegalAddress, User, UserOrganization, UserProfile +from users.models import BlockList, LegalAddress, User, UserProfile class UserLegalAddressInline(admin.StackedInline): diff --git a/users/models.py b/users/models.py index c93ecc1080..0d76a9a497 100644 --- a/users/models.py +++ b/users/models.py @@ -596,80 +596,3 @@ class BlockList(TimestampedModel): """A user's blocklist model""" hashed_email = models.CharField(max_length=128) - - -class UserOrganization(models.Model): - """The user's organizations memberships.""" - - user = models.ForeignKey( - User, on_delete=models.CASCADE, related_name="b2b_organizations" - ) - organization = models.ForeignKey( - "b2b.OrganizationPage", - on_delete=models.CASCADE, - related_name="organization_users", - ) - keep_until_seen = models.BooleanField( - default=False, - help_text="If True, the user will be kept in the organization until the organization is seen in their SSO data.", - ) - - class Meta: - unique_together = ("user", "organization") - - def __str__(self): - """Return a reasonable representation of the object as a string.""" - - return f"UserOrganization: {self.user} in {self.organization}" - - @staticmethod - def process_add_membership(user, organization, *, keep_until_seen=False): - """ - Add a user to an org, and kick off contract processing. - - This allows us to manage UserOrganization records without necessarily - being forced to process contract memberships at the same time. - - Args: - - user (users.models.User): the user to add - - organization (b2b.models.OrganizationPage): the organization to add the user to - - keep_until_seen (bool): if True, the user will be kept in the org until the - organization is seen in their SSO data. - """ - - obj, created = UserOrganization.objects.get_or_create( - user=user, - organization=organization, - ) - if created: - obj.keep_until_seen = keep_until_seen - obj.save() - try: - organization.attach_user(user) - except ConnectionError: - log.exception( - "Could not attach %s to Keycloak org for %s", user, organization - ) - organization.add_user_contracts(user) - - return obj - - @staticmethod - def process_remove_membership(user, organization): - """ - Remove a user from an org, and kick off contract processing. - - Other side of the process_add_membership function - removes the membership - and associated managed contracts. - - Args: - - user (users.models.User): the user to remove - - organization (b2b.models.OrganizationPage): the organization to remove the user from - """ - - organization.remove_user_contracts(user) - - UserOrganization.objects.filter( - user=user, - organization=organization, - ).get().delete() diff --git a/users/models_test.py b/users/models_test.py index 602adbc8c5..b13062adc9 100644 --- a/users/models_test.py +++ b/users/models_test.py @@ -9,9 +9,7 @@ from django.core.exceptions import ValidationError from django.db import transaction -from b2b.factories import ContractPageFactory, OrganizationPageFactory from cms.constants import CMS_EDITORS_GROUP_NAME -from courses.factories import CourseRunEnrollmentFactory, CourseRunFactory from openedx.factories import OpenEdxApiAuthFactory, OpenEdxUserFactory from openedx.models import OpenEdxUser from users.factories import UserFactory @@ -20,7 +18,6 @@ OPENEDX_HIGHEST_EDUCATION_MAPPINGS, LegalAddress, User, - UserOrganization, ) pytestmark = pytest.mark.django_db @@ -221,185 +218,3 @@ def test_user_profile_edx_properties(should_exist): if should_exist else user.legal_address.edx_us_state is None ) - - -def test_user_add_b2b_org(mocked_b2b_org_attach): - """Ensure adding a user to an organization works as expected.""" - - orgs = OrganizationPageFactory.create_batch(2) - user = UserFactory.create() - - # New-style ones - contract_auto = ContractPageFactory.create( - organization=orgs[0], - membership_type="auto", - title="Contract Auto", - name="Contract Auto", - ) - contract_managed = ContractPageFactory.create( - organization=orgs[0], - membership_type="managed", - title="Contract Managed", - name="Contract Managed", - ) - contract_code = ContractPageFactory.create( - organization=orgs[0], - membership_type="code", - title="Contract Enrollment Code", - name="Contract Enrollment Code", - ) - # Legacy ones - these will migrate to "managed" and "code" - contract_sso = ContractPageFactory.create( - organization=orgs[0], - membership_type="sso", - title="Contract SSO", - name="Contract SSO", - ) - contract_non_sso = ContractPageFactory.create( - organization=orgs[0], - membership_type="non-sso", - title="Contract NonSSO", - name="Contract NonSSO", - ) - - UserOrganization.process_add_membership(user, orgs[0]) - - # We should now be in the SSO, auto, and managed contracts - # but not the other two. - - user.refresh_from_db() - assert user.b2b_contracts.count() == 3 - assert user.b2b_organizations.filter(organization=orgs[0]).exists() - assert ( - user.b2b_contracts.filter( - pk__in=[ - contract_auto.id, - contract_sso.id, - contract_managed.id, - ] - ).count() - == 3 - ) - assert ( - user.b2b_contracts.filter( - pk__in=[ - contract_code.id, - contract_non_sso.id, - ] - ).count() - == 0 - ) - - -def test_user_remove_b2b_org(mocked_b2b_org_attach): - """Ensure removing a user from an org also clears the appropriate contracts.""" - - orgs = OrganizationPageFactory.create_batch(2) - user = UserFactory.create() - - # New-style ones - contract_auto = ContractPageFactory.create( - organization=orgs[0], - membership_type="auto", - title="Contract Auto", - name="Contract Auto", - ) - contract_managed = ContractPageFactory.create( - organization=orgs[0], - membership_type="managed", - title="Contract Managed", - name="Contract Managed", - ) - contract_code = ContractPageFactory.create( - organization=orgs[1], - membership_type="code", - title="Contract Enrollment Code", - name="Contract Enrollment Code", - ) - # Legacy ones - these will migrate to "managed" and "code" - contract_sso = ContractPageFactory.create( - organization=orgs[0], - membership_type="sso", - title="Contract SSO", - name="Contract SSO", - ) - contract_non_sso = ContractPageFactory.create( - organization=orgs[1], - membership_type="non-sso", - title="Contract NonSSO", - name="Contract NonSSO", - ) - - managed_ids = [ - contract_auto.id, - contract_managed.id, - contract_sso.id, - ] - unmanaged_ids = [ - contract_code.id, - contract_non_sso.id, - ] - - UserOrganization.process_add_membership(user, orgs[0]) - UserOrganization.process_add_membership(user, orgs[1]) - - user.b2b_contracts.add(contract_code) - user.b2b_contracts.add(contract_non_sso) - user.save() - - user.refresh_from_db() - - assert user.b2b_contracts.count() == 5 - - UserOrganization.process_remove_membership(user, orgs[1]) - - assert user.b2b_contracts.filter(id__in=managed_ids).count() == 3 - assert user.b2b_contracts.filter(id__in=unmanaged_ids).count() == 0 - - UserOrganization.process_remove_membership(user, orgs[0]) - - # we should have no contracts now since we're no longer in any orgs - - assert user.b2b_contracts.count() == 0 - - -def test_b2b_contract_removal_keeps_enrollments(mocked_b2b_org_attach): - """Ensure that removing a user from a B2B contract leaves their enrollments alone.""" - - org = OrganizationPageFactory.create() - user = UserFactory.create() - - contract_auto = ContractPageFactory.create( - organization=org, - membership_type="auto", - title="Contract Auto", - name="Contract Auto", - ) - - courserun = CourseRunFactory.create(b2b_contract=contract_auto) - - UserOrganization.process_add_membership(user, org) - - CourseRunEnrollmentFactory( - user=user, - run=courserun, - ) - - user.refresh_from_db() - - assert courserun.enrollments.filter(user=user).count() == 1 - - UserOrganization.process_remove_membership(user, org) - - assert courserun.enrollments.filter(user=user).count() == 1 - - -def test_b2b_org_attach_calls_keycloak(mocked_b2b_org_attach): - """Test that attaching a user to an org calls Keycloak successfully.""" - - org = OrganizationPageFactory.create() - user = UserFactory.create() - - UserOrganization.process_add_membership(user, org) - - mocked_b2b_org_attach.assert_called() diff --git a/users/serializers.py b/users/serializers.py index 1a2a04e6e8..e286eabeda 100644 --- a/users/serializers.py +++ b/users/serializers.py @@ -11,8 +11,7 @@ from rest_framework import serializers from social_django.models import UserSocialAuth -from b2b.serializers.v0 import ContractPageSerializer -from cms.api import get_wagtail_img_src +from b2b.serializers.v0 import UserOrganizationSerializer from hubspot_sync.task_helpers import sync_hubspot_user # from ecommerce.api import fetch_and_serialize_unused_coupons # noqa: ERA001 @@ -27,7 +26,6 @@ ChangeEmailRequest, LegalAddress, User, - UserOrganization, UserProfile, ) @@ -209,89 +207,6 @@ class Meta: ) -class UserOrganizationSerializer(serializers.ModelSerializer): - """ - Serializer for user organization data. - - Return the user's organizations in a manner that makes them look like - OrganizationPage objects. (Previously, the user organizations were a queryset - of OrganizationPages that related to the user, but now we have a through - table.) - """ - - contracts = serializers.SerializerMethodField() - id = serializers.SerializerMethodField() - name = serializers.SerializerMethodField() - description = serializers.SerializerMethodField() - logo = serializers.SerializerMethodField() - slug = serializers.SerializerMethodField() - - @extend_schema_field(ContractPageSerializer(many=True)) - def get_contracts(self, instance): - """Get the contracts for the organization for the user""" - contracts = ( - self.context["user"] - .b2b_contracts.filter( - organization=instance.organization, - ) - .all() - ) - return ContractPageSerializer(contracts, many=True).data - - @extend_schema_field(int) - def get_id(self, instance): - """Get id""" - return instance.organization.id - - @extend_schema_field(str) - def get_name(self, instance): - """Get name""" - return instance.organization.name - - @extend_schema_field(str) - def get_description(self, instance): - """Get description""" - return instance.organization.description - - @extend_schema_field(str) - def get_logo(self, instance): - """Get logo""" - - if hasattr(instance.organization, "logo"): - try: - return get_wagtail_img_src(instance.organization.logo) - except AttributeError: - pass - - return None - - @extend_schema_field(str) - def get_slug(self, instance): - """Get slug""" - return instance.organization.slug - - class Meta: - """Meta opts for the serializer.""" - - model = UserOrganization - fields = [ - "id", - "name", - "description", - "logo", - "slug", - "contracts", - ] - read_only_fields = [ - "id", - "name", - "description", - "logo", - "slug", - "contracts", - ] - - class UserSerializer(serializers.ModelSerializer): """Serializer for users""" From aa9c730b3233e552ebb79d3ba26528ee1a49fcbb Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 6 Oct 2025 15:39:59 -0500 Subject: [PATCH 08/15] Rework; keep integration_type field for now, add membership_type alongside it, collapse two migrations into one --- b2b/admin.py | 5 +- b2b/api.py | 2 +- b2b/api_test.py | 7 ++ b2b/constants.py | 19 +++ b2b/factories.py | 3 +- b2b/management/commands/b2b_contract.py | 9 +- b2b/management/commands/b2b_list.py | 2 +- ...9_contractpage_membership_type_and_more.py | 117 ++++++++++++++++++ ...9_rename_integration_type_add_new_types.py | 44 ------- ...0010_move_userorganization_model_to_b2b.py | 55 -------- b2b/models.py | 13 +- b2b/models_test.py | 18 +++ b2b/serializers/v0/__init__.py | 2 + b2b/views/v0/views_test.py | 5 + courses/api_test.py | 1 + courses/views/v2/__init__.py | 2 +- users/models_test.py | 7 -- users/views_test.py | 1 + 18 files changed, 194 insertions(+), 118 deletions(-) create mode 100644 b2b/migrations/0009_contractpage_membership_type_and_more.py delete mode 100644 b2b/migrations/0009_rename_integration_type_add_new_types.py delete mode 100644 b2b/migrations/0010_move_userorganization_model_to_b2b.py diff --git a/b2b/admin.py b/b2b/admin.py index 470ae508bc..8a6f4ec58e 100644 --- a/b2b/admin.py +++ b/b2b/admin.py @@ -105,11 +105,11 @@ class ContractPageAdmin(ReadOnlyModelAdmin): "slug", "title", "organization", - "membership_type", + "integration_type", "contract_start", "contract_end", ] - list_filter = ["membership_type", "organization", "contract_start", "contract_end"] + list_filter = ["integration_type", "organization", "contract_start", "contract_end"] date_hierarchy = "contract_start" fields = [ "id", @@ -118,6 +118,7 @@ class ContractPageAdmin(ReadOnlyModelAdmin): "organization", "title", "description", + "integration_type", "membership_type", "contract_start", "contract_end", diff --git a/b2b/api.py b/b2b/api.py index ec80116b85..b4820c5031 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -602,7 +602,7 @@ def ensure_enrollment_codes_exist(contract: ContractPage): log.info("Checking enrollment codes for contract %s", contract) if ( - contract.membership_type in CONTRACT_MEMBERSHIP_AUTOS + contract.integration_type in CONTRACT_MEMBERSHIP_AUTOS and not contract.enrollment_fixed_price ): # SSO contracts w/out price don't need discounts. diff --git a/b2b/api_test.py b/b2b/api_test.py index 918fb90966..d231fc468c 100644 --- a/b2b/api_test.py +++ b/b2b/api_test.py @@ -261,6 +261,9 @@ def test_ensure_enrollment_codes( # noqa: PLR0913 assert_price = price if price else Decimal(0) contract = factories.ContractPageFactory( + integration_type=CONTRACT_MEMBERSHIP_SSO + if is_sso + else CONTRACT_MEMBERSHIP_NONSSO, membership_type=CONTRACT_MEMBERSHIP_SSO if is_sso else CONTRACT_MEMBERSHIP_NONSSO, @@ -305,6 +308,9 @@ def test_ensure_enrollment_codes( # noqa: PLR0913 if update_no_price: contract.enrolment_fixed_price = None if update_sso: + contract.integration_type = ( + CONTRACT_MEMBERSHIP_NONSSO if is_sso else CONTRACT_MEMBERSHIP_SSO + ) contract.membership_type = ( CONTRACT_MEMBERSHIP_NONSSO if is_sso else CONTRACT_MEMBERSHIP_SSO ) @@ -362,6 +368,7 @@ def test_create_b2b_enrollment( # noqa: PLR0913, C901, PLR0915 settings.OPENEDX_SERVICE_WORKER_USERNAME = "a username" contract = factories.ContractPageFactory.create( + integration_type=CONTRACT_MEMBERSHIP_SSO, membership_type=CONTRACT_MEMBERSHIP_SSO, enrollment_fixed_price=Decimal(0) if price_is_zero diff --git a/b2b/constants.py b/b2b/constants.py index dc5178e821..02c2878d65 100644 --- a/b2b/constants.py +++ b/b2b/constants.py @@ -37,6 +37,25 @@ CONTRACT_MEMBERSHIP_AUTO_NAME, ], ) +# When the integration_type field is removed, this should be removed as well. +# It is just here so we can have the same choices in both integration_type and +# membership_type fields. (Using the constant for `choices` consumes it.) +CONTRACT_MEMBERSHIP_TYPE_CHOICES = zip( + [ + CONTRACT_MEMBERSHIP_SSO, + CONTRACT_MEMBERSHIP_NONSSO, + CONTRACT_MEMBERSHIP_MANAGED, + CONTRACT_MEMBERSHIP_CODE, + CONTRACT_MEMBERSHIP_AUTO, + ], + [ + CONTRACT_MEMBERSHIP_SSO_NAME, + CONTRACT_MEMBERSHIP_NONSSO_NAME, + CONTRACT_MEMBERSHIP_MANAGED_NAME, + CONTRACT_MEMBERSHIP_CODE_NAME, + CONTRACT_MEMBERSHIP_AUTO_NAME, + ], +) B2B_RUN_TAG_FORMAT = "{year}_C{contract_id}" diff --git a/b2b/factories.py b/b2b/factories.py index 1cf555facf..811d42e264 100644 --- a/b2b/factories.py +++ b/b2b/factories.py @@ -55,11 +55,12 @@ class ContractPageFactory(wagtail_factories.PageFactory): description = LazyAttribute(lambda _: FAKE.unique.text()) organization = SubFactory(OrganizationPageFactory) parent = LazyAttribute(lambda o: o.organization) - membership_type = LazyFunction( + integration_type = LazyFunction( lambda: CONTRACT_MEMBERSHIP_NONSSO if FAKE.boolean() else CONTRACT_MEMBERSHIP_SSO ) + membership_type = LazyAttribute(lambda o: o.integration_type) class Meta: model = ContractPage diff --git a/b2b/management/commands/b2b_contract.py b/b2b/management/commands/b2b_contract.py index a69178f82d..58382cac1e 100644 --- a/b2b/management/commands/b2b_contract.py +++ b/b2b/management/commands/b2b_contract.py @@ -48,9 +48,9 @@ def add_arguments(self, parser): help="The name of the contract.", ) create_parser.add_argument( - "membership_type", + "integration_type", type=str, - help="The mmebership type for this contract.", + help="The membership type for this contract.", choices=[value[0] for value in CONTRACT_MEMBERSHIP_CHOICES], ) create_parser.add_argument( @@ -161,7 +161,7 @@ def handle_create(self, *args, **kwargs): # noqa: ARG002 """Handle the create subcommand.""" organization_name = kwargs.pop("organization") contract_name = kwargs.pop("contract_name") - membership_type = kwargs.pop("membership_type") + integration_type = kwargs.pop("integration_type") description = kwargs.pop("description") start_date = kwargs.pop("start") end_date = kwargs.pop("end") @@ -199,7 +199,8 @@ def handle_create(self, *args, **kwargs): # noqa: ARG002 contract = ContractPage( name=contract_name, description=description or "", - membership_type=membership_type, + integration_type=integration_type, + membership_type=integration_type, organization=org, contract_start=start_date, contract_end=end_date, diff --git a/b2b/management/commands/b2b_list.py b/b2b/management/commands/b2b_list.py index 1378cefa53..4cfc01c4d5 100644 --- a/b2b/management/commands/b2b_list.py +++ b/b2b/management/commands/b2b_list.py @@ -252,7 +252,7 @@ def handle_list_contracts(self, *args, **kwargs): # noqa: ARG002 contract.name, contract.slug, contract.organization.name, - contract.membership_type, + contract.integration_type, contract.contract_start.strftime("%Y-%m-%d\n%H:%M") if contract.contract_start else "", diff --git a/b2b/migrations/0009_contractpage_membership_type_and_more.py b/b2b/migrations/0009_contractpage_membership_type_and_more.py new file mode 100644 index 0000000000..39127f1f56 --- /dev/null +++ b/b2b/migrations/0009_contractpage_membership_type_and_more.py @@ -0,0 +1,117 @@ +# Generated by Django 4.2.25 on 2025-10-06 20:26 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +def copy_integration_type_to_membership_type(apps, schema_editor): + """Copy existing values from integration_type to membership_type.""" + ContractPage = apps.get_model("b2b", "ContractPage") + for contract in ContractPage.objects.all(): + if contract.integration_type == "sso": + contract.membership_type = "sso" + elif contract.integration_type == "non-sso": + contract.membership_type = "non-sso" + else: + contract.membership_type = "managed" # Default to managed for other cases + contract.save() + + +def noop(apps, schema_editor): + """No operation (placeholder for reverse migration).""" + + +class Migration(migrations.Migration): + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("b2b", "0008_increase_length_on_org_key_field"), + ] + + operations = [ + migrations.AddField( + model_name="contractpage", + name="membership_type", + field=models.CharField( + choices=[ + ("sso", "SSO"), + ("non-sso", "Non-SSO"), + ("managed", "Managed"), + ("code", "Enrollment Code"), + ("auto", "Auto Enrollment"), + ], + default="managed", + help_text="The method to use to manage membership in the contract.", + max_length=255, + ), + ), + migrations.AlterField( + model_name="contractpage", + name="integration_type", + field=models.CharField( + choices=[ + ("sso", "SSO"), + ("non-sso", "Non-SSO"), + ("managed", "Managed"), + ("code", "Enrollment Code"), + ("auto", "Auto Enrollment"), + ], + help_text="The type of integration for this contract.", + max_length=255, + ), + ), + migrations.AlterField( + model_name="contractpage", + name="organization", + field=models.ForeignKey( + help_text="The organization that owns this contract.", + on_delete=django.db.models.deletion.PROTECT, + related_name="contracts", + to="b2b.organizationpage", + ), + ), + migrations.CreateModel( + name="UserOrganization", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "keep_until_seen", + models.BooleanField( + default=False, + help_text="If True, the user will be kept in the organization until the organization is seen in their SSO data.", + ), + ), + ( + "organization", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="organization_users", + to="b2b.organizationpage", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="b2b_organizations", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "unique_together": {("user", "organization")}, + }, + ), + migrations.RunPython( + code=copy_integration_type_to_membership_type, + reverse_code=noop, + ), + ] diff --git a/b2b/migrations/0009_rename_integration_type_add_new_types.py b/b2b/migrations/0009_rename_integration_type_add_new_types.py deleted file mode 100644 index c8b606f383..0000000000 --- a/b2b/migrations/0009_rename_integration_type_add_new_types.py +++ /dev/null @@ -1,44 +0,0 @@ -# Generated by Django 4.2.24 on 2025-09-24 20:45 - -import django.db.models.deletion -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ("b2b", "0008_increase_length_on_org_key_field"), - ] - - operations = [ - migrations.RenameField( - model_name="contractpage", - old_name="integration_type", - new_name="membership_type", - ), - migrations.AlterField( - model_name="contractpage", - name="membership_type", - field=models.CharField( - choices=[ - ("sso", "SSO"), - ("non-sso", "Non-SSO"), - ("managed", "Managed"), - ("code", "Enrollment Code"), - ("auto", "Auto Enrollment"), - ], - default="managed", - help_text="The method to use to manage membership in the contract.", - max_length=255, - ), - ), - migrations.AlterField( - model_name="contractpage", - name="organization", - field=models.ForeignKey( - help_text="The organization that owns this contract.", - on_delete=django.db.models.deletion.PROTECT, - related_name="contracts", - to="b2b.organizationpage", - ), - ), - ] diff --git a/b2b/migrations/0010_move_userorganization_model_to_b2b.py b/b2b/migrations/0010_move_userorganization_model_to_b2b.py deleted file mode 100644 index 87c05d58ba..0000000000 --- a/b2b/migrations/0010_move_userorganization_model_to_b2b.py +++ /dev/null @@ -1,55 +0,0 @@ -# Generated by Django 4.2.25 on 2025-10-06 16:53 - -import django.db.models.deletion -from django.conf import settings -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ("b2b", "0009_rename_integration_type_add_new_types"), - ] - - operations = [ - migrations.CreateModel( - name="UserOrganization", - fields=[ - ( - "id", - models.BigAutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ( - "keep_until_seen", - models.BooleanField( - default=False, - help_text="If True, the user will be kept in the organization until the organization is seen in their SSO data.", - ), - ), - ( - "organization", - models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, - related_name="organization_users", - to="b2b.organizationpage", - ), - ), - ( - "user", - models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, - related_name="b2b_organizations", - to=settings.AUTH_USER_MODEL, - ), - ), - ], - options={ - "unique_together": {("user", "organization")}, - }, - ), - ] diff --git a/b2b/models.py b/b2b/models.py index c875fb6fd2..9d90e54c83 100644 --- a/b2b/models.py +++ b/b2b/models.py @@ -18,6 +18,7 @@ CONTRACT_MEMBERSHIP_AUTOS, CONTRACT_MEMBERSHIP_CHOICES, CONTRACT_MEMBERSHIP_MANAGED, + CONTRACT_MEMBERSHIP_TYPE_CHOICES, ORG_INDEX_SLUG, ) from b2b.exceptions import TargetCourseRunExistsError @@ -143,7 +144,7 @@ def add_user_contracts(self, user): """ contracts_qs = self.contracts.filter( - membership_type__in=CONTRACT_MEMBERSHIP_AUTOS, active=True + integration_type__in=CONTRACT_MEMBERSHIP_AUTOS, active=True ) for contract in contracts_qs.all(): @@ -191,9 +192,16 @@ class ContractPage(Page): description = RichTextField( blank=True, help_text="Any useful extra information about the contract." ) - membership_type = models.CharField( + integration_type = models.CharField( max_length=255, choices=CONTRACT_MEMBERSHIP_CHOICES, + help_text="The type of integration for this contract.", + ) + # This doesn't have a choices setting because you can't re-use a constant. + # + membership_type = models.CharField( + max_length=255, + choices=CONTRACT_MEMBERSHIP_TYPE_CHOICES, help_text="The method to use to manage membership in the contract.", default=CONTRACT_MEMBERSHIP_MANAGED, ) @@ -247,6 +255,7 @@ class ContractPage(Page): ), MultiFieldPanel( [ + FieldPanel("integration_type"), FieldPanel("membership_type"), FieldPanel("max_learners"), FieldPanel("enrollment_fixed_price"), diff --git a/b2b/models_test.py b/b2b/models_test.py index db16a59062..07e091a3dc 100644 --- a/b2b/models_test.py +++ b/b2b/models_test.py @@ -16,6 +16,13 @@ FAKE = faker.Faker() +@pytest.fixture +def mocked_b2b_org_attach(mocker): + """Mock the org attachment call.""" + + return mocker.patch("b2b.api.add_user_org_membership", return_value=True) + + def test_add_program_courses_to_contract(mocker): """Test that adding a program to a contract works as expected.""" @@ -68,18 +75,21 @@ def test_user_add_b2b_org(mocked_b2b_org_attach): contract_auto = ContractPageFactory.create( organization=orgs[0], membership_type="auto", + integration_type="auto", title="Contract Auto", name="Contract Auto", ) contract_managed = ContractPageFactory.create( organization=orgs[0], membership_type="managed", + integration_type="managed", title="Contract Managed", name="Contract Managed", ) contract_code = ContractPageFactory.create( organization=orgs[0], membership_type="code", + integration_type="code", title="Contract Enrollment Code", name="Contract Enrollment Code", ) @@ -87,12 +97,14 @@ def test_user_add_b2b_org(mocked_b2b_org_attach): contract_sso = ContractPageFactory.create( organization=orgs[0], membership_type="sso", + integration_type="sso", title="Contract SSO", name="Contract SSO", ) contract_non_sso = ContractPageFactory.create( organization=orgs[0], membership_type="non-sso", + integration_type="non-sso", title="Contract NonSSO", name="Contract NonSSO", ) @@ -136,18 +148,21 @@ def test_user_remove_b2b_org(mocked_b2b_org_attach): contract_auto = ContractPageFactory.create( organization=orgs[0], membership_type="auto", + integration_type="auto", title="Contract Auto", name="Contract Auto", ) contract_managed = ContractPageFactory.create( organization=orgs[0], membership_type="managed", + integration_type="managed", title="Contract Managed", name="Contract Managed", ) contract_code = ContractPageFactory.create( organization=orgs[1], membership_type="code", + integration_type="code", title="Contract Enrollment Code", name="Contract Enrollment Code", ) @@ -155,12 +170,14 @@ def test_user_remove_b2b_org(mocked_b2b_org_attach): contract_sso = ContractPageFactory.create( organization=orgs[0], membership_type="sso", + integration_type="sso", title="Contract SSO", name="Contract SSO", ) contract_non_sso = ContractPageFactory.create( organization=orgs[1], membership_type="non-sso", + integration_type="non-sso", title="Contract NonSSO", name="Contract NonSSO", ) @@ -207,6 +224,7 @@ def test_b2b_contract_removal_keeps_enrollments(mocked_b2b_org_attach): contract_auto = ContractPageFactory.create( organization=org, membership_type="auto", + integration_type="auto", title="Contract Auto", name="Contract Auto", ) diff --git a/b2b/serializers/v0/__init__.py b/b2b/serializers/v0/__init__.py index e5c83c27a8..3ce59ce5b7 100644 --- a/b2b/serializers/v0/__init__.py +++ b/b2b/serializers/v0/__init__.py @@ -19,6 +19,7 @@ class Meta: "id", "name", "description", + "integration_type", "membership_type", "organization", "contract_start", @@ -31,6 +32,7 @@ class Meta: "id", "name", "description", + "integration_type", "membership_type", "organization", "contract_start", diff --git a/b2b/views/v0/views_test.py b/b2b/views/v0/views_test.py index 2a4432f06a..6daf3913ea 100644 --- a/b2b/views/v0/views_test.py +++ b/b2b/views/v0/views_test.py @@ -48,6 +48,7 @@ def test_b2b_contract_attachment(mocker, user): contract = ContractPageFactory.create( membership_type=CONTRACT_MEMBERSHIP_NONSSO, + integration_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=10, ) @@ -89,6 +90,7 @@ def test_b2b_contract_attachment_invalid_code_dates(user, bad_start_or_end): contract = ContractPageFactory.create( membership_type=CONTRACT_MEMBERSHIP_NONSSO, + integration_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=1, ) @@ -142,6 +144,7 @@ def test_b2b_contract_attachment_invalid_contract_dates(user, bad_start_or_end): contract = ContractPageFactory.create( membership_type=CONTRACT_MEMBERSHIP_NONSSO, + integration_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=1, ) @@ -192,6 +195,7 @@ def test_b2b_contract_attachment_full_contract(mocker): contract = ContractPageFactory.create( membership_type=CONTRACT_MEMBERSHIP_NONSSO, + integration_type=CONTRACT_MEMBERSHIP_NONSSO, max_learners=1, ) @@ -252,6 +256,7 @@ def test_b2b_enroll(mocker, settings, user_has_edx_user, has_price): contract = ContractPageFactory.create( membership_type=CONTRACT_MEMBERSHIP_SSO, + integration_type=CONTRACT_MEMBERSHIP_SSO, enrollment_fixed_price=100 if has_price else 0, ) source_courserun = CourseRunFactory.create(is_source_run=True) diff --git a/courses/api_test.py b/courses/api_test.py index b92234a378..c4e5630edc 100644 --- a/courses/api_test.py +++ b/courses/api_test.py @@ -1808,6 +1808,7 @@ def test_b2b_re_enrollment_after_multiple_unenrollments(mocker, user): organization=org, enrollment_fixed_price=Decimal("0.00"), membership_type=CONTRACT_MEMBERSHIP_NONSSO, + integration_type=CONTRACT_MEMBERSHIP_NONSSO, ) course_run = CourseRunFactory.create(b2b_contract=contract) with reversion.create_revision(): diff --git a/courses/views/v2/__init__.py b/courses/views/v2/__init__.py index efde9a1239..d650cf0908 100644 --- a/courses/views/v2/__init__.py +++ b/courses/views/v2/__init__.py @@ -114,7 +114,7 @@ class ProgramViewSet(viewsets.ReadOnlyModelViewSet): def get_queryset(self): return ( Program.objects.filter() - .select_related("page") + .select_related("page", "page__feature_image") .prefetch_related( Prefetch("departments", queryset=Department.objects.only("id", "name")), Prefetch( diff --git a/users/models_test.py b/users/models_test.py index b13062adc9..d6dcba0b29 100644 --- a/users/models_test.py +++ b/users/models_test.py @@ -23,13 +23,6 @@ pytestmark = pytest.mark.django_db -@pytest.fixture -def mocked_b2b_org_attach(mocker): - """Mock the org attachment call.""" - - return mocker.patch("b2b.api.add_user_org_membership", return_value=True) - - @pytest.mark.parametrize( "create_func,exp_staff,exp_superuser,exp_is_active", # noqa: PT006 [ diff --git a/users/views_test.py b/users/views_test.py index 6009921d99..06f16b753c 100644 --- a/users/views_test.py +++ b/users/views_test.py @@ -74,6 +74,7 @@ def test_get_user_by_me(mocker, client, user, is_anonymous, has_orgs): "name": contract.name, "description": contract.description, "membership_type": contract.membership_type, + "integration_type": contract.integration_type, "contract_start": None, "contract_end": None, "active": True, From 8ab6272db960082a54ce10f374bc27769217466e Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 6 Oct 2025 15:54:13 -0500 Subject: [PATCH 09/15] Move the process_ membership calls to api rather than in the UserOrganization model as static methods --- b2b/api.py | 52 ++++++++++ b2b/api_test.py | 210 ++++++++++++++++++++++++++++++++++++++- b2b/models.py | 52 ---------- b2b/models_test.py | 205 +------------------------------------- b2b/views/v0/__init__.py | 5 +- courses/api.py | 3 +- 6 files changed, 265 insertions(+), 262 deletions(-) diff --git a/b2b/api.py b/b2b/api.py index b4820c5031..6438b5e09b 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -981,3 +981,55 @@ def add_user_org_membership(org, user): return False return org_model.associate("members", org.sso_organization_id, user.global_id) + + +def process_add_org_membership(user, organization, *, keep_until_seen=False): + """ + Add a user to an org, and kick off contract processing. + + This allows us to manage UserOrganization records without necessarily + being forced to process contract memberships at the same time. + + Args: + - user (users.models.User): the user to add + - organization (b2b.models.OrganizationPage): the organization to add the user to + - keep_until_seen (bool): if True, the user will be kept in the org until the + organization is seen in their SSO data. + """ + + obj, created = UserOrganization.objects.get_or_create( + user=user, + organization=organization, + ) + if created: + obj.keep_until_seen = keep_until_seen + obj.save() + try: + organization.attach_user(user) + except ConnectionError: + log.exception( + "Could not attach %s to Keycloak org for %s", user, organization + ) + organization.add_user_contracts(user) + + return obj + + +def process_remove_org_membership(user, organization): + """ + Remove a user from an org, and kick off contract processing. + + Other side of the process_add_org_membership function - removes the membership + and associated managed contracts. + + Args: + - user (users.models.User): the user to remove + - organization (b2b.models.OrganizationPage): the organization to remove the user from + """ + + organization.remove_user_contracts(user) + + UserOrganization.objects.filter( + user=user, + organization=organization, + ).get().delete() diff --git a/b2b/api_test.py b/b2b/api_test.py index d231fc468c..1d4c3f3e70 100644 --- a/b2b/api_test.py +++ b/b2b/api_test.py @@ -18,6 +18,8 @@ create_contract_run, ensure_enrollment_codes_exist, get_active_contracts_from_basket_items, + process_add_org_membership, + process_remove_org_membership, reconcile_keycloak_orgs, reconcile_single_keycloak_org, reconcile_user_orgs, @@ -29,10 +31,14 @@ CONTRACT_MEMBERSHIP_SSO, ) from b2b.exceptions import SourceCourseIncompleteError, TargetCourseRunExistsError -from b2b.factories import OrganizationPageFactory +from b2b.factories import ContractPageFactory, OrganizationPageFactory from b2b.models import OrganizationIndexPage, OrganizationPage, UserOrganization from courses.constants import UAI_COURSEWARE_ID_PREFIX -from courses.factories import CourseFactory, CourseRunFactory +from courses.factories import ( + CourseFactory, + CourseRunEnrollmentFactory, + CourseRunFactory, +) from courses.models import CourseRunEnrollment from ecommerce.api_test import create_basket from ecommerce.constants import REDEMPTION_TYPE_ONE_TIME, REDEMPTION_TYPE_UNLIMITED @@ -60,6 +66,13 @@ ] +@pytest.fixture +def mocked_b2b_org_attach(mocker): + """Mock the org attachment call.""" + + return mocker.patch("b2b.api.add_user_org_membership", return_value=True) + + @pytest.mark.parametrize( ( "has_start", @@ -744,3 +757,196 @@ def test_reconcile_bad_keycloak_org(mocker): page.save() assert "Organization with this Org key already exists." in str(exc) + + +def test_user_add_b2b_org(mocked_b2b_org_attach): + """Ensure adding a user to an organization works as expected.""" + + orgs = OrganizationPageFactory.create_batch(2) + user = UserFactory.create() + + # New-style ones + contract_auto = ContractPageFactory.create( + organization=orgs[0], + membership_type="auto", + integration_type="auto", + title="Contract Auto", + name="Contract Auto", + ) + contract_managed = ContractPageFactory.create( + organization=orgs[0], + membership_type="managed", + integration_type="managed", + title="Contract Managed", + name="Contract Managed", + ) + contract_code = ContractPageFactory.create( + organization=orgs[0], + membership_type="code", + integration_type="code", + title="Contract Enrollment Code", + name="Contract Enrollment Code", + ) + # Legacy ones - these will migrate to "managed" and "code" + contract_sso = ContractPageFactory.create( + organization=orgs[0], + membership_type="sso", + integration_type="sso", + title="Contract SSO", + name="Contract SSO", + ) + contract_non_sso = ContractPageFactory.create( + organization=orgs[0], + membership_type="non-sso", + integration_type="non-sso", + title="Contract NonSSO", + name="Contract NonSSO", + ) + + process_add_org_membership(user, orgs[0]) + + # We should now be in the SSO, auto, and managed contracts + # but not the other two. + + user.refresh_from_db() + assert user.b2b_contracts.count() == 3 + assert user.b2b_organizations.filter(organization=orgs[0]).exists() + assert ( + user.b2b_contracts.filter( + pk__in=[ + contract_auto.id, + contract_sso.id, + contract_managed.id, + ] + ).count() + == 3 + ) + assert ( + user.b2b_contracts.filter( + pk__in=[ + contract_code.id, + contract_non_sso.id, + ] + ).count() + == 0 + ) + + +def test_user_remove_b2b_org(mocked_b2b_org_attach): + """Ensure removing a user from an org also clears the appropriate contracts.""" + + orgs = OrganizationPageFactory.create_batch(2) + user = UserFactory.create() + + # New-style ones + contract_auto = ContractPageFactory.create( + organization=orgs[0], + membership_type="auto", + integration_type="auto", + title="Contract Auto", + name="Contract Auto", + ) + contract_managed = ContractPageFactory.create( + organization=orgs[0], + membership_type="managed", + integration_type="managed", + title="Contract Managed", + name="Contract Managed", + ) + contract_code = ContractPageFactory.create( + organization=orgs[1], + membership_type="code", + integration_type="code", + title="Contract Enrollment Code", + name="Contract Enrollment Code", + ) + # Legacy ones - these will migrate to "managed" and "code" + contract_sso = ContractPageFactory.create( + organization=orgs[0], + membership_type="sso", + integration_type="sso", + title="Contract SSO", + name="Contract SSO", + ) + contract_non_sso = ContractPageFactory.create( + organization=orgs[1], + membership_type="non-sso", + integration_type="non-sso", + title="Contract NonSSO", + name="Contract NonSSO", + ) + + managed_ids = [ + contract_auto.id, + contract_managed.id, + contract_sso.id, + ] + unmanaged_ids = [ + contract_code.id, + contract_non_sso.id, + ] + + process_add_org_membership(user, orgs[0]) + process_add_org_membership(user, orgs[1]) + + user.b2b_contracts.add(contract_code) + user.b2b_contracts.add(contract_non_sso) + user.save() + + user.refresh_from_db() + + assert user.b2b_contracts.count() == 5 + + process_remove_org_membership(user, orgs[1]) + + assert user.b2b_contracts.filter(id__in=managed_ids).count() == 3 + assert user.b2b_contracts.filter(id__in=unmanaged_ids).count() == 0 + + process_remove_org_membership(user, orgs[0]) + + # we should have no contracts now since we're no longer in any orgs + + assert user.b2b_contracts.count() == 0 + + +def test_b2b_contract_removal_keeps_enrollments(mocked_b2b_org_attach): + """Ensure that removing a user from a B2B contract leaves their enrollments alone.""" + + org = OrganizationPageFactory.create() + user = UserFactory.create() + + contract_auto = ContractPageFactory.create( + organization=org, + membership_type="auto", + integration_type="auto", + title="Contract Auto", + name="Contract Auto", + ) + + courserun = CourseRunFactory.create(b2b_contract=contract_auto) + + process_add_org_membership(user, org) + + CourseRunEnrollmentFactory( + user=user, + run=courserun, + ) + + user.refresh_from_db() + + assert courserun.enrollments.filter(user=user).count() == 1 + + process_remove_org_membership(user, org) + + assert courserun.enrollments.filter(user=user).count() == 1 + + +def test_b2b_org_attach_calls_keycloak(mocked_b2b_org_attach): + """Test that attaching a user to an org calls Keycloak successfully.""" + + org = OrganizationPageFactory.create() + user = UserFactory.create() + + process_add_org_membership(user, org) + + mocked_b2b_org_attach.assert_called() diff --git a/b2b/models.py b/b2b/models.py index 9d90e54c83..3f02bf4eef 100644 --- a/b2b/models.py +++ b/b2b/models.py @@ -446,55 +446,3 @@ def __str__(self): """Return a reasonable representation of the object as a string.""" return f"UserOrganization: {self.user} in {self.organization}" - - @staticmethod - def process_add_membership(user, organization, *, keep_until_seen=False): - """ - Add a user to an org, and kick off contract processing. - - This allows us to manage UserOrganization records without necessarily - being forced to process contract memberships at the same time. - - Args: - - user (users.models.User): the user to add - - organization (b2b.models.OrganizationPage): the organization to add the user to - - keep_until_seen (bool): if True, the user will be kept in the org until the - organization is seen in their SSO data. - """ - - obj, created = UserOrganization.objects.get_or_create( - user=user, - organization=organization, - ) - if created: - obj.keep_until_seen = keep_until_seen - obj.save() - try: - organization.attach_user(user) - except ConnectionError: - log.exception( - "Could not attach %s to Keycloak org for %s", user, organization - ) - organization.add_user_contracts(user) - - return obj - - @staticmethod - def process_remove_membership(user, organization): - """ - Remove a user from an org, and kick off contract processing. - - Other side of the process_add_membership function - removes the membership - and associated managed contracts. - - Args: - - user (users.models.User): the user to remove - - organization (b2b.models.OrganizationPage): the organization to remove the user from - """ - - organization.remove_user_contracts(user) - - UserOrganization.objects.filter( - user=user, - organization=organization, - ).get().delete() diff --git a/b2b/models_test.py b/b2b/models_test.py index 07e091a3dc..265962ba03 100644 --- a/b2b/models_test.py +++ b/b2b/models_test.py @@ -3,26 +3,16 @@ import faker import pytest -from b2b.factories import ContractPageFactory, OrganizationPageFactory -from b2b.models import UserOrganization +from b2b.factories import ContractPageFactory from courses.factories import ( - CourseRunEnrollmentFactory, CourseRunFactory, ProgramFactory, ) -from users.factories import UserFactory pytestmark = [pytest.mark.django_db] FAKE = faker.Faker() -@pytest.fixture -def mocked_b2b_org_attach(mocker): - """Mock the org attachment call.""" - - return mocker.patch("b2b.api.add_user_org_membership", return_value=True) - - def test_add_program_courses_to_contract(mocker): """Test that adding a program to a contract works as expected.""" @@ -63,196 +53,3 @@ def test_add_program_courses_to_contract(mocker): assert contract.programs.count() == 1 assert contract.get_course_runs().count() == 4 - - -def test_user_add_b2b_org(mocked_b2b_org_attach): - """Ensure adding a user to an organization works as expected.""" - - orgs = OrganizationPageFactory.create_batch(2) - user = UserFactory.create() - - # New-style ones - contract_auto = ContractPageFactory.create( - organization=orgs[0], - membership_type="auto", - integration_type="auto", - title="Contract Auto", - name="Contract Auto", - ) - contract_managed = ContractPageFactory.create( - organization=orgs[0], - membership_type="managed", - integration_type="managed", - title="Contract Managed", - name="Contract Managed", - ) - contract_code = ContractPageFactory.create( - organization=orgs[0], - membership_type="code", - integration_type="code", - title="Contract Enrollment Code", - name="Contract Enrollment Code", - ) - # Legacy ones - these will migrate to "managed" and "code" - contract_sso = ContractPageFactory.create( - organization=orgs[0], - membership_type="sso", - integration_type="sso", - title="Contract SSO", - name="Contract SSO", - ) - contract_non_sso = ContractPageFactory.create( - organization=orgs[0], - membership_type="non-sso", - integration_type="non-sso", - title="Contract NonSSO", - name="Contract NonSSO", - ) - - UserOrganization.process_add_membership(user, orgs[0]) - - # We should now be in the SSO, auto, and managed contracts - # but not the other two. - - user.refresh_from_db() - assert user.b2b_contracts.count() == 3 - assert user.b2b_organizations.filter(organization=orgs[0]).exists() - assert ( - user.b2b_contracts.filter( - pk__in=[ - contract_auto.id, - contract_sso.id, - contract_managed.id, - ] - ).count() - == 3 - ) - assert ( - user.b2b_contracts.filter( - pk__in=[ - contract_code.id, - contract_non_sso.id, - ] - ).count() - == 0 - ) - - -def test_user_remove_b2b_org(mocked_b2b_org_attach): - """Ensure removing a user from an org also clears the appropriate contracts.""" - - orgs = OrganizationPageFactory.create_batch(2) - user = UserFactory.create() - - # New-style ones - contract_auto = ContractPageFactory.create( - organization=orgs[0], - membership_type="auto", - integration_type="auto", - title="Contract Auto", - name="Contract Auto", - ) - contract_managed = ContractPageFactory.create( - organization=orgs[0], - membership_type="managed", - integration_type="managed", - title="Contract Managed", - name="Contract Managed", - ) - contract_code = ContractPageFactory.create( - organization=orgs[1], - membership_type="code", - integration_type="code", - title="Contract Enrollment Code", - name="Contract Enrollment Code", - ) - # Legacy ones - these will migrate to "managed" and "code" - contract_sso = ContractPageFactory.create( - organization=orgs[0], - membership_type="sso", - integration_type="sso", - title="Contract SSO", - name="Contract SSO", - ) - contract_non_sso = ContractPageFactory.create( - organization=orgs[1], - membership_type="non-sso", - integration_type="non-sso", - title="Contract NonSSO", - name="Contract NonSSO", - ) - - managed_ids = [ - contract_auto.id, - contract_managed.id, - contract_sso.id, - ] - unmanaged_ids = [ - contract_code.id, - contract_non_sso.id, - ] - - UserOrganization.process_add_membership(user, orgs[0]) - UserOrganization.process_add_membership(user, orgs[1]) - - user.b2b_contracts.add(contract_code) - user.b2b_contracts.add(contract_non_sso) - user.save() - - user.refresh_from_db() - - assert user.b2b_contracts.count() == 5 - - UserOrganization.process_remove_membership(user, orgs[1]) - - assert user.b2b_contracts.filter(id__in=managed_ids).count() == 3 - assert user.b2b_contracts.filter(id__in=unmanaged_ids).count() == 0 - - UserOrganization.process_remove_membership(user, orgs[0]) - - # we should have no contracts now since we're no longer in any orgs - - assert user.b2b_contracts.count() == 0 - - -def test_b2b_contract_removal_keeps_enrollments(mocked_b2b_org_attach): - """Ensure that removing a user from a B2B contract leaves their enrollments alone.""" - - org = OrganizationPageFactory.create() - user = UserFactory.create() - - contract_auto = ContractPageFactory.create( - organization=org, - membership_type="auto", - integration_type="auto", - title="Contract Auto", - name="Contract Auto", - ) - - courserun = CourseRunFactory.create(b2b_contract=contract_auto) - - UserOrganization.process_add_membership(user, org) - - CourseRunEnrollmentFactory( - user=user, - run=courserun, - ) - - user.refresh_from_db() - - assert courserun.enrollments.filter(user=user).count() == 1 - - UserOrganization.process_remove_membership(user, org) - - assert courserun.enrollments.filter(user=user).count() == 1 - - -def test_b2b_org_attach_calls_keycloak(mocked_b2b_org_attach): - """Test that attaching a user to an org calls Keycloak successfully.""" - - org = OrganizationPageFactory.create() - user = UserFactory.create() - - UserOrganization.process_add_membership(user, org) - - mocked_b2b_org_attach.assert_called() diff --git a/b2b/views/v0/__init__.py b/b2b/views/v0/__init__.py index a061c1af31..7289660b4b 100644 --- a/b2b/views/v0/__init__.py +++ b/b2b/views/v0/__init__.py @@ -11,12 +11,11 @@ from rest_framework.views import APIView from rest_framework_api_key.permissions import HasAPIKey -from b2b.api import create_b2b_enrollment +from b2b.api import create_b2b_enrollment, process_add_org_membership from b2b.models import ( ContractPage, DiscountContractAttachmentRedemption, OrganizationPage, - UserOrganization, ) from b2b.serializers.v0 import ( ContractPageSerializer, @@ -142,7 +141,7 @@ def post(self, request, enrollment_code: str, format=None): # noqa: A002, ARG00 if contract.is_full(): continue - UserOrganization.process_add_membership( + process_add_org_membership( request.user, contract.organization, keep_until_seen=True ) request.user.b2b_contracts.add(contract) diff --git a/courses/api.py b/courses/api.py index b04e77c97d..c8f39ac8a2 100644 --- a/courses/api.py +++ b/courses/api.py @@ -28,6 +28,7 @@ from requests.exceptions import HTTPError from rest_framework.status import HTTP_404_NOT_FOUND +from b2b.api import process_add_org_membership from b2b.models import UserOrganization from cms.api import create_default_courseware_page from courses import mail_api @@ -217,7 +218,7 @@ def _enroll_learner_into_associated_programs(): # If the run is associated with a B2B contract, add the contract # to the user's contract list and update their org memberships if run.b2b_contract: - UserOrganization.process_add_membership( + process_add_org_membership( user, run.b2b_contract.organization, keep_until_seen=True ) user.b2b_contracts.add(run.b2b_contract) From b728f05bc81a9b929ef8fd08d1447bf13d0cb1ba Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 6 Oct 2025 16:21:33 -0500 Subject: [PATCH 10/15] fix issue with openapi spec --- b2b/serializers/v0/__init__.py | 4 ++++ openapi/specs/v0.yaml | 41 +++++++++++++++++++++++++++------- openapi/specs/v1.yaml | 41 +++++++++++++++++++++++++++------- openapi/specs/v2.yaml | 41 +++++++++++++++++++++++++++------- 4 files changed, 103 insertions(+), 24 deletions(-) diff --git a/b2b/serializers/v0/__init__.py b/b2b/serializers/v0/__init__.py index 3ce59ce5b7..f7855a50ec 100644 --- a/b2b/serializers/v0/__init__.py +++ b/b2b/serializers/v0/__init__.py @@ -3,6 +3,7 @@ from drf_spectacular.utils import extend_schema_field from rest_framework import serializers +from b2b.constants import CONTRACT_MEMBERSHIP_CHOICES, CONTRACT_MEMBERSHIP_TYPE_CHOICES from b2b.models import ContractPage, OrganizationPage, UserOrganization from cms.api import get_wagtail_img_src from main.constants import USER_MSG_TYPE_B2B_CHOICES @@ -13,6 +14,9 @@ class ContractPageSerializer(serializers.ModelSerializer): Serializer for the ContractPage model. """ + integration_type = serializers.CharField(choices=CONTRACT_MEMBERSHIP_CHOICES) + membership_type = serializers.CharField(choices=CONTRACT_MEMBERSHIP_TYPE_CHOICES) + class Meta: model = ContractPage fields = [ diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 5ba87fcf31..acb0a2debb 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -1510,6 +1510,18 @@ components: type: string readOnly: true description: Any useful extra information about the contract. + integration_type: + allOf: + - $ref: '#/components/schemas/IntegrationTypeEnum' + readOnly: true + description: |- + The type of integration for this contract. + + * `sso` - SSO + * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment membership_type: allOf: - $ref: '#/components/schemas/MembershipTypeEnum' @@ -1553,6 +1565,7 @@ components: - contract_start - description - id + - integration_type - membership_type - name - organization @@ -2537,6 +2550,26 @@ components: - Elementary/primary school - No formal education - Other education + IntegrationTypeEnum: + enum: + - sso + - non-sso + - managed + - code + - auto + type: string + description: |- + * `sso` - SSO + * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment + x-enum-descriptions: + - SSO + - Non-SSO + - Managed + - Enrollment Code + - Auto Enrollment LearnerProgramRecordShare: type: object properties: @@ -3621,24 +3654,16 @@ components: properties: id: type: integer - description: Get id - readOnly: true name: type: string - description: Get name - readOnly: true description: type: string - description: Get description - readOnly: true logo: type: string description: Get logo readOnly: true slug: type: string - description: Get slug - readOnly: true contracts: type: array items: diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 652f6739d9..18fceb09da 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -1510,6 +1510,18 @@ components: type: string readOnly: true description: Any useful extra information about the contract. + integration_type: + allOf: + - $ref: '#/components/schemas/IntegrationTypeEnum' + readOnly: true + description: |- + The type of integration for this contract. + + * `sso` - SSO + * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment membership_type: allOf: - $ref: '#/components/schemas/MembershipTypeEnum' @@ -1553,6 +1565,7 @@ components: - contract_start - description - id + - integration_type - membership_type - name - organization @@ -2537,6 +2550,26 @@ components: - Elementary/primary school - No formal education - Other education + IntegrationTypeEnum: + enum: + - sso + - non-sso + - managed + - code + - auto + type: string + description: |- + * `sso` - SSO + * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment + x-enum-descriptions: + - SSO + - Non-SSO + - Managed + - Enrollment Code + - Auto Enrollment LearnerProgramRecordShare: type: object properties: @@ -3621,24 +3654,16 @@ components: properties: id: type: integer - description: Get id - readOnly: true name: type: string - description: Get name - readOnly: true description: type: string - description: Get description - readOnly: true logo: type: string description: Get logo readOnly: true slug: type: string - description: Get slug - readOnly: true contracts: type: array items: diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index 24f65db009..fb55a4dc34 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -1510,6 +1510,18 @@ components: type: string readOnly: true description: Any useful extra information about the contract. + integration_type: + allOf: + - $ref: '#/components/schemas/IntegrationTypeEnum' + readOnly: true + description: |- + The type of integration for this contract. + + * `sso` - SSO + * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment membership_type: allOf: - $ref: '#/components/schemas/MembershipTypeEnum' @@ -1553,6 +1565,7 @@ components: - contract_start - description - id + - integration_type - membership_type - name - organization @@ -2537,6 +2550,26 @@ components: - Elementary/primary school - No formal education - Other education + IntegrationTypeEnum: + enum: + - sso + - non-sso + - managed + - code + - auto + type: string + description: |- + * `sso` - SSO + * `non-sso` - Non-SSO + * `managed` - Managed + * `code` - Enrollment Code + * `auto` - Auto Enrollment + x-enum-descriptions: + - SSO + - Non-SSO + - Managed + - Enrollment Code + - Auto Enrollment LearnerProgramRecordShare: type: object properties: @@ -3621,24 +3654,16 @@ components: properties: id: type: integer - description: Get id - readOnly: true name: type: string - description: Get name - readOnly: true description: type: string - description: Get description - readOnly: true logo: type: string description: Get logo readOnly: true slug: type: string - description: Get slug - readOnly: true contracts: type: array items: From b0bf505028adcd3266166610f41d37153959b158 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 6 Oct 2025 16:50:36 -0500 Subject: [PATCH 11/15] fixing openapi spec properlyish this time made membership_type a char field - we don't allow writing via the serializer anyway; when we remove the integration_type field, membership_type can have the choices instead --- b2b/serializers/v0/__init__.py | 4 +--- openapi/specs/v0.yaml | 32 +------------------------------- openapi/specs/v1.yaml | 32 +------------------------------- openapi/specs/v2.yaml | 32 +------------------------------- 4 files changed, 4 insertions(+), 96 deletions(-) diff --git a/b2b/serializers/v0/__init__.py b/b2b/serializers/v0/__init__.py index f7855a50ec..232f788533 100644 --- a/b2b/serializers/v0/__init__.py +++ b/b2b/serializers/v0/__init__.py @@ -3,7 +3,6 @@ from drf_spectacular.utils import extend_schema_field from rest_framework import serializers -from b2b.constants import CONTRACT_MEMBERSHIP_CHOICES, CONTRACT_MEMBERSHIP_TYPE_CHOICES from b2b.models import ContractPage, OrganizationPage, UserOrganization from cms.api import get_wagtail_img_src from main.constants import USER_MSG_TYPE_B2B_CHOICES @@ -14,8 +13,7 @@ class ContractPageSerializer(serializers.ModelSerializer): Serializer for the ContractPage model. """ - integration_type = serializers.CharField(choices=CONTRACT_MEMBERSHIP_CHOICES) - membership_type = serializers.CharField(choices=CONTRACT_MEMBERSHIP_TYPE_CHOICES) + membership_type = serializers.CharField() class Meta: model = ContractPage diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index acb0a2debb..6c85be86fc 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -1523,17 +1523,7 @@ components: * `code` - Enrollment Code * `auto` - Auto Enrollment membership_type: - allOf: - - $ref: '#/components/schemas/MembershipTypeEnum' - readOnly: true - description: |- - The method to use to manage membership in the contract. - - * `sso` - SSO - * `non-sso` - Non-SSO - * `managed` - Managed - * `code` - Enrollment Code - * `auto` - Auto Enrollment + type: string organization: type: integer readOnly: true @@ -2673,26 +2663,6 @@ components: maxLength: 10 required: - country - MembershipTypeEnum: - enum: - - sso - - non-sso - - managed - - code - - auto - type: string - description: |- - * `sso` - SSO - * `non-sso` - Non-SSO - * `managed` - Managed - * `code` - Enrollment Code - * `auto` - Auto Enrollment - x-enum-descriptions: - - SSO - - Non-SSO - - Managed - - Enrollment Code - - Auto Enrollment NullEnum: enum: - null diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 18fceb09da..95c7cd9850 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -1523,17 +1523,7 @@ components: * `code` - Enrollment Code * `auto` - Auto Enrollment membership_type: - allOf: - - $ref: '#/components/schemas/MembershipTypeEnum' - readOnly: true - description: |- - The method to use to manage membership in the contract. - - * `sso` - SSO - * `non-sso` - Non-SSO - * `managed` - Managed - * `code` - Enrollment Code - * `auto` - Auto Enrollment + type: string organization: type: integer readOnly: true @@ -2673,26 +2663,6 @@ components: maxLength: 10 required: - country - MembershipTypeEnum: - enum: - - sso - - non-sso - - managed - - code - - auto - type: string - description: |- - * `sso` - SSO - * `non-sso` - Non-SSO - * `managed` - Managed - * `code` - Enrollment Code - * `auto` - Auto Enrollment - x-enum-descriptions: - - SSO - - Non-SSO - - Managed - - Enrollment Code - - Auto Enrollment NullEnum: enum: - null diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index fb55a4dc34..06faa156d4 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -1523,17 +1523,7 @@ components: * `code` - Enrollment Code * `auto` - Auto Enrollment membership_type: - allOf: - - $ref: '#/components/schemas/MembershipTypeEnum' - readOnly: true - description: |- - The method to use to manage membership in the contract. - - * `sso` - SSO - * `non-sso` - Non-SSO - * `managed` - Managed - * `code` - Enrollment Code - * `auto` - Auto Enrollment + type: string organization: type: integer readOnly: true @@ -2673,26 +2663,6 @@ components: maxLength: 10 required: - country - MembershipTypeEnum: - enum: - - sso - - non-sso - - managed - - code - - auto - type: string - description: |- - * `sso` - SSO - * `non-sso` - Non-SSO - * `managed` - Managed - * `code` - Enrollment Code - * `auto` - Auto Enrollment - x-enum-descriptions: - - SSO - - Non-SSO - - Managed - - Enrollment Code - - Auto Enrollment NullEnum: enum: - null From 618530afdea25cf3f972cafadf58d27537e258ad Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 8 Oct 2025 14:52:45 -0500 Subject: [PATCH 12/15] rework the b2b_organizations m2m relationship --- b2b/api.py | 4 +-- ...9_contractpage_membership_type_and_more.py | 25 ++----------------- b2b/models.py | 2 +- courses/views/v2/__init__.py | 2 +- .../migrations/0038_user_b2b_organizations.py | 23 +++++++++++++++++ users/models.py | 14 ++++++++--- users/serializers.py | 10 ++++---- 7 files changed, 44 insertions(+), 36 deletions(-) create mode 100644 users/migrations/0038_user_b2b_organizations.py diff --git a/b2b/api.py b/b2b/api.py index 6438b5e09b..2a4ccda425 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -863,10 +863,10 @@ def reconcile_user_orgs(user, organizations): user.refresh_from_db() orgs = [ (str(org.organization.sso_organization_id), not org.keep_until_seen) - for org in user.b2b_organizations.all() + for org in user.user_organizations.all() ] - user.b2b_organizations.filter( + user.user_organizations.filter( organization__sso_organization_id__in=organizations, keep_until_seen=True ).update(keep_until_seen=False) diff --git a/b2b/migrations/0009_contractpage_membership_type_and_more.py b/b2b/migrations/0009_contractpage_membership_type_and_more.py index 39127f1f56..a0ea3a074b 100644 --- a/b2b/migrations/0009_contractpage_membership_type_and_more.py +++ b/b2b/migrations/0009_contractpage_membership_type_and_more.py @@ -1,27 +1,10 @@ -# Generated by Django 4.2.25 on 2025-10-06 20:26 +# Generated by Django 4.2.25 on 2025-10-08 19:32 import django.db.models.deletion from django.conf import settings from django.db import migrations, models -def copy_integration_type_to_membership_type(apps, schema_editor): - """Copy existing values from integration_type to membership_type.""" - ContractPage = apps.get_model("b2b", "ContractPage") - for contract in ContractPage.objects.all(): - if contract.integration_type == "sso": - contract.membership_type = "sso" - elif contract.integration_type == "non-sso": - contract.membership_type = "non-sso" - else: - contract.membership_type = "managed" # Default to managed for other cases - contract.save() - - -def noop(apps, schema_editor): - """No operation (placeholder for reverse migration).""" - - class Migration(migrations.Migration): dependencies = [ migrations.swappable_dependency(settings.AUTH_USER_MODEL), @@ -101,7 +84,7 @@ class Migration(migrations.Migration): "user", models.ForeignKey( on_delete=django.db.models.deletion.CASCADE, - related_name="b2b_organizations", + related_name="user_organizations", to=settings.AUTH_USER_MODEL, ), ), @@ -110,8 +93,4 @@ class Migration(migrations.Migration): "unique_together": {("user", "organization")}, }, ), - migrations.RunPython( - code=copy_integration_type_to_membership_type, - reverse_code=noop, - ), ] diff --git a/b2b/models.py b/b2b/models.py index 3f02bf4eef..e1846f7206 100644 --- a/b2b/models.py +++ b/b2b/models.py @@ -427,7 +427,7 @@ class UserOrganization(models.Model): user = models.ForeignKey( settings.AUTH_USER_MODEL, on_delete=models.CASCADE, - related_name="b2b_organizations", + related_name="user_organizations", ) organization = models.ForeignKey( "b2b.OrganizationPage", diff --git a/courses/views/v2/__init__.py b/courses/views/v2/__init__.py index d650cf0908..4d5ce1c035 100644 --- a/courses/views/v2/__init__.py +++ b/courses/views/v2/__init__.py @@ -71,7 +71,7 @@ def user_has_org_access(user, org_id): user and user.is_authenticated and org_id - and user.b2b_organizations.filter(organization__id=org_id).exists() + and user.b2b_organizations.filter(id=org_id).exists() ) diff --git a/users/migrations/0038_user_b2b_organizations.py b/users/migrations/0038_user_b2b_organizations.py new file mode 100644 index 0000000000..a500f45cd7 --- /dev/null +++ b/users/migrations/0038_user_b2b_organizations.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.25 on 2025-10-08 19:34 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("b2b", "0009_contractpage_membership_type_and_more"), + ("users", "0037_add_global_id_default"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="b2b_organizations", + field=models.ManyToManyField( + help_text="The organizations the user is associated with.", + related_name="+", + through="b2b.UserOrganization", + to="b2b.organizationpage", + ), + ), + ] diff --git a/users/models.py b/users/models.py index 0d76a9a497..9bcaf96943 100644 --- a/users/models.py +++ b/users/models.py @@ -308,6 +308,12 @@ class User( related_name="users", help_text="The contracts the user is associated with.", ) + b2b_organizations = models.ManyToManyField( + "b2b.OrganizationPage", + through="b2b.UserOrganization", + related_name="+", + help_text="The organizations the user is associated with.", + ) objects = UserManager() faulty_openedx_users = FaultyOpenEdxUserManager() @@ -366,11 +372,11 @@ def is_editor(self) -> bool: @cached_property def b2b_organization_sso_ids(self): - """Similar to b2b_organizations, but returns just the UUIDs.""" + """Just the UUIDs for the organizations the user is in.""" return list( - self.b2b_organizations.filter( - sso_organization_id__isnull=False - ).values_list("organization__sso_organization_id", flat=True) + self.organizations.filter(sso_organization_id__isnull=False).values_list( + "sso_organization_id", flat=True + ) ) @property diff --git a/users/serializers.py b/users/serializers.py index e286eabeda..2bae801841 100644 --- a/users/serializers.py +++ b/users/serializers.py @@ -11,7 +11,7 @@ from rest_framework import serializers from social_django.models import UserSocialAuth -from b2b.serializers.v0 import UserOrganizationSerializer +from b2b.serializers.v0 import OrganizationPageSerializer from hubspot_sync.task_helpers import sync_hubspot_user # from ecommerce.api import fetch_and_serialize_unused_coupons # noqa: ERA001 @@ -246,15 +246,15 @@ def validate_username(self, value): def get_grants(self, instance): return instance.get_all_permissions() - @extend_schema_field(UserOrganizationSerializer(many=True)) + @extend_schema_field(OrganizationPageSerializer(many=True)) def get_b2b_organizations(self, instance): """Get the organizations for the user""" if instance.is_anonymous: return [] - organizations = instance.b2b_organizations - return UserOrganizationSerializer( - organizations, many=True, context={"user": instance} + return OrganizationPageSerializer( + instance.b2b_organizations, + many=True, ).data def validate(self, data): From bea2e61db42e440596643fb3749ca6522aa60ea6 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 9 Oct 2025 08:31:30 -0500 Subject: [PATCH 13/15] fixing tests after reworking the m2m --- b2b/api.py | 4 ++-- b2b/api_test.py | 40 ++++++++++++---------------------- b2b/views/v0/views_test.py | 6 ++--- courses/views/v2/views_test.py | 6 ++--- users/views_test.py | 4 ++-- 5 files changed, 23 insertions(+), 37 deletions(-) diff --git a/b2b/api.py b/b2b/api.py index 2a4ccda425..03aa715f10 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -603,8 +603,8 @@ def ensure_enrollment_codes_exist(contract: ContractPage): if ( contract.integration_type in CONTRACT_MEMBERSHIP_AUTOS - and not contract.enrollment_fixed_price - ): + or contract.membership_type in CONTRACT_MEMBERSHIP_AUTOS + ) and not contract.enrollment_fixed_price: # SSO contracts w/out price don't need discounts. return _handle_sso_free_contract(contract) diff --git a/b2b/api_test.py b/b2b/api_test.py index 1d4c3f3e70..b3112e98ab 100644 --- a/b2b/api_test.py +++ b/b2b/api_test.py @@ -565,13 +565,9 @@ def test_b2b_reconcile_user_orgs(): # noqa: PLR0915 user.refresh_from_db() assert user.b2b_organizations.count() == 1 - assert user.b2b_organizations.filter(organization=organization_to_add).exists() - assert not user.b2b_organizations.filter( - organization=organization_to_ignore - ).exists() - assert not user.b2b_organizations.filter( - organization=organization_to_remove - ).exists() + assert user.b2b_organizations.filter(pk=organization_to_add.id).exists() + assert not user.b2b_organizations.filter(pk=organization_to_ignore.id).exists() + assert not user.b2b_organizations.filter(pk=organization_to_remove.id).exists() # Step 2: Add an org through a back channel, and then reconcile # The org should be removed @@ -591,13 +587,9 @@ def test_b2b_reconcile_user_orgs(): # noqa: PLR0915 user.refresh_from_db() assert user.b2b_organizations.count() == 1 - assert user.b2b_organizations.filter(organization=organization_to_add).exists() - assert not user.b2b_organizations.filter( - organization=organization_to_ignore - ).exists() - assert not user.b2b_organizations.filter( - organization=organization_to_remove - ).exists() + assert user.b2b_organizations.filter(pk=organization_to_add.id).exists() + assert not user.b2b_organizations.filter(pk=organization_to_ignore.id).exists() + assert not user.b2b_organizations.filter(pk=organization_to_remove.id).exists() # Step 3: Add the remove org, but set the flag so it should be kept now. @@ -616,11 +608,9 @@ def test_b2b_reconcile_user_orgs(): # noqa: PLR0915 user.refresh_from_db() assert user.b2b_organizations.count() == 2 - assert user.b2b_organizations.filter(organization=organization_to_add).exists() - assert not user.b2b_organizations.filter( - organization=organization_to_ignore - ).exists() - assert user.b2b_organizations.filter(organization=organization_to_remove).exists() + assert user.b2b_organizations.filter(pk=organization_to_add.id).exists() + assert not user.b2b_organizations.filter(pk=organization_to_ignore.id).exists() + assert user.b2b_organizations.filter(pk=organization_to_remove.id).exists() # Step 3.5: now reconcile with the remove org, we should clear the flag @@ -637,11 +627,9 @@ def test_b2b_reconcile_user_orgs(): # noqa: PLR0915 user.refresh_from_db() assert user.b2b_organizations.count() == 2 - assert user.b2b_organizations.filter(organization=organization_to_add).exists() - assert not user.b2b_organizations.filter( - organization=organization_to_ignore - ).exists() - assert user.b2b_organizations.filter(organization=organization_to_remove).exists() + assert user.b2b_organizations.filter(pk=organization_to_add.id).exists() + assert not user.b2b_organizations.filter(pk=organization_to_ignore.id).exists() + assert user.b2b_organizations.filter(pk=organization_to_remove.id).exists() # Step 4: add the weird org that doesn't have a UUID # Legacy non-manged orgs won't have a UUID, so we should leave them alone @@ -663,7 +651,7 @@ def test_b2b_reconcile_user_orgs(): # noqa: PLR0915 user.refresh_from_db() assert user.b2b_organizations.count() == 3 - assert user.b2b_organizations.filter(organization=weird_organization).exists() + assert user.b2b_organizations.filter(pk=weird_organization.id).exists() @pytest.mark.parametrize( @@ -810,7 +798,7 @@ def test_user_add_b2b_org(mocked_b2b_org_attach): user.refresh_from_db() assert user.b2b_contracts.count() == 3 - assert user.b2b_organizations.filter(organization=orgs[0]).exists() + assert user.b2b_organizations.filter(pk=orgs[0].id).exists() assert ( user.b2b_contracts.filter( pk__in=[ diff --git a/b2b/views/v0/views_test.py b/b2b/views/v0/views_test.py index 6daf3913ea..5c15d49cf0 100644 --- a/b2b/views/v0/views_test.py +++ b/b2b/views/v0/views_test.py @@ -70,7 +70,7 @@ def test_b2b_contract_attachment(mocker, user): mocked_attach_user.assert_called() user.refresh_from_db() - assert user.b2b_organizations.filter(organization=contract.organization).exists() + assert user.b2b_organizations.filter(pk=contract.organization.id).exists() assert user.b2b_contracts.filter(pk=contract.id).exists() assert DiscountContractAttachmentRedemption.objects.filter( @@ -235,9 +235,7 @@ def test_b2b_contract_attachment_full_contract(mocker): mocked_attach_user.assert_not_called() user.refresh_from_db() - assert not user.b2b_organizations.filter( - organization=contract.organization - ).exists() + assert not user.b2b_organizations.filter(pk=contract.organization.id).exists() assert not user.b2b_contracts.filter(pk=contract.id).exists() assert not DiscountContractAttachmentRedemption.objects.filter( contract=contract, user=user, discount=contract_code diff --git a/courses/views/v2/views_test.py b/courses/views/v2/views_test.py index 5d168ca4a2..8debd61c31 100644 --- a/courses/views/v2/views_test.py +++ b/courses/views/v2/views_test.py @@ -310,7 +310,7 @@ def test_filter_with_org_id_returns_contracted_course( org = OrganizationPageFactory(name="Test Org") contract = ContractPageFactory(organization=org, active=True) user = UserFactory() - user.b2b_organizations.create(organization=org) + user.b2b_organizations.add(org) user.b2b_contracts.add(contract) user.refresh_from_db() @@ -501,7 +501,7 @@ def test_next_run_id_with_org_filter( # noqa: PLR0915 contract = ContractPageFactory.create(organization=orgs[0]) second_contract = ContractPageFactory.create(organization=orgs[1]) test_user = UserFactory() - test_user.b2b_organizations.create(organization=contract.organization) + test_user.b2b_organizations.add(contract.organization) test_user.b2b_contracts.add(contract) test_user.save() test_user.refresh_from_db() @@ -643,7 +643,7 @@ def test_program_filter_for_b2b_org(user, mock_course_run_clone): contract.add_program_courses(b2b_program) contract.save() - user.b2b_organizations.create(organization=org) + user.b2b_organizations.add(org) user.b2b_contracts.add(contract) user.save() diff --git a/users/views_test.py b/users/views_test.py index 06f16b753c..1d88bf15cc 100644 --- a/users/views_test.py +++ b/users/views_test.py @@ -58,7 +58,7 @@ def test_get_user_by_me(mocker, client, user, is_anonymous, has_orgs): if has_orgs: contract = ContractPageFactory.create() - user.b2b_organizations.create(organization=contract.organization) + user.b2b_organizations.add(contract.organization) user.b2b_contracts.add(contract) user.save() b2b_orgs = [ @@ -66,7 +66,7 @@ def test_get_user_by_me(mocker, client, user, is_anonymous, has_orgs): "id": contract.organization.id, "name": contract.organization.name, "description": contract.organization.description, - "logo": "", + "logo": None, "slug": contract.organization.slug, "contracts": [ { From ede25e2e3dff3aab6dff0bce85c32549e952113f Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 9 Oct 2025 09:43:29 -0500 Subject: [PATCH 14/15] regenerate openapi spec, try to make this unrelated test a bit more robust test_discounted_price is somewhat flaky, but can't really ID why, because the factory randomizes some stuff. So, control said randomization; maybe if it flakes out again it'll be easier to debug. --- ecommerce/discounts_test.py | 8 ++++++-- openapi/specs/v0.yaml | 36 +----------------------------------- openapi/specs/v1.yaml | 36 +----------------------------------- openapi/specs/v2.yaml | 36 +----------------------------------- 4 files changed, 9 insertions(+), 107 deletions(-) diff --git a/ecommerce/discounts_test.py b/ecommerce/discounts_test.py index dea01d1d20..04634fde62 100644 --- a/ecommerce/discounts_test.py +++ b/ecommerce/discounts_test.py @@ -3,6 +3,7 @@ import pytest +from ecommerce.constants import ALL_DISCOUNT_TYPES from ecommerce.discounts import ( DiscountType, DollarsOffDiscount, @@ -85,14 +86,17 @@ def test_discount_factory_adjustment(discounts, products): ) -def test_discounted_price(products): +@pytest.mark.parametrize("discount_type", ALL_DISCOUNT_TYPES) +def test_discounted_price(products, discount_type): """ Tests the get_discounted_price call with some products to make sure the discount is applied successfully. """ product = products[random.randrange(0, len(products), 1)] # noqa: S311 - applied_discounts = [UnlimitedUseDiscountFactory.create()] + applied_discounts = [ + UnlimitedUseDiscountFactory.create(discount_type=discount_type) + ] manually_discounted_prices = DiscountType.for_discount( applied_discounts[0] diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 6c85be86fc..b9ffe8c8b4 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -3598,7 +3598,7 @@ components: b2b_organizations: type: array items: - $ref: '#/components/schemas/UserOrganization' + $ref: '#/components/schemas/OrganizationPage' readOnly: true required: - b2b_organizations @@ -3612,40 +3612,6 @@ components: - is_superuser - legal_address - updated_on - UserOrganization: - type: object - description: |- - Serializer for user organization data. - - Return the user's organizations in a manner that makes them look like - OrganizationPage objects. (Previously, the user organizations were a queryset - of OrganizationPages that related to the user, but now we have a through - table.) - properties: - id: - type: integer - name: - type: string - description: - type: string - logo: - type: string - description: Get logo - readOnly: true - slug: - type: string - contracts: - type: array - items: - $ref: '#/components/schemas/ContractPage' - readOnly: true - required: - - contracts - - description - - id - - logo - - name - - slug UserProfile: type: object description: Serializer for profile diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 95c7cd9850..e20a068305 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -3598,7 +3598,7 @@ components: b2b_organizations: type: array items: - $ref: '#/components/schemas/UserOrganization' + $ref: '#/components/schemas/OrganizationPage' readOnly: true required: - b2b_organizations @@ -3612,40 +3612,6 @@ components: - is_superuser - legal_address - updated_on - UserOrganization: - type: object - description: |- - Serializer for user organization data. - - Return the user's organizations in a manner that makes them look like - OrganizationPage objects. (Previously, the user organizations were a queryset - of OrganizationPages that related to the user, but now we have a through - table.) - properties: - id: - type: integer - name: - type: string - description: - type: string - logo: - type: string - description: Get logo - readOnly: true - slug: - type: string - contracts: - type: array - items: - $ref: '#/components/schemas/ContractPage' - readOnly: true - required: - - contracts - - description - - id - - logo - - name - - slug UserProfile: type: object description: Serializer for profile diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index 06faa156d4..eca38f4cf1 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -3598,7 +3598,7 @@ components: b2b_organizations: type: array items: - $ref: '#/components/schemas/UserOrganization' + $ref: '#/components/schemas/OrganizationPage' readOnly: true required: - b2b_organizations @@ -3612,40 +3612,6 @@ components: - is_superuser - legal_address - updated_on - UserOrganization: - type: object - description: |- - Serializer for user organization data. - - Return the user's organizations in a manner that makes them look like - OrganizationPage objects. (Previously, the user organizations were a queryset - of OrganizationPages that related to the user, but now we have a through - table.) - properties: - id: - type: integer - name: - type: string - description: - type: string - logo: - type: string - description: Get logo - readOnly: true - slug: - type: string - contracts: - type: array - items: - $ref: '#/components/schemas/ContractPage' - readOnly: true - required: - - contracts - - description - - id - - logo - - name - - slug UserProfile: type: object description: Serializer for profile From bfe145c048bb4bc38275c01f6a11304968b5dda8 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 16 Oct 2025 14:44:28 -0500 Subject: [PATCH 15/15] cleaning out merge cruft --- b2b/management/commands/b2b_contract.py | 5 ----- courses/api.py | 1 - 2 files changed, 6 deletions(-) diff --git a/b2b/management/commands/b2b_contract.py b/b2b/management/commands/b2b_contract.py index 58382cac1e..96c8991b59 100644 --- a/b2b/management/commands/b2b_contract.py +++ b/b2b/management/commands/b2b_contract.py @@ -6,14 +6,9 @@ from django.core.management import BaseCommand, CommandError from django.db.models import Q -<<<<<<< HEAD -from b2b.constants import CONTRACT_INTEGRATION_NONSSO, CONTRACT_INTEGRATION_SSO -======= -from b2b.api import create_contract_run from b2b.constants import ( CONTRACT_MEMBERSHIP_CHOICES, ) ->>>>>>> 0528cf9f (Update integration type field to membership type, update org/contract attachment points, update org membership in Keycloak where approrpiate) from b2b.models import ContractPage, OrganizationIndexPage, OrganizationPage log = logging.getLogger(__name__) diff --git a/courses/api.py b/courses/api.py index c8f39ac8a2..b6c87780a9 100644 --- a/courses/api.py +++ b/courses/api.py @@ -29,7 +29,6 @@ from rest_framework.status import HTTP_404_NOT_FOUND from b2b.api import process_add_org_membership -from b2b.models import UserOrganization from cms.api import create_default_courseware_page from courses import mail_api from courses.constants import (