From 83f7bdc23f1a5bf4a727ae68a4bbd31877c6d6ae Mon Sep 17 00:00:00 2001 From: andrey-canon Date: Mon, 5 Aug 2024 16:18:35 -0500 Subject: [PATCH 1/2] refactor: extract completion logic from pipe to implement it into receiver --- eox_nelp/admin/student.py | 29 ++++ eox_nelp/pearson_vue/pipeline.py | 43 ----- eox_nelp/pearson_vue/rti_backend.py | 6 +- eox_nelp/pearson_vue/tests/test_pipeline.py | 147 ------------------ .../pearson_vue/tests/test_rti_backend.py | 37 ----- eox_nelp/signals/receivers.py | 39 +++-- eox_nelp/signals/tests/test_receivers.py | 38 ++++- 7 files changed, 86 insertions(+), 253 deletions(-) diff --git a/eox_nelp/admin/student.py b/eox_nelp/admin/student.py index 8ed85db7..a4ef5513 100644 --- a/eox_nelp/admin/student.py +++ b/eox_nelp/admin/student.py @@ -9,6 +9,8 @@ for selected course enrollments. """ +import logging + from django.conf import settings from django.contrib import admin @@ -16,6 +18,8 @@ from eox_nelp.edxapp_wrapper.student import CourseEnrollment, CourseEnrollmentAdmin from eox_nelp.pearson_vue.tasks import cdd_task, ead_task, real_time_import_task, real_time_import_task_v2 +logger = logging.getLogger(__name__) + @admin.action(description="Execute Pearson RTI request") def pearson_real_time_action(modeladmin, request, queryset): # pylint: disable=unused-argument @@ -31,6 +35,11 @@ def pearson_real_time_action(modeladmin, request, queryset): # pylint: disable= queryset: The QuerySet of selected CourseEnrollment instances. """ for course_enrollment in queryset: + logger.info( + "Initializing rti task for the user %s, action triggered by admin action", + course_enrollment.user.id, + ) + if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): real_time_import_task_v2.delay( user_id=course_enrollment.user.id, @@ -58,6 +67,11 @@ def pearson_add_ead_action(modeladmin, request, queryset): # pylint: disable=un queryset: The QuerySet of selected CourseEnrollment instances. """ for course_enrollment in queryset: + logger.info( + "Initializing ead add task for the user %s, action triggered by admin action", + course_enrollment.user.id, + ) + if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): real_time_import_task_v2.delay( user_id=course_enrollment.user.id, @@ -87,6 +101,11 @@ def pearson_update_ead_action(modeladmin, request, queryset): # pylint: disable queryset: The QuerySet of selected CourseEnrollment instances. """ for course_enrollment in queryset: + logger.info( + "Initializing ead update task for the user %s, action triggered by admin action", + course_enrollment.user.id, + ) + if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): real_time_import_task_v2.delay( user_id=course_enrollment.user.id, @@ -116,6 +135,11 @@ def pearson_delete_ead_action(modeladmin, request, queryset): # pylint: disable queryset: The QuerySet of selected CourseEnrollment instances. """ for course_enrollment in queryset: + logger.info( + "Initializing ead delete task for the user %s, action triggered by admin action", + course_enrollment.user.id, + ) + if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): real_time_import_task_v2.delay( user_id=course_enrollment.user.id, @@ -145,6 +169,11 @@ def pearson_cdd_action(modeladmin, request, queryset): # pylint: disable=unused queryset: The QuerySet of selected CourseEnrollment instances. """ for course_enrollment in queryset: + logger.info( + "Initializing cdd task for the user %s, action triggered by admin action", + course_enrollment.user.id, + ) + if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): real_time_import_task_v2.delay( user_id=course_enrollment.user.id, diff --git a/eox_nelp/pearson_vue/pipeline.py b/eox_nelp/pearson_vue/pipeline.py index 769e6354..7d11c93b 100644 --- a/eox_nelp/pearson_vue/pipeline.py +++ b/eox_nelp/pearson_vue/pipeline.py @@ -5,7 +5,6 @@ called sequentially, where each function processes data and passes it along to the next step in the pipeline. Functions: - handle_course_completion_status: Pipeline function to handle course completion status. get_user_data: Retrieves and processes user data. check_service_availability: Checks the availability of the Pearson VUE RTI service. import_candidate_demographics: Imports candidate demographics data. @@ -36,7 +35,6 @@ PearsonValidationError, ) from eox_nelp.pearson_vue.utils import generate_client_authorization_id, update_xml_with_dict -from eox_nelp.signals.utils import get_completed_and_graded try: from eox_audit_model.decorators import audit_method, rename_function @@ -55,47 +53,6 @@ def rename_function(name): # pylint: disable=unused-argument User = get_user_model() -def handle_course_completion_status(user_id, course_id, **kwargs): - """Pipeline that check the case of completion cases on the pipeline execution. Also this pipe - has 4 behaviours depending the case: - - skip this pipeline if setting PEARSON_RTI_TESTING_SKIP_HANDLE_COURSE_COMPLETION_STATUS is truthy. - Pipeline continues. - - is_passing is true means the course is graded(passed) and dont needs this pipe validation. - The pipeline continues without changes. - - is_complete=True and is_graded=False pipeline should continue. - (completed courses and not graded). - - Otherwise this indicates that the pipeline execution would be stopped, - for grading-courses the COURSE_GRADE_NOW_PASSED signal would act. - - Args: - user_id (int): The ID of the user whose data is to be retrieved. - course_id (str): course_id to check completion or graded. - **kwargs: Additional keyword arguments. - - Returns: - dict: Pipeline dict - """ - if getattr(settings, "PEARSON_RTI_TESTING_SKIP_HANDLE_COURSE_COMPLETION_STATUS", False): - logger.info( - "Skipping `handle_course_completion_status` pipe for user_id:%s and course_id: %s", - str(user_id), - course_id - ) - return None - - if kwargs.get("is_passing"): - return None - - is_complete, is_graded = get_completed_and_graded(user_id, course_id) - - if is_complete and not is_graded: - return None - - return { - "safely_pipeline_termination": True, - } - - def get_user_data(user_id, **kwargs): # pylint: disable=unused-argument """ Retrieves and processes user data for the pipeline. diff --git a/eox_nelp/pearson_vue/rti_backend.py b/eox_nelp/pearson_vue/rti_backend.py index f27dc056..43d62b3b 100644 --- a/eox_nelp/pearson_vue/rti_backend.py +++ b/eox_nelp/pearson_vue/rti_backend.py @@ -29,7 +29,6 @@ get_enrollment_from_id, get_exam_data, get_user_data, - handle_course_completion_status, import_candidate_demographics, import_exam_authorization, validate_cdd_request, @@ -86,9 +85,6 @@ def run_pipeline(self): break self.backend_data.update(result) - if result.get("safely_pipeline_termination"): - self.backend_data["pipeline_index"] = len(pipeline) - 1 - break @abstractmethod def get_pipeline(self): @@ -145,6 +141,7 @@ class RealTimeImport(AbstractBackend): run_pipeline(): Executes the RTI pipeline by iterating through the pipeline functions. get_pipeline(): Returns the RTI pipeline, which is a list of functions to be executed. """ + def handle_error(self, exception, failed_step_pipeline): """ Handles errors during pipeline execution. @@ -166,7 +163,6 @@ def get_pipeline(self): Returns the RTI pipeline, which is a list of functions to be executed. """ return [ - handle_course_completion_status, get_user_data, get_exam_data, build_cdd_request, diff --git a/eox_nelp/pearson_vue/tests/test_pipeline.py b/eox_nelp/pearson_vue/tests/test_pipeline.py index 2c5ac2dd..f7470814 100644 --- a/eox_nelp/pearson_vue/tests/test_pipeline.py +++ b/eox_nelp/pearson_vue/tests/test_pipeline.py @@ -36,7 +36,6 @@ get_enrollment_from_id, get_exam_data, get_user_data, - handle_course_completion_status, import_candidate_demographics, import_exam_authorization, validate_cdd_request, @@ -81,152 +80,6 @@ } -class TestTerminateNotFullCompletionCases(unittest.TestCase): - """ - Unit tests for the handle_course_completion_status function. - """ - - def setUp(self): - """ - Set up the test environment. - """ - self.user_id = 1 - self.course_id = "course-v1:edX+213+2121" - - @override_settings(PEARSON_RTI_TESTING_SKIP_HANDLE_COURSE_COMPLETION_STATUS=True) - @patch("eox_nelp.pearson_vue.pipeline.get_completed_and_graded") - def test_skip_pipe_with_settings(self, get_completed_and_graded_mock): - """Test the pipeline is skipped with truthy - `PEARSON_RTI_TESTING_SKIP_HANDLE_COURSE_COMPLETION_STATUS` setting. - Expected behavior: - - logger info message expected - - get_completed_and_graded_mock is not called. - - Returned value is None - - """ - pipeline_kwargs = {} - log_info = ( - f"INFO:{pipeline.__name__}:Skipping `handle_course_completion_status` " - f"pipe for user_id:{self.user_id} and course_id: {self.course_id}" - ) - - with self.assertLogs(pipeline.__name__, level="INFO") as logs: - result = handle_course_completion_status(self.user_id, self.course_id, **pipeline_kwargs) - - self.assertEqual(logs.output, [log_info]) - get_completed_and_graded_mock.assert_not_called() - self.assertIsNone(result) - - @patch("eox_nelp.pearson_vue.pipeline.get_completed_and_graded") - def test_check_is_passing_bypass(self, get_completed_and_graded_mock): - """Test the pipeline dont do anything if is_passing kwarg is truthy. - Expected behavior: - - get_completed_and_graded_mock is not called. - - Returned value is None - - """ - pipeline_kwargs = {"is_passing": True} - - result = handle_course_completion_status(self.user_id, self.course_id, **pipeline_kwargs) - - get_completed_and_graded_mock.assert_not_called() - self.assertIsNone(result) - - @patch("eox_nelp.pearson_vue.pipeline.get_completed_and_graded") - def test_check_is_complete_not_graded(self, get_completed_and_graded_mock): - """Test the pipeline return expected dict with empty `is_passing`, and - is_complete=True, is_graded=False. - Expected behavior: - - Returned value is None - - """ - pipeline_kwargs = {} - get_complete_and_graded_output = { - "is_complete": True, - "is_graded": False, - } - get_completed_and_graded_mock.return_value = ( - get_complete_and_graded_output["is_complete"], - get_complete_and_graded_output["is_graded"], - ) - - result = handle_course_completion_status(self.user_id, self.course_id, **pipeline_kwargs) - - self.assertIsNone(result) - - @patch("eox_nelp.pearson_vue.pipeline.get_completed_and_graded") - def test_check_not_complete_not_graded(self, get_completed_and_graded_mock): - """Test the pipeline return expected dict(safely_pipeline_termination) with empty `is_passing`, and - is_complete=False, is_graded=False. - Expected behavior: - - Returned value is dict with `safely_pipeline_termination` - """ - pipeline_kwargs = {} - get_complete_and_graded_output = { - "is_complete": False, - "is_graded": False, - } - expected_output = { - "safely_pipeline_termination": True, - } - get_completed_and_graded_mock.return_value = ( - get_complete_and_graded_output["is_complete"], - get_complete_and_graded_output["is_graded"], - ) - - result = handle_course_completion_status(self.user_id, self.course_id, **pipeline_kwargs) - - self.assertDictEqual(expected_output, result) - - @patch("eox_nelp.pearson_vue.pipeline.get_completed_and_graded") - def test_check_not_complete_graded(self, get_completed_and_graded_mock): - """Test the pipeline return expected dict(safely_pipeline_termination) with empty `is_passing`, and - is_complete=False, is_graded=True. - Expected behavior: - - Returned value is dict with `safely_pipeline_termination` - """ - pipeline_kwargs = {} - get_complete_and_graded_output = { - "is_complete": False, - "is_graded": True, - } - expected_output = { - "safely_pipeline_termination": True, - } - get_completed_and_graded_mock.return_value = ( - get_complete_and_graded_output["is_complete"], - get_complete_and_graded_output["is_graded"], - ) - - result = handle_course_completion_status(self.user_id, self.course_id, **pipeline_kwargs) - - self.assertDictEqual(expected_output, result) - - @patch("eox_nelp.pearson_vue.pipeline.get_completed_and_graded") - def test_check_complete_graded(self, get_completed_and_graded_mock): - """Test the pipeline return expected dict(safely_pipeline_termination) with empty `is_passing`, and - is_complete=True, is_graded=True. - Expected behavior: - - Returned value is dict with `safely_pipeline_termination` - """ - pipeline_kwargs = {} - get_complete_and_graded_output = { - "is_complete": True, - "is_graded": True, - } - expected_output = { - "safely_pipeline_termination": True, - } - get_completed_and_graded_mock.return_value = ( - get_complete_and_graded_output["is_complete"], - get_complete_and_graded_output["is_graded"], - ) - - result = handle_course_completion_status(self.user_id, self.course_id, **pipeline_kwargs) - - self.assertDictEqual(expected_output, result) - - @ddt class TestGetUserData(unittest.TestCase): """ diff --git a/eox_nelp/pearson_vue/tests/test_rti_backend.py b/eox_nelp/pearson_vue/tests/test_rti_backend.py index bbc286ea..afd929c8 100644 --- a/eox_nelp/pearson_vue/tests/test_rti_backend.py +++ b/eox_nelp/pearson_vue/tests/test_rti_backend.py @@ -120,43 +120,6 @@ def test_pipeline_index(self): }, ) - def test_safely_pipeline_termination(self): - """ - Test the execution of the RTI finished after the second function call due - `safely_pipeline_termination` kwarg. - - Expected behavior: - - Pipeline method 1 is called with the original data. - - Pipeline method 2 is called with updated data. - - Pipeline method 3 is not called. - - Pipeline method 4 is not called. - - backend_data attribute is the expected value. - Without func3,func4 data and pipeline index in the last. - """ - # Mock pipeline functions - func1 = MagicMock(return_value={"updated_key": "value1"}) - func2 = MagicMock(return_value={"safely_pipeline_termination": True}) - func3 = MagicMock(return_value={"additional_key": "value3"}) - func4 = MagicMock(return_value={"additional_key": "value4"}) - - self.rti.get_pipeline = MagicMock(return_value=[func1, func2, func2]) - - self.rti.run_pipeline() - - func1.assert_called_once_with(**self.backend_data) - func2.assert_called_once_with(**{"updated_key": "value1", "pipeline_index": 1}) - func3.assert_not_called() - func4.assert_not_called() - - self.assertDictEqual( - self.rti.backend_data, - { - "pipeline_index": len(self.rti.get_pipeline()) - 1, # includes total of pipeline methods - **func1(), # Include data from func1 () - **func2(), # Include data from func2 (with safely_pipeline_termination) - }, - ) - def test_get_pipeline(self): """ Test the retrieval of the RTI pipeline. diff --git a/eox_nelp/signals/receivers.py b/eox_nelp/signals/receivers.py index 46b442f7..2695a7d2 100644 --- a/eox_nelp/signals/receivers.py +++ b/eox_nelp/signals/receivers.py @@ -34,7 +34,7 @@ emit_subsection_attempt_event_task, update_mt_training_stage, ) -from eox_nelp.signals.utils import _generate_external_certificate_data +from eox_nelp.signals.utils import _generate_external_certificate_data, get_completed_and_graded User = get_user_model() UserSignupSource = get_user_signup_source() @@ -393,18 +393,26 @@ def pearson_vue_course_completion_handler(instance, **kwargs): # pylint: disabl if not getattr(settings, "PEARSON_RTI_ACTIVATE_COMPLETION_GATE", False): return - if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): - real_time_import_task_v2.delay( - user_id=instance.user_id, - exam_id=str(instance.context_key), - action_name="rti", - ) - else: - real_time_import_task.delay( - user_id=instance.user_id, - course_id=str(instance.context_key), + is_complete, graded = get_completed_and_graded(user_id=instance.user_id, course_id=str(instance.context_key)) + + if is_complete and not graded: + LOGGER.info( + "Initializing rti task for the user %s, action triggered by course completion status", + instance.user_id, ) + if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): + real_time_import_task_v2.delay( + user_id=instance.user_id, + exam_id=str(instance.context_key), + action_name="rti", + ) + else: + real_time_import_task.delay( + user_id=instance.user_id, + course_id=str(instance.context_key), + ) + def pearson_vue_course_passed_handler(user, course_id, **kwargs): # pylint: disable=unused-argument """This receiver is connected to the COURSE_GRADE_NOW_PASSED @@ -419,18 +427,19 @@ def pearson_vue_course_passed_handler(user, course_id, **kwargs): # pylint: dis if not getattr(settings, "PEARSON_RTI_ACTIVATE_GRADED_GATE", False): return + LOGGER.info( + "Initializing rti task for the user %s, action triggered by COURSE_GRADE_NOW_PASSED signal", + user.id, + ) + if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): real_time_import_task_v2.delay( user_id=user.id, exam_id=str(course_id), action_name="rti", - is_passing=True, - is_graded=True, ) else: real_time_import_task.delay( course_id=str(course_id), user_id=user.id, - is_passing=True, - is_graded=True, ) diff --git a/eox_nelp/signals/tests/test_receivers.py b/eox_nelp/signals/tests/test_receivers.py index 9d31da5b..dea3b912 100644 --- a/eox_nelp/signals/tests/test_receivers.py +++ b/eox_nelp/signals/tests/test_receivers.py @@ -11,6 +11,7 @@ """ import unittest +from ddt import data, ddt from django.contrib.auth import get_user_model from django.test import override_settings from django.utils import timezone @@ -693,6 +694,7 @@ def test_call_async_task(self, task_mock): ) +@ddt class PearsonVueCompletionHandlerTestCase(unittest.TestCase): """Test class for pearson_vue_course_completion_handler function.""" @@ -712,9 +714,34 @@ def test_invalid_feature_flag(self, task_mock): task_mock.delay.assert_not_called() + @data( # is_complete and graded values respectively + (True, True), + (False, True), + (False, False), + ) @override_settings(PEARSON_RTI_ACTIVATE_COMPLETION_GATE=True) + @patch("eox_nelp.signals.receivers.get_completed_and_graded") @patch("eox_nelp.signals.receivers.real_time_import_task") - def test_call_async_task(self, task_mock): + def test_invalid_course_state(self, invalid_state, task_mock, get_completed_and_graded_mock): + """Test when the course is graded or incomplete + + Expected behavior: + - real_time_import_task mock has not been called. + """ + instance = Mock() + instance.user_id = 17 + course_id = "course-v1:test+Cx105+2022_T4" + instance.context_key = CourseKey.from_string(course_id) + get_completed_and_graded_mock.return_value = invalid_state + + pearson_vue_course_completion_handler(instance) + + task_mock.delay.assert_not_called() + + @override_settings(PEARSON_RTI_ACTIVATE_COMPLETION_GATE=True) + @patch("eox_nelp.signals.receivers.get_completed_and_graded") + @patch("eox_nelp.signals.receivers.real_time_import_task") + def test_call_async_task(self, task_mock, get_completed_and_graded_mock): """Test that the async task is called with the right parameters Expected behavior: @@ -724,6 +751,7 @@ def test_call_async_task(self, task_mock): instance.user_id = 5 course_id = "course-v1:test+Cx105+2022_T4" instance.context_key = CourseKey.from_string(course_id) + get_completed_and_graded_mock.return_value = (True, False) # is_complete and graded values respectively pearson_vue_course_completion_handler(instance) @@ -733,8 +761,9 @@ def test_call_async_task(self, task_mock): ) @override_settings(PEARSON_RTI_ACTIVATE_COMPLETION_GATE=True, USE_PEARSON_ENGINE_SERVICE=True) + @patch("eox_nelp.signals.receivers.get_completed_and_graded") @patch("eox_nelp.signals.receivers.real_time_import_task_v2") - def test_call_async_task_v2(self, task_mock): + def test_call_async_task_v2(self, task_mock, get_completed_and_graded_mock): """Test that the async task is called with the right parameters Expected behavior: @@ -744,6 +773,7 @@ def test_call_async_task_v2(self, task_mock): instance.user_id = 5 course_id = "course-v1:test+Cx105+2022_T4" instance.context_key = CourseKey.from_string(course_id) + get_completed_and_graded_mock.return_value = (True, False) # is_complete and graded values respectively pearson_vue_course_completion_handler(instance) @@ -787,8 +817,6 @@ def test_call_async_task(self, task_mock): task_mock.delay.assert_called_with( course_id=course_id, user_id=user_instance.id, - is_passing=True, - is_graded=True ) @override_settings(PEARSON_RTI_ACTIVATE_GRADED_GATE=True, USE_PEARSON_ENGINE_SERVICE=True) @@ -808,8 +836,6 @@ def test_call_async_task_v2(self, task_mock): exam_id=course_id, user_id=user_instance.id, action_name="rti", - is_passing=True, - is_graded=True, ) From 59a69007e2503e1075d8824cacd22f621b9a2178 Mon Sep 17 00:00:00 2001 From: andrey-canon Date: Tue, 6 Aug 2024 13:46:34 -0500 Subject: [PATCH 2/2] chore: change condition logic --- eox_nelp/signals/receivers.py | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/eox_nelp/signals/receivers.py b/eox_nelp/signals/receivers.py index 2695a7d2..167c9185 100644 --- a/eox_nelp/signals/receivers.py +++ b/eox_nelp/signals/receivers.py @@ -395,23 +395,25 @@ def pearson_vue_course_completion_handler(instance, **kwargs): # pylint: disabl is_complete, graded = get_completed_and_graded(user_id=instance.user_id, course_id=str(instance.context_key)) - if is_complete and not graded: - LOGGER.info( - "Initializing rti task for the user %s, action triggered by course completion status", - instance.user_id, - ) + if graded or not is_complete: + return - if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): - real_time_import_task_v2.delay( - user_id=instance.user_id, - exam_id=str(instance.context_key), - action_name="rti", - ) - else: - real_time_import_task.delay( - user_id=instance.user_id, - course_id=str(instance.context_key), - ) + LOGGER.info( + "Initializing rti task for the user %s, action triggered by course completion status", + instance.user_id, + ) + + if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False): + real_time_import_task_v2.delay( + user_id=instance.user_id, + exam_id=str(instance.context_key), + action_name="rti", + ) + else: + real_time_import_task.delay( + user_id=instance.user_id, + course_id=str(instance.context_key), + ) def pearson_vue_course_passed_handler(user, course_id, **kwargs): # pylint: disable=unused-argument