From 689a620c00ab481f6bb8af4a9770aec96bd1da2e Mon Sep 17 00:00:00 2001 From: Alexander Dusenbery Date: Wed, 14 Aug 2024 11:34:50 -0400 Subject: [PATCH] feat: can_redeem now checks for enrollment deadline We want the `can_redeem` query to return false for a given (user, content key) if the enrollment deadline for the content key is in the past. This is necessary because some courses have runs with an advertised run for which the deadline is in the future, but also have available recent runs where the upgrade deadline is in the past. ENT-7472 --- .../tests/test_subsidy_access_policy_views.py | 52 +++++++++++++++ .../api/v1/views/subsidy_access_policy.py | 4 +- .../apps/subsidy_access_policy/constants.py | 3 + .../content_metadata_api.py | 11 ++++ .../apps/subsidy_access_policy/models.py | 46 +++++++++++--- .../tests/test_models.py | 63 +++++++++++++++++++ 6 files changed, 171 insertions(+), 8 deletions(-) diff --git a/enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py b/enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py index 5496ab52..ece3dec9 100755 --- a/enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py +++ b/enterprise_access/apps/api/v1/tests/test_subsidy_access_policy_views.py @@ -30,6 +30,7 @@ SYSTEM_ENTERPRISE_OPERATOR_ROLE ) from enterprise_access.apps.subsidy_access_policy.constants import ( + REASON_BEYOND_ENROLLMENT_DEADLINE, REASON_LEARNER_ASSIGNMENT_CANCELLED, REASON_LEARNER_ASSIGNMENT_FAILED, REASON_LEARNER_NOT_ASSIGNED_CONTENT, @@ -2202,6 +2203,57 @@ def test_can_redeem_policy_no_price(self, mock_lms_client, mock_transactions_cac 'detail': f'Could not determine price for content_key: {test_content_key}', } + @mock.patch('enterprise_access.apps.subsidy_access_policy.subsidy_api.get_and_cache_transactions_for_learner') + @mock.patch('enterprise_access.apps.api.v1.views.subsidy_access_policy.LmsApiClient') + def test_can_redeem_policy_beyond_enrollment_deadline(self, mock_lms_client, mock_transactions_cache_for_learner): + """ + Test that the can_redeem endpoint successfully serializes a response for content whose + enrollment deadline has passed. + """ + test_content_key = "course-v1:demox+1234+2T2023" + mock_lms_client.return_value.get_enterprise_customer_data.return_value = { + 'slug': 'sluggy', + 'admin_users': [{'email': 'edx@example.org'}], + } + + self.mock_get_content_metadata.return_value = { + "content_price": 1234, + "enroll_by_date": '2001-01-01T00:00:00Z', + } + + mock_transactions_cache_for_learner.return_value = { + 'transactions': [], + 'aggregates': { + 'total_quantity': 0, + }, + } + + mocked_content_data_from_view = { + "content_uuid": str(uuid4()), + "content_key": test_content_key, + "source": "edX", + "content_price": 1234, + "enroll_by_date": '2001-01-01T00:00:00Z', + } + + with mock.patch( + 'enterprise_access.apps.subsidy_access_policy.content_metadata_api.get_and_cache_content_metadata', + return_value=mocked_content_data_from_view, + ): + query_params = {'content_key': test_content_key} + response = self.client.get(self.subsidy_access_policy_can_redeem_endpoint, query_params) + + assert response.status_code == status.HTTP_200_OK + response_list = response.json() + assert response_list[0]["reasons"] == [ + { + "reason": REASON_BEYOND_ENROLLMENT_DEADLINE, + "user_message": MissingSubsidyAccessReasonUserMessages.BEYOND_ENROLLMENT_DEADLINE, + "metadata": mock.ANY, + "policy_uuids": [str(self.redeemable_policy.uuid)], + }, + ] + @mock.patch('enterprise_access.apps.subsidy_access_policy.subsidy_api.get_versioned_subsidy_client') def test_can_redeem_subsidy_client_http_error(self, mock_get_client): """ diff --git a/enterprise_access/apps/api/v1/views/subsidy_access_policy.py b/enterprise_access/apps/api/v1/views/subsidy_access_policy.py index 5c901dfa..aace29f8 100755 --- a/enterprise_access/apps/api/v1/views/subsidy_access_policy.py +++ b/enterprise_access/apps/api/v1/views/subsidy_access_policy.py @@ -37,6 +37,7 @@ from enterprise_access.apps.events.utils import send_subsidy_redemption_event_to_event_bus from enterprise_access.apps.subsidy_access_policy.constants import ( GROUP_MEMBERS_WITH_AGGREGATES_DEFAULT_PAGE_SIZE, + REASON_BEYOND_ENROLLMENT_DEADLINE, REASON_CONTENT_NOT_IN_CATALOG, REASON_LEARNER_ASSIGNMENT_CANCELLED, REASON_LEARNER_ASSIGNMENT_FAILED, @@ -224,6 +225,7 @@ def _get_user_message_for_reason(reason_slug, enterprise_admin_users): REASON_LEARNER_MAX_SPEND_REACHED: MissingSubsidyAccessReasonUserMessages.LEARNER_LIMITS_REACHED, REASON_LEARNER_MAX_ENROLLMENTS_REACHED: MissingSubsidyAccessReasonUserMessages.LEARNER_LIMITS_REACHED, REASON_CONTENT_NOT_IN_CATALOG: MissingSubsidyAccessReasonUserMessages.CONTENT_NOT_IN_CATALOG, + REASON_BEYOND_ENROLLMENT_DEADLINE: MissingSubsidyAccessReasonUserMessages.BEYOND_ENROLLMENT_DEADLINE, REASON_LEARNER_NOT_ASSIGNED_CONTENT: MissingSubsidyAccessReasonUserMessages.LEARNER_NOT_ASSIGNED_CONTENT, REASON_LEARNER_ASSIGNMENT_CANCELLED: MissingSubsidyAccessReasonUserMessages.LEARNER_ASSIGNMENT_CANCELED, REASON_LEARNER_ASSIGNMENT_FAILED: MissingSubsidyAccessReasonUserMessages.LEARNER_NOT_ASSIGNED_CONTENT, @@ -768,7 +770,7 @@ def can_redeem(self, request, enterprise_customer_uuid): enterprise_customer_uuid, lms_user_id, content_key ) - if not redemptions and not redeemable_policies: + if not successful_redemptions and not redeemable_policies: reasons.extend(_get_reasons_for_no_redeemable_policies( enterprise_customer_uuid, non_redeemable_policies diff --git a/enterprise_access/apps/subsidy_access_policy/constants.py b/enterprise_access/apps/subsidy_access_policy/constants.py index b4819891..f92c6adb 100644 --- a/enterprise_access/apps/subsidy_access_policy/constants.py +++ b/enterprise_access/apps/subsidy_access_policy/constants.py @@ -93,6 +93,8 @@ class MissingSubsidyAccessReasonUserMessages: LEARNER_LIMITS_REACHED = "You can't enroll right now because of limits set by your organization." CONTENT_NOT_IN_CATALOG = \ "You can't enroll right now because this course is no longer available in your organization's catalog." + BEYOND_ENROLLMENT_DEADLINE = \ + "You can't enroll right now because the enrollment deadline for this course has passed." LEARNER_NOT_IN_ENTERPRISE = \ "You can't enroll right now because your account is no longer associated with the organization." LEARNER_NOT_ASSIGNED_CONTENT = \ @@ -104,6 +106,7 @@ class MissingSubsidyAccessReasonUserMessages: REASON_POLICY_EXPIRED = "policy_expired" REASON_SUBSIDY_EXPIRED = "subsidy_expired" REASON_CONTENT_NOT_IN_CATALOG = "content_not_in_catalog" +REASON_BEYOND_ENROLLMENT_DEADLINE = "beyond_enrollment_deadline" REASON_LEARNER_NOT_IN_ENTERPRISE = "learner_not_in_enterprise" REASON_LEARNER_NOT_IN_ENTERPRISE_GROUP = "learner_not_in_enterprise_group" REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY = "not_enough_value_in_subsidy" diff --git a/enterprise_access/apps/subsidy_access_policy/content_metadata_api.py b/enterprise_access/apps/subsidy_access_policy/content_metadata_api.py index f2b24f5b..c3288ad9 100644 --- a/enterprise_access/apps/subsidy_access_policy/content_metadata_api.py +++ b/enterprise_access/apps/subsidy_access_policy/content_metadata_api.py @@ -7,6 +7,7 @@ import requests from django.conf import settings +from django.utils.dateparse import parse_datetime from edx_django_utils.cache import TieredCache from requests.exceptions import HTTPError @@ -125,3 +126,13 @@ def list_price_dict_from_usd_cents(list_price_integer_cents): "usd": list_price_decimal_dollars, "usd_cents": list_price_integer_cents, } + + +def enroll_by_datetime(content_metadata): + """ + Helper to return a datetime object representing + the enrollment deadline for a content_metdata record. + """ + if enroll_by_date := content_metadata.get('enroll_by_date'): + return parse_datetime(enroll_by_date) + return None diff --git a/enterprise_access/apps/subsidy_access_policy/models.py b/enterprise_access/apps/subsidy_access_policy/models.py index ce97a234..46ff611b 100644 --- a/enterprise_access/apps/subsidy_access_policy/models.py +++ b/enterprise_access/apps/subsidy_access_policy/models.py @@ -27,6 +27,7 @@ from .constants import ( CREDIT_POLICY_TYPE_PRIORITY, FORCE_ENROLLMENT_KEYWORD, + REASON_BEYOND_ENROLLMENT_DEADLINE, REASON_CONTENT_NOT_IN_CATALOG, REASON_LEARNER_ASSIGNMENT_CANCELLED, REASON_LEARNER_ASSIGNMENT_EXPIRED, @@ -46,6 +47,7 @@ TransactionStateChoices ) from .content_metadata_api import ( + enroll_by_datetime, get_and_cache_catalog_contains_content, get_and_cache_content_metadata, get_list_price_for_content, @@ -686,7 +688,11 @@ def _log_redeemability(self, is_redeemable, reason, lms_user_id, content_key, ex ) logger.info(message, self.uuid, is_redeemable, reason, lms_user_id, content_key, extra) - def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): + def can_redeem( + self, lms_user_id, content_key, + skip_customer_user_check=False, skip_enrollment_deadline_check=False, + **kwargs, + ): """ Check that a given learner can redeem the given content. The ordering of each conditional is intentional based on an expected @@ -732,7 +738,13 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): self._log_redeemability(False, REASON_CONTENT_NOT_IN_CATALOG, lms_user_id, content_key) return (False, REASON_CONTENT_NOT_IN_CATALOG, []) - # TODO: Add Course Upgrade/Registration Deadline Passed Error here + # Check if the current time is beyond the enrollment deadline for the content, + # but only if late redemption is *not* currently allowed. + if not skip_enrollment_deadline_check and not self.is_late_redemption_allowed: + enrollment_deadline = enroll_by_datetime(content_metadata) + if enrollment_deadline and (timezone.now() > enrollment_deadline): + self._log_redeemability(False, REASON_BEYOND_ENROLLMENT_DEADLINE, lms_user_id, content_key) + return (False, REASON_BEYOND_ENROLLMENT_DEADLINE, []) # We want to wait to do these checks that might require a call # to the enterprise-subsidy service until we *know* we'll need the data. @@ -1115,14 +1127,22 @@ class Meta: """ proxy = True - def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): + def can_redeem( + self, lms_user_id, content_key, + skip_customer_user_check=False, skip_enrollment_deadline_check=False, + **kwargs, + ): """ Checks if the given lms_user_id has a number of existing subsidy transactions LTE to the learner enrollment cap declared by this policy. """ # perform generic access checks should_attempt_redemption, reason, existing_redemptions = \ - super().can_redeem(lms_user_id, content_key, skip_customer_user_check) + super().can_redeem( + lms_user_id, content_key, + skip_customer_user_check, skip_enrollment_deadline_check, + **kwargs, + ) if not should_attempt_redemption: return (False, reason, existing_redemptions) @@ -1185,7 +1205,11 @@ class Meta: """ proxy = True - def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): + def can_redeem( + self, lms_user_id, content_key, + skip_customer_user_check=False, skip_enrollment_deadline_check=False, + **kwargs, + ): """ Determines whether learner can redeem a subsidy access policy given the limits specified on the policy. @@ -1195,6 +1219,8 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): lms_user_id, content_key, skip_customer_user_check, + skip_enrollment_deadline_check, + **kwargs, ) if not should_attempt_redemption: self._log_redeemability(False, reason, lms_user_id, content_key, extra=existing_redemptions) @@ -1356,7 +1382,11 @@ def get_list_price(self, lms_user_id, content_key): # that value has been consumed from a subsidy (though not necessarily fulfilled) return list_price_dict_from_usd_cents(found_assignment.content_quantity * -1) - def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): + def can_redeem( + self, lms_user_id, content_key, + skip_customer_user_check=False, skip_enrollment_deadline_check=False, + **kwargs, + ): """ Checks if the given lms_user_id has an existing assignment on the given content_key, ready to be accepted. """ @@ -1365,6 +1395,8 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): lms_user_id, content_key, skip_customer_user_check, + skip_enrollment_deadline_check, + **kwargs, ) if not should_attempt_redemption: self._log_redeemability(False, reason, lms_user_id, content_key, extra=existing_redemptions) @@ -1731,7 +1763,7 @@ def force_redeem(self, extra_metadata=None): try: with self.subsidy_access_policy.lock(): can_redeem, reason, existing_transactions = self.subsidy_access_policy.can_redeem( - self.lms_user_id, self.course_run_key, + self.lms_user_id, self.course_run_key, skip_enrollment_deadline_check=True, ) extra_metadata = extra_metadata or {} if can_redeem: diff --git a/enterprise_access/apps/subsidy_access_policy/tests/test_models.py b/enterprise_access/apps/subsidy_access_policy/tests/test_models.py index b0f08847..eef43c77 100644 --- a/enterprise_access/apps/subsidy_access_policy/tests/test_models.py +++ b/enterprise_access/apps/subsidy_access_policy/tests/test_models.py @@ -21,6 +21,7 @@ from enterprise_access.apps.content_assignments.models import AssignmentConfiguration from enterprise_access.apps.content_assignments.tests.factories import LearnerContentAssignmentFactory from enterprise_access.apps.subsidy_access_policy.constants import ( + REASON_BEYOND_ENROLLMENT_DEADLINE, REASON_CONTENT_NOT_IN_CATALOG, REASON_LEARNER_ASSIGNMENT_CANCELLED, REASON_LEARNER_ASSIGNMENT_EXPIRED, @@ -318,6 +319,7 @@ def test_learner_enrollment_cap_policy_can_redeem( expected_policy_can_redeem, expect_content_metadata_fetch=True, expect_transaction_fetch=True, + enroll_by_date='2099-01-01T00:00:00Z', ): """ Test the can_redeem method of PerLearnerEnrollmentCapLearnerCreditAccessPolicy model @@ -327,6 +329,7 @@ def test_learner_enrollment_cap_policy_can_redeem( self.mock_catalog_contains_content_key.return_value = catalog_contains_content self.mock_get_content_metadata.return_value = { 'content_price': 200, + 'enroll_by_date': enroll_by_date, } self.mock_subsidy_client.can_redeem.return_value = subsidy_is_redeemable self.mock_transactions_cache_for_learner.return_value = transactions_for_learner @@ -369,6 +372,7 @@ def test_learner_enrollment_cap_policy_can_redeem( # Expected can_redeem result: True 'policy_is_active': True, 'catalog_contains_content': True, + 'enroll_by_date': '2099-01-01T00:00:00Z', 'get_enterprise_user': TEST_USER_RECORD, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, 'transactions_for_learner': {'transactions': [], 'aggregates': {'total_quantity': -100}}, @@ -380,6 +384,7 @@ def test_learner_enrollment_cap_policy_can_redeem( # Expected can_redeem result: False 'policy_is_active': True, 'catalog_contains_content': False, + 'enroll_by_date': '2099-01-01T00:00:00Z', 'get_enterprise_user': TEST_USER_RECORD, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, 'transactions_for_learner': {'transactions': [], 'aggregates': {}}, @@ -388,11 +393,56 @@ def test_learner_enrollment_cap_policy_can_redeem( 'expect_content_metadata_fetch': False, 'expect_transaction_fetch': False, }, + { + # Content enrollment deadline is missing, every other check would succeed. + # Expected can_redeem result: True + 'policy_is_active': True, + 'catalog_contains_content': True, + 'enroll_by_date': None, + 'get_enterprise_user': TEST_USER_RECORD, + 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, + 'transactions_for_learner': {'transactions': [], 'aggregates': {}}, + 'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}}, + 'expected_policy_can_redeem': (True, None, []), + 'expect_content_metadata_fetch': True, + 'expect_transaction_fetch': True, + }, + { + # Content enrollment deadline has passed, every other check would succeed. + # Expected can_redeem result: False + 'policy_is_active': True, + 'catalog_contains_content': True, + 'enroll_by_date': '2020-01-01T00:00:00Z', + 'get_enterprise_user': TEST_USER_RECORD, + 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, + 'transactions_for_learner': {'transactions': [], 'aggregates': {}}, + 'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}}, + 'expected_policy_can_redeem': (False, REASON_BEYOND_ENROLLMENT_DEADLINE, []), + 'expect_content_metadata_fetch': True, + 'expect_transaction_fetch': False, + }, + { + # Content enrollment deadline has passed, every other check would succeed, + # but late redemption is enabled. + # Expected can_redeem result: True + 'policy_is_active': True, + 'catalog_contains_content': True, + 'enroll_by_date': '2020-01-01T00:00:00Z', + 'get_enterprise_user': TEST_USER_RECORD, + 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, + 'transactions_for_learner': {'transactions': [], 'aggregates': {}}, + 'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}}, + 'expected_policy_can_redeem': (True, None, []), + 'expect_content_metadata_fetch': True, + 'expect_transaction_fetch': True, + 'late_redemption_allowed_until': localized_utcnow() + timedelta(days=1), + }, { # Learner is not in the enterprise, every other check would succeed. # Expected can_redeem result: False 'policy_is_active': True, 'catalog_contains_content': True, + 'enroll_by_date': '2099-01-01T00:00:00Z', 'get_enterprise_user': None, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, 'transactions_for_learner': {'transactions': [], 'aggregates': {}}, @@ -406,6 +456,7 @@ def test_learner_enrollment_cap_policy_can_redeem( # Expected can_redeem result: False 'policy_is_active': True, 'catalog_contains_content': True, + 'enroll_by_date': '2099-01-01T00:00:00Z', 'get_enterprise_user': TEST_USER_RECORD_NO_GROUPS, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, 'transactions_for_learner': {'transactions': [], 'aggregates': {}}, @@ -419,6 +470,7 @@ def test_learner_enrollment_cap_policy_can_redeem( # Expected can_redeem result: False 'policy_is_active': True, 'catalog_contains_content': True, + 'enroll_by_date': '2099-01-01T00:00:00Z', 'get_enterprise_user': TEST_USER_RECORD, 'subsidy_is_redeemable': {'can_redeem': False, 'active': True}, 'transactions_for_learner': {'transactions': [], 'aggregates': {}}, @@ -431,6 +483,7 @@ def test_learner_enrollment_cap_policy_can_redeem( # Expected can_redeem result: False 'policy_is_active': True, 'catalog_contains_content': True, + 'enroll_by_date': '2099-01-01T00:00:00Z', 'get_enterprise_user': TEST_USER_RECORD, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, 'transactions_for_learner': { @@ -451,6 +504,7 @@ def test_learner_enrollment_cap_policy_can_redeem( # Expected can_redeem result: False 'policy_is_active': True, 'catalog_contains_content': True, + 'enroll_by_date': '2099-01-01T00:00:00Z', 'get_enterprise_user': TEST_USER_RECORD, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, 'transactions_for_learner': { @@ -470,6 +524,7 @@ def test_learner_enrollment_cap_policy_can_redeem( # Expected can_redeem result: False 'policy_is_active': False, 'catalog_contains_content': True, + 'enroll_by_date': '2099-01-01T00:00:00Z', 'get_enterprise_user': TEST_USER_RECORD, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, 'transactions_for_learner': {'transactions': [], 'aggregates': {}}, @@ -483,6 +538,7 @@ def test_learner_enrollment_cap_policy_can_redeem( # Expected can_redeem result: False 'policy_is_active': True, 'catalog_contains_content': True, + 'enroll_by_date': '2099-01-01T00:00:00Z', 'get_enterprise_user': TEST_USER_RECORD, 'subsidy_is_redeemable': {'can_redeem': True, 'active': False}, 'transactions_for_learner': {'transactions': [], 'aggregates': {}}, @@ -495,6 +551,7 @@ def test_learner_spend_cap_policy_can_redeem( self, policy_is_active, catalog_contains_content, + enroll_by_date, get_enterprise_user, subsidy_is_redeemable, transactions_for_learner, @@ -502,6 +559,7 @@ def test_learner_spend_cap_policy_can_redeem( expected_policy_can_redeem, expect_content_metadata_fetch=True, expect_transaction_fetch=True, + late_redemption_allowed_until=None, ): """ Test the can_redeem method of PerLearnerSpendCapLearnerCreditAccessPolicy model @@ -511,6 +569,7 @@ def test_learner_spend_cap_policy_can_redeem( self.mock_catalog_contains_content_key.return_value = catalog_contains_content self.mock_get_content_metadata.return_value = { 'content_price': 200, + 'enroll_by_date': enroll_by_date, } self.mock_subsidy_client.can_redeem.return_value = subsidy_is_redeemable self.mock_transactions_cache_for_learner.return_value = transactions_for_learner @@ -520,6 +579,9 @@ def test_learner_spend_cap_policy_can_redeem( if policy_is_active: policy_record = self.per_learner_spend_policy + if late_redemption_allowed_until: + policy_record.late_redemption_allowed_until = late_redemption_allowed_until + PolicyGroupAssociationFactory( enterprise_group_uuid=TEST_ENTERPRISE_GROUP_UUID, subsidy_access_policy=policy_record @@ -1066,6 +1128,7 @@ def test_can_redeem( self.mock_catalog_contains_content_key.return_value = True self.mock_get_content_metadata.return_value = { 'content_price': 200, + 'enroll_by_date': '2099-01-01T00:00:00Z', } self.mock_assign_get_content_metadata.return_value = { 'content_price': 200,