From 74a258c68a2ef026e0b73d5a322dde9118537072 Mon Sep 17 00:00:00 2001 From: Hamzah Ullah Date: Tue, 30 Apr 2024 12:12:53 -0400 Subject: [PATCH] feat: Updates groups learner record check to cached learner record (#452) --- .../tests/test_subsidy_access_policy_views.py | 11 +++--- .../apps/subsidy_access_policy/models.py | 37 ++++++++++++------- .../subsidy_access_policy/tests/mixins.py | 12 +++--- .../tests/test_models.py | 2 + 4 files changed, 38 insertions(+), 24 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 b4568546..44e2e6df 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 @@ -1092,17 +1092,17 @@ def setup_mocks(self): self.lms_client_instance = lms_client.return_value self.lms_client_instance.get_enterprise_user.return_value = TEST_USER_RECORD - includes_user_patcher = patch.object( - SubsidyAccessPolicy, 'includes_user' + enterprise_user_record_patcher = patch.object( + SubsidyAccessPolicy, 'enterprise_user_record' ) - self.mock_includes_user = includes_user_patcher.start() - self.mock_includes_user.return_value = True + self.mock_enterprise_user_record = enterprise_user_record_patcher.start() + self.mock_enterprise_user_record.return_value = TEST_USER_RECORD self.addCleanup(lms_client_patcher.stop) self.addCleanup(subsidy_client_patcher.stop) self.addCleanup(contains_key_patcher.stop) self.addCleanup(get_content_metadata_patcher.stop) - self.addCleanup(includes_user_patcher.stop) + self.addCleanup(enterprise_user_record_patcher.stop) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') @mock.patch('enterprise_access.apps.subsidy_access_policy.models.get_and_cache_transactions_for_learner') @@ -1415,6 +1415,7 @@ def test_credits_available_endpoint( 'is_active': is_subsidy_active, } self.lms_client_instance.get_enterprise_user.return_value = get_enterprise_user + self.mock_enterprise_user_record.return_value = get_enterprise_user query_params = { 'enterprise_customer_uuid': str(self.enterprise_uuid), diff --git a/enterprise_access/apps/subsidy_access_policy/models.py b/enterprise_access/apps/subsidy_access_policy/models.py index 269c32ab..62ca4315 100644 --- a/enterprise_access/apps/subsidy_access_policy/models.py +++ b/enterprise_access/apps/subsidy_access_policy/models.py @@ -385,16 +385,16 @@ def subsidy_record_from_tiered_cache(self, *cache_key_args): set_tiered_cache_subsidy_record(record, *cache_key_args) return record - def includes_user(self, lms_user_id): + def enterprise_user_record(self, lms_user_id): """ - Determines if the user is included in the SubsidyAccessPolicy's customer. + Returns the enterprise_user_record. Retrieves it from TieredCache if available, otherwise, it will retrieve and initialize the cache. """ - enterprise_learner_record = get_and_cache_enterprise_learner_record( + enterprise_user_record = get_and_cache_enterprise_learner_record( enterprise_customer_uuid=self.enterprise_customer_uuid, learner_id=lms_user_id, ) - return bool(enterprise_learner_record) + return enterprise_user_record def subsidy_balance(self): """ @@ -494,14 +494,17 @@ def get_list_price(self, lms_user_id, content_key): # pylint: disable=unused-ar self.get_content_metadata(content_key), ) - def includes_learner(self, lms_user_id): + def includes_learner(self, lms_user_id, enterprise_user_record=None): """ Determine whether the lms user is associated properly with both the enterprise and the policy's group(s). """ - learner_record = self.lms_api_client.get_enterprise_user(self.enterprise_customer_uuid, lms_user_id) - if not learner_record: - return False, REASON_LEARNER_NOT_IN_ENTERPRISE + if enterprise_user_record is not None: + learner_record = enterprise_user_record + else: + learner_record = self.enterprise_user_record(lms_user_id) + if not learner_record: + return False, REASON_LEARNER_NOT_IN_ENTERPRISE associated_group_uuids = set(learner_record.get('enterprise_group', [])) # if there are no policy groups, return early @@ -647,12 +650,14 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): # learner not associated to enterprise if not skip_customer_user_check: - if not self.includes_user(lms_user_id): + enterprise_user_record = self.enterprise_user_record(lms_user_id) + if not enterprise_user_record: self._log_redeemability(False, REASON_LEARNER_NOT_IN_ENTERPRISE, lms_user_id, content_key) return (False, REASON_LEARNER_NOT_IN_ENTERPRISE, []) - - if not skip_customer_user_check: - included_in_policy, reason = self.includes_learner(lms_user_id) + included_in_policy, reason = self.includes_learner( + lms_user_id, + enterprise_user_record + ) if not included_in_policy: self._log_redeemability(False, reason, lms_user_id, content_key) return (False, reason, []) @@ -750,12 +755,16 @@ def credit_available( # learner not linked to enterprise if not skip_customer_user_check: - if not self.includes_user(lms_user_id): + enterprise_user_record = self.enterprise_user_record(lms_user_id) + if not enterprise_user_record: logger.info( f'[credit_available] learner {lms_user_id} not linked to enterprise {self.enterprise_customer_uuid}' ) return False - included_in_policy, reason = self.includes_learner(lms_user_id) + included_in_policy, reason = self.includes_learner( + lms_user_id, + enterprise_user_record + ) if not included_in_policy: logger.info(f'[credit_available] learner {lms_user_id} encountered error {reason}') return False diff --git a/enterprise_access/apps/subsidy_access_policy/tests/mixins.py b/enterprise_access/apps/subsidy_access_policy/tests/mixins.py index 3eede37c..ca39a377 100644 --- a/enterprise_access/apps/subsidy_access_policy/tests/mixins.py +++ b/enterprise_access/apps/subsidy_access_policy/tests/mixins.py @@ -5,6 +5,8 @@ from django.core.cache import cache as django_cache +from test_utils import TEST_USER_RECORD + from ..models import SubsidyAccessPolicy @@ -44,17 +46,17 @@ def setUp(self): self.mock_lms_api_client = lms_api_client_patcher.start() - includes_user_patcher = patch.object( - SubsidyAccessPolicy, 'includes_user' + enterprise_user_record_patcher = patch.object( + SubsidyAccessPolicy, 'enterprise_user_record' ) - self.mock_includes_user = includes_user_patcher.start() - self.mock_includes_user.return_value = True + self.mock_enterprise_user_record = enterprise_user_record_patcher.start() + self.mock_enterprise_user_record.return_value = TEST_USER_RECORD self.addCleanup(subsidy_client_patcher.stop) self.addCleanup(transactions_cache_for_learner_patcher.stop) self.addCleanup(catalog_contains_content_key_patcher.stop) self.addCleanup(get_content_metadata_patcher.stop) self.addCleanup(lms_api_client_patcher.stop) - self.addCleanup(includes_user_patcher.stop) + self.addCleanup(enterprise_user_record_patcher.stop) self.addCleanup(django_cache.clear) # clear any leftover policy locks. 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 cb052c4f..1a5ff20b 100644 --- a/enterprise_access/apps/subsidy_access_policy/tests/test_models.py +++ b/enterprise_access/apps/subsidy_access_policy/tests/test_models.py @@ -320,6 +320,7 @@ def test_learner_enrollment_cap_policy_can_redeem( Test the can_redeem method of PerLearnerEnrollmentCapLearnerCreditAccessPolicy model """ self.mock_lms_api_client.get_enterprise_user.return_value = get_enterprise_user + self.mock_enterprise_user_record.return_value = get_enterprise_user self.mock_catalog_contains_content_key.return_value = catalog_contains_content self.mock_get_content_metadata.return_value = { 'content_price': 200, @@ -503,6 +504,7 @@ def test_learner_spend_cap_policy_can_redeem( Test the can_redeem method of PerLearnerSpendCapLearnerCreditAccessPolicy model """ self.mock_lms_api_client.get_enterprise_user.return_value = get_enterprise_user + self.mock_enterprise_user_record.return_value = get_enterprise_user self.mock_catalog_contains_content_key.return_value = catalog_contains_content self.mock_get_content_metadata.return_value = { 'content_price': 200,