From 0be5e39e6d988a276cd549bc4dbc2b66b3557bd2 Mon Sep 17 00:00:00 2001 From: Nicolas Senave Date: Mon, 18 Nov 2024 16:35:16 +0100 Subject: [PATCH 1/7] chore: improve issue templates --- .github/ISSUE_TEMPLATE/bug_report.md | 24 +++++++++++------------ .github/ISSUE_TEMPLATE/feature_request.md | 7 ------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 773904105..74747a528 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -3,32 +3,32 @@ name: Bug report about: Create a report to help us improve title: '' labels: bug -assignees: nsenave --- ## Describe the bug -A clear and concise description of what the bug is. +A clear and concise description of the bug. -- [ ] I tested it on the storybook, which leads me to believe that it's a lunatic bug. +Please indicate which formats are concerned: + +- Input : Pogues / DDI (if you don't knwo, leave both) +- Output : Lunatic / Xforms / Paper / Specifications / All formats ## To Reproduce -Steps to reproduce the behavior: +Please provide a clear description of the case where the generation is wrong, including: -1. With the survey available here/uploaded in issue -2. Go to '...' -3. Click on '....' -4. See error ... +- Context: DEFAULT / HOUSEHOLD / BUSINESS +- Mode: CAPI / CATI / CAWI / PAPI -## Expected behavior +or a parameters file if the error occurs with specific parameters. -A clear and concise description of what you expected to happen. +If the case is not precisely identified, please provide a Pogues json questionnaire, or eventually a DDI. -## Screenshots +## Expected output -If applicable, add screenshots to help explain your problem. +A clear and concise description of what should be generated. ## Version where the bug appeared diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index db60bb68d..85de58247 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -3,7 +3,6 @@ name: Feature request about: Suggest an idea for this project title: '' labels: enhancement -assignees: nsenave --- @@ -27,12 +26,6 @@ Have you considered any alternative solutions or workarounds? If yes, please des Provide any additional information or context that might help in understanding the feature request. -## Priority - -- [ ] Low -- [ ] Medium -- [ ] High - ## Attachments If applicable, include any mockups, diagrams, or additional files that help illustrate the feature. From a77584da6bdbf4e48cd45a1d6d32df0ae16ea577 Mon Sep 17 00:00:00 2001 From: RemiVerriez Date: Mon, 2 Dec 2024 09:14:31 +0100 Subject: [PATCH 2/7] fix: eno xml wrong zip name (#1163) --- build.gradle.kts | 2 +- .../GenerationCustomController.java | 8 ++---- .../GenerationStandardController.java | 13 ++++----- .../GenerationWithMappingController.java | 2 +- .../exception/EnoExceptionController.java | 12 +++++--- .../utils/EnoXmlControllerUtils.java | 28 +++++++++++++++---- .../eno/ws/controller/utils/HeadersUtils.java | 1 - .../ws/exception/EnoRedirectionException.java | 12 ++++++++ 8 files changed, 53 insertions(+), 25 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 54d21f454..a8233c340 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -16,7 +16,7 @@ java { allprojects { group = "fr.insee.eno" - version = "3.29.1-SNAPSHOT" + version = "3.29.2-SNAPSHOT.4" } subprojects { diff --git a/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationCustomController.java b/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationCustomController.java index 507214355..adad9e2a0 100644 --- a/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationCustomController.java +++ b/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationCustomController.java @@ -123,8 +123,7 @@ public ResponseEntity generateXformsCustomParams( addMultipartToBody(multipartBodyBuilder, specificTreatment, "specificTreatment"); // URI uri = xmlControllerUtils.newUriBuilder().path("questionnaire/ddi-2-xforms").build().toUri(); - String outFilename = questionnaireFilename(OutFormat.XFORMS, true); - return xmlControllerUtils.sendPostRequestByte(uri, multipartBodyBuilder, outFilename); + return xmlControllerUtils.sendPostRequestByte(uri, multipartBodyBuilder); } @Operation( @@ -136,7 +135,7 @@ public ResponseEntity generateXformsCustomParams( "You can get a parameters file by using the endpoint `/parameters/xml/{context}/FO`") @PostMapping(value = "ddi-2-fo", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE, consumes = MediaType.MULTIPART_FORM_DATA_VALUE) - public ResponseEntity generateFOCustomParams( + public ResponseEntity generateFOCustomParams( @RequestPart(value="in") MultipartFile in, @RequestPart(value="params") MultipartFile params, @RequestPart(value="metadata") MultipartFile metadata, @@ -152,8 +151,7 @@ public ResponseEntity generateFOCustomParams( addMultipartToBody(multipartBodyBuilder, specificTreatment, "specificTreatment"); // URI uri = xmlControllerUtils.newUriBuilder().path("questionnaire/ddi-2-fo").build().toUri(); - String outFilename = questionnaireFilename(OutFormat.XFORMS, false); - return xmlControllerUtils.sendPostRequest(uri, multipartBodyBuilder, outFilename); + return xmlControllerUtils.sendPostRequestByte(uri, multipartBodyBuilder); } } diff --git a/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationStandardController.java b/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationStandardController.java index ef26075db..c939e62af 100644 --- a/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationStandardController.java +++ b/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationStandardController.java @@ -149,8 +149,7 @@ public ResponseEntity generateXforms( .path("/questionnaire/{context}/xforms") .queryParam("multi-model", multiModel) .build(context); - String outFilename = questionnaireFilename(OutFormat.XFORMS, multiModel); - return xmlControllerUtils.sendPostRequestByte(uri, multipartBodyBuilder, outFilename); + return xmlControllerUtils.sendPostRequestByte(uri, multipartBodyBuilder); } @Operation( @@ -164,7 +163,7 @@ public ResponseEntity generateXforms( "If the multi-model option is set to true, the output questionnaire(s) are put in a zip file." ) @PostMapping(value = "{context}/fo", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE, consumes = MediaType.MULTIPART_FORM_DATA_VALUE) - public ResponseEntity generateFO( + public ResponseEntity generateFO( @RequestPart(value="in") MultipartFile in, @RequestPart(value="metadata", required = false) MultipartFile metadata, @RequestPart(value="specificTreatment", required=false) MultipartFile specificTreatment, @@ -192,8 +191,7 @@ public ResponseEntity generateFO( .queryParam("Capture", capture) .queryParam("multi-model", multiModel) .build(context); - String outFilename = questionnaireFilename(OutFormat.FO, multiModel); - return xmlControllerUtils.sendPostRequest(uri, multipartBodyBuilder, outFilename); + return xmlControllerUtils.sendPostRequestByte(uri, multipartBodyBuilder); } @Operation( @@ -203,7 +201,7 @@ public ResponseEntity generateFO( "context.") @PostMapping(value = "{context}/fodt", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE, consumes = MediaType.MULTIPART_FORM_DATA_VALUE) - public ResponseEntity generateFODT( + public ResponseEntity generateFODT( @RequestPart(value="in") MultipartFile in, @PathVariable Context context) throws EnoControllerException { // @@ -211,8 +209,7 @@ public ResponseEntity generateFODT( addMultipartToBody(multipartBodyBuilder, in, "in"); // URI uri = xmlControllerUtils.newUriBuilder().path("/questionnaire/{context}/fodt").build(context); - String outFilename = questionnaireFilename(OutFormat.FODT, false); - return xmlControllerUtils.sendPostRequest(uri, multipartBodyBuilder, outFilename); + return xmlControllerUtils.sendPostRequestByte(uri, multipartBodyBuilder); } } diff --git a/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationWithMappingController.java b/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationWithMappingController.java index eae89f0af..abcf7a7f1 100644 --- a/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationWithMappingController.java +++ b/eno-ws/src/main/java/fr/insee/eno/ws/controller/GenerationWithMappingController.java @@ -61,7 +61,7 @@ public ResponseEntity generate( addMultipartToBody(multipartBodyBuilder, mapping, "mapping"); // URI uri = xmlControllerUtils.newUriBuilder().path("questionnaire/in-2-out").build().toUri(); - String outFilename = multiModel ? "questionnaire.zip" : "questionnaire.txt"; + String outFilename = multiModel ? "questionnaires.zip" : "questionnaire.txt"; return xmlControllerUtils.sendPostRequest(uri, multipartBodyBuilder, outFilename); } diff --git a/eno-ws/src/main/java/fr/insee/eno/ws/controller/exception/EnoExceptionController.java b/eno-ws/src/main/java/fr/insee/eno/ws/controller/exception/EnoExceptionController.java index 75f467ca1..485e24852 100644 --- a/eno-ws/src/main/java/fr/insee/eno/ws/controller/exception/EnoExceptionController.java +++ b/eno-ws/src/main/java/fr/insee/eno/ws/controller/exception/EnoExceptionController.java @@ -3,10 +3,7 @@ import fr.insee.eno.core.exceptions.business.EnoParametersException; import fr.insee.eno.treatments.exceptions.SpecificTreatmentsDeserializationException; import fr.insee.eno.treatments.exceptions.SpecificTreatmentsValidationException; -import fr.insee.eno.ws.exception.ContextException; -import fr.insee.eno.ws.exception.MetadataFileException; -import fr.insee.eno.ws.exception.ModeParameterException; -import fr.insee.eno.ws.exception.MultiModelException; +import fr.insee.eno.ws.exception.*; import lombok.extern.slf4j.Slf4j; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -59,6 +56,13 @@ public ResponseEntity exception(IOException ioException) { return new ResponseEntity<>("I/O error: " + ioException.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR); } + @ExceptionHandler(EnoRedirectionException.class) + public ResponseEntity exception(EnoRedirectionException enoRedirectionException) { + String message = enoRedirectionException.getMessage(); + log.error(message, enoRedirectionException); + return new ResponseEntity<>(message, enoRedirectionException.getHttpStatusCode()); + } + @ExceptionHandler(value = Exception.class) public ResponseEntity exception(Exception exception) { log.error("Unhandled exception thrown in controller: ", exception); diff --git a/eno-ws/src/main/java/fr/insee/eno/ws/controller/utils/EnoXmlControllerUtils.java b/eno-ws/src/main/java/fr/insee/eno/ws/controller/utils/EnoXmlControllerUtils.java index 139d98fc5..6b5271436 100644 --- a/eno-ws/src/main/java/fr/insee/eno/ws/controller/utils/EnoXmlControllerUtils.java +++ b/eno-ws/src/main/java/fr/insee/eno/ws/controller/utils/EnoXmlControllerUtils.java @@ -1,9 +1,11 @@ package fr.insee.eno.ws.controller.utils; import fr.insee.eno.ws.exception.EnoControllerException; +import fr.insee.eno.ws.exception.EnoRedirectionException; import fr.insee.eno.ws.legacy.parameters.OutFormat; import org.springframework.beans.factory.annotation.Value; import org.springframework.core.io.ByteArrayResource; +import org.springframework.http.HttpStatusCode; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.client.MultipartBodyBuilder; @@ -60,17 +62,33 @@ public ResponseEntity sendPostRequest(URI uri, MultipartBodyBuilder mult .body(result); } - public ResponseEntity sendPostRequestByte(URI uri, MultipartBodyBuilder multipartBodyBuilder, String outFilename) { - byte[] result = webClient.post() + public ResponseEntity sendPostRequestByte(URI uri, MultipartBodyBuilder multipartBodyBuilder) { + ResponseEntity responseEntity = webClient.post() .uri(uri) .accept(MediaType.APPLICATION_OCTET_STREAM) .contentType(MediaType.MULTIPART_FORM_DATA) .body(BodyInserters.fromMultipartData(multipartBodyBuilder.build())) - .exchangeToMono(clientResponse -> clientResponse.bodyToMono(byte[].class)) + .retrieve() + .onStatus( + HttpStatusCode::is4xxClientError, + clientResponse -> clientResponse.bodyToMono(String.class) + .map(responseBody -> + new EnoRedirectionException(responseBody, clientResponse.statusCode())) + ) + .onStatus( + HttpStatusCode::is5xxServerError, + clientResponse -> clientResponse.bodyToMono(String.class) + .map(responseBody -> + new EnoRedirectionException("Server error: " + responseBody, clientResponse.statusCode())) + ) + .toEntity(byte[].class) .block(); + if (responseEntity == null) + throw new EnoRedirectionException("null result from Eno Xml call."); + String fileName = responseEntity.getHeaders().getContentDisposition().getFilename(); return ResponseEntity.ok() - .headers(HeadersUtils.with(outFilename)) - .body(result); + .headers(HeadersUtils.with(fileName)) + .body(responseEntity.getBody()); } public ResponseEntity sendPostRequest(URI uri) { diff --git a/eno-ws/src/main/java/fr/insee/eno/ws/controller/utils/HeadersUtils.java b/eno-ws/src/main/java/fr/insee/eno/ws/controller/utils/HeadersUtils.java index 6302a45bc..03ccf3623 100644 --- a/eno-ws/src/main/java/fr/insee/eno/ws/controller/utils/HeadersUtils.java +++ b/eno-ws/src/main/java/fr/insee/eno/ws/controller/utils/HeadersUtils.java @@ -13,5 +13,4 @@ public static HttpHeaders with(String fileName) { headers.setContentType(MediaType.APPLICATION_OCTET_STREAM); return headers; } - } diff --git a/eno-ws/src/main/java/fr/insee/eno/ws/exception/EnoRedirectionException.java b/eno-ws/src/main/java/fr/insee/eno/ws/exception/EnoRedirectionException.java index 45a0ae71a..10189c713 100644 --- a/eno-ws/src/main/java/fr/insee/eno/ws/exception/EnoRedirectionException.java +++ b/eno-ws/src/main/java/fr/insee/eno/ws/exception/EnoRedirectionException.java @@ -1,9 +1,21 @@ package fr.insee.eno.ws.exception; +import lombok.Getter; +import org.springframework.http.HttpStatusCode; + +@Getter public class EnoRedirectionException extends RuntimeException { + private final HttpStatusCode httpStatusCode; + public EnoRedirectionException(String message) { super(message); + httpStatusCode = null; + } + + public EnoRedirectionException(String message, HttpStatusCode httpStatusCode) { + super(message); + this.httpStatusCode = httpStatusCode; } } From c62ad3c1a5fb6aca1c116cfceca8d5382e1b4a3b Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 2 Dec 2024 09:33:07 +0100 Subject: [PATCH 3/7] chore(deps): update all minor dependencies (#1158) --- eno-treatments/build.gradle.kts | 2 +- gradle/wrapper/gradle-wrapper.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/eno-treatments/build.gradle.kts b/eno-treatments/build.gradle.kts index b75896a15..f5d141397 100644 --- a/eno-treatments/build.gradle.kts +++ b/eno-treatments/build.gradle.kts @@ -19,7 +19,7 @@ tasks.named("jar") { enabled = true } -val jsonSchemaValidatorVersion = "1.5.3" +val jsonSchemaValidatorVersion = "1.5.4" dependencies { // Eno core diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 94113f200..e2847c820 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.11-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.11.1-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME From b6f918c77baaf82eb1170bf601fc64b0da4c9ced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20S=C3=A9nave?= <59770457+nsenave@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:03:42 +0100 Subject: [PATCH 4/7] refactor: improve exception handling in ddi insert labels step (#1167) --- .../insee/eno/core/model/code/CodeList.java | 7 ++++ .../eno/core/model/question/Question.java | 2 +- .../ddi/DDIInsertMultipleChoiceLabels.java | 10 +++++- .../DDIInsertMultipleChoiceLabelsTest.java | 33 +++++++++++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/eno-core/src/main/java/fr/insee/eno/core/model/code/CodeList.java b/eno-core/src/main/java/fr/insee/eno/core/model/code/CodeList.java index 09d4bb62e..530e242c3 100644 --- a/eno-core/src/main/java/fr/insee/eno/core/model/code/CodeList.java +++ b/eno-core/src/main/java/fr/insee/eno/core/model/code/CodeList.java @@ -134,4 +134,11 @@ private void computeHorizontalSizes(CodeItem codeItem) { } } + @Override + public String toString() { + return "CodeList[" + + "id='" + this.getId() + '\'' + + ", name='" + name + '\'' + + ']'; + } } diff --git a/eno-core/src/main/java/fr/insee/eno/core/model/question/Question.java b/eno-core/src/main/java/fr/insee/eno/core/model/question/Question.java index a99581d3b..7b7ae9a8e 100644 --- a/eno-core/src/main/java/fr/insee/eno/core/model/question/Question.java +++ b/eno-core/src/main/java/fr/insee/eno/core/model/question/Question.java @@ -57,7 +57,7 @@ public abstract class Question extends EnoIdentifiableObject implements EnoCompo @Override public String toString() { - return this.getClass() + "[id="+this.getId()+", name="+getName()+"]"; + return this.getClass().getSimpleName() + "[id="+this.getId()+", name="+getName()+"]"; } } diff --git a/eno-core/src/main/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertMultipleChoiceLabels.java b/eno-core/src/main/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertMultipleChoiceLabels.java index 3282a26e9..638e08d9b 100644 --- a/eno-core/src/main/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertMultipleChoiceLabels.java +++ b/eno-core/src/main/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertMultipleChoiceLabels.java @@ -1,5 +1,6 @@ package fr.insee.eno.core.processing.in.steps.ddi; +import fr.insee.eno.core.exceptions.business.IllegalDDIElementException; import fr.insee.eno.core.model.EnoQuestionnaire; import fr.insee.eno.core.model.code.CodeItem; import fr.insee.eno.core.model.code.CodeList; @@ -37,7 +38,14 @@ public void apply(EnoQuestionnaire enoQuestionnaire) { private void insertModalityLabels(SimpleMultipleChoiceQuestion simpleMultipleChoiceQuestion) { CodeList codeList = codeListMap.get(simpleMultipleChoiceQuestion.getCodeListReference()); - for (int i = 0; i < codeList.size(); i ++) { + int codeListSize = codeList.size(); + int responsesSize = simpleMultipleChoiceQuestion.getCodeResponses().size(); + if (codeListSize != responsesSize) + throw new IllegalDDIElementException(String.format( + "Code list '%s' (id=%s) has %s codes, and is used in multiple choice question '%s' (id=%s) that has %s responses.", + codeList.getName(), codeList.getId(), codeListSize, + simpleMultipleChoiceQuestion.getName(), simpleMultipleChoiceQuestion.getId(), responsesSize)); + for (int i = 0; i < codeListSize; i ++) { CodeItem codeItem = codeList.getCodeItems().get(i); CodeResponse codeResponse = simpleMultipleChoiceQuestion.getCodeResponses().get(i); codeResponse.setLabel(codeItem.getLabel()); diff --git a/eno-core/src/test/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertMultipleChoiceLabelsTest.java b/eno-core/src/test/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertMultipleChoiceLabelsTest.java index 6095b1a2b..879752220 100644 --- a/eno-core/src/test/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertMultipleChoiceLabelsTest.java +++ b/eno-core/src/test/java/fr/insee/eno/core/processing/in/steps/ddi/DDIInsertMultipleChoiceLabelsTest.java @@ -2,18 +2,51 @@ import fr.insee.ddi.lifecycle33.instance.DDIInstanceDocument; import fr.insee.eno.core.exceptions.business.DDIParsingException; +import fr.insee.eno.core.exceptions.business.IllegalDDIElementException; import fr.insee.eno.core.mappers.DDIMapper; import fr.insee.eno.core.model.EnoQuestionnaire; +import fr.insee.eno.core.model.code.CodeItem; +import fr.insee.eno.core.model.code.CodeList; import fr.insee.eno.core.model.question.SimpleMultipleChoiceQuestion; +import fr.insee.eno.core.model.response.CodeResponse; import fr.insee.eno.core.serialize.DDIDeserializer; import org.junit.jupiter.api.Test; import java.util.List; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; class DDIInsertMultipleChoiceLabelsTest { + @Test + void failingCase_tooManyCodes() { + // Given + EnoQuestionnaire enoQuestionnaire = new EnoQuestionnaire(); + // + CodeList codeList = new CodeList(); + codeList.setId("code-list-id"); + codeList.setName("CODE_LIST_NAME"); + codeList.getCodeItems().add(new CodeItem()); + codeList.getCodeItems().add(new CodeItem()); + enoQuestionnaire.getCodeLists().add(codeList); + // + SimpleMultipleChoiceQuestion simpleMCQ = new SimpleMultipleChoiceQuestion(); + simpleMCQ.setId("question-id"); + simpleMCQ.setName("QUESTION_NAME"); + simpleMCQ.setCodeListReference("code-list-id"); + simpleMCQ.getCodeResponses().add(new CodeResponse()); + enoQuestionnaire.getMultipleResponseQuestions().add(simpleMCQ); + // When + Then + DDIInsertMultipleChoiceLabels processing = new DDIInsertMultipleChoiceLabels(); + assertThatThrownBy(() -> processing.apply(enoQuestionnaire)) + .isInstanceOf(IllegalDDIElementException.class) + .hasMessageContaining("code-list-id") + .hasMessageContaining("CODE_LIST_NAME") + .hasMessageContaining("question-id") + .hasMessageContaining("QUESTION_NAME"); + } + @Test void integrationTest() throws DDIParsingException { // From 6cc98e13aed7d66b40bd7689cfdc234e472e396d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20S=C3=A9nave?= <59770457+nsenave@users.noreply.github.com> Date: Fri, 6 Dec 2024 15:56:27 +0100 Subject: [PATCH 5/7] feat(lunatic): format control for the year of date questions (#1168) * feat(lunatic): format control for the year of date questions * refactor: some java 21+ syntax refactor * refactor: some clean up in test * chore: bump version * feat: add 'at least 4 digits' in control --- build.gradle.kts | 2 +- .../lunatic/LunaticAddControlFormat.java | 31 ++++- .../lunatic/LunaticAddControlFormatTest.java | 128 +++++++++++++----- 3 files changed, 117 insertions(+), 44 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 3633fa061..51772d668 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -16,7 +16,7 @@ java { allprojects { group = "fr.insee.eno" - version = "3.29.3-SNAPSHOT" + version = "3.30.0-SNAPSHOT.1" } subprojects { diff --git a/eno-core/src/main/java/fr/insee/eno/core/processing/out/steps/lunatic/LunaticAddControlFormat.java b/eno-core/src/main/java/fr/insee/eno/core/processing/out/steps/lunatic/LunaticAddControlFormat.java index 2e03a0d5f..5421c1dfb 100644 --- a/eno-core/src/main/java/fr/insee/eno/core/processing/out/steps/lunatic/LunaticAddControlFormat.java +++ b/eno-core/src/main/java/fr/insee/eno/core/processing/out/steps/lunatic/LunaticAddControlFormat.java @@ -2,6 +2,7 @@ import fr.insee.eno.core.processing.ProcessingStep; import fr.insee.lunatic.model.flat.*; +import lombok.extern.slf4j.Slf4j; import java.math.BigDecimal; import java.math.RoundingMode; @@ -14,6 +15,7 @@ * Processing adding format controls to components * Format controls are controls generated by the min/max/decimals attributes of the Datepicker/InputNumber components */ +@Slf4j public class LunaticAddControlFormat implements ProcessingStep { /** * @@ -114,21 +116,21 @@ private List getFormatControlsFromInputNumberAttributes(String id, String maxValue = formatDoubleValue(max, decimalsCount); String controlExpression = String.format("not(not(isnull(%s)) and (%s>%s or %s<%s))", responseName, minValue, responseName, maxValue, responseName); String controlErrorMessage = String.format("\" La valeur doit être comprise entre %s et %s.\"", minValue, maxValue); - controls.add(0, createFormatControl(controlIdPrefix+"-borne-inf-sup", controlExpression, controlErrorMessage)); + controls.addFirst(createFormatControl(controlIdPrefix+"-borne-inf-sup", controlExpression, controlErrorMessage)); } if(min == null && max != null) { String maxValue = formatDoubleValue(max, decimalsCount); String controlExpression = String.format("not(not(isnull(%s)) and %s<%s)", responseName, maxValue, responseName); String controlErrorMessage = String.format("\" La valeur doit être inférieure à %s.\"", maxValue); - controls.add(0, createFormatControl(controlIdPrefix+"-borne-sup", controlExpression, controlErrorMessage)); + controls.addFirst(createFormatControl(controlIdPrefix+"-borne-sup", controlExpression, controlErrorMessage)); } if(min != null && max == null) { String minValue = formatDoubleValue(min, decimalsCount); String controlExpression = String.format("not(not(isnull(%s)) and %s>%s)", responseName, minValue, responseName); String controlErrorMessage = String.format("\" La valeur doit être supérieure à %s.\"", minValue); - controls.add(0, createFormatControl(controlIdPrefix+"-borne-inf", controlExpression, controlErrorMessage)); + controls.addFirst(createFormatControl(controlIdPrefix+"-borne-inf", controlExpression, controlErrorMessage)); } controls.add(createDecimalsFormatControl(controlIdPrefix, responseName, decimalsCount)); @@ -148,9 +150,12 @@ private void createFormatControlsForDatepicker(Datepicker datepicker) { List controls = datepicker.getControls(); - Optional control = getFormatControlFromDatepickerAttributes(id, minValue, maxValue, format, responseName); - - control.ifPresent(controlType -> controls.add(0, controlType)); + Optional controlYearFormat = generateDatepickerYearControl(id, format, responseName); + Optional controlBounds = getFormatControlFromDatepickerAttributes(id, minValue, maxValue, format, responseName); + controlBounds.ifPresent(controls::addFirst); + controlYearFormat.ifPresent(controls::addFirst); + // Note: it's important that the year format is added in first position, since in some cases only the message + // of the first control is displayed. } /** @@ -188,6 +193,20 @@ private Optional getFormatControlFromDatepickerAttributes(String id } return Optional.empty(); } + private Optional generateDatepickerYearControl(String id, String format, String responseName) { + if (format == null || !format.contains("YYYY")) { + log.warn("Datepicker '{}' (id={}) format is {} which doesn't have the year (YYYY).", + responseName, id, format); + return Optional.empty(); + } + String controlId = id + "-format-year"; + String expression = String.format("not(not(isnull(%s)) and (" + + "cast(cast(cast(%s, date, \"%s\"), string, \"YYYY\"), integer) <= 999 or " + + "cast(cast(cast(%s, date, \"%s\"), string, \"YYYY\"), integer) > 9999))", + responseName, responseName, format, responseName, format); + String message = "\"L'année doit être saisie avec 4 chiffres.\""; + return Optional.of(createFormatControl(controlId, expression, message)); + } /** * diff --git a/eno-core/src/test/java/fr/insee/eno/core/processing/out/steps/lunatic/LunaticAddControlFormatTest.java b/eno-core/src/test/java/fr/insee/eno/core/processing/out/steps/lunatic/LunaticAddControlFormatTest.java index 52e8491bb..fdb6e3405 100644 --- a/eno-core/src/test/java/fr/insee/eno/core/processing/out/steps/lunatic/LunaticAddControlFormatTest.java +++ b/eno-core/src/test/java/fr/insee/eno/core/processing/out/steps/lunatic/LunaticAddControlFormatTest.java @@ -2,6 +2,7 @@ import fr.insee.lunatic.model.flat.*; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import java.math.BigInteger; @@ -25,13 +26,13 @@ void init() { datePicker = new Datepicker(); datePicker.setComponentType(ComponentTypeEnum.DATEPICKER); datePicker.setId("datepicker-id"); - datePicker.setResponse(buildResponse("DATEVAR")); + datePicker.setResponse(buildResponse("DATE_VAR")); number = new InputNumber(); number.setComponentType(ComponentTypeEnum.INPUT_NUMBER); number.setId("number-id"); number.setDecimals(BigInteger.ZERO); - number.setResponse(buildResponse("NUMBERVAR")); + number.setResponse(buildResponse("NUMBER_VAR")); } @Test @@ -45,9 +46,9 @@ void shouldInputNumberHaveDecimalsControl() { List controls = number.getControls(); assertEquals(1, controls.size()); - ControlType control = controls.get(0); + ControlType control = controls.getFirst(); - assertEquals("not(not(isnull(NUMBERVAR)) and round(NUMBERVAR,10)<>NUMBERVAR)", control.getControl().getValue()); + assertEquals("not(not(isnull(NUMBER_VAR)) and round(NUMBER_VAR,10)<>NUMBER_VAR)", control.getControl().getValue()); assertEquals(LabelTypeEnum.VTL, control.getControl().getType()); assertEquals(LabelTypeEnum.VTL_MD, control.getErrorMessage().getType()); assertEquals("number-id-format-decimal", control.getId()); @@ -68,8 +69,8 @@ void shouldInputNumberHaveScaleOnDecimalValuesWhenDecimalsIsSet() { List controls = number.getControls(); - ControlType control = controls.get(0); - assertEquals("not(not(isnull(NUMBERVAR)) and (5.2400000000>NUMBERVAR or 10.1200000000NUMBER_VAR or 10.1200000000 controls = number.getControls(); assertEquals(2, controls.size()); - ControlType control = controls.get(0); + ControlType control = controls.getFirst(); - assertEquals("not(not(isnull(NUMBERVAR)) and (5>NUMBERVAR or 10NUMBER_VAR or 10 controls = number.getControls(); assertEquals(2, controls.size()); - ControlType control = controls.get(0); + ControlType control = controls.getFirst(); - assertEquals("not(not(isnull(NUMBERVAR)) and 5>NUMBERVAR)", control.getControl().getValue()); + assertEquals("not(not(isnull(NUMBER_VAR)) and 5>NUMBER_VAR)", control.getControl().getValue()); assertEquals(LabelTypeEnum.VTL, control.getControl().getType()); assertEquals(LabelTypeEnum.VTL_MD, control.getErrorMessage().getType()); assertEquals("number-id-format-borne-inf", control.getId()); @@ -131,9 +132,9 @@ void shouldInputNumberHaveMaxFormatControl() { List controls = number.getControls(); assertEquals(2, controls.size()); - ControlType control = controls.get(0); + ControlType control = controls.getFirst(); - assertEquals("not(not(isnull(NUMBERVAR)) and 10 controls = datePicker.getControls(); + assertEquals(1, controls.size()); + ControlType yearControl = controls.getFirst(); + + assertEquals("datepicker-id-format-year", yearControl.getId()); + String expected = "not(not(isnull(DATE_VAR)) and (" + + "cast(cast(cast(DATE_VAR, date, \"YYYY-MM-DD\"), string, \"YYYY\"), integer) <= 999 or " + + "cast(cast(cast(DATE_VAR, date, \"YYYY-MM-DD\"), string, \"YYYY\"), integer) > 9999))"; + assertEquals(expected, yearControl.getControl().getValue()); + assertEquals(LabelTypeEnum.VTL, yearControl.getControl().getType()); + assertEquals(LabelTypeEnum.VTL_MD, yearControl.getErrorMessage().getType()); + assertEquals(ControlTypeEnum.FORMAT, yearControl.getTypeOfControl()); + assertEquals(ControlCriticalityEnum.ERROR, yearControl.getCriticality()); + } + + @Test + @DisplayName("Datepicker: min and max format control") + void datepickerMinMaxFormatControl() { lunaticQuestionnaire = new Questionnaire(); lunaticQuestionnaire.getComponents().add(datePicker); datePicker.setMin("2020-01-01"); @@ -159,19 +186,23 @@ void shouldDatepickerHaveMinMaxFormatControl() { processing.apply(lunaticQuestionnaire); List controls = datePicker.getControls(); - assertEquals(1, controls.size()); - ControlType control = controls.get(0); + assertEquals(2, controls.size()); + ControlType control = controls.get(1); - assertEquals("not(not(isnull(DATEVAR)) and (cast(DATEVAR, date, \"YYYY-MM-DD\")cast(\"2023-01-01\", date, \"YYYY-MM-DD\")))", control.getControl().getValue()); + assertEquals("datepicker-id-format-date-borne-inf-sup", control.getId()); + String expected = "not(not(isnull(DATE_VAR)) and " + + "(cast(DATE_VAR, date, \"YYYY-MM-DD\")cast(\"2023-01-01\", date, \"YYYY-MM-DD\")))"; + assertEquals(expected, control.getControl().getValue()); assertEquals(LabelTypeEnum.VTL, control.getControl().getType()); assertEquals(LabelTypeEnum.VTL_MD, control.getErrorMessage().getType()); - assertEquals("datepicker-id-format-date-borne-inf-sup", control.getId()); assertEquals(ControlTypeEnum.FORMAT, control.getTypeOfControl()); assertEquals(ControlCriticalityEnum.ERROR, control.getCriticality()); } @Test - void shouldDatepickerHaveMinFormatControl() { + @DisplayName("Datepicker: min format control") + void datepickerMinFormatControl() { lunaticQuestionnaire = new Questionnaire(); lunaticQuestionnaire.getComponents().add(datePicker); datePicker.setMin("2020-01-01"); @@ -179,19 +210,22 @@ void shouldDatepickerHaveMinFormatControl() { processing.apply(lunaticQuestionnaire); List controls = datePicker.getControls(); - assertEquals(1, controls.size()); - ControlType control = controls.get(0); + assertEquals(2, controls.size()); + ControlType control = controls.get(1); - assertEquals("not(not(isnull(DATEVAR)) and (cast(DATEVAR, date, \"YYYY-MM-DD\") controls = datePicker.getControls(); - assertEquals(1, controls.size()); - ControlType control = controls.get(0); + assertEquals(2, controls.size()); + ControlType boundsControl = controls.get(1); + + assertEquals("datepicker-id-format-date-borne-sup", boundsControl.getId()); + String expected = "not(not(isnull(DATE_VAR)) " + + "and (cast(DATE_VAR, date, \"YYYY-MM-DD\")>cast(\"2023-01-01\", date, \"YYYY-MM-DD\")))"; + assertEquals(expected, boundsControl.getControl().getValue()); + assertEquals(LabelTypeEnum.VTL, boundsControl.getControl().getType()); + assertEquals(LabelTypeEnum.VTL_MD, boundsControl.getErrorMessage().getType()); + assertEquals(ControlTypeEnum.FORMAT, boundsControl.getTypeOfControl()); + assertEquals(ControlCriticalityEnum.ERROR, boundsControl.getCriticality()); + } - assertEquals("not(not(isnull(DATEVAR)) and (cast(DATEVAR, date, \"YYYY-MM-DD\")>cast(\"2023-01-01\", date, \"YYYY-MM-DD\")))", control.getControl().getValue()); - assertEquals(LabelTypeEnum.VTL, control.getControl().getType()); - assertEquals(LabelTypeEnum.VTL_MD, control.getErrorMessage().getType()); - assertEquals("datepicker-id-format-date-borne-sup", control.getId()); - assertEquals(ControlTypeEnum.FORMAT, control.getTypeOfControl()); - assertEquals(ControlCriticalityEnum.ERROR, control.getCriticality()); + @Test + @DisplayName("Datepicker: year format control when min/max is set") + void datepickerYearAndMinMaxFormatControl() { + lunaticQuestionnaire = new Questionnaire(); + lunaticQuestionnaire.getComponents().add(datePicker); + datePicker.setMax("2023-01-01"); + datePicker.setDateFormat("YYYY-MM-DD"); + processing.apply(lunaticQuestionnaire); + + List controls = datePicker.getControls(); + assertEquals(2, controls.size()); + ControlType yearControl = controls.get(0); + ControlType boundsControl = controls.get(1); + // Only testing ids here to check that the order is right, content is tested in other tests + assertEquals("datepicker-id-format-year", yearControl.getId()); + assertEquals("datepicker-id-format-date-borne-sup", boundsControl.getId()); } @Test @@ -226,7 +280,7 @@ void shouldLoopComponentsBeProcessed() { processing.apply(lunaticQuestionnaire); - assertEquals(1, datePicker.getControls().size()); + assertEquals(2, datePicker.getControls().size()); assertEquals(2, number.getControls().size()); } @@ -240,12 +294,12 @@ void shouldTableComponentsHaveSubComponentsControls() { List bodyCells = new ArrayList<>(); bodyCells.add(buildBodyCell("line1")); - bodyCells.add(buildBodyCell(table.getId()+"-co", "CHECKVAR", ComponentTypeEnum.CHECKBOX_ONE)); + bodyCells.add(buildBodyCell(table.getId()+"-co", "CHECKBOX_VAR", ComponentTypeEnum.CHECKBOX_ONE)); bodyLines.add(buildBodyLine(bodyCells)); bodyCells = new ArrayList<>(); bodyCells.add(buildBodyCell("line2")); - bodyCells.add(buildBodyCell(table.getId()+"-number", "NUMBERVAR", ComponentTypeEnum.INPUT_NUMBER, BigInteger.TWO, 2.0, 5.0)); + bodyCells.add(buildBodyCell(table.getId()+"-number", "NUMBER_VAR", ComponentTypeEnum.INPUT_NUMBER, BigInteger.TWO, 2.0, 5.0)); bodyLines.add(buildBodyLine(bodyCells)); lunaticQuestionnaire = new Questionnaire(); @@ -266,8 +320,8 @@ void shouldRosterComponentsHaveSubComponentsControls() { List bodyCells = roster.getComponents(); bodyCells.add(buildBodyCell("line1")); - bodyCells.add(buildBodyCell(roster.getId()+"-number", "NUMBERVAR", ComponentTypeEnum.INPUT_NUMBER, BigInteger.TWO, 2.0, 5.0)); - bodyCells.add(buildBodyCell(roster.getId()+"-co", "CHECKVAR", ComponentTypeEnum.CHECKBOX_ONE)); + bodyCells.add(buildBodyCell(roster.getId()+"-number", "NUMBER_VAR", ComponentTypeEnum.INPUT_NUMBER, BigInteger.TWO, 2.0, 5.0)); + bodyCells.add(buildBodyCell(roster.getId()+"-co", "CHECKBOX_VAR", ComponentTypeEnum.CHECKBOX_ONE)); lunaticQuestionnaire = new Questionnaire(); lunaticQuestionnaire.getComponents().add(roster); From e2bdbfc0fb1f9c66d4dba598405bbe42627d76cd Mon Sep 17 00:00:00 2001 From: Nicolas Senave Date: Wed, 11 Dec 2024 11:04:56 +0100 Subject: [PATCH 6/7] chore(release ci): restore standard version check Regex with the hotfix part doesn't work on shell command --- .github/workflows/create-release.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index 939c4d580..be38b0a9d 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -53,10 +53,10 @@ jobs: exit 1 fi -# if ! [[ "${{ steps.version-step.outputs.version }}" =~ ^[0-9]+.[0-9]+.[0-9]+(-hotfix\.\d+)?$ ]]; then -# echo "Nothing to tag/release, the tag ${{ steps.version-step.outputs.version }} is not in correct format X.Y.Z" -# exit 1 -# fi + if ! [[ "${{ steps.version-step.outputs.version }}" =~ ^[0-9]+.[0-9]+.[0-9]+$ ]]; then + echo "Nothing to tag/release, the tag ${{ steps.version-step.outputs.version }} is not in correct format X.Y.Z" + exit 1 + fi sonar: name: Sonar analysis @@ -135,7 +135,7 @@ jobs: - name: Get previous final release tag id: previousTag - run: echo "previousTag=$(git --no-pager tag --sort=creatordate --merged ${{ github.ref_name }} | grep "^[3-9]\.[0-9]+\.[0-9]+(-hotfix\.\d+)?$" | tail -1)" >> $GITHUB_OUTPUT + run: echo "previousTag=$(git --no-pager tag --sort=creatordate --merged ${{ github.ref_name }} | grep "^[0-9]+.[0-9]+.[0-9]+$" | tail -1)" >> $GITHUB_OUTPUT # Note: the regex works for single digit major version, to be updated if the version goes 10.0.0 or more - name: Create tag From 1c7fde84c1784236bd5d1eefeb93f876f2fc8101 Mon Sep 17 00:00:00 2001 From: Nicolas Senave Date: Wed, 11 Dec 2024 11:05:13 +0100 Subject: [PATCH 7/7] chore: release version 3.30.0 --- build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 51772d668..3ef2dcd07 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -16,7 +16,7 @@ java { allprojects { group = "fr.insee.eno" - version = "3.30.0-SNAPSHOT.1" + version = "3.30.0" } subprojects {