From 547f59ed684a9e1fdd34f250071c99fa87cfff34 Mon Sep 17 00:00:00 2001 From: Kira Miller Date: Thu, 15 Feb 2024 21:04:16 +0000 Subject: [PATCH 1/3] feat: adding the PolicyGroupAssociation table --- ...ntassignmentaction_action_type_and_more.py | 23 +++++++++++++++ .../migrations/0022_policygroupassociation.py | 29 +++++++++++++++++++ .../apps/subsidy_access_policy/models.py | 29 +++++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 enterprise_access/apps/content_assignments/migrations/0018_alter_historicallearnercontentassignmentaction_action_type_and_more.py create mode 100644 enterprise_access/apps/subsidy_access_policy/migrations/0022_policygroupassociation.py diff --git a/enterprise_access/apps/content_assignments/migrations/0018_alter_historicallearnercontentassignmentaction_action_type_and_more.py b/enterprise_access/apps/content_assignments/migrations/0018_alter_historicallearnercontentassignmentaction_action_type_and_more.py new file mode 100644 index 00000000..419138e0 --- /dev/null +++ b/enterprise_access/apps/content_assignments/migrations/0018_alter_historicallearnercontentassignmentaction_action_type_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.10 on 2024-02-15 21:00 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('content_assignments', '0017_migrate_automatic_cancellation_to_expired'), + ] + + operations = [ + migrations.AlterField( + model_name='historicallearnercontentassignmentaction', + name='action_type', + field=models.CharField(choices=[('learner_linked', 'Learner linked to customer'), ('notified', 'Learner notified of assignment'), ('reminded', 'Learner reminded about assignment'), ('redeemed', 'Learner redeemed the assigned content'), ('cancelled', 'Learner assignment cancelled'), ('cancelled_acknowledged', 'Learner assignment cancellation acknowledged by learner'), ('expired', 'Learner assignment expired'), ('expired_acknowledged', 'Learner assignment expiration acknowledged by learner')], db_index=True, help_text='The type of action take on the related assignment record.', max_length=255), + ), + migrations.AlterField( + model_name='learnercontentassignmentaction', + name='action_type', + field=models.CharField(choices=[('learner_linked', 'Learner linked to customer'), ('notified', 'Learner notified of assignment'), ('reminded', 'Learner reminded about assignment'), ('redeemed', 'Learner redeemed the assigned content'), ('cancelled', 'Learner assignment cancelled'), ('cancelled_acknowledged', 'Learner assignment cancellation acknowledged by learner'), ('expired', 'Learner assignment expired'), ('expired_acknowledged', 'Learner assignment expiration acknowledged by learner')], db_index=True, help_text='The type of action take on the related assignment record.', max_length=255), + ), + ] diff --git a/enterprise_access/apps/subsidy_access_policy/migrations/0022_policygroupassociation.py b/enterprise_access/apps/subsidy_access_policy/migrations/0022_policygroupassociation.py new file mode 100644 index 00000000..bf66a447 --- /dev/null +++ b/enterprise_access/apps/subsidy_access_policy/migrations/0022_policygroupassociation.py @@ -0,0 +1,29 @@ +# Generated by Django 4.2.10 on 2024-02-15 21:00 + +from django.db import migrations, models +import django.db.models.deletion +import django_extensions.db.fields +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('subsidy_access_policy', '0021_subsidyaccesspolicy_retired'), + ] + + operations = [ + migrations.CreateModel( + name='PolicyGroupAssociation', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')), + ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('enterprise_group_uuid', models.UUIDField(default=uuid.uuid4, editable=False, help_text='The uuid that uniquely identifies the associated group.', unique=True)), + ('subsidy_access_policy', models.ForeignKey(help_text='The SubsidyAccessPolicy that this group is tied to.', on_delete=django.db.models.deletion.CASCADE, related_name='groups', to='subsidy_access_policy.subsidyaccesspolicy')), + ], + options={ + 'unique_together': {('subsidy_access_policy', 'enterprise_group_uuid')}, + }, + ), + ] diff --git a/enterprise_access/apps/subsidy_access_policy/models.py b/enterprise_access/apps/subsidy_access_policy/models.py index 1839b9e3..c7a33ba5 100644 --- a/enterprise_access/apps/subsidy_access_policy/models.py +++ b/enterprise_access/apps/subsidy_access_policy/models.py @@ -1426,3 +1426,32 @@ def allocate(self, learner_emails, content_key, content_price_cents): content_key, content_price_cents, ) + + +class PolicyGroupAssociation(TimeStampedModel): + """ + This model ties together a policy and a group. + + .. no_pii: This model has no PII + """ + + class Meta: + unique_together = [ + ('subsidy_access_policy', 'enterprise_group_uuid'), + ] + + subsidy_access_policy = models.ForeignKey( + SubsidyAccessPolicy, + related_name="groups", + on_delete=models.CASCADE, + null=False, + help_text="The SubsidyAccessPolicy that this group is tied to.", + ) + + enterprise_group_uuid = models.UUIDField( + default=uuid4, + editable=False, + unique=True, + null=False, + help_text='The uuid that uniquely identifies the associated group.', + ) From cae9a0458ac8ea33e4b7e958e71eab8031e4f4bc Mon Sep 17 00:00:00 2001 From: Kira Miller Date: Tue, 20 Feb 2024 20:08:58 +0000 Subject: [PATCH 2/3] fix: PR requests --- .../apps/subsidy_access_policy/models.py | 2 +- .../subsidy_access_policy/tests/factories.py | 15 ++++++- .../tests/test_models.py | 41 ++++++++++++++++++- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/enterprise_access/apps/subsidy_access_policy/models.py b/enterprise_access/apps/subsidy_access_policy/models.py index c7a33ba5..6c63b504 100644 --- a/enterprise_access/apps/subsidy_access_policy/models.py +++ b/enterprise_access/apps/subsidy_access_policy/models.py @@ -1430,7 +1430,7 @@ def allocate(self, learner_emails, content_key, content_price_cents): class PolicyGroupAssociation(TimeStampedModel): """ - This model ties together a policy and a group. + This model ties together a policy (SubsidyAccessPolicy) and a group (EnterpriseGroup in edx-enterprise). .. no_pii: This model has no PII """ diff --git a/enterprise_access/apps/subsidy_access_policy/tests/factories.py b/enterprise_access/apps/subsidy_access_policy/tests/factories.py index e91fd4ca..3c0b5ee0 100644 --- a/enterprise_access/apps/subsidy_access_policy/tests/factories.py +++ b/enterprise_access/apps/subsidy_access_policy/tests/factories.py @@ -11,7 +11,8 @@ from enterprise_access.apps.subsidy_access_policy.models import ( AssignedLearnerCreditAccessPolicy, PerLearnerEnrollmentCreditAccessPolicy, - PerLearnerSpendCreditAccessPolicy + PerLearnerSpendCreditAccessPolicy, + PolicyGroupAssociation ) FAKER = Faker() @@ -64,3 +65,15 @@ class Meta: spend_limit = factory.LazyAttribute(lambda _: FAKER.pyint()) per_learner_spend_limit = None per_learner_enrollment_limit = None + + +class PolicyGroupAssociationFactory(factory.django.DjangoModelFactory): + """ + Test factory for the `PolicyGroupAssociation` model. + """ + + class Meta: + model = PolicyGroupAssociation + + enterprise_group_uuid = factory.LazyFunction(uuid4) + subsidy_access_policy = factory.SubFactory(SubsidyAccessPolicyFactory) 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 e6d1f900..bcb70764 100644 --- a/enterprise_access/apps/subsidy_access_policy/tests/test_models.py +++ b/enterprise_access/apps/subsidy_access_policy/tests/test_models.py @@ -45,7 +45,8 @@ from enterprise_access.apps.subsidy_access_policy.tests.factories import ( AssignedLearnerCreditAccessPolicyFactory, PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory, - PerLearnerSpendCapLearnerCreditAccessPolicyFactory + PerLearnerSpendCapLearnerCreditAccessPolicyFactory, + PolicyGroupAssociationFactory, ) from enterprise_access.cache_utils import request_cache @@ -1269,3 +1270,41 @@ def test_can_allocate_invalid_price(self, real_price, requested_price): } with self.assertRaisesRegex(PriceValidationError, 'outside of acceptable interval'): self.active_policy.can_allocate(1, self.course_key, requested_price) + + +@ddt.ddt +class PolicyGroupAssociationTests(MockPolicyDependenciesMixin, TestCase): + """ Tests specific to the policy group association model. """ + + lms_user_id = 12345 + group_uuid = uuid4() + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.access_policy = AssignedLearnerCreditAccessPolicyFactory() + + def tearDown(self): + """ + Clears any cached data for the test policy instances between test runs. + """ + super().tearDown() + request_cache(namespace=REQUEST_CACHE_NAMESPACE).clear() + + def test_save(self): + """ + Test that the model-level validation of this model works as expected. + Should be saved with a unique combination of SubsidyAccessPolicy + and group uuid (enterprise_customer_uuid). + """ + + policy = PolicyGroupAssociationFactory( + enterprise_group_uuid=self.group_uuid, + subsidy_access_policy=self.access_policy, + ) + + policy.save() + policy.refresh_from_db() + + self.assertEqual(policy.enterprise_group_uuid, self.group_uuid) + self.assertIsNotNone(policy.subsidy_access_policy) From f1a8686de3fd9105e05042dbaa335c0ad4c3b5c7 Mon Sep 17 00:00:00 2001 From: Kira Miller Date: Tue, 20 Feb 2024 20:59:56 +0000 Subject: [PATCH 3/3] fix: spelling errors --- .../tests/test_subsidy_access_policy_views.py | 2 +- .../tests/test_models.py | 34 +++++++++---------- 2 files changed, 18 insertions(+), 18 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 29fd3127..e9705408 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 @@ -638,7 +638,7 @@ def test_update_views(self, is_patch, request_payload): def test_update_views_fields_disallowed_for_update(self, request_payload): """ Test that the update and partial_update views can NOT modify fields - of a policy record that are not included in the update request serializer fields defintion. + of a policy record that are not included in the update request serializer fields definition. """ # Set the JWT-based auth to an operator. self.set_jwt_cookie([ 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 bcb70764..e6dbdf8f 100644 --- a/enterprise_access/apps/subsidy_access_policy/tests/test_models.py +++ b/enterprise_access/apps/subsidy_access_policy/tests/test_models.py @@ -46,7 +46,7 @@ AssignedLearnerCreditAccessPolicyFactory, PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory, PerLearnerSpendCapLearnerCreditAccessPolicyFactory, - PolicyGroupAssociationFactory, + PolicyGroupAssociationFactory ) from enterprise_access.cache_utils import request_cache @@ -696,34 +696,34 @@ def setUp(self): self.policy_three = PerLearnerEnrollmentCapLearnerCreditAccessPolicyFactory.create() self.policy_four = PerLearnerSpendCapLearnerCreditAccessPolicyFactory.create() - policy_one_subsity_patcher = patch.object( + policy_one_subsidy_patcher = patch.object( self.policy_one, 'subsidy_record' ) - self.mock_policy_one_subsidy_record = policy_one_subsity_patcher.start() + self.mock_policy_one_subsidy_record = policy_one_subsidy_patcher.start() self.mock_policy_one_subsidy_record.return_value = self.mock_subsidy_one - policy_two_subsity_patcher = patch.object( + policy_two_subsidy_patcher = patch.object( self.policy_two, 'subsidy_record' ) - self.mock_policy_two_subsidy_record = policy_two_subsity_patcher.start() + self.mock_policy_two_subsidy_record = policy_two_subsidy_patcher.start() self.mock_policy_two_subsidy_record.return_value = self.mock_subsidy_two - policy_three_subsity_patcher = patch.object( + policy_three_subsidy_patcher = patch.object( self.policy_three, 'subsidy_record' ) - self.mock_policy_three_subsidy_record = policy_three_subsity_patcher.start() + self.mock_policy_three_subsidy_record = policy_three_subsidy_patcher.start() self.mock_policy_three_subsidy_record.return_value = self.mock_subsidy_three - policy_four_subsity_patcher = patch.object( + policy_four_subsidy_patcher = patch.object( self.policy_four, 'subsidy_record' ) - self.mock_policy_four_subsidy_record = policy_four_subsity_patcher.start() + self.mock_policy_four_subsidy_record = policy_four_subsidy_patcher.start() self.mock_policy_four_subsidy_record.return_value = self.mock_subsidy_four - self.addCleanup(policy_one_subsity_patcher.stop) - self.addCleanup(policy_two_subsity_patcher.stop) - self.addCleanup(policy_three_subsity_patcher.stop) - self.addCleanup(policy_four_subsity_patcher.stop) + self.addCleanup(policy_one_subsidy_patcher.stop) + self.addCleanup(policy_two_subsidy_patcher.stop) + self.addCleanup(policy_three_subsidy_patcher.stop) + self.addCleanup(policy_four_subsidy_patcher.stop) def test_setup(self): """ @@ -753,7 +753,7 @@ def test_resolve_two_policies_by_balance(self): @override_settings(MULTI_POLICY_RESOLUTION_ENABLED=True) def test_resolve_two_policies_by_expiration(self): """ - Test resolve given a two policies with different balances, differet expiration + Test resolve given a two policies with different balances, different expiration the sooner expiration policy should be returned. """ policies = [self.policy_one, self.policy_three] @@ -766,7 +766,7 @@ def test_resolve_two_policies_by_type_priority(self): but different type-priority. """ policies = [self.policy_four, self.policy_one] - # artificially set the priority attribute higher on one of the policies (lower priority takes precident) + # artificially set the priority attribute higher on one of the policies (lower priority takes precedent) with patch.object(PerLearnerSpendCreditAccessPolicy, 'priority', new_callable=PropertyMock) as mock: mock.return_value = 100 assert SubsidyAccessPolicy.resolve_policy(policies) == self.policy_one @@ -1080,7 +1080,7 @@ def test_redeem( assert redeemed_action.action_type == AssignmentActions.REDEEMED assert not redeemed_action.error_reason if fail_subsidy_create_transaction: - # sad path should generate a failed redeememd action with populated error_reason and traceback. + # sad path should generate a failed redeemed action with populated error_reason and traceback. redeemed_action = assignment.actions.last() assert redeemed_action.action_type == AssignmentActions.REDEEMED assert redeemed_action.error_reason == AssignmentActionErrors.ENROLLMENT_ERROR @@ -1235,7 +1235,7 @@ def test_can_allocate_happy_path(self): self.mock_assignments_api.get_allocated_quantity_for_configuration.return_value = -1000 # Request a price just slightly different from the canonical price - # the subidy remaining balance and the spend limit are both 10,000 + # the subsidy remaining balance and the spend limit are both 10,000 # ((7 * 1000) + 1000 + 1000) < 10000 can_allocate, _ = self.active_policy.can_allocate(7, self.course_key, 1000)