From 1f8a2f8ef79c9d0875ebd522e006457dd87263a4 Mon Sep 17 00:00:00 2001 From: Andres Felipe Giraldo Malagon Date: Thu, 13 Feb 2025 17:18:56 -0500 Subject: [PATCH] fix: incorrect parameters at pearson engine action --- eox_nelp/pearson_vue/tasks.py | 15 +--- eox_nelp/pearson_vue/tests/test_tasks.py | 74 +++++++++++++++--- eox_nelp/pearson_vue/tests/test_utils.py | 59 +++++++++++++- eox_nelp/pearson_vue/utils.py | 98 ++++++++++++++++-------- 4 files changed, 188 insertions(+), 58 deletions(-) diff --git a/eox_nelp/pearson_vue/tasks.py b/eox_nelp/pearson_vue/tasks.py index 6a99fe52..5d7ce613 100644 --- a/eox_nelp/pearson_vue/tasks.py +++ b/eox_nelp/pearson_vue/tasks.py @@ -17,12 +17,7 @@ ExamAuthorizationDataImport, RealTimeImport, ) -from eox_nelp.pearson_vue.utils import ( - _get_exam_data, - _get_platform_data, - _get_user_data, - update_user_engines -) +from eox_nelp.pearson_vue.utils import filter_action_parameters, update_user_engines User = get_user_model() @@ -138,12 +133,8 @@ def audit_pearson_engine_action(user_id, exam_id, action_key, **kwargs): user = User.objects.get(id=user_id) update_user_engines(user, action_name, exam_id) action = getattr(PearsonEngineApiClient(), action_key) - response = action( - user_data= _get_user_data(user), - exam_data=_get_exam_data(exam_id), - platform_data=_get_platform_data(), - **kwargs - ) + parameters = filter_action_parameters(action_name, user, exam_id) + response = action(**parameters, **kwargs) if response.get("error"): raise Exception(response.get("message", "Unknown error")) # pylint: disable=broad-exception-raised diff --git a/eox_nelp/pearson_vue/tests/test_tasks.py b/eox_nelp/pearson_vue/tests/test_tasks.py index 20315a1c..38371c30 100644 --- a/eox_nelp/pearson_vue/tests/test_tasks.py +++ b/eox_nelp/pearson_vue/tests/test_tasks.py @@ -119,64 +119,97 @@ def setUp(self): self.kwargs = {"extra_info": "test"} @patch("eox_nelp.pearson_vue.tasks.update_user_engines") + @patch("eox_nelp.pearson_vue.tasks.filter_action_parameters") @patch("eox_nelp.pearson_vue.tasks.PearsonEngineApiClient") - def test_real_time_import_rti(self, mock_api_client, update_user_engines_mock): + def test_real_time_import_rti( + self, + mock_api_client, + filter_action_parameters_mock, + update_user_engines_mock + ): """Test real-time import action using the Pearson Engine API. Expected behavior: - update_user_engines is called with correct parameters. - The real_time_import method is called with the correct parameters. """ + rti_parameters = { + "user_data": "test", + "platform_data": "test", + "exam_data": "test", + } mock_action = MagicMock() mock_action.return_value = {"error": False} mock_api_client.return_value = MagicMock(**{"real_time_import": mock_action}) + filter_action_parameters_mock.return_value = rti_parameters action_name = "rti" real_time_import_task_v2(self.user.id, action_name=action_name, **self.kwargs) update_user_engines_mock.assert_called_once_with(self.user, action_name, None) - mock_action.assert_called_once_with(user=self.user, exam_id=None, **self.kwargs) + mock_action.assert_called_once_with(**rti_parameters, **self.kwargs) @patch("eox_nelp.pearson_vue.tasks.update_user_engines") + @patch("eox_nelp.pearson_vue.tasks.filter_action_parameters") @patch("eox_nelp.pearson_vue.tasks.PearsonEngineApiClient") - def test_real_time_import_cdd(self, mock_api_client, update_user_engines_mock): + def test_real_time_import_cdd( + self, + mock_api_client, + filter_action_parameters_mock, + update_user_engines_mock + ): """Test candidate demographics import action using the Pearson Engine API. Expected behavior: - update_user_engines is called with correct parameters. - The import_candidate_demographics method is called with the correct parameters. """ + cdd_parameters = {"user_data": "test", "platform_data": "test"} mock_action = MagicMock() mock_action.return_value = {"error": False} mock_api_client.return_value = MagicMock(**{"import_candidate_demographics": mock_action}) + filter_action_parameters_mock.return_value = cdd_parameters action_name = "cdd" real_time_import_task_v2(self.user.id, action_name=action_name, **self.kwargs) update_user_engines_mock.assert_called_once_with(self.user, action_name, None) - mock_action.assert_called_once_with(user=self.user, exam_id=None, **self.kwargs) + mock_action.assert_called_once_with(**cdd_parameters, **self.kwargs) @patch("eox_nelp.pearson_vue.tasks.update_user_engines") + @patch("eox_nelp.pearson_vue.tasks.filter_action_parameters") @patch("eox_nelp.pearson_vue.tasks.PearsonEngineApiClient") - def test_real_time_import_ead(self, mock_api_client, update_user_engines_mock): + def test_real_time_import_ead( + self, + mock_api_client, + filter_action_parameters_mock, + update_user_engines_mock + ): """Test exam authorization import action using the Pearson Engine API. Expected behavior: - update_user_engines is called with correct parameters. - The import_exam_authorization method is called with the correct parameters. """ + ead_parameters = {"user_data": "test", "exam_data": "test"} mock_action = MagicMock() mock_action.return_value = {"error": False} mock_api_client.return_value = MagicMock(**{"import_exam_authorization": mock_action}) + filter_action_parameters_mock.return_value = ead_parameters action_name = "ead" real_time_import_task_v2(self.user.id, exam_id=self.exam_id, action_name=action_name, **self.kwargs) update_user_engines_mock.assert_called_once_with(self.user, action_name, self.exam_id,) - mock_action.assert_called_once_with(user=self.user, exam_id=self.exam_id, **self.kwargs) + mock_action.assert_called_once_with(**ead_parameters, **self.kwargs) @patch("eox_nelp.pearson_vue.tasks.update_user_engines") - def test_real_time_import_invalid_action(self, update_user_engines_mock): + @patch("eox_nelp.pearson_vue.tasks.filter_action_parameters") + def test_real_time_import_invalid_action( + self, + filter_action_parameters_mock, + update_user_engines_mock + ): """Test that a KeyError is raised for an invalid action name. Expected behavior: @@ -186,10 +219,17 @@ def test_real_time_import_invalid_action(self, update_user_engines_mock): with self.assertRaises(KeyError): real_time_import_task_v2(self.user.id, action_name="invalid_action") update_user_engines_mock.assert_not_called() + filter_action_parameters_mock.assert_not_called() @patch("eox_nelp.pearson_vue.tasks.update_user_engines") + @patch("eox_nelp.pearson_vue.tasks.filter_action_parameters") @patch('eox_nelp.pearson_vue.tasks.PearsonEngineApiClient') - def test_real_time_import_user_not_found(self, mock_api_client, update_user_engines_mock): + def test_real_time_import_user_not_found( + self, + mock_api_client, + filter_action_parameters_mock, + update_user_engines_mock + ): """Test that a DoesNotExist is raised for an invalid user id. Expected behavior: @@ -201,10 +241,17 @@ def test_real_time_import_user_not_found(self, mock_api_client, update_user_engi real_time_import_task_v2(12345678, action_name="rti") mock_api_client.assert_not_called() update_user_engines_mock.assert_not_called() + filter_action_parameters_mock.assert_not_called() @patch("eox_nelp.pearson_vue.tasks.update_user_engines") + @patch("eox_nelp.pearson_vue.tasks.filter_action_parameters") @patch("eox_nelp.pearson_vue.tasks.PearsonEngineApiClient") - def test_raise_exception_on_error_response(self, mock_api_client, update_user_engines_mock): + def test_raise_exception_on_error_response( + self, + mock_api_client, + filter_action_parameters_mock, + update_user_engines_mock + ): """Test that an exception is raised when the API response contains an Error. Expected behavior: @@ -213,6 +260,11 @@ def test_raise_exception_on_error_response(self, mock_api_client, update_user_en - The action method is called with the correct parameters. - Exception contains the expected message. """ + rti_parameters = { + "user_data": "test", + "platform_data": "test", + "exam_data": "test", + } mock_action = MagicMock() expected_message = "Timeout error" mock_action.return_value = { @@ -221,10 +273,10 @@ def test_raise_exception_on_error_response(self, mock_api_client, update_user_en } action_name = "rti" mock_api_client.return_value = MagicMock(**{"real_time_import": mock_action}) - + filter_action_parameters_mock.return_value = rti_parameters with self.assertRaises(Exception) as context: real_time_import_task_v2(self.user.id, action_name=action_name, **self.kwargs) update_user_engines_mock.assert_called_once_with(self.user, action_name, None) - mock_action.assert_called_once_with(user=self.user, exam_id=None, **self.kwargs) + mock_action.assert_called_once_with(**rti_parameters, **self.kwargs) self.assertEqual(expected_message, str(context.exception)) diff --git a/eox_nelp/pearson_vue/tests/test_utils.py b/eox_nelp/pearson_vue/tests/test_utils.py index 23e89ba1..0254349a 100644 --- a/eox_nelp/pearson_vue/tests/test_utils.py +++ b/eox_nelp/pearson_vue/tests/test_utils.py @@ -4,7 +4,7 @@ UpdatePayloadCddRequestTestCase: Tests cases for update_xml_with_dict using payload with cdd request tag cases. UpdatePayloadEadRequestTestCase: Test cased for update_xml_with_dict using payload with ead request tag cases. """ -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import xmltodict from bs4 import BeautifulSoup @@ -15,6 +15,7 @@ from eox_nelp.pearson_vue.constants import PAYLOAD_CDD, PAYLOAD_EAD from eox_nelp.pearson_vue.models import PearsonEngine from eox_nelp.pearson_vue.utils import ( + filter_action_parameters, generate_client_authorization_id, is_cp1252, update_user_engines, @@ -564,3 +565,59 @@ def test_does_not_increment_course_value_for_other_actions(self): self.assertEqual(user.pearsonengine.ead_triggers, 0) self.assertEqual(user.pearsonengine.rti_triggers, 0) self.assertDictEqual(user.pearsonengine.courses, {}) + + +class TestFilterActionParameters(TestCase): + """Class to test the filter_action_parameters function.""" + def setUp(self): + """Set up the test environment for the test cases.""" + + self.get_user_data_patcher = patch( + "eox_nelp.pearson_vue.utils.get_user_data", + return_value={"user_data": "mock"}, + ) + self.get_platform_data_patcher = patch( + "eox_nelp.pearson_vue.utils.get_platform_data", + return_value={"platform_data": "mock"}, + ) + self.get_exam_data_patcher = patch( + "eox_nelp.pearson_vue.utils.get_exam_data", + return_value={"exam_data": "mock"}, + ) + + self.mock_get_user_data = self.get_user_data_patcher.start() + self.mock_get_platform_data = self.get_platform_data_patcher.start() + self.mock_get_exam_data = self.get_exam_data_patcher.start() + + def tearDown(self): + """Restore the state of the mocks.""" + self.get_user_data_patcher.stop() + self.get_platform_data_patcher.stop() + self.get_exam_data_patcher.stop() + + def test_import_candidate_demographics_action_parameters(self): + """Test if the `filter_action_parameters` function returns the correct + parameters for the "cdd" action. + """ + result = filter_action_parameters("cdd", user=MagicMock()) + self.assertNotIn("exam_data", result) + self.assertIn("user_data", result) + self.assertIn("platform_data", result) + + def test_import_exam_authorization_action_parameters(self): + """Test if the `filter_action_parameters` function returns the correct + parameters for the "ead" action. + """ + result = filter_action_parameters("ead", user=MagicMock()) + self.assertNotIn("platform_data", result) + self.assertIn("user_data", result) + self.assertIn("exam_data", result) + + def test_real_time_import_action_parameters(self): + """Test if the `filter_action_parameters` function returns the correct + parameters for the "rti" action. + """ + result = filter_action_parameters("rti", user=MagicMock()) + self.assertIn("user_data", result) + self.assertIn("platform_data", result) + self.assertIn("exam_data", result) diff --git a/eox_nelp/pearson_vue/utils.py b/eox_nelp/pearson_vue/utils.py index c421cd08..f1107e45 100644 --- a/eox_nelp/pearson_vue/utils.py +++ b/eox_nelp/pearson_vue/utils.py @@ -69,38 +69,38 @@ def update_user_engines(user, action_type, course_id=None): if course_id: pearson_engine.increment_course_value(course_id) -def _get_user_data(self, user): - """ - Retrieve user data for the request payload. - - Args: - user: The user object containing user data. - - Returns: - dict: The user data formatted for the request. - """ - user_data = { - "username": user.username, - "first_name": user.first_name, - "last_name": user.last_name, - "email": user.email, - "country": user.profile.country.code, - "city": user.profile.city, - "phone": user.profile.phone_number, - "address": user.profile.mailing_address, - "arabic_full_name": user.extrainfo.arabic_name, - "arabic_first_name": user.extrainfo.arabic_first_name, - "arabic_last_name": user.extrainfo.arabic_last_name, - } - - if user.extrainfo.national_id: - user_data["national_id"] = user.extrainfo.national_id - - return user_data - -def _get_platform_data(self): + +def get_user_data(user): + """Retrieve user data for the request payload. + + Args: + user: The user object containing user data. + + Returns: + dict: The user data formatted for the request. """ - Retrieve platform data for the request payload. + user_data = { + "username": user.username, + "first_name": user.first_name, + "last_name": user.last_name, + "email": user.email, + "country": user.profile.country.code, + "city": user.profile.city, + "phone": user.profile.phone_number, + "address": user.profile.mailing_address, + "arabic_full_name": user.extrainfo.arabic_name, + "arabic_first_name": user.extrainfo.arabic_first_name, + "arabic_last_name": user.extrainfo.arabic_last_name, + } + + if user.extrainfo.national_id: + user_data["national_id"] = user.extrainfo.national_id + + return user_data + + +def get_platform_data(): + """Retrieve platform data for the request payload. Returns: dict: The platform data formatted for the request. @@ -110,9 +110,9 @@ def _get_platform_data(self): "tenant": getattr(settings, "EDNX_TENANT_DOMAIN", None) } -def _get_exam_data(self, exam_id): - """ - Retrieve exam data for the request payload. + +def get_exam_data(exam_id): + """Retrieve exam data for the request payload. Args: exam_id: The external key for the exam. @@ -123,3 +123,33 @@ def _get_exam_data(self, exam_id): return { "external_key": exam_id, } + + +def filter_action_parameters(action_name, user, exam_id=None): + """ + Select the appropriate parameters for the action based on the action name. + + Args: + action_name (str): The name of the action to perform. + user (User): the user to be processed. + exam_id (str, optional): The ID of the exam for authorization. Default is None. + **kwargs: Additional keyword arguments + + Returns: + dict: The parameters for the action. + """ + action_parameters = { + "user_data": get_user_data(user), + "platform_data": get_platform_data(), + "exam_data": get_exam_data(exam_id), + } + + if action_name == "cdd": + del action_parameters["exam_data"] + return action_parameters + + if action_name == "ead": + del action_parameters["platform_data"] + return action_parameters + + return action_parameters