From e97f237c3360a58c6c1100d745ef3b095f3d1eaf Mon Sep 17 00:00:00 2001 From: Enrico Date: Mon, 29 Jun 2026 14:31:11 +0200 Subject: [PATCH] fix: Block updates on expired events [DHIS2-21536] --- .../validator/event/DateValidator.java | 44 ++- .../validator/event/DateValidatorTest.java | 315 +++++++++++------- 2 files changed, 228 insertions(+), 131 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/DateValidator.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/DateValidator.java index 2bb9e30ef8f2..e19bdb81cb9f 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/DateValidator.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/DateValidator.java @@ -73,8 +73,8 @@ public void validate(Reporter reporter, TrackerBundle bundle, Event event) { } validateCompletedDateIsSetOnlyForSupportedStatus(reporter, event); - validateCompletionExpiryDays(reporter, event, program, bundle.getUser()); validateExpiryPeriodType(reporter, event, program, bundle.getUser()); + validateCompletionExpiryDays(reporter, preheat, event, program, bundle.getUser()); } private void validateCompletedDateIsSetOnlyForSupportedStatus(Reporter reporter, Event event) { @@ -84,19 +84,49 @@ private void validateCompletedDateIsSetOnlyForSupportedStatus(Reporter reporter, } private void validateCompletionExpiryDays( - Reporter reporter, Event event, Program program, UserDetails user) { - if (event.getCompletedAt() == null || user.isAuthorized(Authorities.F_EDIT_EXPIRED.name())) { + Reporter reporter, TrackerPreheat preheat, Event event, Program program, UserDetails user) { + if (program.getCompleteEventsExpiryDays() == 0 + || user.isAuthorized(Authorities.F_EDIT_EXPIRED.name())) { return; } - if (program.getCompleteEventsExpiryDays() > 0 - && EventStatus.COMPLETED == event.getStatus() - && now() - .isAfter(event.getCompletedAt().plus(ofDays(program.getCompleteEventsExpiryDays())))) { + Instant completedAt = getCompletedDate(preheat, event); + + if (completedAt != null + && now().isAfter(completedAt.plus(ofDays(program.getCompleteEventsExpiryDays())))) { reporter.addError(event, E1043, event); } } + /** + * Returns the completion date the expiry check should be anchored to, or {@code null} if the + * event is not completed. + * + *

When the event is already completed in the database, the persisted completion date is used. + * This makes the expiry check work even when the update payload does not include {@code + * completedAt}, and prevents the payload from resetting the expiry clock on an already completed + * event. Otherwise, when the payload itself completes the event, its completion date is used. + */ + private Instant getCompletedDate(TrackerPreheat preheat, Event event) { + Date persistedCompletedDate = getPersistedCompletedDate(preheat, event); + if (persistedCompletedDate != null) { + return persistedCompletedDate.toInstant(); + } + + if (EventStatus.COMPLETED == event.getStatus()) { + return event.getCompletedAt(); + } + + return null; + } + + private Date getPersistedCompletedDate(TrackerPreheat preheat, Event event) { + org.hisp.dhis.program.Event persisted = preheat.getEvent(event.getEvent()); + return persisted != null && EventStatus.COMPLETED == persisted.getStatus() + ? persisted.getCompletedDate() + : null; + } + private void validateExpiryPeriodType( Reporter reporter, Event event, Program program, UserDetails user) { PeriodType periodType = program.getExpiryPeriodType(); diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/DateValidatorTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/DateValidatorTest.java index 67fccb86cfc3..9d60ba64fb08 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/DateValidatorTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/DateValidatorTest.java @@ -43,11 +43,11 @@ import java.time.Instant; import java.time.LocalDateTime; import java.time.ZoneOffset; +import java.util.Date; import org.hisp.dhis.common.UID; import org.hisp.dhis.event.EventStatus; import org.hisp.dhis.period.DailyPeriodType; import org.hisp.dhis.program.Program; -import org.hisp.dhis.program.ProgramType; import org.hisp.dhis.security.Authorities; import org.hisp.dhis.test.TestBase; import org.hisp.dhis.tracker.TrackerIdSchemeParams; @@ -60,6 +60,9 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.EnumSource.Mode; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -69,9 +72,7 @@ @ExtendWith(MockitoExtension.class) class DateValidatorTest extends TestBase { - private static final String PROGRAM_WITH_REGISTRATION_ID = "ProgramWithRegistration"; - - private static final String PROGRAM_WITHOUT_REGISTRATION_ID = "ProgramWithoutRegistration"; + private static final String PROGRAM_ID = "ProgramId"; private DateValidator validator; @@ -93,26 +94,34 @@ public void setUp() { } @Test - void testEventIsValid() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITHOUT_REGISTRATION_ID))) - .thenReturn(getProgramWithoutRegistration()); - Event event = new Event(); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITHOUT_REGISTRATION_ID)); - event.setOccurredAt(now()); - event.setStatus(EventStatus.ACTIVE); + void shouldPassWhenEventIsValid() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(0)); + Event event = + Event.builder() + .event(UID.generate()) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(now()) + .status(EventStatus.ACTIVE) + .build(); validator.validate(reporter, bundle, event); assertIsEmpty(reporter.getErrors()); } - @Test - void testEventIsNotValidWhenOccurredDateIsNotPresentAndProgramIsWithoutRegistration() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITHOUT_REGISTRATION_ID))) - .thenReturn(getProgramWithoutRegistration()); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITHOUT_REGISTRATION_ID)); + @ParameterizedTest + @EnumSource( + value = EventStatus.class, + mode = Mode.INCLUDE, + names = {"ACTIVE", "COMPLETED"}) + void shouldFailWhenMandatoryOccurredAtIsNotPresent(EventStatus eventStatus) { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(0)); + Event event = + Event.builder() + .event(UID.generate()) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .status(eventStatus) + .build(); validator.validate(reporter, bundle, event); @@ -120,74 +129,84 @@ void testEventIsNotValidWhenOccurredDateIsNotPresentAndProgramIsWithoutRegistrat } @Test - void testEventIsNotValidWhenOccurredDateIsNotPresentAndEventIsActive() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(5)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setStatus(EventStatus.ACTIVE); + void shouldFailWhenScheduledDateIsNotPresentAndEventIsSchedule() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + Event event = + Event.builder() + .event(UID.generate()) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(now()) + .status(EventStatus.SCHEDULE) + .build(); validator.validate(reporter, bundle, event); - assertHasError(reporter, event, E1031); + assertHasError(reporter, event, E1050); } @Test - void testEventIsNotValidWhenOccurredDateIsNotPresentAndEventIsCompleted() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(5)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setStatus(EventStatus.COMPLETED); + void shouldFailWhenCompletedAtIsPresentAndStatusIsNotCompleted() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + Event event = + Event.builder() + .event(UID.generate()) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(now()) + .completedAt(now()) + .status(EventStatus.ACTIVE) + .build(); validator.validate(reporter, bundle, event); - assertHasError(reporter, event, E1031); + assertHasError(reporter, event, E1051); } @Test - void testEventIsNotValidWhenScheduledDateIsNotPresentAndEventIsSchedule() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(5)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setOccurredAt(Instant.now()); - event.setStatus(EventStatus.SCHEDULE); + void shouldFailWhenCompletedAtIsTooSoonAndEventIsCompleted() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + Event event = + Event.builder() + .event(UID.generate()) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(now()) + .completedAt(sevenDaysAgo()) + .status(EventStatus.COMPLETED) + .build(); validator.validate(reporter, bundle, event); - assertHasError(reporter, event, E1050); + assertHasError(reporter, event, E1043); } @Test - void shouldFailWhenCompletedAtIsPresentAndStatusIsNotCompleted() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(5)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setOccurredAt(now()); - event.setCompletedAt(now()); - event.setStatus(EventStatus.ACTIVE); + void shouldFailWhenCompletionHasExpiredEvenIfPayloadHasNoCompletedAt() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + UID uid = UID.generate(); + when(preheat.getEvent(uid)).thenReturn(completedEvent(sevenDaysAgo())); + Event event = + Event.builder() + .event(uid) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(now()) + .status(EventStatus.COMPLETED) + .build(); validator.validate(reporter, bundle, event); - assertHasError(reporter, event, E1051); + assertHasError(reporter, event, E1043); } @Test - void testEventIsNotValidWhenCompletedAtIsTooSoonAndEventIsCompleted() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(5)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setOccurredAt(now()); - event.setCompletedAt(sevenDaysAgo()); - event.setStatus(EventStatus.COMPLETED); + void shouldFailWhenCompletionHasExpiredEvenIfPayloadHasNoStatusNorCompletedAt() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + UID uid = UID.generate(); + when(preheat.getEvent(uid)).thenReturn(completedEvent(sevenDaysAgo())); + Event event = + Event.builder() + .event(uid) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(now()) + .build(); validator.validate(reporter, bundle, event); @@ -195,45 +214,58 @@ void testEventIsNotValidWhenCompletedAtIsTooSoonAndEventIsCompleted() { } @Test - void testEventIsNotValidWhenOccurredAtAndScheduledAtAreNotPresent() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(5)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setOccurredAt(null); - event.setScheduledAt(null); - event.setStatus(EventStatus.SKIPPED); + void shouldUsePersistedCompletionDateAndIgnorePayloadCompletedAtWhenAlreadyCompleted() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + UID uid = UID.generate(); + // event was completed in the database long ago (expired)... + when(preheat.getEvent(uid)).thenReturn(completedEvent(sevenDaysAgo())); + Event event = + Event.builder() + .event(uid) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(now()) + // ...so a fresh completedAt in the payload must not reset the expiry clock + .completedAt(now()) + .status(EventStatus.COMPLETED) + .build(); validator.validate(reporter, bundle, event); - assertHasError(reporter, event, E1046); + assertHasError(reporter, event, E1043); } @Test - void shouldFailValidationForEventWhenDateBelongsToExpiredPeriod() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(5)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setOccurredAt(sevenDaysAgo()); - event.setStatus(EventStatus.ACTIVE); + void shouldPassWhenPersistedCompletionIsWithinExpiryDays() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + UID uid = UID.generate(); + when(preheat.getEvent(uid)).thenReturn(completedEvent(twoDaysAgo())); + Event event = + Event.builder() + .event(uid) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(now()) + .status(EventStatus.COMPLETED) + .build(); validator.validate(reporter, bundle, event); - assertHasError(reporter, event, E1047); + assertIsEmpty(reporter.getErrors()); } @Test - void shouldPassValidationForEventWhenDateBelongsToPastPeriodWithZeroExpiryDays() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(0)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setOccurredAt(sevenDaysAgo()); - event.setStatus(EventStatus.ACTIVE); + void shouldPassWhenPersistedCompletionHasExpiredButUserIsAuthorized() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + UID uid = UID.generate(); + UserDetails user = mock(UserDetails.class); + when(user.isAuthorized(Authorities.F_EDIT_EXPIRED.name())).thenReturn(true); + bundle.setUser(user); + Event event = + Event.builder() + .event(uid) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(now()) + .status(EventStatus.COMPLETED) + .build(); validator.validate(reporter, bundle, event); @@ -241,48 +273,71 @@ void shouldPassValidationForEventWhenDateBelongsToPastPeriodWithZeroExpiryDays() } @Test - void shouldPassValidationForEventWhenDateBelongsPastEventPeriodButWithinExpiryDays() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(7)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setOccurredAt(sevenDaysAgo()); - event.setStatus(EventStatus.ACTIVE); + void shouldFailWhenOccurredAtAndScheduledAtAreNotPresent() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + Event event = + Event.builder() + .event(UID.generate()) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(null) + .scheduledAt(null) + .status(EventStatus.SKIPPED) + .build(); validator.validate(reporter, bundle, event); - assertIsEmpty(reporter.getErrors()); + assertHasError(reporter, event, E1046); } @Test - void shouldPassValidationForEventWhenScheduledDateBelongsToFuturePeriod() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(5)); - Event event = new Event(); - event.setEvent(UID.generate()); - event.setProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)); - event.setScheduledAt(sevenDaysLater()); - event.setStatus(EventStatus.SCHEDULE); + void shouldFailWhenEventDateBelongsToExpiredPeriod() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); + Event event = expiredEvent(); validator.validate(reporter, bundle, event); - assertIsEmpty(reporter.getErrors()); + assertHasError(reporter, event, E1047); } @Test - void shouldPassValidationForEventWhenDateBelongsToExpiredPeriodIfUserIsAuthorized() { - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID))) - .thenReturn(getProgramWithRegistration(5)); + void shouldPassWhenEventDateBelongsToExpiredPeriodIfUserIsAuthorized() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); UserDetails user = mock(UserDetails.class); when(user.isAuthorized(Authorities.F_EDIT_EXPIRED.name())).thenReturn(true); bundle.setUser(user); + + validator.validate(reporter, bundle, expiredEvent()); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldPassWhenEventDateBelongsToPastPeriodWithZeroExpiryDays() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(0)); + + validator.validate(reporter, bundle, expiredEvent()); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldPassWhenEventDateBelongsPastEventPeriodButWithinExpiryDays() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(14)); + + validator.validate(reporter, bundle, expiredEvent()); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldPassWhenScheduledDateBelongsToFuturePeriod() { + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(getProgram(5)); Event event = Event.builder() .event(UID.generate()) - .program(MetadataIdentifier.ofUid(PROGRAM_WITH_REGISTRATION_ID)) - .occurredAt(sevenDaysAgo()) - .status(EventStatus.ACTIVE) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .scheduledAt(sevenDaysLater()) + .status(EventStatus.SCHEDULE) .build(); validator.validate(reporter, bundle, event); @@ -290,31 +345,43 @@ void shouldPassValidationForEventWhenDateBelongsToExpiredPeriodIfUserIsAuthorize assertIsEmpty(reporter.getErrors()); } - private Program getProgramWithRegistration(int expiryDays) { + private Event expiredEvent() { + return Event.builder() + .event(UID.generate()) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .occurredAt(sevenDaysAgo()) + .status(EventStatus.ACTIVE) + .build(); + } + + private static org.hisp.dhis.program.Event completedEvent(Instant completedDate) { + org.hisp.dhis.program.Event event = new org.hisp.dhis.program.Event(); + event.setStatus(EventStatus.COMPLETED); + event.setCompletedDate(Date.from(completedDate)); + return event; + } + + private Program getProgram(int expiryDays) { Program program = createProgram('A'); - program.setUid(PROGRAM_WITH_REGISTRATION_ID); - program.setProgramType(ProgramType.WITH_REGISTRATION); + program.setUid(PROGRAM_ID); program.setCompleteEventsExpiryDays(5); program.setExpiryDays(expiryDays); program.setExpiryPeriodType(new DailyPeriodType()); return program; } - private Program getProgramWithoutRegistration() { - Program program = createProgram('A'); - program.setUid(PROGRAM_WITHOUT_REGISTRATION_ID); - program.setProgramType(ProgramType.WITHOUT_REGISTRATION); - return program; - } - private Instant now() { return Instant.now(); } - private Instant sevenDaysAgo() { + private static Instant sevenDaysAgo() { return LocalDateTime.now().minusDays(7).toInstant(ZoneOffset.UTC); } + private static Instant twoDaysAgo() { + return LocalDateTime.now().minusDays(2).toInstant(ZoneOffset.UTC); + } + private Instant sevenDaysLater() { return LocalDateTime.now().plusDays(7).toInstant(ZoneOffset.UTC); }