From 321f1035193bbf72b34dab209d229be81924a6ae Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Wed, 30 Oct 2024 17:22:07 -0400 Subject: [PATCH 1/4] feat: introduce LmsUserApiClient with method to fetch enrollment intentions for learner --- .../apps/api_client/base_user.py | 7 + .../apps/api_client/license_manager_client.py | 1 + .../apps/api_client/lms_client.py | 37 +++++ .../tests/test_license_manager_client.py | 15 +- .../apps/api_client/tests/test_lms_client.py | 133 +++++++++++++++++- 5 files changed, 182 insertions(+), 11 deletions(-) diff --git a/enterprise_access/apps/api_client/base_user.py b/enterprise_access/apps/api_client/base_user.py index b420d6f2..7c41f11d 100644 --- a/enterprise_access/apps/api_client/base_user.py +++ b/enterprise_access/apps/api_client/base_user.py @@ -44,6 +44,13 @@ def __init__(self, original_request, **kwargs): if self.headers.get(settings.REQUEST_ID_RESPONSE_HEADER) is None and request_id is not None: self.headers[settings.REQUEST_ID_RESPONSE_HEADER] = request_id + @property + def request_user(self): + """ + Returns the user associated with the original request. + """ + return self.original_request.user + def request(self, method, url, headers=None, **kwargs): # pylint: disable=arguments-differ if headers: headers.update(self.headers) diff --git a/enterprise_access/apps/api_client/license_manager_client.py b/enterprise_access/apps/api_client/license_manager_client.py index 29450c15..b9b90372 100644 --- a/enterprise_access/apps/api_client/license_manager_client.py +++ b/enterprise_access/apps/api_client/license_manager_client.py @@ -93,6 +93,7 @@ def get_subscription_licenses_for_learner(self, enterprise_customer_uuid): url = self.learner_licenses_endpoint try: response = self.get(url, params=query_params, timeout=settings.LICENSE_MANAGER_CLIENT_TIMEOUT) + response.raise_for_status() return response.json() except requests.exceptions.HTTPError as exc: logger.exception(f"Failed to get subscription licenses for learner: {exc}") diff --git a/enterprise_access/apps/api_client/lms_client.py b/enterprise_access/apps/api_client/lms_client.py index f3638c7e..419cac2d 100755 --- a/enterprise_access/apps/api_client/lms_client.py +++ b/enterprise_access/apps/api_client/lms_client.py @@ -10,6 +10,7 @@ from rest_framework import status from enterprise_access.apps.api_client.base_oauth import BaseOAuthClient +from enterprise_access.apps.api_client.base_user import BaseUserApiClient from enterprise_access.apps.api_client.exceptions import FetchGroupMembersConflictingParamsException from enterprise_access.apps.enterprise_groups.constants import GROUP_MEMBERSHIP_EMAIL_ERROR_STATUS from enterprise_access.cache_utils import versioned_cache_key @@ -396,3 +397,39 @@ def update_pending_learner_status(self, enterprise_group_uuid, learner_email): except KeyError: logger.exception('failed to update group membership status. [%s]', url) return None + + +class LmsUserApiClient(BaseUserApiClient): + """ + API client for user-specific calls to the LMS service. + """ + enterprise_api_base_url = f"{settings.LMS_URL}/enterprise/api/v1/" + default_enterprise_enrollment_intentions_learner_status_endpoint = ( + f'{enterprise_api_base_url}default-enterprise-enrollment-intentions/learner-status/' + ) + + def get_default_enterprise_enrollment_intentions_learner_status(self, enterprise_customer_uuid): + """ + Fetches learner status from the default enterprise enrollment intentions endpoint. + + Arguments: + enterprise_customer_uuid (str): UUID of the enterprise customer + + Returns: + dict: Dictionary representation of the JSON response from the API + """ + query_params = {'enterprise_customer_uuid': enterprise_customer_uuid} + try: + response = self.get( + self.default_enterprise_enrollment_intentions_learner_status_endpoint, + params=query_params, + timeout=settings.LMS_CLIENT_TIMEOUT + ) + response.raise_for_status() + return response.json() + except requests.exceptions.HTTPError as exc: + logger.exception( + f"Failed to fetch default enterprise enrollment intentions for enterprise customer " + f"{enterprise_customer_uuid} and learner {self.request_user.lms_user_id}: {exc}" + ) + raise diff --git a/enterprise_access/apps/api_client/tests/test_license_manager_client.py b/enterprise_access/apps/api_client/tests/test_license_manager_client.py index e0082419..a95a73c2 100644 --- a/enterprise_access/apps/api_client/tests/test_license_manager_client.py +++ b/enterprise_access/apps/api_client/tests/test_license_manager_client.py @@ -179,9 +179,7 @@ def test_get_subscription_licenses_for_learner(self, mock_crum_get_current_reque "request": request } - mock_request = mock.MagicMock() - mock_request.headers.get.return_value = request.headers.get(self.request_id_key) - mock_crum_get_current_request.return_value = mock_request + mock_crum_get_current_request.return_value = request mock_response = mock.Mock() mock_response.status_code = 200 @@ -207,12 +205,15 @@ def test_get_subscription_licenses_for_learner(self, mock_crum_get_current_reque expected_params = {'enterprise_customer_uuid': [self.mock_enterprise_customer_uuid]} self.assertEqual(parsed_params, expected_params) + # Assert headers are correctly set self.assertEqual(prepared_request.headers['Authorization'], 'test-auth') self.assertEqual(prepared_request.headers[self.request_id_key], 'test-request-id') + # Assert timeout is set self.assertIn('timeout', prepared_request_kwargs) self.assertEqual(prepared_request_kwargs['timeout'], settings.LICENSE_MANAGER_CLIENT_TIMEOUT) + # Assert result is as expected self.assertEqual(result, expected_result) @mock.patch('requests.Session.send') @@ -230,9 +231,7 @@ def test_activate_license(self, mock_crum_get_current_request, mock_send): "request": request } - mock_request = mock.MagicMock() - mock_request.headers.get.return_value = request.headers.get(self.request_id_key) - mock_crum_get_current_request.return_value = mock_request + mock_crum_get_current_request.return_value = request mock_response = mock.Mock() mock_response.status_code = 200 @@ -281,9 +280,7 @@ def test_auto_apply_license(self, mock_crum_get_current_request, mock_send): "request": request } - mock_request = mock.MagicMock() - mock_request.headers.get.return_value = request.headers.get(self.request_id_key) - mock_crum_get_current_request.return_value = mock_request + mock_crum_get_current_request.return_value = request mock_response = mock.Mock() mock_response.status_code = 200 diff --git a/enterprise_access/apps/api_client/tests/test_lms_client.py b/enterprise_access/apps/api_client/tests/test_lms_client.py index 7f6df280..0ad85a19 100644 --- a/enterprise_access/apps/api_client/tests/test_lms_client.py +++ b/enterprise_access/apps/api_client/tests/test_lms_client.py @@ -3,16 +3,19 @@ """ from datetime import datetime, timedelta from unittest import mock +from urllib.parse import parse_qs, urlparse from uuid import uuid4 import ddt import requests from django.conf import settings -from django.test import TestCase +from django.test import RequestFactory, TestCase +from faker import Faker from rest_framework import status -from enterprise_access.apps.api_client.lms_client import LmsApiClient +from enterprise_access.apps.api_client.lms_client import LmsApiClient, LmsUserApiClient from enterprise_access.apps.api_client.tests.test_utils import MockResponse +from enterprise_access.apps.core.tests.factories import UserFactory from test_utils import TEST_ENTERPRISE_UUID, TEST_USER_ID, TEST_USER_RECORD TEST_USER_EMAILS = [ @@ -294,3 +297,129 @@ def test_get_pending_enterprise_group_memberships(self, mock_oauth_client, mock_ timeout=settings.LMS_CLIENT_TIMEOUT ) assert pending_enterprise_group_memberships == expected_return + + +class TestLmsUserApiClient(TestCase): + """ + Test LmsUserApiClient. + """ + + def setUp(self): + super().setUp() + self.factory = RequestFactory() + self.faker = Faker() + self.request_id_key = settings.REQUEST_ID_RESPONSE_HEADER + + self.user = UserFactory() + + self.mock_enterprise_customer_uuid = self.faker.uuid4() + self.mock_course_key = 'edX+DemoX' + self.mock_course_run_key = 'course-v1:edX+DemoX+Demo_Course' + self.mock_enterprise_catalog_uuid = self.faker.uuid4() + + self.mock_default_enterprise_enrollment_intentions_learner_status = { + "uuid": self.faker.uuid4(), + "content_key": self.mock_course_key, + "enterprise_customer": self.mock_enterprise_customer_uuid, + "course_key": self.mock_course_key, + "course_run_key": self.mock_course_run_key, + "is_course_run_enrollable": True, + "best_mode_for_course_run": "verified", + "applicable_enterprise_catalog_uuids": [ + self.mock_enterprise_catalog_uuid + ], + "course_run_normalized_metadata": { + "start_date": "2024-09-17T14:00:00Z", + "end_date": "2025-09-15T22:30:00Z", + "enroll_by_date": "2025-09-05T23:59:59Z", + "enroll_start_date": "2024-08-17T14:00:00Z", + "content_price": 49 + }, + "created": "2024-10-25T13:20:04.082376Z", + "modified": "2024-10-29T22:04:56.731518Z", + "has_existing_enrollment": False, + "is_existing_enrollment_active": None, + "is_existing_enrollment_audit": None + } + + @mock.patch('requests.Session.send') + @mock.patch('crum.get_current_request') + def test_get_default_enterprise_enrollment_intentions_learner_status( + self, + mock_crum_get_current_request, + mock_send, + ): + """ + Verify client hits the right URL for default enterprise enrollment intentions learner status. + """ + expected_url = LmsUserApiClient.default_enterprise_enrollment_intentions_learner_status_endpoint + request = self.factory.get(expected_url) + request.headers = { + "Authorization": 'test-auth', + self.request_id_key: 'test-request-id' + } + request.user = self.user + context = { + "request": request + } + + mock_crum_get_current_request.return_value = request + + expected_result = { + "lms_user_id": self.user.id, + "user_email": self.user.email, + "enterprise_customer_uuid": self.mock_enterprise_customer_uuid, + "enrollment_statuses": { + "needs_enrollment": { + "enrollable": [ + self.mock_default_enterprise_enrollment_intentions_learner_status + ], + "not_enrollable": [] + }, + "already_enrolled": [] + }, + "metadata": { + "total_default_enterprise_enrollment_intentions": 1, + "total_needs_enrollment": { + "enrollable": 1, + "not_enrollable": 0 + }, + "total_already_enrolled": 0 + } + } + mock_response = mock.Mock() + mock_response.status_code = 200 + mock_response.json.return_value = expected_result + + mock_send.return_value = mock_response + + client = LmsUserApiClient(context['request']) + result = client.get_default_enterprise_enrollment_intentions_learner_status( + self.mock_enterprise_customer_uuid + ) + + mock_send.assert_called_once() + + prepared_request = mock_send.call_args[0][0] + prepared_request_kwargs = mock_send.call_args[1] + + # Assert base request URL/method is correct + parsed_url = urlparse(prepared_request.url) + self.assertEqual(f"{parsed_url.scheme}://{parsed_url.netloc}{parsed_url.path}", expected_url) + self.assertEqual(prepared_request.method, 'GET') + + # Assert query parameters are correctly set + parsed_params = parse_qs(parsed_url.query) + expected_params = {'enterprise_customer_uuid': [self.mock_enterprise_customer_uuid]} + self.assertEqual(parsed_params, expected_params) + + # Assert headers are correctly set + self.assertEqual(prepared_request.headers['Authorization'], 'test-auth') + self.assertEqual(prepared_request.headers[self.request_id_key], 'test-request-id') + + # Assert timeout is set + self.assertIn('timeout', prepared_request_kwargs) + self.assertEqual(prepared_request_kwargs['timeout'], settings.LICENSE_MANAGER_CLIENT_TIMEOUT) + + # Assert the response is as expected + self.assertEqual(result, expected_result) From c5eb2701cd223480f6e9167bf5c4ed643e033834 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Thu, 31 Oct 2024 08:28:22 -0400 Subject: [PATCH 2/4] chore: test coverage for httperror exception --- .../apps/api_client/tests/test_lms_client.py | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/enterprise_access/apps/api_client/tests/test_lms_client.py b/enterprise_access/apps/api_client/tests/test_lms_client.py index 0ad85a19..58fb295d 100644 --- a/enterprise_access/apps/api_client/tests/test_lms_client.py +++ b/enterprise_access/apps/api_client/tests/test_lms_client.py @@ -423,3 +423,50 @@ def test_get_default_enterprise_enrollment_intentions_learner_status( # Assert the response is as expected self.assertEqual(result, expected_result) + + @mock.patch('requests.Session.send') + @mock.patch('crum.get_current_request') + @mock.patch('enterprise_access.apps.api_client.lms_client.logger', return_value=mock.MagicMock()) + def test_get_default_enterprise_enrollment_intentions_learner_status_http_error( + self, + mock_logger, + mock_crum_get_current_request, + mock_send, + ): + """ + Verify client raises HTTPError on non-200 response. + """ + expected_url = LmsUserApiClient.default_enterprise_enrollment_intentions_learner_status_endpoint + request = self.factory.get(expected_url) + request.headers = { + "Authorization": 'test-auth', + self.request_id_key: 'test-request-id' + } + request.user = self.user + context = { + "request": request + } + + mock_crum_get_current_request.return_value = request + + mock_response = mock.Mock() + mock_response.status_code = 400 + mock_response.json.return_value = {'detail': 'Bad Request'} + mock_response.raise_for_status.side_effect = requests.exceptions.HTTPError("HTTPError") + + mock_send.return_value = mock_response + + client = LmsUserApiClient(context['request']) + + with self.assertRaises(requests.exceptions.HTTPError): + client.get_default_enterprise_enrollment_intentions_learner_status( + self.mock_enterprise_customer_uuid + ) + + mock_send.assert_called_once() + + # Verify that logger.exception was called with the expected message + mock_logger.exception.assert_called_once_with( + f"Failed to fetch default enterprise enrollment intentions for enterprise customer " + f"{self.mock_enterprise_customer_uuid} and learner {self.user.lms_user_id}: HTTPError" + ) \ No newline at end of file From 43f76b82c70e167f9b85aa2cab3c0d1301acedc6 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Thu, 31 Oct 2024 15:07:46 -0400 Subject: [PATCH 3/4] fix: add response.content to exception message in LmsUserApiClient --- enterprise_access/apps/api_client/lms_client.py | 3 ++- enterprise_access/apps/api_client/tests/test_lms_client.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/enterprise_access/apps/api_client/lms_client.py b/enterprise_access/apps/api_client/lms_client.py index 419cac2d..19394b43 100755 --- a/enterprise_access/apps/api_client/lms_client.py +++ b/enterprise_access/apps/api_client/lms_client.py @@ -430,6 +430,7 @@ def get_default_enterprise_enrollment_intentions_learner_status(self, enterprise except requests.exceptions.HTTPError as exc: logger.exception( f"Failed to fetch default enterprise enrollment intentions for enterprise customer " - f"{enterprise_customer_uuid} and learner {self.request_user.lms_user_id}: {exc}" + f"{enterprise_customer_uuid} and learner {self.request_user.lms_user_id}: {exc} " + f"Response content: {response.content}" ) raise diff --git a/enterprise_access/apps/api_client/tests/test_lms_client.py b/enterprise_access/apps/api_client/tests/test_lms_client.py index 58fb295d..6ecee2c8 100644 --- a/enterprise_access/apps/api_client/tests/test_lms_client.py +++ b/enterprise_access/apps/api_client/tests/test_lms_client.py @@ -468,5 +468,6 @@ def test_get_default_enterprise_enrollment_intentions_learner_status_http_error( # Verify that logger.exception was called with the expected message mock_logger.exception.assert_called_once_with( f"Failed to fetch default enterprise enrollment intentions for enterprise customer " - f"{self.mock_enterprise_customer_uuid} and learner {self.user.lms_user_id}: HTTPError" - ) \ No newline at end of file + f"{self.mock_enterprise_customer_uuid} and learner {self.user.lms_user_id}: HTTPError " + f"Response content: {mock_response.content}" + ) From 734a90f0c09745f9a375768a593c9a8719c5e382 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Thu, 31 Oct 2024 15:09:55 -0400 Subject: [PATCH 4/4] chore: handle null response in exception --- enterprise_access/apps/api_client/lms_client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/enterprise_access/apps/api_client/lms_client.py b/enterprise_access/apps/api_client/lms_client.py index 19394b43..368069f9 100755 --- a/enterprise_access/apps/api_client/lms_client.py +++ b/enterprise_access/apps/api_client/lms_client.py @@ -419,6 +419,7 @@ def get_default_enterprise_enrollment_intentions_learner_status(self, enterprise dict: Dictionary representation of the JSON response from the API """ query_params = {'enterprise_customer_uuid': enterprise_customer_uuid} + response = None try: response = self.get( self.default_enterprise_enrollment_intentions_learner_status_endpoint, @@ -431,6 +432,6 @@ def get_default_enterprise_enrollment_intentions_learner_status(self, enterprise logger.exception( f"Failed to fetch default enterprise enrollment intentions for enterprise customer " f"{enterprise_customer_uuid} and learner {self.request_user.lms_user_id}: {exc} " - f"Response content: {response.content}" + f"Response content: {response.content if response else None}" ) raise