From b0f03c95b3fe665e9dd52a9836812c6f99ed0931 Mon Sep 17 00:00:00 2001 From: John Nagro Date: Thu, 2 Nov 2023 13:47:08 -0400 Subject: [PATCH] feat: add lms_user_email to redemptions (#162) --- enterprise_subsidy/apps/api/v1/serializers.py | 2 + .../apps/api/v1/tests/test_views.py | 85 ++++++++++++++++--- .../api/v2/tests/test_transaction_views.py | 15 ++++ .../apps/api_client/lms_user.py | 81 ++++++++++++++++++ .../apps/api_client/tests/test_lms_user.py | 81 ++++++++++++++++++ enterprise_subsidy/apps/subsidy/models.py | 50 +++++++++++ .../apps/subsidy/tests/test_models.py | 3 + enterprise_subsidy/settings/base.py | 5 ++ requirements/base.txt | 2 +- requirements/dev.txt | 2 +- requirements/doc.txt | 2 +- requirements/production.txt | 2 +- requirements/quality.txt | 2 +- requirements/test.txt | 2 +- requirements/validation.txt | 2 +- 15 files changed, 319 insertions(+), 17 deletions(-) create mode 100644 enterprise_subsidy/apps/api_client/lms_user.py create mode 100644 enterprise_subsidy/apps/api_client/tests/test_lms_user.py diff --git a/enterprise_subsidy/apps/api/v1/serializers.py b/enterprise_subsidy/apps/api/v1/serializers.py index 383eaa5a..f8e3c1d7 100644 --- a/enterprise_subsidy/apps/api/v1/serializers.py +++ b/enterprise_subsidy/apps/api/v1/serializers.py @@ -128,7 +128,9 @@ class Meta: "state", "idempotency_key", "lms_user_id", + "lms_user_email", "content_key", + "content_title", "quantity", "unit", # Manually fetch from parent ledger via get_unit(). "fulfillment_identifier", diff --git a/enterprise_subsidy/apps/api/v1/tests/test_views.py b/enterprise_subsidy/apps/api/v1/tests/test_views.py index fb0d8ca2..925d3cf5 100644 --- a/enterprise_subsidy/apps/api/v1/tests/test_views.py +++ b/enterprise_subsidy/apps/api/v1/tests/test_views.py @@ -51,7 +51,10 @@ class APITestBase(APITestMixin): subsidy_access_policy_1_uuid = str(uuid.uuid4()) subsidy_access_policy_2_uuid = str(uuid.uuid4()) content_key_1 = "course-v1:edX+test+course.1" + content_title_1 = "edx: Test Course 1" content_key_2 = "course-v1:edX+test+course.2" + content_title_2 = "edx: Test Course 2" + lms_user_email = 'edx@example.com' def setUp(self): super().setUp() @@ -69,8 +72,10 @@ def setUp(self): quantity=-1000, ledger=self.subsidy_1.ledger, lms_user_id=STATIC_LMS_USER_ID, # This is the only transaction belonging to the requester. + lms_user_email=self.lms_user_email, subsidy_access_policy_uuid=self.subsidy_access_policy_1_uuid, content_key=self.content_key_1, + content_title=self.content_title_1, ) self.subsidy_1_transaction_2 = TransactionFactory( uuid=self.subsidy_1_transaction_2_uuid, @@ -78,8 +83,10 @@ def setUp(self): quantity=-1000, ledger=self.subsidy_1.ledger, lms_user_id=STATIC_LMS_USER_ID+1000, + lms_user_email=self.lms_user_email, subsidy_access_policy_uuid=self.subsidy_access_policy_2_uuid, content_key=self.content_key_2, + content_title=self.content_title_2, ) # Create an extra subsidy with the same enterprise_customer_uuid, but the learner does not have any transactions @@ -96,6 +103,7 @@ def setUp(self): quantity=-1000, ledger=self.subsidy_2.ledger, lms_user_id=STATIC_LMS_USER_ID+1000, + lms_user_email=self.lms_user_email, ) TransactionFactory( uuid=self.subsidy_2_transaction_2_uuid, @@ -103,6 +111,7 @@ def setUp(self): quantity=-1000, ledger=self.subsidy_2.ledger, lms_user_id=STATIC_LMS_USER_ID+1000, + lms_user_email=self.lms_user_email, ) # Create third subsidy with a different enterprise_customer_uuid. Neither test learner nor the test admin @@ -119,6 +128,7 @@ def setUp(self): quantity=-1000, ledger=self.subsidy_3.ledger, lms_user_id=STATIC_LMS_USER_ID+1000, + lms_user_email=self.lms_user_email, ) TransactionFactory( uuid=self.subsidy_3_transaction_2_uuid, @@ -126,6 +136,7 @@ def setUp(self): quantity=-1000, ledger=self.subsidy_3.ledger, lms_user_id=STATIC_LMS_USER_ID+1000, + lms_user_email=self.lms_user_email, ) self.all_initial_transactions = set([ @@ -254,14 +265,18 @@ def test_can_redeem_bad_request(self, query_params): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @mock.patch('enterprise_subsidy.apps.api.v1.views.subsidy.can_redeem') + @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.lms_user_client") @ddt.data(False, True) - def test_can_redeem_happy_path(self, has_existing_transaction, mock_can_redeem): + def test_can_redeem_happy_path(self, has_existing_transaction, mock_lms_user_client, mock_can_redeem): """ Tests that the result of ``api.can_redeem()`` is returned as the response payload for a POST to the can_redeem action, including any relevant existing transaction. """ self.set_up_admin(enterprise_uuids=[self.subsidy_1.enterprise_customer_uuid]) + mock_lms_user_client.return_value.best_effort_user_data.return_value = { + 'email': self.lms_user_email, + } expected_redeemable = True expected_active = True expected_price = 350 @@ -292,8 +307,10 @@ def test_can_redeem_happy_path(self, has_existing_transaction, mock_can_redeem): 'state': TransactionStateChoices.COMMITTED, 'quantity': -1000, 'lms_user_id': STATIC_LMS_USER_ID, + 'lms_user_email': self.lms_user_email, 'subsidy_access_policy_uuid': str(self.subsidy_access_policy_1_uuid), 'content_key': self.content_key_1, + 'content_title': self.content_title_1, 'external_reference': [], 'transaction_status_api_url': f"{self.transaction_status_api_url}/{self.subsidy_1_transaction_1_uuid}/", 'courseware_url': f"http://localhost:2000/course/{self.content_key_1}/home", @@ -959,10 +976,18 @@ def test_retrieve_invalid_uuid(self): @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client") @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content") @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary") - def test_create(self, mock_get_content_summary, mock_price_for_content, mock_enterprise_client): + @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.lms_user_client") + def test_create( + self, + mock_lms_user_client, + mock_get_content_summary, + mock_price_for_content, + mock_enterprise_client, + ): """ Test create Transaction, happy case. """ + mock_lms_user_client.return_value.best_effort_user_data.return_value = {'email': 'edx@example.com'} url = reverse("api:v1:transaction-list") test_enroll_enterprise_fulfillment_uuid = "test-enroll-reference-id" mock_enterprise_client.enroll.return_value = test_enroll_enterprise_fulfillment_uuid @@ -970,6 +995,7 @@ def test_create(self, mock_get_content_summary, mock_price_for_content, mock_ent mock_get_content_summary.return_value = { 'content_uuid': 'course-v1:edX-test-course', 'content_key': 'course-v1:edX-test-course', + 'content_title': 'edX: Test Course', 'source': 'edX', 'mode': 'verified', 'content_price': 10000, @@ -994,7 +1020,9 @@ def test_create(self, mock_get_content_summary, mock_price_for_content, mock_ent f"{self.transaction_status_api_url}/{create_response_data['uuid']}/" ) assert create_response_data["content_key"] == post_data["content_key"] + assert create_response_data["content_title"] == 'edX: Test Course' assert create_response_data["lms_user_id"] == post_data["lms_user_id"] + assert create_response_data["lms_user_email"] == 'edx@example.com' assert create_response_data["subsidy_access_policy_uuid"] == post_data["subsidy_access_policy_uuid"] assert create_response_data["courseware_url"] == f"http://localhost:2000/course/{post_data['content_key']}/home" self.assertDictEqual(create_response_data["metadata"], {}) @@ -1011,6 +1039,7 @@ def test_create(self, mock_get_content_summary, mock_price_for_content, mock_ent retrieve_response_data = retrieve_response.json() assert retrieve_response_data["uuid"] == create_response_data["uuid"] assert retrieve_response_data["idempotency_key"] == create_response_data["idempotency_key"] + # assert retrieve_response_data["content_title"] == 'edX: Test Course' # Uncomment after Segment events are setup: # @@ -1030,10 +1059,18 @@ def test_create(self, mock_get_content_summary, mock_price_for_content, mock_ent @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client") @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content") @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary") - def test_create_with_metadata(self, mock_get_content_summary, mock_price_for_content, mock_enterprise_client): + @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.lms_user_client") + def test_create_with_metadata( + self, + mock_lms_user_client, + mock_get_content_summary, + mock_price_for_content, + mock_enterprise_client + ): """ Test create Transaction, happy case. """ + mock_lms_user_client.return_value.best_effort_user_data.return_value = {'email': 'edx@example.com'} url = reverse("api:v1:transaction-list") test_enroll_enterprise_fulfillment_uuid = "test-enroll-reference-id" mock_enterprise_client.enroll.return_value = test_enroll_enterprise_fulfillment_uuid @@ -1085,10 +1122,27 @@ def test_create_with_metadata(self, mock_get_content_summary, mock_price_for_con @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client") @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content") - def test_create_ledger_locked(self, mock_price_for_content, mock_enterprise_client): + @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary") + @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.lms_user_client") + def test_create_ledger_locked( + self, + mock_lms_user_client, + mock_get_content_summary, + mock_price_for_content, + mock_enterprise_client, + ): """ Test create Transaction, 429 response due to the ledger being locked. """ + mock_get_content_summary.return_value = { + 'content_uuid': 'course-v1:edX-test-course', + 'content_key': 'course-v1:edX-test-course', + 'source': 'edX', + 'mode': 'verified', + 'content_price': 10000, + 'geag_variant_id': None, + } + mock_lms_user_client.return_value.best_effort_user_data.return_value = {'email': 'edx@example.com'} url = reverse("api:v1:transaction-list") test_enroll_enterprise_fulfillment_uuid = "test-enroll-reference-id" mock_enterprise_client.enroll.return_value = test_enroll_enterprise_fulfillment_uuid @@ -1171,29 +1225,40 @@ def test_create_content_not_in_catalog(self, mock_content_metadata_api): @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client") @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.content_metadata_api") @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary") + @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.lms_user_client") def test_create_external_enroll_failed( self, + mock_lms_user_client, mock_get_content_summary, - mock_content_metadata_api, - mock_enterprise_client + mock_subsidy_content_metadata_api, + mock_enterprise_client, ): """ Test create Transaction, 5xx response due to the external enrollment failing. Check that a transaction is created, then rolled back to "failed" state. """ mock_get_content_summary.return_value = { - 'content_uuid': 'course-v1:edX+test+course.enroll.failed', - 'content_key': 'course-v1:edX+test+course.enroll.failed', + 'content_uuid': 'course-v1:edX-test-course', + 'content_key': 'course-v1:edX-test-course', 'source': 'edX', 'mode': 'verified', - 'content_price': 10000, + 'content_price': 100, + 'geag_variant_id': None, + } + mock_subsidy_content_metadata_api().get_content_summary.return_value = { + 'content_uuid': 'course-v1:edX-test-course', + 'content_key': 'course-v1:edX-test-course', + 'source': 'edX', + 'mode': 'verified', + 'content_price': 100, 'geag_variant_id': None, } + mock_subsidy_content_metadata_api().get_course_price.return_value = 100 + mock_lms_user_client.return_value.best_effort_user_data.return_value = {'email': 'edx@example.com'} # Create privileged staff user that should be able to create Transactions. self.set_up_operator() url = reverse("api:v1:transaction-list") mock_enterprise_client.enroll.side_effect = HTTPError() - mock_content_metadata_api().get_course_price.return_value = 100 test_content_key = "course-v1:edX+test+course.enroll.failed" test_lms_user_id = 1234 post_data = { diff --git a/enterprise_subsidy/apps/api/v2/tests/test_transaction_views.py b/enterprise_subsidy/apps/api/v2/tests/test_transaction_views.py index ddad4ff5..c13cfe38 100644 --- a/enterprise_subsidy/apps/api/v2/tests/test_transaction_views.py +++ b/enterprise_subsidy/apps/api/v2/tests/test_transaction_views.py @@ -33,6 +33,7 @@ class APITestBase(APITestMixin): Contains boilerplate to create a couple of subsidies with related ledgers and starting transactions. """ + lms_user_email = 'edx@example.com' enterprise_1_uuid = STATIC_ENTERPRISE_UUID enterprise_2_uuid = str(uuid.uuid4()) @@ -56,6 +57,8 @@ class APITestBase(APITestMixin): content_key_1 = "course-v1:edX+test+course.1" content_key_2 = "course-v1:edX+test+course.2" + content_title_1 = "edX: Test Course 1" + content_title_2 = "edx: Test Course 2" @classmethod def setUpClass(cls): @@ -108,8 +111,10 @@ def _setup_transactions(cls): quantity=-1000, ledger=cls.subsidy_1.ledger, lms_user_id=STATIC_LMS_USER_ID+1000, + lms_user_email=cls.lms_user_email, subsidy_access_policy_uuid=cls.subsidy_access_policy_2_uuid, content_key=cls.content_key_2, + content_title=cls.content_title_2, ) # Also create a reversed transaction, and also include metadata on both the transaction and reversal. cls.subsidy_1_transaction_3 = TransactionFactory( @@ -118,8 +123,10 @@ def _setup_transactions(cls): quantity=-1000, ledger=cls.subsidy_1.ledger, lms_user_id=STATIC_LMS_USER_ID, + lms_user_email=cls.lms_user_email, subsidy_access_policy_uuid=cls.subsidy_access_policy_2_uuid, content_key=cls.content_key_2, + content_title=cls.content_title_2, metadata={"bin": "baz"}, ) cls.subsidy_1_transaction_3_reversal = ReversalFactory( @@ -256,8 +263,10 @@ def setUpClass(cls): quantity=0, ledger=cls.subsidy_1.ledger, lms_user_id=STATIC_LMS_USER_ID, # This is the only transaction belonging to the requester. + lms_user_email=cls.lms_user_email, subsidy_access_policy_uuid=cls.subsidy_access_policy_1_uuid, content_key=cls.content_key_1, + content_title=cls.content_title_1, ) def test_list_transactions_metadata_format(self): @@ -705,8 +714,10 @@ def test_operator_creation_happy_path_transaction_exists(self, mock_price_for_co @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.enterprise_client") @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.price_for_content") @mock.patch("enterprise_subsidy.apps.content_metadata.api.ContentMetadataApi.get_content_summary") + @mock.patch("enterprise_subsidy.apps.subsidy.models.Subsidy.lms_user_client") def test_operator_creation_happy_path_201( self, + mock_lms_user_client, mock_get_content_summary, mock_price_for_content, mock_enterprise_client @@ -717,11 +728,15 @@ def test_operator_creation_happy_path_201( """ self.set_up_operator() + mock_lms_user_client.return_value.best_effort_user_data.return_value = { + 'email': self.lms_user_email, + } mock_enterprise_client.enroll.return_value = 'my-fulfillment-id' mock_price_for_content.return_value = 1000 mock_get_content_summary.return_value = { 'content_uuid': self.content_key_1, 'content_key': self.content_key_1, + 'content_title': self.content_title_1, 'source': 'edX', 'mode': 'verified', 'content_price': 10000, diff --git a/enterprise_subsidy/apps/api_client/lms_user.py b/enterprise_subsidy/apps/api_client/lms_user.py new file mode 100644 index 00000000..4837b579 --- /dev/null +++ b/enterprise_subsidy/apps/api_client/lms_user.py @@ -0,0 +1,81 @@ +""" +LMS User Data api client for the subsidy service. +""" +import logging + +import requests +from django.conf import settings +from django.core.cache import cache + +from enterprise_subsidy.apps.api_client.base_oauth import BaseOAuthClient + +logger = logging.getLogger(__name__) + + +class LmsUserApiClient(BaseOAuthClient): + """ + API client for LMS User Data. + """ + api_base_url = settings.LMS_URL + '/api/user/v1' + accounts_url = api_base_url + '/accounts' + + def user_account_url(self, lms_user_id): + return f"{self.accounts_url}?lms_user_id={lms_user_id}" + + def get_user_data(self, lms_user_id): + """ + Gets the data for an LMS User given their lms user id. + + Arguments: + lms_user_id (int): LMS User Id of a learner + Returns: + response (dict): JSON response data + Raises: + requests.exceptions.HTTPError: if service is down/unavailable or status code comes back >= 300, + the method will log and throw an HTTPError exception. + """ + lms_account_url = self.user_account_url(lms_user_id) + try: + response = self.client.get(lms_account_url) + response.raise_for_status() + data = response.json() + if data: + return data.pop() + else: + return None + except requests.exceptions.HTTPError as exc: + if hasattr(response, 'text'): + logger.error( + f'Failed to fetch user data for {lms_user_id} because {response.text}', + ) + raise exc + + def best_effort_user_data(self, lms_user_id): + """ + Gets the data for an LMS User given their lms user id. + Tries to use cache. + Rescues exceptions + logs without reraising. + + Arguments: + lms_user_id (int): LMS User Id of a learner + Returns: + response (dict): JSON response data + """ + try: + cache_key = 'LmsUserApiClient:lms_user_id:{lms_user_id}'.format(lms_user_id=lms_user_id) + user_data = cache.get(cache_key) + + if not isinstance(user_data, dict): + user_data = self.get_user_data(lms_user_id) + + if not isinstance(user_data, dict): + logger.warning('Received unexpected user_data for lms_user_id %s', lms_user_id) + return None + + cache.set(cache_key, user_data, settings.LMS_USER_DATA_CACHE_TIMEOUT) + return user_data + except requests.exceptions.HTTPError: + logger.exception( + f'Failed best effort attempt to fetch user data for {lms_user_id}', + ) + return None diff --git a/enterprise_subsidy/apps/api_client/tests/test_lms_user.py b/enterprise_subsidy/apps/api_client/tests/test_lms_user.py new file mode 100644 index 00000000..a99c33d2 --- /dev/null +++ b/enterprise_subsidy/apps/api_client/tests/test_lms_user.py @@ -0,0 +1,81 @@ +from unittest import mock + +import ddt +from django.test import TestCase +from requests.exceptions import HTTPError + +from enterprise_subsidy.apps.api_client.lms_user import LmsUserApiClient +from test_utils.utils import MockResponse + + +@ddt.ddt +class LmsUserApiClientTests(TestCase): + """ + Tests for the lms user api client. + """ + @classmethod + def setUpTestData(cls): + super().setUpTestData() + + cls.user_id = 12345 + cls.user_email = 'user@example.com' + + @mock.patch('enterprise_subsidy.apps.api_client.base_oauth.OAuthAPIClient', return_value=mock.MagicMock()) + def test_successful_get_user_data(self, mock_oauth_client): + """ + Test the happy path of getting user data from the lms + """ + mock_oauth_client.return_value.get.return_value = MockResponse( + [{ + "name": "Example User", + "email": "user@example.com", + "id": 12345, + }], + 200, + ) + lms_user_client = LmsUserApiClient() + response = lms_user_client.get_user_data(self.user_id) + assert response.get('id') == self.user_id + mock_oauth_client().get.assert_called_with( + f'{LmsUserApiClient.accounts_url}?lms_user_id={self.user_id}' + ) + + @mock.patch('enterprise_subsidy.apps.api_client.base_oauth.OAuthAPIClient', return_value=mock.MagicMock()) + def test_failed_get_user_data(self, mock_oauth_client): + """ + Test the when something fails getting user data from the lms + """ + mock_oauth_client.return_value.get.return_value = MockResponse(None, 400) + lms_user_client = LmsUserApiClient() + with self.assertRaises(HTTPError): + lms_user_client.get_user_data(self.user_id) + + @mock.patch('enterprise_subsidy.apps.api_client.base_oauth.OAuthAPIClient', return_value=mock.MagicMock()) + def test_successful_best_effort_user_data(self, mock_oauth_client): + """ + Test the happy path of the best effort version + """ + mock_oauth_client.return_value.get.return_value = MockResponse( + [{ + "name": "Example User", + "email": "user@example.com", + "id": 12345, + }], + 200, + ) + lms_user_client = LmsUserApiClient() + response = lms_user_client.best_effort_user_data(self.user_id) + assert response.get('id') == self.user_id + mock_oauth_client().get.assert_called_with( + f'{LmsUserApiClient.accounts_url}?lms_user_id={self.user_id}' + ) + + @mock.patch('enterprise_subsidy.apps.api_client.base_oauth.OAuthAPIClient', return_value=mock.MagicMock()) + def test_failed_best_effort_user_data(self, mock_oauth_client): + """ + Test the that the best effort version fails without exception + """ + mock_oauth_client.return_value.get.return_value = MockResponse(None, 400) + lms_user_client = LmsUserApiClient() + response = lms_user_client.best_effort_user_data(self.user_id) + assert response is None diff --git a/enterprise_subsidy/apps/subsidy/models.py b/enterprise_subsidy/apps/subsidy/models.py index b004b498..2ae374a8 100644 --- a/enterprise_subsidy/apps/subsidy/models.py +++ b/enterprise_subsidy/apps/subsidy/models.py @@ -27,6 +27,7 @@ from simple_history.models import HistoricalRecords from enterprise_subsidy.apps.api_client.enterprise import EnterpriseApiClient +from enterprise_subsidy.apps.api_client.lms_user import LmsUserApiClient from enterprise_subsidy.apps.content_metadata.api import ContentMetadataApi from enterprise_subsidy.apps.core.utils import localized_utcnow from enterprise_subsidy.apps.fulfillment.api import GEAGFulfillmentHandler @@ -270,6 +271,43 @@ def geag_fulfillment_handler(self): """ return GEAGFulfillmentHandler() + def lms_user_client(self): + """ + API layer for interacting with the LMS Account API + """ + return LmsUserApiClient() + + def email_for_learner(self, lms_user_id): + """ + Return the email associated with an LMS learner + """ + user_data = self.lms_user_client().best_effort_user_data(lms_user_id) + if isinstance(user_data, dict): + return user_data.get('email') + return None + + def title_for_content(self, content_key): + """ + Best effort return the title of the given content. + + Returns: + string: Title of content or None. + """ + content_title = None + try: + content_summary = self.content_metadata_api().get_content_summary( + self.enterprise_customer_uuid, + content_key + ) + if content_summary: + content_title = content_summary.get('content_title') + except HTTPError as exc: + if exc.response.status_code == status.HTTP_404_NOT_FOUND: + raise ContentNotFoundForCustomerException( + 'The given content_key is not in any catalog for this customer.' + ) from exc + return content_title + def price_for_content(self, content_key): """ Return the price of the given content in USD Cents. @@ -298,7 +336,9 @@ def create_transaction( idempotency_key, quantity, lms_user_id=None, + lms_user_email=None, content_key=None, + content_title=None, subsidy_access_policy_uuid=None, **transaction_metadata ): @@ -316,7 +356,9 @@ def create_transaction( quantity=quantity, idempotency_key=idempotency_key, lms_user_id=lms_user_id, + lms_user_email=lms_user_email, content_key=content_key, + content_title=content_title, subsidy_access_policy_uuid=subsidy_access_policy_uuid, **transaction_metadata, ) @@ -383,10 +425,14 @@ def redeem(self, lms_user_id, content_key, subsidy_access_policy_uuid, idempoten logger.info(base_exception_msg, 'Not enough balance in the subsidy') return (None, False) try: + lms_user_email = self.email_for_learner(lms_user_id) + content_title = self.title_for_content(content_key) transaction = self._create_redemption( lms_user_id, content_key, subsidy_access_policy_uuid, + lms_user_email=lms_user_email, + content_title=content_title, idempotency_key=idempotency_key, metadata=metadata, ) @@ -414,6 +460,8 @@ def _create_redemption( lms_user_id, content_key, subsidy_access_policy_uuid, + content_title=None, + lms_user_email=None, idempotency_key=None, metadata=None ): @@ -465,7 +513,9 @@ def _create_redemption( idempotency_key, quantity, content_key=content_key, + content_title=content_title, lms_user_id=lms_user_id, + lms_user_email=lms_user_email, subsidy_access_policy_uuid=subsidy_access_policy_uuid, **tx_metadata, ) diff --git a/enterprise_subsidy/apps/subsidy/tests/test_models.py b/enterprise_subsidy/apps/subsidy/tests/test_models.py index 0cca4d03..17714daf 100644 --- a/enterprise_subsidy/apps/subsidy/tests/test_models.py +++ b/enterprise_subsidy/apps/subsidy/tests/test_models.py @@ -198,6 +198,8 @@ def setUp(self): self.subsidy = SubsidyFactory.create( enterprise_customer_uuid=self.enterprise_customer_uuid, ) + self.subsidy.lms_user_client = mock.MagicMock() + self.subsidy.lms_user_client.return_value.best_effort_user_data.return_value = {'email': 'edx@example.com'} super().setUp() def test_get_committed_transaction_no_reversal(self): @@ -300,6 +302,7 @@ def test_redeem_with_metadata(self, mock_get_content_summary, mock_enterprise_cl mock_get_content_summary.return_value = { 'content_uuid': 'course-v1:edX+test+course', 'content_key': 'course-v1:edX+test+course', + 'content_title': 'edx: Test Course', 'source': 'edX', 'mode': 'verified', 'content_price': 10000, diff --git a/enterprise_subsidy/settings/base.py b/enterprise_subsidy/settings/base.py index 5ded0a9c..c020dfaa 100644 --- a/enterprise_subsidy/settings/base.py +++ b/enterprise_subsidy/settings/base.py @@ -364,3 +364,8 @@ def root(*path_fragments): # disable indexing on history_date SIMPLE_HISTORY_DATE_INDEX = False + + +# How long we keep API Client data in cache. (seconds) +ONE_HOUR = 60 * 60 +LMS_USER_DATA_CACHE_TIMEOUT = ONE_HOUR diff --git a/requirements/base.txt b/requirements/base.txt index fc68866a..4f8dab16 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -173,7 +173,7 @@ openedx-events==8.6.0 # via # -r requirements/base.in # openedx-ledger -openedx-ledger==1.2.0 +openedx-ledger==1.3.2 # via -r requirements/base.in packaging==23.2 # via drf-yasg diff --git a/requirements/dev.txt b/requirements/dev.txt index deb9932c..975b79d7 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -344,7 +344,7 @@ openedx-events==8.6.0 # via # -r requirements/validation.txt # openedx-ledger -openedx-ledger==1.2.0 +openedx-ledger==1.3.2 # via -r requirements/validation.txt packaging==23.2 # via diff --git a/requirements/doc.txt b/requirements/doc.txt index 629db544..385bf157 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -331,7 +331,7 @@ openedx-events==8.6.0 # via # -r requirements/test.txt # openedx-ledger -openedx-ledger==1.2.0 +openedx-ledger==1.3.2 # via -r requirements/test.txt packaging==23.2 # via diff --git a/requirements/production.txt b/requirements/production.txt index 398a7920..334db2d4 100644 --- a/requirements/production.txt +++ b/requirements/production.txt @@ -213,7 +213,7 @@ openedx-events==8.6.0 # via # -r requirements/base.txt # openedx-ledger -openedx-ledger==1.2.0 +openedx-ledger==1.3.2 # via -r requirements/base.txt packaging==23.2 # via diff --git a/requirements/quality.txt b/requirements/quality.txt index c6c83222..ada9b80a 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -310,7 +310,7 @@ openedx-events==8.6.0 # via # -r requirements/test.txt # openedx-ledger -openedx-ledger==1.2.0 +openedx-ledger==1.3.2 # via -r requirements/test.txt packaging==23.2 # via diff --git a/requirements/test.txt b/requirements/test.txt index 44964162..bf41fc90 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -258,7 +258,7 @@ openedx-events==8.6.0 # via # -r requirements/base.txt # openedx-ledger -openedx-ledger==1.2.0 +openedx-ledger==1.3.2 # via -r requirements/base.txt packaging==23.2 # via diff --git a/requirements/validation.txt b/requirements/validation.txt index 6b6153d6..b4371938 100644 --- a/requirements/validation.txt +++ b/requirements/validation.txt @@ -399,7 +399,7 @@ openedx-events==8.6.0 # -r requirements/quality.txt # -r requirements/test.txt # openedx-ledger -openedx-ledger==1.2.0 +openedx-ledger==1.3.2 # via # -r requirements/quality.txt # -r requirements/test.txt