From cadaa1ba05f551bae306fa7eca0b05f7e652c4e6 Mon Sep 17 00:00:00 2001 From: rikuke <33894149+rikuke@users.noreply.github.com> Date: Wed, 18 Sep 2024 11:18:34 +0300 Subject: [PATCH] fix: HL 1465 improve Ahjo callback error logging (#3325) * fix: request error logging when no failureDetails * feat: store request id in ahjoStatus on failure --- .../api/v1/ahjo_integration_views.py | 15 ++++++---- backend/benefit/applications/enums.py | 8 ++++++ .../0083_ahjostatus_ahjo_request_id.py | 17 +++++++++++ backend/benefit/applications/models.py | 6 ++++ .../tests/test_ahjo_integration.py | 28 +++++++++++++++++-- 5 files changed, 65 insertions(+), 9 deletions(-) create mode 100644 backend/benefit/applications/migrations/0083_ahjostatus_ahjo_request_id.py diff --git a/backend/benefit/applications/api/v1/ahjo_integration_views.py b/backend/benefit/applications/api/v1/ahjo_integration_views.py index c2f527b08d..be77de983f 100644 --- a/backend/benefit/applications/api/v1/ahjo_integration_views.py +++ b/backend/benefit/applications/api/v1/ahjo_integration_views.py @@ -24,6 +24,7 @@ AhjoStatus as AhjoStatusEnum, ApplicationBatchStatus, ApplicationStatus, + DEFAULT_AHJO_CALLBACK_ERROR_MESSAGE, ) from applications.models import AhjoStatus, Application, ApplicationBatch, Attachment from common.permissions import BFIsHandler, SafeListPermission @@ -169,11 +170,6 @@ def post(self, request, *args, **kwargs): return Response( {"error": "Application not found"}, status=status.HTTP_404_NOT_FOUND ) - # TODO how to check the success of the callback if it has no message property? - if request_type == AhjoRequestType.SEND_DECISION_PROPOSAL: - return self.handle_success_callback( - request, application, callback_data, request_type - ) if callback_data["message"] == AhjoCallBackStatus.SUCCESS: return self.handle_success_callback( @@ -258,7 +254,14 @@ def handle_failure_callback( self, application: Application, callback_data: dict ) -> Response: latest_status = application.ahjo_status.latest() - latest_status.error_from_ahjo = callback_data.get("failureDetails", None) + + latest_status.ahjo_request_id = callback_data.get( + "requestId", "no request id received" + ) + + latest_status.error_from_ahjo = callback_data.get( + "failureDetails", DEFAULT_AHJO_CALLBACK_ERROR_MESSAGE + ) latest_status.save() self._log_failure_details(application, callback_data) return Response( diff --git a/backend/benefit/applications/enums.py b/backend/benefit/applications/enums.py index 6199726808..b26f9ed805 100644 --- a/backend/benefit/applications/enums.py +++ b/backend/benefit/applications/enums.py @@ -274,6 +274,14 @@ class AhjoDecisionDetails: decision_date: datetime +DEFAULT_AHJO_CALLBACK_ERROR_MESSAGE = [ + { + "id": "NO_ID", + "context": "Received an error but no failure details in the callback.", + "message": "Ahjo-pyynnössä tapahtui virhe, mutta Ahjo ei palauttanut tarkempia tietoja.", + } +] + # Call gettext on some of the enums so that "makemessages" command can find them when used dynamically in templates _("granted") _("granted_aged") diff --git a/backend/benefit/applications/migrations/0083_ahjostatus_ahjo_request_id.py b/backend/benefit/applications/migrations/0083_ahjostatus_ahjo_request_id.py new file mode 100644 index 0000000000..bf71f06b60 --- /dev/null +++ b/backend/benefit/applications/migrations/0083_ahjostatus_ahjo_request_id.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.11 on 2024-09-13 11:17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("applications", "0082_ahjodecisionproposaldraft_decision_maker_id_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="ahjostatus", + name="ahjo_request_id", + field=models.CharField(blank=True, max_length=64, null=True), + ), + ] diff --git a/backend/benefit/applications/models.py b/backend/benefit/applications/models.py index 628a11660a..35024be88f 100755 --- a/backend/benefit/applications/models.py +++ b/backend/benefit/applications/models.py @@ -1173,6 +1173,12 @@ class AhjoStatus(TimeStampedModel): blank=True, ) + ahjo_request_id = models.CharField( + max_length=64, + null=True, + blank=True, + ) + def __str__(self): return self.status diff --git a/backend/benefit/applications/tests/test_ahjo_integration.py b/backend/benefit/applications/tests/test_ahjo_integration.py index 388660d223..b20cc32b91 100644 --- a/backend/benefit/applications/tests/test_ahjo_integration.py +++ b/backend/benefit/applications/tests/test_ahjo_integration.py @@ -23,6 +23,7 @@ ApplicationTalpaStatus, AttachmentType, BenefitType, + DEFAULT_AHJO_CALLBACK_ERROR_MESSAGE, ) from applications.models import ( AhjoDecisionText, @@ -589,10 +590,26 @@ def test_subscribe_to_decisions_callback_success( } ], ), + ( + AhjoRequestType.SEND_DECISION_PROPOSAL, + AhjoStatusEnum.DECISION_PROPOSAL_SENT, + [ + { + "id": "INVALID_RECORD_TYPE", + "message": "Asiakirjan tyyppi ei ole sallittu.", + "context": "(Tähän tulisi tietoa virheen esiintymispaikasta jos mahdollista antaa.)", + } + ], + ), + ( + AhjoRequestType.SEND_DECISION_PROPOSAL, + AhjoStatusEnum.DECISION_PROPOSAL_SENT, + None, + ), ], ) @pytest.mark.django_db -def test_ahjo_open_case_callback_failure( +def test_ahjo_callback_failure( ahjo_client, ahjo_user_token, decided_application, @@ -611,7 +628,8 @@ def test_ahjo_open_case_callback_failure( status=last_ahjo_status, ) - ahjo_callback_payload["failureDetails"] = failure_details + if failure_details: + ahjo_callback_payload["failureDetails"] = failure_details url = reverse( "ahjo_callback_url", @@ -633,7 +651,11 @@ def test_ahjo_open_case_callback_failure( latest_status = decided_application.ahjo_status.latest() assert latest_status.status == last_ahjo_status - assert latest_status.error_from_ahjo == ahjo_callback_payload["failureDetails"] + if not failure_details: + assert latest_status.error_from_ahjo == DEFAULT_AHJO_CALLBACK_ERROR_MESSAGE + else: + assert latest_status.error_from_ahjo == ahjo_callback_payload["failureDetails"] + assert latest_status.ahjo_request_id == ahjo_callback_payload["requestId"] @pytest.mark.parametrize(