From 62f8b1862a2328c50369d88fe635d70af8f9325c Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Wed, 9 Jul 2025 15:35:30 -0400 Subject: [PATCH 1/5] [BI-2656] - added null check --- .../appendoverwrite/factory/data/OverwrittenData.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java index 54a1afe06..ced100205 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java @@ -137,9 +137,13 @@ public PendingImportObject constructPendingObservation() { if (!isTimestampMatched()) { // Update the timestamp - DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; - String formattedTimeStampValue = formatter.format(observationService.parseDateTime(timestamp)); - update.setObservationTimeStamp(OffsetDateTime.parse(formattedTimeStampValue)); + if (timestamp == null) { + update.setObservationTimeStamp(null); + } else { + DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; + String formattedTimeStampValue = formatter.format(observationService.parseDateTime(timestamp)); + update.setObservationTimeStamp(OffsetDateTime.parse(formattedTimeStampValue)); + } // Add original timestamp to changelog entry original = Optional.ofNullable(original).map(o -> o + " " + observation.getObservationTimeStamp()).orElse(String.valueOf(observation.getObservationTimeStamp())); From 5d25aab5338b8ba6bfbcefb9da531b41c3dfa758 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Thu, 10 Jul 2025 15:47:26 -0400 Subject: [PATCH 2/5] [BI-2656] - prevent setting timestamp to null --- .../factory/data/OverwrittenData.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java index ced100205..9354d638c 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java @@ -135,15 +135,11 @@ public PendingImportObject constructPendingObservation() { original = observation.getValue(); } - if (!isTimestampMatched()) { + if (!isTimestampMatched() && timestamp != null) { // Update the timestamp - if (timestamp == null) { - update.setObservationTimeStamp(null); - } else { - DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; - String formattedTimeStampValue = formatter.format(observationService.parseDateTime(timestamp)); - update.setObservationTimeStamp(OffsetDateTime.parse(formattedTimeStampValue)); - } + DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; + String formattedTimeStampValue = formatter.format(observationService.parseDateTime(timestamp)); + update.setObservationTimeStamp(OffsetDateTime.parse(formattedTimeStampValue)); // Add original timestamp to changelog entry original = Optional.ofNullable(original).map(o -> o + " " + observation.getObservationTimeStamp()).orElse(String.valueOf(observation.getObservationTimeStamp())); From 14eeca3a6aa86e6bd95b046886c82be248a0a9e4 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Wed, 16 Jul 2025 17:57:41 -0400 Subject: [PATCH 3/5] [BI-2656] - added integration test --- .../importer/ExperimentFileImportTest.java | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 975c2b083..881768cbf 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -1172,6 +1172,82 @@ public void importNewObsAfterFirstExpWithObs(boolean commit) { } } + /* + Scenario: + - an experiment was created with observations and timestamps + - do a second upload with additional observations for the experiment, but without the timestamp column + - verify the second set of observations get uploaded successfully + */ + @Test + @SneakyThrows + public void importNewObsAfterFirstExpWithObsAndTimestamps() { + log.debug("importNewObsAfterFirstExpWithObsAndTimestamps"); + List traits = importTestUtils.createTraits(2); + Program program = createProgram("Exp with TS and additional Uploads ", "EXTSAU", "EXTSAU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Map newExp = new HashMap<>(); + newExp.put(Columns.GERMPLASM_GID, Integer.valueOf("1")); + newExp.put(Columns.TEST_CHECK, "T"); + newExp.put(Columns.EXP_TITLE, "Test Exp"); + newExp.put(Columns.EXP_UNIT, "Plot"); + newExp.put(Columns.EXP_TYPE, "Phenotyping"); + newExp.put(Columns.ENV, "New Env"); + newExp.put(Columns.ENV_LOCATION, "Location A"); + newExp.put(Columns.ENV_YEAR, Integer.valueOf("2025")); + newExp.put(Columns.EXP_UNIT_ID, "a-1"); + newExp.put(Columns.REP_NUM, Integer.valueOf("1")); + newExp.put(Columns.BLOCK_NUM, Integer.valueOf("1")); + newExp.put(Columns.ROW, Integer.valueOf("1")); + newExp.put(Columns.COLUMN, Integer.valueOf("1")); + newExp.put(traits.get(0).getObservationVariableName(), "1"); + newExp.put("TS:" + traits.get(0).getObservationVariableName(), "2019-12-19T12:14:50Z"); + + // In the first upload, only 1 trait should be present. + List initialTraits = List.of(traits.get(0)); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), initialTraits), null, true, client, program, mappingId, newExperimentWorkflowId); + + BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); + Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); + assertTrue(trialIdXref.isPresent()); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); + + BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); + Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); + assertTrue(ouIdXref.isPresent()); + + Map newObservation = new HashMap<>(); + newObservation.put(Columns.GERMPLASM_GID, Integer.valueOf("1")); + newObservation.put(Columns.TEST_CHECK, "T"); + newObservation.put(Columns.EXP_TITLE, "Test Exp"); + newObservation.put(Columns.EXP_UNIT, "Plot"); + newObservation.put(Columns.EXP_TYPE, "Phenotyping"); + newObservation.put(Columns.ENV, "New Env"); + newObservation.put(Columns.ENV_LOCATION, "Location A"); + newObservation.put(Columns.ENV_YEAR, Integer.valueOf("2025")); + newObservation.put(Columns.EXP_UNIT_ID, "a-1"); + newObservation.put(Columns.REP_NUM, Integer.valueOf("1")); + newObservation.put(Columns.BLOCK_NUM, Integer.valueOf("1")); + newObservation.put(Columns.ROW, Integer.valueOf("1")); + newObservation.put(Columns.COLUMN, Integer.valueOf("1")); + newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); + newObservation.put(traits.get(0).getObservationVariableName(), "1"); + newObservation.put(traits.get(1).getObservationVariableName(), "1"); + + // Pass overwrite in request body to allow append workflow to work normally. + Map userData = Map.of("overwrite", "true", "overwriteReason", "testing"); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits), userData, true, client, program, mappingId, appendOverwriteWorkflowId); + + JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); + assertEquals(1, previewRows.size()); + JsonObject row = previewRows.get(0).getAsJsonObject(); + + assertEquals("EXISTING", row.getAsJsonObject("trial").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); + assertRowSaved(newObservation, program, traits); + + } + /* Scenario: - Create an experiment with valid observations. From 35f6303c660076803dc9fc9a7ac9276691264d65 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Wed, 16 Jul 2025 18:08:06 -0400 Subject: [PATCH 4/5] [BI-2656] - cleaned up test --- .../importer/ExperimentFileImportTest.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 881768cbf..7a984072c 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -1185,19 +1185,19 @@ public void importNewObsAfterFirstExpWithObsAndTimestamps() { List traits = importTestUtils.createTraits(2); Program program = createProgram("Exp with TS and additional Uploads ", "EXTSAU", "EXTSAU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); - newExp.put(Columns.GERMPLASM_GID, Integer.valueOf("1")); + newExp.put(Columns.GERMPLASM_GID, "1"); newExp.put(Columns.TEST_CHECK, "T"); newExp.put(Columns.EXP_TITLE, "Test Exp"); newExp.put(Columns.EXP_UNIT, "Plot"); newExp.put(Columns.EXP_TYPE, "Phenotyping"); newExp.put(Columns.ENV, "New Env"); newExp.put(Columns.ENV_LOCATION, "Location A"); - newExp.put(Columns.ENV_YEAR, Integer.valueOf("2025")); + newExp.put(Columns.ENV_YEAR, "2025"); newExp.put(Columns.EXP_UNIT_ID, "a-1"); - newExp.put(Columns.REP_NUM, Integer.valueOf("1")); - newExp.put(Columns.BLOCK_NUM, Integer.valueOf("1")); - newExp.put(Columns.ROW, Integer.valueOf("1")); - newExp.put(Columns.COLUMN, Integer.valueOf("1")); + newExp.put(Columns.REP_NUM, "1"); + newExp.put(Columns.BLOCK_NUM, "1"); + newExp.put(Columns.ROW, "1"); + newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), "1"); newExp.put("TS:" + traits.get(0).getObservationVariableName(), "2019-12-19T12:14:50Z"); @@ -1215,24 +1215,24 @@ public void importNewObsAfterFirstExpWithObsAndTimestamps() { assertTrue(ouIdXref.isPresent()); Map newObservation = new HashMap<>(); - newObservation.put(Columns.GERMPLASM_GID, Integer.valueOf("1")); + newObservation.put(Columns.GERMPLASM_GID, "1"); newObservation.put(Columns.TEST_CHECK, "T"); newObservation.put(Columns.EXP_TITLE, "Test Exp"); newObservation.put(Columns.EXP_UNIT, "Plot"); newObservation.put(Columns.EXP_TYPE, "Phenotyping"); newObservation.put(Columns.ENV, "New Env"); newObservation.put(Columns.ENV_LOCATION, "Location A"); - newObservation.put(Columns.ENV_YEAR, Integer.valueOf("2025")); + newObservation.put(Columns.ENV_YEAR, "2025"); newObservation.put(Columns.EXP_UNIT_ID, "a-1"); - newObservation.put(Columns.REP_NUM, Integer.valueOf("1")); - newObservation.put(Columns.BLOCK_NUM, Integer.valueOf("1")); - newObservation.put(Columns.ROW, Integer.valueOf("1")); - newObservation.put(Columns.COLUMN, Integer.valueOf("1")); + newObservation.put(Columns.REP_NUM, "1"); + newObservation.put(Columns.BLOCK_NUM, "1"); + newObservation.put(Columns.ROW, "1"); + newObservation.put(Columns.COLUMN, "1"); newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObservation.put(traits.get(0).getObservationVariableName(), "1"); newObservation.put(traits.get(1).getObservationVariableName(), "1"); - // Pass overwrite in request body to allow append workflow to work normally. + // Send overwrite parameters in request body to allow the append workflow to work normally. Map userData = Map.of("overwrite", "true", "overwriteReason", "testing"); JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits), userData, true, client, program, mappingId, appendOverwriteWorkflowId); From 42c9c2ffb6a4021de57bc812ad9902f188862a5f Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 29 Jul 2025 14:47:14 -0400 Subject: [PATCH 5/5] [BI-2656] - handled timestamp edge case prevents erroneous mutated data indication on upload preview --- .../middleware/process/ImportTableProcess.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java index 770576b67..4a2846fcd 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java @@ -284,7 +284,7 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext BrAPIObservation observation = gson.fromJson(gson.toJson(observationByObsHash.get(observationHash)), BrAPIObservation.class); // Is there a change to the prior data? - if (isChanged(cellData, observation, cell.timestamp)) { + if (isChanged(cellData, observation, cell.timestamp, tsColByPheno.containsKey(phenoColumnName))) { // Is prior data protected? /** @@ -394,14 +394,18 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext } } - private boolean isChanged(String cellData, BrAPIObservation observation, String newTimestamp) { + private boolean isChanged(String cellData, BrAPIObservation observation, String newTimestamp, boolean timestampColumnPresent) { if (!cellData.isBlank() && !cellData.equals(observation.getValue())){ return true; } - if (StringUtils.isBlank(newTimestamp)) { - return (observation.getObservationTimeStamp()!=null); + // Only check timestamp if the TS: column was present in the uploaded file. + if (timestampColumnPresent) { + if (StringUtils.isBlank(newTimestamp)) { + return (observation.getObservationTimeStamp()!=null); + } + return !observationService.parseDateTime(newTimestamp).equals(observation.getObservationTimeStamp()); } - return !observationService.parseDateTime(newTimestamp).equals(observation.getObservationTimeStamp()); + return false; } /**