fix: Skip mandatory data value validation for UPDATE [DHIS2-17560] #17835
Conversation
enricocolasante
force-pushed
the
DHIS2-17560
branch
from
17f743a
to
73b7d37
Compare
| if (!needsToValidateDataValues(event, programStage)) { | ||
| return notPresentMandatoryDataElements; | ||
| return List.of(); |
There was a problem hiding this comment.
out of the scope of this PR, but why do we skip the validation if the event status is SCHEDULE?
There was a problem hiding this comment.
A SCHEDULE event is created at some point in time but it is meant to happen later at some point, so when it is created it is kind of empty and it doesn't make sense to require mandatory data values at that point.
There was a problem hiding this comment.
and in that case, are we covered if I create and event this way, and then I provide the values later on?
what I mean by that is, at the moment I provide the values, do we treat it as an import strategy CREATE again to make sure all mandatory fields are set?
if it's something different than CREATE, then I guess I could have mandatory values set to null, couldn't I?
There was a problem hiding this comment.
Oh no, you are right.
At the moment if you create a schedule event without the mandatory datavalues and later you just change the status of the event then you can have an empty mandatory datavalues were they are meant to be present.
Will need to check how we manage the transition from a status to another
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
.../org/hisp/dhis/tracker/imports/programrule/executor/event/SetMandatoryFieldExecutorTest.java
Fixed
Show fixed
Hide fixed
|
| } | ||
|
|
||
| return notPresentMandatoryDataElements; | ||
| EventStatus savedStatus = bundle.getPreheat().getEvent(event.getUid()).getStatus(); | ||
| return EventStatus.STATUSES_WITHOUT_DATA_VALUES.contains(savedStatus) |
There was a problem hiding this comment.
do the event statuses OVERDUE and SKIPPED also need to be considered here? or SCHEDULE would be enough?
There was a problem hiding this comment.
Yes they do. They have the exact same behaviour as SCHEDULE events, so we need to check them to be sure we are in a consistent state
Event update was failing when mandatory data values were not present in the payload even though they were already present in the DB.
Now tracker importer is validating mandatory data values only when creating an event.
Same logic is applied to
SetMandatoryFieldrule action that is dynamically adding a mandatory check on a data value.To resume, when strategy is create:- If a mandatory data element is not provided, a validation error is returned- If a mandatory data element is removed (set to null), a validation error is return. This is covered even if it doesn't really make sense to remove a data value when creating an event.When strategy is update:- If a mandatory data element is not provided, validation passes- If a mandatory data element is removed (set to null), a validation error is return.Statuses that allow to define data values are
ACTIVE,COMPLETEDorVISITED.Statuses that DO NOT allow to define data values are
SCHEDULE,SKIPPEDorOVERDUE.To resume, when strategy is create:
When strategy is update: