From 16f19360341e4b352d1c1e91b6d5c663b8381f50 Mon Sep 17 00:00:00 2001 From: benedwards Date: Wed, 20 Nov 2024 16:57:54 +0000 Subject: [PATCH 1/5] Updated case expiry logic to only expire event when all linked cases have expired --- .../CaseExpiryDeletionAutomatedTaskITest.java | 47 +++++++++++++++- .../testutils/PostgresIntegrationBase.java | 19 +++++-- ...hCleanupArmResponseFilesServiceCommon.java | 1 - .../repository/EventLinkedCaseRepository.java | 12 ++++ .../impl/DataAnonymisationServiceImpl.java | 22 ++++++-- .../darts/event/service/EventService.java | 2 + .../event/service/impl/EventServiceImpl.java | 4 ++ .../DataAnonymisationServiceImplTest.java | 56 ++++++++++++++++--- .../service/impl/EventServiceImplTest.java | 15 +++++ 9 files changed, 157 insertions(+), 21 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java b/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java index ba45bf1726c..e17cf7444af 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java @@ -12,6 +12,7 @@ import uk.gov.hmcts.darts.common.entity.DefenceEntity; import uk.gov.hmcts.darts.common.entity.DefendantEntity; import uk.gov.hmcts.darts.common.entity.EventEntity; +import uk.gov.hmcts.darts.common.entity.EventLinkedCaseEntity; import uk.gov.hmcts.darts.common.entity.HearingEntity; import uk.gov.hmcts.darts.common.entity.ProsecutorEntity; import uk.gov.hmcts.darts.common.entity.TranscriptionCommentEntity; @@ -37,7 +38,11 @@ import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.time.temporal.ChronoUnit; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.UUID; import java.util.regex.Pattern; @@ -66,6 +71,36 @@ void positiveRetentionDatePassed() { assertCase(caseId1, true); } + @Test + void positiveRetentionDatePassedForOneCaseButNotAnotherEventNotAnoymised() { + CourtCaseEntity courtCase1 = createCase(-1, CaseRetentionStatus.COMPLETE); + CourtCaseEntity courtCase2 = createCase(-1, CaseRetentionStatus.PENDING); + + EventEntity event = dartsDatabase.getEventLinkedCaseRepository().findAllByCourtCase(courtCase1).getFirst().getEvent(); + eventLinkedCaseStub.createCaseLinkedEvent(event, courtCase2); + final int caseId1 = courtCase1.getId(); + + caseExpiryDeletionAutomatedTask.preRunTask(); + caseExpiryDeletionAutomatedTask.runTask(); + + assertCase(caseId1, true, event.getId()); + } + + @Test + void positiveRetentionDatePassedForBothCaseLinkedEventsAnoymised() { + CourtCaseEntity courtCase1 = createCase(-1, CaseRetentionStatus.COMPLETE); + CourtCaseEntity courtCase2 = createCase(-1, CaseRetentionStatus.COMPLETE); + + EventEntity event = dartsDatabase.getEventLinkedCaseRepository().findAllByCourtCase(courtCase1).getFirst().getEvent(); + eventLinkedCaseStub.createCaseLinkedEvent(event, courtCase2); + final int caseId1 = courtCase1.getId(); + + caseExpiryDeletionAutomatedTask.preRunTask(); + caseExpiryDeletionAutomatedTask.runTask(); + + assertCase(caseId1, true); + } + @Test void positiveRetentionDateNotPassed() { @@ -111,7 +146,7 @@ void positiveMultipleToAnonymiseAndSomeNotTo() { } - private void assertCase(int caseId, boolean isAnonymised) { + private void assertCase(int caseId, boolean isAnonymised, int... excludeEventIds) { transactionalUtil.executeInTransaction(() -> { CourtCaseEntity courtCase = dartsDatabase.getCourtCaseStub().getCourtCase(caseId); assertThat(courtCase.isDataAnonymised()) @@ -140,7 +175,15 @@ private void assertCase(int caseId, boolean isAnonymised) { courtCase.getHearings().forEach(hearingEntity -> assertHearing(hearingEntity, isAnonymised)); - dartsDatabase.getEventRepository().findAllByCaseId(caseId).forEach(eventEntity -> assertEvent(eventEntity, isAnonymised)); + Set eventIdsToExclude = new HashSet<>(); + Arrays.stream(excludeEventIds).forEach(eventIdsToExclude::add); + List eventLinkedCaseEntities = new ArrayList<>(); + eventLinkedCaseEntities.addAll(dartsDatabase.getEventLinkedCaseRepository().findAllByCourtCase(courtCase)); + eventLinkedCaseEntities.addAll(dartsDatabase.getEventLinkedCaseRepository().findAllByCaseNumberAndCourthouseName( + courtCase.getCaseNumber(), + courtCase.getCourthouse().getCourthouseName())); + eventLinkedCaseEntities.stream().map(EventLinkedCaseEntity::getEvent) + .forEach(eventEntity -> assertEvent(eventEntity, eventIdsToExclude.contains(eventEntity.getId()) ? false : isAnonymised)); assertAuditEntries(courtCase, isAnonymised); }); diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java index 7989b436d01..40f73672158 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java @@ -14,6 +14,8 @@ import uk.gov.hmcts.darts.testutils.stubs.DartsDatabaseStub; import uk.gov.hmcts.darts.testutils.stubs.DartsPersistence; +import java.util.List; + /** * Base class for integration tests running against a containerized Postgres with Testcontainers. */ @@ -42,11 +44,18 @@ public class PostgresIntegrationBase { */ private static final int SERVER_MAX_CONNECTIONS = 50; - private static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer<>( - "postgres:16-alpine" - ).withDatabaseName("darts") - .withUsername("darts") - .withPassword("darts"); + private static final PostgreSQLContainer POSTGRES; + + static { + POSTGRES = new PostgreSQLContainer<>( + "postgres:16-alpine" + ).withDatabaseName("darts") + .withUsername("darts") + .withPassword("darts"); + POSTGRES.setPortBindings(List.of( + "5433:5432" + )); + } @DynamicPropertySource static void configureProperties(DynamicPropertyRegistry registry) { diff --git a/src/main/java/uk/gov/hmcts/darts/arm/service/impl/BatchCleanupArmResponseFilesServiceCommon.java b/src/main/java/uk/gov/hmcts/darts/arm/service/impl/BatchCleanupArmResponseFilesServiceCommon.java index e67ba65a41d..b279952d79d 100644 --- a/src/main/java/uk/gov/hmcts/darts/arm/service/impl/BatchCleanupArmResponseFilesServiceCommon.java +++ b/src/main/java/uk/gov/hmcts/darts/arm/service/impl/BatchCleanupArmResponseFilesServiceCommon.java @@ -52,7 +52,6 @@ public class BatchCleanupArmResponseFilesServiceCommon implements BatchCleanupAr @Override public void cleanupResponseFiles(int batchsize) { - System.out.println("TMP: " + manifestFilePrefix); if (batchsize == 0) { log.warn("{}: Batch Cleanup ARM Response Files - Batch size is 0, so not running", manifestFilePrefix); return; diff --git a/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java b/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java index a49940c43c2..de31e42e5d4 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java +++ b/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java @@ -1,8 +1,10 @@ package uk.gov.hmcts.darts.common.repository; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; import org.springframework.stereotype.Repository; import uk.gov.hmcts.darts.common.entity.CourtCaseEntity; +import uk.gov.hmcts.darts.common.entity.EventEntity; import uk.gov.hmcts.darts.common.entity.EventLinkedCaseEntity; import java.util.List; @@ -13,4 +15,14 @@ public interface EventLinkedCaseRepository extends JpaRepository findAllByCourtCase(CourtCaseEntity courtCase); List findAllByCaseNumberAndCourthouseName(String caseNumber, String courthouseName); + + @Query(""" + SELECT COUNT(DISTINCT cc) = (COUNT(cc.isDataAnonymised) filter (where cc.isDataAnonymised = true)) + FROM EventLinkedCaseEntity elc + LEFT JOIN CourtCaseEntity cc ON (elc.courtCase = cc or (cc.courthouse.courthouseName = elc.courthouseName and cc.caseNumber = elc.caseNumber)) + WHERE elc.event = :eventEntity + group by elc.event + """ + ) + boolean allAssociatedCasesAnonymised(EventEntity eventEntity); } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImpl.java b/src/main/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImpl.java index 309627cde8c..59c661c8357 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImpl.java +++ b/src/main/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImpl.java @@ -65,14 +65,13 @@ void anonymiseCourtCaseEntity(UserAccountEntity userAccount, CourtCaseEntity cou courtCase.getDefenceList().forEach(defenceEntity -> anonymiseDefenceEntity(userAccount, defenceEntity)); courtCase.getProsecutorList().forEach(prosecutorEntity -> anonymiseProsecutorEntity(userAccount, prosecutorEntity)); courtCase.getHearings().forEach(hearingEntity -> anonymiseHearingEntity(userAccount, hearingEntity)); - anonymiseAllEventsFromCase(userAccount, courtCase); - courtCase.markAsExpired(userAccount); //This also saves defendant, defence and prosecutor entities tidyUpTransformedMediaEntities(userAccount, courtCase); auditApi.record(AuditActivity.CASE_EXPIRED, userAccount, courtCase); anonymiseCreatedModifiedBaseEntity(userAccount, courtCase); caseService.saveCase(courtCase); + anonymiseAllEventsFromCase(userAccount, courtCase); //Required for Dynatrace dashboards logApi.caseDeletedDueToExpiry(courtCase.getId(), courtCase.getCaseNumber()); @@ -108,13 +107,21 @@ public void anonymiseEventByIds(UserAccountEntity userAccount, List eve @Override public void anonymiseEvent(UserAccountEntity userAccount, EventEntity eventEntity) { - anonymiseEventEntity(userAccount, eventEntity); + anonymiseEventEntity(userAccount, eventEntity, false); eventService.saveEvent(eventEntity); auditApi.record(AuditActivity.MANUAL_OBFUSCATION, userAccount, eventEntity.getId().toString()); logApi.manualObfuscation(eventEntity); } - void anonymiseEventEntity(UserAccountEntity userAccount, EventEntity eventEntity) { + void anonymiseEventEntity(UserAccountEntity userAccount, EventEntity eventEntity, boolean onlyAnonymiseIfAllCasesExpired) { + if (eventEntity.isDataAnonymised()) { + log.debug("Event {} already anonymised skipping", eventEntity.getId()); + return; + } + if (onlyAnonymiseIfAllCasesExpired && !eventService.allAssociatedCasesAnonymised(eventEntity)) { + log.debug("Event {} not anonymised as not all cases are expired", eventEntity.getId()); + return; + } eventEntity.setEventText(UUID.randomUUID().toString()); eventEntity.setDataAnonymised(true); anonymiseCreatedModifiedBaseEntity(userAccount, eventEntity); @@ -127,6 +134,10 @@ void anonymiseTranscriptionEntity(UserAccountEntity userAccount, TranscriptionEn } void anonymiseTranscriptionCommentEntity(UserAccountEntity userAccount, TranscriptionCommentEntity transcriptionCommentEntity) { + if (transcriptionCommentEntity.isDataAnonymised()) { + log.debug("Transcription comment {} already anonymised skipping", transcriptionCommentEntity.getId()); + return; + } transcriptionCommentEntity.setComment(UUID.randomUUID().toString()); transcriptionCommentEntity.setDataAnonymised(true); anonymiseCreatedModifiedBaseEntity(userAccount, transcriptionCommentEntity); @@ -134,7 +145,6 @@ void anonymiseTranscriptionCommentEntity(UserAccountEntity userAccount, Transcri void anonymiseTranscriptionWorkflowEntity(TranscriptionWorkflowEntity transcriptionWorkflowEntity) { transcriptionWorkflowEntity.close(); - } void tidyUpTransformedMediaEntities(UserAccountEntity userAccount, CourtCaseEntity courtCase) { @@ -190,6 +200,6 @@ private void anonymiseCreatedModifiedBaseEntity(UserAccountEntity userAccount, C } private void anonymiseAllEventsFromCase(UserAccountEntity userAccount, CourtCaseEntity courtCase) { - eventService.getAllCourtCaseEventVersions(courtCase).forEach(eventEntity -> anonymiseEventEntity(userAccount, eventEntity)); + eventService.getAllCourtCaseEventVersions(courtCase).forEach(eventEntity -> anonymiseEventEntity(userAccount, eventEntity, true)); } } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/darts/event/service/EventService.java b/src/main/java/uk/gov/hmcts/darts/event/service/EventService.java index 2d963cebcae..5d9138e58c7 100644 --- a/src/main/java/uk/gov/hmcts/darts/event/service/EventService.java +++ b/src/main/java/uk/gov/hmcts/darts/event/service/EventService.java @@ -14,4 +14,6 @@ public interface EventService { EventEntity saveEvent(EventEntity eventEntity); Set getAllCourtCaseEventVersions(CourtCaseEntity courtCase); + + boolean allAssociatedCasesAnonymised(EventEntity eventEntity); } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImpl.java b/src/main/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImpl.java index 663e8333284..d3ccb36e21d 100644 --- a/src/main/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImpl.java +++ b/src/main/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImpl.java @@ -64,4 +64,8 @@ public Set getAllCourtCaseEventVersions(CourtCaseEntity courtCase) return allEvents; } + @Override + public boolean allAssociatedCasesAnonymised(EventEntity eventEntity) { + return eventLinkedCaseRepository.allAssociatedCasesAnonymised(eventEntity); + } } \ No newline at end of file diff --git a/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java b/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java index cfd56040c1d..f3e52ebc8ac 100644 --- a/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java +++ b/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java @@ -87,16 +87,58 @@ private void assertLastModifiedByAndAt(CreatedModifiedBaseEntity entity, UserAcc @Test - void positiveEventEntityAnonymise() { + void positiveEventEntityAnonymiseNotUpdatedAsNotAllCasesExpiredAndOnlyAnonymiseIfAllCasesExpiredIsTrue() { + EventEntity eventEntity = new EventEntity(); + eventEntity.setEventText("event text"); + + UserAccountEntity userAccount = new UserAccountEntity(); + when(eventService.allAssociatedCasesAnonymised(eventEntity)).thenReturn(false); + dataAnonymisationService.anonymiseEventEntity(userAccount, eventEntity, true); + assertThat(eventEntity.getEventText()).isEqualTo("event text"); + assertThat(eventEntity.isDataAnonymised()).isFalse(); + verify(eventService).allAssociatedCasesAnonymised(eventEntity); + } + + @Test + void positiveEventEntityAnonymiseUpdatedAsAllCasesExpiredAndOnlyAnonymiseIfAllCasesExpiredIsTrue() { + setupOffsetDateTime(); + EventEntity eventEntity = new EventEntity(); + eventEntity.setEventText("event text"); + + UserAccountEntity userAccount = new UserAccountEntity(); + when(eventService.allAssociatedCasesAnonymised(eventEntity)).thenReturn(true); + dataAnonymisationService.anonymiseEventEntity(userAccount, eventEntity, true); + assertThat(eventEntity.getEventText()).matches(TestUtils.UUID_REGEX); + assertThat(eventEntity.isDataAnonymised()).isTrue(); + assertLastModifiedByAndAt(eventEntity, userAccount); + verify(eventService).allAssociatedCasesAnonymised(eventEntity); + } + + @Test + void positiveEventEntityAnonymiseUpdatedAsNotAllCasesExpiredAndOnlyAnonymiseIfAllCasesExpiredIsFalse() { setupOffsetDateTime(); EventEntity eventEntity = new EventEntity(); eventEntity.setEventText("event text"); UserAccountEntity userAccount = new UserAccountEntity(); - dataAnonymisationService.anonymiseEventEntity(userAccount, eventEntity); + dataAnonymisationService.anonymiseEventEntity(userAccount, eventEntity, false); assertThat(eventEntity.getEventText()).matches(TestUtils.UUID_REGEX); assertThat(eventEntity.isDataAnonymised()).isTrue(); assertLastModifiedByAndAt(eventEntity, userAccount); + verifyNoMoreInteractions(eventService); + } + + @Test + void positiveEventEntityNotUpdatedAsAlreadyAnonymised() { + EventEntity eventEntity = new EventEntity(); + eventEntity.setEventText("event text"); + eventEntity.setDataAnonymised(true); + + UserAccountEntity userAccount = new UserAccountEntity(); + dataAnonymisationService.anonymiseEventEntity(userAccount, eventEntity, false); + assertThat(eventEntity.getEventText()).isEqualTo("event text"); + assertThat(eventEntity.isDataAnonymised()).isTrue(); + verifyNoMoreInteractions(eventService); } @Test @@ -201,8 +243,8 @@ void assertPositiveAnonymiseCourtCaseEntity() { verify(dataAnonymisationService, times(1)).anonymiseHearingEntity(userAccount, hearingEntity1); verify(dataAnonymisationService, times(1)).anonymiseHearingEntity(userAccount, hearingEntity2); - verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, entityEntity1); - verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, entityEntity2); + verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, entityEntity1, true); + verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, entityEntity2, true); verify(dataAnonymisationService, times(1)).tidyUpTransformedMediaEntities(userAccount, courtCase); verify(caseService, times(1)).saveCase(courtCase); @@ -409,9 +451,9 @@ void positiveAnonymiseEventByIds() { dataAnonymisationService.anonymiseEventByIds(userAccount, List.of(1, 2, 3, 4)); - verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, event1); - verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, event2); - verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, event3); + verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, event1, false); + verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, event2, false); + verify(dataAnonymisationService, times(1)).anonymiseEventEntity(userAccount, event3, false); verify(eventService, times(1)).getEventByEveId(1); verify(eventService, times(1)).getEventByEveId(2); diff --git a/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java b/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java index 4bbf9172d88..ea5a9aa4c16 100644 --- a/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java +++ b/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java @@ -98,4 +98,19 @@ void positiveGetAllCourtCaseEventVersions() { } + @Test + void positiveAllAssociatedCasesAnonymisedTrue() { + EventEntity event = mock(EventEntity.class); + when(eventLinkedCaseRepository.allAssociatedCasesAnonymised(event)).thenReturn(true); + assertThat(eventService.allAssociatedCasesAnonymised(event)).isTrue(); + verify(eventLinkedCaseRepository).allAssociatedCasesAnonymised(event); + } + + @Test + void positiveAllAssociatedCasesAnonymisedFalse() { + EventEntity event = mock(EventEntity.class); + when(eventLinkedCaseRepository.allAssociatedCasesAnonymised(event)).thenReturn(false); + assertThat(eventService.allAssociatedCasesAnonymised(event)).isFalse(); + verify(eventLinkedCaseRepository).allAssociatedCasesAnonymised(event); + } } \ No newline at end of file From de4043f746c1687f6fed479ea1cf210150a83dc5 Mon Sep 17 00:00:00 2001 From: benedwards Date: Wed, 20 Nov 2024 17:36:53 +0000 Subject: [PATCH 2/5] Fixed styles --- .../runner/impl/CaseExpiryDeletionAutomatedTaskITest.java | 2 +- .../darts/common/repository/EventLinkedCaseRepository.java | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java b/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java index e17cf7444af..110c3631dde 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java @@ -183,7 +183,7 @@ private void assertCase(int caseId, boolean isAnonymised, int... excludeEventIds courtCase.getCaseNumber(), courtCase.getCourthouse().getCourthouseName())); eventLinkedCaseEntities.stream().map(EventLinkedCaseEntity::getEvent) - .forEach(eventEntity -> assertEvent(eventEntity, eventIdsToExclude.contains(eventEntity.getId()) ? false : isAnonymised)); + .forEach(eventEntity -> assertEvent(eventEntity, !eventIdsToExclude.contains(eventEntity.getId()) && isAnonymised)); assertAuditEntries(courtCase, isAnonymised); }); diff --git a/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java b/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java index de31e42e5d4..4e5a1b8b4fb 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java +++ b/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java @@ -19,7 +19,9 @@ public interface EventLinkedCaseRepository extends JpaRepository Date: Thu, 21 Nov 2024 11:57:58 +0000 Subject: [PATCH 3/5] Applied review comments --- .../runner/impl/CaseExpiryDeletionAutomatedTaskITest.java | 2 ++ .../common/repository/EventLinkedCaseRepository.java | 2 +- .../common/service/impl/DataAnonymisationServiceImpl.java | 2 +- .../hmcts/darts/event/service/impl/EventServiceImpl.java | 2 +- .../service/impl/DataAnonymisationServiceImplTest.java | 5 ++++- .../darts/event/service/impl/EventServiceImplTest.java | 8 ++++---- 6 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java b/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java index 110c3631dde..1f719ce8db1 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java @@ -72,6 +72,7 @@ void positiveRetentionDatePassed() { } @Test + @DisplayName("Two cases linked to the same event, one case has passed retention date, the other has not. Event should not be anonymised") void positiveRetentionDatePassedForOneCaseButNotAnotherEventNotAnoymised() { CourtCaseEntity courtCase1 = createCase(-1, CaseRetentionStatus.COMPLETE); CourtCaseEntity courtCase2 = createCase(-1, CaseRetentionStatus.PENDING); @@ -87,6 +88,7 @@ void positiveRetentionDatePassedForOneCaseButNotAnotherEventNotAnoymised() { } @Test + @DisplayName("Two cases linked to the same event, both cases have passed retention date. Event should be anonymised") void positiveRetentionDatePassedForBothCaseLinkedEventsAnoymised() { CourtCaseEntity courtCase1 = createCase(-1, CaseRetentionStatus.COMPLETE); CourtCaseEntity courtCase2 = createCase(-1, CaseRetentionStatus.COMPLETE); diff --git a/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java b/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java index 4e5a1b8b4fb..9e3f72a870b 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java +++ b/src/main/java/uk/gov/hmcts/darts/common/repository/EventLinkedCaseRepository.java @@ -26,5 +26,5 @@ SELECT COUNT(DISTINCT cc) = (COUNT(cc.isDataAnonymised) filter (where cc.isDataA group by elc.event """ ) - boolean allAssociatedCasesAnonymised(EventEntity eventEntity); + boolean areAllAssociatedCasesAnonymised(EventEntity eventEntity); } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImpl.java b/src/main/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImpl.java index 59c661c8357..5805b575a57 100644 --- a/src/main/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImpl.java +++ b/src/main/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImpl.java @@ -108,7 +108,6 @@ public void anonymiseEventByIds(UserAccountEntity userAccount, List eve @Override public void anonymiseEvent(UserAccountEntity userAccount, EventEntity eventEntity) { anonymiseEventEntity(userAccount, eventEntity, false); - eventService.saveEvent(eventEntity); auditApi.record(AuditActivity.MANUAL_OBFUSCATION, userAccount, eventEntity.getId().toString()); logApi.manualObfuscation(eventEntity); } @@ -125,6 +124,7 @@ void anonymiseEventEntity(UserAccountEntity userAccount, EventEntity eventEntity eventEntity.setEventText(UUID.randomUUID().toString()); eventEntity.setDataAnonymised(true); anonymiseCreatedModifiedBaseEntity(userAccount, eventEntity); + eventService.saveEvent(eventEntity); } void anonymiseTranscriptionEntity(UserAccountEntity userAccount, TranscriptionEntity transcriptionEntity) { diff --git a/src/main/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImpl.java b/src/main/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImpl.java index d3ccb36e21d..17813d354fa 100644 --- a/src/main/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImpl.java +++ b/src/main/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImpl.java @@ -66,6 +66,6 @@ public Set getAllCourtCaseEventVersions(CourtCaseEntity courtCase) @Override public boolean allAssociatedCasesAnonymised(EventEntity eventEntity) { - return eventLinkedCaseRepository.allAssociatedCasesAnonymised(eventEntity); + return eventLinkedCaseRepository.areAllAssociatedCasesAnonymised(eventEntity); } } \ No newline at end of file diff --git a/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java b/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java index f3e52ebc8ac..fcd020d01a6 100644 --- a/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java +++ b/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java @@ -97,6 +97,7 @@ void positiveEventEntityAnonymiseNotUpdatedAsNotAllCasesExpiredAndOnlyAnonymiseI assertThat(eventEntity.getEventText()).isEqualTo("event text"); assertThat(eventEntity.isDataAnonymised()).isFalse(); verify(eventService).allAssociatedCasesAnonymised(eventEntity); + verify(eventService, never()).saveEvent(eventEntity); } @Test @@ -112,6 +113,7 @@ void positiveEventEntityAnonymiseUpdatedAsAllCasesExpiredAndOnlyAnonymiseIfAllCa assertThat(eventEntity.isDataAnonymised()).isTrue(); assertLastModifiedByAndAt(eventEntity, userAccount); verify(eventService).allAssociatedCasesAnonymised(eventEntity); + verify(eventService).saveEvent(eventEntity); } @Test @@ -125,7 +127,7 @@ void positiveEventEntityAnonymiseUpdatedAsNotAllCasesExpiredAndOnlyAnonymiseIfAl assertThat(eventEntity.getEventText()).matches(TestUtils.UUID_REGEX); assertThat(eventEntity.isDataAnonymised()).isTrue(); assertLastModifiedByAndAt(eventEntity, userAccount); - verifyNoMoreInteractions(eventService); + verify(eventService).saveEvent(eventEntity); } @Test @@ -139,6 +141,7 @@ void positiveEventEntityNotUpdatedAsAlreadyAnonymised() { assertThat(eventEntity.getEventText()).isEqualTo("event text"); assertThat(eventEntity.isDataAnonymised()).isTrue(); verifyNoMoreInteractions(eventService); + verify(eventService, never()).saveEvent(eventEntity); } @Test diff --git a/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java b/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java index ea5a9aa4c16..a6b4fc36c09 100644 --- a/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java +++ b/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java @@ -101,16 +101,16 @@ void positiveGetAllCourtCaseEventVersions() { @Test void positiveAllAssociatedCasesAnonymisedTrue() { EventEntity event = mock(EventEntity.class); - when(eventLinkedCaseRepository.allAssociatedCasesAnonymised(event)).thenReturn(true); + when(eventLinkedCaseRepository.areAllAssociatedCasesAnonymised(event)).thenReturn(true); assertThat(eventService.allAssociatedCasesAnonymised(event)).isTrue(); - verify(eventLinkedCaseRepository).allAssociatedCasesAnonymised(event); + verify(eventLinkedCaseRepository).areAllAssociatedCasesAnonymised(event); } @Test void positiveAllAssociatedCasesAnonymisedFalse() { EventEntity event = mock(EventEntity.class); - when(eventLinkedCaseRepository.allAssociatedCasesAnonymised(event)).thenReturn(false); + when(eventLinkedCaseRepository.areAllAssociatedCasesAnonymised(event)).thenReturn(false); assertThat(eventService.allAssociatedCasesAnonymised(event)).isFalse(); - verify(eventLinkedCaseRepository).allAssociatedCasesAnonymised(event); + verify(eventLinkedCaseRepository).areAllAssociatedCasesAnonymised(event); } } \ No newline at end of file From 4aa27771e4089ea5aa8f7d2c42db07c9f0a17a77 Mon Sep 17 00:00:00 2001 From: benedwards Date: Mon, 25 Nov 2024 10:18:52 +0000 Subject: [PATCH 4/5] Updated test names --- .../impl/CaseExpiryDeletionAutomatedTaskITest.java | 4 ++-- .../service/impl/DataAnonymisationServiceImplTest.java | 10 +++++++--- .../darts/event/service/impl/EventServiceImplTest.java | 4 +++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java b/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java index 1f719ce8db1..31c240794f8 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/task/runner/impl/CaseExpiryDeletionAutomatedTaskITest.java @@ -73,7 +73,7 @@ void positiveRetentionDatePassed() { @Test @DisplayName("Two cases linked to the same event, one case has passed retention date, the other has not. Event should not be anonymised") - void positiveRetentionDatePassedForOneCaseButNotAnotherEventNotAnoymised() { + void retentionDatePassedForOneCaseButNotAnotherEventNotAnoymised() { CourtCaseEntity courtCase1 = createCase(-1, CaseRetentionStatus.COMPLETE); CourtCaseEntity courtCase2 = createCase(-1, CaseRetentionStatus.PENDING); @@ -89,7 +89,7 @@ void positiveRetentionDatePassedForOneCaseButNotAnotherEventNotAnoymised() { @Test @DisplayName("Two cases linked to the same event, both cases have passed retention date. Event should be anonymised") - void positiveRetentionDatePassedForBothCaseLinkedEventsAnoymised() { + void retentionDatePassedForBothCaseLinkedEventsAnoymised() { CourtCaseEntity courtCase1 = createCase(-1, CaseRetentionStatus.COMPLETE); CourtCaseEntity courtCase2 = createCase(-1, CaseRetentionStatus.COMPLETE); diff --git a/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java b/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java index fcd020d01a6..76c7e5a8dc8 100644 --- a/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java +++ b/src/test/java/uk/gov/hmcts/darts/common/service/impl/DataAnonymisationServiceImplTest.java @@ -1,5 +1,6 @@ package uk.gov.hmcts.darts.common.service.impl; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; @@ -87,7 +88,8 @@ private void assertLastModifiedByAndAt(CreatedModifiedBaseEntity entity, UserAcc @Test - void positiveEventEntityAnonymiseNotUpdatedAsNotAllCasesExpiredAndOnlyAnonymiseIfAllCasesExpiredIsTrue() { + @DisplayName("Event should not be anonymised if one or more assocaited cases are not anonymised") + void eventEntityAnonymiseNotUpdatedAsNotAllCasesExpiredAndOnlyAnonymiseIfAllCasesExpiredIsTrue() { EventEntity eventEntity = new EventEntity(); eventEntity.setEventText("event text"); @@ -117,7 +119,8 @@ void positiveEventEntityAnonymiseUpdatedAsAllCasesExpiredAndOnlyAnonymiseIfAllCa } @Test - void positiveEventEntityAnonymiseUpdatedAsNotAllCasesExpiredAndOnlyAnonymiseIfAllCasesExpiredIsFalse() { + @DisplayName("Event should be anonymised if one or more assocaited cases are not anonymised and the onlyAnonymiseIfAllCasesExpired flag is false") + void eventEntityAnonymiseUpdatedAsNotAllCasesExpiredAndOnlyAnonymiseIfAllCasesExpiredIsFalse() { setupOffsetDateTime(); EventEntity eventEntity = new EventEntity(); eventEntity.setEventText("event text"); @@ -131,7 +134,8 @@ void positiveEventEntityAnonymiseUpdatedAsNotAllCasesExpiredAndOnlyAnonymiseIfAl } @Test - void positiveEventEntityNotUpdatedAsAlreadyAnonymised() { + @DisplayName("Event should not be anonymised again if the event is already anonymised") + void eventEntityNotUpdatedAsAlreadyAnonymised() { EventEntity eventEntity = new EventEntity(); eventEntity.setEventText("event text"); eventEntity.setDataAnonymised(true); diff --git a/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java b/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java index a6b4fc36c09..d990103208f 100644 --- a/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java +++ b/src/test/java/uk/gov/hmcts/darts/event/service/impl/EventServiceImplTest.java @@ -1,5 +1,6 @@ package uk.gov.hmcts.darts.event.service.impl; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; @@ -99,7 +100,8 @@ void positiveGetAllCourtCaseEventVersions() { @Test - void positiveAllAssociatedCasesAnonymisedTrue() { + @DisplayName("areAllAssociatedCasesAnonymised(...) method, should return true if all associated cases are anonymised") + void allAssociatedCasesAnonymisedTrue() { EventEntity event = mock(EventEntity.class); when(eventLinkedCaseRepository.areAllAssociatedCasesAnonymised(event)).thenReturn(true); assertThat(eventService.allAssociatedCasesAnonymised(event)).isTrue(); From f37fa4b24244b1ea65b086253ffa825f7cac921b Mon Sep 17 00:00:00 2001 From: Ben Edwards <147524406+Ben-Edwards-cgi@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:21:59 +0000 Subject: [PATCH 5/5] Update PostgresIntegrationBase.java --- .../gov/hmcts/darts/testutils/PostgresIntegrationBase.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java index 40f73672158..f536160aa53 100644 --- a/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java +++ b/src/integrationTest/java/uk/gov/hmcts/darts/testutils/PostgresIntegrationBase.java @@ -14,8 +14,6 @@ import uk.gov.hmcts.darts.testutils.stubs.DartsDatabaseStub; import uk.gov.hmcts.darts.testutils.stubs.DartsPersistence; -import java.util.List; - /** * Base class for integration tests running against a containerized Postgres with Testcontainers. */ @@ -52,9 +50,6 @@ public class PostgresIntegrationBase { ).withDatabaseName("darts") .withUsername("darts") .withPassword("darts"); - POSTGRES.setPortBindings(List.of( - "5433:5432" - )); } @DynamicPropertySource @@ -82,4 +77,4 @@ void clearDb() { void clearTestData() { logAppender.reset(); } -} \ No newline at end of file +}