Skip to content

Commit b4606a3

Browse files
enricocolasanteenricocolasante
committed
fix: Add event validation when updating status [DHIS2-17658] (#17890)
* fix: Add event validation when updating status [DHIS2-17658] * Fix sonar issues * Fix code review comments * Fix code review comments * Fix code review comments * Fix code review comments
1 parent 2cf9ca0 commit b4606a3

File tree

5 files changed

+230
-12
lines changed

5 files changed

+230
-12
lines changed

‎dhis-2/dhis-api/src/main/java/org/hisp/dhis/event/EventStatus.java‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
package org.hisp.dhis.event;
2929

3030
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement;
31+
import java.util.Set;
3132
import org.hisp.dhis.common.DxfNamespaces;
3233

3334
/**
@@ -52,17 +53,9 @@ public int getValue() {
5253
return value;
5354
}
5455

55-
public static EventStatus fromInt(int status) {
56-
for (EventStatus eventStatus : EventStatus.values()) {
57-
if (eventStatus.getValue() == status) {
58-
return eventStatus;
59-
}
60-
}
56+
public static final Set<EventStatus> STATUSES_WITH_DATA_VALUES =
57+
Set.of(ACTIVE, VISITED, COMPLETED);
6158

62-
throw new IllegalArgumentException();
63-
}
64-
65-
public static boolean isExistingEvent(EventStatus status) {
66-
return status != null && (COMPLETED.equals(status) || VISITED.equals(status));
67-
}
59+
public static final Set<EventStatus> STATUSES_WITHOUT_DATA_VALUES =
60+
Set.of(SCHEDULE, SKIPPED, OVERDUE);
6861
}

‎dhis-2/dhis-api/src/main/java/org/hisp/dhis/tracker/imports/validation/ValidationCode.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ public enum ValidationCode {
157157
E1312("Referral events need to have both sides of a relationship."),
158158
E1313(
159159
"Event {0} of an enrollment does not point to an existing tracked entity. The data in your system might be corrupted"),
160+
E1316("No event can transition from status `{0}` to status `{1}.`"),
160161

161162
/* Relationship */
162163
E4000("Relationship: `{0}` cannot link to itself"),

‎dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/EventValidator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ public EventValidator(
6565
new GeoValidator(),
6666
new NoteValidator(),
6767
new DataValuesValidator(),
68+
new StatusUpdateValidator(),
6869
new AssignedUserValidator()))),
6970
field(TrackerBundle::getEvents, new RepeatedEventsValidator()));
7071
}

‎dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/StatusUpdateValidator.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*
2+
* Copyright (c) 2004-2022, University of Oslo
3+
* All rights reserved.
4+
*
5+
* Redistribution and use in source and binary forms, with or without
6+
* modification, are permitted provided that the following conditions are met:
7+
* Redistributions of source code must retain the above copyright notice, this
8+
* list of conditions and the following disclaimer.
9+
*
10+
* Redistributions in binary form must reproduce the above copyright notice,
11+
* this list of conditions and the following disclaimer in the documentation
12+
* and/or other materials provided with the distribution.
13+
* Neither the name of the HISP project nor the names of its contributors may
14+
* be used to endorse or promote products derived from this software without
15+
* specific prior written permission.
16+
*
17+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
18+
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
19+
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
20+
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
21+
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
22+
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
23+
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
24+
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
25+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
26+
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
27+
*/
28+
package org.hisp.dhis.tracker.imports.validation.validator.event;
29+
30+
import static org.hisp.dhis.tracker.imports.TrackerImportStrategy.UPDATE;
31+
import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1316;
32+
33+
import org.hisp.dhis.event.EventStatus;
34+
import org.hisp.dhis.tracker.imports.TrackerImportStrategy;
35+
import org.hisp.dhis.tracker.imports.bundle.TrackerBundle;
36+
import org.hisp.dhis.tracker.imports.domain.Event;
37+
import org.hisp.dhis.tracker.imports.validation.Reporter;
38+
import org.hisp.dhis.tracker.imports.validation.Validator;
39+
40+
class StatusUpdateValidator implements Validator<Event> {
41+
@Override
42+
public void validate(Reporter reporter, TrackerBundle bundle, Event event) {
43+
org.hisp.dhis.program.Event savedEvent = bundle.getPreheat().getEvent(event.getUid());
44+
45+
if (checkInvalidStatusTransition(savedEvent.getStatus(), event.getStatus())) {
46+
reporter.addError(event, E1316, savedEvent.getStatus(), event.getStatus());
47+
}
48+
}
49+
50+
private boolean checkInvalidStatusTransition(EventStatus fromStatus, EventStatus toStatus) {
51+
return switch (fromStatus) {
52+
// An event cannot transition from a STATUSES_WITH_DATA_VALUES to a
53+
// STATUSES_WITHOUT_DATA_VALUES
54+
case VISITED, ACTIVE, COMPLETED ->
55+
EventStatus.STATUSES_WITHOUT_DATA_VALUES.contains(toStatus);
56+
// An event can transition from a STATUSES_WITHOUT_DATA_VALUES to any status
57+
// TODO: Is OVERDUE a read-only status?
58+
case OVERDUE, SKIPPED, SCHEDULE -> false;
59+
};
60+
}
61+
62+
@Override
63+
public boolean needsToRun(TrackerImportStrategy strategy) {
64+
return strategy == UPDATE;
65+
}
66+
}

