From 0ea3745446eac85e1b269bc5724ec6c4cb0b351c Mon Sep 17 00:00:00 2001 From: rikuke <33894149+rikuke@users.noreply.github.com> Date: Tue, 26 Nov 2024 07:43:16 +0200 Subject: [PATCH] fix: HL 1558 ahjo fixes (#3565) * fix: only accepted applications with instalments * fix: duplicate decision proposal request bug * chore: refactor ahjo exceptions into one place * feat: more readable error messages for handler * feat: log request_type on callback failure * feat: print application numbers after requests * feat: remove decision callback required fields * feat: always return 200 OK after decision callback --- .../api/v1/ahjo_integration_views.py | 85 ++++++++------ .../api/v1/serializers/ahjo_callback.py | 8 +- .../management/commands/send_ahjo_requests.py | 44 ++++++-- backend/benefit/applications/models.py | 5 +- .../applications/services/ahjo/__init__.py | 0 .../applications/services/ahjo/exceptions.py | 105 ++++++++++++++++++ .../services/ahjo_authentication.py | 20 ++-- .../applications/services/ahjo_client.py | 63 ++++++----- .../services/ahjo_decision_service.py | 14 ++- .../services/ahjo_error_writer.py | 34 ++++-- .../applications/services/ahjo_integration.py | 45 +++++--- .../applications/tests/test_ahjo_requests.py | 15 ++- .../tests/test_application_tasks.py | 11 +- .../tests/test_talpa_integration.py | 48 +++++++- 14 files changed, 372 insertions(+), 125 deletions(-) create mode 100644 backend/benefit/applications/services/ahjo/__init__.py create mode 100644 backend/benefit/applications/services/ahjo/exceptions.py diff --git a/backend/benefit/applications/api/v1/ahjo_integration_views.py b/backend/benefit/applications/api/v1/ahjo_integration_views.py index 7d27e860fb..4a3ebc76b2 100644 --- a/backend/benefit/applications/api/v1/ahjo_integration_views.py +++ b/backend/benefit/applications/api/v1/ahjo_integration_views.py @@ -27,6 +27,7 @@ DEFAULT_AHJO_CALLBACK_ERROR_MESSAGE, ) from applications.models import AhjoStatus, Application, ApplicationBatch, Attachment +from applications.services.ahjo.exceptions import AhjoCallbackError from common.permissions import BFIsHandler, SafeListPermission from shared.audit_log import audit_logging from shared.audit_log.enums import Operation @@ -34,10 +35,6 @@ LOGGER = logging.getLogger(__name__) -class AhjoCallbackError(Exception): - pass - - class AhjoApplicationView(APIView): permission_classes = [BFIsHandler] @@ -176,7 +173,11 @@ def post(self, request, *args, **kwargs): request, application, callback_data, request_type ) elif callback_data["message"] == AhjoCallBackStatus.FAILURE: - return self.handle_failure_callback(application, callback_data) + return self.handle_failure_callback( + application=application, + callback_data=callback_data, + request_type=request_type, + ) def cb_info_message( self, @@ -252,7 +253,10 @@ def handle_success_callback( ) def handle_failure_callback( - self, application: Application, callback_data: dict + self, + application: Application, + callback_data: dict, + request_type: AhjoRequestType, ) -> Response: latest_status = application.ahjo_status.latest() @@ -264,7 +268,11 @@ def handle_failure_callback( "failureDetails", DEFAULT_AHJO_CALLBACK_ERROR_MESSAGE ) latest_status.save() - self._log_failure_details(application, callback_data) + self._log_failure_details( + application=application, + callback_data=callback_data, + request_type=request_type, + ) return Response( {"message": "Callback received but request was unsuccessful at AHJO"}, status=status.HTTP_200_OK, @@ -338,9 +346,14 @@ def handle_decision_proposal_success(self, application: Application): batch.status = ApplicationBatchStatus.AWAITING_AHJO_DECISION batch.save() - def _log_failure_details(self, application, callback_data): + def _log_failure_details( + self, + application: Application, + callback_data: dict, + request_type: AhjoRequestType, + ): LOGGER.error( - f"Received unsuccessful callback for application {application.id} \ + f"Received unsuccessful callback for {request_type} for application {application.id} \ with request_id {callback_data['requestId']}, callback data: {callback_data}" ) for cb_record in callback_data.get("records", []): @@ -364,29 +377,37 @@ def post(self, request, *args, **kwargs): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) callback_data = serializer.validated_data - ahjo_case_id = callback_data["caseId"] - update_type = callback_data["updatetype"] - - application = get_object_or_404( - Application, ahjo_case_id=ahjo_case_id, handled_by_ahjo_automation=True - ) + ahjo_case_id = callback_data.get("caseId", None) + update_type = callback_data.get("updatetype", None) + + if ahjo_case_id: + try: + application = Application.objects.get( + ahjo_case_id=ahjo_case_id, + handled_by_ahjo_automation=True, + ) + if update_type == AhjoDecisionUpdateType.ADDED: + AhjoStatus.objects.create( + application=application, status=AhjoStatusEnum.SIGNED_IN_AHJO + ) + elif update_type == AhjoDecisionUpdateType.REMOVED: + AhjoStatus.objects.create( + application=application, status=AhjoStatusEnum.REMOVED_IN_AHJO + ) - if update_type == AhjoDecisionUpdateType.ADDED: - AhjoStatus.objects.create( - application=application, status=AhjoStatusEnum.SIGNED_IN_AHJO - ) - elif update_type == AhjoDecisionUpdateType.REMOVED: - AhjoStatus.objects.create( - application=application, status=AhjoStatusEnum.REMOVED_IN_AHJO - ) - # TODO what to do if updatetype is "updated" - audit_logging.log( - request.user, - "", - Operation.UPDATE, - application, - additional_information=f"Decision proposal update type: {update_type} was received \ - from Ahjo for application {application.application_number}", - ) + # TODO what to do if updatetype is "updated" + audit_logging.log( + request.user, + "", + Operation.UPDATE, + application, + additional_information=f"Decision proposal callback of type: {update_type} was received \ + from Ahjo for application {application.application_number}", + ) + except Application.DoesNotExist: + # Ahjo needs a 200 OK response even if an application is not found + return Response( + {"message": "Callback received"}, status=status.HTTP_200_OK + ) return Response({"message": "Callback received"}, status=status.HTTP_200_OK) diff --git a/backend/benefit/applications/api/v1/serializers/ahjo_callback.py b/backend/benefit/applications/api/v1/serializers/ahjo_callback.py index 4be8a83a32..7cbe40cf88 100644 --- a/backend/benefit/applications/api/v1/serializers/ahjo_callback.py +++ b/backend/benefit/applications/api/v1/serializers/ahjo_callback.py @@ -18,7 +18,7 @@ def validate_message(self, message): class AhjoDecisionCallbackSerializer(serializers.Serializer): - updatetype = serializers.CharField(required=True) - id = serializers.CharField(required=True) - caseId = serializers.CharField(required=True) - caseGuid = serializers.UUIDField(format="hex_verbose", required=True) + updatetype = serializers.CharField(required=False) + id = serializers.CharField(required=False) + caseId = serializers.CharField(required=False) + caseGuid = serializers.UUIDField(format="hex_verbose", required=False) diff --git a/backend/benefit/applications/management/commands/send_ahjo_requests.py b/backend/benefit/applications/management/commands/send_ahjo_requests.py index 0acfa8b95a..46e004db1f 100644 --- a/backend/benefit/applications/management/commands/send_ahjo_requests.py +++ b/backend/benefit/applications/management/commands/send_ahjo_requests.py @@ -14,6 +14,7 @@ ApplicationStatus, ) from applications.models import AhjoStatus, Application +from applications.services.ahjo.exceptions import DecisionProposalAlreadyAcceptedError from applications.services.ahjo_application_service import AhjoApplicationsService from applications.services.ahjo_authentication import ( AhjoToken, @@ -70,6 +71,9 @@ def add_arguments(self, parser): not moved to the next status in the last x hours", ) + def get_application_numbers(self, applications: QuerySet[Application]) -> str: + return ", ".join(str(app.application_number) for app in applications) + def handle(self, *args, **options): try: ahjo_auth_token = get_token() @@ -126,9 +130,8 @@ def run_requests( successful_applications = [] failed_applications = [] - application_numbers = ", ".join( - str(app.application_number) for app in applications - ) + application_numbers = self.get_application_numbers(applications) + message_start = "Retrying" if self.is_retry else "Sending" message = f"{message_start} {ahjo_request_type} request to Ahjo \ @@ -143,6 +146,7 @@ def run_requests( ValueError: "Value error for application", ObjectDoesNotExist: "Object not found error for application", ImproperlyConfigured: "Improperly configured error for application", + DecisionProposalAlreadyAcceptedError: "Decision proposal error for application", } for application in applications: @@ -153,11 +157,12 @@ def run_requests( application, ahjo_auth_token ) except tuple(exception_messages.keys()) as e: - LOGGER.error( - f"{exception_messages[type(e)]} {application.application_number}: {e}" - ) + error_text = f"{exception_messages[type(e)]} {application.application_number}: {e}" + LOGGER.error(error_text) failed_applications.append(application) - self._handle_failed_request(counter, application, ahjo_request_type) + self._handle_failed_request( + counter, application, ahjo_request_type, error_text + ) continue if sent_application: @@ -187,10 +192,14 @@ def _print_results( elapsed_time, ): if successful_applications: + successful_application_numbers = self.get_application_numbers( + successful_applications + ) self.stdout.write( self.style.SUCCESS( self._print_with_timestamp( - f"Sent {ahjo_request_type} requests for {len(successful_applications)} applications to Ahjo" + f"Sent {ahjo_request_type} requests for {len(successful_applications)} \ +application(s): {successful_application_numbers} to Ahjo" ) ) ) @@ -199,10 +208,15 @@ def _print_results( requests took {elapsed_time} seconds to run." ) if failed_applications: + failed_application_numbers = self.get_application_numbers( + failed_applications + ) + self.stdout.write( self.style.ERROR( self._print_with_timestamp( - f"Failed to submit {ahjo_request_type} {len(failed_applications)} applications to Ahjo" + f"Failed to submit {ahjo_request_type} {len(failed_applications)} \ +application(s): {failed_application_numbers} to Ahjo" ) ) ) @@ -265,13 +279,21 @@ def _handle_successful_request( self.stdout.write(self.style.SUCCESS(self._print_with_timestamp(success_text))) def _handle_failed_request( - self, counter: int, application: Application, request_type: AhjoRequestType + self, + counter: int, + application: Application, + request_type: AhjoRequestType, + error_text: str = None, ): + additional_error_text = "" + if error_text: + additional_error_text = f"Error: {error_text}" + self.stdout.write( self.style.ERROR( self._print_with_timestamp( f"{counter}. Failed to submit {request_type} for application {application.id} \ - number: {application.application_number}, to Ahjo" + number: {application.application_number}, to Ahjo. {additional_error_text}" ) ) ) diff --git a/backend/benefit/applications/models.py b/backend/benefit/applications/models.py index e6101f0c98..13e1b4e897 100755 --- a/backend/benefit/applications/models.py +++ b/backend/benefit/applications/models.py @@ -174,6 +174,7 @@ def with_due_instalments(self, status: InstalmentStatus): """Query applications with instalments with past due date and a specific status.""" return ( self.filter( + status=ApplicationStatus.ACCEPTED, calculation__instalments__due_date__lte=timezone.now().date(), calculation__instalments__status=status, ) @@ -1251,7 +1252,9 @@ class AhjoStatus(TimeStampedModel): ) def __str__(self): - return self.status + return f"{self.status} for application {self.application.application_number}, \ +created_at: {self.created_at}, modified_at: {self.modified_at}, \ +ahjo_request_id: {self.ahjo_request_id}" class Meta: db_table = "bf_applications_ahjo_status" diff --git a/backend/benefit/applications/services/ahjo/__init__.py b/backend/benefit/applications/services/ahjo/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/benefit/applications/services/ahjo/exceptions.py b/backend/benefit/applications/services/ahjo/exceptions.py new file mode 100644 index 0000000000..0e92da20d6 --- /dev/null +++ b/backend/benefit/applications/services/ahjo/exceptions.py @@ -0,0 +1,105 @@ +from applications.models import AhjoStatus + + +class DecisionProposalError(Exception): + """Custom exception for errors in the sending of decision proposals.""" + + pass + + +class DecisionProposalAlreadyAcceptedError(DecisionProposalError): + """ + Raised when a decision proposal already has been accepted in Ahjo, + but for some reason a decision proposal for the application is still being sent. + + Attributes: + ahjo_status (AhjosStatus): The decision_proposal_accepted status. + """ + + def __init__(self, message: str, ahjo_status: AhjoStatus) -> None: + self.message = message + self.ahjo_status = ahjo_status + super().__init__(self.message) + + +class AhjoApiClientException(Exception): + """ + Raised when an error occurs in the AhjoApiClient. + """ + + pass + + +class MissingAhjoCaseIdError(AhjoApiClientException): + """ + Raised when a Ahjo request that requires a case id is missing the case id. + """ + + pass + + +class MissingHandlerIdError(AhjoApiClientException): + """ + Raised when a Ahjo request that requires a handler id is missing the handler id. + """ + + pass + + +class MissingOrganizationIdentifier(Exception): + """ + Raised when an organization identifier is missing from AhjoSettings in the database. + """ + + pass + + +class AhjoTokenExpiredException(Exception): + """ + Raised when the Ahjo token has expired. The token should be re-configured manually, see instructions at: + https://helsinkisolutionoffice.atlassian.net/wiki/spaces/KAN/pages/8687517756/Siirto+yll+pitoon#Ahjo-autentikaatio-tokenin-haku-ja-asettaminen-manuaalisesti. + """ + + pass + + +class AhjoTokenRetrievalException(Exception): + """ + Raised when the Ahjo token has expired or it could not be otherwise refreshed automatically. + The token should be re-configured manually, see instructions at: + https://helsinkisolutionoffice.atlassian.net/wiki/spaces/KAN/pages/8687517756/Siirto+yll+pitoon#Ahjo-autentikaatio-tokenin-haku-ja-asettaminen-manuaalisesti. + """ + + pass + + +class InvalidAhjoTokenException(Exception): + """ + Raised when the Ahjo token is missing data or is otherwise invalid. + """ + + pass + + +class AhjoCallbackError(Exception): + """ + Raised when an error occurs in the Ahjo callback. + """ + + pass + + +class AhjoDecisionError(Exception): + """ + Raised when an error occurs in substituting application data into the decision text. + """ + + pass + + +class AhjoDecisionDetailsParsingError(Exception): + """ + Raised when an error occurs in parsing the decision details after a details query to Ahjo. + """ + + pass diff --git a/backend/benefit/applications/services/ahjo_authentication.py b/backend/benefit/applications/services/ahjo_authentication.py index 8240c8a404..16347e58b1 100644 --- a/backend/benefit/applications/services/ahjo_authentication.py +++ b/backend/benefit/applications/services/ahjo_authentication.py @@ -7,23 +7,15 @@ from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist from applications.models import AhjoSetting +from applications.services.ahjo.exceptions import ( + AhjoTokenExpiredException, + AhjoTokenRetrievalException, +) AUTH_TOKEN_GRANT_TYPE = "authorization_code" REFRESH_TOKEN_GRANT_TYPE = "refresh_token" -class AhjoTokenExpiredException(Exception): - pass - - -class AhjoTokenRetrievalException(Exception): - pass - - -class InvalidTokenException(Exception): - pass - - @dataclass class AhjoToken: access_token: str = "" @@ -111,7 +103,9 @@ def do_token_request(self, payload: Dict[str, str]) -> Union[AhjoToken, None]: self.token_url, headers=self.headers, data=payload, timeout=self.timeout ) except requests.exceptions.RequestException as e: - raise Exception(f"Failed to get or refresh token from Ahjo: {e}") + raise AhjoTokenRetrievalException( + f"Failed to get or refresh token from Ahjo: {e}" + ) # Check if the request was successful if response.status_code == 200: diff --git a/backend/benefit/applications/services/ahjo_client.py b/backend/benefit/applications/services/ahjo_client.py index ee06b3794f..cd9681d9d8 100644 --- a/backend/benefit/applications/services/ahjo_client.py +++ b/backend/benefit/applications/services/ahjo_client.py @@ -9,8 +9,15 @@ 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 +from applications.services.ahjo.exceptions import ( + AhjoApiClientException, + InvalidAhjoTokenException, + MissingAhjoCaseIdError, + MissingHandlerIdError, + MissingOrganizationIdentifier, +) +from applications.services.ahjo_authentication import AhjoToken +from applications.services.ahjo_error_writer import AhjoErrorWriter, AhjoFormattedError LOGGER = logging.getLogger(__name__) @@ -145,26 +152,14 @@ def org_identifier() -> str: try: return AhjoSetting.objects.get(name="ahjo_org_identifier").data["id"] except AhjoSetting.DoesNotExist: - raise AhjoSetting.DoesNotExist( - "No organization identifier found in the database" + raise MissingOrganizationIdentifier( + "No organization identifier found in the database." ) def api_url(self) -> str: return f"{self.url_base}/agents/decisionmakers?start={self.org_identifier()}" -class AhjoApiClientException(Exception): - pass - - -class MissingAhjoCaseIdError(AhjoApiClientException): - pass - - -class MissingHandlerIdError(AhjoApiClientException): - pass - - class AhjoApiClient: def __init__(self, ahjo_token: AhjoToken, ahjo_request: AhjoRequest) -> None: self._timeout = settings.AHJO_REQUEST_TIMEOUT @@ -183,9 +178,11 @@ def request(self) -> AhjoRequest: @ahjo_token.setter def ahjo_token(self, token: AhjoToken) -> None: if not isinstance(token, AhjoToken): - raise InvalidTokenException("Invalid token, not an instance of AhjoToken") + raise InvalidAhjoTokenException( + "Invalid token, not an instance of AhjoToken" + ) if not token.access_token or not token.expires_in or not token.refresh_token: - raise InvalidTokenException("Invalid token, token is missing data") + raise InvalidAhjoTokenException("Invalid token, token is missing data") self._ahjo_token = token def prepare_ahjo_headers(self) -> dict: @@ -268,18 +265,27 @@ def send_request_to_ahjo( except MissingHandlerIdError as 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) + self.write_error_to_ahjo_status( + context=error_message, + message_to_handler="Hakemuksen käsittelijältä puuttuu AD-tunnus.", + ) except MissingAhjoCaseIdError as 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) + self.write_error_to_ahjo_status( + context=error_message, + message_to_handler="Hakemuksella ei ole Ahjosta saatua Diaaria.", + ) except requests.exceptions.HTTPError as e: self.handle_http_error(e) except requests.exceptions.RequestException as e: error_message = ( f"A network error occurred while sending {self._request} to Ahjo: {e}" ) - self.write_error_to_ahjo_status(error_message) + self.write_error_to_ahjo_status( + context=error_message, + message_to_handler="Ahjo-pyynnössä tapahtui verkkoyhteysvirhe.", + ) LOGGER.error(error_message) except AhjoApiClientException as e: LOGGER.error( @@ -306,8 +312,11 @@ def handle_http_error(self, e: requests.exceptions.HTTPError) -> None: error_message = self.format_error_message(e) if error_json: - error_message += f" Error message: {error_json}" - self.write_error_to_ahjo_status(error_message) + error_message += f"{error_json}" + self.write_error_to_ahjo_status( + context=error_json, + message_to_handler="Ahjo palautti validaatiovirheen tai muun HTTP-virheen.", + ) LOGGER.error(error_message) @@ -319,11 +328,15 @@ def format_error_message( 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: + def write_error_to_ahjo_status(self, context: str, message_to_handler: 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 + AhjoFormattedError( + application=self.request.application, + context=context, + message_to_handler=message_to_handler, + ) ) diff --git a/backend/benefit/applications/services/ahjo_decision_service.py b/backend/benefit/applications/services/ahjo_decision_service.py index 6cf3385a8b..c7a04c84fd 100644 --- a/backend/benefit/applications/services/ahjo_decision_service.py +++ b/backend/benefit/applications/services/ahjo_decision_service.py @@ -11,16 +11,16 @@ Application, DecisionProposalTemplateSection, ) +from applications.services.ahjo.exceptions import ( + AhjoDecisionDetailsParsingError, + AhjoDecisionError, +) from applications.tests.factories import ( AcceptedDecisionProposalFactory, DeniedDecisionProposalFactory, ) -class AhjoDecisionError(Exception): - pass - - def replace_decision_template_placeholders( text_to_replace: str, decision_type: DecisionType, @@ -126,9 +126,11 @@ def parse_details_from_decision_response(decision_data: Dict) -> AhjoDecisionDet decision_date=decision_date, ) except KeyError as e: - raise AhjoDecisionError(f"Error in parsing decision details: {e}") + raise AhjoDecisionDetailsParsingError(f"Error in parsing decision details: {e}") except ValueError as e: - raise AhjoDecisionError(f"Error in parsing decision details date: {e}") + raise AhjoDecisionDetailsParsingError( + f"Error in parsing decision details date: {e}" + ) def parse_decision_maker_from_html(html_content: str) -> str: diff --git a/backend/benefit/applications/services/ahjo_error_writer.py b/backend/benefit/applications/services/ahjo_error_writer.py index e06965c4df..b0b93e6242 100644 --- a/backend/benefit/applications/services/ahjo_error_writer.py +++ b/backend/benefit/applications/services/ahjo_error_writer.py @@ -1,21 +1,39 @@ +from dataclasses import dataclass + from applications.models import Application +@dataclass +class AhjoFormattedError: + application: Application + context: str = "" + error_id: str = "NO_ID" + message_to_handler: str = ( + "Ahjo-pyynnössä tapahtui virhe, mutta tarkempia tietoja virheestä ei saatu." + ) + + class AhjoErrorWriter: @staticmethod - def write_error_to_ahjo_status(application: Application, error: str) -> None: - latest_ahjo_status = application.ahjo_status.latest() + def write_error_to_ahjo_status(formatted_error: AhjoFormattedError) -> None: + latest_ahjo_status = formatted_error.application.ahjo_status.latest() + latest_ahjo_status.error_from_ahjo = { - "id": "NO_ID", - "context": f"{error}", - "message": "Ahjo-pyynnössä tapahtui virhe, mutta Ahjo ei palauttanut tarkempia tietoja.", + "id": formatted_error.error_id, + "context": formatted_error.context, + "message": formatted_error.message_to_handler, } latest_ahjo_status.save() @staticmethod - def write_to_validation_error(application: Application, error_message: str) -> None: + def write_to_validation_error(formatted_error: AhjoFormattedError) -> 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 = formatted_error.application.ahjo_status.latest() + + status.validation_error_from_ahjo = { + "id": formatted_error.error_id, + "context": formatted_error.context, + "message": formatted_error.message_to_handler, + } status.save() diff --git a/backend/benefit/applications/services/ahjo_integration.py b/backend/benefit/applications/services/ahjo_integration.py index e0938d12fd..a4702073a3 100644 --- a/backend/benefit/applications/services/ahjo_integration.py +++ b/backend/benefit/applications/services/ahjo_integration.py @@ -31,6 +31,10 @@ ApplicationBatch, Attachment, ) +from applications.services.ahjo.exceptions import ( + DecisionProposalAlreadyAcceptedError, + DecisionProposalError, +) from applications.services.ahjo_authentication import AhjoConnector, AhjoToken from applications.services.ahjo_client import ( AhjoAddRecordsRequest, @@ -44,7 +48,7 @@ AhjoSubscribeDecisionRequest, AhjoUpdateRecordsRequest, ) -from applications.services.ahjo_error_writer import AhjoErrorWriter +from applications.services.ahjo_error_writer import AhjoErrorWriter, AhjoFormattedError from applications.services.ahjo_payload import ( prepare_attachment_records_payload, prepare_decision_proposal_payload, @@ -554,17 +558,25 @@ def send_new_attachment_records_to_ahjo( return result, response_text -class DecisionProposalError(Exception): - """Custom exception for errors in XML generation.""" - - pass - - def send_decision_proposal_to_ahjo( application: Application, ahjo_token: AhjoToken ) -> Union[Tuple[Application, str], None]: """Send a decision proposal and it's XML attachments to Ahjo.""" + # https://helsinkisolutionoffice.atlassian.net/browse/HL-1558 + # Check if the application already has a decision proposal accepted in Ahjo status, + # bail out if it does + + if application.ahjo_status.filter(status=AhjoStatusEnum.DECISION_PROPOSAL_ACCEPTED): + existing_status = application.ahjo_status.get( + status=AhjoStatusEnum.DECISION_PROPOSAL_ACCEPTED + ) + raise DecisionProposalAlreadyAcceptedError( + f"The application \ +already has a decision proposal accepted in Ahjo at {existing_status.created_at}. Not sending a new one.", + existing_status, + ) + ahjo_request = AhjoDecisionProposalRequest(application=application) ahjo_client = AhjoApiClient(ahjo_token, ahjo_request) decision = AhjoDecisionText.objects.get(application=application) @@ -591,18 +603,19 @@ def send_decision_proposal_to_ahjo( decision_text=decision, secret_xml=secret_xml, ) - except XMLSchemaParseError as e: - AhjoErrorWriter.write_error_to_ahjo_status(application, str(e)) - return (None, None) - except XMLSyntaxError as e: - AhjoErrorWriter.write_error_to_ahjo_status(application, str(e)) - return (None, None) - except etree.DocumentInvalid as e: - AhjoErrorWriter.write_error_to_ahjo_status(application, str(e)) + except (XMLSchemaParseError, XMLSyntaxError, etree.DocumentInvalid) as e: + AhjoErrorWriter.write_error_to_ahjo_status( + AhjoFormattedError( + application=application, + context=str(e), + message_to_handler="""Helsinki-lisä: Virhe päätöksen XML-tiedostossa. +Tarkista tiedosto sekä päätösteksti ja yritä uudelleen.""", + ) + ) return (None, None) except DecisionProposalError as e: LOGGER.error( - f"Error in generating decision proposal payload\ + f"Error in sending decision proposal payload\ for application {application.application_number}: {e}" ) return (None, None) diff --git a/backend/benefit/applications/tests/test_ahjo_requests.py b/backend/benefit/applications/tests/test_ahjo_requests.py index 621f47d7f1..f9857732e3 100644 --- a/backend/benefit/applications/tests/test_ahjo_requests.py +++ b/backend/benefit/applications/tests/test_ahjo_requests.py @@ -9,11 +9,15 @@ from applications.enums import AhjoRequestType, AhjoStatus as AhjoStatusEnum from applications.models import AhjoSetting, AhjoStatus -from applications.services.ahjo_authentication import AhjoToken, InvalidTokenException +from applications.services.ahjo.exceptions import ( + AhjoApiClientException, + InvalidAhjoTokenException, + MissingHandlerIdError, +) +from applications.services.ahjo_authentication import AhjoToken from applications.services.ahjo_client import ( AhjoAddRecordsRequest, AhjoApiClient, - AhjoApiClientException, AhjoDecisionDetailsRequest, AhjoDecisionMakerRequest, AhjoDecisionProposalRequest, @@ -21,7 +25,6 @@ AhjoOpenCaseRequest, AhjoSubscribeDecisionRequest, AhjoUpdateRecordsRequest, - MissingHandlerIdError, ) API_CASES_BASE = "/cases" @@ -291,10 +294,10 @@ def test_requests_exceptions( with pytest.raises(MissingHandlerIdError): ahjo_request_without_ad_username.api_url() - with pytest.raises(InvalidTokenException): + with pytest.raises(InvalidAhjoTokenException): AhjoApiClient(ahjo_token="foo", ahjo_request=ahjo_request_without_ad_username) - with pytest.raises(InvalidTokenException): + with pytest.raises(InvalidAhjoTokenException): AhjoApiClient( ahjo_token=AhjoToken(access_token=None), ahjo_request=ahjo_request_without_ad_username, @@ -329,7 +332,7 @@ def test_requests_exceptions( 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 + assert validation_error in ahjo_status.validation_error_from_ahjo["context"] exception = requests.exceptions.RequestException with requests_mock.Mocker() as m: diff --git a/backend/benefit/applications/tests/test_application_tasks.py b/backend/benefit/applications/tests/test_application_tasks.py index 7487e9b1f3..dfb9e749e7 100755 --- a/backend/benefit/applications/tests/test_application_tasks.py +++ b/backend/benefit/applications/tests/test_application_tasks.py @@ -233,7 +233,10 @@ def test_send_ahjo_requests( mock_response = f"{uuid.uuid4()}" applications = [ - MagicMock(spec=Application, handled_by_ahjo_automation=True) + MagicMock( + spec=Application, + handled_by_ahjo_automation=True, + ) for _ in range(number_to_send) ] applications_qs = MagicMock() @@ -245,6 +248,8 @@ def test_send_ahjo_requests( mock_response, ) + app_numbers = ", ".join([str(application.application_number)] * number_to_send) + # Call the command out = StringIO() call_command( @@ -254,13 +259,15 @@ def test_send_ahjo_requests( stdout=out, ) + print(out.getvalue()) + assert ( f"Sending {request_type} request to Ahjo for {number_to_send} applications" in out.getvalue() ) assert ( - f"Sent {request_type} requests for {number_to_send} applications to Ahjo" + f"Sent {request_type} requests for {number_to_send} application(s): {app_numbers} to Ahjo" in out.getvalue() ) diff --git a/backend/benefit/applications/tests/test_talpa_integration.py b/backend/benefit/applications/tests/test_talpa_integration.py index 12643599df..0fd7619395 100644 --- a/backend/benefit/applications/tests/test_talpa_integration.py +++ b/backend/benefit/applications/tests/test_talpa_integration.py @@ -4,7 +4,12 @@ import pytest from django.urls import reverse -from applications.enums import ApplicationBatchStatus, ApplicationTalpaStatus +from applications.enums import ( + ApplicationBatchStatus, + ApplicationStatus, + ApplicationTalpaStatus, +) +from applications.models import Application from applications.tests.common import ( check_csv_cell_list_lines_generator, check_csv_string_lines_generator, @@ -338,3 +343,44 @@ def test_talpa_callback_rejected_application( assert ( decided_application.batch.status == ApplicationBatchStatus.REJECTED_BY_TALPA ) + + +@pytest.mark.parametrize( + "application_status", + [ + (ApplicationStatus.ACCEPTED), + (ApplicationStatus.DRAFT), + (ApplicationStatus.RECEIVED), + (ApplicationStatus.REJECTED), + (ApplicationStatus.ARCHIVAL), + (ApplicationStatus.CANCELLED), + (ApplicationStatus.HANDLING), + (ApplicationStatus.ADDITIONAL_INFORMATION_NEEDED), + ], +) +def test_talpa_csv_applications_query( + multiple_decided_applications, application_status, settings +): + settings.TALPA_CALLBACK_ENABLED = True + settings.PAYMENT_INSTALMENTS_ENABLED = True + + for app in multiple_decided_applications: + app.calculation.instalments.all().delete() + Instalment.objects.create( + calculation=app.calculation, + amount=decimal.Decimal("123.45"), + instalment_number=1, + status=InstalmentStatus.ACCEPTED, + due_date=datetime.now(timezone.utc).date(), + ) + app.status = application_status + + app.save() + + applications_for_csv = Application.objects.with_due_instalments( + InstalmentStatus.ACCEPTED + ) + if application_status == ApplicationStatus.ACCEPTED: + assert applications_for_csv.count() == len(multiple_decided_applications) + else: + assert applications_for_csv.count() == 0