diff --git a/dj-be/poetry.lock b/dj-be/poetry.lock index fbd56e7..0535a55 100644 --- a/dj-be/poetry.lock +++ b/dj-be/poetry.lock @@ -1112,6 +1112,23 @@ files = [ [package.dependencies] referencing = ">=0.31.0" +[[package]] +name = "lark" +version = "1.3.1" +description = "a modern parsing library" +optional = false +python-versions = ">=3.8" +files = [ + {file = "lark-1.3.1-py3-none-any.whl", hash = "sha256:c629b661023a014c37da873b4ff58a817398d12635d3bbb2c5a03be7fe5d1e12"}, + {file = "lark-1.3.1.tar.gz", hash = "sha256:b426a7a6d6d53189d318f2b6236ab5d6429eaf09259f1ca33eb716eed10d2905"}, +] + +[package.extras] +atomic-cache = ["atomicwrites"] +interegular = ["interegular (>=0.3.1,<0.4.0)"] +nearley = ["js2py"] +regex = ["regex"] + [[package]] name = "lxml" version = "5.4.0" @@ -1197,8 +1214,11 @@ files = [ {file = "lxml-5.4.0-cp36-cp36m-win_amd64.whl", hash = "sha256:7ce1a171ec325192c6a636b64c94418e71a1964f56d002cc28122fceff0b6121"}, {file = "lxml-5.4.0-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:795f61bcaf8770e1b37eec24edf9771b307df3af74d1d6f27d812e15a9ff3872"}, {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:29f451a4b614a7b5b6c2e043d7b64a15bd8304d7e767055e8ab68387a8cacf4e"}, + {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:891f7f991a68d20c75cb13c5c9142b2a3f9eb161f1f12a9489c82172d1f133c0"}, {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:4aa412a82e460571fad592d0f93ce9935a20090029ba08eca05c614f99b0cc92"}, + {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_28_aarch64.whl", hash = "sha256:ac7ba71f9561cd7d7b55e1ea5511543c0282e2b6450f122672a2694621d63b7e"}, {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_28_x86_64.whl", hash = "sha256:c5d32f5284012deaccd37da1e2cd42f081feaa76981f0eaa474351b68df813c5"}, + {file = "lxml-5.4.0-cp37-cp37m-musllinux_1_2_aarch64.whl", hash = "sha256:ce31158630a6ac85bddd6b830cffd46085ff90498b397bd0a259f59d27a12188"}, {file = "lxml-5.4.0-cp37-cp37m-musllinux_1_2_x86_64.whl", hash = "sha256:31e63621e073e04697c1b2d23fcb89991790eef370ec37ce4d5d469f40924ed6"}, {file = "lxml-5.4.0-cp37-cp37m-win32.whl", hash = "sha256:be2ba4c3c5b7900246a8f866580700ef0d538f2ca32535e991027bdaba944063"}, {file = "lxml-5.4.0-cp37-cp37m-win_amd64.whl", hash = "sha256:09846782b1ef650b321484ad429217f5154da4d6e786636c38e434fa32e94e49"}, @@ -1393,13 +1413,13 @@ files = [ [[package]] name = "openpyxl" -version = "3.0.9" +version = "3.1.5" description = "A Python library to read/write Excel 2010 xlsx/xlsm files" optional = false -python-versions = ">=3.6" +python-versions = ">=3.8" files = [ - {file = "openpyxl-3.0.9-py2.py3-none-any.whl", hash = "sha256:8f3b11bd896a95468a4ab162fc4fcd260d46157155d1f8bfaabb99d88cfcf79f"}, - {file = "openpyxl-3.0.9.tar.gz", hash = "sha256:40f568b9829bf9e446acfffce30250ac1fa39035124d55fc024025c41481c90f"}, + {file = "openpyxl-3.1.5-py2.py3-none-any.whl", hash = "sha256:5282c12b107bffeef825f4617dc029afaf41d0ea60823bbb665ef3079dc79de2"}, + {file = "openpyxl-3.1.5.tar.gz", hash = "sha256:cf0e3cf56142039133628b5acffe8ef0c12bc902d2aadd3e0fe5878dc08d1050"}, ] [package.dependencies] @@ -1790,20 +1810,24 @@ files = [ [[package]] name = "pyxform" -version = "1.12.2" +version = "4.5.0" description = "A Python package to create XForms for ODK Collect." optional = false -python-versions = ">=3.7" +python-versions = ">=3.10" files = [ - {file = "pyxform-1.12.2-py3-none-any.whl", hash = "sha256:27ceb648a9f2a49e40f30a625bf82557a0ee01d6febf749b0d81614bb6544ac8"}, - {file = "pyxform-1.12.2.tar.gz", hash = "sha256:fb67cee3e89fb21329d5efa6acb87518cbdffe835224f7dbae16bf7e84b519f7"}, + {file = "pyxform-4.5.0-py3-none-any.whl", hash = "sha256:f6bc546d6b53c54a2b505f50850aaa45e7e306eb37d9c5141745eae17bfe9a94"}, + {file = "pyxform-4.5.0.tar.gz", hash = "sha256:c2b7744b52fb4ca2f05353cab846891690f7a33ac89c1443e7703dafd223755e"}, ] [package.dependencies] defusedxml = "0.7.1" -openpyxl = "3.0.9" +lark = "1.3.1" +openpyxl = "3.1.5" xlrd = "2.0.1" +[package.extras] +dev = ["formencode (==2.1.1)", "lxml (==6.0.0)", "psutil (==7.0.0)", "ruff (==0.12.4)"] + [[package]] name = "pyyaml" version = "6.0.2" @@ -2330,4 +2354,4 @@ test = ["big-O", "jaraco.functools", "jaraco.itertools", "jaraco.test", "more-it [metadata] lock-version = "2.0" python-versions = "^3.11" -content-hash = "e4f0a49890e0d9fea7633d4d944ceaa30736b0892a9e440fc2a0d2dcb5a27852" +content-hash = "175f0e3423d675e19f8b3ceebff9575774a1148e32b08475414714318281ca76" diff --git a/dj-be/pyproject.toml b/dj-be/pyproject.toml index 1c6f69e..4324db1 100644 --- a/dj-be/pyproject.toml +++ b/dj-be/pyproject.toml @@ -22,7 +22,8 @@ Authlib = "1.5.2" pandas = "1.5.3" python-docx = "1.1.2" requests = "2.32.4" -pyxform = "1.12.2" +pyxform = "4.5.0" +lark = "1.3.1" django-admin-sortable2 = "2.2.2" django-rq = "2.10.3" django-nested-admin = "3.4.1" @@ -49,7 +50,7 @@ idna = "3.7" setuptools = "78.1.1" zipp = "3.19.1" rq = "1.16.2" -openpyxl = "3.0.9" +openpyxl = "3.1.5" numpy = "1.25.2" [tool.poetry.group.dev.dependencies] diff --git a/dj-be/survey_designer/apps/change_requests/tests/test_views.py b/dj-be/survey_designer/apps/change_requests/tests/test_views.py index d3710df..4a4cd5d 100644 --- a/dj-be/survey_designer/apps/change_requests/tests/test_views.py +++ b/dj-be/survey_designer/apps/change_requests/tests/test_views.py @@ -16,7 +16,6 @@ SubmoduleRequiredGroup, ) from openpyxl import load_workbook -from openpyxl.writer.excel import save_virtual_workbook from organization.models import Organization from questions.const import QuestionType from questions.models import ( @@ -29,6 +28,7 @@ Suffix, ) from questions.services import QuestionsExport +from questions.services.workbook import save_virtual_workbook def test_submit_change_request_view_get( diff --git a/dj-be/survey_designer/apps/modules/tests/test_organization_scope.py b/dj-be/survey_designer/apps/modules/tests/test_organization_scope.py index 06ad872..b5ed9d2 100644 --- a/dj-be/survey_designer/apps/modules/tests/test_organization_scope.py +++ b/dj-be/survey_designer/apps/modules/tests/test_organization_scope.py @@ -130,7 +130,8 @@ def test_assigned_admin_can_queue_generation_for_foreign_selected_scope( @pytest.mark.parametrize( - "endpoint", ["/api/generate/", "/api/preview/", "/api/upload/"] + "endpoint", + ["/api/generate/", "/api/preview/", "/api/upload/", "/api/validate/"], ) def test_out_of_scope_content_fails_before_generation_or_external_side_effects( mocker, diff --git a/dj-be/survey_designer/apps/modules/tests/test_views.py b/dj-be/survey_designer/apps/modules/tests/test_views.py index 90d1259..80224a6 100644 --- a/dj-be/survey_designer/apps/modules/tests/test_views.py +++ b/dj-be/survey_designer/apps/modules/tests/test_views.py @@ -9,6 +9,7 @@ from django.core.cache import cache from django.core.files.base import ContentFile from django.db import connection +from django.test import override_settings from django.test.utils import CaptureQueriesContext from django_rq import get_queue from documents.models import Document @@ -20,6 +21,7 @@ SubmoduleRequiredGroup, ) from modules.views import generate_docx +from pyxform.errors import PyXFormError from questions.const import QuestionType from questions.models import ( RootQuestion, @@ -62,6 +64,20 @@ def generate(self): return StubXLSForm(external_files) +def mock_successful_xml_conversion(mocker, xml=None): + conversion = mocker.Mock() + conversion.run.return_value = xml or ( + '' + "" + "" + ) + conversion.warnings = [] + conversion.errors = [] + mocker.patch("modules.views.XMLConversion", return_value=conversion) + return conversion + + @pytest.fixture(autouse=True) def selected_organization_header( logged_admin_client, api_client_authenticated_admin, organization_1 @@ -577,6 +593,92 @@ def test_preview_xls_form( assert Survey.objects.count() == 1 +@pytest.mark.django_db +@pytest.mark.parametrize( + ("conversion_error", "expected_status", "expected_code"), + [ + ( + PyXFormError("invalid XLSForm"), + status.HTTP_400_BAD_REQUEST, + "PYXFORM_CONVERSION_ERROR", + ), + ( + RuntimeError("converter crashed"), + status.HTTP_503_SERVICE_UNAVAILABLE, + "VALIDATOR_UNAVAILABLE", + ), + ( + TimeoutError("converter timed out"), + status.HTTP_503_SERVICE_UNAVAILABLE, + "VALIDATOR_UNAVAILABLE", + ), + ], +) +def test_converter_failures_keep_input_and_infrastructure_statuses( + mocker, + api_client_authenticated_admin, + submodule_1, + conversion_error, + expected_status, + expected_code, +): + mocker.patch( + "questions.services.xml_conversion.xls2xform.convert", + side_effect=conversion_error, + ) + payload = { + "name": "Converter failure survey", + "submodules": [submodule_1.id], + "submodules_order": [submodule_1.id], + "sub_questions": [], + "languages": ["en"], + } + + response = api_client_authenticated_admin.post( + "/api/validate/", payload, format="json" + ) + + assert response.status_code == expected_status + assert response.json()["errors"][0]["code"] == expected_code + + +@pytest.mark.django_db +@override_settings(CORS_ALLOWED_ORIGINS=["http://localhost:3000"]) +def test_download_exposes_validation_warning_headers_to_frontend( + mocker, api_client_authenticated_admin, submodule_1 +): + conversion = mock_successful_xml_conversion(mocker) + conversion.warnings = ["non-blocking warning"] + payload = { + "name": "Warning survey", + "submodules": [submodule_1.id], + "submodules_order": [submodule_1.id], + "sub_questions": [], + "languages": ["en"], + } + + response = api_client_authenticated_admin.post( + "/api/generate/", + payload, + format="json", + HTTP_ORIGIN="http://localhost:3000", + ) + + assert response.status_code == status.HTTP_200_OK + warnings = json.loads(response["X-Survey-Validation-Warnings"]) + assert warnings[0]["code"] == "PYXFORM_WARNING" + assert response["Access-Control-Allow-Origin"] == "http://localhost:3000" + exposed_headers = { + header.strip().lower() + for header in response["Access-Control-Expose-Headers"].split(",") + } + assert { + "x-survey-validation-warnings", + "x-validation-warnings", + "x-survey-artifact-hash", + }.issubset(exposed_headers) + + @pytest.mark.django_db def test_preview_xls_form_with_external_media( mocker, @@ -591,13 +693,9 @@ def test_preview_xls_form_with_external_media( "fruits.csv": ContentFile(csv_content, name="fruits.csv") } - mocker.patch("modules.views.get_xlsx_from_request", return_value=stub_form) + mocker.patch("modules.views.get_xlsx_from_data", return_value=stub_form) - xml_conversion = mocker.Mock() - xml_conversion.run.return_value = "" - xml_conversion.warnings = [] - xml_conversion.errors = [] - mocker.patch("modules.views.XMLConversion", return_value=xml_conversion) + mock_successful_xml_conversion(mocker) saved_paths = [] @@ -670,7 +768,7 @@ def test_preview_xls_form_rewrites_external_file_links( "logo.png": ContentFile(img_content, name="logo.png"), } - mocker.patch("modules.views.get_xlsx_from_request", return_value=stub_form) + mocker.patch("modules.views.get_xlsx_from_data", return_value=stub_form) xml_payload = ( '' "" ) - xml_conversion = mocker.Mock() - xml_conversion.run.return_value = xml_payload - xml_conversion.warnings = [] - xml_conversion.errors = [] - mocker.patch("modules.views.XMLConversion", return_value=xml_conversion) + mock_successful_xml_conversion(mocker, xml_payload) saved_contents = {} @@ -896,6 +990,7 @@ def test_upload_xls_form_moda_uploads_metadata( fake_file = FakeFieldFile("fruits.csv", b"name,color\nbanana,yellow\n") stub_form = build_stub_xls_form({"fruits.csv": fake_file}) mocker.patch("modules.views.get_xlsx_from_data", return_value=stub_form) + mock_successful_xml_conversion(mocker) upload_response = mocker.Mock() upload_response.ok = True @@ -966,6 +1061,7 @@ def test_upload_xls_form_moda_without_attachments( site = moda_api_key.site stub_form = build_stub_xls_form({}) mocker.patch("modules.views.get_xlsx_from_data", return_value=stub_form) + mock_successful_xml_conversion(mocker) upload_response = mocker.Mock() upload_response.ok = True @@ -1009,6 +1105,7 @@ def test_upload_xls_form_moda_metadata_failure( ): stub_form = build_stub_xls_form({"fruits.csv": FakeFieldFile("fruits.csv")}) mocker.patch("modules.views.get_xlsx_from_data", return_value=stub_form) + mock_successful_xml_conversion(mocker) upload_response = mocker.Mock() upload_response.ok = True diff --git a/dj-be/survey_designer/apps/modules/urls.py b/dj-be/survey_designer/apps/modules/urls.py index 05bb479..50b64d2 100644 --- a/dj-be/survey_designer/apps/modules/urls.py +++ b/dj-be/survey_designer/apps/modules/urls.py @@ -10,6 +10,7 @@ SubmodulesOrderValidationView, SubmoduleViewSet, UploadXLSForm, + ValidateXLSForm, ) router = routers.SimpleRouter() @@ -25,6 +26,7 @@ path("generate-doc/", GenerateDocForm.as_view(), name="generate_doc_form"), path("upload/", UploadXLSForm.as_view(), name="upload_xls_form"), path("preview/", PreviewXLSForm.as_view(), name="preview_xls_form"), + path("validate/", ValidateXLSForm.as_view(), name="validate_xls_form"), path( "order-validation/", SubmodulesOrderValidationView.as_view(), diff --git a/dj-be/survey_designer/apps/modules/views.py b/dj-be/survey_designer/apps/modules/views.py index 2870a82..d83e5b8 100644 --- a/dj-be/survey_designer/apps/modules/views.py +++ b/dj-be/survey_designer/apps/modules/views.py @@ -1,4 +1,5 @@ import io +import json import logging import os import uuid @@ -59,9 +60,21 @@ RootQuestion, SubQuestion, ) -from questions.services import DocConversion, XLSForm, XMLConversion +from questions.services import ( + ArtifactInfrastructureError, + ArtifactInputError, + DocConversion, + ValidationIssue, + ValidationResult, + ValidatorInfrastructureError, + XLSForm, + XMLConversion, + build_generated_artifact, + failed_validation_result, + validate_generated_artifact, +) from rest_framework import serializers, status -from rest_framework.exceptions import ValidationError +from rest_framework.exceptions import APIException, ValidationError from rest_framework.generics import GenericAPIView from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -89,12 +102,134 @@ def generate_docx(data, user): return doc_model.id -def get_xlsx_from_request(get_serializer, request, as_wb=False): - serializer = get_serializer(data=request.data) - serializer.is_valid(raise_exception=True) - data = serializer.validated_data - validate_generation_scope(request, data) - return get_xlsx_from_data(data, as_wb=as_wb) +def _validation_issues_from_detail(detail, field=None): + if isinstance(detail, dict): + issues = [] + for key, value in detail.items(): + issues.extend(_validation_issues_from_detail(value, field=str(key))) + return issues + if isinstance(detail, (list, tuple)): + issues = [] + for value in detail: + issues.extend(_validation_issues_from_detail(value, field=field)) + return issues + return [ + ValidationIssue( + code="INPUT_INVALID", + layer="composition", + severity="error", + message=str(detail), + field=field, + ) + ] + + +def prepare_validated_artifact(get_serializer, request): + """Build and validate one exact artifact before an action has side effects.""" + + try: + serializer = get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + data = serializer.validated_data + validate_generation_scope(request, data) + except APIException as exc: + result = ValidationResult( + valid=False, + errors=tuple(_validation_issues_from_detail(exc.detail)), + ) + return None, None, result, status.HTTP_400_BAD_REQUEST + + try: + xlsx_form = get_xlsx_from_data(data, as_wb=True) + artifact = build_generated_artifact(xlsx_form) + except ArtifactInputError as exc: + return ( + data, + None, + failed_validation_result(exc.issue), + status.HTTP_400_BAD_REQUEST, + ) + except ArtifactInfrastructureError as exc: + issue = ValidationIssue( + code="ARTIFACT_UNAVAILABLE", + layer="composition", + severity="error", + message=str(exc), + ) + return ( + data, + None, + failed_validation_result(issue), + status.HTTP_503_SERVICE_UNAVAILABLE, + ) + except Exception as exc: + issue = ValidationIssue( + code="ARTIFACT_GENERATION_FAILED", + layer="composition", + severity="error", + message=f"Unable to generate the survey artifact: {exc}", + ) + return ( + data, + None, + failed_validation_result(issue), + status.HTTP_503_SERVICE_UNAVAILABLE, + ) + + try: + result = validate_generated_artifact(artifact, converter_cls=XMLConversion) + except ValidatorInfrastructureError as exc: + issue = ValidationIssue( + code="VALIDATOR_UNAVAILABLE", + layer="validator", + severity="error", + message=str(exc), + ) + return ( + data, + xlsx_form, + failed_validation_result(issue, artifact_hash=artifact.artifact_hash), + status.HTTP_503_SERVICE_UNAVAILABLE, + ) + except Exception as exc: + issue = ValidationIssue( + code="VALIDATOR_FAILURE", + layer="validator", + severity="error", + message=f"Survey validation could not complete: {exc}", + ) + return ( + data, + xlsx_form, + failed_validation_result(issue, artifact_hash=artifact.artifact_hash), + status.HTTP_503_SERVICE_UNAVAILABLE, + ) + + return ( + data, + xlsx_form, + result, + (status.HTTP_200_OK if result.valid else status.HTTP_400_BAD_REQUEST), + ) + + +def _validation_response(result, http_status): + return Response(result.as_dict(), status=http_status) + + +def _validation_payload(result): + return result.as_dict() + + +def _add_validation_headers(response, result): + warnings = json.dumps( + [warning.as_dict() for warning in result.warnings], separators=(",", ":") + ) + response["X-Survey-Validation-Warnings"] = warnings + # Keep a short generic alias for clients that do not use the product prefix. + response["X-Validation-Warnings"] = warnings + response["X-Survey-Artifact-Hash"] = result.artifact_hash + return response INDICATOR_ORGANIZATION_RELATIONS = ( @@ -189,7 +324,8 @@ def _get_response_error_details(self, response): try: return response.json() except (JSONDecodeError, ValueError): - return {"response": response.text[:2000] if response.text else ""} + response_text = getattr(response, "text", "") or "" + return {"response": response_text[:2000]} def _handle_bad_response(self, response, service_name, action): details = self._get_response_error_details(response) @@ -198,7 +334,7 @@ def _handle_bad_response(self, response, service_name, action): service_name, action, response.status_code, - response.url, + getattr(response, "url", ""), details, ) @@ -211,8 +347,8 @@ def _handle_bad_response(self, response, service_name, action): } ) - def handle_kobo(self, url, xlsx_form, xlsx_file, token): - name = xlsx_form.id_name + def handle_kobo(self, url, artifact, token): + name = artifact.form_name response = requests.post( f"{url}?format=json", @@ -229,7 +365,7 @@ def handle_kobo(self, url, xlsx_form, xlsx_file, token): upload_response = requests.post( "https://kobo.humanitarianresponse.info/api/v2/imports/", data={"assetUid": uid, "destination": data["url"], "name": name}, - files={"file": (f"{name}.xlsx", xlsx_file)}, + files={"file": (f"{name}.xlsx", artifact.xlsx_bytes)}, headers={"Authorization": f"Token {token}"}, ) @@ -242,10 +378,10 @@ def handle_kobo(self, url, xlsx_form, xlsx_file, token): return Response({"preview_url": preview_url}) - def handle_ona(self, site, url, xlsx_form, xlsx_file, token): + def handle_ona(self, site, url, artifact, token): response = requests.post( url, - files={"xls_file": (f"{xlsx_form.id_name}.xlsx", xlsx_file)}, + files={"xls_file": (f"{artifact.form_name}.xlsx", artifact.xlsx_bytes)}, headers={"Authorization": f"Token {token}"}, ) @@ -261,7 +397,7 @@ def handle_ona(self, site, url, xlsx_form, xlsx_file, token): "preview_url": data.get("enketo_preview_url", ""), } - external_files = getattr(xlsx_form, "external_files", {}) + external_files = artifact.external_files if site.is_moda and external_files: uploaded_files = self._upload_moda_metadata( site, token, data, external_files @@ -288,46 +424,21 @@ def _upload_moda_metadata(self, site, token, submission_data, external_files): } uploaded_files = [] - for file_name, field_file in external_files.items(): - if not field_file: + for file_name, file_bytes in external_files.items(): + if not file_bytes: continue - try: - field_file.open("rb") - except FileNotFoundError: - logger.warning( - "Moda metadata upload failed because choices file is missing. file=%s", - file_name, - ) - raise ValidationError( - {"message": f"Choices file '{file_name}' could not be found."} - ) - except Exception as exc: - logger.exception( - "Moda metadata upload failed while opening choices file. file=%s", - file_name, - ) - raise ValidationError( - { - "message": f"Unable to open choices file '{file_name}' for Moda metadata upload: {exc}" - } - ) - - try: - file_obj = getattr(field_file, "file", field_file) - files = { - "xform": (None, str(form_id)), - "data_type": (None, "media"), - "data_value": (None, file_name), - "data_file": (file_name, file_obj, "text/csv"), - } - response = requests.post( - metadata_url, - headers=headers, - files=files, - ) - finally: - field_file.close() + files = { + "xform": (None, str(form_id)), + "data_type": (None, "media"), + "data_value": (None, file_name), + "data_file": (file_name, io.BytesIO(file_bytes), "text/csv"), + } + response = requests.post( + metadata_url, + headers=headers, + files=files, + ) if not response.ok: self._handle_moda_metadata_error(response, file_name) @@ -341,7 +452,7 @@ def _handle_moda_metadata_error(self, response, file_name): logger.warning( "Moda metadata upload failed. status=%s url=%s file=%s details=%s", response.status_code, - response.url, + getattr(response, "url", ""), file_name, details, ) @@ -373,12 +484,13 @@ def _extract_moda_form_id(submission_data): @extend_schema(responses={200: upload_xls_form_response}) def post(self, request, *args, **kwargs): - serializer = self.get_serializer(data=request.data) - serializer.is_valid(raise_exception=True) - data = serializer.validated_data - validate_generation_scope(request, data) - xlsx_form = get_xlsx_from_data(data, as_wb=True) - xlsx_file = xlsx_form.generate() + data, _, validation, validation_status = prepare_validated_artifact( + self.get_serializer, request + ) + if not validation.valid: + return _validation_response(validation, validation_status) + + artifact = validation.artifact api_key_id = data.get("id") project_id = data.get("project_id") @@ -412,9 +524,13 @@ def post(self, request, *args, **kwargs): token = api_configuration.get_key() if site.is_kobo: - return self.handle_kobo(url, xlsx_form, xlsx_file, token) + response = self.handle_kobo(url, artifact, token) + response.data.update(_validation_payload(validation)) + return response if site.is_ona: - return self.handle_ona(site, url, xlsx_form, xlsx_file, token) + response = self.handle_ona(site, url, artifact, token) + response.data.update(_validation_payload(validation)) + return response raise ValidationError({"message": "Site is not configured."}) @@ -511,43 +627,62 @@ class GenerateXLSForm(GenericAPIView): @extend_schema(responses={200: generate_xls_form_response}) def post(self, request, *args, **kwargs): - xlsx_form = get_xlsx_from_request(self.get_serializer, request, as_wb=True) - xlsx_bytes = xlsx_form.generate() + _, _, validation, validation_status = prepare_validated_artifact( + self.get_serializer, request + ) + if not validation.valid: + # Even with responseType=blob, invalid artifacts are always JSON. + return _validation_response(validation, validation_status) - external_files = getattr(xlsx_form, "external_files", {}) + artifact = validation.artifact + external_files = artifact.external_files if external_files: zip_buffer = io.BytesIO() - with zipfile.ZipFile(zip_buffer, "w", zipfile.ZIP_DEFLATED) as zip_file: - zip_file.writestr("survey.xlsx", xlsx_bytes) - for filename, file_obj in external_files.items(): - try: - data = PreviewXLSForm._read_file_content(file_obj) - if data: - zip_file.writestr(filename, data) - else: - return JsonResponse( - {"detail": f"File {filename} read as empty."}, - status=status.HTTP_400_BAD_REQUEST, - ) - except Exception as e: - return JsonResponse( - {"detail": f"Error reading external file {filename}: {e}"}, - status=status.HTTP_400_BAD_REQUEST, - ) + try: + with zipfile.ZipFile(zip_buffer, "w", zipfile.ZIP_DEFLATED) as zip_file: + zip_file.writestr("survey.xlsx", artifact.xlsx_bytes) + for filename, file_bytes in external_files.items(): + zip_file.writestr(filename, file_bytes) + except Exception as exc: + issue = ValidationIssue( + code="ARTIFACT_SERIALIZATION_FAILURE", + layer="storage", + severity="error", + message=f"Unable to package the validated survey artifact: {exc}", + ) + return _validation_response( + failed_validation_result( + issue, artifact_hash=validation.artifact_hash + ), + status.HTTP_503_SERVICE_UNAVAILABLE, + ) response = HttpResponse( zip_buffer.getvalue(), content_type="application/zip", ) response["Content-Disposition"] = "attachment; filename=survey.zip" - return response + return _add_validation_headers(response, validation) response = HttpResponse( - xlsx_bytes, + artifact.xlsx_bytes, content_type="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", ) response["Content-Disposition"] = "attachment; filename=survey.xlsx" - return response + return _add_validation_headers(response, validation) + + +class ValidateXLSForm(GenericAPIView): + """Validate the exact generated artifact without storage or network effects.""" + + serializer_class = GenerateXLSFormSerializer + permission_classes = [IsAuthenticated] + + def post(self, request, *args, **kwargs): + _, _, validation, validation_status = prepare_validated_artifact( + self.get_serializer, request + ) + return _validation_response(validation, validation_status) preview_xls_form_response = OpenApiResponse( @@ -576,64 +711,40 @@ class PreviewXLSForm(GenericAPIView): serializer_class = GenerateXLSFormSerializer permission_classes = [IsAuthenticated] - @staticmethod - def _read_file_content(file_obj): - """ - Read file content regardless of whether we received a FieldFile or ContentFile. - """ - if hasattr(file_obj, "open"): - file_obj.open("rb") - try: - data = file_obj.read() - finally: - file_obj.close() - else: - data = file_obj.read() - if hasattr(file_obj, "seek"): - file_obj.seek(0) - return data - @extend_schema(responses={200: preview_xls_form_response}) def post(self, request, *args, **kwargs): - xlsx_form = get_xlsx_from_request(self.get_serializer, request, as_wb=True) - xlsx_b = io.BytesIO(xlsx_form.generate()) - xml_conversion = XMLConversion(xlsx_b) - xml_data = xml_conversion.run() + _, _, validation, validation_status = prepare_validated_artifact( + self.get_serializer, request + ) + if not validation.valid: + return _validation_response(validation, validation_status) + + artifact = validation.artifact + xml_data = artifact.xml url = "" enketo_url = "" - external_files = getattr(xlsx_form, "external_files", {}) - if xml_data: - preview_id = uuid.uuid4().hex - storage_prefix = os.path.join("previews", preview_id) + external_files = artifact.external_files + preview_id = uuid.uuid4().hex + storage_prefix = os.path.join("previews", preview_id) + + try: survey = Survey.objects.create() storage = survey.file.storage file_url_map = {} - for filename, file_obj in external_files.items(): - if not filename or not file_obj: - continue - try: - data = self._read_file_content(file_obj) - except FileNotFoundError: - continue - if data is None: - continue - + for filename, file_bytes in external_files.items(): storage_path = storage.save( - f"{storage_prefix}/{filename}", ContentFile(data) + f"{storage_prefix}/{filename}", ContentFile(file_bytes) ) file_url_map[filename] = storage.url(storage_path) xml_input = ( xml_data.encode("utf-8") if isinstance(xml_data, str) else xml_data ) - try: - root = ET.fromstring(xml_input) - except (ET.ParseError, ValueError): - root = None + root = ET.fromstring(xml_input) - if root is not None and file_url_map: + if file_url_map: ns = { "xf": "http://www.w3.org/2002/xforms", "h": "http://www.w3.org/1999/xhtml", @@ -661,13 +772,25 @@ def post(self, request, *args, **kwargs): survey.file.save(file_name, ContentFile(xml_bytes)) url = survey.file.url enketo_url = survey.get_enketo_preview_url() + except Exception as exc: + issue = ValidationIssue( + code="ARTIFACT_STORAGE_FAILURE", + layer="storage", + severity="error", + message=f"Unable to store the validated preview artifact: {exc}", + ) + return _validation_response( + failed_validation_result(issue, artifact_hash=validation.artifact_hash), + status.HTTP_503_SERVICE_UNAVAILABLE, + ) - response = { - "url": url, - "enketo_url": enketo_url, - "warnings": xml_conversion.warnings, - "errors": xml_conversion.errors, - } + response = _validation_payload(validation) + response.update( + { + "url": url, + "enketo_url": enketo_url, + } + ) return Response(response) diff --git a/dj-be/survey_designer/apps/questions/services/__init__.py b/dj-be/survey_designer/apps/questions/services/__init__.py index f532260..81ad075 100644 --- a/dj-be/survey_designer/apps/questions/services/__init__.py +++ b/dj-be/survey_designer/apps/questions/services/__init__.py @@ -1,5 +1,20 @@ from .doc_conversion import DocConversion # noqa +from .form_validation import ( # noqa + ArtifactInfrastructureError, + ArtifactInputError, + GeneratedSurveyArtifact, + ValidationIssue, + ValidationResult, + ValidatorInfrastructureError, + build_generated_artifact, + compute_artifact_hash, + failed_validation_result, + materialize_external_files, + validate_generated_artifact, + validate_xml_compatibility, +) from .questions_export import QuestionsExport # noqa from .questions_import import DataImport # noqa +from .workbook import save_virtual_workbook # noqa from .xls_form import XLSForm # noqa from .xml_conversion import XMLConversion # noqa diff --git a/dj-be/survey_designer/apps/questions/services/form_validation.py b/dj-be/survey_designer/apps/questions/services/form_validation.py new file mode 100644 index 0000000..5f50509 --- /dev/null +++ b/dj-be/survey_designer/apps/questions/services/form_validation.py @@ -0,0 +1,394 @@ +"""Exact XLSForm artifact validation used by publication boundaries. + +This module deliberately stops at pyxform's Python conversion boundary. The +application does not install or invoke Java/ODK Validate. The small XML +compatibility validator below is versioned so that its coverage can be +expanded without changing the public issue contract. +""" + +from __future__ import annotations + +import hashlib +import io +import re +from dataclasses import dataclass, field, replace +from types import MappingProxyType +from typing import Any, Mapping, Sequence +from xml.etree import ElementTree as ET + +PYXFORM_VERSION = "4.5.0" +COMPATIBILITY_VERSION = "1.0" +_EMPTY_ARTIFACT_HASH = "sha256:" + hashlib.sha256(b"").hexdigest() +_XFORMS_NS = "http://www.w3.org/2002/xforms" +_XHTML_NS = "http://www.w3.org/1999/xhtml" +_EXTERNAL_REFERENCE = re.compile(r"jr://file-csv/([^/]+)$") + + +class ArtifactInputError(Exception): + """The submitted/generated artifact cannot be published.""" + + def __init__(self, issue: "ValidationIssue") -> None: + super().__init__(issue.message) + self.issue = issue + + +class ArtifactInfrastructureError(Exception): + """The generator or artifact storage could not produce a stable artifact.""" + + +class ValidatorInfrastructureError(Exception): + """The configured validator could not complete reliably.""" + + +@dataclass(frozen=True) +class ValidationIssue: + code: str + layer: str + severity: str + message: str + owner: Mapping[str, Any] | None = None + field: str | None = None + sheet: str | None = None + column: str | None = None + row: int | None = None + + def as_dict(self) -> dict[str, Any]: + result: dict[str, Any] = { + "code": self.code, + "layer": self.layer, + "severity": self.severity, + "message": self.message, + } + for key in ("owner", "field", "sheet", "column", "row"): + value = getattr(self, key) + if value is not None: + result[key] = dict(value) if key == "owner" else value + return result + + to_dict = as_dict + + +def compute_artifact_hash( + xlsx_bytes: bytes, external_files: Mapping[str, bytes] | None = None +) -> str: + """Return a stable, unambiguous hash for the complete publication artifact.""" + + digest = hashlib.sha256() + digest.update(b"survey-designer-artifact-v1\0") + + def add_part(value: bytes) -> None: + digest.update(len(value).to_bytes(8, byteorder="big")) + digest.update(value) + + add_part(bytes(xlsx_bytes)) + for filename in sorted((external_files or {})): + name_bytes = str(filename).encode("utf-8") + add_part(name_bytes) + add_part(bytes((external_files or {})[filename])) + return f"sha256:{digest.hexdigest()}" + + +@dataclass(frozen=True) +class GeneratedSurveyArtifact: + """The immutable bytes and conversion output for one generated survey.""" + + xlsx_bytes: bytes + external_files: Mapping[str, bytes] = field(default_factory=dict) + artifact_hash: str | None = None + xml: str | None = None + row_source_map: Mapping[str, Any] | None = None + form_name: str = "survey" + + def __post_init__(self) -> None: + xlsx_bytes = bytes(self.xlsx_bytes) + external_files = { + str(name): bytes(content) + for name, content in dict(self.external_files).items() + } + object.__setattr__(self, "xlsx_bytes", xlsx_bytes) + object.__setattr__(self, "external_files", MappingProxyType(external_files)) + if self.row_source_map is not None: + object.__setattr__( + self, + "row_source_map", + MappingProxyType(dict(self.row_source_map)), + ) + artifact_hash = self.artifact_hash or compute_artifact_hash( + xlsx_bytes, external_files + ) + object.__setattr__(self, "artifact_hash", artifact_hash) + + +@dataclass(frozen=True) +class ValidationResult: + """Public validation response plus the successful exact artifact internally.""" + + valid: bool + artifact_hash: str = _EMPTY_ARTIFACT_HASH + errors: Sequence[ValidationIssue] = field(default_factory=tuple) + warnings: Sequence[ValidationIssue] = field(default_factory=tuple) + validator: Mapping[str, str] = field( + default_factory=lambda: { + "pyxform": PYXFORM_VERSION, + "compatibility": COMPATIBILITY_VERSION, + } + ) + artifact: GeneratedSurveyArtifact | None = field(default=None, repr=False) + + def __post_init__(self) -> None: + errors = tuple(self.errors) + warnings = tuple(self.warnings) + object.__setattr__(self, "errors", errors) + object.__setattr__(self, "warnings", warnings) + object.__setattr__(self, "valid", not errors) + object.__setattr__(self, "validator", MappingProxyType(dict(self.validator))) + + def as_dict(self) -> dict[str, Any]: + return { + "valid": self.valid, + "artifact_hash": self.artifact_hash, + "errors": [issue.as_dict() for issue in self.errors], + "warnings": [issue.as_dict() for issue in self.warnings], + "validator": { + "pyxform": self.validator.get("pyxform", PYXFORM_VERSION), + "compatibility": self.validator.get( + "compatibility", COMPATIBILITY_VERSION + ), + }, + } + + # This alias makes the contract convenient for serializers and callers that + # use the usual ``to_dict`` naming. + to_dict = as_dict + + +def _issue( + code: str, + layer: str, + message: str, + *, + severity: str = "error", + **details: Any, +) -> ValidationIssue: + return ValidationIssue( + code=code, + layer=layer, + severity=severity, + message=str(message), + **{key: value for key, value in details.items() if value is not None}, + ) + + +def _normalise_messages( + messages: Sequence[Any], *, warning: bool +) -> list[ValidationIssue]: + return [ + _issue( + "PYXFORM_WARNING" if warning else "PYXFORM_CONVERSION_ERROR", + "pyxform", + str(message), + severity="warning" if warning else "error", + ) + for message in messages + if str(message).strip() + ] + + +def validate_xml_compatibility( + xml: str | bytes | None, external_files: Mapping[str, bytes] | None = None +) -> list[ValidationIssue]: + """Check minimal pyxform output and exact-artifact file references. + + This is not a JavaRosa or ODK Validate replacement. Pyxform performs the + XLSForm checks; this seam only confirms the generated XForm shell and that + referenced CSV files are present in the materialized artifact. + """ + + try: + root = ET.fromstring(xml) + except (ET.ParseError, TypeError, ValueError) as exc: + return [ + _issue( + "XML_MALFORMED", + "compatibility", + f"Generated XML is not well formed: {exc}", + sheet="survey", + ) + ] + + issues: list[ValidationIssue] = [] + if root.tag != f"{{{_XHTML_NS}}}html": + issues.append( + _issue( + "XML_ROOT_INVALID", + "compatibility", + "Generated XForm must have an XHTML html root element.", + sheet="survey", + ) + ) + + model = root.find(f".//{{{_XFORMS_NS}}}model") + body = root.find(f".//{{{_XHTML_NS}}}body") + instances = root.findall(f".//{{{_XFORMS_NS}}}instance") + if model is None: + issues.append( + _issue( + "XML_MODEL_MISSING", + "compatibility", + "Generated XForm is missing its xforms model.", + sheet="survey", + ) + ) + if body is None: + issues.append( + _issue( + "XML_BODY_MISSING", + "compatibility", + "Generated XForm is missing its XHTML body.", + sheet="survey", + ) + ) + if not instances: + issues.append( + _issue( + "XML_INSTANCE_MISSING", + "compatibility", + "Generated XForm is missing a primary instance.", + sheet="survey", + ) + ) + + available_files = set((external_files or {}).keys()) + for element in root.iter(): + for attribute in ("src", "href"): + value = element.get(attribute) + if not value: + continue + match = _EXTERNAL_REFERENCE.match(value) + if match and match.group(1) not in available_files: + issues.append( + _issue( + "EXTERNAL_FILE_MISSING", + "compatibility", + f"Generated XForm references external file '{match.group(1)}', but that file is not materialized.", + sheet="survey", + field=match.group(1), + ) + ) + return issues + + +def materialize_external_files( + external_files: Mapping[str, Any] | None, +) -> dict[str, bytes]: + """Read every selected external file exactly once and retain its bytes.""" + + materialized: dict[str, bytes] = {} + for filename, file_obj in (external_files or {}).items(): + filename = str(filename) + try: + stream = file_obj.open("rb") + try: + content = stream.read() + finally: + stream.close() + except FileNotFoundError: + raise ArtifactInputError( + _issue( + "EXTERNAL_FILE_MISSING", + "composition", + f"External file '{filename}' could not be found.", + field=filename, + ) + ) + except OSError as exc: + raise ArtifactInfrastructureError( + f"Unable to read external file '{filename}': {exc}" + ) from exc + + if not content: + raise ArtifactInputError( + _issue( + "EXTERNAL_FILE_EMPTY", + "composition", + f"External file '{filename}' is empty.", + field=filename, + ) + ) + materialized[filename] = content + return materialized + + +def build_generated_artifact(xlsx_form: Any) -> GeneratedSurveyArtifact: + """Generate a workbook once and materialize its external files once.""" + + try: + xlsx_bytes = xlsx_form.generate() + except Exception as exc: + raise ArtifactInfrastructureError(f"Unable to generate XLSX: {exc}") from exc + + external_files = materialize_external_files(xlsx_form.external_files) + return GeneratedSurveyArtifact( + xlsx_bytes=xlsx_bytes, + external_files=external_files, + form_name=str(xlsx_form.id_name or "survey"), + ) + + +def validate_generated_artifact( + artifact: GeneratedSurveyArtifact, + *, + converter_cls: Any | None = None, +) -> ValidationResult: + """Convert and validate one exact artifact without generating it again.""" + + if converter_cls is None: + from .xml_conversion import XMLConversion + + converter_cls = XMLConversion + + try: + conversion = converter_cls(io.BytesIO(artifact.xlsx_bytes)) + xml = conversion.run() + except Exception as exc: + raise ValidatorInfrastructureError( + f"pyxform conversion failed internally: {exc}" + ) from exc + + errors = _normalise_messages(conversion.errors, warning=False) + warnings = _normalise_messages(conversion.warnings, warning=True) + if not errors: + errors.extend(validate_xml_compatibility(xml, artifact.external_files)) + + validated_artifact = replace(artifact, xml=xml) + return ValidationResult( + valid=not errors, + artifact_hash=validated_artifact.artifact_hash or artifact.artifact_hash, + errors=errors, + warnings=warnings, + artifact=validated_artifact, + ) + + +def failed_validation_result( + issue: ValidationIssue, *, artifact_hash: str = _EMPTY_ARTIFACT_HASH +) -> ValidationResult: + return ValidationResult(valid=False, artifact_hash=artifact_hash, errors=(issue,)) + + +__all__ = [ + "ArtifactInfrastructureError", + "ArtifactInputError", + "COMPATIBILITY_VERSION", + "GeneratedSurveyArtifact", + "PYXFORM_VERSION", + "ValidatorInfrastructureError", + "ValidationIssue", + "ValidationResult", + "build_generated_artifact", + "compute_artifact_hash", + "failed_validation_result", + "materialize_external_files", + "validate_generated_artifact", + "validate_xml_compatibility", +] diff --git a/dj-be/survey_designer/apps/questions/services/questions_export.py b/dj-be/survey_designer/apps/questions/services/questions_export.py index cad2f35..c6d8dce 100644 --- a/dj-be/survey_designer/apps/questions/services/questions_export.py +++ b/dj-be/survey_designer/apps/questions/services/questions_export.py @@ -8,7 +8,6 @@ from openpyxl import Workbook from openpyxl.comments import Comment from openpyxl.styles import Font -from openpyxl.writer.excel import save_virtual_workbook from questions.models import ( BaseQuestion, Choice, @@ -29,6 +28,8 @@ SubQuestionExportSerializer, ) +from .workbook import save_virtual_workbook + class QuestionsExport: STATIC_COLUMNS = ( diff --git a/dj-be/survey_designer/apps/questions/services/workbook.py b/dj-be/survey_designer/apps/questions/services/workbook.py new file mode 100644 index 0000000..e7038d6 --- /dev/null +++ b/dj-be/survey_designer/apps/questions/services/workbook.py @@ -0,0 +1,11 @@ +"""Small openpyxl compatibility helpers for in-memory workbook exports.""" + +from io import BytesIO + + +def save_virtual_workbook(workbook) -> bytes: + """Save a workbook without relying on openpyxl's removed helper.""" + + buffer = BytesIO() + workbook.save(buffer) + return buffer.getvalue() diff --git a/dj-be/survey_designer/apps/questions/services/xls_form.py b/dj-be/survey_designer/apps/questions/services/xls_form.py index 7de7ceb..8485b2b 100644 --- a/dj-be/survey_designer/apps/questions/services/xls_form.py +++ b/dj-be/survey_designer/apps/questions/services/xls_form.py @@ -16,7 +16,6 @@ ) from openpyxl import Workbook from openpyxl.styles import Font -from openpyxl.writer.excel import save_virtual_workbook from questions.const import QuestionType from questions.models import ( BaseQuestion, @@ -32,6 +31,8 @@ ) from survey_designer import ___version___ as SURVEY_DESIGNER_VERSION +from .workbook import save_virtual_workbook + class XLSForm: STATIC_COLUMNS = ( diff --git a/dj-be/survey_designer/apps/questions/services/xml_conversion.py b/dj-be/survey_designer/apps/questions/services/xml_conversion.py index e28e51c..e6e0059 100644 --- a/dj-be/survey_designer/apps/questions/services/xml_conversion.py +++ b/dj-be/survey_designer/apps/questions/services/xml_conversion.py @@ -1,4 +1,5 @@ -from pyxform import builder, xls2json +from pyxform import xls2xform +from pyxform.errors import PyXFormError class XMLConversion: @@ -14,20 +15,17 @@ def filter_warnings(self): def run(self): try: - json_survey = xls2json.parse_file_to_json( - "preview.xlsx", warnings=self.warnings, file_object=self.xls_file - ) - survey = builder.create_survey_element_from_dict(json_survey) - - xml = survey.to_xml( - validate=False, - pretty_print=True, - warnings=self.warnings, - enketo=False, - ) + kwargs = { + "warnings": self.warnings, + "validate": False, + "pretty_print": True, + "enketo": False, + } + converted = xls2xform.convert(self.xls_file, **kwargs) + xml = converted.xform self.filter_warnings() - except Exception as e: + except PyXFormError as e: self.filter_warnings() self.errors.append(str(e)) return diff --git a/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_choices_import.py b/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_choices_import.py index 09063f0..eba4140 100644 --- a/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_choices_import.py +++ b/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_choices_import.py @@ -7,9 +7,9 @@ from django.contrib.auth import get_user_model from django.test import TestCase from openpyxl import Workbook, load_workbook -from openpyxl.writer.excel import save_virtual_workbook from questions.models import Choice, ChoiceGroup from questions.services.questions_import.choices import ChoicesImport +from questions.services.workbook import save_virtual_workbook def build_choices_wb(choices_rows, languages=("en", "English"), with_other_sheets=True): diff --git a/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_errors_scenario.py b/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_errors_scenario.py index 0bd8fba..ce96f76 100644 --- a/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_errors_scenario.py +++ b/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_errors_scenario.py @@ -3,9 +3,9 @@ import pytest from openpyxl import Workbook, load_workbook from openpyxl.worksheet.worksheet import Worksheet -from openpyxl.writer.excel import save_virtual_workbook from organization.models import Organization from questions.services import DataImport +from questions.services.workbook import save_virtual_workbook _TEST_PARAMETER_NAMES = "upload_organizations_names, module_name, submodule_name, question_name, errors_count, errors_dict" diff --git a/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_sunny_scenario.py b/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_sunny_scenario.py index d6661b6..af56214 100644 --- a/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_sunny_scenario.py +++ b/dj-be/survey_designer/apps/questions/tests/bulk_upload/test_sunny_scenario.py @@ -6,7 +6,6 @@ from modules.factories import ModuleFactory, SubmoduleFactory from modules.models import Module, Submodule from openpyxl import Workbook, load_workbook -from openpyxl.writer.excel import save_virtual_workbook from organization.models import Organization from organization.tests.factories import OrganizationFactory from questions.const import QuestionType @@ -16,6 +15,7 @@ from questions.services.questions_import.submodule_required_group import ( SubmoduleRequiredGroupImport, ) +from questions.services.workbook import save_virtual_workbook def test_sunny_scenario(admin): diff --git a/dj-be/survey_designer/apps/questions/tests/test_form_validation.py b/dj-be/survey_designer/apps/questions/tests/test_form_validation.py new file mode 100644 index 0000000..b17ec22 --- /dev/null +++ b/dj-be/survey_designer/apps/questions/tests/test_form_validation.py @@ -0,0 +1,142 @@ +import io + +import pytest +from django.core.files.base import ContentFile +from questions.services.form_validation import ( + ArtifactInputError, + GeneratedSurveyArtifact, + ValidationIssue, + build_generated_artifact, + compute_artifact_hash, + materialize_external_files, + validate_generated_artifact, + validate_xml_compatibility, +) +from questions.services.xml_conversion import XMLConversion + + +def test_artifact_hash_is_order_independent_for_external_files(): + first = compute_artifact_hash(b"xlsx", {"b.csv": b"2", "a.csv": b"1"}) + second = compute_artifact_hash(b"xlsx", {"a.csv": b"1", "b.csv": b"2"}) + + assert first == second + assert first.startswith("sha256:") + assert first != compute_artifact_hash(b"different", {"a.csv": b"1", "b.csv": b"2"}) + + +def test_build_generated_artifact_generates_and_reads_each_external_file_once(): + class ReadOnceFile: + def __init__(self, content): + self.content = content + self.open_mode = None + self.read_count = 0 + self.close_count = 0 + + def open(self, mode): + self.open_mode = mode + return self + + def read(self): + self.read_count += 1 + return self.content + + def close(self): + self.close_count += 1 + + external = ReadOnceFile(b"name\nvalue\n") + + class Form: + id_name = "generated-form" + + def __init__(self): + self.generate_count = 0 + self.external_files = {"choices.csv": external} + + def generate(self): + self.generate_count += 1 + return b"xlsx-bytes" + + form = Form() + artifact = build_generated_artifact(form) + + assert artifact.xlsx_bytes == b"xlsx-bytes" + assert artifact.external_files == {"choices.csv": b"name\nvalue\n"} + assert form.generate_count == 1 + assert external.open_mode == "rb" + assert external.read_count == 1 + assert external.close_count == 1 + + +def test_compatibility_requires_referenced_csv_in_exact_artifact(): + xml = ( + '' + "" + '' + "" + ) + + issues = validate_xml_compatibility(xml) + assert [issue.code for issue in issues] == ["EXTERNAL_FILE_MISSING"] + assert validate_xml_compatibility(xml, {"choices.csv": b"data"}) == [] + + +def test_validation_result_normalizes_compatibility_issues(): + class Conversion: + def __init__(self, xlsx_file): + self.xlsx_file = xlsx_file + self.errors = [] + self.warnings = ["non-blocking warning"] + + def run(self): + return ( + '' + "" + "" + ) + + artifact = GeneratedSurveyArtifact(b"xlsx") + result = validate_generated_artifact(artifact, converter_cls=Conversion) + + assert result.valid is True + assert result.errors == () + assert result.warnings[0].as_dict() == { + "code": "PYXFORM_WARNING", + "layer": "pyxform", + "severity": "warning", + "message": "non-blocking warning", + } + assert result.as_dict()["validator"] == { + "pyxform": "4.5.0", + "compatibility": "1.0", + } + + +def test_empty_external_file_is_a_structured_input_error(): + with pytest.raises(ArtifactInputError) as raised: + materialize_external_files( + {"choices.csv": ContentFile(b"", name="choices.csv")} + ) + + assert isinstance(raised.value.issue, ValidationIssue) + assert raised.value.issue.code == "EXTERNAL_FILE_EMPTY" + + +def test_xml_conversion_disables_java_validation(monkeypatch): + calls = {} + + class Converted: + xform = "" + + def convert(xlsform, **kwargs): + calls.update(kwargs) + return Converted() + + monkeypatch.setattr("questions.services.xml_conversion.xls2xform.convert", convert) + + conversion = XMLConversion(io.BytesIO(b"xlsx")) + assert conversion.run() == "" + assert calls["validate"] is False + assert calls["pretty_print"] is True + assert calls["enketo"] is False diff --git a/dj-be/survey_designer/settings.py b/dj-be/survey_designer/settings.py index c5977e8..5ffc283 100644 --- a/dj-be/survey_designer/settings.py +++ b/dj-be/survey_designer/settings.py @@ -379,6 +379,11 @@ "content-type", "survey-designer-organizations", ] +CORS_EXPOSE_HEADERS = [ + "X-Survey-Validation-Warnings", + "X-Validation-Warnings", + "X-Survey-Artifact-Hash", +] # Sentry SEND_TO_SENTRY = environ.get("SEND_TO_SENTRY", "False").lower() == "true" diff --git a/react-ui/src/app/components/Generate/index.tsx b/react-ui/src/app/components/Generate/index.tsx index 33d42e4..4732603 100644 --- a/react-ui/src/app/components/Generate/index.tsx +++ b/react-ui/src/app/components/Generate/index.tsx @@ -14,7 +14,6 @@ import { useAppDispatch, useAppSelector } from "../../redux/store"; import { useModules } from "../../contexts/ModulesContext"; import { fetchProjects } from "../../redux/actions/projectsActions"; import { API, capitalize } from "../../utils"; -import { renderApiErrorMessage } from "../../utils/apiError"; import { clearJob } from "../../redux/actions/docFetcherActions"; import { notificationsActions } from "../../redux/reducers/notificationReducer"; import { @@ -22,6 +21,12 @@ import { UploadResponse, UserAPIKey, } from "../../types/api"; +import { + formatValidationIssues, + getApiErrorTitle, + parseApiError, + renderApiErrorMessage, +} from "../../utils/apiError"; import { GenerateProps } from "./Generate.interface"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { faFileAlt, faSearch } from "@fortawesome/free-solid-svg-icons"; @@ -70,6 +75,7 @@ function Generate({ next }: GenerateProps) { function uploadXLS() { setUploading(true); + setUploadResponse(null); const data = { site: selectedSite?.name, id: selectedSite?.id, @@ -93,15 +99,19 @@ function Generate({ next }: GenerateProps) { setUploading(false); setUploadResponse(res.data); }) - .catch((err) => { + .catch(async (err) => { setUploading(false); + const parsedError = await parseApiError(err); dispatch( notificationsActions.setErrorNotification({ msg: renderApiErrorMessage( - err, + parsedError, "The survey could not be published.", ), - title: t("generate.notification.uploadError"), + title: getApiErrorTitle( + parsedError, + t("generate.notification.uploadError"), + ), }), ); }); @@ -132,13 +142,26 @@ function Generate({ next }: GenerateProps) { ); } if (uploadResponse) { - dispatch( - notificationsActions.setSuccessNotification({ - msg: t("generate.notification.uploadConfirmation", { - name: selectedSite?.name, - }), - }), + const warningMessages = formatValidationIssues( + uploadResponse.warnings || [], ); + const confirmation = t("generate.notification.uploadConfirmation", { + name: selectedSite?.name, + }); + if (warningMessages.length) { + dispatch( + notificationsActions.setWarnNotification({ + title: `${confirmation} (warnings)`, + msg: [confirmation, ...warningMessages].join("\n"), + }), + ); + } else { + dispatch( + notificationsActions.setSuccessNotification({ + msg: confirmation, + }), + ); + } } }, [uploadResponse, uploading]); diff --git a/react-ui/src/app/components/Review/Review.test.tsx b/react-ui/src/app/components/Review/Review.test.tsx new file mode 100644 index 0000000..5343218 --- /dev/null +++ b/react-ui/src/app/components/Review/Review.test.tsx @@ -0,0 +1,106 @@ +import React from "react"; +import "@testing-library/jest-dom"; +import { Provider } from "react-redux"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { render, waitFor } from "../../utils/tests"; +import { createTestStore } from "../../redux/store"; +import Review from "./index"; +import { API } from "../../utils"; +import userEvent from "@testing-library/user-event"; + +vi.mock("../SubmoduleList", () => ({ + default: () => null, +})); + +vi.mock("../../contexts/ModulesContext", () => ({ + useModules: () => ({ + current: { + modules_order: [], + submodules_order: {}, + indicator_areas_order: [], + indicators_order: {}, + }, + }), +})); +vi.mock("../../utils", async () => { + const actual = await vi.importActual("../../utils"); + return { + ...actual, + API: { + post: vi.fn(), + }, + }; +}); + +const previewValidationError = { + response: { + status: 400, + data: { + valid: false, + errors: [ + { + code: "RELEVANT_VALIDATION_ERROR", + layer: "compatibility", + severity: "error", + message: "The same validation error", + }, + ], + warnings: [], + }, + }, +}; + +function renderReview() { + const store = createTestStore({ + submodules: { + isLoading: false, + error: null, + data: [], + selectedOptions: [], + }, + }); + + return render( + , + { + wrapper: ({ children }) => {children}, + }, + ); +} + +describe("Review preview notifications", () => { + beforeEach(() => { + vi.mocked(API.post).mockReset(); + vi.mocked(API.post).mockRejectedValue(previewValidationError as never); + }); + + it("shows the same validation error again after it is dismissed", async () => { + const user = userEvent.setup(); + const { getByRole, getByText, queryByRole } = renderReview(); + const previewButton = getByText("review.previewSurvey"); + + await user.click(previewButton); + await waitFor(() => expect(getByRole("api_errors")).toBeVisible()); + + const closeButton = getByRole("api_errors").querySelector("button"); + expect(closeButton).not.toBeNull(); + await user.click(closeButton as HTMLElement); + expect(queryByRole("api_errors")).not.toBeInTheDocument(); + + await user.click(previewButton); + await waitFor(() => expect(getByRole("api_errors")).toBeVisible()); + expect(getByRole("api_errors")).toHaveTextContent( + "The same validation error", + ); + expect(API.post).toHaveBeenCalledTimes(2); + }); +}); diff --git a/react-ui/src/app/components/Review/index.tsx b/react-ui/src/app/components/Review/index.tsx index 20d72e1..e6df79b 100644 --- a/react-ui/src/app/components/Review/index.tsx +++ b/react-ui/src/app/components/Review/index.tsx @@ -19,7 +19,6 @@ import { surveyFormActions } from "../../redux/reducers/surveyFormReducer"; import { fetchSubmodules } from "../../redux/actions/submodulesActions"; import { submodulesActions } from "../../redux/reducers/submodulesReducer"; import { API } from "../../utils"; -import { getApiErrorSummary } from "../../utils/apiError"; import { useModules } from "../../contexts/ModulesContext"; import { ChoiceTranslation, @@ -28,10 +27,17 @@ import { SubQuestion, Submodule, SubmoduleWithQuestions, + ValidationIssue, } from "../../types/api"; import { ApiError } from "../../types"; import { ReviewProps } from "./Review.interface"; import { getOrderedSubmodules } from "../../utils/generate"; +import { + formatValidationIssues, + getApiErrorSummary, + getApiErrorTitle, + parseApiError, +} from "../../utils/apiError"; import { getSavedSelectionsForSubmoduleItems, getSavedSubquestionIdsBySubmodule, @@ -59,9 +65,11 @@ function Review({ const [languages, setLanguages] = useState([]); const [subQuestions, setSubQuestions] = useState(surveyForm.sub_questions); const [previewData, setPreviewData] = useState<{ - errors: string[]; - warnings: string[]; + errors: Array; + warnings: Array; } | null>(null); + const [previewNotificationVersion, setPreviewNotificationVersion] = + useState(0); const [isPreviewing, setIsPreviewing] = useState(false); const [APIError, setAPIError] = useState(null); const modulesData = useModules(); @@ -156,10 +164,11 @@ function Review({ next((proceed) => { handleSubmit((data) => { data.submodules = getOrderedSubmodules(surveyForm, modulesData); - data.modules_order = modulesData.current.modules_order.filter((moduleId) => - modulesData.current.submodules_order[moduleId].some((submoduleId) => - data.submodules.includes(submoduleId), - ), + data.modules_order = modulesData.current.modules_order.filter( + (moduleId) => + modulesData.current.submodules_order[moduleId].some((submoduleId) => + data.submodules.includes(submoduleId), + ), ); data.submodules_order = data.submodules; data.indicator_areas_order = [ @@ -210,6 +219,8 @@ function Review({ }) .then((res) => { setPreviewData(res.data); + setPreviewNotificationVersion((version) => version + 1); + setAPIError(null); // eslint-disable-next-line no-console console.log("warnings", res.data.warnings); // eslint-disable-next-line no-console @@ -219,16 +230,17 @@ function Review({ } setIsPreviewing(false); }) - .catch((err) => { - setAPIError(err); + .catch(async (err) => { + setAPIError(await parseApiError(err)); + setPreviewNotificationVersion((version) => version + 1); setIsPreviewing(false); }); } - function wrapMessages(messages: string[]) { + function wrapMessages(messages: Array) { return (
- {messages.map((msg) => ( + {formatValidationIssues(messages).map((msg) => ( // eslint-disable-next-line react/jsx-key
{msg}
))} @@ -353,6 +365,7 @@ function Review({
{APIError && ( 0 && ( 0 && ( ; + field?: string; + sheet?: string; + column?: string; + row?: number; +} + +export interface ValidationValidator { + pyxform: string; + compatibility: string; +} + +export interface ValidationResult { + valid: boolean; + artifact_hash: string; + errors: ValidationIssue[]; + warnings: ValidationIssue[]; + validator: ValidationValidator; } export interface SuffixDict { diff --git a/react-ui/src/app/types/index.d.ts b/react-ui/src/app/types/index.d.ts index 24a9fa6..44480ea 100644 --- a/react-ui/src/app/types/index.d.ts +++ b/react-ui/src/app/types/index.d.ts @@ -1,12 +1,12 @@ import { AxiosError } from "axios"; import { Dispatch, SetStateAction } from "react"; -import { Indicator, IndicatorArea } from "./api"; +import { Indicator, IndicatorArea, ValidationIssue } from "./api"; interface StepCallback { ( proceed?: () => void, step?: number, - setStep?: Dispatch> + setStep?: Dispatch>, ): void; } @@ -30,5 +30,8 @@ export type ApiError = AxiosError<{ code?: number; service?: string; non_field_errors?: string[]; + errors?: ValidationIssue[]; + warnings?: ValidationIssue[]; + valid?: boolean; [key: string]: unknown; }>; diff --git a/react-ui/src/app/utils/apiError.test.tsx b/react-ui/src/app/utils/apiError.test.tsx new file mode 100644 index 0000000..be0f1a2 --- /dev/null +++ b/react-ui/src/app/utils/apiError.test.tsx @@ -0,0 +1,139 @@ +import { + formatValidationIssues, + getApiErrorStatus, + getApiErrorSummary, + getApiErrorTitle, + parseApiError, + parseValidationWarningsHeader, +} from "./apiError"; + +describe("API error handling", () => { + it("preserves generic nested error formatting", () => { + const error = { + response: { + status: 400, + data: { + message: "Request failed", + detail: "The survey could not be published.", + fields: { name: ["This field is required."] }, + }, + }, + }; + + const summary = getApiErrorSummary(error); + expect(summary).toContain("Request failed"); + expect(summary).toContain("fields.name: This field is required."); + expect(getApiErrorTitle(error, "Publish failed")).toBe("Publish failed"); + }); + + it("parses structured validation JSON returned as an Axios Blob", async () => { + const parsed = await parseApiError({ + response: { + status: 400, + data: new Blob([ + JSON.stringify({ + valid: false, + artifact_hash: "sha256:test", + errors: [ + { + code: "PYXFORM_CONVERSION_ERROR", + layer: "pyxform", + severity: "error", + message: "Unknown question type", + sheet: "survey", + column: "type", + row: 4, + }, + ], + warnings: [], + validator: { pyxform: "4.5.0", compatibility: "1.0" }, + }), + ]), + }, + }); + + expect(getApiErrorStatus(parsed)).toBe(400); + expect(getApiErrorTitle(parsed)).toBe("Survey validation failed"); + expect(getApiErrorSummary(parsed)).toContain( + "Unknown question type (sheet survey, column type, row 4)", + ); + }); + + it("distinguishes validator unavailability with HTTP 503", async () => { + const parsed = await parseApiError({ + response: { + status: 503, + data: new Blob([ + JSON.stringify({ + valid: false, + artifact_hash: "sha256:test", + errors: [ + { + code: "VALIDATOR_UNAVAILABLE", + layer: "validator", + severity: "error", + message: "Validator unavailable", + }, + ], + warnings: [], + validator: { pyxform: "4.5.0", compatibility: "1.0" }, + }), + ]), + }, + }); + + expect(getApiErrorStatus(parsed)).toBe(503); + expect(getApiErrorTitle(parsed)).toBe("Survey validation unavailable"); + expect(getApiErrorSummary(parsed)).toBe("Validator unavailable"); + }); + + it("formats legacy strings and retains fields absent from issue messages", () => { + expect( + formatValidationIssues([ + "legacy warning", + { + code: "XML_NAME_INVALID", + layer: "compatibility", + severity: "error", + message: "Invalid name", + field: "household_name", + }, + ]), + ).toEqual([ + "legacy warning", + "Invalid name (field household_name)", + ]); + }); + + it("omits a field location duplicated in the issue message", () => { + expect( + formatValidationIssues([ + { + code: "EXTERNAL_FILE_MISSING", + layer: "compatibility", + severity: "error", + message: + "External file 'test_fail_missing_choices.csv' could not be found.", + field: "test_fail_missing_choices.csv", + }, + ]), + ).toEqual([ + "External file 'test_fail_missing_choices.csv' could not be found.", + ]); + }); + + it("parses structured warning headers from binary responses", () => { + expect( + parseValidationWarningsHeader( + JSON.stringify([ + { + code: "PYXFORM_WARNING", + layer: "pyxform", + severity: "warning", + message: "A non-blocking warning", + }, + ]), + ), + ).toHaveLength(1); + }); +}); diff --git a/react-ui/src/app/utils/apiError.tsx b/react-ui/src/app/utils/apiError.tsx index 6fbd6e1..7617c04 100644 --- a/react-ui/src/app/utils/apiError.tsx +++ b/react-ui/src/app/utils/apiError.tsx @@ -1,4 +1,6 @@ import React, { ReactNode } from "react"; +import { ApiError } from "../types"; +import { ValidationIssue, ValidationResult } from "../types/api"; type UnknownRecord = Record; @@ -21,7 +23,67 @@ function pushMessage(messages: string[], message: unknown, prefix?: string) { messages.push(prefix ? `${humanizeKey(prefix)}: ${text}` : text); } +function isValidationIssue(value: unknown): value is ValidationIssue { + if (!isRecord(value)) return false; + + return ( + typeof value.code === "string" && + typeof value.layer === "string" && + typeof value.severity === "string" && + typeof value.message === "string" + ); +} + +function isValidationResult(value: unknown): value is ValidationResult { + if (!isRecord(value)) return false; + + return ( + typeof value.valid === "boolean" && + typeof value.artifact_hash === "string" && + Array.isArray(value.errors) && + Array.isArray(value.warnings) && + isRecord(value.validator) + ); +} + +function getStructuredValidationIssues(value: unknown): ValidationIssue[] { + if (isValidationResult(value)) return value.errors.filter(isValidationIssue); + if (isRecord(value) && Array.isArray(value.errors)) { + return value.errors.filter(isValidationIssue); + } + return []; +} + +function issueLocation(issue: ValidationIssue) { + const location = [ + issue.sheet && `sheet ${issue.sheet}`, + issue.column && `column ${issue.column}`, + issue.row !== undefined && `row ${issue.row}`, + issue.field && + !issue.message.includes(issue.field) && + `field ${issue.field}`, + ].filter(Boolean); + + return location.length ? ` (${location.join(", ")})` : ""; +} + +export function formatValidationIssues( + issues: Array, +): string[] { + return issues.map((issue) => { + if (typeof issue === "string") return issue; + return `${issue.message}${issueLocation(issue)}`; + }); +} + function collectMessages(value: unknown, messages: string[], prefix?: string) { + const validationIssues = getStructuredValidationIssues(value); + if (validationIssues.length) { + if (isRecord(value)) pushMessage(messages, value.message); + messages.push(...formatValidationIssues(validationIssues)); + return; + } + if (Array.isArray(value)) { value.forEach((item) => collectMessages(item, messages, prefix)); return; @@ -55,7 +117,7 @@ export function getApiErrorMessages( const fallbackMessage = (error as any)?.message || fallback; const messages: string[] = []; - if (responseData instanceof Blob) { + if (typeof Blob !== "undefined" && responseData instanceof Blob) { return [fallbackMessage]; } @@ -66,6 +128,73 @@ export function getApiErrorMessages( return uniqueMessages.length > 0 ? uniqueMessages : [fallbackMessage]; } +function readBlob(data: Blob): Promise { + if (typeof data.text === "function") return data.text(); + + if (typeof FileReader !== "undefined") { + return new Promise((resolve) => { + const reader = new FileReader(); + reader.onload = () => resolve(String(reader.result || "")); + reader.onerror = () => resolve(""); + reader.readAsText(data); + }); + } + + return Promise.resolve(""); +} + +async function readErrorPayload(data: unknown): Promise { + let payload = data; + if (typeof Blob !== "undefined" && data instanceof Blob) { + payload = await readBlob(data); + } + + if (typeof payload === "string") { + try { + return JSON.parse(payload); + } catch { + return payload; + } + } + + return payload; +} + +/** Decode a JSON error body returned as a Blob while preserving AxiosError shape. */ +export async function parseApiError(error: unknown): Promise { + if (!isRecord(error) || !isRecord(error.response)) { + return error as unknown as ApiError; + } + + const { response } = error; + if (!("data" in response)) return error as unknown as ApiError; + + const payload = await readErrorPayload(response.data); + if (payload === response.data) return error as unknown as ApiError; + + return { + ...error, + response: { + ...response, + data: payload, + }, + } as ApiError; +} + +export function getApiErrorStatus(error: unknown): number | undefined { + const status = (error as any)?.response?.status; + return typeof status === "number" ? status : undefined; +} + +export function getApiErrorTitle(error: unknown, fallback = "Error"): string { + const status = getApiErrorStatus(error); + if (isValidationResult((error as any)?.response?.data)) { + if (status === 400) return "Survey validation failed"; + if (status === 503) return "Survey validation unavailable"; + } + return fallback; +} + export function renderApiErrorMessage( error: unknown, fallback = "An unknown error occurred.", @@ -95,3 +224,16 @@ export function getApiErrorSummary( ) { return getApiErrorMessages(error, fallback).join(" "); } + +export function parseValidationWarningsHeader( + value: unknown, +): ValidationIssue[] { + if (typeof value !== "string") return []; + + try { + const parsed = JSON.parse(value); + return Array.isArray(parsed) ? parsed.filter(isValidationIssue) : []; + } catch { + return []; + } +} diff --git a/react-ui/src/app/utils/generate.test.tsx b/react-ui/src/app/utils/generate.test.tsx new file mode 100644 index 0000000..08ab0d8 --- /dev/null +++ b/react-ui/src/app/utils/generate.test.tsx @@ -0,0 +1,80 @@ +import type { AxiosResponse } from "axios"; +import { vi } from "vitest"; +import { API } from "."; +import { AppDispatch } from "../redux/store"; +import { generateDoc, getXLS } from "./generate"; + +vi.mock("./download", () => ({ + downloadFile: vi.fn(), +})); + +describe("survey generation organization headers", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + const dispatch = vi.fn() as unknown as AppDispatch; + const response = { + data: {}, + headers: {}, + } as unknown as AxiosResponse; + + it("sends shared survey organizations for XLSX and DOCX generation", async () => { + const post = vi + .spyOn(API, "post") + .mockReturnValue(Promise.resolve(response)); + const surveyForm = { + submodules: [1], + submodules_order: [], + subquestion_submodule_mapping: {}, + organizations: [{ id: 1 }], + }; + + await getXLS(dispatch, surveyForm, undefined, true); + generateDoc(dispatch, surveyForm, undefined, true); + + expect(post).toHaveBeenNthCalledWith( + 1, + "/generate/", + expect.anything(), + expect.objectContaining({ + headers: expect.objectContaining({ + "Survey-Designer-Organizations": "1", + }), + }), + ); + expect(post).toHaveBeenNthCalledWith( + 2, + "/generate-doc/", + expect.anything(), + expect.objectContaining({ + headers: expect.objectContaining({ + "Survey-Designer-Organizations": "1", + }), + }), + ); + }); + + it("omits the organization header when the survey has no organizations", async () => { + const post = vi + .spyOn(API, "post") + .mockReturnValue(Promise.resolve(response)); + const surveyForm = { + submodules: [1], + submodules_order: [], + subquestion_submodule_mapping: {}, + }; + + await getXLS(dispatch, surveyForm, undefined, true); + + expect(post).toHaveBeenCalledWith( + "/generate/", + expect.anything(), + expect.objectContaining({ + headers: expect.not.objectContaining({ + "Survey-Designer-Organizations": expect.anything(), + }), + }), + ); + }); +}); diff --git a/react-ui/src/app/utils/generate.tsx b/react-ui/src/app/utils/generate.tsx index d3da5ac..1dfe184 100644 --- a/react-ui/src/app/utils/generate.tsx +++ b/react-ui/src/app/utils/generate.tsx @@ -6,7 +6,13 @@ import { downloadFile } from "./download"; import { notificationsActions } from "../redux/reducers/notificationReducer"; import { docFetcherActions } from "../redux/reducers/docFetcherReducer"; import { AppDispatch } from "../redux/store"; -import { renderApiErrorMessage } from "./apiError"; +import { + formatValidationIssues, + getApiErrorTitle, + parseApiError, + parseValidationWarningsHeader, + renderApiErrorMessage, +} from "./apiError"; export function getOrderedSubmodules( surveyForm: any, @@ -78,6 +84,24 @@ export function getDataForGeneration( }; } +type SurveyFormOrganizations = { + organizations?: Array<{ id: number | string }>; +}; + +const getOrganizationHeaders = ( + surveyForm: SurveyFormOrganizations | null | undefined, +): Record => { + if (!surveyForm?.organizations?.length) { + return {}; + } + + return { + "Survey-Designer-Organizations": surveyForm.organizations + .map(({ id }) => id) + .join(","), + }; +}; + export const getXLS = ( dispatch: AppDispatch, surveyForm: any, @@ -92,6 +116,7 @@ export const getXLS = ( responseType: "blob", headers: { "X-CSRFToken": csrfToken, + ...getOrganizationHeaders(surveyForm), }, }) .then((res) => { @@ -106,12 +131,25 @@ export const getXLS = ( } downloadFile(res, timestamp, mimeType, extension); + const warnings = parseValidationWarningsHeader( + res.headers["x-survey-validation-warnings"] || + res.headers["x-validation-warnings"], + ); + if (warnings.length) { + dispatch( + notificationsActions.setWarnNotification({ + title: "Download completed with warnings", + msg: formatValidationIssues(warnings).join("\n"), + }), + ); + } }) - .catch((err) => { + .catch(async (err) => { + const parsedError = await parseApiError(err); dispatch( notificationsActions.setErrorNotification({ - msg: renderApiErrorMessage(err, "Error getting XLS file."), - title: "Error getting XLS file.", + msg: renderApiErrorMessage(parsedError, "Error getting XLS file."), + title: getApiErrorTitle(parsedError, "Error getting XLS file."), }), ); }); @@ -129,6 +167,7 @@ export const generateDoc = ( API.post("/generate-doc/", data, { headers: { "X-CSRFToken": csrfToken, + ...getOrganizationHeaders(surveyForm), }, }) .then((res) => {