[BI-1830] Overwrite entity dataset by dmeidlin · Pull Request #305 · Breeding-Insight/bi-api

dmeidlin · 2023-10-23T19:15:04Z

Description

Story: BI-1830

The expected behavior of a user-supplied observation unit dbid superseding experiment, environment, and unit import values has apparently regressed. Fixing this regression will be handled in a separate card, and the parts of ExperimentProcessor involved commented out with TODOs. For this feature, observations are updated by filling out the complete import row with the ObsUnitID value blank.

Fields overwrite and overwriteReason added to ExperimentObservation
validation of existing observations updated
a changeLog entry is added to additionalInfo when committing updates to existing observations
postBrapiData was updated to both post new observations and put updates to existing observations

Dependencies

bi-web feature/BI-1265

Testing

import germplasm, observation variables, and a new experiment
alter some of the rows of the experiment import file

change the values of a prior observation
leave the rest (environment, experiment, unit #, etc.)of the import row the same

import the new file
view details of the experiment and verify any changes to observation values.

Checklist:

I have performed a self-review of my own code
I have tested my code and ensured it meets the acceptance criteria of the story
I have tested that my code works with both the brapi-java-server and BreedBase
I have create/modified unit tests to cover this change
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to documentation
I have run TAF: <please include a link to TAF run>

davedrp

Developer test passed

davedrp · 2023-11-15T20:39:31Z

src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationDAO.java

        return brAPIDAOUtil.post(brAPIObservationList, upload, api::observationsPost, importDAO::update);
    }

