Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Oct 22, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0120de6
chore: Clean up data values persistence [DHIS2-18223]
enricocolasante Oct 18, 2024
f2e0b83
Fix e2e test
enricocolasante Oct 18, 2024
8c37f42
Merge remote-tracking branch 'origin/master' into DHIS2-18223
enricocolasante Oct 21, 2024
6c73f47
Merge remote-tracking branch 'origin/master' into DHIS2-18223
enricocolasante Oct 21, 2024
127a2ee
Fix formatting
enricocolasante Oct 21, 2024
974ba02
Fix review comments
enricocolasante Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions ...vice-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/TrackerObjectsMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import org.hisp.dhis.category.CategoryOptionCombo;
import org.hisp.dhis.dataelement.DataElement;
import org.hisp.dhis.event.EventStatus;
import org.hisp.dhis.eventdatavalue.EventDataValue;
import org.hisp.dhis.note.Note;
import org.hisp.dhis.organisationunit.OrganisationUnit;
import org.hisp.dhis.program.Enrollment;
Expand All @@ -50,7 +48,6 @@
import org.hisp.dhis.relationship.RelationshipType;
import org.hisp.dhis.trackedentity.TrackedEntity;
import org.hisp.dhis.trackedentity.TrackedEntityType;
import org.hisp.dhis.tracker.imports.domain.DataValue;
import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
import org.hisp.dhis.tracker.imports.util.RelationshipKeySupport;
import org.hisp.dhis.user.User;
Expand Down Expand Up @@ -236,22 +233,6 @@ private TrackerObjectsMapper() {
assignedUser.ifPresent(dbEvent::setAssignedUser);
}

// TODO(DHIS2-18223): Remove data value mapping and fix changelog logic
for (DataValue dataValue : event.getDataValues()) {
DataElement dataElement = preheat.getDataElement(dataValue.getDataElement());

EventDataValue eventDataValue = new EventDataValue();
eventDataValue.setDataElement(dataElement.getUid());
eventDataValue.setCreated(DateUtils.fromInstant(dataValue.getCreatedAt()));
eventDataValue.setCreatedByUserInfo(UserInfoSnapshot.from(user));
eventDataValue.setValue(dataValue.getValue());
eventDataValue.setLastUpdated(now);
eventDataValue.setProvidedElsewhere(dataValue.isProvidedElsewhere());
eventDataValue.setLastUpdatedByUserInfo(UserInfoSnapshot.from(user));

dbEvent.getEventDataValues().add(eventDataValue);
}

