From ada412d93821931f89ecbc4565652b6a662a0021 Mon Sep 17 00:00:00 2001 From: Muhammad Afaq Shuaib Date: Wed, 18 Dec 2024 19:13:01 +0500 Subject: [PATCH] refactor: update mock with responses --- .../course_metadata/data_loaders/constants.py | 4 ++ .../data_loaders/csv_loader.py | 18 +++++++-- .../data_loaders/tests/test_csv_loader.py | 37 +++++++++---------- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/course_discovery/apps/course_metadata/data_loaders/constants.py b/course_discovery/apps/course_metadata/data_loaders/constants.py index d335916007d..19ff4b5f0de 100644 --- a/course_discovery/apps/course_metadata/data_loaders/constants.py +++ b/course_discovery/apps/course_metadata/data_loaders/constants.py @@ -45,6 +45,10 @@ class CSVIngestionErrorMessages: COURSE_UPDATE_ERROR = '[COURSE_UPDATE_ERROR] Unable to update course {course_title} in the system. The update ' \ 'failed with the exception: {exception_message}' + COURSE_ENTITLEMENT_PRICE_UPDATE_ERROR = '[COURSE_ENTITLEMENT_PRICE_UPDATE_ERROR] Unable to update ' \ + 'course entitlement price for the course {course_title} in the system. ' \ + 'The update failed with the exception: {exception_message}' + COURSE_RUN_UPDATE_ERROR = '[COURSE_RUN_UPDATE_ERROR] Unable to update course run of the course {course_title} ' \ 'in the system. The update failed with the exception: {exception_message}' diff --git a/course_discovery/apps/course_metadata/data_loaders/csv_loader.py b/course_discovery/apps/course_metadata/data_loaders/csv_loader.py index 9a32238bac7..d361ecea033 100644 --- a/course_discovery/apps/course_metadata/data_loaders/csv_loader.py +++ b/course_discovery/apps/course_metadata/data_loaders/csv_loader.py @@ -253,9 +253,20 @@ def ingest(self): # pylint: disable=too-many-statements self._register_ingestion_error(CSVIngestionErrors.LOGO_IMAGE_DOWNLOAD_FAILURE, error_message) else: - self._update_course_entitlement_price( - data=row, course_uuid=course.uuid, course_type=course_type, is_draft=is_draft, - ) + try: + self._update_course_entitlement_price( + data=row, course_uuid=course.uuid, course_type=course_type, is_draft=is_draft, + ) + except Exception as exc: # pylint: disable=broad-except + exception_message = exc + if hasattr(exc, 'response'): + error_message = CSVIngestionErrorMessages.COURSE_UPDATE_ERROR.format( + course_title=course_title, + exception_message=exception_message + ) + logger.exception(error_message) + self._register_ingestion_error(CSVIngestionErrors.COURSE_UPDATE_ERROR, error_message) + continue # No need to update the course run if the run is already in the review if not course_run.in_review: @@ -727,7 +738,6 @@ def _update_course_entitlement_price(self, data, course_uuid, course_type, is_dr 'title': data['title'], 'prices': pricing, } - response = self._call_course_api('PATCH', url, request_data) if response.status_code not in (200, 201): logger.info(f'Entitlement price update response: {response.content}') diff --git a/course_discovery/apps/course_metadata/data_loaders/tests/test_csv_loader.py b/course_discovery/apps/course_metadata/data_loaders/tests/test_csv_loader.py index ccd0e68bcd7..eb02fa26220 100644 --- a/course_discovery/apps/course_metadata/data_loaders/tests/test_csv_loader.py +++ b/course_discovery/apps/course_metadata/data_loaders/tests/test_csv_loader.py @@ -3,6 +3,7 @@ """ import copy import datetime +import pytest # For exception handling and assertions from decimal import Decimal from tempfile import NamedTemporaryFile from unittest import mock @@ -11,6 +12,7 @@ from ddt import data, ddt, unpack from edx_toggles.toggles.testutils import override_waffle_switch from pytz import UTC +import requests from testfixtures import LogCapture from course_discovery.apps.api.v1.tests.test_views.mixins import APITestCase, OAuth2Mixin @@ -35,6 +37,9 @@ LOGGER_PATH = 'course_discovery.apps.course_metadata.data_loaders.csv_loader' +class MockExceptionWithResponse(Exception): + def __init__(self, response_content): + self.response = mock.Mock(content=response_content) @ddt @mock.patch( @@ -695,10 +700,6 @@ def test_exception_flow_for_update_course(self, jwt_decode_patch): # pylint: di loader._register_ingestion_error = mock.MagicMock() loader._update_course = mock.MagicMock() - class MockExceptionWithResponse(Exception): - def __init__(self, response_content): - self.response = mock.Mock(content=response_content) - # pylint: disable=protected-access loader._update_course.side_effect = MockExceptionWithResponse(b"Update course error") @@ -717,11 +718,10 @@ def __init__(self, response_content): assert CourseRun.everything.count() == 1 @responses.activate - @mock.patch("course_discovery.apps.course_metadata.data_loaders.csv_loader.CSVDataLoader._call_course_api") @mock.patch("course_discovery.apps.course_metadata.data_loaders.csv_loader.reverse") @mock.patch("course_discovery.apps.course_metadata.data_loaders.csv_loader.settings") def test_update_course_entitlement_price_failure( - self, mock_settings, mock_reverse, mock_call_api, jwt_decode_patch + self, mock_settings, mock_reverse, jwt_decode_patch ): # pylint: disable=unused-argument """ Verify that the course entitlement price update fails if an exception is raised while updating the course. @@ -741,10 +741,14 @@ def test_update_course_entitlement_price_failure( ) mock_settings.DISCOVERY_BASE_URL = "http://localhost:18381" mock_reverse.return_value = f"/api/v1/course/{course.uuid}" - mock_response = mock.Mock() - mock_response.status_code = 400 - mock_response.content = b"Error occurred" - mock_call_api.return_value = mock_response + + entitlement_price_url = f"http://localhost:18381/api/v1/course/{course.uuid}" + responses.add( + responses.PATCH, + entitlement_price_url, + json={"detail": "Error occurred"}, + status=204, + ) req_data = { "verified_price": "200", @@ -753,6 +757,7 @@ def test_update_course_entitlement_price_failure( } course_uuid = course.uuid course_type = CourseTypeFactory(slug=CourseType.EXECUTIVE_EDUCATION_2U) + with NamedTemporaryFile() as csv: csv = self._write_csv(csv, [mock_data.VALID_COURSE_AND_COURSE_RUN_CSV_DICT]) @@ -761,22 +766,14 @@ def test_update_course_entitlement_price_failure( # pylint: disable=protected-access response = loader._update_course_entitlement_price(req_data, course_uuid, course_type, is_draft=False) mock_reverse.assert_called_once_with("api:v1:course-detail", kwargs={"key": course_uuid}) - mock_call_api.assert_called_once_with( - "PATCH", - f"http://localhost:18381/api/v1/course/{course_uuid}", - { - "draft": False, "title": "Test Course", - "prices": loader.get_pricing_representation("200", course_type), - }, - ) log_capture.check_present( ( LOGGER_PATH, "INFO", - f"Entitlement price update response: {mock_response.content}", + "Entitlement price update response: b'{\"detail\": \"Error occurred\"}'", ), ) - assert response == mock_response.json() + assert response == {"detail": "Error occurred"} @responses.activate @mock.patch("course_discovery.apps.course_metadata.data_loaders.csv_loader.download_and_save_course_image")