From 3429f23083daae130f5c37b7e4758e252ca90bc9 Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Tue, 19 Mar 2024 13:15:43 -0400 Subject: [PATCH 1/5] [BI-2065] - Experiment overwrite does not do all data validations on phenotype data --- .../services/processors/ExperimentProcessor.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index b67305c79..c349c57d5 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -765,6 +765,15 @@ 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 move into separate method if not too messy + 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) { BrAPIObservation pendingObservation = observationByHash.get(importHash).getBrAPIObject(); From 95fda13b0e1b0f1913305291203d5b8af355e510 Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Tue, 19 Mar 2024 13:38:20 -0400 Subject: [PATCH 2/5] [BI-2065] - Missing bracket --- .../brapps/importer/services/processors/ExperimentProcessor.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index c349c57d5..6ab1c48b7 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -773,6 +773,7 @@ private void validateObservations(ValidationErrors validationErrors, 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) { From d7d8dc6670632d6063d9bf515fb16b8f1142879e Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Tue, 19 Mar 2024 21:13:55 -0400 Subject: [PATCH 3/5] [BI-2065] - Clarify todo --- .../importer/services/processors/ExperimentProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 6ab1c48b7..c42b843a8 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -766,7 +766,7 @@ private void validateObservations(ValidationErrors validationErrors, } else if (existingObsByObsHash.containsKey(importHash) && !isObservationMatched(importHash, importObsValue, phenoCol, rowNum)) { // different data means validations still need to happen - // TODO move into separate method if not too messy + // TODO consider moving these two calls into a separate method since called twice together validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(rowNum), phenoCol.name(), validationErrors, rowNum); //Timestamp validation From bd77c55d7fd17db61827b223032ff306114fb8d1 Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Tue, 2 Apr 2024 14:31:25 -0400 Subject: [PATCH 4/5] Timestamp Error Handling --- .../services/processors/ExperimentProcessor.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index c42b843a8..ec7588fa4 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -776,7 +776,8 @@ private void validateObservations(ValidationErrors validationErrors, } // 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()); @@ -1061,7 +1062,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) { From 5b26088890d29b447bd84563933f332c300ba427 Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Thu, 2 May 2024 09:45:39 -0400 Subject: [PATCH 5/5] [BI-2065] - Code review timestamp fix Handled case where present observation but blank associated timestamp led to an unhandled attempt to parse said blank timestamp --- .../importer/services/processors/ExperimentProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index ec7588fa4..f01578e21 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -1111,7 +1111,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));