From 1c2d9e4f8c04199cfb9940ab03aeac0ce8aef2f4 Mon Sep 17 00:00:00 2001 From: rikuke <33894149+rikuke@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:59:13 +0200 Subject: [PATCH] feat: don't create new status on retry, save error (#3526) --- .../applications/services/ahjo_client.py | 39 +++++++++++++------ .../services/ahjo_error_writer.py | 8 ++++ .../applications/tests/test_ahjo_requests.py | 12 +++--- 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/backend/benefit/applications/services/ahjo_client.py b/backend/benefit/applications/services/ahjo_client.py index a089ecbfe7..ee06b3794f 100644 --- a/backend/benefit/applications/services/ahjo_client.py +++ b/backend/benefit/applications/services/ahjo_client.py @@ -10,6 +10,7 @@ from applications.enums import AhjoRequestType, AhjoStatus as AhjoStatusEnum from applications.models import AhjoSetting, AhjoStatus, Application from applications.services.ahjo_authentication import AhjoToken, InvalidTokenException +from applications.services.ahjo_error_writer import AhjoErrorWriter LOGGER = logging.getLogger(__name__) @@ -24,6 +25,10 @@ class AhjoRequest: lang: str = "fi" url_base: str = field(default_factory=lambda: settings.AHJO_REST_API_URL) + @property + def has_application(self) -> bool: + return self.application is not None + def __str__(self): return f"Request of type {self.request_type}" @@ -240,9 +245,10 @@ def send_request_to_ahjo( timeout=self._timeout, data=data, ) - + # Create new ahjo status or update the last ahjo_status if a similar status exists, + # which means that this is a retry if hasattr(self._request, "result_status") and self._request.result_status: - AhjoStatus.objects.create( + AhjoStatus.objects.update_or_create( application=self._request.application, status=self._request.result_status, ) @@ -260,19 +266,21 @@ def send_request_to_ahjo( else: return self._request.application, response.json() except MissingHandlerIdError as e: - LOGGER.error( - f"Missing handler id for application {self._request.application.application_number}: {e}" - ) + error_message = f"Missing handler id for application {self.request.application.application_number}: {e}" + LOGGER.error(error_message) + self.write_error_to_ahjo_status(error_message) except MissingAhjoCaseIdError as e: - LOGGER.error( - f"Missing Ahjo case id for application {self._request.application.application_number}: {e}" - ) + error_message = f"Missing Ahjo case id for application {self.request.application.application_number}: {e}" + LOGGER.error(error_message) + self.write_error_to_ahjo_status(error_message) except requests.exceptions.HTTPError as e: self.handle_http_error(e) except requests.exceptions.RequestException as e: - LOGGER.error( + error_message = ( f"A network error occurred while sending {self._request} to Ahjo: {e}" ) + self.write_error_to_ahjo_status(error_message) + LOGGER.error(error_message) except AhjoApiClientException as e: LOGGER.error( f"An error occurred while sending {self._request} to Ahjo: {e}" @@ -299,9 +307,7 @@ def handle_http_error(self, e: requests.exceptions.HTTPError) -> None: if error_json: error_message += f" Error message: {error_json}" - status = self._request.application.ahjo_status.latest() - status.validation_error_from_ahjo = error_json - status.save() + self.write_error_to_ahjo_status(error_message) LOGGER.error(error_message) @@ -312,3 +318,12 @@ def format_error_message( ) -> str: return f"A HTTP or network error occurred while sending {self.request} for application \ {application_number} to Ahjo: {e}" + + def write_error_to_ahjo_status(self, error_message: str) -> None: + """Write the error message to the Ahjo status of the application for all requests that have an application. + The DecisionMaker request does not have an application, so it does not have an Ahjo status. + """ + if self.request.has_application: + AhjoErrorWriter.write_to_validation_error( + self.request.application, error_message + ) diff --git a/backend/benefit/applications/services/ahjo_error_writer.py b/backend/benefit/applications/services/ahjo_error_writer.py index 0f756db14c..e06965c4df 100644 --- a/backend/benefit/applications/services/ahjo_error_writer.py +++ b/backend/benefit/applications/services/ahjo_error_writer.py @@ -11,3 +11,11 @@ def write_error_to_ahjo_status(application: Application, error: str) -> None: "message": "Ahjo-pyynnössä tapahtui virhe, mutta Ahjo ei palauttanut tarkempia tietoja.", } latest_ahjo_status.save() + + @staticmethod + def write_to_validation_error(application: Application, error_message: str) -> None: + """Write the error message to the Ahjo status of the application.""" + + status = application.ahjo_status.latest() + status.validation_error_from_ahjo = error_message + status.save() diff --git a/backend/benefit/applications/tests/test_ahjo_requests.py b/backend/benefit/applications/tests/test_ahjo_requests.py index 773d2b8a64..c06107c885 100644 --- a/backend/benefit/applications/tests/test_ahjo_requests.py +++ b/backend/benefit/applications/tests/test_ahjo_requests.py @@ -267,7 +267,7 @@ def test_ahjo_application_requests( AhjoDecisionDetailsRequest, AhjoRequestType.GET_DECISION_DETAILS, "GET", - AhjoStatusEnum.DETAILS_RECEIVED_FROM_AHJO, + AhjoStatusEnum.DECISION_DETAILS_REQUEST_SENT, ), ], ) @@ -324,10 +324,12 @@ def test_requests_exceptions( ) client.send_request_to_ahjo() mock_logger.error.assert_called() - assert ( - application.ahjo_status.latest().validation_error_from_ahjo - == validation_error - ) + ahjo_status = application.ahjo_status.latest() + assert ahjo_status.status == previous_status + assert ahjo_status.validation_error_from_ahjo is not None + + for validation_error in validation_error: + assert f"{validation_error}" in ahjo_status.validation_error_from_ahjo exception = requests.exceptions.RequestException with requests_mock.Mocker() as m: