From a403ebd181f51ff611a28d9af42a5adcaa2ad30b Mon Sep 17 00:00:00 2001 From: Troy Sankey Date: Fri, 12 Jan 2024 09:58:51 -0800 Subject: [PATCH] feat: new redeemability_disabled_at field on policy models Implements ADR 0016-policy-retirement ENT-8206 --- .../api/serializers/subsidy_access_policy.py | 7 +++ .../tests/test_subsidy_access_policy_views.py | 16 ++++++- .../api/v1/views/subsidy_access_policy.py | 5 +-- .../apps/subsidy_access_policy/admin.py | 5 +++ ...yaccesspolicy_redeemability_disabled_at.py | 33 ++++++++++++++ .../apps/subsidy_access_policy/models.py | 38 ++++++++++++++-- .../tests/test_models.py | 43 ++++++++++++++----- 7 files changed, 129 insertions(+), 18 deletions(-) create mode 100644 enterprise_access/apps/subsidy_access_policy/migrations/0021_subsidyaccesspolicy_redeemability_disabled_at.py diff --git a/enterprise_access/apps/api/serializers/subsidy_access_policy.py b/enterprise_access/apps/api/serializers/subsidy_access_policy.py index 0f285dae1..a9315f460 100644 --- a/enterprise_access/apps/api/serializers/subsidy_access_policy.py +++ b/enterprise_access/apps/api/serializers/subsidy_access_policy.py @@ -161,6 +161,7 @@ class Meta: 'display_name', 'description', 'active', + 'redeemability_disabled_at', 'enterprise_customer_uuid', 'catalog_uuid', 'subsidy_uuid', @@ -194,6 +195,7 @@ class Meta: 'display_name', 'description', 'active', + 'redeemability_disabled_at', 'enterprise_customer_uuid', 'catalog_uuid', 'subsidy_uuid', @@ -395,6 +397,7 @@ class Meta: 'display_name', 'description', 'active', + 'redeemability_disabled_at', 'catalog_uuid', 'subsidy_uuid', 'access_method', @@ -422,6 +425,10 @@ class Meta: 'allow_null': False, 'required': False, }, + 'redeemability_disabled_at': { + 'allow_null': True, + 'required': False, + }, 'catalog_uuid': { 'allow_null': False, 'required': False, 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 9cf28c3c5..e73937a81 100644 --- 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 @@ -270,6 +270,7 @@ def test_detail_view(self, role_context_dict): self.assertEqual({ 'access_method': 'direct', 'active': True, + 'redeemability_disabled_at': None, 'catalog_uuid': str(self.redeemable_policy.catalog_uuid), 'display_name': self.redeemable_policy.display_name, 'description': 'A generic description', @@ -361,6 +362,7 @@ def test_list_view(self, role_context_dict): { 'access_method': 'direct', 'active': True, + 'redeemability_disabled_at': None, 'catalog_uuid': str(self.non_redeemable_policy.catalog_uuid), 'display_name': self.non_redeemable_policy.display_name, 'description': 'A generic description', @@ -387,6 +389,7 @@ def test_list_view(self, role_context_dict): { 'access_method': 'direct', 'active': True, + 'redeemability_disabled_at': None, 'catalog_uuid': str(self.redeemable_policy.catalog_uuid), 'display_name': self.redeemable_policy.display_name, 'description': 'A generic description', @@ -480,6 +483,7 @@ def test_destroy_view(self, request_payload, expected_change_reason): expected_response = { 'access_method': 'direct', 'active': False, + 'redeemability_disabled_at': None, 'catalog_uuid': str(self.redeemable_policy.catalog_uuid), 'display_name': self.redeemable_policy.display_name, 'description': 'A generic description', @@ -526,6 +530,7 @@ def test_destroy_view(self, request_payload, expected_change_reason): 'description': 'the new description', 'display_name': 'new display_name', 'active': True, + 'redeemability_disabled_at': '2023-01-01T00:00:00Z', 'catalog_uuid': str(uuid4()), 'subsidy_uuid': str(uuid4()), 'access_method': AccessMethods.ASSIGNED, @@ -540,6 +545,7 @@ def test_destroy_view(self, request_payload, expected_change_reason): 'description': 'the new description', 'display_name': 'new display_name', 'active': True, + 'redeemability_disabled_at': '2023-01-01T00:00:00Z', 'catalog_uuid': str(uuid4()), 'subsidy_uuid': str(uuid4()), 'access_method': AccessMethods.ASSIGNED, @@ -586,6 +592,7 @@ def test_update_views(self, is_patch, request_payload): # Fields that we officially support PATCHing. 'access_method': policy_for_edit.access_method, 'active': policy_for_edit.active, + 'redeemability_disabled_at': policy_for_edit.redeemability_disabled_at, 'catalog_uuid': str(policy_for_edit.catalog_uuid), 'display_name': policy_for_edit.display_name, 'description': policy_for_edit.description, @@ -802,12 +809,13 @@ def test_create_view(self, policy_type, extra_fields, expected_response_code, ex }, ]) - # Test the retrieve endpoint + # Test the create endpoint payload = { 'policy_type': policy_type, 'display_name': 'created policy', 'description': 'test description', 'active': True, + 'redeemability_disabled_at': None, 'enterprise_customer_uuid': str(TEST_ENTERPRISE_UUID), 'catalog_uuid': str(uuid4()), 'subsidy_uuid': str(uuid4()), @@ -863,6 +871,7 @@ def test_idempotent_create_view(self, policy_type, extra_fields, expected_respon 'display_name': 'new policy', 'description': 'test description', 'active': True, + 'redeemability_disabled_at': None, 'enterprise_customer_uuid': enterprise_customer_uuid, 'catalog_uuid': catalog_uuid, 'subsidy_uuid': subsidy_uuid, @@ -1288,6 +1297,11 @@ def test_credits_available_endpoint( enterprise_customer_uuid=self.enterprise_uuid, active=False, ) + # The following policy should never be returned as it has redeemability disabled. + PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory( + enterprise_customer_uuid=self.enterprise_uuid, + redeemability_disabled_at=datetime.utcnow() - timedelta(days=1), + ) # The following policy should never be returned as it's had more spend than the `spend_limit`. PerLearnerSpendCapLearnerCreditAccessPolicyFactory( enterprise_customer_uuid=self.enterprise_uuid, 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 de90d994c..bf25b5f41 100644 --- a/enterprise_access/apps/api/v1/views/subsidy_access_policy.py +++ b/enterprise_access/apps/api/v1/views/subsidy_access_policy.py @@ -388,12 +388,11 @@ def get_permission_object(self): def get_queryset(self): """ - Base queryset that returns all active policies associated + Base queryset that returns all active & redeemable policies associated with the customer uuid requested by the client. """ - return SubsidyAccessPolicy.objects.filter( + return SubsidyAccessPolicy.policies_with_redemption_enabled().filter( enterprise_customer_uuid=self.enterprise_customer_uuid, - active=True, ).order_by('-created') def evaluate_policies(self, enterprise_customer_uuid, lms_user_id, content_key): diff --git a/enterprise_access/apps/subsidy_access_policy/admin.py b/enterprise_access/apps/subsidy_access_policy/admin.py index fdfc30955..ff182c592 100644 --- a/enterprise_access/apps/subsidy_access_policy/admin.py +++ b/enterprise_access/apps/subsidy_access_policy/admin.py @@ -47,11 +47,13 @@ class BaseSubsidyAccessPolicyMixin(admin.ModelAdmin): 'uuid', 'modified', 'active', + 'redeemability_disabled_at', 'display_name_or_short_description', 'policy_spend_limit_dollars', ) list_filter = ( 'active', + 'redeemability_disabled_at', 'access_method', ) ordering = ['-modified'] @@ -140,6 +142,7 @@ class PerLearnerEnrollmentCreditAccessPolicy(DjangoQLSearchMixin, BaseSubsidyAcc 'display_name', 'description', 'active', + 'redeemability_disabled_at', 'catalog_uuid', 'subsidy_uuid', 'created', @@ -188,6 +191,7 @@ class PerLearnerSpendCreditAccessPolicy(DjangoQLSearchMixin, BaseSubsidyAccessPo 'display_name', 'description', 'active', + 'redeemability_disabled_at', 'catalog_uuid', 'subsidy_uuid', 'created', @@ -244,6 +248,7 @@ class LearnerContentAssignmentAccessPolicy(DjangoQLSearchMixin, BaseSubsidyAcces 'display_name', 'description', 'active', + 'redeemability_disabled_at', 'catalog_uuid', 'subsidy_uuid', 'assignment_configuration', diff --git a/enterprise_access/apps/subsidy_access_policy/migrations/0021_subsidyaccesspolicy_redeemability_disabled_at.py b/enterprise_access/apps/subsidy_access_policy/migrations/0021_subsidyaccesspolicy_redeemability_disabled_at.py new file mode 100644 index 000000000..1eb16c9e5 --- /dev/null +++ b/enterprise_access/apps/subsidy_access_policy/migrations/0021_subsidyaccesspolicy_redeemability_disabled_at.py @@ -0,0 +1,33 @@ +# Generated by Django 4.2.9 on 2024-01-16 05:25 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('subsidy_access_policy', '0020_alter_historicalassignedlearnercreditaccesspolicy_options_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='historicalsubsidyaccesspolicy', + name='redeemability_disabled_at', + field=models.DateTimeField(blank=True, default=None, help_text="Defines when redeemability of content using this policy was disabled. Set this to deactivate the policy but keep it visible from an admin's perspective (useful when you want to just expire a policy without expiring the whole plan).", null=True), + ), + migrations.AddField( + model_name='subsidyaccesspolicy', + name='redeemability_disabled_at', + field=models.DateTimeField(blank=True, default=None, help_text="Defines when redeemability of content using this policy was disabled. Set this to deactivate the policy but keep it visible from an admin's perspective (useful when you want to just expire a policy without expiring the whole plan).", null=True), + ), + migrations.AlterField( + model_name='historicalsubsidyaccesspolicy', + name='active', + field=models.BooleanField(default=False, help_text='Set to FALSE to deactivate and hide this policy. Use this when you want to disable redemption and make it disappear from all frontends, effectively soft-deleting it. Default is False (deactivated).'), + ), + migrations.AlterField( + model_name='subsidyaccesspolicy', + name='active', + field=models.BooleanField(default=False, help_text='Set to FALSE to deactivate and hide this policy. Use this when you want to disable redemption and make it disappear from all frontends, effectively soft-deleting it. Default is False (deactivated).'), + ), + ] diff --git a/enterprise_access/apps/subsidy_access_policy/models.py b/enterprise_access/apps/subsidy_access_policy/models.py index c5f876505..0765a97b6 100644 --- a/enterprise_access/apps/subsidy_access_policy/models.py +++ b/enterprise_access/apps/subsidy_access_policy/models.py @@ -132,7 +132,20 @@ class Meta: ) active = models.BooleanField( default=False, - help_text='Whether this policy is active, defaults to false.', + help_text=( + 'Set to FALSE to deactivate and hide this policy. Use this when you want to disable redemption and make ' + 'it disappear from all frontends, effectively soft-deleting it. Default is False (deactivated).' + ), + ) + redeemability_disabled_at = models.DateTimeField( + null=True, + blank=True, + default=None, + help_text=( + "Defines when redeemability of content using this policy was disabled. " + "Set this to deactivate the policy but keep it visible from an admin's perspective " + "(useful when you want to just expire a policy without expiring the whole plan)." + ), ) catalog_uuid = models.UUIDField( db_index=True, @@ -195,6 +208,23 @@ class Meta: # ProxyAwareHistoricalRecords docstring for more info. history = ProxyAwareHistoricalRecords(inherit=True) + @classmethod + def policies_with_redemption_enabled(cls): + """ + Return all policies which have redemption enabled. + """ + return cls.objects.filter( + active=True, + redeemability_disabled_at__isnull=True, + ) + + @property + def is_redemption_enabled(self): + """ + Return True if this policy has redemption enabled. + """ + return self.active and not self.redeemability_disabled_at + @property def subsidy_active_datetime(self): """ @@ -526,7 +556,7 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): * third a list of any transactions representing existing redemptions (any state). """ # inactive policy - if not self.active: + if not self.is_redemption_enabled: return (False, REASON_POLICY_EXPIRED, []) # learner not associated to enterprise @@ -605,7 +635,7 @@ def credit_available(self, lms_user_id, skip_customer_user_check=False): * Whether the transactions associated with policy have exceeded the policy-wide spend limit. """ # inactive policy - if not self.active: + if not self.is_redemption_enabled: logger.info('[credit_available] policy %s inactive', self.uuid) return False @@ -1227,7 +1257,7 @@ def can_allocate(self, number_of_learners, content_key, content_price_cents): self.validate_requested_allocation_price(content_key, content_price_cents) # inactive policy - if not self.active: + if not self.is_redemption_enabled: return (False, REASON_POLICY_EXPIRED) # no content key in catalog 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 b1c7bdb4e..59490b605 100644 --- a/enterprise_access/apps/subsidy_access_policy/tests/test_models.py +++ b/enterprise_access/apps/subsidy_access_policy/tests/test_models.py @@ -119,6 +119,10 @@ def setUpClass(cls): per_learner_enrollment_limit=5, active=False, ) + cls.active_non_redeemable_per_learner_enroll_policy = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory( + per_learner_enrollment_limit=5, + redeemability_disabled_at=datetime.utcnow() - timedelta(days=1), + ) cls.per_learner_spend_policy = PerLearnerSpendCapLearnerCreditAccessPolicyFactory( uuid=ACTIVE_LEARNER_SPEND_CAP_POLICY_UUID, per_learner_spend_limit=500, @@ -128,6 +132,10 @@ def setUpClass(cls): per_learner_spend_limit=500, active=False, ) + cls.active_non_redeemable_per_learner_spend_policy = PerLearnerSpendCapLearnerCreditAccessPolicyFactory( + per_learner_spend_limit=500, + redeemability_disabled_at=datetime.utcnow() - timedelta(days=1), + ) def tearDown(self): """ @@ -197,7 +205,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args): # Happy path: content in catalog, learner in enterprise, subsidy has value, # existing transactions for learner and policy below the policy limits. # Expected can_redeem result: True - 'policy_is_active': True, + 'policy_active_type': 'active', 'catalog_contains_content': True, 'enterprise_contains_learner': True, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, @@ -208,7 +216,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args): { # Content not in catalog, every other check would succeed. # Expected can_redeem result: False - 'policy_is_active': True, + 'policy_active_type': 'active', 'catalog_contains_content': False, 'enterprise_contains_learner': True, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, @@ -221,7 +229,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args): { # Learner is not in the enterprise, every other check would succeed. # Expected can_redeem result: False - 'policy_is_active': True, + 'policy_active_type': 'active', 'catalog_contains_content': True, 'enterprise_contains_learner': False, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, @@ -234,7 +242,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args): { # The subsidy is not redeemable, every other check would succeed. # Expected can_redeem result: False - 'policy_is_active': True, + 'policy_active_type': 'active', 'catalog_contains_content': True, 'enterprise_contains_learner': True, 'subsidy_is_redeemable': {'can_redeem': False, 'active': True}, @@ -246,7 +254,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args): # The subsidy is redeemable, but the learner has already enrolled more than the limit. # Every other check would succeed. # Expected can_redeem result: False - 'policy_is_active': True, + 'policy_active_type': 'active', 'catalog_contains_content': True, 'enterprise_contains_learner': True, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, @@ -266,7 +274,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args): # The subsidy is redeemable, but another redemption would exceed the policy-wide ``spend_limit``. # Every other check would succeed. # Expected can_redeem result: False - 'policy_is_active': True, + 'policy_active_type': 'active', 'catalog_contains_content': True, 'enterprise_contains_learner': True, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, @@ -285,7 +293,20 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args): { # The subsidy access policy is not active, every other check would succeed. # Expected can_redeem result: False - 'policy_is_active': False, + 'policy_active_type': 'inactive', + 'catalog_contains_content': True, + 'enterprise_contains_learner': 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, []), + 'expect_content_metadata_fetch': False, + 'expect_transaction_fetch': False, + }, + { + # The subsidy access policy is not redeemable, every other check would succeed. + # Expected can_redeem result: False + 'policy_active_type': 'non_redeemable', 'catalog_contains_content': True, 'enterprise_contains_learner': True, 'subsidy_is_redeemable': {'can_redeem': True, 'active': True}, @@ -298,7 +319,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args): { # The subsidy is not active, every other check would succeed. # Expected can_redeem result: False - 'policy_is_active': True, + 'policy_active_type': 'active', 'catalog_contains_content': True, 'enterprise_contains_learner': True, 'subsidy_is_redeemable': {'can_redeem': True, 'active': False}, @@ -310,7 +331,7 @@ def test_object_creation_with_policy_type_in_kwarg(self, *args): @ddt.unpack def test_learner_enrollment_cap_policy_can_redeem( self, - policy_is_active, + policy_active_type, catalog_contains_content, enterprise_contains_learner, subsidy_is_redeemable, @@ -333,8 +354,10 @@ def test_learner_enrollment_cap_policy_can_redeem( self.mock_subsidy_client.list_subsidy_transactions.return_value = transactions_for_policy policy_record = self.inactive_per_learner_enroll_policy - if policy_is_active: + if policy_active_type == 'active': policy_record = self.per_learner_enroll_policy + elif policy_active_type == 'non_redeemable': + policy_record = self.active_non_redeemable_per_learner_enroll_policy can_redeem_result = policy_record.can_redeem(self.lms_user_id, self.course_id)