From 0b4576f9dc2c614151a6f0d8e982cb159a0f270f Mon Sep 17 00:00:00 2001 From: Alex Dusenbery Date: Thu, 27 Jul 2023 15:02:42 -0400 Subject: [PATCH] fix: when we fetch inter-service data in can_redeem matters for performance --- .../apps/subsidy_access_policy/models.py | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/enterprise_access/apps/subsidy_access_policy/models.py b/enterprise_access/apps/subsidy_access_policy/models.py index fd67c300..6c660f41 100644 --- a/enterprise_access/apps/subsidy_access_policy/models.py +++ b/enterprise_access/apps/subsidy_access_policy/models.py @@ -333,15 +333,9 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): * second element contains a reason code if the content is not redeemable, * 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 policy + if not self.active: + return (False, REASON_POLICY_EXPIRED, []) # learner not associated to enterprise if not skip_customer_user_check: @@ -352,20 +346,31 @@ def can_redeem(self, lms_user_id, content_key, skip_customer_user_check=False): if not self.catalog_contains_content_key(content_key): return (False, REASON_CONTENT_NOT_IN_CATALOG, []) + # Wait to fetch content metadata with a call to the enterprise-subsidy + # service until we *know* that we'll need it. + content_metadata = self.get_content_metadata(content_key) + # no content key in content metadata if not content_metadata: - return (False, REASON_CONTENT_NOT_IN_CATALOG, existing_transactions) + return (False, REASON_CONTENT_NOT_IN_CATALOG, []) # TODO: Add Course Upgrade/Registration Deadline Passed Error here - # inactive subsidy + # We want to wait to do these checks that might require a call + # to the enterprise-subsidy service until we *know* we'll need the data. + 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_EXPIRED, []) - # 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)