From e9942e5eba19a882e05e7644fc53815f49fa96a9 Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 09:00:18 +0000 Subject: [PATCH 01/18] New methods to handle all deceased aliases. Also makes getResponseCaseData package accessible so we can directly test its behaviour. --- .../CallbackResponseTransformer.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java index 5d432b421f..ae0857c071 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java @@ -1008,7 +1008,7 @@ private CallbackResponse transformResponse(ResponseCaseData responseCaseData) { return CallbackResponse.builder().data(responseCaseData).build(); } - private ResponseCaseDataBuilder getResponseCaseData(CaseDetails caseDetails, String eventId, + ResponseCaseDataBuilder getResponseCaseData(CaseDetails caseDetails, String eventId, boolean transform) { CaseData caseData = caseDetails.getData(); @@ -1311,6 +1311,25 @@ private CallbackResponse transformResponse(ResponseCaseData responseCaseData) { return builder; } + void handleDeceasedAliases( + final ResponseCaseDataBuilder builder, + final CaseData caseData, + final Long caseRef) { + } + + Set> convertAliasOnWillToSolsDecAliasList( + final Long caseRef, + final String otherNameOnWill, + final String foreNames, + final String lastName) { + return Set.of(); + } + + Set> convertDecAliasesSolsDecAliasList( + final List> decAliases) { + return Set.of(); + } + OrganisationPolicy buildOrganisationPolicy(CaseDetails caseDetails, String authToken) { CaseData caseData = caseDetails.getData(); OrganisationEntityResponse organisationEntityResponse = null; From 6ce7cc9c0355ae0c96e1160e78deac55f223252e Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 12:29:02 +0000 Subject: [PATCH 02/18] Add testing for new inner methods. --- .../CallbackResponseTransformerTest.java | 231 ++++++++++++++++++ 1 file changed, 231 insertions(+) diff --git a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java index bd0c15c6d5..f05975ad87 100644 --- a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.function.Executable; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import org.mockito.InjectMocks; @@ -81,8 +82,10 @@ import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.stream.Stream; @@ -95,6 +98,7 @@ import static org.hamcrest.Matchers.comparesEqualTo; import static org.hamcrest.Matchers.empty; import static org.hamcrest.core.Is.is; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -314,6 +318,43 @@ class CallbackResponseTransformerTest { private static final LocalDateTime dateTime = LocalDateTime.of(2024, 1, 1, 1, 1, 1, 1); private static final String DEFAULT_DATE_OF_DEATHTYPE = "diedOn"; private static final String RESPONSE_CONTENT = "I responded"; + + private static final String WILL_ALIAS_FN = "WillFN"; + private static final String WILL_ALIAS_LN = "WillLN"; + private static final String WILL_ALIAS = WILL_ALIAS_FN + " " + WILL_ALIAS_LN; + private static final ProbateAliasName WILL_PROBATE_ALIAS_NAME = ProbateAliasName.builder() + .forenames(WILL_ALIAS_FN) + .lastName(WILL_ALIAS_LN) + .build(); + private static final CollectionMember WILL_PROBATE_ALIAS_NAME_CM = new CollectionMember<>( + WILL_PROBATE_ALIAS_NAME); + private static final AliasName WILL_ALIAS_NAME = AliasName.builder().solsAliasname(WILL_ALIAS).build(); + private static final CollectionMember WILL_ALIAS_NAME_CM = new CollectionMember<>(WILL_ALIAS_NAME); + + private static final String DEC_ALIAS_FN = "DecAliasFN"; + private static final String DEC_ALIAS_LN = "DecAliasLN"; + private static final String DEC_ALIAS = DEC_ALIAS_FN + " " + DEC_ALIAS_LN; + private static final ProbateAliasName DEC_PROBATE_ALIAS_NAME = ProbateAliasName.builder() + .forenames(DEC_ALIAS_FN) + .lastName(DEC_ALIAS_LN) + .build(); + private static final CollectionMember DEC_PROBATE_ALIAS_NAME_CM = new CollectionMember<>( + DEC_PROBATE_ALIAS_NAME); + private static final AliasName DEC_ALIAS_NAME = AliasName.builder().solsAliasname(DEC_ALIAS).build(); + private static final CollectionMember DEC_ALIAS_NAME_CM = new CollectionMember<>(DEC_ALIAS_NAME); + + private static final String SOL_DEC_ALIAS_FN = "SolDecAliasFN"; + private static final String SOL_DEC_ALIAS_LN = "SolDecAliasLN"; + private static final String SOL_DEC_ALIAS = SOL_DEC_ALIAS_FN + " " + SOL_DEC_ALIAS_LN; + private static final ProbateAliasName SOL_DEC_PROBATE_ALIAS_NAME = ProbateAliasName.builder() + .forenames(SOL_DEC_ALIAS_FN) + .lastName(SOL_DEC_ALIAS_LN) + .build(); + private static final CollectionMember SOL_DEC_PROBATE_ALIAS_NAME_CM = new CollectionMember<>( + SOL_DEC_PROBATE_ALIAS_NAME); + private static final AliasName SOL_DEC_ALIAS_NAME = AliasName.builder().solsAliasname(SOL_DEC_ALIAS).build(); + private static final CollectionMember SOL_DEC_ALIAS_NAME_CM = new CollectionMember<>(SOL_DEC_ALIAS_NAME); + private List caseMatches = new ArrayList<>(); @Mock @@ -4733,4 +4774,194 @@ void shouldNotDefaultInformationRequestSwitch() { private String format(DateTimeFormatter formatter, ResponseCaseData caseData, int ind) { return formatter.format(caseData.getRegistrarDirections().get(ind).getValue().getAddedDateTime()); } + + @Test + void testConvertWillAlias_sameOnWillNull() { + final Long caseRef = 1L; + final String nameSameOnWill = null; + final String foreNames = WILL_ALIAS_FN; + final String lastName = WILL_ALIAS_LN; + + final var res = underTest.convertAliasOnWillToSolsDecAliasList( + caseRef, + nameSameOnWill, + foreNames, + lastName); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(0, res.size(), "Expected converted will aliases to be empty")); + + assertAll(assertions); + } + + @Test + void testConvertWillAlias_sameOnWillYes() { + final Long caseRef = 1L; + final String otherNameOnWill = YES; + final String foreNames = WILL_ALIAS_FN; + final String lastName = WILL_ALIAS_LN; + + final var res = underTest.convertAliasOnWillToSolsDecAliasList( + caseRef, + otherNameOnWill, + foreNames, + lastName); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(0, res.size(), "Expected converted will aliases to be empty")); + + assertAll(assertions); + } + + @Test + void testConvertWillAlias_sameOnWillNo_fnNull_lnPresent() { + final Long caseRef = 1L; + final String otherNameOnWill = NO; + final String foreNames = null; + final String lastName = WILL_ALIAS_LN; + + final var res = underTest.convertAliasOnWillToSolsDecAliasList( + caseRef, + otherNameOnWill, + foreNames, + lastName); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(0, res.size(), "Expected converted will aliases to be empty")); + + assertAll(assertions); + } + + @Test + void testConvertWillAlias_sameOnWillNo_fnPresent_lnNull() { + final Long caseRef = 1L; + final String otherNameOnWill = NO; + final String foreNames = WILL_ALIAS_FN; + final String lastName = null; + + final var res = underTest.convertAliasOnWillToSolsDecAliasList( + caseRef, + otherNameOnWill, + foreNames, + lastName); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(0, res.size(), "Expected converted will aliases to be empty")); + + assertAll(assertions); + } + + @Test + void testConvertWillAlias_sameOnWillNo_fnPresent_lnPresent() { + final Long caseRef = 1L; + final String otherNameOnWill = NO; + final String foreNames = WILL_ALIAS_FN; + final String lastName = WILL_ALIAS_LN; + + final var res = underTest.convertAliasOnWillToSolsDecAliasList( + caseRef, + otherNameOnWill, + foreNames, + lastName); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(1, res.size(), "Expected converted will aliases to have one entry")); + assertions.add(() -> assertTrue(res.contains(WILL_ALIAS_NAME_CM), "Expected will alias present")); + + assertAll(assertions); + } + + @Test + void testConvertDecAliases_null() { + final List> aliases = null; + + final var res = underTest.convertDecAliasesSolsDecAliasList( + aliases); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(0, res.size(), "Expected converted will aliases to be empty")); + + assertAll(assertions); + } + + @Test + void testConvertDecAliases_empty() { + final List> aliases = List.of(); + + final var res = underTest.convertDecAliasesSolsDecAliasList( + aliases); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(0, res.size(), "Expected converted dec aliases to be empty")); + + assertAll(assertions); + } + + @Test + void testConvertDecAliases_one() { + final List> aliases = List.of( + DEC_PROBATE_ALIAS_NAME_CM); + + final var res = underTest.convertDecAliasesSolsDecAliasList( + aliases); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(1, res.size(), "Expected converted dec aliases to be have one entry")); + assertions.add(() -> assertTrue(res.contains(DEC_ALIAS_NAME_CM), "Expected dec alias present")); + + assertAll(assertions); + } + + @Test + void testConvertDecAliases_two() { + final List> aliases = List.of( + DEC_PROBATE_ALIAS_NAME_CM, + WILL_PROBATE_ALIAS_NAME_CM); + + final var res = underTest.convertDecAliasesSolsDecAliasList( + aliases); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(2, res.size(), "Expected converted will aliases to have two entries")); + assertions.add(() -> assertTrue(res.contains(DEC_ALIAS_NAME_CM), "Expected dec alias present")); + assertions.add(() -> assertTrue(res.contains(WILL_ALIAS_NAME_CM), "Expected will alias present")); + + assertAll(assertions); + } + + @Test + void testConvertDecAliases_twoDuplicate() { + final List> aliases = List.of( + DEC_PROBATE_ALIAS_NAME_CM, + DEC_PROBATE_ALIAS_NAME_CM); + + final var res = underTest.convertDecAliasesSolsDecAliasList( + aliases); + + final Collection assertions = new HashSet<>(); + + assertions.add(() -> assertNotNull(res, "Expected non-null response")); + assertions.add(() -> assertEquals(1, res.size(), "Expected converted dec aliases to have one entry")); + assertions.add(() -> assertTrue(res.contains(DEC_ALIAS_NAME_CM), "Expected will alias present")); + + assertAll(assertions); + } } From 406ea02c338dc62285340543c3b43f025ddefe59 Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 12:32:11 +0000 Subject: [PATCH 03/18] Implementation for new functions. --- .../CallbackResponseTransformer.java | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java index ae0857c071..b141795933 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java @@ -1,6 +1,7 @@ package uk.gov.hmcts.probate.transformer; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Component; @@ -57,6 +58,7 @@ import java.util.Optional; import java.util.HashSet; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import static java.lang.Boolean.TRUE; @@ -101,6 +103,7 @@ @Component @RequiredArgsConstructor +@Slf4j public class CallbackResponseTransformer { public static final String ANSWER_YES = "Yes"; @@ -1319,15 +1322,61 @@ void handleDeceasedAliases( Set> convertAliasOnWillToSolsDecAliasList( final Long caseRef, - final String otherNameOnWill, + final String differentNameOnWill, final String foreNames, final String lastName) { + if (differentNameOnWill != null && NO.equals(differentNameOnWill)) { + if (foreNames != null && lastName != null) { + final String aliasValue = new StringBuilder() + .append(foreNames) + .append(" ") + .append(lastName) + .toString(); + + final AliasName alias = AliasName.builder() + .solsAliasname(aliasValue) + .build(); + + final CollectionMember listMember = new CollectionMember<>(alias); + return Set.of(listMember); + } else { + log.info("For case {}, foreNames == null: {}, lastName == null: {}," + + " so alias is not being added to solsDecAlias list", + caseRef, + foreNames == null, + lastName == null); + } + } return Set.of(); } Set> convertDecAliasesSolsDecAliasList( final List> decAliases) { - return Set.of(); + if (decAliases == null || decAliases.isEmpty()) { + return Set.of(); + } + + final Function, ProbateAliasName> unwrap = c -> c.getValue(); + + final Function convert = p -> { + final String aliasValue = new StringBuilder() + .append(p.getForenames()) + .append(" ") + .append(p.getLastName()) + .toString(); + + return AliasName.builder() + .solsAliasname(aliasValue) + .build(); + }; + + final Function> wrap = a -> new CollectionMember<>(a); + + return decAliases.stream() + .map(unwrap) + .map(convert) + .map(wrap) + .collect(Collectors.toUnmodifiableSet()); } OrganisationPolicy buildOrganisationPolicy(CaseDetails caseDetails, String authToken) { From a968efc63bab21a9b079e3a205fdf7bf80f31c6f Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 12:38:51 +0000 Subject: [PATCH 04/18] Add testing for new utility method. --- .../CallbackResponseTransformerTest.java | 415 ++++++++++++++++++ 1 file changed, 415 insertions(+) diff --git a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java index f05975ad87..e4e17b2dde 100644 --- a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.api.function.Executable; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.ArgumentMatcher; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Spy; @@ -88,6 +89,8 @@ import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import static java.lang.Boolean.FALSE; @@ -107,8 +110,11 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -4775,6 +4781,415 @@ private String format(DateTimeFormatter formatter, ResponseCaseData caseData, in return formatter.format(caseData.getRegistrarDirections().get(ind).getValue().getAddedDateTime()); } + private static final class AliasMatcher implements ArgumentMatcher>> { + private final Set> expects; + + public AliasMatcher(Set> expects) { + this.expects = expects; + } + + @Override + public boolean matches(List> arg) { + if (arg == null) { + return false; + } + if (arg.size() != expects.size()) { + return false; + } + + return arg.containsAll(expects); + } + + @Override + public String toString() { + final String elements = expects.stream() + .map(c -> c.getValue().getSolsAliasname()) + .collect(Collectors.joining(", ")); + final StringBuilder description = new StringBuilder(); + + description.append("List of ") + .append(expects.size()) + .append(" elements, containing all of: [") + .append(elements) + .append("] in any order"); + return description.toString(); + } + + public ArgumentMatcher>> invert() { + return new InvertAliasMatcher(this); + } + } + + private static final class InvertAliasMatcher implements ArgumentMatcher>> { + private final AliasMatcher aliasMatcher; + + public InvertAliasMatcher(final AliasMatcher aliasMatcher) { + this.aliasMatcher = aliasMatcher; + } + + @Override + public boolean matches(List> argument) { + return !(aliasMatcher.matches(argument)); + } + + @Override + public String toString() { + final StringBuilder description = new StringBuilder(); + description.append("Anything except a ") + .append(aliasMatcher.toString()); + + return description.toString(); + } + } + + @Test + void givenNoAliasesThenNoAliasesAdded() { + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .build(); + + final Long caseRef = 1L; + + final AliasMatcher expAliasMatcher = new AliasMatcher(Set.of()); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + @Test + void givenAliasOnWillThenWillAliasAdded() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(NO) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .build(); + + final Long caseRef = 1L; + + final Set> expAliases = Set.of(WILL_ALIAS_NAME_CM); + final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + @Test + void givenAliasOnWillAndAnyOtherNameYESThenNoWillAliasAdded() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(YES) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .build(); + + final Long caseRef = 1L; + + final AliasMatcher expAliasMatcher = new AliasMatcher(Set.of()); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + @Test + void givenAliasOnWillAndAnyOtherNameNullThenNoWillAliasAdded() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(null) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .build(); + + final Long caseRef = 1L; + + final AliasMatcher expAliasMatcher = new AliasMatcher(Set.of()); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + @Test + void givenNullFNAliasOnWillThenNoAliasAdded() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(NO) + .deceasedAliasFirstNameOnWill(null) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .build(); + + final Long caseRef = 1L; + + final AliasMatcher expAliasMatcher = new AliasMatcher(Set.of()); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + @Test + void givenNullLNAliasOnWillThenNoAliasAdded() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(NO) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(null) + .build(); + + final Long caseRef = 1L; + + final AliasMatcher expAliasMatcher = new AliasMatcher(Set.of()); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + @Test + void givenDecAliasThenDecAliasAdded() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAliasNameList(List.of(DEC_PROBATE_ALIAS_NAME_CM)) + .build(); + + final Long caseRef = 1L; + + final Set> expAliases = Set.of(DEC_ALIAS_NAME_CM); + final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + @Test + void givenSolsDecAliasThenSolsDecAliasAdded() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .solsDeceasedAliasNamesList(List.of(SOL_DEC_ALIAS_NAME_CM)) + .build(); + + final Long caseRef = 1L; + + final Set> expAliases = Set.of(SOL_DEC_ALIAS_NAME_CM); + final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + @Test + void givenAliasOnWillAndDecAliasThenWillAliasAndDecAliasAdded() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(NO) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .deceasedAliasNameList(List.of(DEC_PROBATE_ALIAS_NAME_CM)) + .build(); + + final Long caseRef = 1L; + + final Set> expAliases = Set.of(WILL_ALIAS_NAME_CM, DEC_ALIAS_NAME_CM); + final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + // This is akin to the current situation where a caseworker deletes the will alias from the sols list and then + // it gets added back. (Except we no longer store the will alias after this action.) + @Test + void givenAliasOnWillAndSolsDecAliasThenWillAliasAndSolsDecAliasAdded() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(NO) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .solsDeceasedAliasNamesList(List.of(SOL_DEC_ALIAS_NAME_CM)) + .build(); + + final Long caseRef = 1L; + + final Set> expAliases = Set.of( + WILL_ALIAS_NAME_CM, + SOL_DEC_ALIAS_NAME_CM); + final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + // If alias on will is already in the sols alias list then it'll not get added a second time + @Test + void givenAliasOnWillAndWillAliasInSolsDecAliasThenNoDuplicates() { + + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(NO) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .solsDeceasedAliasNamesList(List.of(WILL_ALIAS_NAME_CM, SOL_DEC_ALIAS_NAME_CM)) + .build(); + + final Long caseRef = 1L; + + final Set> expAliases = Set.of( + WILL_ALIAS_NAME_CM, + SOL_DEC_ALIAS_NAME_CM); + final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + + underTest.handleDeceasedAliases( + builderSpy, + caseData, + caseRef); + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + @Test void testConvertWillAlias_sameOnWillNull() { final Long caseRef = 1L; From 238cc73e51e2e2fba4ded81552daa7b78c57dc5d Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 12:40:03 +0000 Subject: [PATCH 05/18] New utility method implementation. --- .../CallbackResponseTransformer.java | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java index b141795933..441c8b645f 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java @@ -1318,6 +1318,46 @@ void handleDeceasedAliases( final ResponseCaseDataBuilder builder, final CaseData caseData, final Long caseRef) { + // Question this asks is "Is the name on the will the same?" Not "Are there other names on the will?" as the + // name of the variable in the CaseData object suggests. + final String decNameOnWillSame = caseData.getDeceasedAnyOtherNameOnWill(); + final var decAliases = caseData.getDeceasedAliasNameList(); + final var solsDecAliases = caseData.getSolsDeceasedAliasNamesList(); + + { + final boolean hasAlternateNameOnWill = decNameOnWillSame != null && NO.equals(decNameOnWillSame); + final boolean hasDecAliases = decAliases != null && !decAliases.isEmpty(); + final boolean hasSolsDecAliases = solsDecAliases != null && !solsDecAliases.isEmpty(); + + if ((hasAlternateNameOnWill || hasDecAliases) && hasSolsDecAliases) { + // This is one of the contributing causes for DTSPB-4388 + log.info("For case {} found both non-sols and sols aliases: hasAltNameOnWill: {}, hasDecAliases: {}," + + " hasSolsDecAliases: {}", + caseRef, + hasAlternateNameOnWill, + hasDecAliases, + hasSolsDecAliases); + } + } + + Set> newSolsDecAliases = new HashSet<>(); + + final String decAliasFNOnWill = caseData.getDeceasedAliasFirstNameOnWill(); + final String decAliasLNOnWill = caseData.getDeceasedAliasLastNameOnWill(); + + newSolsDecAliases.addAll(convertAliasOnWillToSolsDecAliasList( + caseRef, + decNameOnWillSame, + decAliasFNOnWill, + decAliasLNOnWill)); + + newSolsDecAliases.addAll(convertDecAliasesSolsDecAliasList(decAliases)); + + if (solsDecAliases != null) { + newSolsDecAliases.addAll(solsDecAliases); + } + + builder.solsDeceasedAliasNamesList(newSolsDecAliases.stream().toList()); } Set> convertAliasOnWillToSolsDecAliasList( From 48794751819ccd6bdc7302cd7e9817c018a2e201 Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 12:41:54 +0000 Subject: [PATCH 06/18] Update tests for new behaviour, Add test for existing getResponseCaseData. the new test simply validates that we never add anything to the non-sols fields for aliases. These are ultimately the reason the behaviour we were seeing occurs. --- .../CallbackResponseTransformerTest.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java index e4e17b2dde..662d6226cb 100644 --- a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java @@ -11,6 +11,7 @@ import org.mockito.ArgumentMatcher; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Spy; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.test.util.ReflectionTestUtils; @@ -113,6 +114,7 @@ import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -4413,6 +4415,7 @@ void testDefaultDateOfDeathDefaultsToDiedOn() { @Test void shouldAddDeceasedAliasNamesToCaseDataUpdateCaseBuilder() { caseDataBuilder.applicationType(ApplicationType.PERSONAL) + .deceasedAnyOtherNameOnWill(NO) .deceasedAliasFirstNameOnWill("John") .deceasedAliasLastNameOnWill("Doe"); @@ -4436,6 +4439,7 @@ void shouldAddDeceasedAliasNamesToCaseDataUpdateCaseBuilderEvenIfOneExists() { List> deceasedAliasNamesList = new ArrayList<>(); deceasedAliasNamesList.add(createdDeceasedAliasName("0", ALIAS_FORENAME, ALIAS_SURNAME, YES)); caseDataBuilder.applicationType(ApplicationType.PERSONAL) + .deceasedAnyOtherNameOnWill(NO) .deceasedAliasFirstNameOnWill("John") .deceasedAliasLastNameOnWill("Doe") .deceasedAliasNameList(deceasedAliasNamesList); @@ -4459,6 +4463,7 @@ void shouldAddDeceasedAliasNamesToCaseDataUpdateCaseBuilderEvenIfOneExists() { void shouldAddDeceasedAliasNamesToCaseDataUpdateCaseBuilderForTransformCase() { caseDataBuilder.applicationType(ApplicationType.PERSONAL) .ihtReferenceNumber("020e920e920e902e2") + .deceasedAnyOtherNameOnWill(NO) .deceasedAliasFirstNameOnWill("Jane") .deceasedAliasLastNameOnWill("Smith") .solsExecutorAliasNames("Dee ceased lol"); @@ -4781,6 +4786,46 @@ private String format(DateTimeFormatter formatter, ResponseCaseData caseData, in return formatter.format(caseData.getRegistrarDirections().get(ind).getValue().getAddedDateTime()); } + @Test + void neverAddNonSolsAliasesToBuilder() { + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(NO) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .deceasedAliasNameList(List.of(DEC_PROBATE_ALIAS_NAME_CM)) + .solsDeceasedAliasNamesList(List.of(SOL_DEC_ALIAS_NAME_CM)) + .build(); + + when(caseDetailsMock.getData()).thenReturn(caseData); + + final String eventId = "TEST_EVENT"; + final boolean transform = false; + + final Set> expAliases = Set.of( + WILL_ALIAS_NAME_CM, + DEC_ALIAS_NAME_CM, + SOL_DEC_ALIAS_NAME_CM); + final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + + try (MockedStatic respCaseData = mockStatic(ResponseCaseData.class)) { + respCaseData.when(ResponseCaseData::builder).thenReturn(builderSpy); + + underTest.getResponseCaseData(caseDetailsMock, eventId, transform); + } + + verify(builderSpy, never()).deceasedAnyOtherNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasFirstNameOnWill(any()); + verify(builderSpy, never()).deceasedAliasLastNameOnWill(any()); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + private static final class AliasMatcher implements ArgumentMatcher>> { private final Set> expects; From 24b7e3ee7e777201041511221ffc3a9318ede40a Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 14:02:07 +0000 Subject: [PATCH 07/18] Add tests to validate existing getResponseCaseData behaviour. --- .../CallbackResponseTransformerTest.java | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java index 662d6226cb..21f25c830f 100644 --- a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java @@ -4786,6 +4786,86 @@ private String format(DateTimeFormatter formatter, ResponseCaseData caseData, in return formatter.format(caseData.getRegistrarDirections().get(ind).getValue().getAddedDateTime()); } + @Test + void verify_getResponseCaseData_no_willAlias_decAlias_solDecAlias() { + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(NO) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .deceasedAliasNameList(List.of(DEC_PROBATE_ALIAS_NAME_CM)) + .solsDeceasedAliasNamesList(List.of(SOL_DEC_ALIAS_NAME_CM)) + .build(); + + when(caseDetailsMock.getData()).thenReturn(caseData); + + final String eventId = "TEST_EVENT"; + final boolean transform = false; + + final Set> expAliases = Set.of( + WILL_ALIAS_NAME_CM, + DEC_ALIAS_NAME_CM, + SOL_DEC_ALIAS_NAME_CM); + final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + + try (MockedStatic respCaseData = mockStatic(ResponseCaseData.class)) { + respCaseData.when(ResponseCaseData::builder).thenReturn(builderSpy); + + underTest.getResponseCaseData(caseDetailsMock, eventId, transform); + } + + verify(builderSpy).deceasedAnyOtherNameOnWill(NO); + verify(builderSpy).deceasedAliasFirstNameOnWill(WILL_ALIAS_FN); + verify(builderSpy).deceasedAliasLastNameOnWill(WILL_ALIAS_LN); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + + @Test + void verify_getResponseCaseData_yes_willAlias_decAlias_solDecAlias() { + final var builder = ResponseCaseData.builder(); + final var builderSpy = spy(builder); + + final CaseData caseData = CaseData.builder() + .deceasedAnyOtherNameOnWill(YES) + .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) + .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .deceasedAliasNameList(List.of(DEC_PROBATE_ALIAS_NAME_CM)) + .solsDeceasedAliasNamesList(List.of(SOL_DEC_ALIAS_NAME_CM)) + .build(); + + when(caseDetailsMock.getData()).thenReturn(caseData); + + final String eventId = "TEST_EVENT"; + final boolean transform = false; + + final Set> expAliases = Set.of( + WILL_ALIAS_NAME_CM, + DEC_ALIAS_NAME_CM, + SOL_DEC_ALIAS_NAME_CM); + final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + + try (MockedStatic respCaseData = mockStatic(ResponseCaseData.class)) { + respCaseData.when(ResponseCaseData::builder).thenReturn(builderSpy); + + underTest.getResponseCaseData(caseDetailsMock, eventId, transform); + } + + verify(builderSpy).deceasedAnyOtherNameOnWill(YES); + verify(builderSpy).deceasedAliasFirstNameOnWill(WILL_ALIAS_FN); + verify(builderSpy).deceasedAliasLastNameOnWill(WILL_ALIAS_LN); + + verify(builderSpy, never()).deceasedAliasNamesList(any()); + + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); + verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); + } + @Test void neverAddNonSolsAliasesToBuilder() { final var builder = ResponseCaseData.builder(); From f1c9e69802b656f3da641593f6661d631c1e821d Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 12:42:45 +0000 Subject: [PATCH 08/18] Use newly introduced functionality. --- .../probate/transformer/CallbackResponseTransformer.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java index 441c8b645f..af46620feb 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java @@ -1047,7 +1047,6 @@ private CallbackResponse transformResponse(ResponseCaseData responseCaseData) { .primaryApplicantAddress(caseData.getPrimaryApplicantAddress()) .primaryApplicantNotRequiredToSendDocuments(caseData.getPrimaryApplicantNotRequiredToSendDocuments()) .solsAdditionalInfo(caseData.getSolsAdditionalInfo()) - .solsDeceasedAliasNamesList(getSolsDeceasedAliasNamesList(caseData)) .caseMatches(caseData.getCaseMatches()) .solsSOTNeedToUpdate(caseData.getSolsSOTNeedToUpdate()) @@ -1284,9 +1283,6 @@ private CallbackResponse transformResponse(ResponseCaseData responseCaseData) { .paymentTaken(caseData.getPaymentTaken()) .hmrcLetterId(caseData.getHmrcLetterId()) .uniqueProbateCodeId(caseData.getUniqueProbateCodeId()) - .deceasedAnyOtherNameOnWill(caseData.getDeceasedAnyOtherNameOnWill()) - .deceasedAliasFirstNameOnWill(caseData.getDeceasedAliasFirstNameOnWill()) - .deceasedAliasLastNameOnWill(caseData.getDeceasedAliasLastNameOnWill()) .boHandoffReasonList(getHandoffReasonList(caseData)) .lastModifiedDateForDormant(getLastModifiedDate(eventId, caseData.getLastModifiedDateForDormant())) .applicationSubmittedBy(caseData.getApplicationSubmittedBy()) @@ -1300,6 +1296,11 @@ private CallbackResponse transformResponse(ResponseCaseData responseCaseData) { .citizenDocumentsUploaded(caseData.getCitizenDocumentsUploaded()) .isSaveAndClose(caseData.getIsSaveAndClose()); + handleDeceasedAliases( + builder, + caseData, + caseDetails.getId()); + if (transform) { updateCaseBuilderForTransformCase(caseData, builder); } else { From ac5304c2df2203e6b69973ad6f3af76525d7134d Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 14:03:22 +0000 Subject: [PATCH 09/18] Use feature toggle to control new behaviour. --- .../probate/service/FeatureToggleService.java | 3 +++ .../CallbackResponseTransformer.java | 19 +++++++++++++++---- .../CallbackResponseTransformerTest.java | 6 ++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/probate/service/FeatureToggleService.java b/src/main/java/uk/gov/hmcts/probate/service/FeatureToggleService.java index dc8ae619c0..fb8a7a6fd2 100644 --- a/src/main/java/uk/gov/hmcts/probate/service/FeatureToggleService.java +++ b/src/main/java/uk/gov/hmcts/probate/service/FeatureToggleService.java @@ -38,4 +38,7 @@ public boolean isFeatureToggleOn(String featureToggleCode, boolean defaultValue) return this.ldClient.boolVariation(featureToggleCode, this.ldUser, defaultValue); } + public boolean enableNewAliasTransformation() { + return this.isFeatureToggleOn("probate-enable-new-alias-transformation", false); + } } diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java index af46620feb..c624f6094f 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java @@ -34,6 +34,7 @@ import uk.gov.hmcts.probate.model.payments.pba.OrganisationEntityResponse; import uk.gov.hmcts.probate.service.ExceptedEstateDateOfDeathChecker; import uk.gov.hmcts.probate.service.ExecutorsApplyingNotificationService; +import uk.gov.hmcts.probate.service.FeatureToggleService; import uk.gov.hmcts.probate.service.organisations.OrganisationsRetrievalService; import uk.gov.hmcts.probate.service.solicitorexecutor.FormattingService; import uk.gov.hmcts.probate.service.tasklist.TaskListUpdateService; @@ -143,6 +144,8 @@ public class CallbackResponseTransformer { private final IhtEstateDefaulter ihtEstateDefaulter; private final Iht400421Defaulter iht400421Defaulter; private final ExceptedEstateDateOfDeathChecker exceptedEstateDateOfDeathChecker; + private final FeatureToggleService featureToggleService; + @Value("${make_dormant.add_time_minutes}") private int makeDormantAddTimeMinutes; @@ -1296,10 +1299,18 @@ private CallbackResponse transformResponse(ResponseCaseData responseCaseData) { .citizenDocumentsUploaded(caseData.getCitizenDocumentsUploaded()) .isSaveAndClose(caseData.getIsSaveAndClose()); - handleDeceasedAliases( - builder, - caseData, - caseDetails.getId()); + if (featureToggleService.enableNewAliasTransformation()) { + handleDeceasedAliases( + builder, + caseData, + caseDetails.getId()); + } else { + builder.solsDeceasedAliasNamesList(getSolsDeceasedAliasNamesList(caseData)); + + builder.deceasedAnyOtherNameOnWill(caseData.getDeceasedAnyOtherNameOnWill()); + builder.deceasedAliasFirstNameOnWill(caseData.getDeceasedAliasFirstNameOnWill()); + builder.deceasedAliasLastNameOnWill(caseData.getDeceasedAliasLastNameOnWill()); + } if (transform) { updateCaseBuilderForTransformCase(caseData, builder); diff --git a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java index 21f25c830f..29070bf48e 100644 --- a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java @@ -54,6 +54,7 @@ import uk.gov.hmcts.probate.model.payments.pba.OrganisationEntityResponse; import uk.gov.hmcts.probate.service.ExceptedEstateDateOfDeathChecker; import uk.gov.hmcts.probate.service.ExecutorsApplyingNotificationService; +import uk.gov.hmcts.probate.service.FeatureToggleService; import uk.gov.hmcts.probate.service.StateChangeService; import uk.gov.hmcts.probate.service.organisations.OrganisationsRetrievalService; import uk.gov.hmcts.probate.service.solicitorexecutor.ExecutorListMapperService; @@ -556,6 +557,9 @@ class CallbackResponseTransformerTest { @Mock Document coversheetMock; + @Mock + private FeatureToggleService featureToggleService; + @BeforeEach public void setup() { @@ -4890,6 +4894,8 @@ void neverAddNonSolsAliasesToBuilder() { SOL_DEC_ALIAS_NAME_CM); final AliasMatcher expAliasMatcher = new AliasMatcher(expAliases); + when(featureToggleService.enableNewAliasTransformation()).thenReturn(true); + try (MockedStatic respCaseData = mockStatic(ResponseCaseData.class)) { respCaseData.when(ResponseCaseData::builder).thenReturn(builderSpy); From 0725463c785c00f5acd23f5049558bde98b3dccc Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 16:48:17 +0000 Subject: [PATCH 10/18] Update tests to not always have the same id so dupes can happen. --- .../CallbackResponseTransformerTest.java | 54 ++++++++++++++----- 1 file changed, 42 insertions(+), 12 deletions(-) diff --git a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java index 29070bf48e..01f9e5d629 100644 --- a/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformerTest.java @@ -336,9 +336,12 @@ class CallbackResponseTransformerTest { .lastName(WILL_ALIAS_LN) .build(); private static final CollectionMember WILL_PROBATE_ALIAS_NAME_CM = new CollectionMember<>( + "WILL_PROBATE_ALIAS_NAME_ID", WILL_PROBATE_ALIAS_NAME); private static final AliasName WILL_ALIAS_NAME = AliasName.builder().solsAliasname(WILL_ALIAS).build(); - private static final CollectionMember WILL_ALIAS_NAME_CM = new CollectionMember<>(WILL_ALIAS_NAME); + private static final CollectionMember WILL_ALIAS_NAME_CM = new CollectionMember<>( + "WILL_ALIAS_NAME_ID", + WILL_ALIAS_NAME); private static final String DEC_ALIAS_FN = "DecAliasFN"; private static final String DEC_ALIAS_LN = "DecAliasLN"; @@ -348,9 +351,12 @@ class CallbackResponseTransformerTest { .lastName(DEC_ALIAS_LN) .build(); private static final CollectionMember DEC_PROBATE_ALIAS_NAME_CM = new CollectionMember<>( + "DEC_PROBATE_ALIAS_NAME_ID", DEC_PROBATE_ALIAS_NAME); private static final AliasName DEC_ALIAS_NAME = AliasName.builder().solsAliasname(DEC_ALIAS).build(); - private static final CollectionMember DEC_ALIAS_NAME_CM = new CollectionMember<>(DEC_ALIAS_NAME); + private static final CollectionMember DEC_ALIAS_NAME_CM = new CollectionMember<>( + "DEC_ALIAS_NAME_ID", + DEC_ALIAS_NAME); private static final String SOL_DEC_ALIAS_FN = "SolDecAliasFN"; private static final String SOL_DEC_ALIAS_LN = "SolDecAliasLN"; @@ -360,9 +366,12 @@ class CallbackResponseTransformerTest { .lastName(SOL_DEC_ALIAS_LN) .build(); private static final CollectionMember SOL_DEC_PROBATE_ALIAS_NAME_CM = new CollectionMember<>( + "SOL_DEC_PROBATE_ALIAS_NAME_ID", SOL_DEC_PROBATE_ALIAS_NAME); private static final AliasName SOL_DEC_ALIAS_NAME = AliasName.builder().solsAliasname(SOL_DEC_ALIAS).build(); - private static final CollectionMember SOL_DEC_ALIAS_NAME_CM = new CollectionMember<>(SOL_DEC_ALIAS_NAME); + private static final CollectionMember SOL_DEC_ALIAS_NAME_CM = new CollectionMember<>( + "SOL_DEC_ALIAS_NAME_ID", + SOL_DEC_ALIAS_NAME); private List caseMatches = new ArrayList<>(); @@ -4913,10 +4922,12 @@ void neverAddNonSolsAliasesToBuilder() { } private static final class AliasMatcher implements ArgumentMatcher>> { - private final Set> expects; + private final Set expects; public AliasMatcher(Set> expects) { - this.expects = expects; + this.expects = expects.stream() + .map(cm -> cm.getValue().getSolsAliasname()) + .collect(Collectors.toUnmodifiableSet()); } @Override @@ -4928,13 +4939,15 @@ public boolean matches(List> arg) { return false; } - return arg.containsAll(expects); + return arg.stream() + .map(cm -> cm.getValue().getSolsAliasname()) + .toList() + .containsAll(expects); } @Override public String toString() { final String elements = expects.stream() - .map(c -> c.getValue().getSolsAliasname()) .collect(Collectors.joining(", ")); final StringBuilder description = new StringBuilder(); @@ -5297,6 +5310,7 @@ void givenAliasOnWillAndWillAliasInSolsDecAliasThenNoDuplicates() { .deceasedAnyOtherNameOnWill(NO) .deceasedAliasFirstNameOnWill(WILL_ALIAS_FN) .deceasedAliasLastNameOnWill(WILL_ALIAS_LN) + .deceasedAliasNameList(List.of(WILL_PROBATE_ALIAS_NAME_CM)) .solsDeceasedAliasNamesList(List.of(WILL_ALIAS_NAME_CM, SOL_DEC_ALIAS_NAME_CM)) .build(); @@ -5318,6 +5332,7 @@ void givenAliasOnWillAndWillAliasInSolsDecAliasThenNoDuplicates() { verify(builderSpy, never()).deceasedAliasNamesList(any()); + verify(builderSpy, never()).solsDeceasedAliasNamesList(argThat(expAliasMatcher.invert())); verify(builderSpy).solsDeceasedAliasNamesList(argThat(expAliasMatcher)); } @@ -5422,7 +5437,10 @@ void testConvertWillAlias_sameOnWillNo_fnPresent_lnPresent() { assertions.add(() -> assertNotNull(res, "Expected non-null response")); assertions.add(() -> assertEquals(1, res.size(), "Expected converted will aliases to have one entry")); - assertions.add(() -> assertTrue(res.contains(WILL_ALIAS_NAME_CM), "Expected will alias present")); + assertions.add(() -> assertTrue(res.stream() + .map(cm -> cm.getValue().getSolsAliasname()) + .anyMatch(WILL_ALIAS::equals), + "Expected will alias present")); assertAll(assertions); } @@ -5469,7 +5487,10 @@ void testConvertDecAliases_one() { assertions.add(() -> assertNotNull(res, "Expected non-null response")); assertions.add(() -> assertEquals(1, res.size(), "Expected converted dec aliases to be have one entry")); - assertions.add(() -> assertTrue(res.contains(DEC_ALIAS_NAME_CM), "Expected dec alias present")); + assertions.add(() -> assertTrue(res.stream() + .map(cm -> cm.getValue().getSolsAliasname()) + .anyMatch(DEC_ALIAS::equals), + "Expected dec alias present")); assertAll(assertions); } @@ -5487,8 +5508,14 @@ void testConvertDecAliases_two() { assertions.add(() -> assertNotNull(res, "Expected non-null response")); assertions.add(() -> assertEquals(2, res.size(), "Expected converted will aliases to have two entries")); - assertions.add(() -> assertTrue(res.contains(DEC_ALIAS_NAME_CM), "Expected dec alias present")); - assertions.add(() -> assertTrue(res.contains(WILL_ALIAS_NAME_CM), "Expected will alias present")); + assertions.add(() -> assertTrue(res.stream() + .map(cm -> cm.getValue().getSolsAliasname()) + .anyMatch(DEC_ALIAS::equals), + "Expected dec alias present")); + assertions.add(() -> assertTrue(res.stream() + .map(cm -> cm.getValue().getSolsAliasname()) + .anyMatch(WILL_ALIAS::equals), + "Expected will alias present")); assertAll(assertions); } @@ -5506,7 +5533,10 @@ void testConvertDecAliases_twoDuplicate() { assertions.add(() -> assertNotNull(res, "Expected non-null response")); assertions.add(() -> assertEquals(1, res.size(), "Expected converted dec aliases to have one entry")); - assertions.add(() -> assertTrue(res.contains(DEC_ALIAS_NAME_CM), "Expected will alias present")); + assertions.add(() -> assertTrue(res.stream() + .map(cm -> cm.getValue().getSolsAliasname()) + .anyMatch(DEC_ALIAS::equals), + "Expected dec alias present")); assertAll(assertions); } From 4f2416e659cf147c55cbc1e3bb599227e77bf61d Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 16:49:13 +0000 Subject: [PATCH 11/18] Filter responses before adding to remove dupes. --- .../probate/transformer/CallbackResponseTransformer.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java index c624f6094f..ac4b1cc34c 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java @@ -1369,7 +1369,12 @@ void handleDeceasedAliases( newSolsDecAliases.addAll(solsDecAliases); } - builder.solsDeceasedAliasNamesList(newSolsDecAliases.stream().toList()); + + Set seenAliasNames = new HashSet<>(); + + builder.solsDeceasedAliasNamesList(newSolsDecAliases.stream() + .filter(a -> seenAliasNames.add(a.getValue().getSolsAliasname())) + .toList()); } Set> convertAliasOnWillToSolsDecAliasList( From 0c5c2a167e54d8dba52a9bbfbced5a994f14f26b Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Mon, 18 Nov 2024 17:02:29 +0000 Subject: [PATCH 12/18] Use List so we can have more control over ordering. This is the opposite behaviour to the existing handling, which means if an additional value is appended it ends up at the _end_ of the new list. This probably aids in the current overriding behaviour where the values in the list end up being pushed off the end. --- .../CallbackResponseTransformer.java | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java index ac4b1cc34c..3bf64f69d8 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/CallbackResponseTransformer.java @@ -1352,23 +1352,24 @@ void handleDeceasedAliases( } } - Set> newSolsDecAliases = new HashSet<>(); - - final String decAliasFNOnWill = caseData.getDeceasedAliasFirstNameOnWill(); - final String decAliasLNOnWill = caseData.getDeceasedAliasLastNameOnWill(); - - newSolsDecAliases.addAll(convertAliasOnWillToSolsDecAliasList( - caseRef, - decNameOnWillSame, - decAliasFNOnWill, - decAliasLNOnWill)); - - newSolsDecAliases.addAll(convertDecAliasesSolsDecAliasList(decAliases)); + List> newSolsDecAliases = new ArrayList<>(); if (solsDecAliases != null) { newSolsDecAliases.addAll(solsDecAliases); } + newSolsDecAliases.addAll(convertDecAliasesSolsDecAliasList(decAliases)); + + { + final String decAliasFNOnWill = caseData.getDeceasedAliasFirstNameOnWill(); + final String decAliasLNOnWill = caseData.getDeceasedAliasLastNameOnWill(); + + newSolsDecAliases.addAll(convertAliasOnWillToSolsDecAliasList( + caseRef, + decNameOnWillSame, + decAliasFNOnWill, + decAliasLNOnWill)); + } Set seenAliasNames = new HashSet<>(); @@ -1377,7 +1378,7 @@ void handleDeceasedAliases( .toList()); } - Set> convertAliasOnWillToSolsDecAliasList( + List> convertAliasOnWillToSolsDecAliasList( final Long caseRef, final String differentNameOnWill, final String foreNames, @@ -1395,7 +1396,7 @@ Set> convertAliasOnWillToSolsDecAliasList( .build(); final CollectionMember listMember = new CollectionMember<>(alias); - return Set.of(listMember); + return List.of(listMember); } else { log.info("For case {}, foreNames == null: {}, lastName == null: {}," + " so alias is not being added to solsDecAlias list", @@ -1404,13 +1405,13 @@ Set> convertAliasOnWillToSolsDecAliasList( lastName == null); } } - return Set.of(); + return List.of(); } - Set> convertDecAliasesSolsDecAliasList( + List> convertDecAliasesSolsDecAliasList( final List> decAliases) { if (decAliases == null || decAliases.isEmpty()) { - return Set.of(); + return List.of(); } final Function, ProbateAliasName> unwrap = c -> c.getValue(); @@ -1429,11 +1430,14 @@ Set> convertDecAliasesSolsDecAliasList( final Function> wrap = a -> new CollectionMember<>(a); + Set seenAliasNames = new HashSet<>(); + return decAliases.stream() .map(unwrap) .map(convert) .map(wrap) - .collect(Collectors.toUnmodifiableSet()); + .filter(cm -> seenAliasNames.add(cm.getValue().getSolsAliasname())) + .toList(); } OrganisationPolicy buildOrganisationPolicy(CaseDetails caseDetails, String authToken) { From 02fee8ac38902343b2e0d79bbde9c8c066957c85 Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Tue, 29 Oct 2024 14:20:49 +0000 Subject: [PATCH 13/18] Add logback-test for ease of configuring logging whilst testing. --- src/main/resources/logback-test.xml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 src/main/resources/logback-test.xml diff --git a/src/main/resources/logback-test.xml b/src/main/resources/logback-test.xml new file mode 100644 index 0000000000..a945f554c7 --- /dev/null +++ b/src/main/resources/logback-test.xml @@ -0,0 +1,19 @@ + + + + + + + + + + %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} -%kvp- %msg%n + + + + + + + + + From 087fad22d4830af5a10cfae3b7b727eac08c0851 Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Tue, 29 Oct 2024 14:24:54 +0000 Subject: [PATCH 14/18] Update constant name, add constant for NoneOfThese. --- src/main/java/uk/gov/hmcts/probate/model/Constants.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/gov/hmcts/probate/model/Constants.java b/src/main/java/uk/gov/hmcts/probate/model/Constants.java index 5797ba4207..728bf0c52b 100644 --- a/src/main/java/uk/gov/hmcts/probate/model/Constants.java +++ b/src/main/java/uk/gov/hmcts/probate/model/Constants.java @@ -35,7 +35,7 @@ public final class Constants { public static final String DOC_TYPE_CHERISHED = "cherished"; public static final String DOC_TYPE_OTHER = "other"; public static final String DATE_OF_DEATH_TYPE_DEFAULT = "diedOn"; - public static final String CASE_TYPE_DEFAULT = "gop"; + public static final String CASE_TYPE_GRANT_OF_PROBATE = "gop"; public static final String DOCMOSIS_OUTPUT_PDF = "pdf"; public static final String DOCMOSIS_OUTPUT_HTML = "html"; public static final String REDEC_NOTIFICATION_SENT_STATE = "BORedecNotificationSent"; @@ -74,6 +74,7 @@ public final class Constants { public static final String TITLE_AND_CLEARING_PARTNER_OTHERS_RENOUNCING = "TCTPartOthersRenouncing"; public static final String TITLE_AND_CLEARING_PARTNER_SUCC_ALL_RENOUNCING = "TCTPartSuccAllRenouncing"; public static final String TITLE_AND_CLEARING_PARTNER_ALL_RENOUNCING = "TCTPartAllRenouncing"; + public static final String TITLE_AND_CLEARING_NONE_OF_THESE = "TCTNoT"; public static final String REASON_FOR_NOT_APPLYING_RENUNCIATION = "Renunciation"; public static final String REASON_FOR_NOT_APPLYING_MENTALLY_INCAPABLE = "MentallyIncapable"; public static final String REASON_FOR_NOT_APPLYING_DIED_BEFORE = "DiedBefore"; From 4de454b14adfe77d4ce4f5a4c8ef7c71ba245ca8 Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Tue, 29 Oct 2024 14:26:04 +0000 Subject: [PATCH 15/18] Add feature toggle for these changes. Also reworks the FeatureToggleService to not use deprecated functionality. --- .../probate/service/FeatureToggleService.java | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/probate/service/FeatureToggleService.java b/src/main/java/uk/gov/hmcts/probate/service/FeatureToggleService.java index fb8a7a6fd2..c998312496 100644 --- a/src/main/java/uk/gov/hmcts/probate/service/FeatureToggleService.java +++ b/src/main/java/uk/gov/hmcts/probate/service/FeatureToggleService.java @@ -1,6 +1,6 @@ package uk.gov.hmcts.probate.service; -import com.launchdarkly.sdk.LDUser; +import com.launchdarkly.sdk.LDContext; import com.launchdarkly.sdk.server.LDClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -10,32 +10,43 @@ public class FeatureToggleService { private final LDClient ldClient; - private final LDUser ldUser; - private final LDUser.Builder ldUserBuilder; + private final LDContext ldContext; + @Autowired public FeatureToggleService(LDClient ldClient, @Value("${ld.user.key}") String ldUserKey, @Value("${ld.user.firstName}") String ldUserFirstName, @Value("${ld.user.lastName}") String ldUserLastName) { + + final String contextName = new StringBuilder() + .append(ldUserFirstName) + .append(" ") + .append(ldUserLastName) + .toString(); + this.ldClient = ldClient; - - this.ldUserBuilder = new LDUser.Builder(ldUserKey) - .firstName(ldUserFirstName) - .lastName(ldUserLastName) - .custom("timestamp", String.valueOf(System.currentTimeMillis())); - this.ldUser = this.ldUserBuilder.build(); + this.ldContext = LDContext.builder(ldUserKey) + .name(contextName) + .kind("application") + .set("timestamp", String.valueOf(System.currentTimeMillis())) + .build(); + } public boolean isNewFeeRegisterCodeEnabled() { - return this.ldClient.boolVariation("probate-newfee-register-code", this.ldUser, true); + return isFeatureToggleOn("probate-newfee-register-code", true); } public boolean enableNewMarkdownFiltering() { - return this.ldClient.boolVariation("probate-enable-new-markdown-filtering", this.ldUser, false); + return isFeatureToggleOn("probate-enable-new-markdown-filtering", false); + } + + public boolean enableDuplicateExecutorFiltering() { + return isFeatureToggleOn("probate-enable-duplicate-executor-filtering", false); } public boolean isFeatureToggleOn(String featureToggleCode, boolean defaultValue) { - return this.ldClient.boolVariation(featureToggleCode, this.ldUser, defaultValue); + return this.ldClient.boolVariation(featureToggleCode, this.ldContext, defaultValue); } public boolean enableNewAliasTransformation() { From fa4cf19d5fc6ab99aa81678e5473bb97b3dcc8a0 Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Tue, 29 Oct 2024 14:29:21 +0000 Subject: [PATCH 16/18] Add handling in CaseData to clear the primary applicant. The behaviour here matches what the result of creating a non-NoneOfThese case so may not be quite what is expected in a generic handler. --- .../model/ccd/raw/request/CaseData.java | 27 +++++++++++ .../model/ccd/raw/request/CaseDataTest.java | 48 ++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/gov/hmcts/probate/model/ccd/raw/request/CaseData.java b/src/main/java/uk/gov/hmcts/probate/model/ccd/raw/request/CaseData.java index e18cba8411..c88752795c 100644 --- a/src/main/java/uk/gov/hmcts/probate/model/ccd/raw/request/CaseData.java +++ b/src/main/java/uk/gov/hmcts/probate/model/ccd/raw/request/CaseData.java @@ -8,6 +8,7 @@ import lombok.Getter; import lombok.experimental.SuperBuilder; import lombok.extern.jackson.Jacksonized; +import lombok.extern.slf4j.Slf4j; import uk.gov.hmcts.probate.controller.validation.AmendCaseDetailsGroup; import uk.gov.hmcts.probate.controller.validation.ApplicationAdmonGroup; import uk.gov.hmcts.probate.controller.validation.ApplicationCreatedGroup; @@ -68,12 +69,14 @@ import static uk.gov.hmcts.probate.model.Constants.NO; import static uk.gov.hmcts.probate.model.Constants.YES; +import static uk.gov.hmcts.probate.transformer.CallbackResponseTransformer.ANSWER_NO; @JsonInclude(JsonInclude.Include.NON_NULL) @SuperBuilder @Jacksonized @EqualsAndHashCode(callSuper = true) @Data +@Slf4j public class CaseData extends CaseDataParent { // Tasklist update @@ -708,4 +711,28 @@ public boolean isLanguagePreferenceWelsh() { return YES.equals(getLanguagePreferenceWelsh()); } + public void clearPrimaryApplicant() { + log.debug("Clearing primary applicant information from CaseData"); + + + this.setPrimaryApplicantIsApplying(null); + + this.setPrimaryApplicantForenames(null); + this.setPrimaryApplicantSurname(null); + + // This is to be consistent with the behaviour currently exhibited by the service when creating + // a case with a non-NoneOfThese TitleAndClearingType. + this.setPrimaryApplicantHasAlias(ANSWER_NO); + this.setPrimaryApplicantAlias(null); + + + // As above this is to be consistent with the behaviour currently exhibited by the service when + // creating a case with a non-NoneOfThese TitleAndClearingType. + final SolsAddress nullAddress = SolsAddress.builder().build(); + this.setPrimaryApplicantAddress(nullAddress); + + this.setPrimaryApplicantEmailAddress(null); + this.setPrimaryApplicantPhoneNumber(null); + } + } diff --git a/src/test/java/uk/gov/hmcts/probate/model/ccd/raw/request/CaseDataTest.java b/src/test/java/uk/gov/hmcts/probate/model/ccd/raw/request/CaseDataTest.java index 254cfc953b..c86e1c46b0 100644 --- a/src/test/java/uk/gov/hmcts/probate/model/ccd/raw/request/CaseDataTest.java +++ b/src/test/java/uk/gov/hmcts/probate/model/ccd/raw/request/CaseDataTest.java @@ -1,5 +1,6 @@ package uk.gov.hmcts.probate.model.ccd.raw.request; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; @@ -31,6 +32,7 @@ import static org.mockito.MockitoAnnotations.openMocks; import static uk.gov.hmcts.probate.model.Constants.NO; import static uk.gov.hmcts.probate.model.Constants.YES; +import static uk.gov.hmcts.probate.transformer.CallbackResponseTransformer.ANSWER_NO; class CaseDataTest { @@ -92,10 +94,12 @@ class CaseDataTest { private CaseData underTest; + private AutoCloseable closeableMocks; + @BeforeEach public void setup() { - openMocks(this); + closeableMocks = openMocks(this); when(additionalExecutors1Mock.getValue()).thenReturn(additionalExecutor1Mock); when(additionalExecutors2Mock.getValue()).thenReturn(additionalExecutor2Mock); @@ -136,6 +140,11 @@ public void setup() { .build(); } + @AfterEach + void tearDown() throws Exception { + closeableMocks.close(); + } + @Test void shouldReturnPrimaryApplicantFullName() { final CaseData caseData = CaseData.builder() @@ -953,4 +962,41 @@ void shouldApplyWillConditionAttributes() { assertEquals("Yes", caseData.getDeceasedWrittenWishes()); } + + @Test + void clearPrimaryApplicantClearsPrimaryApplicant() { + final SolsAddress nullAddress = SolsAddress.builder().build(); + + final CaseData.CaseDataBuilder caseDataBuilder = CaseData.builder() + .primaryApplicantHasAlias(ANSWER_NO) + .primaryApplicantAddress(nullAddress); + + final CaseData baseCaseData = caseDataBuilder.build(); + + final SolsAddress applAddress = SolsAddress.builder() + .addressLine1("addr1") + .addressLine2("addr2") + .addressLine3("addr3") + .country("country") + .county("county") + .postCode("postcode") + .postTown("town") + .build(); + + final CaseData applCaseData = caseDataBuilder + .primaryApplicantIsApplying("yes") + .primaryApplicantHasAlias("yes") + .primaryApplicantForenames("fname") + .primaryApplicantSurname("sname") + .primaryApplicantHasAlias("yes") + .primaryApplicantAlias("alias") + .primaryApplicantEmailAddress("email") + .primaryApplicantPhoneNumber("phone") + .primaryApplicantAddress(applAddress) + .build(); + + applCaseData.clearPrimaryApplicant(); + + assertEquals(baseCaseData, applCaseData); + } } From 745a4aed35a40b12dc5b639d4d43b7b45c1c049d Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Tue, 29 Oct 2024 14:44:45 +0000 Subject: [PATCH 17/18] Add feature flag service as a parameter to the Transformer. Not currently used, but also updates the tests to use the new parameter, and converts them to be slightly more independant (i.e. to not reuse mocks and builders between test cases). --- ...citorApplicationCompletionTransformer.java | 10 +- .../ExecutorsTransformerTest.java | 33 +++- ...rApplicationCompletionTransformerTest.java | 146 ++++++++++-------- 3 files changed, 116 insertions(+), 73 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformer.java index ff860335d6..1603d39860 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformer.java @@ -7,6 +7,7 @@ import uk.gov.hmcts.probate.model.ccd.raw.request.CaseData; import uk.gov.hmcts.probate.model.ccd.raw.request.CaseDetails; import uk.gov.hmcts.probate.service.DateFormatterService; +import uk.gov.hmcts.probate.service.FeatureToggleService; import uk.gov.hmcts.probate.service.solicitorexecutor.ExecutorListMapperService; import java.math.BigDecimal; @@ -21,11 +22,16 @@ // for caseworker or solicitor journeys public class SolicitorApplicationCompletionTransformer extends LegalStatementExecutorTransformer { + private final FeatureToggleService featureToggleService; + private static final String NOT_APPLICABLE = "NotApplicable"; - public SolicitorApplicationCompletionTransformer(ExecutorListMapperService executorListMapperService, - DateFormatterService dateFormatterService) { + public SolicitorApplicationCompletionTransformer( + final ExecutorListMapperService executorListMapperService, + final DateFormatterService dateFormatterService, + final FeatureToggleService featureToggleService) { super(executorListMapperService, dateFormatterService); + this.featureToggleService = featureToggleService; } /** diff --git a/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/ExecutorsTransformerTest.java b/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/ExecutorsTransformerTest.java index 45d212ab73..f8e5b0be71 100644 --- a/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/ExecutorsTransformerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/ExecutorsTransformerTest.java @@ -17,17 +17,22 @@ import uk.gov.hmcts.probate.model.ccd.raw.request.CaseDetails; import uk.gov.hmcts.probate.model.ccd.raw.response.ResponseCaseData; import uk.gov.hmcts.probate.service.DateFormatterService; +import uk.gov.hmcts.probate.service.FeatureToggleService; import uk.gov.hmcts.probate.service.solicitorexecutor.ExecutorListMapperService; import java.util.ArrayList; import java.util.List; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static uk.gov.hmcts.probate.model.Constants.TITLE_AND_CLEARING_SOLE_PRINCIPLE; import static uk.gov.hmcts.probate.model.Constants.TITLE_AND_CLEARING_TRUST_CORP; @@ -246,22 +251,36 @@ void shouldNotChangeResponseCaseData() { assertEquals(SOLICITOR_NOT_APPLYING_REASON, cd.getSolsSolicitorNotApplyingReason()); } + /* Should this test be in this Test class? It's testing a subclass and is really relying on a third + * service (ExecutorListMapperService) to do most of the work here. + */ @Test void shouldSwapSolicitorToNotApplyingList() { - caseDataBuilder + final CaseData caseData = CaseData.builder() .additionalExecutorsApplying(additionalExecutorApplying) .additionalExecutorsNotApplying(additionalExecutorNotApplying) .solsSolicitorIsExec(YES) .solsSolicitorIsApplying(NO) - .primaryApplicantForenames(EXEC_FIRST_NAME); + .primaryApplicantForenames(EXEC_FIRST_NAME) + .build(); - final CaseData cd = caseDataBuilder.build(); + final var executorListMapperSpy = spy(ExecutorListMapperService.class); + + // We only need these to construct the SolicitorXformer object, we verifyNoInteractions to confirm + final var dateFormatterMock = mock(DateFormatterService.class); + final var featureToggleMock = mock(FeatureToggleService.class); - new SolicitorApplicationCompletionTransformer(new ExecutorListMapperService(), new DateFormatterService()) - .mapSolicitorExecutorFieldsOnCompletion(cd); + final var solXformer = new SolicitorApplicationCompletionTransformer( + executorListMapperSpy, + dateFormatterMock, + featureToggleMock); + solXformer.mapSolicitorExecutorFieldsOnCompletion(caseData); - assertEquals(0, cd.getAdditionalExecutorsApplying().size()); - assertEquals(1, cd.getAdditionalExecutorsNotApplying().size()); + assertAll( + () -> assertEquals(0, caseData.getAdditionalExecutorsApplying().size()), + () -> assertEquals(1, caseData.getAdditionalExecutorsNotApplying().size()), + () -> verifyNoInteractions(dateFormatterMock, featureToggleMock) + ); } @Test diff --git a/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformerTest.java b/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformerTest.java index 2fc03b1677..0d0f54a42d 100644 --- a/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformerTest.java @@ -1,11 +1,11 @@ package uk.gov.hmcts.probate.transformer.solicitorexecutors; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.mockito.MockitoAnnotations; import uk.gov.hmcts.probate.model.ccd.raw.AdditionalExecutor; import uk.gov.hmcts.probate.model.ccd.raw.AdditionalExecutorApplying; import uk.gov.hmcts.probate.model.ccd.raw.AdditionalExecutorNotApplying; @@ -17,6 +17,7 @@ import uk.gov.hmcts.probate.model.ccd.raw.request.CaseData; import uk.gov.hmcts.probate.model.ccd.raw.request.CaseDetails; import uk.gov.hmcts.probate.service.DateFormatterService; +import uk.gov.hmcts.probate.service.FeatureToggleService; import uk.gov.hmcts.probate.service.solicitorexecutor.ExecutorListMapperService; import java.math.BigDecimal; @@ -25,8 +26,13 @@ import java.util.Arrays; import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static uk.gov.hmcts.probate.model.Constants.TITLE_AND_CLEARING_TRUST_CORP; import static uk.gov.hmcts.probate.util.CommonVariables.ADDITIONAL_EXECUTOR_APPLYING; @@ -44,22 +50,21 @@ import static uk.gov.hmcts.probate.util.CommonVariables.SOLS_EXEC_NOT_APPLYING; import static uk.gov.hmcts.probate.util.CommonVariables.YES; -@ExtendWith(SpringExtension.class) class SolicitorApplicationCompletionTransformerTest { - private final CaseData.CaseDataBuilder caseDataBuilder = CaseData.builder(); - - @Mock - private CaseDetails caseDetailsMock; - @Mock private DateFormatterService dateFormatterServiceMock; @Mock private ExecutorListMapperService executorListMapperServiceMock; + @Mock + private FeatureToggleService featureToggleServiceMock; + @InjectMocks - private SolicitorApplicationCompletionTransformer solicitorApplicationCompletionTransformerMock; + private SolicitorApplicationCompletionTransformer solicitorApplicationCompletionTransformer; + + private AutoCloseable closeableMocks; private List> additionalExecutorApplying; private List> additionalExecutorNotApplying; @@ -94,42 +99,57 @@ public void setUp() { dispenseWithNoticeExecList = new ArrayList<>(); dispenseWithNoticeExecList.add(DISPENSE_WITH_NOTICE_EXEC); + + closeableMocks = MockitoAnnotations.openMocks(this); } + @AfterEach + void cleanUp() throws Exception { + closeableMocks.close(); + } + /* Should this test be in this Test class? It's really relying on the ExecutorListMapperService + * to do the work being tested here. (Hence needing to create a new instance with a spy rather + * than the common handling with a mock.) + */ @Test void shouldSetLegalStatementFieldsWithApplyingExecutorInfo() { - - caseDataBuilder + final CaseData caseData = CaseData.builder() .solsSolicitorIsExec(YES) .solsSolicitorIsApplying(YES) .titleAndClearingType(TITLE_AND_CLEARING_TRUST_CORP) .anyOtherApplyingPartnersTrustCorp(YES) .additionalExecutorsTrustCorpList(trustCorpsExecutorList) - .solsAdditionalExecutorList(solsAdditionalExecutorList); + .solsAdditionalExecutorList(solsAdditionalExecutorList) + .build(); - CaseData caseData = caseDataBuilder.build(); + final var executorListMapperSpy = spy(ExecutorListMapperService.class); - SolicitorApplicationCompletionTransformer solJourneyCompletion = - new SolicitorApplicationCompletionTransformer(new ExecutorListMapperService(), new DateFormatterService()); + final var solApplComplXform = new SolicitorApplicationCompletionTransformer( + executorListMapperSpy, + dateFormatterServiceMock, + featureToggleServiceMock); - solJourneyCompletion.mapSolicitorExecutorFieldsOnCompletion(caseData); + solApplComplXform.mapSolicitorExecutorFieldsOnCompletion(caseData); - assertEquals(2, caseData.getAdditionalExecutorsApplying().size()); - assertEquals(3, caseData.getExecutorsApplyingLegalStatement().size()); + assertAll( + () -> assertEquals(2, caseData.getAdditionalExecutorsApplying().size()), + () -> assertEquals(3, caseData.getExecutorsApplyingLegalStatement().size()), + () -> verifyNoInteractions(dateFormatterServiceMock, featureToggleServiceMock) + ); } @Test void shouldSetLegalStatementFieldsWithNotApplyingExecutorInfo() { - caseDataBuilder + final CaseDetails caseDetailsMock = mock(CaseDetails.class); + final CaseData caseData = CaseData.builder() .solsSolicitorIsExec(YES) .solsSolicitorIsApplying(NO) .additionalExecutorsTrustCorpList(null) .otherPartnersApplyingAsExecutors(null) .dispenseWithNoticeOtherExecsList(dispenseWithNoticeExecList) - .solsAdditionalExecutorList(solsAdditionalExecutorList); - - CaseData caseData = caseDataBuilder.build(); + .solsAdditionalExecutorList(solsAdditionalExecutorList) + .build(); when(caseDetailsMock.getData()).thenReturn(caseData); when(executorListMapperServiceMock.mapFromDispenseWithNoticeExecsToNotApplyingExecutors( @@ -137,7 +157,7 @@ void shouldSetLegalStatementFieldsWithNotApplyingExecutorInfo() { when(executorListMapperServiceMock.addSolicitorToNotApplyingList( caseDetailsMock.getData(), additionalExecutorNotApplying)).thenReturn(additionalExecutorNotApplying); - solicitorApplicationCompletionTransformerMock.mapSolicitorExecutorFieldsOnCompletion(caseData); + solicitorApplicationCompletionTransformer.mapSolicitorExecutorFieldsOnCompletion(caseData); List> legalStatementExecutors = new ArrayList<>(); legalStatementExecutors.addAll(additionalExecutorNotApplying); @@ -148,19 +168,19 @@ void shouldSetLegalStatementFieldsWithNotApplyingExecutorInfo() { @Test void shouldSetLegalStatementFieldsWithApplyingExecutorInfo_PrimaryApplicantApplying() { - caseDataBuilder - .primaryApplicantForenames(EXEC_FIRST_NAME) - .primaryApplicantSurname(EXEC_SURNAME) - .primaryApplicantAlias(PRIMARY_EXEC_ALIAS_NAMES) - .primaryApplicantAddress(EXEC_ADDRESS) - .primaryApplicantIsApplying(YES); + final CaseData caseData = CaseData.builder() + .primaryApplicantForenames(EXEC_FIRST_NAME) + .primaryApplicantSurname(EXEC_SURNAME) + .primaryApplicantAlias(PRIMARY_EXEC_ALIAS_NAMES) + .primaryApplicantAddress(EXEC_ADDRESS) + .primaryApplicantIsApplying(YES) + .build(); + - when(caseDetailsMock.getData()).thenReturn(caseDataBuilder.build()); - when(executorListMapperServiceMock.mapFromPrimaryApplicantToApplyingExecutor( - caseDetailsMock.getData())).thenReturn(new CollectionMember<>(EXEC_ID, ADDITIONAL_EXECUTOR_APPLYING)); + when(executorListMapperServiceMock.mapFromPrimaryApplicantToApplyingExecutor(caseData)) + .thenReturn(new CollectionMember<>(EXEC_ID, ADDITIONAL_EXECUTOR_APPLYING)); - CaseData caseData = caseDetailsMock.getData(); - solicitorApplicationCompletionTransformerMock.mapSolicitorExecutorFieldsOnCompletion(caseData); + solicitorApplicationCompletionTransformer.mapSolicitorExecutorFieldsOnCompletion(caseData); assertEquals(additionalExecutorApplying, caseData.getExecutorsApplyingLegalStatement()); assertEquals(new ArrayList<>(), caseData.getExecutorsNotApplyingLegalStatement()); @@ -168,18 +188,16 @@ void shouldSetLegalStatementFieldsWithApplyingExecutorInfo_PrimaryApplicantApply @Test void shouldSetLegalStatementFieldsWithApplyingExecutorInfo_PrimaryApplicantNotApplying() { - caseDataBuilder + final CaseData caseData = CaseData.builder() .primaryApplicantIsApplying(NO) .solsSolicitorIsApplying(NO) - .solsSolicitorIsExec(YES); + .solsSolicitorIsExec(YES) + .build(); - when(caseDetailsMock.getData()).thenReturn(caseDataBuilder.build()); - when(executorListMapperServiceMock.mapFromPrimaryApplicantToNotApplyingExecutor( - caseDetailsMock.getData())).thenReturn(new CollectionMember<>(EXEC_ID, - ADDITIONAL_EXECUTOR_NOT_APPLYING)); + when(executorListMapperServiceMock.mapFromPrimaryApplicantToNotApplyingExecutor(caseData)) + .thenReturn(new CollectionMember<>(EXEC_ID, ADDITIONAL_EXECUTOR_NOT_APPLYING)); - CaseData caseData = caseDetailsMock.getData(); - solicitorApplicationCompletionTransformerMock.mapSolicitorExecutorFieldsOnCompletion(caseData); + solicitorApplicationCompletionTransformer.mapSolicitorExecutorFieldsOnCompletion(caseData); assertEquals(additionalExecutorNotApplying, caseData.getExecutorsNotApplyingLegalStatement()); assertEquals(new ArrayList<>(), caseData.getExecutorsApplyingLegalStatement()); @@ -187,18 +205,17 @@ void shouldSetLegalStatementFieldsWithApplyingExecutorInfo_PrimaryApplicantNotAp @Test void shouldSetLegalStatementFieldsWithApplyingExecutorInfoYesNo() { - caseDataBuilder - .primaryApplicantIsApplying(NO) - .solsSolicitorIsApplying(NO) - .solsSolicitorIsExec(YES); + final CaseData caseData = CaseData.builder() + .primaryApplicantIsApplying(NO) + .solsSolicitorIsApplying(NO) + .solsSolicitorIsExec(YES) + .build(); - when(caseDetailsMock.getData()).thenReturn(caseDataBuilder.build()); - when(executorListMapperServiceMock.mapFromPrimaryApplicantToNotApplyingExecutor( - caseDetailsMock.getData())).thenReturn(new CollectionMember<>(EXEC_ID, - ADDITIONAL_EXECUTOR_NOT_APPLYING)); - CaseData caseData = caseDetailsMock.getData(); - solicitorApplicationCompletionTransformerMock.mapSolicitorExecutorFieldsOnAppDetailsComplete(caseData); + when(executorListMapperServiceMock.mapFromPrimaryApplicantToNotApplyingExecutor(caseData)) + .thenReturn(new CollectionMember<>(EXEC_ID, ADDITIONAL_EXECUTOR_NOT_APPLYING)); + + solicitorApplicationCompletionTransformer.mapSolicitorExecutorFieldsOnAppDetailsComplete(caseData); assertEquals(additionalExecutorNotApplying, caseData.getExecutorsNotApplyingLegalStatement()); assertEquals(new ArrayList<>(), caseData.getExecutorsApplyingLegalStatement()); @@ -211,13 +228,14 @@ void shouldEraseCodicilAddedDateIfWillHasNoCodicils() { .dateCodicilAdded(LocalDate.now().minusDays(1)).build())); final List> formattedDate = Arrays.asList(new CollectionMember<>("Formatted Date")); - caseDataBuilder + + final CaseData caseData = CaseData.builder() .willHasCodicils(NO) .codicilAddedDateList(codicilDates) - .codicilAddedFormattedDateList(formattedDate); + .codicilAddedFormattedDateList(formattedDate) + .build(); - CaseData caseData = caseDataBuilder.build(); - solicitorApplicationCompletionTransformerMock.eraseCodicilAddedDateIfWillHasNoCodicils(caseData); + solicitorApplicationCompletionTransformer.eraseCodicilAddedDateIfWillHasNoCodicils(caseData); assertNull(caseData.getCodicilAddedDateList()); assertNull(caseData.getCodicilAddedFormattedDateList()); @@ -225,20 +243,20 @@ void shouldEraseCodicilAddedDateIfWillHasNoCodicils() { @Test void shouldSetServiceRequest() { - BigDecimal totalAmount = BigDecimal.valueOf(100000); - CaseData caseData = caseDataBuilder.build(); - CaseDetails caseDetails = new CaseDetails(caseData, null, 0L); - solicitorApplicationCompletionTransformerMock.setFieldsOnServiceRequest(caseDetails, totalAmount); + final BigDecimal totalAmount = BigDecimal.valueOf(100000); + final CaseData caseData = CaseData.builder().build(); + final CaseDetails caseDetails = new CaseDetails(caseData, null, 0L); + solicitorApplicationCompletionTransformer.setFieldsOnServiceRequest(caseDetails, totalAmount); assertNull(caseData.getPaymentTaken()); } @Test void shouldSetPaymentTakenNotApplicableWhenNoServiceRequest() { - BigDecimal totalAmount = BigDecimal.ZERO; - CaseData caseData = caseDataBuilder.build(); - CaseDetails caseDetails = new CaseDetails(caseData, null, 0L); - solicitorApplicationCompletionTransformerMock.setFieldsOnServiceRequest(caseDetails, totalAmount); + final BigDecimal totalAmount = BigDecimal.ZERO; + final CaseData caseData = CaseData.builder().build(); + final CaseDetails caseDetails = new CaseDetails(caseData, null, 0L); + solicitorApplicationCompletionTransformer.setFieldsOnServiceRequest(caseDetails, totalAmount); assertEquals(NOT_APPLICABLE, caseData.getPaymentTaken()); } From ed0d7f7855a5f23595c9f6c3d8eb24fb010c9e3f Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Tue, 29 Oct 2024 16:03:26 +0000 Subject: [PATCH 18/18] Add call into new handling to remove duplicate execs. --- .../transformer/CaseDataTransformer.java | 7 +- ...citorApplicationCompletionTransformer.java | 28 ++++ ...rApplicationCompletionTransformerTest.java | 121 ++++++++++++++++++ 3 files changed, 155 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/CaseDataTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/CaseDataTransformer.java index de70f468d8..1a0c68f6f2 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/CaseDataTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/CaseDataTransformer.java @@ -56,7 +56,12 @@ public void transformForSolicitorApplicationCompletion(CallbackRequest callbackR public void transformCaseDataForValidateProbate(CallbackRequest callbackRequest) { - final var caseData = callbackRequest.getCaseDetails().getData(); + final var caseDetails = callbackRequest.getCaseDetails(); + final var caseData = caseDetails.getData(); + + solicitorApplicationCompletionTransformer.clearPrimaryApplicantWhenNotInNoneOfTheseTitleAndClearingType( + caseDetails); + resetCaseDataTransformer.resetExecutorLists(caseData); solicitorApplicationCompletionTransformer.setFieldsIfSolicitorIsNotNamedInWillAsAnExecutor(caseData); solicitorApplicationCompletionTransformer.mapSolicitorExecutorFieldsOnAppDetailsComplete(caseData); diff --git a/src/main/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformer.java b/src/main/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformer.java index 1603d39860..1d798545e1 100644 --- a/src/main/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformer.java +++ b/src/main/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformer.java @@ -14,7 +14,9 @@ import java.util.List; import static uk.gov.hmcts.probate.model.ApplicationState.CASE_PRINTED; +import static uk.gov.hmcts.probate.model.Constants.CASE_TYPE_GRANT_OF_PROBATE; import static uk.gov.hmcts.probate.model.Constants.NO; +import static uk.gov.hmcts.probate.model.Constants.TITLE_AND_CLEARING_NONE_OF_THESE; @Component @Slf4j @@ -69,4 +71,30 @@ public void setFieldsOnServiceRequest(CaseDetails caseDetails, BigDecimal totalA caseDetails.setState(CASE_PRINTED.getId()); } } + + public void clearPrimaryApplicantWhenNotInNoneOfTheseTitleAndClearingType(CaseDetails caseDetails) { + if (featureToggleService.enableDuplicateExecutorFiltering()) { + final var caseId = caseDetails.getId(); + final var caseData = caseDetails.getData(); + + final var titleAndClearingType = caseData.getTitleAndClearingType(); + final var caseType = caseData.getCaseType(); + + final var primaryApplicantApplying = caseData.isPrimaryApplicantApplying(); + final var isNotNoneOfTheseTCT = titleAndClearingType != null + && !TITLE_AND_CLEARING_NONE_OF_THESE.equalsIgnoreCase(titleAndClearingType); + final var isGrantOfProbate = CASE_TYPE_GRANT_OF_PROBATE.equalsIgnoreCase(caseType); + + if (isNotNoneOfTheseTCT + && primaryApplicantApplying + && isGrantOfProbate) { + log.info("In GrantOfProbate case {} we have primary applicant applying for non-NoneOfThese " + + "TitleAndClearingType {}, clear PrimaryApplicant fields", + caseId, + titleAndClearingType); + + caseData.clearPrimaryApplicant(); + } + } + } } diff --git a/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformerTest.java b/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformerTest.java index 0d0f54a42d..9b605f728b 100644 --- a/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/transformer/solicitorexecutors/SolicitorApplicationCompletionTransformerTest.java @@ -32,8 +32,12 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; +import static uk.gov.hmcts.probate.model.Constants.CASE_TYPE_GRANT_OF_PROBATE; +import static uk.gov.hmcts.probate.model.Constants.TITLE_AND_CLEARING_NONE_OF_THESE; import static uk.gov.hmcts.probate.model.Constants.TITLE_AND_CLEARING_TRUST_CORP; import static uk.gov.hmcts.probate.util.CommonVariables.ADDITIONAL_EXECUTOR_APPLYING; import static uk.gov.hmcts.probate.util.CommonVariables.ADDITIONAL_EXECUTOR_NOT_APPLYING; @@ -260,4 +264,121 @@ void shouldSetPaymentTakenNotApplicableWhenNoServiceRequest() { assertEquals(NOT_APPLICABLE, caseData.getPaymentTaken()); } + + // given Case + // and CaseType is GrantOfProbate + // and TitleClearingType is not NoneOfThese + // and PrimaryApplicantApplying is true + // when transformer clearPrimaryForNoneOfThese called + // then primaryApplicantClear is called + @Test + void givenCaseWithoutNoneOfTheseTitleClearingTypeANDPrimaryApplicant_whenChecked_thenPrimaryApplicantDataCleared() { + final CaseData caseData = mock(CaseData.class); + final CaseDetails caseDetails = new CaseDetails(caseData, null, 0L); + + when(featureToggleServiceMock.enableDuplicateExecutorFiltering()).thenReturn(true); + + when(caseData.getCaseType()).thenReturn(CASE_TYPE_GRANT_OF_PROBATE); + when(caseData.getTitleAndClearingType()).thenReturn(""); + when(caseData.isPrimaryApplicantApplying()).thenReturn(true); + + solicitorApplicationCompletionTransformer.clearPrimaryApplicantWhenNotInNoneOfTheseTitleAndClearingType( + caseDetails); + + verify(caseData, times(1)).clearPrimaryApplicant(); + } + + // given Case + // and CaseType is NOT GrantOfProbate + // and TitleClearingType is not NoneOfThese + // and PrimaryApplicantApplying is true + // when transformer clearPrimaryForNoneOfThese called + // then primaryApplicantClear is NOT called + @Test + void givenCaseNotGOPWoutNoneOfTheseTCTypeANDPrimaryApplicant_whenChecked_thenPrimaryApplicantDataNOTCleared() { + final CaseData caseData = mock(CaseData.class); + final CaseDetails caseDetails = new CaseDetails(caseData, null, 0L); + + when(featureToggleServiceMock.enableDuplicateExecutorFiltering()).thenReturn(true); + + when(caseData.getCaseType()).thenReturn(""); + when(caseData.getTitleAndClearingType()).thenReturn(""); + when(caseData.isPrimaryApplicantApplying()).thenReturn(true); + + solicitorApplicationCompletionTransformer.clearPrimaryApplicantWhenNotInNoneOfTheseTitleAndClearingType( + caseDetails); + + verify(caseData, times(0)).clearPrimaryApplicant(); + } + + // given Case + // and CaseType is GrantOfProbate + // and TitleClearingType is NoneOfThese + // and PrimaryApplicantApplying is true + // when transformer clearPrimaryForNoneOfThese called + // then primaryApplicantClear is not called + @Test + void givenCaseWithNoneOfTheseTitleClearingTypeANDPrimaryApplicant_whenChecked_thenPrimaryApplicantDataNOTCleared() { + final CaseData caseData = mock(CaseData.class); + final CaseDetails caseDetails = new CaseDetails(caseData, null, 0L); + + when(featureToggleServiceMock.enableDuplicateExecutorFiltering()).thenReturn(true); + + when(caseData.getCaseType()).thenReturn(CASE_TYPE_GRANT_OF_PROBATE); + when(caseData.getTitleAndClearingType()).thenReturn(TITLE_AND_CLEARING_NONE_OF_THESE); + when(caseData.isPrimaryApplicantApplying()).thenReturn(true); + + solicitorApplicationCompletionTransformer.clearPrimaryApplicantWhenNotInNoneOfTheseTitleAndClearingType( + caseDetails); + + verify(caseData, times(0)).clearPrimaryApplicant(); + } + + // given Case + // and CaseType is GrantOfProbate + // and TitleClearingType is not NoneOfThese + // and PrimaryApplicantApplying is false + // when transformer clearPrimaryForNoneOfThese called + // then primaryApplicantClear is not called + @Test + void givenCaseWoutNoneOfTheseTitleClearingTypeANDWoutPrmryApplicant_whenChecked_thenPrmryApplicantDataNOTCleared() { + final CaseData realCaseData = CaseData.builder().build(); + final CaseData spyCaseData = spy(realCaseData); + final CaseDetails caseDetails = new CaseDetails(spyCaseData, null, 0L); + + when(featureToggleServiceMock.enableDuplicateExecutorFiltering()).thenReturn(true); + + when(spyCaseData.getCaseType()).thenReturn(CASE_TYPE_GRANT_OF_PROBATE); + when(spyCaseData.getTitleAndClearingType()).thenReturn(""); + when(spyCaseData.isPrimaryApplicantApplying()).thenReturn(false); + + solicitorApplicationCompletionTransformer.clearPrimaryApplicantWhenNotInNoneOfTheseTitleAndClearingType( + caseDetails); + + verify(spyCaseData, times(0)).clearPrimaryApplicant(); + } + + // given Case + // and CaseType is GrantOfProbate + // and TitleClearingType is not NoneOfThese + // and PrimaryApplicantApplying is true + // and enableDuplicateApplicant is false + // when transformer clearPrimaryForNoneOfThese called + // then primaryApplicantClear is called + @Test + void givenCaseWithoutNoneOfTheseTtlClrngTypeANDPrmryApplANDDuplDisabled_whenChecked_thenPrmryApplDataNOTCleared() { + final CaseData caseData = mock(CaseData.class); + final CaseDetails caseDetails = new CaseDetails(caseData, null, 0L); + + when(featureToggleServiceMock.enableDuplicateExecutorFiltering()).thenReturn(false); + + when(caseData.getCaseType()).thenReturn(CASE_TYPE_GRANT_OF_PROBATE); + when(caseData.getTitleAndClearingType()).thenReturn(""); + when(caseData.isPrimaryApplicantApplying()).thenReturn(true); + + solicitorApplicationCompletionTransformer.clearPrimaryApplicantWhenNotInNoneOfTheseTitleAndClearingType( + caseDetails); + + verify(caseData, times(0)).clearPrimaryApplicant(); + } }