From 1f52c16c98c6559caa96aa21acf7d97ef5830ce3 Mon Sep 17 00:00:00 2001 From: Ashley Wong <ashley.wong@justice.gov.uk> Date: Wed, 15 Jan 2025 17:09:19 +0000 Subject: [PATCH 1/8] Remove HasCaseDocument interface it is a temporary fields. --- .../ccd/draftorders/upload/suggested/UploadedDraftOrder.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/model/ccd/draftorders/upload/suggested/UploadedDraftOrder.java b/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/model/ccd/draftorders/upload/suggested/UploadedDraftOrder.java index 90d7a5b37c..81307f81d7 100644 --- a/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/model/ccd/draftorders/upload/suggested/UploadedDraftOrder.java +++ b/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/model/ccd/draftorders/upload/suggested/UploadedDraftOrder.java @@ -8,7 +8,6 @@ import lombok.Data; import lombok.NoArgsConstructor; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.CaseDocument; -import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.HasCaseDocument; import java.util.List; @@ -18,7 +17,7 @@ @AllArgsConstructor @NoArgsConstructor @JsonInclude(JsonInclude.Include.NON_NULL) -public class UploadedDraftOrder implements HasCaseDocument { +public class UploadedDraftOrder { @JsonProperty("suggestedDraftOrderDocument") private CaseDocument suggestedDraftOrderDocument; From 0bb54022933e3eb9dd2fb7b2fe3df03e2a0a2624 Mon Sep 17 00:00:00 2001 From: Ashley Wong <ashley.wong@justice.gov.uk> Date: Wed, 15 Jan 2025 17:28:21 +0000 Subject: [PATCH 2/8] DFR-3536 Add validation on additional documents. --- .../UploadDraftOrdersMidEventHandler.java | 87 ++++++++++++------- 1 file changed, 56 insertions(+), 31 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandler.java b/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandler.java index aa93e38ee7..a9191a5b9d 100644 --- a/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandler.java +++ b/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandler.java @@ -11,7 +11,9 @@ import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.CaseType; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.FinremCaseData; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.FinremCaseDetails; +import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.agreed.AgreedDraftOrderAdditionalDocumentsCollection; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.agreed.UploadAgreedDraftOrderCollection; +import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.SuggestedDraftOrderAdditionalDocumentsCollection; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadSuggestedDraftOrderCollection; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadedDraftOrder; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.wrapper.DraftOrdersWrapper; @@ -19,9 +21,8 @@ import java.util.ArrayList; import java.util.List; -import java.util.Objects; -import static java.util.Optional.ofNullable; +import static org.apache.commons.collections4.ListUtils.emptyIfNull; import static uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.DraftOrdersConstants.AGREED_DRAFT_ORDER_OPTION; import static uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.DraftOrdersConstants.SUGGESTED_DRAFT_ORDER_OPTION; @@ -29,60 +30,84 @@ @Service public class UploadDraftOrdersMidEventHandler extends FinremCallbackHandler { + private static final String HAVING_NON_WORD_DOCUMENT_IN_ORDER_OR_PSA_ERROR_MESSAGE = + "You must upload Microsoft Word documents. Document names should clearly reflect the party name, " + + "the type of hearing and the date of the hearing."; + + private static final String HAVING_WORD_DOCUMENT_IN_ADDITIONAL_ATTACHMENTS_ERROR_MESSAGE + = "You must upload a PDF file in the additional attachments."; + public UploadDraftOrdersMidEventHandler(FinremCaseDetailsMapper finremCaseDetailsMapper) { super(finremCaseDetailsMapper); } @Override public boolean canHandle(CallbackType callbackType, CaseType caseType, EventType eventType) { - return CallbackType.MID_EVENT.equals(callbackType) - && CaseType.CONTESTED.equals(caseType) - && EventType.DRAFT_ORDERS.equals(eventType); + return CallbackType.MID_EVENT.equals(callbackType) && CaseType.CONTESTED.equals(caseType) && EventType.DRAFT_ORDERS.equals(eventType); } @Override public GenericAboutToStartOrSubmitCallbackResponse<FinremCaseData> handle(FinremCallbackRequest callbackRequest, String userAuthorisation) { FinremCaseDetails caseDetails = callbackRequest.getCaseDetails(); - String caseId = String.valueOf(caseDetails.getId()); - log.info("Invoking contested {} mid event callback for Case ID: {}", callbackRequest.getEventType(), - caseId); + log.info("Invoking contested {} mid event callback for Case ID: {}", callbackRequest.getEventType(), caseDetails.getId()); FinremCaseData finremCaseData = caseDetails.getData(); List<String> errors = new ArrayList<>(); DraftOrdersWrapper draftOrdersWrapper = finremCaseData.getDraftOrdersWrapper(); - boolean hasNonWordDocument; - String error = "You must upload Microsoft Word documents. Document names should clearly reflect the party name, " - + "the type of hearing and the date of the hearing."; String typeOfDraftOrder = draftOrdersWrapper.getTypeOfDraftOrder(); if (SUGGESTED_DRAFT_ORDER_OPTION.equals(typeOfDraftOrder)) { - hasNonWordDocument = ofNullable(draftOrdersWrapper.getUploadSuggestedDraftOrder().getUploadSuggestedDraftOrderCollection()) - .orElse(List.of()) - .stream() - .map(UploadSuggestedDraftOrderCollection::getValue) - .filter(Objects::nonNull) - .map(UploadedDraftOrder::getSuggestedDraftOrderDocument) - .anyMatch(document -> document != null && !FileUtils.isWordDocument(document)); - if (hasNonWordDocument) { - errors.add(error); + if (isSuggestedDraftOrderHavingNonWordDocument(draftOrdersWrapper)) { + errors.add(HAVING_NON_WORD_DOCUMENT_IN_ORDER_OR_PSA_ERROR_MESSAGE); + } + if (isSuggestedDraftOrderAdditionalAttachingHavingNonPdfDocument(draftOrdersWrapper)) { + errors.add(HAVING_WORD_DOCUMENT_IN_ADDITIONAL_ATTACHMENTS_ERROR_MESSAGE); } } else if (AGREED_DRAFT_ORDER_OPTION.equals(typeOfDraftOrder)) { - hasNonWordDocument = ofNullable(draftOrdersWrapper.getUploadAgreedDraftOrder().getUploadAgreedDraftOrderCollection()) - .orElse(List.of()) - .stream() - .map(UploadAgreedDraftOrderCollection::getValue) - .filter(Objects::nonNull) - .map(uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.agreed.UploadedDraftOrder::getAgreedDraftOrderDocument) - .anyMatch(document -> document != null && !FileUtils.isWordDocument(document)); - if (hasNonWordDocument) { - errors.add(error); + if (isAgreedDraftOrderHavingNonWordDocument(draftOrdersWrapper)) { + errors.add(HAVING_NON_WORD_DOCUMENT_IN_ORDER_OR_PSA_ERROR_MESSAGE); + } + if (isAgreedDraftOrderAdditionalAttachingHavingNonPdfDocument(draftOrdersWrapper)) { + errors.add(HAVING_WORD_DOCUMENT_IN_ADDITIONAL_ATTACHMENTS_ERROR_MESSAGE); } } - return GenericAboutToStartOrSubmitCallbackResponse.<FinremCaseData>builder() - .data(finremCaseData).errors(errors).build(); + return GenericAboutToStartOrSubmitCallbackResponse.<FinremCaseData>builder().data(finremCaseData).errors(errors).build(); } + private boolean isSuggestedDraftOrderHavingNonWordDocument(DraftOrdersWrapper draftOrdersWrapper) { + return emptyIfNull(draftOrdersWrapper.getUploadSuggestedDraftOrder().getUploadSuggestedDraftOrderCollection()) + .stream() + .map(UploadSuggestedDraftOrderCollection::getValue) + .map(UploadedDraftOrder::getSuggestedDraftOrderDocument) + .anyMatch(document -> document != null && !FileUtils.isWordDocument(document)); + } + + private boolean isAgreedDraftOrderHavingNonWordDocument(DraftOrdersWrapper draftOrdersWrapper) { + return emptyIfNull(draftOrdersWrapper.getUploadAgreedDraftOrder().getUploadAgreedDraftOrderCollection()) + .stream() + .map(UploadAgreedDraftOrderCollection::getValue) + .map(uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.agreed.UploadedDraftOrder::getAgreedDraftOrderDocument) + .anyMatch(document -> document != null && !FileUtils.isWordDocument(document)); + } + + private boolean isSuggestedDraftOrderAdditionalAttachingHavingNonPdfDocument(DraftOrdersWrapper draftOrdersWrapper) { + return emptyIfNull(draftOrdersWrapper.getUploadSuggestedDraftOrder().getUploadSuggestedDraftOrderCollection()) + .stream() + .map(UploadSuggestedDraftOrderCollection::getValue) + .flatMap(order -> emptyIfNull(order.getSuggestedDraftOrderAdditionalDocumentsCollection()).stream()) + .map(SuggestedDraftOrderAdditionalDocumentsCollection::getValue) + .anyMatch(document -> document != null && !FileUtils.isPdf(document)); + } + + private boolean isAgreedDraftOrderAdditionalAttachingHavingNonPdfDocument(DraftOrdersWrapper draftOrdersWrapper) { + return emptyIfNull(draftOrdersWrapper.getUploadAgreedDraftOrder().getUploadAgreedDraftOrderCollection()) + .stream() + .map(UploadAgreedDraftOrderCollection::getValue) + .flatMap(order -> emptyIfNull(order.getAgreedDraftOrderAdditionalDocumentsCollection()).stream()) + .map(AgreedDraftOrderAdditionalDocumentsCollection::getValue) + .anyMatch(document -> document != null && !FileUtils.isPdf(document)); + } } From 93c15b807419aa72bd0694c320ba6bd608d5661e Mon Sep 17 00:00:00 2001 From: Ashley Wong <ashley.wong@justice.gov.uk> Date: Wed, 15 Jan 2025 17:36:26 +0000 Subject: [PATCH 3/8] Fix NPE --- .../UploadDraftOrdersMidEventHandler.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandler.java b/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandler.java index a9191a5b9d..301c9932f6 100644 --- a/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandler.java +++ b/src/main/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandler.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Objects; import static org.apache.commons.collections4.ListUtils.emptyIfNull; import static uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.DraftOrdersConstants.AGREED_DRAFT_ORDER_OPTION; @@ -81,33 +82,41 @@ private boolean isSuggestedDraftOrderHavingNonWordDocument(DraftOrdersWrapper dr return emptyIfNull(draftOrdersWrapper.getUploadSuggestedDraftOrder().getUploadSuggestedDraftOrderCollection()) .stream() .map(UploadSuggestedDraftOrderCollection::getValue) + .filter(Objects::nonNull) .map(UploadedDraftOrder::getSuggestedDraftOrderDocument) - .anyMatch(document -> document != null && !FileUtils.isWordDocument(document)); + .filter(Objects::nonNull) + .anyMatch(document -> !FileUtils.isWordDocument(document)); } private boolean isAgreedDraftOrderHavingNonWordDocument(DraftOrdersWrapper draftOrdersWrapper) { return emptyIfNull(draftOrdersWrapper.getUploadAgreedDraftOrder().getUploadAgreedDraftOrderCollection()) .stream() .map(UploadAgreedDraftOrderCollection::getValue) + .filter(Objects::nonNull) .map(uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.agreed.UploadedDraftOrder::getAgreedDraftOrderDocument) - .anyMatch(document -> document != null && !FileUtils.isWordDocument(document)); + .filter(Objects::nonNull) + .anyMatch(document -> !FileUtils.isWordDocument(document)); } private boolean isSuggestedDraftOrderAdditionalAttachingHavingNonPdfDocument(DraftOrdersWrapper draftOrdersWrapper) { return emptyIfNull(draftOrdersWrapper.getUploadSuggestedDraftOrder().getUploadSuggestedDraftOrderCollection()) .stream() .map(UploadSuggestedDraftOrderCollection::getValue) + .filter(Objects::nonNull) .flatMap(order -> emptyIfNull(order.getSuggestedDraftOrderAdditionalDocumentsCollection()).stream()) .map(SuggestedDraftOrderAdditionalDocumentsCollection::getValue) - .anyMatch(document -> document != null && !FileUtils.isPdf(document)); + .filter(Objects::nonNull) + .anyMatch(document -> !FileUtils.isPdf(document)); } private boolean isAgreedDraftOrderAdditionalAttachingHavingNonPdfDocument(DraftOrdersWrapper draftOrdersWrapper) { return emptyIfNull(draftOrdersWrapper.getUploadAgreedDraftOrder().getUploadAgreedDraftOrderCollection()) .stream() .map(UploadAgreedDraftOrderCollection::getValue) + .filter(Objects::nonNull) .flatMap(order -> emptyIfNull(order.getAgreedDraftOrderAdditionalDocumentsCollection()).stream()) .map(AgreedDraftOrderAdditionalDocumentsCollection::getValue) - .anyMatch(document -> document != null && !FileUtils.isPdf(document)); + .filter(Objects::nonNull) + .anyMatch(document -> !FileUtils.isPdf(document)); } } From 9b47c9ef552a89ba5a6a1204099b6ec5d9782615 Mon Sep 17 00:00:00 2001 From: Ashley Wong <ashley.wong@justice.gov.uk> Date: Thu, 16 Jan 2025 11:08:09 +0000 Subject: [PATCH 4/8] Renamed --- ...ndlerTest.java => UploadDraftOrdersMidEventHandlerTest.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/{UploadDraftOrderMidEventHandlerTest.java => UploadDraftOrdersMidEventHandlerTest.java} (99%) diff --git a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrderMidEventHandlerTest.java b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java similarity index 99% rename from src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrderMidEventHandlerTest.java rename to src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java index 79ac3d01b4..af3838d1cb 100644 --- a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrderMidEventHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java @@ -32,7 +32,7 @@ import static uk.gov.hmcts.reform.finrem.caseorchestration.test.Assertions.assertCanHandle; @ExtendWith(MockitoExtension.class) -class UploadDraftOrderMidEventHandlerTest { +class UploadDraftOrdersMidEventHandlerTest { @InjectMocks private UploadDraftOrdersMidEventHandler handler; From fceed8a48bf9d81a96b4ecff2151eca827ead849 Mon Sep 17 00:00:00 2001 From: Ashley Wong <ashley.wong@justice.gov.uk> Date: Thu, 16 Jan 2025 11:33:09 +0000 Subject: [PATCH 5/8] Tidy up --- .../UploadDraftOrdersMidEventHandlerTest.java | 243 +++++------------- 1 file changed, 69 insertions(+), 174 deletions(-) diff --git a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java index af3838d1cb..2f854e2d79 100644 --- a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java @@ -45,6 +45,22 @@ void canHandle() { assertCanHandle(handler, CallbackType.MID_EVENT, CaseType.CONTESTED, EventType.DRAFT_ORDERS); } + private static UploadAgreedDraftOrderCollection toUploadAgreedDraftOrderCollection(String documentName) { + return UploadAgreedDraftOrderCollection.builder() + .value(UploadedDraftOrder.builder() + .agreedDraftOrderDocument(CaseDocument.builder().documentFilename(documentName).build()) + .build()) + .build(); + } + + private static UploadSuggestedDraftOrderCollection toUploadSuggestedDraftOrderCollection(String documentName) { + return UploadSuggestedDraftOrderCollection.builder() + .value(uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadedDraftOrder.builder() + .suggestedDraftOrderDocument(CaseDocument.builder().documentFilename(documentName).build()) + .build()) + .build(); + } + @ParameterizedTest @MethodSource("provideAgreedDraftOrderTestCases") void shouldReturnNoErrorsWhenAllDocumentsAreWordFiles(List<UploadAgreedDraftOrderCollection> agreedDraftOrderCollection) { @@ -65,79 +81,30 @@ void shouldReturnNoErrorsWhenAllDocumentsAreWordFiles(List<UploadAgreedDraftOrde private static Stream<Arguments> provideAgreedDraftOrderTestCases() { return Stream.of( // Case 1: All valid Word documents - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample1.doc").build()) - .build()) - .build(), - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample2.docx").build()) - .build()) - .build() - ) - ), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample1.doc"), + toUploadAgreedDraftOrderCollection("sample2.doc") + )), // Case 2: One null document value - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(null) // Null document - .build(), - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample.doc").build()) - .build()) - .build() - ) - ), + Arguments.of(List.of( + UploadAgreedDraftOrderCollection.builder().value(null).build(), + toUploadAgreedDraftOrderCollection("sample.doc") + )), // Case 3: Special characters in filenames - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample@file.doc").build()) - .build()) - .build(), - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample file.doc").build()) - .build()) - .build() - ) - ), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample@file.doc"), + toUploadAgreedDraftOrderCollection("sample file.doc") + )), // Case 4: Very long filename - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - // Long filename - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("a".repeat(255) + ".doc").build()) - .build()) - .build() - ) - ), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("a".repeat(255) + ".doc") + )), // Case 5: Multiple valid Word documents - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("valid1.doc").build()) - .build()) - .build(), - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("valid2.docx").build()) - .build()) - .build(), - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("valid3.doc").build()) - .build()) - .build() - ) - ) + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("valid1.doc"), + toUploadAgreedDraftOrderCollection("valid2.doc"), + toUploadAgreedDraftOrderCollection("valid3.doc") + )) ); } @@ -167,81 +134,33 @@ void shouldReturnErrorWhenNonWordDocumentsAreUploaded(List<UploadAgreedDraftOrde private static Stream<org.junit.jupiter.params.provider.Arguments> provideNonWordDocumentEdgeCases() { return Stream.of( - // Case 2: One document with an empty filename - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("").build()) - .build()) - .build(), - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample.doc").build()) - .build()) - .build() - ), true - ), + // Case 0: One document with an empty filename + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection(""), + toUploadAgreedDraftOrderCollection("sample.doc") + ), true), // Case 1: Empty document filename - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("").build()) - .build()) - .build() - ), true - ), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("") + ), true), // Case 2: Null document - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(null) // Null document - .build()) - .build() - ), false - ), + Arguments.of(List.of( + UploadAgreedDraftOrderCollection.builder().value(UploadedDraftOrder.builder().agreedDraftOrderDocument(null).build()).build() + ), false), // Case 3: Mixed document types - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample.doc").build()) - .build()) - .build(), - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample.pdf").build()) - .build()) - .build() - ), true - ), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample.doc"), + toUploadAgreedDraftOrderCollection("sample.pdf") + ), true), // Case 4: Unusual file extension - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample.docx.txt").build()) - .build()) - .build() - ), true - ), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample.docx.txt") + ), true), // Case 5: Multiple non-Word documents - Arguments.of( - List.of( - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample1.pdf").build()) - .build()) - .build(), - UploadAgreedDraftOrderCollection.builder() - .value(UploadedDraftOrder.builder() - .agreedDraftOrderDocument(CaseDocument.builder().documentFilename("sample2.txt").build()) - .build()) - .build() - ), true - ) + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample1.pdf"), + toUploadAgreedDraftOrderCollection("sample2.txt") + ), true) ); } @@ -272,43 +191,19 @@ void shouldReturnNoErrorsWhenAllDocumentsAreWordFilesForSuggestedDraftOrders(Lis private static Stream<Arguments> provideSuggestedDraftOrderTestCases() { return Stream.of( // Case 1: All valid Word documents - Arguments.of( - List.of( - UploadSuggestedDraftOrderCollection.builder() - .value(uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadedDraftOrder.builder() - .suggestedDraftOrderDocument(CaseDocument.builder().documentFilename("suggested1.doc").build()) - .build()) - .build(), - UploadSuggestedDraftOrderCollection.builder() - .value(uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadedDraftOrder.builder() - .suggestedDraftOrderDocument(CaseDocument.builder().documentFilename("suggested2.doc").build()) - .build()) - .build() - ), false - ), + Arguments.of(List.of( + toUploadSuggestedDraftOrderCollection("suggested1.doc"), + toUploadSuggestedDraftOrderCollection("suggested2.doc") + ), false), // Case 2: One null document value - Arguments.of( - List.of( - UploadSuggestedDraftOrderCollection.builder() - .value(null) // Null document - .build(), - UploadSuggestedDraftOrderCollection.builder() - .value(uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadedDraftOrder.builder() - .suggestedDraftOrderDocument(CaseDocument.builder().documentFilename("suggested.doc").build()) - .build()) - .build() - ), false - ), + Arguments.of(List.of( + UploadSuggestedDraftOrderCollection.builder().value(null).build(), + toUploadSuggestedDraftOrderCollection("suggested.doc") + ), false), // Case 3: Invalid file type - Arguments.of( - List.of( - UploadSuggestedDraftOrderCollection.builder() - .value(uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadedDraftOrder.builder() - .suggestedDraftOrderDocument(CaseDocument.builder().documentFilename("suggested.pdf").build()) - .build()) - .build() - ), true - ) + Arguments.of(List.of( + toUploadSuggestedDraftOrderCollection("suggested.pdf") + ), true) ); } } From bc0ff971f0b072ec3da5e217cbf1a6d932afa3ff Mon Sep 17 00:00:00 2001 From: Ashley Wong <ashley.wong@justice.gov.uk> Date: Thu, 16 Jan 2025 12:00:34 +0000 Subject: [PATCH 6/8] add test cases --- .../UploadDraftOrdersMidEventHandlerTest.java | 113 +++++++++++++++--- 1 file changed, 94 insertions(+), 19 deletions(-) diff --git a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java index 2f854e2d79..fab48f752d 100644 --- a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java @@ -16,13 +16,17 @@ import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.CaseDocument; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.CaseType; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.FinremCaseData; +import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.agreed.AgreedDraftOrderAdditionalDocumentsCollection; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.agreed.UploadAgreedDraftOrder; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.agreed.UploadAgreedDraftOrderCollection; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.agreed.UploadedDraftOrder; +import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.SuggestedDraftOrderAdditionalDocumentsCollection; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadSuggestedDraftOrder; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadSuggestedDraftOrderCollection; import uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.wrapper.DraftOrdersWrapper; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.stream.Stream; @@ -45,18 +49,38 @@ void canHandle() { assertCanHandle(handler, CallbackType.MID_EVENT, CaseType.CONTESTED, EventType.DRAFT_ORDERS); } - private static UploadAgreedDraftOrderCollection toUploadAgreedDraftOrderCollection(String documentName) { + private static UploadAgreedDraftOrderCollection toUploadAgreedDraftOrderCollection(String documentName, String... attachmentDocumentNames) { + List<AgreedDraftOrderAdditionalDocumentsCollection> additionalDocuments = attachmentDocumentNames == null + || attachmentDocumentNames.length == 0 + ? Collections.emptyList() + : Arrays.stream(attachmentDocumentNames) + .map(name -> AgreedDraftOrderAdditionalDocumentsCollection.builder() + .value(CaseDocument.builder().documentFilename(name).build()) + .build()) + .toList(); + return UploadAgreedDraftOrderCollection.builder() .value(UploadedDraftOrder.builder() .agreedDraftOrderDocument(CaseDocument.builder().documentFilename(documentName).build()) + .agreedDraftOrderAdditionalDocumentsCollection(additionalDocuments) .build()) .build(); } - private static UploadSuggestedDraftOrderCollection toUploadSuggestedDraftOrderCollection(String documentName) { + private static UploadSuggestedDraftOrderCollection toUploadSuggestedDraftOrderCollection(String documentName, String... attachmentDocumentNames) { + List<SuggestedDraftOrderAdditionalDocumentsCollection> additionalDocuments = attachmentDocumentNames == null + || attachmentDocumentNames.length == 0 + ? Collections.emptyList() + : Arrays.stream(attachmentDocumentNames) + .map(name -> SuggestedDraftOrderAdditionalDocumentsCollection.builder() + .value(CaseDocument.builder().documentFilename(name).build()) + .build()) + .toList(); + return UploadSuggestedDraftOrderCollection.builder() .value(uk.gov.hmcts.reform.finrem.caseorchestration.model.ccd.draftorders.upload.suggested.UploadedDraftOrder.builder() .suggestedDraftOrderDocument(CaseDocument.builder().documentFilename(documentName).build()) + .suggestedDraftOrderAdditionalDocumentsCollection(additionalDocuments) .build()) .build(); } @@ -111,7 +135,7 @@ private static Stream<Arguments> provideAgreedDraftOrderTestCases() { @ParameterizedTest @MethodSource("provideNonWordDocumentEdgeCases") void shouldReturnErrorWhenNonWordDocumentsAreUploaded(List<UploadAgreedDraftOrderCollection> agreedDraftOrderCollection, - boolean showErrorMessage) { + boolean agreedDraftOrderRejected, boolean attachmentFormatRejected) { // Create the FinremCallbackRequest using the provided UploadAgreedDraftOrderCollection GenericAboutToStartOrSubmitCallbackResponse<FinremCaseData> response = handler.handle(FinremCallbackRequestFactory.from(1727874196328932L, FinremCaseData.builder() @@ -123,44 +147,70 @@ void shouldReturnErrorWhenNonWordDocumentsAreUploaded(List<UploadAgreedDraftOrde .build()) .build()), AUTH_TOKEN); - if (!showErrorMessage) { + if (!(agreedDraftOrderRejected || attachmentFormatRejected)) { assertThat(response.getErrors()).isEmpty(); } else { - assertThat(response.getErrors()).containsExactly("You must upload Microsoft Word documents. " - + "Document names should clearly reflect the party name, the type of hearing and the date of the hearing."); + String wordDocErrorMessage = "You must upload Microsoft Word documents. " + + "Document names should clearly reflect the party name, the type of hearing and the date of the hearing."; + String rejectedAttachmentMessage = "You must upload a PDF file in the additional attachments."; + + if (agreedDraftOrderRejected && attachmentFormatRejected) { + assertThat(response.getErrors()).containsExactlyInAnyOrder(wordDocErrorMessage, rejectedAttachmentMessage); + } else if (agreedDraftOrderRejected) { + assertThat(response.getErrors()).contains(wordDocErrorMessage); + } else { + assertThat(response.getErrors()).contains(rejectedAttachmentMessage); + } } assertThat(response.getData()).isNotNull(); } + private static Stream<org.junit.jupiter.params.provider.Arguments> provideNonWordDocumentEdgeCases() { return Stream.of( // Case 0: One document with an empty filename Arguments.of(List.of( toUploadAgreedDraftOrderCollection(""), toUploadAgreedDraftOrderCollection("sample.doc") - ), true), + ), true, false), // Case 1: Empty document filename Arguments.of(List.of( toUploadAgreedDraftOrderCollection("") - ), true), + ), true, false), // Case 2: Null document Arguments.of(List.of( UploadAgreedDraftOrderCollection.builder().value(UploadedDraftOrder.builder().agreedDraftOrderDocument(null).build()).build() - ), false), + ), false, false), // Case 3: Mixed document types Arguments.of(List.of( toUploadAgreedDraftOrderCollection("sample.doc"), toUploadAgreedDraftOrderCollection("sample.pdf") - ), true), + ), true, false), // Case 4: Unusual file extension Arguments.of(List.of( toUploadAgreedDraftOrderCollection("sample.docx.txt") - ), true), + ), true, false), // Case 5: Multiple non-Word documents Arguments.of(List.of( toUploadAgreedDraftOrderCollection("sample1.pdf"), toUploadAgreedDraftOrderCollection("sample2.txt") - ), true) + ), true, false), + // Case 6: With attachment cases + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample.doc", "sampleAttachment.pdf") + ), false, false), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample.doc", "sampleAttachment.docx") + ), false, true), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample.doc", "sampleAttachment.pdf", "sampleAttachment.txt") + ), false, true), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample.pdf", "sampleAttachment.pdf") + ), true, false), + Arguments.of(List.of( + toUploadAgreedDraftOrderCollection("sample.pdf", "sampleAttachment.xls") + ), true, true) ); } @@ -168,7 +218,7 @@ private static Stream<org.junit.jupiter.params.provider.Arguments> provideNonWor @MethodSource("provideSuggestedDraftOrderTestCases") void shouldReturnNoErrorsWhenAllDocumentsAreWordFilesForSuggestedDraftOrders(List<UploadSuggestedDraftOrderCollection> suggestedDraftOrderCollection, - boolean showErrorMessage) { + boolean suggestedOrderRejected, boolean attachmentFormatRejected) { GenericAboutToStartOrSubmitCallbackResponse<FinremCaseData> response = handler.handle(FinremCallbackRequestFactory.from(1727874196328932L, FinremCaseData.builder() .draftOrdersWrapper(DraftOrdersWrapper.builder() @@ -179,11 +229,20 @@ void shouldReturnNoErrorsWhenAllDocumentsAreWordFilesForSuggestedDraftOrders(Lis .build()) .build()), AUTH_TOKEN); - if (!showErrorMessage) { + if (!(suggestedOrderRejected || attachmentFormatRejected)) { assertThat(response.getErrors()).isEmpty(); } else { - assertThat(response.getErrors()).containsExactly("You must upload Microsoft Word documents. " - + "Document names should clearly reflect the party name, the type of hearing and the date of the hearing."); + String wordDocErrorMessage = "You must upload Microsoft Word documents. " + + "Document names should clearly reflect the party name, the type of hearing and the date of the hearing."; + String rejectedAttachmentMessage = "You must upload a PDF file in the additional attachments."; + + if (suggestedOrderRejected && attachmentFormatRejected) { + assertThat(response.getErrors()).containsExactlyInAnyOrder(wordDocErrorMessage, rejectedAttachmentMessage); + } else if (suggestedOrderRejected) { + assertThat(response.getErrors()).contains(wordDocErrorMessage); + } else { + assertThat(response.getErrors()).contains(rejectedAttachmentMessage); + } } assertThat(response.getData()).isNotNull(); } @@ -194,16 +253,32 @@ private static Stream<Arguments> provideSuggestedDraftOrderTestCases() { Arguments.of(List.of( toUploadSuggestedDraftOrderCollection("suggested1.doc"), toUploadSuggestedDraftOrderCollection("suggested2.doc") - ), false), + ), false, false), // Case 2: One null document value Arguments.of(List.of( UploadSuggestedDraftOrderCollection.builder().value(null).build(), toUploadSuggestedDraftOrderCollection("suggested.doc") - ), false), + ), false, false), // Case 3: Invalid file type Arguments.of(List.of( toUploadSuggestedDraftOrderCollection("suggested.pdf") - ), true) + ), true, false), + // Case 4: With attachment cases + Arguments.of(List.of( + toUploadSuggestedDraftOrderCollection("suggested.pdf", "sampleAttachment.pdf") + ), true, false), + Arguments.of(List.of( + toUploadSuggestedDraftOrderCollection("suggested.doc", "sampleAttachment.pdf") + ), false, false), + Arguments.of(List.of( + toUploadSuggestedDraftOrderCollection("suggested.doc", "sampleAttachment.doc") + ), false, true), + Arguments.of(List.of( + toUploadSuggestedDraftOrderCollection("suggested.doc", "sampleAttachment.pdf", "sampleAttachment1.docx") + ), false, true), + Arguments.of(List.of( + toUploadSuggestedDraftOrderCollection("suggested.png", "sampleAttachment1.bmp") + ), true, true) ); } } From e51e068fbf228f6bf0b3e7a6e7486d3a71aee6e4 Mon Sep 17 00:00:00 2001 From: Ashley Wong <ashley.wong@justice.gov.uk> Date: Thu, 16 Jan 2025 12:06:53 +0000 Subject: [PATCH 7/8] Remove an extra line. --- .../upload/UploadDraftOrdersMidEventHandlerTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java index fab48f752d..835f6f12ce 100644 --- a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java @@ -164,8 +164,7 @@ void shouldReturnErrorWhenNonWordDocumentsAreUploaded(List<UploadAgreedDraftOrde } assertThat(response.getData()).isNotNull(); } - - + private static Stream<org.junit.jupiter.params.provider.Arguments> provideNonWordDocumentEdgeCases() { return Stream.of( // Case 0: One document with an empty filename From b86d6a05533199695fa0f630941d90acd0b91357 Mon Sep 17 00:00:00 2001 From: Ashley Wong <ashley.wong@justice.gov.uk> Date: Fri, 17 Jan 2025 15:22:02 +0000 Subject: [PATCH 8/8] checkstyle --- .../upload/UploadDraftOrdersMidEventHandlerTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java index 835f6f12ce..1e1d0af80b 100644 --- a/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/reform/finrem/caseorchestration/handler/draftorders/upload/UploadDraftOrdersMidEventHandlerTest.java @@ -150,8 +150,8 @@ void shouldReturnErrorWhenNonWordDocumentsAreUploaded(List<UploadAgreedDraftOrde if (!(agreedDraftOrderRejected || attachmentFormatRejected)) { assertThat(response.getErrors()).isEmpty(); } else { - String wordDocErrorMessage = "You must upload Microsoft Word documents. " + - "Document names should clearly reflect the party name, the type of hearing and the date of the hearing."; + String wordDocErrorMessage = "You must upload Microsoft Word documents. " + + "Document names should clearly reflect the party name, the type of hearing and the date of the hearing."; String rejectedAttachmentMessage = "You must upload a PDF file in the additional attachments."; if (agreedDraftOrderRejected && attachmentFormatRejected) { @@ -231,8 +231,8 @@ void shouldReturnNoErrorsWhenAllDocumentsAreWordFilesForSuggestedDraftOrders(Lis if (!(suggestedOrderRejected || attachmentFormatRejected)) { assertThat(response.getErrors()).isEmpty(); } else { - String wordDocErrorMessage = "You must upload Microsoft Word documents. " + - "Document names should clearly reflect the party name, the type of hearing and the date of the hearing."; + String wordDocErrorMessage = "You must upload Microsoft Word documents. " + + "Document names should clearly reflect the party name, the type of hearing and the date of the hearing."; String rejectedAttachmentMessage = "You must upload a PDF file in the additional attachments."; if (suggestedOrderRejected && attachmentFormatRejected) {