Skip to content

[BI-2065] - Experiment overwrite missing phenotype validations #340

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

Merged
HMS17 merged 7 commits into from
May 6, 2024
Merged

[BI-2065] - Experiment overwrite missing phenotype validations #340

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3429f23
[BI-2065] - Experiment overwrite does not do all data validations on …
HMS17 Mar 19, 2024
95fda13
[BI-2065] - Missing bracket
HMS17 Mar 19, 2024
d7d8dc6
[BI-2065] - Clarify todo
HMS17 Mar 20, 2024
bd77c55
Timestamp Error Handling
HMS17 Apr 2, 2024
5b26088
[BI-2065] - Code review timestamp fix
HMS17 May 2, 2024
7ca9b7e
Merge branch 'develop' into bug/BI-2065
HMS17 May 2, 2024
f83e467
Merge branch 'develop' into bug/BI-2065
HMS17 May 6, 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
24 changes: 21 additions & 3 deletions ...ain/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,8 +882,19 @@ private void validateObservations(ValidationErrors validationErrors,
// had been saved prior to import
} else if (existingObsByObsHash.containsKey(importHash) && !isObservationMatched(importHash, importObsValue, phenoCol, rowNum)) {

// different data means validations still need to happen
// TODO consider moving these two calls into a separate method since called twice together
Copy link
Contributor

@mlm483 mlm483 Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this referring to validateObservationValue and validateTimeStampValue?

Copy link
Contributor Author

@HMS17 HMS17 Apr 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, since we have validation in two places in the method (one for the overwrite case and one for the non-overwrite case, it feels like it should be cleaned up in some manner)

validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(rowNum), phenoCol.name(), validationErrors, rowNum);

//Timestamp validation
if(timeStampColByPheno.containsKey(phenoCol.name())) {
Column<?> timeStampCol = timeStampColByPheno.get(phenoCol.name());
validateTimeStampValue(timeStampCol.getString(rowNum), timeStampCol.name(), validationErrors, rowNum);
}

// add a change log entry when updating the value of an observation
if (commit) {
// only will update and thereby need change log entry if no error
if (commit && (!validationErrors.hasErrors())) {
BrAPIObservation pendingObservation = observationByHash.get(importHash).getBrAPIObject();
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd:hh-mm-ssZ");
String timestamp = formatter.format(OffsetDateTime.now());
Expand Down Expand Up @@ -1182,7 +1193,14 @@ boolean isTimestampMatched(String observationHash, String timeStamp) {
if (priorStamp == null) {
return timeStamp == null;
}
return priorStamp.isEqual(OffsetDateTime.parse(timeStamp));
boolean isMatched = false;
try {
isMatched = priorStamp.isEqual(OffsetDateTime.parse(timeStamp));
} catch(DateTimeParseException e){
//if timestamp is invalid DateTime not equal to validated priorStamp
log.error(e.getMessage(), e);
}
return isMatched;
}

boolean isValueMatched(String observationHash, String value) {
Expand Down Expand Up @@ -1232,7 +1250,7 @@ private void fetchOrCreateObservationPIO(Program program,
newObservation = gson.fromJson(gson.toJson(existingObsByObsHash.get(key)), BrAPIObservation.class);
if (!isValueMatched(key, value)){
newObservation.setValue(value);
} else if (!isTimestampMatched(key, timeStampValue)) {
} else if (!StringUtils.isBlank(timeStampValue) && !isTimestampMatched(key, timeStampValue)) {
DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT;
String formattedTimeStampValue = formatter.format(OffsetDateTime.parse(timeStampValue));
newObservation.setObservationTimeStamp(OffsetDateTime.parse(formattedTimeStampValue));
Expand Down