From 37120d5936a5f608e402acd36ed83ebc3d445f70 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 9 Sep 2026 16:04:06 -0500 Subject: [PATCH 01/11] Add a FK to ContractPage for ProgramEnrollment and CourseRunEnrollment, and a backfill migration It will become harder to figure out whether or not a particular enrollment is for a B2B run or not, once we allow course runs to be in multiple contracts. So, adding a field to track that specifically (to be followed by API changes to ensure that the field is getting set properly). --- .../0104_add_enrollment_contract_fks.py | 36 ++++++++++++++++++ .../0105_backfill_enrollment_contracts.py | 38 +++++++++++++++++++ courses/models.py | 14 +++++++ 3 files changed, 88 insertions(+) create mode 100644 courses/migrations/0104_add_enrollment_contract_fks.py create mode 100644 courses/migrations/0105_backfill_enrollment_contracts.py diff --git a/courses/migrations/0104_add_enrollment_contract_fks.py b/courses/migrations/0104_add_enrollment_contract_fks.py new file mode 100644 index 0000000000..2366d1d17a --- /dev/null +++ b/courses/migrations/0104_add_enrollment_contract_fks.py @@ -0,0 +1,36 @@ +# Generated by Django 5.2.15 on 2026-09-09 20:35 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("b2b", "0027_discountcontractattachmentredemption_email_message_id_and_more"), + ("courses", "0103_gate_certificate_creation"), + ] + + operations = [ + migrations.AddField( + model_name="courserunenrollment", + name="b2b_contract", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.DO_NOTHING, + related_name="course_run_enrollments", + to="b2b.contractpage", + ), + ), + migrations.AddField( + model_name="programenrollment", + name="b2b_contract", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.DO_NOTHING, + related_name="program_enrollments", + to="b2b.contractpage", + ), + ), + ] diff --git a/courses/migrations/0105_backfill_enrollment_contracts.py b/courses/migrations/0105_backfill_enrollment_contracts.py new file mode 100644 index 0000000000..cf070c105f --- /dev/null +++ b/courses/migrations/0105_backfill_enrollment_contracts.py @@ -0,0 +1,38 @@ +# Generated by Django 5.2.15 on 2026-09-09 20:36 + +from django.db import migrations +from django.db.models import F + + +def populate_enrollment_contracts(apps, schema_editor): + """ + Backfill the b2b_contract field that's been added to CourseRunEnrollment and + ProgramEnrollment. + """ + + CourseRunEnrollment = apps.get_model("courses", "CourseRunEnrollment") + CourseRunEnrollment.objects.filter(run__b2b_contract__isnull=False).update( + b2b_contract_id=F("run__b2b_contract__id") + ) + + # Course run enrollments are easy since we only allow (now) a run to belong + # to a contract or not. Programs can belong to any number of contracts so it's + # a bit more tricky. So I think, for each program that has an association with + # a contract, find the users who are enrolled in the program and see what + # runs they're enrolled in that count for the program. If the runs are B2B, + # then we can update the program enrollment accordingly. + + ProgramEnrollment = apps.get_model("courses", "ProgramEnrollment") + ProgramEnrollment.objects.filter() + + +def reverse_noop(apps, schema_editor): + """Do nothing - won't be able to determine who touched the enrollment contract field last.""" + + +class Migration(migrations.Migration): + dependencies = [ + ("courses", "0104_add_enrollment_contract_fks"), + ] + + operations = [migrations.RunPython(populate_enrollment_contracts, reverse_noop)] diff --git a/courses/models.py b/courses/models.py index 075fff9345..7c19b91512 100644 --- a/courses/models.py +++ b/courses/models.py @@ -2447,6 +2447,13 @@ class CourseRunEnrollment(EnrollmentModel): "longer retried automatically." ), ) + b2b_contract = models.ForeignKey( + "b2b.ContractPage", + on_delete=models.DO_NOTHING, + related_name="course_run_enrollments", + null=True, + blank=True, + ) objects = ActiveCourseRunEnrollmentManager() all_objects = CourseRunEnrollmentManager() @@ -2585,6 +2592,13 @@ class ProgramEnrollment(EnrollmentModel): program = models.ForeignKey( "courses.Program", on_delete=models.CASCADE, related_name="enrollments" ) + b2b_contract = models.ForeignKey( + "b2b.ContractPage", + on_delete=models.DO_NOTHING, + related_name="program_enrollments", + null=True, + blank=True, + ) objects = ActiveProgramEnrollmentManager() all_objects = ProgramEnrollmentManager() From cdbb125fd35d6a4ceffa0e5033aa66553ca7388b Mon Sep 17 00:00:00 2001 From: James Kachel Date: Tue, 15 Sep 2026 21:28:35 +0000 Subject: [PATCH 02/11] WIP: Update logic to check for/store contract for a given enrollment no-verify commit because not done yet; need to work out changes to `create_run_enrollments` still and some other things. --- b2b/api.py | 194 ++++++++++++++++++++++++++++++++++++---------- courses/api.py | 16 +++- courses/models.py | 22 ++++++ main/constants.py | 4 + 4 files changed, 192 insertions(+), 44 deletions(-) diff --git a/b2b/api.py b/b2b/api.py index fa44fd3173..e391a6b280 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -79,6 +79,7 @@ from main.utils import date_to_datetime from openedx.constants import EDX_ENROLLMENT_AUDIT_MODE, EDX_ENROLLMENT_VERIFIED_MODE from openedx.tasks import clone_courserun +from users.models import User log = logging.getLogger(__name__) @@ -1337,17 +1338,128 @@ def ensure_enrollment_codes_exist(contract: ContractPage): return (total_created, total_updated, total_errors) -def _validate_b2b_enrollment_prerequisites(user, product: Product) -> Union[dict, None]: # noqa: PLR0911 +def _determine_contract_for_user_product( + user: User, + product: Product, + *, + program: Program | None = None, + contract_id: int | None = None, +): + """ + Determine what the contract should be for the given options supplied. + + If the contract ID is specified, then this just needs to validate everything - + make sure the product item, user and program (if there) are all part of that + contract. If there's no contract ID, this figures out what contract overlaps + these pieces (user, item, program); if it's just one, then this continues on + as if that one had been specified explicitly; otherwise, return an error. + """ + + item = product.purchasable_object + + if not item: + msg = f"Product {product} doesn't appear to have a purchasable object." + raise ValueError(msg) + + user_contract_ids = list(user.b2b_contracts.values_list("id", flat=True)) + + if program and not program.b2b_contracts.exists(): + log.error( + "User %s tried to use product %s with program %s but program is not attached to any contracts", + user, + product, + program, + ) + return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} + + if not contract_id or not ContractPage.objects.filter(pk=contract_id).exists(): + log.info( + "_determine_contract_for_user_product: no contract specified for %s purchasing %s", + user, + product, + ) + + if not item.b2b_contracts.filter(id__in=user_contract_ids).exists(): + return { + "result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, + "failed_match": "item", + } + + if ( + program + and not program.b2b_contracts.filter(id__in=user_contract_ids).exists() + ): + return { + "result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, + "failed_match": "program", + } + + overlap_item_contracts = set( + item.b2b_contracts.filter(id__in=user_contract_ids).values_list( + "id", flat=True + ) + ) + + if program: + overlap_item_contracts = ( + set( + program.b2b_contracts.filter(id__in=user_contract_ids).values_list( + "id", flat=True + ) + ) + & overlap_item_contracts + ) + + contract_matches = set(user_contract_ids) & overlap_item_contracts + + if len(contract_matches) != 1: + log.error( + "User %s tried to use product %s but the contract to use is ambiguous", + user, + product, + ) + return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT} + + return contract_matches.pop() + + if ( + user.b2b_contracts.filter(id=contract_id).exists() + and item.b2b_contracts.filter(id=contract_id).exists() + and (not program or program.b2b_contracts.filter(id=contract_id).exists()) + ): + return contract_id + + log.error( + "User %s tried to use product %s (and/or program %s) for contract %s but one or more parts of the transaction weren't in the contract", + user, + product, + program, + contract_id, + ) + return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} + + +def _validate_b2b_enrollment_prerequisites( + user, + product: Product, + *, + program: Program | None = None, + contract_id: int | None = None, +) -> Union[dict, None]: """ Validate prerequisites for B2B enrollment. Returns: - dict with error result if validation fails, None if validation passes. + dict with error result if validation fails, applicable contract if validation passes. """ if not user.is_authenticated: log.error("B2B enroll: attempted to use %s with no user account", product) return {"result": main_constants.USER_MSG_TYPE_B2B_DISALLOWED} + resolved_contract_id = _determine_contract_for_user_product( + user, product, contract_id=contract_id, program=program + ) + purchasable_object = product.purchasable_object if not purchasable_object: log.error( @@ -1356,34 +1468,12 @@ def _validate_b2b_enrollment_prerequisites(user, product: Product) -> Union[dict ) return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_PRODUCT} - contract = None - if isinstance(purchasable_object, CourseRun): - if purchasable_object.b2b_contracts.count() > 1: - # More than one contract attached to this run, so this is ambiguous. - # This should be updated to accept a particular contract but for now it - # will just bail out if there's more than one to consider. - log.error( - "B2B enroll: run %s has more than one contract", purchasable_object - ) - return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} + if isinstance(resolved_contract_id, dict): + return resolved_contract_id - contract = purchasable_object.b2b_contracts.first() + contract = ContractPage.active_objects.filter(pk=resolved_contract_id).first() if not contract: - log.error("B2B enroll: run %s has no contract", purchasable_object) - return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} - - if isinstance( - purchasable_object, CourseRun - ) and not purchasable_object.enrollable_for_contract(contract): - log.error( - "B2B enroll: attempted to use %s but %s is not enrollable for B2B", - product, - purchasable_object, - ) - return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NOT_ENROLLABLE} - - if not ContractPage.active_objects.filter(id=contract.id).exists(): log.error( "B2B enroll: %s attempted to use %s but contract %s either doesn't exist or is invalid", user, @@ -1392,14 +1482,22 @@ def _validate_b2b_enrollment_prerequisites(user, product: Product) -> Union[dict ) return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} - if not user.b2b_contracts.filter(id=contract.id).exists(): + if not isinstance(purchasable_object, (CourseRun, Program)): log.error( - "B2B enroll: attempted to use %s but %s is not in the contract %s", + "B2B enroll: attempted to use %s but %s is not a program or course run", product, - user, + purchasable_object, + ) + return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NOT_ENROLLABLE} + + if not purchasable_object.enrollable_for_contract(contract): + log.error( + "B2B enroll: attempted to use %s but %s is not enrollable for B2B contract %s", + product, + purchasable_object, contract, ) - return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} + return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NOT_ENROLLABLE} if ( isinstance(purchasable_object, CourseRun) @@ -1421,7 +1519,7 @@ def _validate_b2b_enrollment_prerequisites(user, product: Product) -> Union[dict ) return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED} - return None + return contract def _prepare_basket_for_b2b_enrollment(request, product: Product) -> Basket: @@ -1513,7 +1611,13 @@ def _apply_available_discount(request, product: Product, basket: Basket) -> None basket_discount.save() -def create_b2b_enrollment(request, product: Product, program_id: str | None = None): +def create_b2b_enrollment( + request, + product: Product, + *, + program_id: str | None = None, + contract_id: int | None = None, +): """ Create a B2B enrollment for the given product for the current user. @@ -1546,12 +1650,18 @@ def create_b2b_enrollment(request, product: Product, program_id: str | None = No """ from ecommerce.api import generate_checkout_payload # noqa: PLC0415 + program = None + if program_id: + program = Program.objects.get(pk=program_id) + # Validate prerequisites for B2B enrollment - validation_error = _validate_b2b_enrollment_prerequisites(request.user, product) + prereq_check = _validate_b2b_enrollment_prerequisites( + request.user, product, program=program, contract_id=contract_id + ) if ( - validation_error - and validation_error.get("result", None) + prereq_check + and prereq_check.get("result", None) == main_constants.USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED ): # User has a verified enrollment in the run already - try to find the @@ -1569,8 +1679,10 @@ def create_b2b_enrollment(request, product: Product, program_id: str | None = No "order": order.id if order else "", } - if validation_error: - return validation_error + if not isinstance(prereq_check, ContractPage): + return prereq_check + + contract = prereq_check # Prepare the basket for enrollment basket = _prepare_basket_for_b2b_enrollment(request, product) @@ -1588,7 +1700,7 @@ def create_b2b_enrollment(request, product: Product, program_id: str | None = No if "no_checkout" in response: # Course run enrollment succeeded - now handle program enrollment if program_id: - _enroll_in_program_for_b2b(request.user, product, program_id) + _enroll_in_program_for_b2b(request.user, product, program_id, contract) return { "result": main_constants.USER_MSG_TYPE_B2B_ENROLL_SUCCESS, @@ -1607,7 +1719,9 @@ def create_b2b_enrollment(request, product: Product, program_id: str | None = No } -def _enroll_in_program_for_b2b(user, product: Product, program_id: str): +def _enroll_in_program_for_b2b( + user, product: Product, program_id: str, contract: ContractPage +): """ Enroll the user in the specified program as part of a B2B course enrollment. diff --git a/courses/api.py b/courses/api.py index 5a07776da8..9d626146eb 100644 --- a/courses/api.py +++ b/courses/api.py @@ -198,7 +198,7 @@ def create_run_enrollments( # noqa: C901, PLR0913 Args: user (User): The user to enroll - runs (iterable of CourseRun): The course runs to enroll in + runs (iterable of CourseRun or tuple (CourseRun,Contract)): The course runs to enroll in change_status (str): The status of the enrollment keep_failed_enrollments: (boolean): If True, keeps the local enrollment record in the database even if the enrollment fails in edX. @@ -237,7 +237,7 @@ def send_enrollment_emails(): subscribe_edx_course_emails.delay(enrollment.id) edx_request_success = True - if not runs[0].is_fake_course_run: + if not first_run.is_fake_course_run: # Make the API call to enroll the user in edX only if the run is not a fake course run try: enroll_in_edx_course_runs( @@ -339,7 +339,7 @@ def create_program_enrollments( Args: user (User): The user to enroll - programs (iterable of Program): The course runs to enroll in + programs (iterable of Program or tuple of Program,Contract): The programs to enroll in Kwargs: enrollment_mode (str): The mode the enrollment should be in @@ -348,13 +348,21 @@ def create_program_enrollments( list of ProgramEnrollment: A list of enrollment objects that were successfully created """ successful_enrollments = [] - for program in programs: + for program_data in programs: + # If we've been given a tuple here, then this is a B2B enrollment, and + # as such we have a contract that we should link the enrollment to. + contract = None + program = program_data + if isinstance(program_data, tuple): + program, contract = program_data + _verify_exports_compliance_for_enrollment(user, program) try: enrollment, created = ProgramEnrollment.all_objects.get_or_create( user=user, program=program, + b2b_contract=contract, defaults={ "enrollment_mode": enrollment_mode, }, diff --git a/courses/models.py b/courses/models.py index 7c19b91512..c21aa65380 100644 --- a/courses/models.py +++ b/courses/models.py @@ -795,6 +795,28 @@ def collections(self): ).distinct() ) + @cached_property + def is_enrollable(self): + """ + Determines if the program is enrollable + """ + now = now_in_utc() + return ( + (self.enrollment_end is None or self.enrollment_end > now) + and self.enrollment_start is not None + and self.enrollment_start <= now + and self.live is True + and self.start_date is not None + ) + + def enrollable_for_contract(self, contract) -> bool: + """Determine if the run is enrollable for the specified contract.""" + + if not self.b2b_contracts.filter(pk=contract.id).exists(): + return False + + return self.is_enrollable + class RelatedProgram(TimestampedModel, ValidateOnSaveMixin): """ diff --git a/main/constants.py b/main/constants.py index 5d1030f3dd..971de5ccb1 100644 --- a/main/constants.py +++ b/main/constants.py @@ -22,7 +22,9 @@ USER_MSG_TYPE_B2B_DISALLOWED = "b2b-disallowed" USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED = "b2b-error-already-enrolled" +USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT = "b2b-error-ambiguous-contract" USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT = "b2b-error-no-contract" +USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH = "b2b-error-no-matching-contract" USER_MSG_TYPE_B2B_ERROR_NO_PRODUCT = "b2b-error-no-product" USER_MSG_TYPE_B2B_ERROR_MISSING_ENROLLMENT_CODE = "b2b-error-missing-enrollment-code" USER_MSG_TYPE_B2B_ERROR_INVALID_ENROLLMENT_CODE = "b2b-error-invalid-enrollment-code" @@ -34,7 +36,9 @@ USER_MSG_TYPE_B2B = [ USER_MSG_TYPE_B2B_DISALLOWED, USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED, + USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT, USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT, + USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, USER_MSG_TYPE_B2B_ERROR_NO_PRODUCT, USER_MSG_TYPE_B2B_ERROR_MISSING_ENROLLMENT_CODE, USER_MSG_TYPE_B2B_ERROR_INVALID_ENROLLMENT_CODE, From 6d97f3ea9df5a6f85e7feebd9d14624df5e0e864 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 17 Sep 2026 15:53:24 +0000 Subject: [PATCH 03/11] Renumber these migrations and finish out the backfill one These will likely need to get renumbered again. --- .../0105_backfill_enrollment_contracts.py | 38 ------------ ...py => 0107_add_enrollment_contract_fks.py} | 2 +- .../0108_backfill_enrollment_contracts.py | 61 +++++++++++++++++++ 3 files changed, 62 insertions(+), 39 deletions(-) delete mode 100644 courses/migrations/0105_backfill_enrollment_contracts.py rename courses/migrations/{0104_add_enrollment_contract_fks.py => 0107_add_enrollment_contract_fks.py} (93%) create mode 100644 courses/migrations/0108_backfill_enrollment_contracts.py diff --git a/courses/migrations/0105_backfill_enrollment_contracts.py b/courses/migrations/0105_backfill_enrollment_contracts.py deleted file mode 100644 index cf070c105f..0000000000 --- a/courses/migrations/0105_backfill_enrollment_contracts.py +++ /dev/null @@ -1,38 +0,0 @@ -# Generated by Django 5.2.15 on 2026-09-09 20:36 - -from django.db import migrations -from django.db.models import F - - -def populate_enrollment_contracts(apps, schema_editor): - """ - Backfill the b2b_contract field that's been added to CourseRunEnrollment and - ProgramEnrollment. - """ - - CourseRunEnrollment = apps.get_model("courses", "CourseRunEnrollment") - CourseRunEnrollment.objects.filter(run__b2b_contract__isnull=False).update( - b2b_contract_id=F("run__b2b_contract__id") - ) - - # Course run enrollments are easy since we only allow (now) a run to belong - # to a contract or not. Programs can belong to any number of contracts so it's - # a bit more tricky. So I think, for each program that has an association with - # a contract, find the users who are enrolled in the program and see what - # runs they're enrolled in that count for the program. If the runs are B2B, - # then we can update the program enrollment accordingly. - - ProgramEnrollment = apps.get_model("courses", "ProgramEnrollment") - ProgramEnrollment.objects.filter() - - -def reverse_noop(apps, schema_editor): - """Do nothing - won't be able to determine who touched the enrollment contract field last.""" - - -class Migration(migrations.Migration): - dependencies = [ - ("courses", "0104_add_enrollment_contract_fks"), - ] - - operations = [migrations.RunPython(populate_enrollment_contracts, reverse_noop)] diff --git a/courses/migrations/0104_add_enrollment_contract_fks.py b/courses/migrations/0107_add_enrollment_contract_fks.py similarity index 93% rename from courses/migrations/0104_add_enrollment_contract_fks.py rename to courses/migrations/0107_add_enrollment_contract_fks.py index 2366d1d17a..c0cb810e05 100644 --- a/courses/migrations/0104_add_enrollment_contract_fks.py +++ b/courses/migrations/0107_add_enrollment_contract_fks.py @@ -7,7 +7,7 @@ class Migration(migrations.Migration): dependencies = [ ("b2b", "0027_discountcontractattachmentredemption_email_message_id_and_more"), - ("courses", "0103_gate_certificate_creation"), + ("courses", "0106_drop_courserun_b2b_contract_constraints"), ] operations = [ diff --git a/courses/migrations/0108_backfill_enrollment_contracts.py b/courses/migrations/0108_backfill_enrollment_contracts.py new file mode 100644 index 0000000000..efa5e6caf0 --- /dev/null +++ b/courses/migrations/0108_backfill_enrollment_contracts.py @@ -0,0 +1,61 @@ +# Generated by Django 5.2.15 on 2026-09-09 20:36 + +from django.db import migrations + + +def populate_enrollment_contracts(apps, schema_editor): + """ + Backfill the b2b_contract field that's been added to CourseRunEnrollment and + ProgramEnrollment. + """ + + CourseRunEnrollment = apps.get_model("courses", "CourseRunEnrollment") + contract_course_run_enrollments = CourseRunEnrollment.objects.filter(run__b2b_contract__isnull=False).all() + + for enrollment in contract_course_run_enrollments: + # We haven't dropped the b2b_contract FK as of yet, so use that to determine + # the ownership for the enrollment. + enrollment.b2b_contract = enrollment.run.b2b_contract + enrollment.save() + + # Program backfill works differently. The program may be associated with one + # or more contracts. So, each enrollment may be for a different contract, or + # it may not be a B2B enrollment at all (if the program isn't flagged b2b_only). + # Loop through and see what + + ProgramEnrollment = apps.get_model("courses", "ProgramEnrollment") + + # Using objects here and not all_objects - want to avoid enrollments that might + # be for no longer valid contracts; if the user re-enrolls in the program, + # that will cause the enrollment to be linked up to the correct contract. + contract_program_enrollments = ProgramEnrollment.objects.filter(program__b2b_contracts__isnull=False).all() + updated_enrollments = [] + + for enrollment in contract_program_enrollments: + user = enrollment.user + first_user_contract = user.b2b_contracts.filter(id__in=list(enrollment.program.b2b_contracts.values_list("id", flat=True))).first() + + if not first_user_contract and enrollment.program.b2b_only: + msg = f"Enrollment for {user} in {enrollment.program} seems invalid - program is marked B2B-only but the user isn't in the contract." + raise ValueError(msg) + + if not first_user_contract: + continue + + enrollment.b2b_contract = first_user_contract + updated_enrollments.append(enrollment) + + if len(updated_enrollments) > 0: + ProgramEnrollment.objects.bulk_update(updated_enrollments, ["b2b_contract"]) + + +def reverse_noop(apps, schema_editor): + """Do nothing - won't be able to determine who touched the enrollment contract field last.""" + + +class Migration(migrations.Migration): + dependencies = [ + ("courses", "0107_add_enrollment_contract_fks"), + ] + + operations = [migrations.RunPython(populate_enrollment_contracts, reverse_noop)] From 2154b15e9a6c316c8e8ee6dfebb790f9cb3c1a26 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Thu, 17 Sep 2026 21:35:38 +0000 Subject: [PATCH 04/11] Finished up course run enrollment update stuff Rolls back the prior changes to try to link the contract in during enroll and instead moves that elsewhere so I'm not messing with the enrollment code. Adds a b2b_contract field to the line/basketitem so that it can keep track of that later too. Did get purchases done (B2B and not) and things worked as expected! --- b2b/api.py | 58 +++++++++++++------ courses/api.py | 16 ++--- .../0108_backfill_enrollment_contracts.py | 16 +++-- ecommerce/hooks/process_transaction_line.py | 42 +++++++++++++- .../0055_add_contract_fields_to_line_items.py | 34 +++++++++++ ecommerce/models.py | 35 +++++++++-- 6 files changed, 161 insertions(+), 40 deletions(-) create mode 100644 ecommerce/migrations/0055_add_contract_fields_to_line_items.py diff --git a/b2b/api.py b/b2b/api.py index e391a6b280..8c696a9946 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -1358,14 +1358,14 @@ def _determine_contract_for_user_product( item = product.purchasable_object if not item: - msg = f"Product {product} doesn't appear to have a purchasable object." + msg = f"_determine_contract_for_user_product: Product {product} doesn't appear to have a purchasable object." raise ValueError(msg) user_contract_ids = list(user.b2b_contracts.values_list("id", flat=True)) - if program and not program.b2b_contracts.exists(): + if program and not program.contract_memberships.exists(): log.error( - "User %s tried to use product %s with program %s but program is not attached to any contracts", + "_determine_contract_for_user_product: User %s tried to use product %s with program %s but program is not attached to any contracts", user, product, program, @@ -1380,6 +1380,11 @@ def _determine_contract_for_user_product( ) if not item.b2b_contracts.filter(id__in=user_contract_ids).exists(): + log.info( + "_determine_contract_for_user_product: no contract match between for %s purchasing %s", + user, + product, + ) return { "result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, "failed_match": "item", @@ -1387,8 +1392,15 @@ def _determine_contract_for_user_product( if ( program - and not program.b2b_contracts.filter(id__in=user_contract_ids).exists() + and not program.contract_memberships.filter(contract__id__in=user_contract_ids).exists() ): + log.info( + "_determine_contract_for_user_product: no contract match between %s purchasing %s for program %s", + user, + product, + program, + ) + return { "result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, "failed_match": "program", @@ -1400,23 +1412,31 @@ def _determine_contract_for_user_product( ) ) + log.info( + "Item contracts: %s", + ",".join([ str(i) for i in overlap_item_contracts ]) + ) + if program: - overlap_item_contracts = ( - set( - program.b2b_contracts.filter(id__in=user_contract_ids).values_list( - "id", flat=True - ) - ) - & overlap_item_contracts + program_overlaps = set( + program.contract_memberships.filter(contract__id__in=user_contract_ids).values_list( + "contract__id", flat=True + ) + ) + log.info( + "Program contracts: %s", + ",".join([ str(i) for i in program_overlaps ]) ) + overlap_item_contracts = (program_overlaps & overlap_item_contracts) contract_matches = set(user_contract_ids) & overlap_item_contracts if len(contract_matches) != 1: log.error( - "User %s tried to use product %s but the contract to use is ambiguous", + "User %s tried to use product %s but the contract to use is ambiguous (%s)", user, product, + ",".join([ str(i) for i in contract_matches ]) ) return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT} @@ -1425,7 +1445,7 @@ def _determine_contract_for_user_product( if ( user.b2b_contracts.filter(id=contract_id).exists() and item.b2b_contracts.filter(id=contract_id).exists() - and (not program or program.b2b_contracts.filter(id=contract_id).exists()) + and (not program or program.contract_memberships.filter(contract__id=contract_id).exists()) ): return contract_id @@ -1522,7 +1542,9 @@ def _validate_b2b_enrollment_prerequisites( return contract -def _prepare_basket_for_b2b_enrollment(request, product: Product) -> Basket: +def _prepare_basket_for_b2b_enrollment( + request, product: Product, contract: ContractPage +) -> Basket: """ Prepare basket for B2B enrollment by clearing it and adding the product. @@ -1537,7 +1559,9 @@ def _prepare_basket_for_b2b_enrollment(request, product: Product) -> Basket: basket.basket_items.all().delete() basket.discounts.all().delete() - item = BasketItem.objects.create(product=product, basket=basket, quantity=1) + item = BasketItem.objects.create( + product=product, basket=basket, quantity=1, b2b_contract=contract + ) item.save() # Sync with HubSpot for CourseRun @@ -1660,7 +1684,7 @@ def create_b2b_enrollment( ) if ( - prereq_check + isinstance(prereq_check, dict) and prereq_check.get("result", None) == main_constants.USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED ): @@ -1685,7 +1709,7 @@ def create_b2b_enrollment( contract = prereq_check # Prepare the basket for enrollment - basket = _prepare_basket_for_b2b_enrollment(request, product) + basket = _prepare_basket_for_b2b_enrollment(request, product, contract) # Apply any available discount to the basket _apply_available_discount(request, product, basket) diff --git a/courses/api.py b/courses/api.py index 9d626146eb..5a07776da8 100644 --- a/courses/api.py +++ b/courses/api.py @@ -198,7 +198,7 @@ def create_run_enrollments( # noqa: C901, PLR0913 Args: user (User): The user to enroll - runs (iterable of CourseRun or tuple (CourseRun,Contract)): The course runs to enroll in + runs (iterable of CourseRun): The course runs to enroll in change_status (str): The status of the enrollment keep_failed_enrollments: (boolean): If True, keeps the local enrollment record in the database even if the enrollment fails in edX. @@ -237,7 +237,7 @@ def send_enrollment_emails(): subscribe_edx_course_emails.delay(enrollment.id) edx_request_success = True - if not first_run.is_fake_course_run: + if not runs[0].is_fake_course_run: # Make the API call to enroll the user in edX only if the run is not a fake course run try: enroll_in_edx_course_runs( @@ -339,7 +339,7 @@ def create_program_enrollments( Args: user (User): The user to enroll - programs (iterable of Program or tuple of Program,Contract): The programs to enroll in + programs (iterable of Program): The course runs to enroll in Kwargs: enrollment_mode (str): The mode the enrollment should be in @@ -348,21 +348,13 @@ def create_program_enrollments( list of ProgramEnrollment: A list of enrollment objects that were successfully created """ successful_enrollments = [] - for program_data in programs: - # If we've been given a tuple here, then this is a B2B enrollment, and - # as such we have a contract that we should link the enrollment to. - contract = None - program = program_data - if isinstance(program_data, tuple): - program, contract = program_data - + for program in programs: _verify_exports_compliance_for_enrollment(user, program) try: enrollment, created = ProgramEnrollment.all_objects.get_or_create( user=user, program=program, - b2b_contract=contract, defaults={ "enrollment_mode": enrollment_mode, }, diff --git a/courses/migrations/0108_backfill_enrollment_contracts.py b/courses/migrations/0108_backfill_enrollment_contracts.py index efa5e6caf0..9d9089a9fd 100644 --- a/courses/migrations/0108_backfill_enrollment_contracts.py +++ b/courses/migrations/0108_backfill_enrollment_contracts.py @@ -10,7 +10,9 @@ def populate_enrollment_contracts(apps, schema_editor): """ CourseRunEnrollment = apps.get_model("courses", "CourseRunEnrollment") - contract_course_run_enrollments = CourseRunEnrollment.objects.filter(run__b2b_contract__isnull=False).all() + contract_course_run_enrollments = CourseRunEnrollment.objects.filter( + run__b2b_contract__isnull=False + ).all() for enrollment in contract_course_run_enrollments: # We haven't dropped the b2b_contract FK as of yet, so use that to determine @@ -21,19 +23,25 @@ def populate_enrollment_contracts(apps, schema_editor): # Program backfill works differently. The program may be associated with one # or more contracts. So, each enrollment may be for a different contract, or # it may not be a B2B enrollment at all (if the program isn't flagged b2b_only). - # Loop through and see what + # Loop through and see what ProgramEnrollment = apps.get_model("courses", "ProgramEnrollment") # Using objects here and not all_objects - want to avoid enrollments that might # be for no longer valid contracts; if the user re-enrolls in the program, # that will cause the enrollment to be linked up to the correct contract. - contract_program_enrollments = ProgramEnrollment.objects.filter(program__b2b_contracts__isnull=False).all() + contract_program_enrollments = ProgramEnrollment.objects.filter( + program__contract_memberships__isnull=False + ).all() updated_enrollments = [] for enrollment in contract_program_enrollments: user = enrollment.user - first_user_contract = user.b2b_contracts.filter(id__in=list(enrollment.program.b2b_contracts.values_list("id", flat=True))).first() + first_user_contract = user.b2b_contracts.filter( + id__in=list( + enrollment.program.contract_memberships.values_list("id", flat=True) + ) + ).first() if not first_user_contract and enrollment.program.b2b_only: msg = f"Enrollment for {user} in {enrollment.program} seems invalid - program is marked B2B-only but the user isn't in the contract." diff --git a/ecommerce/hooks/process_transaction_line.py b/ecommerce/hooks/process_transaction_line.py index ad4ff683d0..330761b29e 100644 --- a/ecommerce/hooks/process_transaction_line.py +++ b/ecommerce/hooks/process_transaction_line.py @@ -4,7 +4,13 @@ import pluggy -from courses.models import CourseRun, PaidCourseRun, PaidProgram, Program +from courses.models import ( + CourseRun, + CourseRunEnrollment, + PaidCourseRun, + PaidProgram, + Program, +) from openedx.constants import EDX_ENROLLMENT_VERIFIED_MODE hookimpl = pluggy.HookimplMarker("mitxonline") @@ -39,6 +45,34 @@ def _create_courserun_enrollment(line) -> str | None: log.debug("Created course run enrollment for %s", purchased_run) +def _link_b2b_course_run_contracts(line) -> str | None: + """If the purchased line was a B2B run, make the resulting enrollment a B2B enrollment""" + + purchased_run = line.purchased_object + + if not isinstance(purchased_run, CourseRun): + log.debug( + "_link_b2b_course_run_contracts: Item purchased %s is not a course run, skipping", + purchased_run, + ) + return + + enrollment_qs = CourseRunEnrollment.objects.filter( + run=purchased_run, user=line.order.purchaser, enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE + ) + + if enrollment_qs.count() != 1: + log.error( + "_link_b2b_course_run_contracts: Purchaser %s has an improper number of enrollments (%s) for %s in order %s", + line.order.purchaser, + enrollment_qs.count(), + purchased_run, + line.order.reference_number, + ) + + enrollment_qs.update(b2b_contract=line.b2b_contract) + + def _create_program_enrollment(line) -> str | None: """Create a program enrollment for the line, if we need to.""" @@ -87,3 +121,9 @@ def create_program_enrollment(self, line) -> str | None: """Call the internal function (so we can test it)""" return _create_program_enrollment(line=line) + + @hookimpl(specname="process_transaction_line", trylast=True) + def link_b2b_courserun_enrollment(self, line) -> str | None: + """Call the internal function""" + + return _link_b2b_course_run_contracts(line) diff --git a/ecommerce/migrations/0055_add_contract_fields_to_line_items.py b/ecommerce/migrations/0055_add_contract_fields_to_line_items.py new file mode 100644 index 0000000000..3d2a0e3e67 --- /dev/null +++ b/ecommerce/migrations/0055_add_contract_fields_to_line_items.py @@ -0,0 +1,34 @@ +# Generated by Django 5.2.15 on 2026-09-17 19:18 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("b2b", "0028_organizationidentityprovider_organizationonboarding"), + ("ecommerce", "0054_paid_amount_off_discounts"), + ] + + operations = [ + migrations.AddField( + model_name="basketitem", + name="b2b_contract", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.DO_NOTHING, + related_name="+", + to="b2b.contractpage", + ), + ), + migrations.AddField( + model_name="line", + name="b2b_contract", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.DO_NOTHING, + related_name="+", + to="b2b.contractpage", + ), + ), + ] diff --git a/ecommerce/models.py b/ecommerce/models.py index 1a45c40799..72a980b861 100644 --- a/ecommerce/models.py +++ b/ecommerce/models.py @@ -4,7 +4,7 @@ from collections.abc import Iterable # noqa: TC003 from datetime import datetime, timedelta from decimal import Decimal -from typing import List # noqa: UP035 +from typing import TYPE_CHECKING, List, Tuple # noqa: UP035 from zoneinfo import ZoneInfo import reversion @@ -51,6 +51,9 @@ from main.plugin_manager import get_plugin_manager from users.models import User +if TYPE_CHECKING: + from b2b.models import ContractPage + User = get_user_model() # noqa: F811 @@ -225,7 +228,19 @@ def get_products(self): Returns the products that have been added to the basket so far. """ - return [item.product for item in self.basket_items.select_related("product")] + return [ + item.product for item in self.basket_items.select_related("product") + ] + + def get_products_contracts(self): + """ + get_products, but adds in the contracts too. + """ + + return [ + (item.product, item.b2b_contract) + for item in self.basket_items.select_related("product") + ] class BasketItem(TimestampedModel): @@ -238,6 +253,9 @@ class BasketItem(TimestampedModel): Basket, on_delete=models.CASCADE, related_name="basket_items" ) quantity = models.PositiveIntegerField(default=1) + b2b_contract = models.ForeignKey( + "b2b.ContractPage", on_delete=models.DO_NOTHING, related_name="+", null=True + ) @cached_property def discounted_price(self): @@ -1206,7 +1224,7 @@ class PendingOrder(Order): @transaction.atomic def _get_or_create( self, - products: List[Product], # noqa: UP006 + products: List[Tuple[Product, ContractPage]], # noqa: UP006 user: User, discounts: List[Discount] | None = None, # noqa: UP006 gateway_type: str = settings.ECOMMERCE_DEFAULT_PAYMENT_GATEWAY, @@ -1231,7 +1249,7 @@ def _get_or_create( """ # Get the details from each Product. product_versions, product_object_ids, product_content_types = [], [], [] - for product in products: + for product, _ in products: # Per docs, this should sort most recent first. product_version = Version.objects.get_for_object(product).first() @@ -1290,7 +1308,8 @@ def _get_or_create( # Create or get Line for each product. Calculate the Order total based on Lines and discount. total = 0 - for i, product in enumerate(products): + for i, product_tuple in enumerate(products): + product, contract = product_tuple line, created = Line.objects.get_or_create( order=order, purchased_object_id=product.object_id, @@ -1307,6 +1326,7 @@ def _get_or_create( order, product_versions[i] ) ), + "b2b_contract": contract, }, ) if not created: @@ -1338,7 +1358,7 @@ def create_from_basket( Returns: PendingOrder: the created pending order """ - products = basket.get_products() + products = basket.get_products_contracts() discounts = [ basket_discount.redeemed_discount for basket_discount in basket.discounts.all() @@ -1492,6 +1512,9 @@ def _order_line_product_versions(): max_digits=20, help_text="Post-discount price of one unit, recorded when the order was priced.", ) + b2b_contract = models.ForeignKey( + "b2b.ContractPage", on_delete=models.DO_NOTHING, related_name="+", null=True + ) # denormalized reference which otherwise requires the lookup: line.product_version.product.purchasable_object purchased_content_type = models.ForeignKey( From 2702740def1dd31c1e100df4424b3643b771dfc1 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 21:38:05 +0000 Subject: [PATCH 05/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- b2b/api.py | 29 ++++++++++++--------- ecommerce/hooks/process_transaction_line.py | 4 ++- ecommerce/models.py | 4 +-- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/b2b/api.py b/b2b/api.py index 8c696a9946..b3c4c2c3c3 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -1392,7 +1392,9 @@ def _determine_contract_for_user_product( if ( program - and not program.contract_memberships.filter(contract__id__in=user_contract_ids).exists() + and not program.contract_memberships.filter( + contract__id__in=user_contract_ids + ).exists() ): log.info( "_determine_contract_for_user_product: no contract match between %s purchasing %s for program %s", @@ -1400,7 +1402,7 @@ def _determine_contract_for_user_product( product, program, ) - + return { "result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, "failed_match": "program", @@ -1413,21 +1415,19 @@ def _determine_contract_for_user_product( ) log.info( - "Item contracts: %s", - ",".join([ str(i) for i in overlap_item_contracts ]) + "Item contracts: %s", ",".join([str(i) for i in overlap_item_contracts]) ) if program: program_overlaps = set( - program.contract_memberships.filter(contract__id__in=user_contract_ids).values_list( - "contract__id", flat=True - ) - ) + program.contract_memberships.filter( + contract__id__in=user_contract_ids + ).values_list("contract__id", flat=True) + ) log.info( - "Program contracts: %s", - ",".join([ str(i) for i in program_overlaps ]) + "Program contracts: %s", ",".join([str(i) for i in program_overlaps]) ) - overlap_item_contracts = (program_overlaps & overlap_item_contracts) + overlap_item_contracts = program_overlaps & overlap_item_contracts contract_matches = set(user_contract_ids) & overlap_item_contracts @@ -1436,7 +1436,7 @@ def _determine_contract_for_user_product( "User %s tried to use product %s but the contract to use is ambiguous (%s)", user, product, - ",".join([ str(i) for i in contract_matches ]) + ",".join([str(i) for i in contract_matches]), ) return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT} @@ -1445,7 +1445,10 @@ def _determine_contract_for_user_product( if ( user.b2b_contracts.filter(id=contract_id).exists() and item.b2b_contracts.filter(id=contract_id).exists() - and (not program or program.contract_memberships.filter(contract__id=contract_id).exists()) + and ( + not program + or program.contract_memberships.filter(contract__id=contract_id).exists() + ) ): return contract_id diff --git a/ecommerce/hooks/process_transaction_line.py b/ecommerce/hooks/process_transaction_line.py index 330761b29e..1ee4043490 100644 --- a/ecommerce/hooks/process_transaction_line.py +++ b/ecommerce/hooks/process_transaction_line.py @@ -58,7 +58,9 @@ def _link_b2b_course_run_contracts(line) -> str | None: return enrollment_qs = CourseRunEnrollment.objects.filter( - run=purchased_run, user=line.order.purchaser, enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE + run=purchased_run, + user=line.order.purchaser, + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, ) if enrollment_qs.count() != 1: diff --git a/ecommerce/models.py b/ecommerce/models.py index 72a980b861..437d388077 100644 --- a/ecommerce/models.py +++ b/ecommerce/models.py @@ -228,9 +228,7 @@ def get_products(self): Returns the products that have been added to the basket so far. """ - return [ - item.product for item in self.basket_items.select_related("product") - ] + return [item.product for item in self.basket_items.select_related("product")] def get_products_contracts(self): """ From 8b9489ba82f2a7ba50e5bb6f219894406a4154a8 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 21 Sep 2026 15:07:40 -0500 Subject: [PATCH 06/11] renumber migration --- ..._line_items.py => 0056_add_contract_fields_to_line_items.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename ecommerce/migrations/{0055_add_contract_fields_to_line_items.py => 0056_add_contract_fields_to_line_items.py} (94%) diff --git a/ecommerce/migrations/0055_add_contract_fields_to_line_items.py b/ecommerce/migrations/0056_add_contract_fields_to_line_items.py similarity index 94% rename from ecommerce/migrations/0055_add_contract_fields_to_line_items.py rename to ecommerce/migrations/0056_add_contract_fields_to_line_items.py index 3d2a0e3e67..77c0435e99 100644 --- a/ecommerce/migrations/0055_add_contract_fields_to_line_items.py +++ b/ecommerce/migrations/0056_add_contract_fields_to_line_items.py @@ -7,7 +7,7 @@ class Migration(migrations.Migration): dependencies = [ ("b2b", "0028_organizationidentityprovider_organizationonboarding"), - ("ecommerce", "0054_paid_amount_off_discounts"), + ("ecommerce", "0055_internal_redemption_type"), ] operations = [ From 61a3d5c5fcf2d1ef5c5bc3cf75c7fa25ebe5bd49 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 21 Sep 2026 16:22:25 -0500 Subject: [PATCH 07/11] Fix up a bunch of tests --- b2b/api_test.py | 22 +++++++++++----------- b2b/views/v0/views_test.py | 3 ++- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/b2b/api_test.py b/b2b/api_test.py index 1009ece258..319cfa95e3 100644 --- a/b2b/api_test.py +++ b/b2b/api_test.py @@ -85,7 +85,7 @@ USER_MSG_TYPE_B2B_DISALLOWED, USER_MSG_TYPE_B2B_ENROLL_SUCCESS, USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED, - USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT, + USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, USER_MSG_TYPE_B2B_ERROR_REQUIRES_CHECKOUT, ) from main.utils import date_to_datetime @@ -498,11 +498,11 @@ def test_create_b2b_enrollment( # noqa: PLR0913, C901, PLR0915 assert Basket.objects.filter(user=user).count() == assert_test if not product_in_contract: - assert result["result"] == USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT + assert result["result"] == USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH return if not user_in_contract: - assert result["result"] == USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT + assert result["result"] == USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH return if not price_is_zero: @@ -567,7 +567,7 @@ def test_enroll_in_program_for_b2b(program_in_contract, program_exists): product = ProductFactory.create(purchasable_object=run) - _enroll_in_program_for_b2b(user, product, program_id) + _enroll_in_program_for_b2b(user, product, program_id, contract) if program_in_contract and program_exists: assert ProgramEnrollment.objects.filter(user=user, program=program).exists() @@ -1667,8 +1667,7 @@ def test_apply_available_discount_seat_limit(): request = RequestFactory() request.user = user_orgs[2].user - # Test the validate step - this gets called before the apply call and should - # fail. (So, in real life, trying to add this third user should not work.) + # Test the validate step user_orgs[2].user.b2b_contracts.add(contract) user_orgs[2].user.save() @@ -1677,7 +1676,7 @@ def test_apply_available_discount_seat_limit(): # We've added the user to the contract - the seat limit is exceeded but because # we manually did it above this should return successfully. - assert result is None + assert result == contract # Calling this directly should result in a new discount being created. @@ -1751,15 +1750,16 @@ def test_apply_available_discount_unlimited_seats(existing_discounts): request = RequestFactory() request.user = user_orgs[2].user - # Test the validate step - this gets called before the apply call and should - # fail. (So, in real life, trying to add this third user should not work.) + # Test the validate step user_orgs[2].user.b2b_contracts.add(contract) user_orgs[2].user.save() result = _validate_b2b_enrollment_prerequisites(user_orgs[2].user, products[0]) - assert not result + # We've added the user to the contract - the seat limit is exceeded but because + # we manually did it above this should return successfully. + assert result == contract _apply_available_discount(request, products[0], basket) @@ -2285,7 +2285,7 @@ def test_enroll_prereqs_existing_enrollment(mocker, change_status): result = _validate_b2b_enrollment_prerequisites(user, product) if change_status == ENROLL_CHANGE_STATUS_UNENROLLED: - assert not result + assert result == contract else: assert result assert "result" in result diff --git a/b2b/views/v0/views_test.py b/b2b/views/v0/views_test.py index 6dabaa199c..a8284cd5b5 100644 --- a/b2b/views/v0/views_test.py +++ b/b2b/views/v0/views_test.py @@ -25,6 +25,7 @@ USER_MSG_TYPE_B2B_ENROLL_SUCCESS, USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED, USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT, + USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, USER_MSG_TYPE_B2B_ERROR_NOT_ENROLLABLE, USER_MSG_TYPE_B2B_ERROR_REQUIRES_CHECKOUT, ) @@ -512,7 +513,7 @@ def test_b2b_enroll( # noqa: PLR0915, PLR0913, C901 if contract_active in ["date", "flag"]: assert resp.status_code == 400 - assert resp.json()["result"] == USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT + assert resp.json()["result"] == USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH return if not run_is_enrollable: From a6ef61bffba868337d64a590fc90635e4f172f33 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Mon, 21 Sep 2026 16:49:38 -0500 Subject: [PATCH 08/11] Fix up remaining tests from last run --- courses/models_test.py | 1 + ecommerce/models.py | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/courses/models_test.py b/courses/models_test.py index 9087cd2147..c5da84a4c4 100644 --- a/courses/models_test.py +++ b/courses/models_test.py @@ -528,6 +528,7 @@ def test_audit(user, is_program): "user": enrollment.user.id, "username": enrollment.user.edx_username, "enrollment_mode": enrollment.enrollment_mode, + "b2b_contract": None, } if not is_program: expected["edx_enrolled"] = enrollment.edx_enrolled diff --git a/ecommerce/models.py b/ecommerce/models.py index 437d388077..0a93c342be 100644 --- a/ecommerce/models.py +++ b/ecommerce/models.py @@ -1222,7 +1222,7 @@ class PendingOrder(Order): @transaction.atomic def _get_or_create( self, - products: List[Tuple[Product, ContractPage]], # noqa: UP006 + products: List[Tuple[Product, ContractPage | None]], # noqa: UP006 user: User, discounts: List[Discount] | None = None, # noqa: UP006 gateway_type: str = settings.ECOMMERCE_DEFAULT_PAYMENT_GATEWAY, @@ -1301,7 +1301,9 @@ def _get_or_create( redemption_date=now, redeemed_by=user, redeemed_discount=discount, - source_line=source_line_for(discount, user, products), + source_line=source_line_for( + discount, user, [product[0] for product in products] + ), ) # Create or get Line for each product. Calculate the Order total based on Lines and discount. @@ -1385,7 +1387,9 @@ def create_from_product( PendingOrder: the created pending order """ - order = cls._get_or_create(cls, [product], user, [discount], gateway_type) + order = cls._get_or_create( + cls, [(product, None)], user, [discount], gateway_type + ) return order # noqa: RET504 From fec4f33c764135df237be80571274936a0b764ec Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 23 Sep 2026 10:46:03 -0500 Subject: [PATCH 09/11] update openapi spec --- openapi/specs/v0.yaml | 6 ++++++ openapi/specs/v1.yaml | 6 ++++++ openapi/specs/v2.yaml | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index dc3c0fa578..fd6a35f40b 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -10033,7 +10033,9 @@ components: enum: - b2b-disallowed - b2b-error-already-enrolled + - b2b-error-ambiguous-contract - b2b-error-no-contract + - b2b-error-no-matching-contract - b2b-error-no-product - b2b-error-missing-enrollment-code - b2b-error-invalid-enrollment-code @@ -10044,7 +10046,9 @@ components: description: |- * `b2b-disallowed` - b2b-disallowed * `b2b-error-already-enrolled` - b2b-error-already-enrolled + * `b2b-error-ambiguous-contract` - b2b-error-ambiguous-contract * `b2b-error-no-contract` - b2b-error-no-contract + * `b2b-error-no-matching-contract` - b2b-error-no-matching-contract * `b2b-error-no-product` - b2b-error-no-product * `b2b-error-missing-enrollment-code` - b2b-error-missing-enrollment-code * `b2b-error-invalid-enrollment-code` - b2b-error-invalid-enrollment-code @@ -10054,7 +10058,9 @@ components: x-enum-descriptions: - b2b-disallowed - b2b-error-already-enrolled + - b2b-error-ambiguous-contract - b2b-error-no-contract + - b2b-error-no-matching-contract - b2b-error-no-product - b2b-error-missing-enrollment-code - b2b-error-invalid-enrollment-code diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index bdf4838656..e1cd58bded 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -10033,7 +10033,9 @@ components: enum: - b2b-disallowed - b2b-error-already-enrolled + - b2b-error-ambiguous-contract - b2b-error-no-contract + - b2b-error-no-matching-contract - b2b-error-no-product - b2b-error-missing-enrollment-code - b2b-error-invalid-enrollment-code @@ -10044,7 +10046,9 @@ components: description: |- * `b2b-disallowed` - b2b-disallowed * `b2b-error-already-enrolled` - b2b-error-already-enrolled + * `b2b-error-ambiguous-contract` - b2b-error-ambiguous-contract * `b2b-error-no-contract` - b2b-error-no-contract + * `b2b-error-no-matching-contract` - b2b-error-no-matching-contract * `b2b-error-no-product` - b2b-error-no-product * `b2b-error-missing-enrollment-code` - b2b-error-missing-enrollment-code * `b2b-error-invalid-enrollment-code` - b2b-error-invalid-enrollment-code @@ -10054,7 +10058,9 @@ components: x-enum-descriptions: - b2b-disallowed - b2b-error-already-enrolled + - b2b-error-ambiguous-contract - b2b-error-no-contract + - b2b-error-no-matching-contract - b2b-error-no-product - b2b-error-missing-enrollment-code - b2b-error-invalid-enrollment-code diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index a8d17bc0f6..c3a82c0460 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -10033,7 +10033,9 @@ components: enum: - b2b-disallowed - b2b-error-already-enrolled + - b2b-error-ambiguous-contract - b2b-error-no-contract + - b2b-error-no-matching-contract - b2b-error-no-product - b2b-error-missing-enrollment-code - b2b-error-invalid-enrollment-code @@ -10044,7 +10046,9 @@ components: description: |- * `b2b-disallowed` - b2b-disallowed * `b2b-error-already-enrolled` - b2b-error-already-enrolled + * `b2b-error-ambiguous-contract` - b2b-error-ambiguous-contract * `b2b-error-no-contract` - b2b-error-no-contract + * `b2b-error-no-matching-contract` - b2b-error-no-matching-contract * `b2b-error-no-product` - b2b-error-no-product * `b2b-error-missing-enrollment-code` - b2b-error-missing-enrollment-code * `b2b-error-invalid-enrollment-code` - b2b-error-invalid-enrollment-code @@ -10054,7 +10058,9 @@ components: x-enum-descriptions: - b2b-disallowed - b2b-error-already-enrolled + - b2b-error-ambiguous-contract - b2b-error-no-contract + - b2b-error-no-matching-contract - b2b-error-no-product - b2b-error-missing-enrollment-code - b2b-error-invalid-enrollment-code From 3bd68aca1d680fc935e6c3c91b290eb9eac7d760 Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 23 Sep 2026 14:40:28 -0500 Subject: [PATCH 10/11] Update B2B enroll API to accept a contract; add tests, fix other issues --- b2b/api.py | 73 +++-- b2b/api_test.py | 582 ++++++++++++++++++++++++++++++++- b2b/serializers/v0/__init__.py | 8 + b2b/views/v0/__init__.py | 15 +- b2b/views/v0/views_test.py | 84 ++++- courses/admin.py | 12 + ecommerce/models_test.py | 160 ++++++++- openapi/specs/v0.yaml | 6 + openapi/specs/v1.yaml | 6 + openapi/specs/v2.yaml | 6 + 10 files changed, 916 insertions(+), 36 deletions(-) diff --git a/b2b/api.py b/b2b/api.py index b3c4c2c3c3..babf8751fe 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -1338,19 +1338,19 @@ def ensure_enrollment_codes_exist(contract: ContractPage): return (total_created, total_updated, total_errors) -def _determine_contract_for_user_product( +def _determine_contract_for_user_product( # noqa: PLR0911 user: User, product: Product, *, program: Program | None = None, - contract_id: int | None = None, + contract_slug: str | None = None, ): """ Determine what the contract should be for the given options supplied. - If the contract ID is specified, then this just needs to validate everything - + If the contract slug is specified, then this just needs to validate everything - make sure the product item, user and program (if there) are all part of that - contract. If there's no contract ID, this figures out what contract overlaps + contract. If there's no contract slug, this figures out what contract overlaps these pieces (user, item, program); if it's just one, then this continues on as if that one had been specified explicitly; otherwise, return an error. """ @@ -1362,6 +1362,9 @@ def _determine_contract_for_user_product( raise ValueError(msg) user_contract_ids = list(user.b2b_contracts.values_list("id", flat=True)) + item_b2b_contracts = ( + item.b2b_contracts if isinstance(item, CourseRun) else item.contract_memberships + ) if program and not program.contract_memberships.exists(): log.error( @@ -1372,14 +1375,17 @@ def _determine_contract_for_user_product( ) return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} - if not contract_id or not ContractPage.objects.filter(pk=contract_id).exists(): + if ( + not contract_slug + or not ContractPage.objects.filter(slug=contract_slug).exists() + ): log.info( "_determine_contract_for_user_product: no contract specified for %s purchasing %s", user, product, ) - if not item.b2b_contracts.filter(id__in=user_contract_ids).exists(): + if not item_b2b_contracts.filter(id__in=user_contract_ids).exists(): log.info( "_determine_contract_for_user_product: no contract match between for %s purchasing %s", user, @@ -1409,7 +1415,7 @@ def _determine_contract_for_user_product( } overlap_item_contracts = set( - item.b2b_contracts.filter(id__in=user_contract_ids).values_list( + item_b2b_contracts.filter(id__in=user_contract_ids).values_list( "id", flat=True ) ) @@ -1443,31 +1449,33 @@ def _determine_contract_for_user_product( return contract_matches.pop() if ( - user.b2b_contracts.filter(id=contract_id).exists() - and item.b2b_contracts.filter(id=contract_id).exists() + user.b2b_contracts.filter(slug=contract_slug).exists() + and item_b2b_contracts.filter(slug=contract_slug).exists() and ( not program - or program.contract_memberships.filter(contract__id=contract_id).exists() + or program.contract_memberships.filter( + contract__slug=contract_slug + ).exists() ) ): - return contract_id + return item_b2b_contracts.filter(slug=contract_slug).get().id log.error( "User %s tried to use product %s (and/or program %s) for contract %s but one or more parts of the transaction weren't in the contract", user, product, program, - contract_id, + contract_slug, ) return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} -def _validate_b2b_enrollment_prerequisites( +def _validate_b2b_enrollment_prerequisites( # noqa: PLR0911 user, product: Product, *, program: Program | None = None, - contract_id: int | None = None, + contract_slug: str | None = None, ) -> Union[dict, None]: """ Validate prerequisites for B2B enrollment. @@ -1479,8 +1487,8 @@ def _validate_b2b_enrollment_prerequisites( log.error("B2B enroll: attempted to use %s with no user account", product) return {"result": main_constants.USER_MSG_TYPE_B2B_DISALLOWED} - resolved_contract_id = _determine_contract_for_user_product( - user, product, contract_id=contract_id, program=program + contract_resolution_result = _determine_contract_for_user_product( + user, product, contract_slug=contract_slug, program=program ) purchasable_object = product.purchasable_object @@ -1491,10 +1499,10 @@ def _validate_b2b_enrollment_prerequisites( ) return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_PRODUCT} - if isinstance(resolved_contract_id, dict): - return resolved_contract_id + if isinstance(contract_resolution_result, dict): + return contract_resolution_result - contract = ContractPage.active_objects.filter(pk=resolved_contract_id).first() + contract = ContractPage.active_objects.filter(pk=contract_resolution_result).first() if not contract: log.error( @@ -1585,7 +1593,9 @@ def _prepare_basket_for_b2b_enrollment( return basket -def _apply_available_discount(request, product: Product, basket: Basket) -> None: +def _apply_available_discount( + request, product: Product, basket: Basket, contract: ContractPage +) -> None: """Apply available discount to the basket if one exists.""" # Changed to only check redemption count if the discount isn't unlimited - @@ -1610,12 +1620,13 @@ def _apply_available_discount(request, product: Product, basket: Basket) -> None if ( not product.purchasable_object - or product.purchasable_object.b2b_contracts.count() != 1 + or not product.purchasable_object.b2b_contracts.filter( + pk=contract.id + ).exists() ): - msg = f"Product {product} has no purchasable object or the purchasable object has <> 1 B2B contract" + msg = f"Product {product} has no purchasable object or the purchasable object is not in contract {contract}" raise ValueError(msg) - contract = product.purchasable_object.b2b_contracts.first() discount_amount = contract.enrollment_fixed_price redemption_type = ( REDEMPTION_TYPE_ONE_TIME @@ -1643,7 +1654,7 @@ def create_b2b_enrollment( product: Product, *, program_id: str | None = None, - contract_id: int | None = None, + contract_slug: str | None = None, ): """ Create a B2B enrollment for the given product for the current user. @@ -1669,6 +1680,7 @@ def create_b2b_enrollment( - request: The HTTP request object containing the user and basket data. - product: The Product object representing the B2B product to enroll in. - program_id: Optional readable_id of the program to enroll the user in. + - contract_slug: Optional slug of the contract the user's enrollments should belong to. Returns: a dict containing - "result": the result of the attempt; one of the USER_MSG_TYPE_B2B constants. - "order": the order ID if the enrollment was successful and no checkout is needed. @@ -1679,11 +1691,11 @@ def create_b2b_enrollment( program = None if program_id: - program = Program.objects.get(pk=program_id) + program = Program.objects.get(readable_id=program_id) # Validate prerequisites for B2B enrollment prereq_check = _validate_b2b_enrollment_prerequisites( - request.user, product, program=program, contract_id=contract_id + request.user, product, program=program, contract_slug=contract_slug ) if ( @@ -1715,7 +1727,7 @@ def create_b2b_enrollment( basket = _prepare_basket_for_b2b_enrollment(request, product, contract) # Apply any available discount to the basket - _apply_available_discount(request, product, basket) + _apply_available_discount(request, product, basket, contract) # Calculate basket total more efficiently basket_price = sum(item.discounted_price for item in basket.basket_items.all()) @@ -1790,10 +1802,15 @@ def _enroll_in_program_for_b2b( ) return - create_program_enrollments( + created_enrollments = create_program_enrollments( user, [program], enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE ) + if contract: + for program_enrollment in created_enrollments: + program_enrollment.b2b_contract = contract + program_enrollment.save() + log.info( "B2B enroll: created program enrollment for user %s in program %s", user, diff --git a/b2b/api_test.py b/b2b/api_test.py index 319cfa95e3..b69c07639a 100644 --- a/b2b/api_test.py +++ b/b2b/api_test.py @@ -8,6 +8,7 @@ import faker import freezegun import pytest +import reversion from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.core.exceptions import ValidationError @@ -18,6 +19,7 @@ from b2b import factories from b2b.api import ( _apply_available_discount, + _determine_contract_for_user_product, _enroll_in_program_for_b2b, _get_source_runs_for_course, _handle_extra_enrollment_codes, @@ -65,7 +67,11 @@ ) from courses.models import CourseRunEnrollment, ProgramEnrollment from ecommerce.api_test import create_basket -from ecommerce.constants import REDEMPTION_TYPE_ONE_TIME, REDEMPTION_TYPE_UNLIMITED +from ecommerce.constants import ( + DISCOUNT_TYPE_FIXED_PRICE, + REDEMPTION_TYPE_ONE_TIME, + REDEMPTION_TYPE_UNLIMITED, +) from ecommerce.factories import ( BasketFactory, BasketItemFactory, @@ -79,13 +85,17 @@ BasketDiscount, DiscountProduct, DiscountRedemption, + Line, OrderStatus, ) from main.constants import ( USER_MSG_TYPE_B2B_DISALLOWED, USER_MSG_TYPE_B2B_ENROLL_SUCCESS, USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED, + USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT, + USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT, USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, + USER_MSG_TYPE_B2B_ERROR_NOT_ENROLLABLE, USER_MSG_TYPE_B2B_ERROR_REQUIRES_CHECKOUT, ) from main.utils import date_to_datetime @@ -2290,3 +2300,573 @@ def test_enroll_prereqs_existing_enrollment(mocker, change_status): assert result assert "result" in result assert result["result"] == USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED + + +@pytest.fixture +def overlapping_contracts(): + """ + Build two contracts with some shared and some unique resources. + + - run_a / program_a belong only to contract A + - run_b / program_b belong only to contract B + - run_ab / program_ab belong to both contracts + - run_none / program_none aren't in any contract + """ + + contract_a, contract_b = factories.ContractPageFactory.create_batch( + 2, membership_type=CONTRACT_MEMBERSHIP_MANAGED, enrollment_fixed_price=0 + ) + + runs = { + "a": CourseRunFactory.create(b2b_only=True, b2b_contracts=[contract_a]), + "b": CourseRunFactory.create(b2b_only=True, b2b_contracts=[contract_b]), + "ab": CourseRunFactory.create( + b2b_only=True, b2b_contracts=[contract_a, contract_b] + ), + "none": CourseRunFactory.create(), + } + with reversion.create_revision(): + products = { + key: ProductFactory.create(purchasable_object=run, price=Decimal(0)) + for key, run in runs.items() + } + + programs = {key: ProgramFactory.create() for key in ("a", "b", "ab", "none")} + for key, contracts in ( + ("a", [contract_a]), + ("b", [contract_b]), + ("ab", [contract_a, contract_b]), + ): + for contract in contracts: + ContractProgramItem.objects.create( + contract=contract, program=programs[key], sort_order=0 + ) + + return { + "contracts": {"a": contract_a, "b": contract_b}, + "runs": runs, + "products": products, + "programs": programs, + } + + +def _make_contract_user(contracts, keys): + """Make a user that belongs to the specified contracts.""" + + user = UserFactory.create() + for key in keys: + user.b2b_contracts.add(contracts[key]) + return user + + +@pytest.mark.parametrize( + ("user_contracts", "run_key", "program_key", "expected"), + [ + # Single overlap, no program + (["a"], "a", None, "a"), + (["b"], "ab", None, "b"), + # User is in both, run only in one + (["a", "b"], "a", None, "a"), + (["a", "b"], "b", None, "b"), + # Program in the same contract + (["a"], "a", "a", "a"), + (["a"], "a", "ab", "a"), + # Program disambiguates a run that's in both contracts + (["a", "b"], "ab", "a", "a"), + (["a", "b"], "ab", "b", "b"), + # Run disambiguates a program that's in both contracts + (["a", "b"], "b", "ab", "b"), + ], +) +def test_determine_contract_resolves_without_slug( + overlapping_contracts, user_contracts, run_key, program_key, expected +): + """With no contract slug, the single overlapping contract should be returned.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, user_contracts) + product = overlapping_contracts["products"][run_key] + program = overlapping_contracts["programs"][program_key] if program_key else None + + result = _determine_contract_for_user_product(user, product, program=program) + + assert result == contracts[expected].id + + +@pytest.mark.parametrize( + ("user_contracts", "run_key", "program_key", "failed_match"), + [ + # User isn't in any contract + ([], "a", None, "item"), + # User is in a different contract than the run + (["b"], "a", None, "item"), + # Run isn't in a contract at all + (["a", "b"], "none", None, "item"), + # Run matches, program is in a contract the user isn't in + (["a"], "a", "b", "program"), + (["a"], "ab", "b", "program"), + ], +) +def test_determine_contract_no_match_without_slug( + overlapping_contracts, user_contracts, run_key, program_key, failed_match +): + """If the user, run, and program don't share a contract, report which part failed.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, user_contracts) + product = overlapping_contracts["products"][run_key] + program = overlapping_contracts["programs"][program_key] if program_key else None + + result = _determine_contract_for_user_product(user, product, program=program) + + assert result == { + "result": USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, + "failed_match": failed_match, + } + + +@pytest.mark.parametrize("contract_slug", [None, "not-a-real-contract"]) +@pytest.mark.parametrize( + ("run_key", "program_key"), + [ + # Run is in both contracts, no program to narrow it down + ("ab", None), + # Run and program are both in both contracts + ("ab", "ab"), + # Run and program each match one of the user's contracts, but not the + # same one - there's no single contract that covers everything + ("a", "b"), + ], +) +def test_determine_contract_ambiguous( + overlapping_contracts, run_key, program_key, contract_slug +): + """If the right contract can't be narrowed down to one, the result is ambiguous.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + product = overlapping_contracts["products"][run_key] + program = overlapping_contracts["programs"][program_key] if program_key else None + + result = _determine_contract_for_user_product( + user, product, program=program, contract_slug=contract_slug + ) + + assert result == {"result": USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT} + + +@pytest.mark.parametrize("slug_key", ["a", "b"]) +@pytest.mark.parametrize("program_key", [None, "ab"]) +def test_determine_contract_with_slug(overlapping_contracts, slug_key, program_key): + """An explicit contract slug should pick that contract out of several valid ones.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + product = overlapping_contracts["products"]["ab"] + program = overlapping_contracts["programs"][program_key] if program_key else None + + result = _determine_contract_for_user_product( + user, product, program=program, contract_slug=contracts[slug_key].slug + ) + + assert result == contracts[slug_key].id + + +@pytest.mark.parametrize( + ("user_contracts", "run_key", "program_key"), + [ + # User isn't in the specified contract + (["b"], "ab", None), + # Run isn't in the specified contract + (["a", "b"], "b", None), + # Program isn't in the specified contract + (["a", "b"], "ab", "b"), + ], +) +def test_determine_contract_with_slug_mismatch( + overlapping_contracts, user_contracts, run_key, program_key +): + """If any part of the transaction isn't in the specified contract, it should fail.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, user_contracts) + product = overlapping_contracts["products"][run_key] + program = overlapping_contracts["programs"][program_key] if program_key else None + + result = _determine_contract_for_user_product( + user, product, program=program, contract_slug=contracts["a"].slug + ) + + assert result == {"result": USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} + + +def test_determine_contract_unknown_slug_falls_back(overlapping_contracts): + """A slug that doesn't match a contract should fall back to finding the overlap.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + + result = _determine_contract_for_user_product( + user, overlapping_contracts["products"]["a"], contract_slug="not-a-contract" + ) + + assert result == contracts["a"].id + + +@pytest.mark.parametrize("contract_slug", [None, "slug"]) +def test_determine_contract_program_not_in_contract( + overlapping_contracts, contract_slug +): + """A program that isn't in any contract can't be used for a B2B enrollment.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + + result = _determine_contract_for_user_product( + user, + overlapping_contracts["products"]["a"], + program=overlapping_contracts["programs"]["none"], + contract_slug=contracts["a"].slug if contract_slug else None, + ) + + assert result == {"result": USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} + + +def test_determine_contract_no_purchasable_object(mocker): + """A product without a purchasable object is an error.""" + + product = mocker.Mock(purchasable_object=None) + + with pytest.raises(ValueError, match="purchasable object"): + _determine_contract_for_user_product(UserFactory.create(), product) + + +@pytest.mark.xfail( + raises=AttributeError, + strict=True, + reason="Program has no b2b_contracts relation, so program products can't be resolved.", +) +def test_determine_contract_program_product(overlapping_contracts): + """A product for a program should resolve to the program's contract.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a"]) + with reversion.create_revision(): + product = ProductFactory.create( + purchasable_object=overlapping_contracts["programs"]["a"] + ) + + assert _determine_contract_for_user_product(user, product) == contracts["a"].id + + +@pytest.mark.parametrize( + ("run_key", "program_key", "slug_key", "expected"), + [ + ("a", None, None, "a"), + ("ab", "b", None, "b"), + ("ab", None, "a", "a"), + ("ab", "ab", "b", "b"), + ], +) +def test_validate_b2b_prereqs_returns_contract( + overlapping_contracts, run_key, program_key, slug_key, expected +): + """When validation passes, the resolved contract object should be returned.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + program = overlapping_contracts["programs"][program_key] if program_key else None + + result = _validate_b2b_enrollment_prerequisites( + user, + overlapping_contracts["products"][run_key], + program=program, + contract_slug=contracts[slug_key].slug if slug_key else None, + ) + + assert isinstance(result, ContractPage) + assert result == contracts[expected] + + +@pytest.mark.parametrize( + ("user_contracts", "run_key", "program_key", "expected"), + [ + (["b"], "a", None, USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH), + (["a"], "a", "b", USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH), + (["a", "b"], "ab", None, USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT), + (["a", "b"], "a", "none", USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT), + ], +) +def test_validate_b2b_prereqs_contract_errors( + overlapping_contracts, user_contracts, run_key, program_key, expected +): + """Contract resolution errors should be passed back out.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, user_contracts) + program = overlapping_contracts["programs"][program_key] if program_key else None + + result = _validate_b2b_enrollment_prerequisites( + user, overlapping_contracts["products"][run_key], program=program + ) + + assert result["result"] == expected + + +@pytest.mark.parametrize("inactive_by", ["flag", "date"]) +def test_validate_b2b_prereqs_inactive_contract(overlapping_contracts, inactive_by): + """A contract that resolves but isn't active can't be enrolled in.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + + if inactive_by == "flag": + contracts["a"].active = False + else: + contracts["a"].contract_end = now_in_utc() - timedelta(days=1) + contracts["a"].save() + + result = _validate_b2b_enrollment_prerequisites( + user, + overlapping_contracts["products"]["ab"], + contract_slug=contracts["a"].slug, + ) + + assert result == {"result": USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} + + +def test_validate_b2b_prereqs_run_not_enrollable(overlapping_contracts): + """A run in the contract whose enrollment period has closed isn't enrollable.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a"]) + run = overlapping_contracts["runs"]["a"] + run.enrollment_end = now_in_utc() - timedelta(days=1) + run.save() + + result = _validate_b2b_enrollment_prerequisites( + user, overlapping_contracts["products"]["a"] + ) + + assert result == {"result": USER_MSG_TYPE_B2B_ERROR_NOT_ENROLLABLE} + + +def test_validate_b2b_prereqs_already_enrolled_other_contract(overlapping_contracts): + """ + An existing verified enrollment in the run blocks a new enrollment, even if + it was made through the user's other contract. + """ + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + CourseRunEnrollmentFactory.create( + user=user, + run=overlapping_contracts["runs"]["ab"], + enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE, + b2b_contract=contracts["a"], + ) + + result = _validate_b2b_enrollment_prerequisites( + user, + overlapping_contracts["products"]["ab"], + contract_slug=contracts["b"].slug, + ) + + assert result == {"result": USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED} + + +@pytest.fixture +def b2b_enrollment_mocks(mocker, settings): + """Mock out the external calls that a B2B enrollment makes.""" + + mocker.patch("openedx.api.enroll_in_edx_course_runs") + mocker.patch("hubspot_sync.task_helpers.sync_hubspot_deal") + mocker.patch("hubspot_sync.tasks.sync_deal_with_hubspot.apply_async") + mocker.patch("hubspot_sync.tasks.sync_cart_add_event_with_hubspot.apply_async") + settings.OPENEDX_SERVICE_WORKER_API_TOKEN = "a token" # noqa: S105 + settings.OPENEDX_SERVICE_WORKER_USERNAME = "a username" + + +def _attach_bulk_discount(product, amount=Decimal(0)): + """ + Attach an unlimited fixed-price bulk discount to the product. + + _apply_available_discount can only create a discount on the fly for runs + in a single contract, so runs in several contracts need one ahead of time. + """ + + discount = UnlimitedUseDiscountFactory.create( + is_bulk=True, discount_type=DISCOUNT_TYPE_FIXED_PRICE, amount=amount + ) + DiscountProduct.objects.create(discount=discount, product=product) + return discount + + +def _b2b_request(user): + """Make a request for the user.""" + + request = RequestFactory().get("/") + request.user = user + return request + + +@pytest.mark.parametrize("slug_key", ["a", "b"]) +def test_create_b2b_enrollment_stores_contract( + b2b_enrollment_mocks, overlapping_contracts, slug_key +): + """ + Enrolling in a run that's in several contracts should record the chosen + contract on the order line and on the resulting enrollment. + """ + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + run = overlapping_contracts["runs"]["ab"] + product = overlapping_contracts["products"]["ab"] + _attach_bulk_discount(product) + + result = create_b2b_enrollment( + _b2b_request(user), product, contract_slug=contracts[slug_key].slug + ) + + assert result["result"] == USER_MSG_TYPE_B2B_ENROLL_SUCCESS + + enrollment = CourseRunEnrollment.objects.get(user=user, run=run) + assert enrollment.enrollment_mode == EDX_ENROLLMENT_VERIFIED_MODE + assert enrollment.b2b_contract == contracts[slug_key] + + line = Line.objects.get(order__purchaser=user, purchased_object_id=run.id) + assert line.b2b_contract == contracts[slug_key] + + +def test_create_b2b_enrollment_with_program_stores_contract( + b2b_enrollment_mocks, overlapping_contracts +): + """ + The program should narrow the contract down, and both the course run and + program enrollments should be linked to it. + """ + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + run = overlapping_contracts["runs"]["ab"] + product = overlapping_contracts["products"]["ab"] + program = overlapping_contracts["programs"]["b"] + _attach_bulk_discount(product) + + result = create_b2b_enrollment( + _b2b_request(user), product, program_id=program.readable_id + ) + + assert result["result"] == USER_MSG_TYPE_B2B_ENROLL_SUCCESS + assert ( + CourseRunEnrollment.objects.get(user=user, run=run).b2b_contract + == contracts["b"] + ) + assert ( + ProgramEnrollment.objects.get(user=user, program=program).b2b_contract + == contracts["b"] + ) + + +def test_create_b2b_enrollment_single_contract_run( + b2b_enrollment_mocks, overlapping_contracts +): + """A run in one contract should be linked without a slug or pre-made discount.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + run = overlapping_contracts["runs"]["a"] + + result = create_b2b_enrollment( + _b2b_request(user), overlapping_contracts["products"]["a"] + ) + + assert result["result"] == USER_MSG_TYPE_B2B_ENROLL_SUCCESS + assert ( + CourseRunEnrollment.objects.get(user=user, run=run).b2b_contract + == contracts["a"] + ) + + +def test_create_b2b_enrollment_requires_checkout_keeps_contract( + b2b_enrollment_mocks, overlapping_contracts +): + """If the user has to pay, the basket item should hold the chosen contract.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + run = overlapping_contracts["runs"]["ab"] + product = overlapping_contracts["products"]["ab"] + product.price = Decimal(100) + product.save() + _attach_bulk_discount(product, amount=Decimal(50)) + + result = create_b2b_enrollment( + _b2b_request(user), product, contract_slug=contracts["b"].slug + ) + + assert result["result"] == USER_MSG_TYPE_B2B_ERROR_REQUIRES_CHECKOUT + basket_item = Basket.objects.get(user=user).basket_items.get() + assert basket_item.product == product + assert basket_item.b2b_contract == contracts["b"] + assert not CourseRunEnrollment.objects.filter(user=user, run=run).exists() + + +@pytest.mark.parametrize( + ("user_contracts", "run_key", "program_key", "contract_slug_key", "expected"), + [ + (["a", "b"], "ab", None, None, USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT), + (["b"], "a", None, None, USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH), + (["a"], "a", "b", None, USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH), + (["a"], "ab", None, "b", USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT), + ], +) +def test_create_b2b_enrollment_contract_errors( # noqa: PLR0913 + b2b_enrollment_mocks, + overlapping_contracts, + user_contracts, + run_key, + program_key, + contract_slug_key, + expected, +): + """If the contract can't be resolved, nothing should be enrolled or put in the basket.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, user_contracts) + program = overlapping_contracts["programs"][program_key] if program_key else None + + result = create_b2b_enrollment( + _b2b_request(user), + overlapping_contracts["products"][run_key], + program_id=program.readable_id if program else None, + contract_slug=contracts[contract_slug_key].slug if contract_slug_key else None, + ) + + assert result["result"] == expected + assert not Basket.objects.filter(user=user).exists() + assert not CourseRunEnrollment.all_objects.filter(user=user).exists() + assert not ProgramEnrollment.all_objects.filter(user=user).exists() + + +@pytest.mark.xfail( + raises=ValueError, + strict=True, + reason="_apply_available_discount can't create a discount for a run in more than one contract.", +) +def test_create_b2b_enrollment_multi_contract_run_without_discount( + b2b_enrollment_mocks, overlapping_contracts +): + """A run in several contracts should be enrollable once a contract is chosen.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + + result = create_b2b_enrollment( + _b2b_request(user), + overlapping_contracts["products"]["ab"], + contract_slug=contracts["a"].slug, + ) + + assert result["result"] == USER_MSG_TYPE_B2B_ENROLL_SUCCESS diff --git a/b2b/serializers/v0/__init__.py b/b2b/serializers/v0/__init__.py index 91772062af..e12df52cd9 100644 --- a/b2b/serializers/v0/__init__.py +++ b/b2b/serializers/v0/__init__.py @@ -145,6 +145,9 @@ class B2BEnrollRequestSerializer(serializers.Serializer): Accepts an optional program_id so the user can be enrolled in the appropriate program alongside the course run enrollment. + Accepts an optional contract_slug so it can identify which contract + the user is working in, so the enrollments can be linked back to the + right contract. """ program_id = serializers.CharField( @@ -152,6 +155,11 @@ class B2BEnrollRequestSerializer(serializers.Serializer): allow_blank=True, help_text="The readable_id of the program to enroll the user in.", ) + contract_slug = serializers.CharField( + required=False, + allow_blank=True, + help_text="The slug for the contract the user is in.", + ) class CreateB2BEnrollmentSerializer(serializers.Serializer): diff --git a/b2b/views/v0/__init__.py b/b2b/views/v0/__init__.py index 640ca29734..8c82fba7cc 100644 --- a/b2b/views/v0/__init__.py +++ b/b2b/views/v0/__init__.py @@ -158,9 +158,11 @@ def post(self, request, readable_id: str, format=None): # noqa: A002, ARG002 """Create an enrollment for the given course run.""" course_run_content_type = ContentType.objects.get_for_model(CourseRun) - courserun = CourseRun.objects.filter( - courseware_id=readable_id, b2b_contracts__isnull=False - ).get() + courserun = ( + CourseRun.objects.annotate(b2b_contract_count=Count("b2b_contracts")) + .filter(courseware_id=readable_id, b2b_contract_count__gt=0) + .get() + ) product = Product.objects.filter( content_type=course_run_content_type, object_id=courserun.id ).get() @@ -169,13 +171,16 @@ def post(self, request, readable_id: str, format=None): # noqa: A002, ARG002 request_serializer = B2BEnrollRequestSerializer(data=request.data) request_serializer.is_valid(raise_exception=True) program_id = request_serializer.validated_data.get("program_id") + contract_slug = request_serializer.validated_data.get("contract_slug") - response = create_b2b_enrollment(request, product, program_id=program_id) + response = create_b2b_enrollment( + request, product, program_id=program_id, contract_slug=contract_slug + ) return Response( CreateB2BEnrollmentSerializer(response).data, status=status.HTTP_201_CREATED - if response["result"] == USER_MSG_TYPE_B2B_ENROLL_SUCCESS + if response and response["result"] == USER_MSG_TYPE_B2B_ENROLL_SUCCESS else status.HTTP_400_BAD_REQUEST, ) diff --git a/b2b/views/v0/views_test.py b/b2b/views/v0/views_test.py index a8284cd5b5..57e1cd4652 100644 --- a/b2b/views/v0/views_test.py +++ b/b2b/views/v0/views_test.py @@ -19,11 +19,14 @@ from b2b.factories import ContractPageFactory from b2b.models import DiscountContractAttachmentRedemption, UserOrganization from courses.factories import CourseRunFactory -from courses.models import CourseRunEnrollment +from courses.models import CourseRun, CourseRunEnrollment +from ecommerce.constants import DISCOUNT_TYPE_FIXED_PRICE from ecommerce.factories import ProductFactory, UnlimitedUseDiscountFactory +from ecommerce.models import DiscountProduct from main.constants import ( USER_MSG_TYPE_B2B_ENROLL_SUCCESS, USER_MSG_TYPE_B2B_ERROR_ALREADY_ENROLLED, + USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT, USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT, USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH, USER_MSG_TYPE_B2B_ERROR_NOT_ENROLLABLE, @@ -847,6 +850,85 @@ def test_enroll_omits_program_id_when_not_provided(mocker): assert kwargs["program_id"] is None +@pytest.mark.parametrize("send_slug", [True, False]) +def test_enroll_passes_contract_slug_to_api(mocker, send_slug): + """The contract_slug from the request body should be forwarded to create_b2b_enrollment.""" + contract = ContractPageFactory.create( + membership_type=CONTRACT_MEMBERSHIP_MANAGED, + enrollment_fixed_price=0, + ) + courserun = CourseRunFactory.create(b2b_only=True) + courserun.b2b_contracts.add(contract) + ProductFactory.create(purchasable_object=courserun) + + mocked_enroll = mocker.patch( + "b2b.views.v0.create_b2b_enrollment", + return_value={"result": USER_MSG_TYPE_B2B_ENROLL_SUCCESS}, + ) + + user = UserFactory.create() + user.b2b_contracts.add(contract) + client = APIClient() + client.force_login(user) + + url = reverse("b2b:enroll-user", kwargs={"readable_id": courserun.courseware_id}) + resp = client.post( + url, + data={"contract_slug": contract.slug} if send_slug else {}, + format="json", + ) + + assert resp.status_code == 201 + _, kwargs = mocked_enroll.call_args + assert kwargs["contract_slug"] == (contract.slug if send_slug else None) + + +@pytest.mark.xfail( + raises=CourseRun.MultipleObjectsReturned, + strict=True, + reason="The run lookup joins b2b_contracts, so it returns one row per contract.", +) +def test_enroll_multi_contract_run_with_slug(mocker): + """ + Enrolling through the API in a run that's in two contracts should use the + contract named in the request. + """ + mocker.patch("openedx.api.enroll_in_edx_course_runs") + mocker.patch("hubspot_sync.task_helpers.sync_hubspot_deal") + mocker.patch("hubspot_sync.tasks.sync_deal_with_hubspot.apply_async") + mocker.patch("hubspot_sync.tasks.sync_cart_add_event_with_hubspot.apply_async") + + contracts = ContractPageFactory.create_batch( + 2, membership_type=CONTRACT_MEMBERSHIP_MANAGED, enrollment_fixed_price=0 + ) + courserun = CourseRunFactory.create(b2b_only=True, b2b_contracts=contracts) + with reversion.create_revision(): + product = ProductFactory.create(purchasable_object=courserun, price=0) + discount = UnlimitedUseDiscountFactory.create( + is_bulk=True, discount_type=DISCOUNT_TYPE_FIXED_PRICE, amount=0 + ) + DiscountProduct.objects.create(discount=discount, product=product) + + user = UserFactory.create() + user.b2b_contracts.add(*contracts) + client = APIClient() + client.force_login(user) + + url = reverse("b2b:enroll-user", kwargs={"readable_id": courserun.courseware_id}) + + resp = client.post(url, format="json") + assert resp.status_code == 400 + assert resp.json()["result"] == USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT + + resp = client.post(url, data={"contract_slug": contracts[1].slug}, format="json") + assert resp.status_code == 201 + assert resp.json()["result"] == USER_MSG_TYPE_B2B_ENROLL_SUCCESS + assert ( + CourseRunEnrollment.objects.get(user=user, run=courserun).b2b_contract + == contracts[1] + ) + + def test_enroll_courserun_without_b2b_contract_not_found(mocker): """A course run that exists but has no b2b_contract should not be matched.""" mocker.patch("b2b.views.v0.create_b2b_enrollment") diff --git a/courses/admin.py b/courses/admin.py index 02bfeceee8..90d1a46838 100644 --- a/courses/admin.py +++ b/courses/admin.py @@ -740,6 +740,7 @@ class CourseRunEnrollmentAdmin(ModelAdminRunActionsForAllMixin, AuditableModelAd "user__username", "run__courseware_id", "run__title", + "b2b_contract__slug", ] list_filter = [ "active", @@ -747,6 +748,7 @@ class CourseRunEnrollmentAdmin(ModelAdminRunActionsForAllMixin, AuditableModelAd "edx_enrolled", "enrollment_mode", RepairExhaustedFilter, + "b2b_contract__slug", ] list_display = ( "id", @@ -757,6 +759,7 @@ class CourseRunEnrollmentAdmin(ModelAdminRunActionsForAllMixin, AuditableModelAd "created_on", "edx_enrollment_retry_count", "repair_exhausted", + "b2b_contract__slug", ) raw_id_fields = ( "user", @@ -817,6 +820,15 @@ def get_run_courseware_id(self, obj): """Returns the related CourseRun courseware_id""" return obj.run.courseware_id + @admin.display( + description="B2B Contract", + ordering="b2b_contract__slug", + ) + def get_b2b_contract(self, obj): + """Return the associated B2B contract.""" + + return obj.b2b_contract.slug + @admin.action(description="Retry all failed Open edX enrollments") def retry_all_failed_edx_enrollment(self, request, queryset): # noqa: ARG002 """Admin action to retry all failed Open edX enrollments""" diff --git a/ecommerce/models_test.py b/ecommerce/models_test.py index ba02747340..13569d5b82 100644 --- a/ecommerce/models_test.py +++ b/ecommerce/models_test.py @@ -15,7 +15,13 @@ from reversion.models import Version from b2b.factories import ContractPageFactory -from courses.factories import CourseRunFactory, ProgramFactory +from courses.factories import ( + CourseRunEnrollmentFactory, + CourseRunFactory, + ProgramEnrollmentFactory, + ProgramFactory, +) +from courses.models import CourseRunEnrollment, ProgramEnrollment from ecommerce.constants import ( DISCOUNT_TYPE_DOLLARS_OFF, DISCOUNT_TYPE_FIXED_PRICE, @@ -46,6 +52,7 @@ make_purchase, ) from ecommerce.fixtures import stripe_event +from ecommerce.hooks.process_transaction_line import _link_b2b_course_run_contracts from ecommerce.models import ( Basket, BasketDiscount, @@ -66,6 +73,7 @@ Transaction, UserDiscount, ) +from openedx.constants import EDX_ENROLLMENT_AUDIT_MODE, EDX_ENROLLMENT_VERIFIED_MODE from users.factories import UserFactory pytestmark = [pytest.mark.django_db] @@ -2069,3 +2077,153 @@ def test_chaining_credits_each_dollar_at_most_once(user): assert resolve_program_child_purchase(user, parent_product).amount == Decimal( "200.00" ) + + +def test_basket_get_products_contracts(user): + """get_products_contracts should pair each basket product with its B2B contract.""" + + contract = ContractPageFactory.create() + b2b_item = BasketItemFactory.create(basket__user=user, b2b_contract=contract) + regular_item = BasketItemFactory.create(basket=b2b_item.basket) + + assert sorted( + b2b_item.basket.get_products_contracts(), key=lambda pair: pair[0].id + ) == sorted( + [(b2b_item.product, contract), (regular_item.product, None)], + key=lambda pair: pair[0].id, + ) + + +def test_create_from_basket_copies_b2b_contract_to_lines(user): + """Creating an order from a basket should carry each item's contract onto its line.""" + + contract = ContractPageFactory.create() + b2b_run = CourseRunFactory.create(b2b_only=True, b2b_contracts=[contract]) + with reversion.create_revision(): + b2b_product = ProductFactory.create(purchasable_object=b2b_run) + regular_product = ProductFactory.create() + + basket = BasketFactory.create(user=user) + BasketItem.objects.create(basket=basket, product=b2b_product, b2b_contract=contract) + BasketItem.objects.create(basket=basket, product=regular_product) + + order = PendingOrder.create_from_basket(basket) + + assert order.lines.get(purchased_object_id=b2b_run.id).b2b_contract == contract + assert ( + order.lines.get(purchased_object_id=regular_product.object_id).b2b_contract + is None + ) + + +@pytest.mark.parametrize("line_has_contract", [True, False]) +def test_link_b2b_course_run_contracts(user, line_has_contract): + """ + The verified enrollment for the purchased run should get the line's contract. + + Enrollments that don't belong to the purchase - another user's enrollment in + the same run, or the purchaser's enrollment in a different run of the same + contract - should be left alone. + """ + + contract = ContractPageFactory.create() + run = CourseRunFactory.create(b2b_only=True, b2b_contracts=[contract]) + other_run = CourseRunFactory.create(b2b_only=True, b2b_contracts=[contract]) + other_user = UserFactory.create() + + enrollment = CourseRunEnrollmentFactory.create( + user=user, run=run, enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE + ) + other_run_enrollment = CourseRunEnrollmentFactory.create( + user=user, run=other_run, enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE + ) + other_user_enrollment = CourseRunEnrollmentFactory.create( + user=other_user, run=run, enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE + ) + + line = make_purchase(user, run, Decimal("0.00")) + line.b2b_contract = contract if line_has_contract else None + line.save() + + _link_b2b_course_run_contracts(line) + + enrollment.refresh_from_db() + other_run_enrollment.refresh_from_db() + other_user_enrollment.refresh_from_db() + + assert enrollment.b2b_contract == (contract if line_has_contract else None) + assert other_run_enrollment.b2b_contract is None + assert other_user_enrollment.b2b_contract is None + + +def test_link_b2b_course_run_contracts_ignores_audit_enrollment(user): + """Only the verified enrollment should be linked to the contract.""" + + contract = ContractPageFactory.create() + run = CourseRunFactory.create(b2b_only=True, b2b_contracts=[contract]) + enrollment = CourseRunEnrollmentFactory.create( + user=user, run=run, enrollment_mode=EDX_ENROLLMENT_AUDIT_MODE + ) + + line = make_purchase(user, run, Decimal("0.00")) + line.b2b_contract = contract + line.save() + + _link_b2b_course_run_contracts(line) + + enrollment.refresh_from_db() + assert enrollment.b2b_contract is None + + +def test_link_b2b_course_run_contracts_skips_programs(user): + """Program lines aren't handled by this hook, so enrollments are left alone.""" + + contract = ContractPageFactory.create() + program = ProgramFactory.create(b2b_only=True) + program_enrollment = ProgramEnrollmentFactory.create( + user=user, program=program, enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE + ) + + line = make_purchase(user, program, Decimal("0.00")) + line.b2b_contract = contract + line.save() + + assert _link_b2b_course_run_contracts(line) is None + + program_enrollment.refresh_from_db() + assert program_enrollment.b2b_contract is None + assert not CourseRunEnrollment.all_objects.filter(user=user).exists() + assert ProgramEnrollment.all_objects.filter(user=user).count() == 1 + + +@pytest.mark.skip_nplusone_check +def test_fulfill_links_b2b_contract_to_enrollment( + mocker, user, django_capture_on_commit_callbacks +): + """ + Fulfilling an order with a B2B line should run the hooks in order: create the + enrollment, then link it to the line's contract. + """ + + mocker.patch("openedx.api.enroll_in_edx_course_runs") + mocker.patch("ecommerce.tasks.send_ecommerce_order_receipt.delay") + mocker.patch("hubspot_sync.task_helpers.sync_hubspot_deal") + + contract = ContractPageFactory.create() + other_contract = ContractPageFactory.create() + run = CourseRunFactory.create( + b2b_only=True, b2b_contracts=[contract, other_contract] + ) + with reversion.create_revision(): + product = ProductFactory.create(purchasable_object=run, price=Decimal(0)) + + basket = BasketFactory.create(user=user) + BasketItem.objects.create(basket=basket, product=product, b2b_contract=contract) + order = PendingOrder.create_from_basket(basket) + + with django_capture_on_commit_callbacks(execute=True): + order.get_object_flow().fulfill(ZERO_PAYMENT_DATA, skip_receipt=True) + + enrollment = CourseRunEnrollment.objects.get(user=user, run=run) + assert enrollment.enrollment_mode == EDX_ENROLLMENT_VERIFIED_MODE + assert enrollment.b2b_contract == contract diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index fd6a35f40b..89d4ab975e 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -4677,10 +4677,16 @@ components: Accepts an optional program_id so the user can be enrolled in the appropriate program alongside the course run enrollment. + Accepts an optional contract_slug so it can identify which contract + the user is working in, so the enrollments can be linked back to the + right contract. properties: program_id: type: string description: The readable_id of the program to enroll the user in. + contract_slug: + type: string + description: The slug for the contract the user is in. BaseContractPage: type: object description: Simplified serializer for the ContractPage model. diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index e1cd58bded..5773a69f8f 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -4677,10 +4677,16 @@ components: Accepts an optional program_id so the user can be enrolled in the appropriate program alongside the course run enrollment. + Accepts an optional contract_slug so it can identify which contract + the user is working in, so the enrollments can be linked back to the + right contract. properties: program_id: type: string description: The readable_id of the program to enroll the user in. + contract_slug: + type: string + description: The slug for the contract the user is in. BaseContractPage: type: object description: Simplified serializer for the ContractPage model. diff --git a/openapi/specs/v2.yaml b/openapi/specs/v2.yaml index c3a82c0460..6c2c361bf8 100644 --- a/openapi/specs/v2.yaml +++ b/openapi/specs/v2.yaml @@ -4677,10 +4677,16 @@ components: Accepts an optional program_id so the user can be enrolled in the appropriate program alongside the course run enrollment. + Accepts an optional contract_slug so it can identify which contract + the user is working in, so the enrollments can be linked back to the + right contract. properties: program_id: type: string description: The readable_id of the program to enroll the user in. + contract_slug: + type: string + description: The slug for the contract the user is in. BaseContractPage: type: object description: Simplified serializer for the ContractPage model. From dbfbacb6de79eb210b6636b0600e92d7e58c589f Mon Sep 17 00:00:00 2001 From: James Kachel Date: Wed, 23 Sep 2026 15:18:21 -0500 Subject: [PATCH 11/11] Final test fixes and some other bug updates --- b2b/api.py | 38 ++++++------ b2b/api_test.py | 118 ++++++++++++++++++++++++++++++++----- b2b/views/v0/views_test.py | 8 +-- courses/models.py | 2 +- 4 files changed, 124 insertions(+), 42 deletions(-) diff --git a/b2b/api.py b/b2b/api.py index babf8751fe..a953966b28 100644 --- a/b2b/api.py +++ b/b2b/api.py @@ -1363,7 +1363,9 @@ def _determine_contract_for_user_product( # noqa: PLR0911 user_contract_ids = list(user.b2b_contracts.values_list("id", flat=True)) item_b2b_contracts = ( - item.b2b_contracts if isinstance(item, CourseRun) else item.contract_memberships + item.b2b_contracts.all() + if isinstance(item, CourseRun) + else ContractPage.objects.filter(contract_programs__program=item) ) if program and not program.contract_memberships.exists(): @@ -1448,26 +1450,24 @@ def _determine_contract_for_user_product( # noqa: PLR0911 return contract_matches.pop() - if ( - user.b2b_contracts.filter(slug=contract_slug).exists() - and item_b2b_contracts.filter(slug=contract_slug).exists() - and ( - not program - or program.contract_memberships.filter( - contract__slug=contract_slug - ).exists() + contract_qs = item_b2b_contracts.filter( + slug=contract_slug, id__in=user_contract_ids + ) + if program: + contract_qs = contract_qs.filter(contract_programs__program=program) + contract = contract_qs.first() + + if not contract: + log.error( + "User %s tried to use product %s (and/or program %s) for contract %s but one or more parts of the transaction weren't in the contract", + user, + product, + program, + contract_slug, ) - ): - return item_b2b_contracts.filter(slug=contract_slug).get().id + return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} - log.error( - "User %s tried to use product %s (and/or program %s) for contract %s but one or more parts of the transaction weren't in the contract", - user, - product, - program, - contract_slug, - ) - return {"result": main_constants.USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} + return contract.id def _validate_b2b_enrollment_prerequisites( # noqa: PLR0911 diff --git a/b2b/api_test.py b/b2b/api_test.py index b69c07639a..8a6030296a 100644 --- a/b2b/api_test.py +++ b/b2b/api_test.py @@ -1690,7 +1690,7 @@ def test_apply_available_discount_seat_limit(): # Calling this directly should result in a new discount being created. - _apply_available_discount(request, products[0], basket) + _apply_available_discount(request, products[0], basket, contract) assert contract.get_discounts().count() == 5 @@ -1771,7 +1771,7 @@ def test_apply_available_discount_unlimited_seats(existing_discounts): # we manually did it above this should return successfully. assert result == contract - _apply_available_discount(request, products[0], basket) + _apply_available_discount(request, products[0], basket, contract) assert contract.get_discounts().count() == (2 if existing_discounts else 1) @@ -2541,22 +2541,113 @@ def test_determine_contract_no_purchasable_object(mocker): _determine_contract_for_user_product(UserFactory.create(), product) -@pytest.mark.xfail( - raises=AttributeError, - strict=True, - reason="Program has no b2b_contracts relation, so program products can't be resolved.", +@pytest.mark.parametrize( + ("user_contracts", "program_key", "slug_key", "expected"), + [ + (["a"], "a", None, "a"), + (["a", "b"], "b", None, "b"), + (["a", "b"], "ab", "a", "a"), + (["a", "b"], "ab", "b", "b"), + (["a", "b"], "ab", None, USER_MSG_TYPE_B2B_ERROR_AMBIGUOUS_CONTRACT), + (["b"], "a", None, USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT_MATCH), + (["a", "b"], "a", "b", USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT), + ], ) -def test_determine_contract_program_product(overlapping_contracts): - """A product for a program should resolve to the program's contract.""" +def test_determine_contract_program_product( + overlapping_contracts, user_contracts, program_key, slug_key, expected +): + """A product for a program should resolve against the program's contracts.""" contracts = overlapping_contracts["contracts"] - user = _make_contract_user(contracts, ["a"]) + user = _make_contract_user(contracts, user_contracts) with reversion.create_revision(): product = ProductFactory.create( - purchasable_object=overlapping_contracts["programs"]["a"] + purchasable_object=overlapping_contracts["programs"][program_key] ) - assert _determine_contract_for_user_product(user, product) == contracts["a"].id + result = _determine_contract_for_user_product( + user, product, contract_slug=contracts[slug_key].slug if slug_key else None + ) + + if expected in contracts: + assert result == contracts[expected].id + else: + assert result["result"] == expected + + +def test_validate_b2b_prereqs_program_product(overlapping_contracts): + """An enrollable program in the user's contract should validate.""" + + contracts = overlapping_contracts["contracts"] + user = _make_contract_user(contracts, ["a", "b"]) + program = overlapping_contracts["programs"]["ab"] + program.live = True + program.start_date = now_in_utc() - timedelta(days=1) + program.enrollment_start = now_in_utc() - timedelta(days=1) + program.enrollment_end = now_in_utc() + timedelta(days=30) + program.save() + with reversion.create_revision(): + product = ProductFactory.create(purchasable_object=program) + + result = _validate_b2b_enrollment_prerequisites( + user, product, contract_slug=contracts["b"].slug + ) + + assert result == contracts["b"] + + +@pytest.mark.parametrize("run_in_users_contract", [True, False]) +def test_determine_contract_with_duplicate_slug(run_in_users_contract): + """ + Contract slugs are only unique within an organization, so a slug can name + contracts in two organizations. The resolved contract must be the one the + user is actually in. + """ + + users_contract = factories.ContractPageFactory.create(slug="shared-slug") + other_contract = factories.ContractPageFactory.create(slug="shared-slug") + assert users_contract.organization != other_contract.organization + + run_contracts = [other_contract] + if run_in_users_contract: + run_contracts.append(users_contract) + run = CourseRunFactory.create(b2b_only=True, b2b_contracts=run_contracts) + with reversion.create_revision(): + product = ProductFactory.create(purchasable_object=run) + + user = UserFactory.create() + user.b2b_contracts.add(users_contract) + + result = _determine_contract_for_user_product( + user, product, contract_slug="shared-slug" + ) + + if run_in_users_contract: + assert result == users_contract.id + else: + assert result == {"result": USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} + + +def test_validate_b2b_prereqs_duplicate_slug_other_org(): + """ + A user can't enroll through another organization's contract just because + it has the same slug as their own. + """ + + users_contract = factories.ContractPageFactory.create(slug="shared-slug") + other_contract = factories.ContractPageFactory.create(slug="shared-slug") + run = CourseRunFactory.create(b2b_only=True, b2b_contracts=[other_contract]) + with reversion.create_revision(): + product = ProductFactory.create(purchasable_object=run) + + user = UserFactory.create() + user.b2b_contracts.add(users_contract) + + result = _validate_b2b_enrollment_prerequisites( + user, product, contract_slug="shared-slug" + ) + + assert result == {"result": USER_MSG_TYPE_B2B_ERROR_NO_CONTRACT} @pytest.mark.parametrize( @@ -2850,11 +2941,6 @@ def test_create_b2b_enrollment_contract_errors( # noqa: PLR0913 assert not ProgramEnrollment.all_objects.filter(user=user).exists() -@pytest.mark.xfail( - raises=ValueError, - strict=True, - reason="_apply_available_discount can't create a discount for a run in more than one contract.", -) def test_create_b2b_enrollment_multi_contract_run_without_discount( b2b_enrollment_mocks, overlapping_contracts ): diff --git a/b2b/views/v0/views_test.py b/b2b/views/v0/views_test.py index 57e1cd4652..84ed6ec93e 100644 --- a/b2b/views/v0/views_test.py +++ b/b2b/views/v0/views_test.py @@ -19,7 +19,7 @@ from b2b.factories import ContractPageFactory from b2b.models import DiscountContractAttachmentRedemption, UserOrganization from courses.factories import CourseRunFactory -from courses.models import CourseRun, CourseRunEnrollment +from courses.models import CourseRunEnrollment from ecommerce.constants import DISCOUNT_TYPE_FIXED_PRICE from ecommerce.factories import ProductFactory, UnlimitedUseDiscountFactory from ecommerce.models import DiscountProduct @@ -883,11 +883,7 @@ def test_enroll_passes_contract_slug_to_api(mocker, send_slug): assert kwargs["contract_slug"] == (contract.slug if send_slug else None) -@pytest.mark.xfail( - raises=CourseRun.MultipleObjectsReturned, - strict=True, - reason="The run lookup joins b2b_contracts, so it returns one row per contract.", -) +@pytest.mark.skip_nplusone_check def test_enroll_multi_contract_run_with_slug(mocker): """ Enrolling through the API in a run that's in two contracts should use the diff --git a/courses/models.py b/courses/models.py index c21aa65380..c754ed39ca 100644 --- a/courses/models.py +++ b/courses/models.py @@ -812,7 +812,7 @@ def is_enrollable(self): def enrollable_for_contract(self, contract) -> bool: """Determine if the run is enrollable for the specified contract.""" - if not self.b2b_contracts.filter(pk=contract.id).exists(): + if not self.contract_memberships.filter(contract_id=contract.id).exists(): return False return self.is_enrollable