+    public BrAPIObservation updateBrAPIObservation(String dbId, BrAPIObservation observation, UUID programId) throws ApiException {


IntellaJ says: "Return value of the method is never used". Does this need a return value

Thanks, I put a check in place that will compare the requested value and timestamp to the updated values returned from the brapi service, throwing an exception if they don't match.

davedrp · 2023-11-15T20:44:05Z

src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java

The following import statements are no longer needed;
import org.brapi.client.v2.model.exceptions.ApiException;
import org.breedinginsight.services.exceptions.DoesNotExistException;
import org.breedinginsight.services.exceptions.MissingRequiredInfoException;
import org.breedinginsight.services.exceptions.UnprocessableEntityException;
import org.breedinginsight.services.exceptions.ValidatorException;

davedrp · 2023-11-15T20:48:07Z

...dinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java

The following Import statements are no loner needed:
import org.brapi.client.v2.model.exceptions.ApiException;
import org.breedinginsight.brapps.importer.model.imports.germplasm.GermplasmImport;
import org.breedinginsight.services.exceptions.MissingRequiredInfoException;
import org.breedinginsight.services.exceptions.UnprocessableEntityException;
import org.breedinginsight.services.exceptions.ValidatorException;

davedrp · 2023-11-15T20:51:25Z

...java/org/breedinginsight/brapps/importer/model/imports/germplasm/GermplasmImportService.java

The following import statements are no longer used:
import org.brapi.client.v2.model.exceptions.ApiException;
import org.breedinginsight.services.exceptions.DoesNotExistException;
import org.breedinginsight.services.exceptions.MissingRequiredInfoException;
import org.breedinginsight.services.exceptions.UnprocessableEntityException;
import org.breedinginsight.services.exceptions.ValidatorException;

davedrp · 2023-11-15T21:07:37Z

src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java

+    public static String missingColumn = "Column name \"%s\" does not exist in file";
+    public static String missingUserInput = "User input, \"%s\" is required";
+    public static String wrongUserInputDataType = "User input, \"%s\" must be an %s";
+


The above 5-fields are never used

davedrp · 2023-11-16T14:29:44Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

 import java.util.function.Supplier;
 import java.util.stream.Collectors;

+import static org.breedinginsight.brapps.importer.services.FileMappingUtil.EXPERIMENT_TEMPLATE_NAME;


Import not used

davedrp · 2023-11-16T14:32:17Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

+            "If you’re trying to add these units to the experiment, please create a new environment" +
+            " with all appropriate experiment units (NOTE: this will generate new Observation Unit Ids " +
+            "for each experiment unit).";
    private static final String MISSING_OBS_UNIT_ID_ERROR = "Experiment Units are missing Observation Unit Id.<br/><br/>" +


MISSING_OBS_UNIT_ID_ERROR is no longer used

I'm going to keep this in for now since it will likely be used again in the near future once the functionality of specifying by OU id is restored.

davedrp · 2023-11-16T14:33:33Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

@@ -90,6 +104,9 @@ public class ExperimentProcessor implements Processor {
    private static final String TIMESTAMP_REGEX = "^"+TIMESTAMP_PREFIX+"\\s*";
    private static final String COMMA_DELIMITER = ",";
    private static final String BLANK_FIELD = "Required field is blank";


'BLANK_FIELD' is never used

davedrp · 2023-11-16T14:47:32Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/Processor.java

import org.breedinginsight.services.exceptions.MissingRequiredInfoException no longer used

davedrp · 2023-11-16T14:51:30Z

src/main/java/org/breedinginsight/utilities/Utilities.java

 import org.apache.commons.lang3.StringUtils;
 import org.brapi.client.v2.model.exceptions.ApiException;
 import org.brapi.v2.model.BrAPIExternalReference;
+import org.brapi.v2.model.pheno.BrAPIObservationUnit;


Import unused

nickpalladino · 2023-11-16T16:39:16Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

+                                      int rowNum,
+                                      ExperimentObservation importRow,
+                                      List<Column<?>> phenotypeCols,
+                                      Map<String, Trait> colVarMap,


Why was this changed from CaseInsensitiveMap to Map?

I don't think that was deliberate. I reverted it back to CaseInsensitiveMap.

nickpalladino · 2023-11-16T16:46:07Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

+                        pendingObservation.getAdditionalInfo().add(BrAPIAdditionalInfoFields.CHANGELOG, new JsonArray());
+                    }
+
+                    if (pendingObservation.getAdditionalInfo() != null && !pendingObservation.getAdditionalInfo().has(BrAPIAdditionalInfoFields.CHANGELOG)) {


Does this work with multiple edits? Will it add entries to the array or create a new one?

It does work with multiple edits. The changelog field in additional info stores an array, and each edit to an observation will add a new entry to that array. I've added some comments in the code here to help clarify.

nickpalladino · 2023-11-16T16:53:41Z

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

+        existingObsByObsHash = fetchExistingObservations(referencedTraits, program);
+        if (existingObsByObsHash.containsKey(key)) {
+            if (StringUtils.isNotBlank(value) &&
+                    !existingObsByObsHash.get(key).getValue().equals(value)) {


Mutated is set based on the observation value changing. Not sure if timestamp should be included here or not as well?

I made changes here and throughout to enable updating timestamps. So, now an observation PIO will have a MUTATED state if either or both value and timestamp differ from what is stored in the brapi service.

…iteReason

github-actions bot added the feature label Oct 23, 2023

dmeidlin force-pushed the feature/BI-1830 branch from 03a5d4f to f1221c2 Compare November 9, 2023 22:10

dmeidlin requested review from a team, nickpalladino and timparsons and removed request for a team November 10, 2023 15:29

dmeidlin assigned timparsons Nov 10, 2023

dmeidlin requested a review from davedrp November 13, 2023 19:31

dmeidlin assigned davedrp and unassigned timparsons Nov 13, 2023

dmeidlin removed the request for review from timparsons November 13, 2023 19:32

davedrp requested changes Nov 16, 2023

View reviewed changes

nickpalladino reviewed Nov 16, 2023

View reviewed changes

dmeidlin requested review from davedrp and nickpalladino November 28, 2023 16:11

davedrp approved these changes Nov 29, 2023

View reviewed changes

nickpalladino approved these changes Dec 1, 2023

View reviewed changes

dmeidlin force-pushed the feature/BI-1830 branch from 3a7867c to d708409 Compare December 5, 2023 19:20

dmeidlin added 12 commits December 6, 2023 11:34

update ExperimentObservation.java to have fields overwrite and overwr…

2faaf92

…iteReason

add count of existing observations to preview

38231a9

create BrAPIObservationDAO#updateBrAPIObservation

90bf5ac

add change log to additional info of mutated observation

0990232

update mutated observations in postBrapiData

ad3d566

add check for NPE

d1e8fff

count mutated observations

72dc5f7

change overwrite field type to String

6da9980

[BI-1830] init observation pio for updated value

cc94473

[BI-1830] refactor

7e4f17a

remove comments

c9d5241

code cleanup

d8b8c74

dmeidlin added 10 commits December 6, 2023 11:34

revert row indexing

f274c7b

create ChangeLogEntry class

9a9c335

check for adding new OU to existing environment

ec83971

[BI-1830] format pios for display

bb66651

map season year to study pio

800234c

[BI-1830] update experiment import test

41dd3bd

update test

c774071

add fixes

6890a6b

update observation timestamps

44e7b50

fix NPEs

4471a0a

dmeidlin force-pushed the feature/BI-1830 branch from d708409 to 4471a0a Compare December 6, 2023 17:07

dmeidlin merged commit dadbcf3 into develop Dec 6, 2023

dmeidlin deleted the feature/BI-1830 branch December 6, 2023 21:17

dmeidlin mentioned this pull request Jan 18, 2024

[BI-1265-2] Overwrite entity dataset #327

Merged

7 tasks

Conversation

dmeidlin commented Oct 23, 2023 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Testing

Checklist:

Uh oh!

davedrp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dmeidlin commented Oct 23, 2023 •

edited by atlassian bot

Loading