From c7af1ef8a62dc16fa97fce01428209fb8de5343c Mon Sep 17 00:00:00 2001 From: Cyril Nxumalo <80963114+zwidekalanga@users.noreply.github.com> Date: Mon, 1 Jun 2026 09:57:40 +0200 Subject: [PATCH 1/2] fix: validate late enrollment course run metadata --- .../apps/api_client/enterprise.py | 3 +- enterprise_subsidy/apps/subsidy/constants.py | 3 ++ enterprise_subsidy/apps/subsidy/models.py | 7 ++++ .../apps/subsidy/tests/test_models.py | 34 +++++++++++++++++++ 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/enterprise_subsidy/apps/api_client/enterprise.py b/enterprise_subsidy/apps/api_client/enterprise.py index 9b49dd2e..178932c6 100644 --- a/enterprise_subsidy/apps/api_client/enterprise.py +++ b/enterprise_subsidy/apps/api_client/enterprise.py @@ -10,6 +10,7 @@ from enterprise_subsidy.apps.api_client.base_oauth import BaseOAuthClient from enterprise_subsidy.apps.core.utils import localized_utcnow +from enterprise_subsidy.apps.subsidy.constants import ALLOW_LATE_ENROLLMENT_KEY logger = logging.getLogger(__name__) @@ -109,7 +110,7 @@ def enroll(self, lms_user_id, course_run_key, ledger_transaction): } # If late enrollment has been enabled for this transaction, inform the enterprise bulk enroll endpoint to bypass # any enrollment deadline validation. - if ledger_transaction.metadata and ledger_transaction.metadata.get('allow_late_enrollment', False): + if ledger_transaction.metadata and ledger_transaction.metadata.get(ALLOW_LATE_ENROLLMENT_KEY, False): enrollment_info['force_enrollment'] = True customer_uuid = ledger_transaction.ledger.subsidy.enterprise_customer_uuid response = self.bulk_enroll_enterprise_learners(customer_uuid, [enrollment_info]) diff --git a/enterprise_subsidy/apps/subsidy/constants.py b/enterprise_subsidy/apps/subsidy/constants.py index 2047593b..8150526b 100644 --- a/enterprise_subsidy/apps/subsidy/constants.py +++ b/enterprise_subsidy/apps/subsidy/constants.py @@ -5,6 +5,9 @@ # Numeric constants CENTS_PER_DOLLAR = 100 +# Transaction metadata keys +ALLOW_LATE_ENROLLMENT_KEY = 'allow_late_enrollment' + # System-wide roles defined by edx-enterprise, used across the entire openedx instance, and can be found in JWT tokens. SYSTEM_ENTERPRISE_LEARNER_ROLE = 'enterprise_learner' SYSTEM_ENTERPRISE_ADMIN_ROLE = 'enterprise_admin' diff --git a/enterprise_subsidy/apps/subsidy/models.py b/enterprise_subsidy/apps/subsidy/models.py index 5454bb1f..eeef71ba 100644 --- a/enterprise_subsidy/apps/subsidy/models.py +++ b/enterprise_subsidy/apps/subsidy/models.py @@ -34,6 +34,7 @@ from enterprise_subsidy.apps.core.utils import localized_utcnow from enterprise_subsidy.apps.fulfillment.api import GEAGFulfillmentHandler, is_geag_fulfillment from enterprise_subsidy.apps.fulfillment.exceptions import IncompleteContentMetadataException +from enterprise_subsidy.apps.subsidy.constants import ALLOW_LATE_ENROLLMENT_KEY MOCK_CATALOG_CLIENT = mock.MagicMock() MOCK_ENROLLMENT_CLIENT = mock.MagicMock() @@ -519,6 +520,12 @@ def redeem( # Fetch one or more metadata keys from catalog service, with overall metadata request locally cached. content_metadata_summary = self.metadata_summary_for_content(content_key) + is_late_enrollment = bool(metadata and metadata.get(ALLOW_LATE_ENROLLMENT_KEY)) + if is_late_enrollment and not content_metadata_summary.get('course_run_key'): + raise ContentNotFoundForCustomerException( + f'Late enrollment for {content_key} must resolve to a concrete course run. ' + 'Use the exact course run key for custom or one-off presentations.' + ) content_title = content_metadata_summary.get('content_title') parent_content_key = content_metadata_summary.get('content_key') course_run_start_date = content_metadata_summary.get('course_run_start_date') diff --git a/enterprise_subsidy/apps/subsidy/tests/test_models.py b/enterprise_subsidy/apps/subsidy/tests/test_models.py index 6f966e6d..413bf5ec 100644 --- a/enterprise_subsidy/apps/subsidy/tests/test_models.py +++ b/enterprise_subsidy/apps/subsidy/tests/test_models.py @@ -22,6 +22,7 @@ from enterprise_subsidy.apps.content_metadata.constants import ProductSources from enterprise_subsidy.apps.fulfillment.api import InvalidFulfillmentMetadataException from enterprise_subsidy.apps.fulfillment.exceptions import IncompleteContentMetadataException +from enterprise_subsidy.apps.subsidy.constants import ALLOW_LATE_ENROLLMENT_KEY from test_utils.utils import MockResponse from ..models import ContentNotFoundForCustomerException, PriceValidationError, Subsidy @@ -472,6 +473,39 @@ def test_redeem_with_metadata( assert new_transaction.quantity == -mock_content_price assert new_transaction.metadata == tx_metadata + @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content') + @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client') + @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary") + def test_redeem_late_enrollment_requires_resolved_course_run( + self, mock_get_content_summary, mock_enterprise_client, mock_price_for_content + ): + """ + Late enrollment should not attempt LMS enrollment when catalog metadata has no concrete run. + """ + lms_user_id = 1 + content_key = "edX+test" + subsidy_access_policy_uuid = str(uuid4()) + mock_get_content_summary.return_value = { + 'content_uuid': 'edX+test', + 'content_key': 'edX+test', + 'content_title': 'edx: Test Course', + 'source': 'edX', + 'mode': 'verified', + 'content_price': 10000, + 'geag_variant_id': None, + } + mock_price_for_content.return_value = 1000 + + with self.assertRaisesRegex(ContentNotFoundForCustomerException, 'exact course run key'): + self.subsidy.redeem( + lms_user_id, + content_key, + subsidy_access_policy_uuid, + metadata={ALLOW_LATE_ENROLLMENT_KEY: True}, + ) + + mock_enterprise_client.enroll.assert_not_called() + @mock.patch('enterprise_subsidy.apps.subsidy.models.is_geag_fulfillment', return_value=True) @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content') @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client') From b980ac53f84133b290c3432fff1c7f4ce1500c38 Mon Sep 17 00:00:00 2001 From: Cyril Nxumalo <80963114+zwidekalanga@users.noreply.github.com> Date: Mon, 1 Jun 2026 12:16:10 +0200 Subject: [PATCH 2/2] fix: require exact run for late enrollments --- enterprise_subsidy/apps/subsidy/models.py | 7 +- .../apps/subsidy/tests/test_models.py | 81 +++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/enterprise_subsidy/apps/subsidy/models.py b/enterprise_subsidy/apps/subsidy/models.py index eeef71ba..d79d9cbb 100644 --- a/enterprise_subsidy/apps/subsidy/models.py +++ b/enterprise_subsidy/apps/subsidy/models.py @@ -521,10 +521,11 @@ def redeem( # Fetch one or more metadata keys from catalog service, with overall metadata request locally cached. content_metadata_summary = self.metadata_summary_for_content(content_key) is_late_enrollment = bool(metadata and metadata.get(ALLOW_LATE_ENROLLMENT_KEY)) - if is_late_enrollment and not content_metadata_summary.get('course_run_key'): + resolved_course_run_key = content_metadata_summary.get('course_run_key') + if is_late_enrollment and content_key != resolved_course_run_key: raise ContentNotFoundForCustomerException( - f'Late enrollment for {content_key} must resolve to a concrete course run. ' - 'Use the exact course run key for custom or one-off presentations.' + f'Late enrollment for {content_key} must use the exact course run key. ' + f'Resolved course run: {resolved_course_run_key or "none"}.' ) content_title = content_metadata_summary.get('content_title') parent_content_key = content_metadata_summary.get('content_key') diff --git a/enterprise_subsidy/apps/subsidy/tests/test_models.py b/enterprise_subsidy/apps/subsidy/tests/test_models.py index 413bf5ec..cede581f 100644 --- a/enterprise_subsidy/apps/subsidy/tests/test_models.py +++ b/enterprise_subsidy/apps/subsidy/tests/test_models.py @@ -506,6 +506,87 @@ def test_redeem_late_enrollment_requires_resolved_course_run( mock_enterprise_client.enroll.assert_not_called() + @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content') + @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client') + @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary") + def test_redeem_late_enrollment_requires_exact_course_run( + self, mock_get_content_summary, mock_enterprise_client, mock_price_for_content + ): + """ + Late enrollment should reject a parent course key even when metadata advertises a concrete run. + """ + lms_user_id = 1 + content_key = "edX+test" + course_run_key = "course-v1:edX+test+2026" + subsidy_access_policy_uuid = str(uuid4()) + mock_get_content_summary.return_value = { + 'content_uuid': 'edX+test', + 'content_key': 'edX+test', + 'course_run_key': course_run_key, + 'content_title': 'edx: Test Course', + 'source': 'edX', + 'mode': 'verified', + 'content_price': 10000, + 'geag_variant_id': None, + } + mock_price_for_content.return_value = 1000 + + with self.assertRaisesRegex(ContentNotFoundForCustomerException, 'exact course run key'): + self.subsidy.redeem( + lms_user_id, + content_key, + subsidy_access_policy_uuid, + metadata={ALLOW_LATE_ENROLLMENT_KEY: True}, + ) + + mock_enterprise_client.enroll.assert_not_called() + + @mock.patch('enterprise_subsidy.apps.subsidy.models.is_geag_fulfillment', return_value=False) + @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content') + @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client') + @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary") + def test_redeem_late_enrollment_accepts_exact_course_run( + self, mock_get_content_summary, mock_enterprise_client, mock_price_for_content, + mock_is_geag_fulfillment, # pylint: disable=unused-argument + ): + """ + Late enrollment succeeds when the caller uses the concrete course run key. + """ + lms_user_id = 1 + parent_content_key = "edX+test" + course_run_key = "course-v1:edX+test+2026" + subsidy_access_policy_uuid = str(uuid4()) + mock_enterprise_fulfillment_uuid = str(uuid4()) + mock_content_price = 1000 + mock_get_content_summary.return_value = { + 'content_uuid': course_run_key, + 'content_key': parent_content_key, + 'course_run_key': course_run_key, + 'content_title': 'edx: Test Course', + 'source': 'edX', + 'mode': 'verified', + 'content_price': 10000, + 'geag_variant_id': None, + } + mock_price_for_content.return_value = mock_content_price + mock_enterprise_client.enroll.return_value = mock_enterprise_fulfillment_uuid + + new_transaction, transaction_created = self.subsidy.redeem( + lms_user_id, + course_run_key, + subsidy_access_policy_uuid, + metadata={ALLOW_LATE_ENROLLMENT_KEY: True}, + ) + + assert transaction_created + assert new_transaction.state == TransactionStateChoices.COMMITTED + assert new_transaction.quantity == -mock_content_price + mock_enterprise_client.enroll.assert_called_once_with( + lms_user_id, + course_run_key, + new_transaction, + ) + @mock.patch('enterprise_subsidy.apps.subsidy.models.is_geag_fulfillment', return_value=True) @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content') @mock.patch('enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client')