From c8884fbbc50105222d4d7356bd1a64a297323aff Mon Sep 17 00:00:00 2001 From: rikuke <33894149+rikuke@users.noreply.github.com> Date: Thu, 28 Nov 2024 12:20:49 +0200 Subject: [PATCH] fix: Hl 1549 xml exception (#3607) * fix: always use fi language in secret XML * fix: always use fi language in ahjo payload * fix: raise error after xml validation fails * fix: handle ampersand in decision XML * chore: removed unused code --- .../applications/services/ahjo_payload.py | 11 ++-------- .../applications/services/ahjo_xml_builder.py | 21 ++++++++++++------- .../benefit/applications/tests/conftest.py | 13 +++++++++--- .../tests/test_ahjo_xml_builder.py | 11 +++++----- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/backend/benefit/applications/services/ahjo_payload.py b/backend/benefit/applications/services/ahjo_payload.py index 0e933d85dc..dc68fb1567 100644 --- a/backend/benefit/applications/services/ahjo_payload.py +++ b/backend/benefit/applications/services/ahjo_payload.py @@ -218,15 +218,8 @@ def truncate_from_end_of_string(string_to_truncate: str, limit: int): def resolve_payload_language(application: Application) -> str: - """Ahjo cannot at the moment handle en and sv language cases, so if the language is en or sv we use fi""" - if application.applicant_language in [ - APPLICATION_LANGUAGE_CHOICES[1][0], - APPLICATION_LANGUAGE_CHOICES[2][0], - ]: - language = APPLICATION_LANGUAGE_CHOICES[0][0] - else: - language = application.applicant_language - return language + """Ahjo cannot at the moment handle en and sv language cases, so if always return fi""" + return APPLICATION_LANGUAGE_CHOICES[0][0] def _prepare_top_level_dict( diff --git a/backend/benefit/applications/services/ahjo_xml_builder.py b/backend/benefit/applications/services/ahjo_xml_builder.py index 31473651ff..f3c040dcbb 100644 --- a/backend/benefit/applications/services/ahjo_xml_builder.py +++ b/backend/benefit/applications/services/ahjo_xml_builder.py @@ -3,7 +3,7 @@ import os from dataclasses import dataclass from datetime import date -from typing import List, Tuple +from typing import List, Tuple, Union from django.conf import settings from django.template.loader import render_to_string @@ -13,7 +13,11 @@ from lxml.etree import XMLSchema, XMLSchemaParseError, XMLSyntaxError from applications.enums import ApplicationStatus -from applications.models import AhjoDecisionText, Application +from applications.models import ( + AhjoDecisionText, + Application, + APPLICATION_LANGUAGE_CHOICES, +) from calculator.enums import RowType from calculator.models import Calculation, CalculationRow @@ -48,7 +52,9 @@ def load_xsd_as_string(self, xsd_path: str) -> str: with open(xsd_path, "r") as file: return file.read() - def validate_against_schema(self, xml_string: str, xsd_string: str) -> bool: + def validate_against_schema( + self, xml_string: str, xsd_string: str + ) -> Union[bool, None]: try: # Parse the XML string xml_doc = etree.fromstring(xml_string.encode("utf-8")) @@ -76,7 +82,7 @@ def validate_against_schema(self, xml_string: str, xsd_string: str) -> bool: LOGGER.error( f"Decision proposal Validation Error for application {self.application.application_number}: {e}" ) - return False # Return False if the document is invalid + raise class AhjoPublicXMLBuilder(AhjoXMLBuilder): @@ -95,11 +101,9 @@ def sanitize_text_input(text: str) -> str: for target, replacement in replacements.items(): text = text.replace(target, replacement) + text = text.replace("&", "&") return text - def remove_non_breaking_spaces(self, text: str) -> str: - return text.replace("\u00A0", " ") - def generate_xml(self) -> AhjoXMLString: xml_string = ( f"{XML_VERSION}{self.ahjo_decision_text.decision_text}" @@ -215,7 +219,8 @@ def get_context_for_secret_xml(self) -> dict: context = { "application": self.application, "benefit_type": _("Salary Benefit"), - "language": self.application.applicant_language, + # Ahjo only supports Finnish language + "language": APPLICATION_LANGUAGE_CHOICES[0][0], "include_calculation_data": False, } if self.application.status == ApplicationStatus.ACCEPTED: diff --git a/backend/benefit/applications/tests/conftest.py b/backend/benefit/applications/tests/conftest.py index ae36dce9b8..6d6a31babb 100755 --- a/backend/benefit/applications/tests/conftest.py +++ b/backend/benefit/applications/tests/conftest.py @@ -22,6 +22,7 @@ from applications.models import ( AhjoSetting, Application, + APPLICATION_LANGUAGE_CHOICES, ApplicationAlteration, ApplicationBatch, ) @@ -477,7 +478,10 @@ def denied_ahjo_decision_section(): def accepted_ahjo_decision_text(decided_application): template = AcceptedDecisionProposalFactory() replaced_decision_text = replace_decision_template_placeholders( - template.template_decision_text + template.template_justification_text, + f""" +

Päätös

{template.template_decision_text}
+
+

Päätöksen perustelut

{template.template_justification_text}
""", DecisionType.ACCEPTED, decided_application, ) @@ -485,7 +489,7 @@ def accepted_ahjo_decision_text(decided_application): decision_type=DecisionType.ACCEPTED, application=decided_application, decision_text=replaced_decision_text, - language=decided_application.applicant_language, + language=APPLICATION_LANGUAGE_CHOICES[0][0], ) @@ -544,7 +548,10 @@ def generate_ahjo_case_id(): def application_with_ahjo_decision(application_with_ahjo_case_id, fake_decisionmakers): template = AcceptedDecisionProposalFactory() replaced_decision_text = replace_decision_template_placeholders( - template.template_decision_text + template.template_justification_text, + f""" +

Päätös

{template.template_decision_text}
+
+

Päätöksen perustelut

{template.template_justification_text}
""", DecisionType.ACCEPTED, application_with_ahjo_case_id, ) diff --git a/backend/benefit/applications/tests/test_ahjo_xml_builder.py b/backend/benefit/applications/tests/test_ahjo_xml_builder.py index 194650fa62..abf840f4f4 100644 --- a/backend/benefit/applications/tests/test_ahjo_xml_builder.py +++ b/backend/benefit/applications/tests/test_ahjo_xml_builder.py @@ -3,6 +3,7 @@ import pytest from applications.enums import ApplicationStatus +from applications.models import APPLICATION_LANGUAGE_CHOICES from applications.services.ahjo_xml_builder import ( AhjoPublicXMLBuilder, AhjoSecretXMLBuilder, @@ -86,11 +87,10 @@ def total_eur_row(calculation, ordering: int = 8): @pytest.mark.django_db -def test_generate_secret_xml_string(decided_application, secret_xml_builder): - application = decided_application +def test_generate_secret_xml_string(secret_xml_builder): xml_content = secret_xml_builder.generate_xml() - wanted_language = application.applicant_language + wanted_language = APPLICATION_LANGUAGE_CHOICES[0][0] # Check if the returned XML string contains the expected content assert f'
' in xml_content @@ -241,7 +241,7 @@ def test_get_context_for_secret_xml_for_rejected_application( context = secret_xml_builder.get_context_for_secret_xml() assert context["application"] == decided_application - assert context["language"] == decided_application.applicant_language + assert context["language"] == APPLICATION_LANGUAGE_CHOICES[0][0] assert context["include_calculation_data"] is False assert "calculation_periods" not in context assert "total_amount_row" not in context @@ -262,7 +262,7 @@ def test_get_context_for_secret_xml_with_single_period( monthly_row_1.amount ) assert context["calculation_periods"][0].total_amount == int(total_eur_row.amount) - assert context["language"] == decided_application.applicant_language + assert context["language"] == APPLICATION_LANGUAGE_CHOICES[0][0] assert isinstance(context["total_amount_row"], CalculationRow) assert context["total_amount_row"] == total_eur_row assert context["total_amount_row"].amount == int(total_eur_row.amount) @@ -333,6 +333,7 @@ def test_get_context_for_secret_xml_with_multiple_periods( " \u200b\u00a0\u200bTest\u200b\u200b", " Test", ), # Mixed invisible characters + ("Test & Co", "Test & Co"), # No changes expected ("No special characters", "No special characters"), # No changes expected ], )