‎dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/StatusUpdateValidatorTest.java‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/*
2+
* Copyright (c) 2004-2022, University of Oslo
3+
* All rights reserved.
4+
*
5+
* Redistribution and use in source and binary forms, with or without
6+
* modification, are permitted provided that the following conditions are met:
7+
* Redistributions of source code must retain the above copyright notice, this
8+
* list of conditions and the following disclaimer.
9+
*
10+
* Redistributions in binary form must reproduce the above copyright notice,
11+
* this list of conditions and the following disclaimer in the documentation
12+
* and/or other materials provided with the distribution.
13+
* Neither the name of the HISP project nor the names of its contributors may
14+
* be used to endorse or promote products derived from this software without
15+
* specific prior written permission.
16+
*
17+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
18+
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
19+
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
20+
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
21+
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
22+
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
23+
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
24+
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
25+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
26+
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
27+
*/
28+
package org.hisp.dhis.tracker.imports.validation.validator.event;
29+
30+
import static org.hisp.dhis.event.EventStatus.ACTIVE;
31+
import static org.hisp.dhis.event.EventStatus.COMPLETED;
32+
import static org.hisp.dhis.event.EventStatus.OVERDUE;
33+
import static org.hisp.dhis.event.EventStatus.SCHEDULE;
34+
import static org.hisp.dhis.event.EventStatus.SKIPPED;
35+
import static org.hisp.dhis.event.EventStatus.VISITED;
36+
import static org.hisp.dhis.tracker.imports.validation.validator.AssertValidations.assertHasError;
37+
import static org.hisp.dhis.utils.Assertions.assertIsEmpty;
38+
import static org.mockito.Mockito.when;
39+
40+
import java.util.stream.Stream;
41+
import org.hisp.dhis.event.EventStatus;
42+
import org.hisp.dhis.tracker.imports.TrackerIdSchemeParams;
43+
import org.hisp.dhis.tracker.imports.bundle.TrackerBundle;
44+
import org.hisp.dhis.tracker.imports.domain.Event;
45+
import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
46+
import org.hisp.dhis.tracker.imports.validation.Reporter;
47+
import org.hisp.dhis.tracker.imports.validation.ValidationCode;
48+
import org.junit.jupiter.api.BeforeEach;
49+
import org.junit.jupiter.api.extension.ExtendWith;
50+
import org.junit.jupiter.params.ParameterizedTest;
51+
import org.junit.jupiter.params.provider.Arguments;
52+
import org.junit.jupiter.params.provider.MethodSource;
53+
import org.mockito.Mock;
54+
import org.mockito.junit.jupiter.MockitoExtension;
55+
56+
/**
57+
* @author Enrico Colasante
58+
*/
59+
@ExtendWith(MockitoExtension.class)
60+
class StatusUpdateValidatorTest {
61+
62+
private StatusUpdateValidator validator;
63+
64+
@Mock TrackerPreheat preheat;
65+
66+
private static final String EVENT_UID = "h4w96yEMlzO";
67+
68+
@Mock private TrackerBundle bundle;
69+
70+
private Reporter reporter;
71+
72+
@BeforeEach
73+
public void setUp() {
74+
validator = new StatusUpdateValidator();
75+
76+
when(bundle.getPreheat()).thenReturn(preheat);
77+
78+
TrackerIdSchemeParams idSchemes = TrackerIdSchemeParams.builder().build();
79+
reporter = new Reporter(idSchemes);
80+
}
81+
82+
@ParameterizedTest
83+
@MethodSource("validTransitions")
84+
void shouldPassValidationWhenGoingFromStatusToStatus(
85+
EventStatus fromStatus, EventStatus toStatus) {
86+
org.hisp.dhis.program.Event savedEvent = new org.hisp.dhis.program.Event();
87+
savedEvent.setUid(EVENT_UID);
88+
savedEvent.setStatus(fromStatus);
89+
when(preheat.getEvent(EVENT_UID)).thenReturn(savedEvent);
90+
91+
Event event = Event.builder().event(EVENT_UID).status(toStatus).build();
92+
93+
validator.validate(reporter, bundle, event);
94+
95+
assertIsEmpty(reporter.getErrors());
96+
}
97+
98+
@ParameterizedTest
99+
@MethodSource("invalidTransitions")
100+
void shouldFailValidationWhenGoingFromStatusToStatus(
101+
EventStatus fromStatus, EventStatus toStatus) {
102+
org.hisp.dhis.program.Event savedEvent = new org.hisp.dhis.program.Event();
103+
savedEvent.setUid(EVENT_UID);
104+
savedEvent.setStatus(fromStatus);
105+
when(preheat.getEvent(EVENT_UID)).thenReturn(savedEvent);
106+
107+
Event event = Event.builder().event(EVENT_UID).status(toStatus).build();
108+
109+
validator.validate(reporter, bundle, event);
110+
111+
assertHasError(reporter, event, ValidationCode.E1316);
112+
}
113+
114+
private static Stream<Arguments> validTransitions() {
115+
return Stream.of(
116+
Arguments.of(ACTIVE, ACTIVE),
117+
Arguments.of(ACTIVE, COMPLETED),
118+
Arguments.of(ACTIVE, VISITED),
119+
Arguments.of(VISITED, VISITED),
120+
Arguments.of(VISITED, ACTIVE),
121+
Arguments.of(VISITED, COMPLETED),
122+
Arguments.of(COMPLETED, VISITED),
123+
Arguments.of(COMPLETED, ACTIVE),
124+
Arguments.of(COMPLETED, COMPLETED),
125+
Arguments.of(SCHEDULE, ACTIVE),
126+
Arguments.of(SCHEDULE, COMPLETED),
127+
Arguments.of(SCHEDULE, VISITED),
128+
Arguments.of(SCHEDULE, SCHEDULE),
129+
Arguments.of(SCHEDULE, SKIPPED),
130+
Arguments.of(SKIPPED, ACTIVE),
131+
Arguments.of(SKIPPED, COMPLETED),
132+
Arguments.of(SKIPPED, VISITED),
133+
Arguments.of(SKIPPED, SCHEDULE),
134+
Arguments.of(SKIPPED, SKIPPED),
135+
Arguments.of(OVERDUE, ACTIVE),
136+
Arguments.of(OVERDUE, COMPLETED),
137+
Arguments.of(OVERDUE, VISITED),
138+
Arguments.of(OVERDUE, SCHEDULE),
139+
Arguments.of(OVERDUE, SKIPPED),
140+
Arguments.of(SCHEDULE, OVERDUE),
141+
Arguments.of(SKIPPED, OVERDUE),
142+
Arguments.of(OVERDUE, OVERDUE));
143+
}
144+
145+
private static Stream<Arguments> invalidTransitions() {
146+
return Stream.of(
147+
Arguments.of(ACTIVE, OVERDUE),
148+
Arguments.of(ACTIVE, SKIPPED),
149+
Arguments.of(ACTIVE, SCHEDULE),
150+
Arguments.of(VISITED, OVERDUE),
151+
Arguments.of(VISITED, SKIPPED),
152+
Arguments.of(VISITED, SCHEDULE),
153+
Arguments.of(COMPLETED, OVERDUE),
154+
Arguments.of(COMPLETED, SKIPPED),
155+
Arguments.of(COMPLETED, SCHEDULE));
156+
}
157+
}

0 commit comments

Comments
 (0)