if (isNotEmpty(event.getNotes())) {
dbEvent
.getNotes()
Expand Down
220 changes: 115 additions & 105 deletions ...-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@
*/
package org.hisp.dhis.tracker.imports.bundle.persister;

import static com.google.common.base.Preconditions.checkNotNull;

import jakarta.persistence.EntityManager;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
Expand All @@ -41,15 +38,16 @@
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import lombok.Builder;
import lombok.Data;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import org.apache.commons.lang3.StringUtils;
import org.hisp.dhis.changelog.ChangeLogType;
import org.hisp.dhis.common.UID;
import org.hisp.dhis.dataelement.DataElement;
import org.hisp.dhis.event.EventStatus;
import org.hisp.dhis.eventdatavalue.EventDataValue;
import org.hisp.dhis.program.Event;
import org.hisp.dhis.program.UserInfoSnapshot;
import org.hisp.dhis.reservedvalue.ReservedValueService;
import org.hisp.dhis.tracker.TrackerType;
import org.hisp.dhis.tracker.export.event.EventChangeLogService;
Expand All @@ -62,7 +60,6 @@
import org.hisp.dhis.tracker.imports.job.TrackerNotificationDataBundle;
import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
import org.hisp.dhis.user.UserDetails;
import org.hisp.dhis.util.DateUtils;
import org.springframework.stereotype.Component;

/**
Expand Down Expand Up @@ -170,65 +167,119 @@ private void handleDataValues(
.orElse(new HashMap<>());

payloadDataValues.forEach(
dv -> {
DataElement dataElement = preheat.getDataElement(dv.getDataElement());
checkNotNull(
dataElement,
"Data element should never be NULL here if validation is enforced before commit.");

// EventDataValue.dataElement contains a UID
EventDataValue eventDataValue = dataValueDBMap.get(dataElement.getUid());
dataValue -> {
DataElement dataElement = preheat.getDataElement(dataValue.getDataElement());
EventDataValue dbDataValue = dataValueDBMap.get(dataElement.getUid());

if (isNewDataValue(dbDataValue, dataValue)) {
logDataValueChange(
user.getUsername(),
dataElement,
event,
dataValue.getValue(),
dataValue.isProvidedElsewhere(),
ChangeLogType.CREATE);
saveDataValue(dataValue, event, dataElement, user, entityManager, preheat);
} else if (isUpdate(dbDataValue, dataValue)) {
logDataValueChange(
user.getUsername(),
dataElement,
event,
dbDataValue.getValue(),
dbDataValue.getProvidedElsewhere(),
ChangeLogType.UPDATE);
updateDataValue(
dbDataValue, dataValue, event, dataElement, user, entityManager, preheat);
} else if (isDeletion(dbDataValue, dataValue)) {
logDataValueChange(
user.getUsername(),
dataElement,
event,
dbDataValue.getValue(),
dbDataValue.getProvidedElsewhere(),
ChangeLogType.DELETE);
deleteDataValue(dbDataValue, event, dataElement, entityManager, preheat);
}
});
}

ValuesHolder valuesHolder = getAuditAndDateParameters(eventDataValue, dv);
private void saveDataValue(
DataValue dv,
Event event,
DataElement dataElement,
UserDetails user,
EntityManager entityManager,
TrackerPreheat preheat) {
EventDataValue eventDataValue = new EventDataValue();
eventDataValue.setDataElement(dataElement.getUid());
eventDataValue.setCreated(new Date());
eventDataValue.setCreatedByUserInfo(UserInfoSnapshot.from(user));
eventDataValue.setStoredBy(user.getUsername());

eventDataValue = valuesHolder.getEventDataValue();
eventDataValue.setLastUpdated(new Date());
eventDataValue.setLastUpdatedByUserInfo(UserInfoSnapshot.from(user));

eventDataValue.setDataElement(dataElement.getUid());
eventDataValue.setStoredBy(dv.getStoredBy());
eventDataValue.setValue(dv.getValue());
eventDataValue.setProvidedElsewhere(dv.isProvidedElsewhere());

if (StringUtils.isEmpty(dv.getValue())) {
if (dataElement.isFileType()) {
unassignFileResource(
entityManager, preheat, event.getUid(), eventDataValue.getValue());
}
if (dataElement.isFileType()) {
assignFileResource(entityManager, preheat, event.getUid(), eventDataValue.getValue());
}

event.getEventDataValues().remove(eventDataValue);
} else {
eventDataValue.setValue(dv.getValue());
event.getEventDataValues().add(eventDataValue);
}

if (dataElement.isFileType()) {
assignFileResource(entityManager, preheat, event.getUid(), eventDataValue.getValue());
}
private void updateDataValue(
EventDataValue eventDataValue,
DataValue dv,
Event event,
DataElement dataElement,
UserDetails user,
EntityManager entityManager,
TrackerPreheat preheat) {
eventDataValue.setLastUpdated(new Date());
eventDataValue.setLastUpdatedByUserInfo(UserInfoSnapshot.from(user));

event.getEventDataValues().remove(eventDataValue);
event.getEventDataValues().add(eventDataValue);
}
if (dataElement.isFileType()) {
unassignFileResource(entityManager, preheat, event.getUid(), eventDataValue.getValue());
assignFileResource(entityManager, preheat, event.getUid(), dv.getValue());
}

logTrackedEntityDataValueHistory(
user.getUsername(), dataElement, event, new Date(), valuesHolder);
});
eventDataValue.setProvidedElsewhere(dv.isProvidedElsewhere());
eventDataValue.setValue(dv.getValue());
}

private Date getFromOrNewDate(DataValue dv, Function<DataValue, Instant> dateGetter) {
return Optional.of(dv).map(dateGetter).map(DateUtils::fromInstant).orElseGet(Date::new);
private void deleteDataValue(
EventDataValue eventDataValue,
Event event,
DataElement dataElement,
EntityManager entityManager,
TrackerPreheat preheat) {
if (dataElement.isFileType()) {
unassignFileResource(entityManager, preheat, event.getUid(), eventDataValue.getValue());
}

event.getEventDataValues().remove(eventDataValue);
enricocolasante marked this conversation as resolved.
Show resolved Hide resolved
}

private void logTrackedEntityDataValueHistory(
String userName, DataElement de, Event event, Date created, ValuesHolder valuesHolder) {
ChangeLogType changeLogType = valuesHolder.getChangeLogType();

if (changeLogType != null) {
TrackedEntityDataValueChangeLog valueAudit = new TrackedEntityDataValueChangeLog();
valueAudit.setEvent(event);
valueAudit.setValue(valuesHolder.getValue());
valueAudit.setAuditType(changeLogType);
valueAudit.setDataElement(de);
valueAudit.setModifiedBy(userName);
valueAudit.setProvidedElsewhere(valuesHolder.isProvidedElseWhere());
valueAudit.setCreated(created);

eventChangeLogService.addTrackedEntityDataValueChangeLog(valueAudit);
}
private void logDataValueChange(
String userName,
DataElement de,
Event event,
String value,
boolean providedElsewhere,
ChangeLogType changeLogType) {

TrackedEntityDataValueChangeLog changeLog = new TrackedEntityDataValueChangeLog();
changeLog.setEvent(event);
changeLog.setValue(value);
changeLog.setAuditType(changeLogType);
changeLog.setDataElement(de);
changeLog.setModifiedBy(userName);
changeLog.setProvidedElsewhere(providedElsewhere);
changeLog.setCreated(new Date());

eventChangeLogService.addTrackedEntityDataValueChangeLog(changeLog);
}

@Override
Expand All @@ -245,61 +296,20 @@ protected String getUpdatedTrackedEntity(Event entity) {
.orElse(null);
}

private boolean isNewDataValue(EventDataValue eventDataValue, DataValue dv) {
return eventDataValue == null
|| (eventDataValue.getCreated() == null && StringUtils.isNotBlank(dv.getValue()));
}

private boolean isDeletion(EventDataValue eventDataValue, DataValue dv) {
return StringUtils.isNotBlank(eventDataValue.getValue()) && StringUtils.isBlank(dv.getValue());
private boolean isNewDataValue(
@CheckForNull EventDataValue eventDataValue, @Nonnull DataValue dv) {
return eventDataValue == null && !StringUtils.isBlank(dv.getValue());
}

private boolean isUpdate(EventDataValue eventDataValue, DataValue dv) {
return !StringUtils.equals(dv.getValue(), eventDataValue.getValue());
private boolean isDeletion(@CheckForNull EventDataValue eventDataValue, @Nonnull DataValue dv) {
return eventDataValue != null
&& StringUtils.isNotBlank(eventDataValue.getValue())
&& StringUtils.isBlank(dv.getValue());
}

private ValuesHolder getAuditAndDateParameters(EventDataValue eventDataValue, DataValue dv) {
String persistedValue;

ChangeLogType changeLogType = null;

if (isNewDataValue(eventDataValue, dv)) {
eventDataValue = new EventDataValue();
eventDataValue.setCreated(getFromOrNewDate(dv, DataValue::getCreatedAt));
eventDataValue.setLastUpdated(getFromOrNewDate(dv, DataValue::getUpdatedAt));
persistedValue = dv.getValue();
changeLogType = ChangeLogType.CREATE;
} else {
persistedValue = eventDataValue.getValue();

if (isUpdate(eventDataValue, dv)) {
changeLogType = ChangeLogType.UPDATE;
eventDataValue.setLastUpdated(getFromOrNewDate(dv, DataValue::getUpdatedAt));
}

if (isDeletion(eventDataValue, dv)) {
changeLogType = ChangeLogType.DELETE;
eventDataValue.setLastUpdated(getFromOrNewDate(dv, DataValue::getUpdatedAt));
}
}

return ValuesHolder.builder()
.value(persistedValue)
.providedElseWhere(dv.isProvidedElsewhere())
.changeLogType(changeLogType)
.eventDataValue(eventDataValue)
.build();
}

@Data
@Builder
static class ValuesHolder {
private final String value;

private final boolean providedElseWhere;

private final ChangeLogType changeLogType;

private final EventDataValue eventDataValue;
private boolean isUpdate(@CheckForNull EventDataValue eventDataValue, @Nonnull DataValue dv) {
return eventDataValue != null
&& !StringUtils.isBlank(dv.getValue())
&& !StringUtils.equals(dv.getValue(), eventDataValue.getValue());
}
}
5 changes: 0 additions & 5 deletions ...es/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/domain/DataValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

import com.fasterxml.jackson.annotation.JsonProperty;
import java.io.Serializable;
import java.time.Instant;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
Expand All @@ -44,10 +43,6 @@
@AllArgsConstructor
// TODO(DHIS2-18223) Remove unused fields when fixing data values logic
public class DataValue implements Serializable {
@JsonProperty private Instant createdAt; // remove

@JsonProperty private Instant updatedAt; // remove

@JsonProperty private String storedBy;

@JsonProperty private boolean providedElsewhere;
Expand Down
Loading
Loading