fix: update pearson engine api client #245

Ang-m4 · 2025-02-14T15:59:37Z

Description

This PR removes the pearson_engine module from the deprecated api_clients package and introduces modifications to the real_time_import_task_v2 task.

Changes

Updated the pearson_engine imports in the pearson_vue.tasks file.
With the new reference for pearson_engine, additional parameters for the CDD, EAD, and RTI requests were needed. A new method was added to generate these parameters.
Updated the unit tests for real_time_import_task_v2 to reflect the changes.
Added tests in pearson_vue.utils to cover the new methods.

Tests

Local CDD, EAD and RTI action - requests.

andrey-canon

Generally functionality is working I just left some minor comments

andrey-canon · 2025-02-20T17:48:39Z

eox_nelp/pearson_vue/utils.py

+    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


These lines are not being tested

Got it covered now,

andrey-canon · 2025-02-20T17:48:45Z

eox_nelp/pearson_vue/utils.py

+    return {
+        "name": settings.PLATFORM_NAME,
+        "tenant": getattr(settings, "EDNX_TENANT_DOMAIN", None)
+    }


These lines are not being tested

Got it covered now,

andrey-canon · 2025-02-20T17:48:51Z

eox_nelp/pearson_vue/utils.py

+    return {
+        "external_key": exam_id,
+    }


These lines are not being tested

andrey-canon · 2025-02-20T17:50:01Z

eox_nelp/pearson_vue/utils.py

+    }
+
+
+def generate_action_parameters(user, exam_id=None):


why do you need exam_id=None ?

andrey-canon · 2025-02-20T17:52:34Z

eox_nelp/pearson_vue/utils.py

+
+
+def get_exam_data(exam_id):
+    """Retrieve exam data for the request payload.


This only works for the request payload, I cannot use this method for other thing ?

andrey-canon · 2025-02-20T17:56:49Z

eox_nelp/pearson_vue/tests/test_tasks.py

        """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.
        """
+


andrey-canon · 2025-02-20T17:59:24Z

eox_nelp/pearson_vue/tests/test_tasks.py


    @patch("eox_nelp.pearson_vue.tasks.update_user_engines")
+    @patch("eox_nelp.pearson_vue.tasks.generate_action_parameters")


You are doing nothing with this mock, why do you need that in every single test when you can make a general mock ?

Ang-m4 · 2025-02-24T14:10:44Z

I fixed the tests as you suggested and improved the coverage in the utils file. Additionally, I made some minor fixes.

Add unit testing for missing methods.
Update the generate_action_parameters function argumets: As a default value for exam_id is being setted at the momment the method is called it is not necessary.
Update the real_time_import_v2_task tests.

I also made an squash for related commits in this branch.

andrey-canon · 2025-02-24T16:55:33Z

I fixed the tests as you suggested and improved the coverage in the utils file. Additionally, I made some minor fixes.

Add unit testing for missing methods.

Update the generate_action_parameters function argumets: As a default value for exam_id is being setted at the momment the method is called it is not necessary.

Update the real_time_import_v2_task tests.

I also made an squash for related commits in this branch.

In the future please don't squash commits it's not clear what changes you made

andrey-canon · 2025-02-24T16:58:15Z

eox_nelp/pearson_vue/tests/test_tasks.py

@@ -186,6 +195,7 @@ 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()
+        self.generate_action_parameters_mock.assert_not_called()


Add this verification to the expected behavior doc string

andrey-canon · 2025-02-24T16:58:27Z

eox_nelp/pearson_vue/tests/test_tasks.py

@@ -201,6 +211,7 @@ 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()
+        self.generate_action_parameters_mock.assert_not_called()


same comment as above

andrey-canon · 2025-02-24T17:05:18Z

eox_nelp/pearson_vue/tests/test_utils.py

+        self.user = MagicMock()
+        self.user.username = "testuser"
+        self.user.first_name = "Test"
+        self.user.last_name = "User"
+        self.user.email = "test@example.com"
+        self.user.profile.country.code = "US"
+        self.user.profile.city = "Test City"
+        self.user.profile.phone_number = "123-456-7890"
+        self.user.profile.mailing_address = "123 Test St"
+        self.user.extrainfo.arabic_name = "اسم المستخدم"
+        self.user.extrainfo.arabic_first_name = "الاسم الاول"
+        self.user.extrainfo.arabic_last_name = "اسم العائلة"
+        self.user.extrainfo.national_id = "123456789"


Suggested change

self.user = MagicMock()

self.user.username = "testuser"

self.user.first_name = "Test"

self.user.last_name = "User"

self.user.email = "test@example.com"

self.user.profile.country.code = "US"

self.user.profile.city = "Test City"

self.user.profile.phone_number = "123-456-7890"

self.user.profile.mailing_address = "123 Test St"

self.user.extrainfo.arabic_name = "اسم المستخدم"

self.user.extrainfo.arabic_first_name = "الاسم الاول"

self.user.extrainfo.arabic_last_name = "اسم العائلة"

self.user.extrainfo.national_id = "123456789"

self.user = MagicMock(

username="testuser",

first_name="Test",

last_name="User",

email="test@example.com",

profile=MagicMock(

country=MagicMock(code="US"),

city="Test City",

phone_number="123-456-7890",

mailing_address="123 Test St",

),

extrainfo=MagicMock(

arabic_name="اسم المستخدم",

arabic_first_name="الاسم الاول",

arabic_last_name="اسم العائلة",

national_id="123456789",

),

)

andrey-canon · 2025-02-24T17:06:08Z

eox_nelp/pearson_vue/tests/test_utils.py

+            - The result is a dict with all user data.
+        """
+        expected_result = {
+            "username": "testuser",


Suggested change

"username": "testuser",

"username": self.user.username,

...

andrey-canon · 2025-02-24T17:07:42Z

eox_nelp/pearson_vue/tests/test_utils.py

+        Expected behavior:
+            - The result is a dict with platform name and tenant domain.
+        """
+        self.mock_settings.PLATFORM_NAME = "Test Platform"


please use override_settings

What a nice DJango feature for testing.
I just updated the case.

andrey-canon · 2025-02-24T17:08:03Z

eox_nelp/pearson_vue/tests/test_utils.py

+        Expected behavior:
+            - The result is a dict with platform name and tenant is None.
+        """
+        self.mock_settings.PLATFORM_NAME = "Test Platform"


same comment as above

- update pearson engine client action parameters

andrey-canon

Please add missing trailing commas

andrey-canon · 2025-02-25T21:26:55Z

eox_nelp/pearson_vue/utils.py

+    action_parameters = {
+        "user_data": get_user_data(user),
+        "platform_data": get_platform_data(),
+        "exam_data": {"external_key": exam_id}


Suggested change

"exam_data": {"external_key": exam_id}

"exam_data": {"external_key": exam_id},

andrey-canon · 2025-02-25T21:27:07Z

eox_nelp/pearson_vue/utils.py

+    """
+    return {
+        "name": settings.PLATFORM_NAME,
+        "tenant": getattr(settings, "EDNX_TENANT_DOMAIN", None)


Suggested change

"tenant": getattr(settings, "EDNX_TENANT_DOMAIN", None)

"tenant": getattr(settings, "EDNX_TENANT_DOMAIN", None),

github-actions bot added test size/l labels Feb 14, 2025

Ang-m4 changed the title ~~Afg/pearson engine api client removal~~ fix: update pearson engine api client Feb 14, 2025

Ang-m4 mentioned this pull request Feb 14, 2025

fix: update pearson rti api client package #244

Merged

Ang-m4 requested a review from johanseto February 14, 2025 19:05

Ang-m4 marked this pull request as ready for review February 14, 2025 19:05

andrey-canon requested changes Feb 20, 2025

View reviewed changes

Ang-m4 force-pushed the afg/pearson-engine-api-client-removal branch 3 times, most recently from 2f93d9e to 272e294 Compare February 24, 2025 14:05

Ang-m4 requested a review from andrey-canon February 24, 2025 14:12

Ang-m4 mentioned this pull request Feb 24, 2025

feat: add full name attribute to pearson vue request #252

Merged

andrey-canon requested changes Feb 24, 2025

View reviewed changes

Ang-m4 force-pushed the afg/pearson-engine-api-client-removal branch from 272e294 to 1827e9e Compare February 24, 2025 18:33

Ang-m4 requested a review from andrey-canon February 24, 2025 18:59

Ang-m4 force-pushed the afg/mt-api-client-removal branch 2 times, most recently from b182944 to b796212 Compare February 24, 2025 20:28

Base automatically changed from afg/mt-api-client-removal to master February 24, 2025 20:35

chore: remove unused pearson engine api client

2bca602

- update pearson engine client action parameters

Ang-m4 force-pushed the afg/pearson-engine-api-client-removal branch from 1827e9e to 2bca602 Compare February 24, 2025 21:34

Ang-m4 requested review from andrey-canon and removed request for andrey-canon February 25, 2025 21:08

andrey-canon approved these changes Feb 25, 2025

View reviewed changes

fix: add trailing commas

400f25e

Ang-m4 merged commit 0d47685 into master Feb 25, 2025
7 checks passed

Ang-m4 deleted the afg/pearson-engine-api-client-removal branch February 25, 2025 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update pearson engine api client #245

fix: update pearson engine api client #245

Ang-m4 commented Feb 14, 2025 •

edited

Loading

andrey-canon left a comment

andrey-canon Feb 20, 2025 •

edited

Loading

Ang-m4 Feb 24, 2025

andrey-canon Feb 20, 2025 •

edited

Loading

Ang-m4 Feb 24, 2025

andrey-canon Feb 20, 2025 •

edited

Loading

andrey-canon Feb 20, 2025

andrey-canon Feb 20, 2025

andrey-canon Feb 20, 2025

andrey-canon Feb 20, 2025

Ang-m4 commented Feb 24, 2025

andrey-canon commented Feb 24, 2025

andrey-canon Feb 24, 2025

andrey-canon Feb 24, 2025

andrey-canon Feb 24, 2025

andrey-canon Feb 24, 2025

andrey-canon Feb 24, 2025

Ang-m4 Feb 24, 2025

andrey-canon Feb 24, 2025

andrey-canon left a comment

andrey-canon Feb 25, 2025

andrey-canon Feb 25, 2025



		def get_exam_data(exam_id):
		"""Retrieve exam data for the request payload.


		@patch("eox_nelp.pearson_vue.tasks.update_user_engines")
		@patch("eox_nelp.pearson_vue.tasks.generate_action_parameters")

	"exam_data": {"external_key": exam_id}
	"exam_data": {"external_key": exam_id},

	"tenant": getattr(settings, "EDNX_TENANT_DOMAIN", None)
	"tenant": getattr(settings, "EDNX_TENANT_DOMAIN", None),

fix: update pearson engine api client #245

fix: update pearson engine api client #245

Conversation

Ang-m4 commented Feb 14, 2025 • edited Loading

Description

Changes

Tests

andrey-canon left a comment

Choose a reason for hiding this comment

andrey-canon Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrey-canon Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrey-canon Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ang-m4 commented Feb 24, 2025

andrey-canon commented Feb 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrey-canon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ang-m4 commented Feb 14, 2025 •

edited

Loading

andrey-canon Feb 20, 2025 •

edited

Loading

andrey-canon Feb 20, 2025 •

edited

Loading

andrey-canon Feb 20, 2025 •

edited

Loading