feat: Adds additional error reasons to SubsidyAccessPolicy

openedx · Jul 25, 2023 · 41ebf59 · 41ebf59
1 parent cfde7ed
commit 41ebf59
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 35 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
@@ -531,6 +531,7 @@ def setup_mocks(self):
         subsidy_client = subsidy_client_patcher.start()
         subsidy_client.can_redeem.return_value = {
             'can_redeem': True,
+            'active': True,
             'content_price': 0,
             'unit': 'usd_cents',
             'all_transactions': [],
@@ -709,6 +710,7 @@ def test_redeem_policy_redemption_idempotency_key_versions(
             existing_transactions.append(existing_transaction)
         self.redeemable_policy.subsidy_client.can_redeem.return_value = {
             'can_redeem': True,
+            'active': True,
             'content_price': 5000,
             'unit': 'usd_cents',
             'all_transactions': existing_transactions,
@@ -820,6 +822,7 @@ def setup_mocks(self):
         subsidy_client = subsidy_client_patcher.start()
         subsidy_client.can_redeem.return_value = {
             'can_redeem': True,
+            'active': True,
             'content_price': 5000,
             'unit': 'usd_cents',
             'all_transactions': [],
@@ -960,7 +963,7 @@ def test_can_redeem_policy_none_redeemable(
         self, mock_lms_client, mock_transactions_cache_for_learner, has_admin_users
     ):
         """
-        Test that the can_redeem endpoint returns resons for why each non-redeemable policy failed.
+        Test that the can_redeem endpoint returns reasons for why each non-redeemable policy failed.
         """
         slug = 'sluggy'
         admin_email = 'edx@example.org'
@@ -977,6 +980,7 @@ def test_can_redeem_policy_none_redeemable(
         }
         self.redeemable_policy.subsidy_client.can_redeem.return_value = {
             'can_redeem': False,
+            'active': True,
             'content_price': 5000,  # value is ignored.
             'unit': 'usd_cents',
             'all_transactions': [],
@@ -1093,6 +1097,7 @@ def test_can_redeem_policy_existing_redemptions(self, mock_transactions_cache_fo
 
         self.redeemable_policy.subsidy_client.can_redeem.return_value = {
             'can_redeem': False,
+            'active': True,
         }
         self.mock_get_content_metadata.return_value = {'content_price': 19900}
 
@@ -1166,6 +1171,7 @@ def test_can_redeem_policy_existing_reversed_redemptions(self, mock_transactions
 
         self.redeemable_policy.subsidy_client.can_redeem.return_value = {
             'can_redeem': True,
+            'active': True,
         }
         self.mock_get_content_metadata.return_value = {'content_price': 19900}
 

diff --git a/enterprise_access/apps/api/v1/views/subsidy_access_policy.py b/enterprise_access/apps/api/v1/views/subsidy_access_policy.py
@@ -44,8 +44,9 @@
     REASON_LEARNER_MAX_SPEND_REACHED,
     REASON_LEARNER_NOT_IN_ENTERPRISE,
     REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY,
-    REASON_POLICY_NOT_ACTIVE,
+    REASON_POLICY_EXPIRED,
     REASON_POLICY_SPEND_LIMIT_REACHED,
+    REASON_SUBSIDY_EXPIRED,
     MissingSubsidyAccessReasonUserMessages,
     TransactionStateChoices
 )
@@ -503,8 +504,21 @@ def _get_user_message_for_reason(self, reason_slug, enterprise_admin_users):
             else MissingSubsidyAccessReasonUserMessages.ORGANIZATION_NO_FUNDS_NO_ADMINS
         )
 
+        user_message_organization_fund_expired = (
+            MissingSubsidyAccessReasonUserMessages.ORGANIZATION_EXPIRED_FUNDS
+            if has_enterprise_admin_users
+            else MissingSubsidyAccessReasonUserMessages.ORGANIZATION_EXPIRED_FUNDS_NO_ADMINS
+        )
+
+        user_message_organization_plan_expired = (
+            MissingSubsidyAccessReasonUserMessages.ORGANIZATION_EXPIRED_PLAN
+            if has_enterprise_admin_users
+            else MissingSubsidyAccessReasonUserMessages.ORGANIZATION_EXPIRED_PLAN_NO_ADMINS
+        )
+
         MISSING_SUBSIDY_ACCESS_POLICY_REASONS = {
-            REASON_POLICY_NOT_ACTIVE: user_message_organization_no_funds,
+            REASON_POLICY_EXPIRED: user_message_organization_fund_expired,
+            REASON_SUBSIDY_EXPIRED: user_message_organization_plan_expired,
             REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY: user_message_organization_no_funds,
             REASON_POLICY_SPEND_LIMIT_REACHED: user_message_organization_no_funds,
             REASON_LEARNER_NOT_IN_ENTERPRISE: MissingSubsidyAccessReasonUserMessages.LEARNER_NOT_IN_ENTERPRISE,

diff --git a/enterprise_access/apps/subsidy_access_policy/constants.py b/enterprise_access/apps/subsidy_access_policy/constants.py
@@ -85,14 +85,21 @@ class MissingSubsidyAccessReasonUserMessages:
     ORGANIZATION_NO_FUNDS_NO_ADMINS = \
         "You can't enroll right now because your organization doesn't have enough funds. " \
         "Contact your administrator to request more."
+    ORGANIZATION_EXPIRED_FUNDS = "You can't enroll right now because your funds expired."
+    ORGANIZATION_EXPIRED_FUNDS_NO_ADMINS = "You can't enroll right now because your funds expired. " \
+                                           "Contact your administrator for help."
+    ORGANIZATION_EXPIRED_PLAN = "You can't enroll right now because your plan expired."
+    ORGANIZATION_EXPIRED_PLAN_NO_ADMINS = "You can't enroll right now because your plan expired. " \
+                                          "Contact your administrator for help."
     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."
     LEARNER_NOT_IN_ENTERPRISE = \
         "You can't enroll right now because your account is no longer associated with the organization."
 
 
-REASON_POLICY_NOT_ACTIVE = "policy_not_active"
+REASON_POLICY_EXPIRED = "policy_expired"
+REASON_SUBSIDY_EXPIRED = "subsidy_expired"
 REASON_CONTENT_NOT_IN_CATALOG = "content_not_in_catalog"
 REASON_LEARNER_NOT_IN_ENTERPRISE = "learner_not_in_enterprise"
 REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY = "not_enough_value_in_subsidy"

diff --git a/enterprise_access/apps/subsidy_access_policy/models.py b/enterprise_access/apps/subsidy_access_policy/models.py
@@ -21,8 +21,9 @@
     REASON_LEARNER_MAX_SPEND_REACHED,
     REASON_LEARNER_NOT_IN_ENTERPRISE,
     REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY,
-    REASON_POLICY_NOT_ACTIVE,
+    REASON_POLICY_EXPIRED,
     REASON_POLICY_SPEND_LIMIT_REACHED,
+    REASON_SUBSIDY_EXPIRED,
     AccessMethods,
     TransactionStateChoices
 )
@@ -311,6 +312,7 @@ def will_exceed_spend_limit(self, content_key, content_metadata=None):
 
         content_price = self.get_content_price(content_key, content_metadata=content_metadata)
         spent_amount = self.aggregates_for_policy().get('total_quantity') or 0
+
         return self.content_would_exceed_limit(spent_amount, self.spend_limit, content_price)
 
     def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False):
@@ -321,32 +323,44 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False):
             3-tuple of (bool, str, list of dict):
                 * first element is true if the learner can redeem the content,
                 * second element contains a reason code if the content is not redeemable,
-                * third a list of any transactions represending existing redemptions (any state).
+                * third a list of any transactions representing existing redemptions (any state).
         """
+        content_metadata = self.get_content_metadata(content_key)
+        subsidy_can_redeem_payload = self.subsidy_client.can_redeem(
+            self.subsidy_uuid,
+            lms_user_id,
+            content_key,
+        )
+
+        active_subsidy = subsidy_can_redeem_payload.get('active', False)
+        existing_transactions = subsidy_can_redeem_payload.get('all_transactions', [])
+
+        # inactive subsidy
+        if not active_subsidy:
+            return (False, REASON_SUBSIDY_EXPIRED, [])
+
+        # inactive policy
         if not self.active:
-            return (False, REASON_POLICY_NOT_ACTIVE, [])
+            return (False, REASON_POLICY_EXPIRED, [])
 
+        # learner not associated to enterprise
         if not skip_customer_user_check:
             if not self.lms_api_client.enterprise_contains_learner(self.enterprise_customer_uuid, lms_user_id):
                 return (False, REASON_LEARNER_NOT_IN_ENTERPRISE, [])
 
+        # no content key in catalog
         if not self.catalog_contains_content_key(content_key):
             return (False, REASON_CONTENT_NOT_IN_CATALOG, [])
 
-        subsidy_can_redeem_payload = self.subsidy_client.can_redeem(
-            self.subsidy_uuid,
-            lms_user_id,
-            content_key,
-        )
-        existing_transactions = subsidy_can_redeem_payload.get('all_transactions', [])
+        # no content key in content metadata
+        if not content_metadata:
+            return (False, REASON_CONTENT_NOT_IN_CATALOG, existing_transactions)
 
+        # can_redeem false from subsidy
         if not subsidy_can_redeem_payload.get('can_redeem', False):
             return (False, REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY, existing_transactions)
 
-        content_metadata = self.get_content_metadata(content_key)
-        if not content_metadata:
-            return (False, REASON_CONTENT_NOT_IN_CATALOG, existing_transactions)
-
+        # not enough funds on policy
         if self.will_exceed_spend_limit(content_key, content_metadata=content_metadata):
             return (False, REASON_POLICY_SPEND_LIMIT_REACHED, existing_transactions)
 

diff --git a/enterprise_access/apps/subsidy_access_policy/tests/test_models.py b/enterprise_access/apps/subsidy_access_policy/tests/test_models.py
@@ -17,8 +17,9 @@
     REASON_LEARNER_MAX_SPEND_REACHED,
     REASON_LEARNER_NOT_IN_ENTERPRISE,
     REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY,
-    REASON_POLICY_NOT_ACTIVE,
-    REASON_POLICY_SPEND_LIMIT_REACHED
+    REASON_POLICY_EXPIRED,
+    REASON_POLICY_SPEND_LIMIT_REACHED,
+    REASON_SUBSIDY_EXPIRED
 )
 from enterprise_access.apps.subsidy_access_policy.models import (
     PerLearnerEnrollmentCreditAccessPolicy,
@@ -159,7 +160,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args):
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            'subsidy_is_redeemable': {'can_redeem': True, 'active': True},
             'transactions_for_learner': {'transactions': [], 'aggregates': {'total_quantity': -100}},
             'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}},
             'expected_policy_can_redeem': (True, None, []),
@@ -170,7 +171,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args):
             'policy_is_active': True,
             'catalog_contains_content': False,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            '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_CONTENT_NOT_IN_CATALOG, []),
@@ -181,7 +182,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args):
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': False,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            '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_LEARNER_NOT_IN_ENTERPRISE, []),
@@ -192,7 +193,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args):
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': False},
+            'subsidy_is_redeemable': {'can_redeem': False, 'active': True},
             'transactions_for_learner': {'transactions': [], 'aggregates': {}},
             'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}},
             'expected_policy_can_redeem': (False, REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY, []),
@@ -204,7 +205,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args):
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            'subsidy_is_redeemable': {'can_redeem': True, 'active': True},
             'transactions_for_learner': {
                 'transactions': [{
                     'subsidy_access_policy_uuid': str(ACTIVE_LEARNER_ENROLL_CAP_POLICY_UUID),
@@ -224,7 +225,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args):
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            'subsidy_is_redeemable': {'can_redeem': True, 'active': True},
             'transactions_for_learner': {
                 'transactions': [{
                     'subsidy_access_policy_uuid': str(ACTIVE_LEARNER_ENROLL_CAP_POLICY_UUID),
@@ -243,10 +244,21 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args):
             'policy_is_active': False,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            '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_POLICY_NOT_ACTIVE, []),
+            'expected_policy_can_redeem': (False, REASON_POLICY_EXPIRED, []),
+        },
+        {
+            # The subsidy is not active, every other check would succeed.
+            # Expected can_redeem result: False
+            'policy_is_active': True,
+            'catalog_contains_content': True,
+            'enterprise_contains_learner': True,
+            'subsidy_is_redeemable': {'can_redeem': True, 'active': False},
+            'transactions_for_learner': {'transactions': [], 'aggregates': {}},
+            'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}},
+            'expected_policy_can_redeem': (False, REASON_SUBSIDY_EXPIRED, []),
         },
     )
     @ddt.unpack
@@ -288,7 +300,7 @@ def test_learner_enrollment_cap_policy_can_redeem(
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            'subsidy_is_redeemable': {'can_redeem': True, 'active': True},
             'transactions_for_learner': {'transactions': [], 'aggregates': {'total_quantity': -100}},
             'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}},
             'expected_policy_can_redeem': (True, None, []),
@@ -299,7 +311,7 @@ def test_learner_enrollment_cap_policy_can_redeem(
             'policy_is_active': True,
             'catalog_contains_content': False,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            '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_CONTENT_NOT_IN_CATALOG, []),
@@ -310,7 +322,7 @@ def test_learner_enrollment_cap_policy_can_redeem(
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': False,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            '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_LEARNER_NOT_IN_ENTERPRISE, []),
@@ -321,7 +333,7 @@ def test_learner_enrollment_cap_policy_can_redeem(
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': False},
+            'subsidy_is_redeemable': {'can_redeem': False, 'active': True},
             'transactions_for_learner': {'transactions': [], 'aggregates': {}},
             'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}},
             'expected_policy_can_redeem': (False, REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY, []),
@@ -333,7 +345,7 @@ def test_learner_enrollment_cap_policy_can_redeem(
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            'subsidy_is_redeemable': {'can_redeem': True, 'active': True},
             'transactions_for_learner': {
                 'transactions': [{
                     'subsidy_access_policy_uuid': str(ACTIVE_LEARNER_SPEND_CAP_POLICY_UUID),
@@ -353,7 +365,7 @@ def test_learner_enrollment_cap_policy_can_redeem(
             'policy_is_active': True,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            'subsidy_is_redeemable': {'can_redeem': True, 'active': True},
             'transactions_for_learner': {
                 'transactions': [{
                     'subsidy_access_policy_uuid': str(ACTIVE_LEARNER_SPEND_CAP_POLICY_UUID),
@@ -372,10 +384,21 @@ def test_learner_enrollment_cap_policy_can_redeem(
             'policy_is_active': False,
             'catalog_contains_content': True,
             'enterprise_contains_learner': True,
-            'subsidy_is_redeemable': {'can_redeem': True},
+            '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_POLICY_EXPIRED, []),
+        },
+        {
+            # The subsidy is not active, every other check would succeed.
+            # Expected can_redeem result: False
+            'policy_is_active': True,
+            'catalog_contains_content': True,
+            'enterprise_contains_learner': True,
+            'subsidy_is_redeemable': {'can_redeem': True, 'active': False},
             'transactions_for_learner': {'transactions': [], 'aggregates': {}},
             'transactions_for_policy': {'results': [], 'aggregates': {'total_quantity': -200}},
-            'expected_policy_can_redeem': (False, REASON_POLICY_NOT_ACTIVE, []),
+            'expected_policy_can_redeem': (False, REASON_SUBSIDY_EXPIRED, []),
         },
     )
     @ddt.unpack