Skip to content

Commit

Permalink
feat: Updates groups learner record check to cached learner record (#452
Browse files Browse the repository at this point in the history
)
  • Loading branch information
brobro10000 authored Apr 30, 2024
1 parent 4a0d9f4 commit 74a258c
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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),
Expand Down
37 changes: 23 additions & 14 deletions enterprise_access/apps/subsidy_access_policy/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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, [])
Expand Down Expand Up @@ -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
Expand Down
12 changes: 7 additions & 5 deletions enterprise_access/apps/subsidy_access_policy/tests/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

from django.core.cache import cache as django_cache

from test_utils import TEST_USER_RECORD

from ..models import SubsidyAccessPolicy


Expand Down Expand Up @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 74a258c

Please sign in to comment.