chore: Clean up data values persistence [DHIS2-18223]

[enricocolasante]

The flow of persisting data values was quite complicated and error prone.
Now we are dealing with data values in Event in the same way we are dealing with attributes in Enrollment and TrackedEntity. We expect only the changed values (null value represent a DELETE).

• dataValues field is not mapped in TrackerObjectsMapper
• In EventPersister, for every dataValue in the payload we decide if it is a new value, if it is an update or a deletion and we deal with FileResources and ChangeLog accordingly.
• createdAt, updatedAt, createdBy and updatedBy fields follow the same logic as implemented for all tracker objects.
• It was possible for the client to set createdAt for a dataValue. This is a bug. https://dhis2.atlassian.net/browse/DHIS2-18252. A failing e2e test, solved by fixing this file, was showing this issue.

[teleivo]

expect only the changed values (null value represent a DELETE)

and an empty string as well right? Here it looks like we are checking for blank, while for attributes we check for empty. Not that we have to support that but both data values and attributes should behave the same way.

dhis2-core/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java

Line 322 in accad72

boolean isDelete = StringUtils.isEmpty(attribute.getValue());

[teleivo]

Is there a way for us to prevent test data to have these autogenerated fields in them?

[enricocolasante]

I am not sure.
Maybe we can find a way to have a jsonMapper just for tests in the integration tests that is going to be strict and fail if there are unknown fields.
In e2e tests that wouldn't work because we are using the view model and those fields are actually used in the output.
Anyway, I cannot think of something easy to implement.

[enricocolasante]

expect only the changed values (null value represent a DELETE)

and an empty string as well right? Here it looks like we are checking for blank, while for attributes we check for empty. Not that we have to support that but both data values and attributes should behave the same way.

dhis2-core/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java

Line 322 in accad72

boolean isDelete = StringUtils.isEmpty(attribute.getValue());

I agree.
Maybe even at API level we should agree on one way, like we are always considering a blank string as an empty string.
We can ask in the backend meeting if anyone has any input on that.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud