[BI-1590] - Disallow Adding New Observation Unit to Existing Environment

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

@@ -23,6 +23,7 @@
 import org.breedinginsight.model.Program;
 import org.breedinginsight.model.User;
 import org.breedinginsight.services.exceptions.DoesNotExistException;
+import org.breedinginsight.services.exceptions.MissingRequiredInfoException;
 import org.breedinginsight.services.exceptions.UnprocessableEntityException;
 import org.breedinginsight.services.exceptions.ValidatorException;
 import tech.tablesaw.api.Table;
@@ -33,5 +34,5 @@ public abstract class BrAPIImportService {
     public String getImportTypeId() {return null;}
     public BrAPIImport getImportClass() {return null;}
     public ImportPreviewResponse process(List<BrAPIImport> brAPIImports, Table data, Program program, ImportUpload upload, User user, Boolean commit)
-            throws UnprocessableEntityException, DoesNotExistException, ValidatorException, ApiException {return null;}
+            throws UnprocessableEntityException, DoesNotExistException, ValidatorException, ApiException, MissingRequiredInfoException {return null;}
 }

src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java

@@ -26,6 +26,7 @@
 import org.breedinginsight.brapps.importer.services.processors.*;
 import org.breedinginsight.model.Program;
 import org.breedinginsight.model.User;
+import org.breedinginsight.services.exceptions.MissingRequiredInfoException;
 import org.breedinginsight.services.exceptions.UnprocessableEntityException;
 import org.breedinginsight.services.exceptions.ValidatorException;
 import tech.tablesaw.api.Table;
@@ -41,8 +42,8 @@ public class ExperimentImportService extends BrAPIImportService {
 
     private final String IMPORT_TYPE_ID = "ExperimentImport";
 
-    private Provider<ExperimentProcessor> experimentProcessorProvider;
-    private Provider<ProcessorManager> processorManagerProvider;
+    private final Provider<ExperimentProcessor> experimentProcessorProvider;
+    private final Provider<ProcessorManager> processorManagerProvider;
 
     @Inject
     public ExperimentImportService(Provider<ExperimentProcessor> experimentProcessorProvider, Provider<ProcessorManager> processorManagerProvider)
@@ -63,7 +64,7 @@ public String getImportTypeId() {
 
     @Override
     public ImportPreviewResponse process(List<BrAPIImport> brAPIImports, Table data, Program program, ImportUpload upload, User user, Boolean commit)
-            throws UnprocessableEntityException, ValidatorException, ApiException {
+            throws UnprocessableEntityException, ValidatorException, ApiException, MissingRequiredInfoException {
 
         ImportPreviewResponse response = null;
         List<Processor> processors = List.of(experimentProcessorProvider.get());

src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java

@@ -107,8 +107,8 @@ public class ExperimentObservation implements BrAPIImport {
     private String treatmentFactors;
 
     @ImportFieldType(type= ImportFieldTypeEnum.TEXT)
-    @ImportFieldMetadata(id="ObsUnitID", name="Observation Unit ID", description = "A database generated unique identifier for experimental observation units")
-    private String ObsUnitID;
+    @ImportFieldMetadata(id="obsUnitID", name="Observation Unit ID", description = "A database generated unique identifier for experimental observation units")
+    private String obsUnitID;
 
     public BrAPITrial constructBrAPITrial(Program program, boolean commit, String referenceSource, UUID id, String expSeqValue) {
         BrAPIProgram brapiProgram = program.getBrapiProgram();
@@ -255,6 +255,10 @@ public BrAPIObservationUnit constructBrAPIObservationUnit(
             observationUnit.setTreatments(List.of(treatment));
         }
 
+        if (getObsUnitID() != null) {
+            observationUnit.setObservationUnitDbId(getObsUnitID());
+        }
+
         return observationUnit;
     }
 

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

@@ -27,6 +27,7 @@
 import org.breedinginsight.model.Program;
 import org.breedinginsight.model.User;
 import org.breedinginsight.services.exceptions.DoesNotExistException;
+import org.breedinginsight.services.exceptions.MissingRequiredInfoException;
 import org.breedinginsight.services.exceptions.UnprocessableEntityException;
 import org.breedinginsight.services.exceptions.ValidatorException;
 import tech.tablesaw.api.Table;
@@ -40,10 +41,10 @@
 @Slf4j
 public class GermplasmImportService extends BrAPIImportService {
 
-    private String IMPORT_TYPE_ID = "GermplasmImport";
+    private final String IMPORT_TYPE_ID = "GermplasmImport";
 
-    private Provider<GermplasmProcessor> germplasmProcessorProvider;
-    private Provider<ProcessorManager> processorManagerProvider;
+    private final Provider<GermplasmProcessor> germplasmProcessorProvider;
+    private final Provider<ProcessorManager> processorManagerProvider;
 
     @Inject
     public GermplasmImportService(Provider<GermplasmProcessor> germplasmProcessorProvider,
@@ -65,7 +66,7 @@ public String getImportTypeId() {
 
     @Override
     public ImportPreviewResponse process(List<BrAPIImport> brAPIImports, Table data, Program program, ImportUpload upload, User user, Boolean commit)
-            throws UnprocessableEntityException, DoesNotExistException, ValidatorException, ApiException {
+            throws UnprocessableEntityException, DoesNotExistException, ValidatorException, ApiException, MissingRequiredInfoException {
 
         ImportPreviewResponse response = null;
         List<Processor> processors = List.of(germplasmProcessorProvider.get());

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

@@ -67,17 +67,17 @@
 @Singleton
 public class FileImportService {
 
-    private ProgramUserService programUserService;
-    private ProgramService programService;
-    private UserService userService;
-    private MimeTypeParser mimeTypeParser;
-    private ImportMappingDAO importMappingDAO;
-    private ObjectMapper objectMapper;
-    private MappingManager mappingManager;
-    private ImportConfigManager configManager;
-    private ImportDAO importDAO;
-    private DSLContext dsl;
-    private ImportMappingProgramDAO importMappingProgramDAO;
+    private final ProgramUserService programUserService;
+    private final ProgramService programService;
+    private final UserService userService;
+    private final MimeTypeParser mimeTypeParser;
+    private final ImportMappingDAO importMappingDAO;
+    private final ObjectMapper objectMapper;
+    private final MappingManager mappingManager;
+    private final ImportConfigManager configManager;
+    private final ImportDAO importDAO;
+    private final DSLContext dsl;
+    private final ImportMappingProgramDAO importMappingProgramDAO;
 
     @Inject
     FileImportService(ProgramUserService programUserService, ProgramService programService, MimeTypeParser mimeTypeParser,
@@ -425,7 +425,14 @@ private void processFile(List<BrAPIImport> finalBrAPIImportList, Table data, Pro
                 progress.setMessage(e.getMessage());
                 progress.setUpdatedBy(actingUser.getId());
                 importDAO.update(upload);
-            } catch (ValidatorException e) {
+            } catch (MissingRequiredInfoException e) {
+                log.error(e.getMessage(), e);
+                ImportProgress progress = upload.getProgress();
+                progress.setStatuscode((short) HttpStatus.UNPROCESSABLE_ENTITY.getCode());
+                progress.setMessage(e.getMessage());
+                progress.setUpdatedBy(actingUser.getId());
+                importDAO.update(upload);
+            }catch (ValidatorException e) {
                 log.info("Validation errors", e);
                 ImportProgress progress = upload.getProgress();
                 progress.setStatuscode((short) HttpStatus.UNPROCESSABLE_ENTITY.getCode());

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

@@ -45,6 +45,7 @@
 import org.breedinginsight.brapps.importer.services.ExternalReferenceSource;
 import org.breedinginsight.model.Program;
 import org.breedinginsight.model.User;
+import org.breedinginsight.services.exceptions.MissingRequiredInfoException;
 import org.breedinginsight.services.exceptions.ValidatorException;
 import org.breedinginsight.utilities.Utilities;
 import org.jooq.DSLContext;
@@ -60,23 +61,27 @@
 public class ExperimentProcessor implements Processor {
 
     private static final String NAME = "Experiment";
+    private static final String MISSING_OBS_UNIT_ID_ERROR = "Experiment Units are missing Observation Unit Id.\n" +
+            "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).";
 
     @Property(name = "brapi.server.reference-source")
     private String BRAPI_REFERENCE_SOURCE;
 
 
-    private DSLContext dsl;
-    private BrAPITrialDAO brapiTrialDAO;
-    private BrAPILocationDAO brAPILocationDAO;
-    private BrAPIStudyDAO brAPIStudyDAO;
-    private BrAPIObservationUnitDAO brAPIObservationUnitDAO;
-    private BrAPISeasonDAO brAPISeasonDAO;
-    private BrAPIGermplasmDAO brAPIGermplasmDAO;
+    private final DSLContext dsl;
+    private final BrAPITrialDAO brapiTrialDAO;
+    private final BrAPILocationDAO brAPILocationDAO;
+    private final BrAPIStudyDAO brAPIStudyDAO;
+    private final BrAPIObservationUnitDAO brAPIObservationUnitDAO;
+    private final BrAPISeasonDAO brAPISeasonDAO;
+    private final BrAPIGermplasmDAO brAPIGermplasmDAO;
 
     // used to make the yearsToSeasonDbId() function more efficient
-    private Map<String, String > yearToSeasonDbIdCache = new HashMap<>();
+    private final Map<String, String > yearToSeasonDbIdCache = new HashMap<>();
     // used to make the seasonDbIdtoYear() function more efficient
-    private Map<String, String > seasonDbIdToYearCache = new HashMap<>();
+    private final Map<String, String > seasonDbIdToYearCache = new HashMap<>();
 
     //These BrapiData-objects are initially populated by the getExistingBrapiData() method,
     // then updated by the getNewBrapiData() method.
@@ -142,7 +147,7 @@ public Map<String, ImportPreviewStatistics> process(
             Map<Integer, PendingImport> mappedBrAPIImport,
             Program program,
             User user,
-            boolean commit) throws ValidatorException {
+            boolean commit) throws ValidatorException, MissingRequiredInfoException {
 
         ValidationErrors validationErrors = new ValidationErrors();
 
@@ -164,6 +169,12 @@ public Map<String, ImportPreviewStatistics> process(
             if (! StringUtils.isBlank( importRow.getGid() )) { // if GID is blank, don't bother to check if it is valid.
                 validateGermplasm(importRow,validationErrors, i, germplasmPIO);
             }
+
+            //Check if existing environment. If so, ObsUnitId must be assigned
+            if ((this.studyByNameNoScope.get(importRow.getEnv()).getState() == ImportObjectState.EXISTING) && (StringUtils.isBlank(importRow.getObsUnitID()))){
+                throw new MissingRequiredInfoException(MISSING_OBS_UNIT_ID_ERROR);
+            }
+
             // Construct Observations -- Done in another card
             mappedBrAPIImport.put(i, mappedImportRow);
         }
@@ -730,7 +741,7 @@ private String yearToSeasonDbIdFromDatabase(String year, UUID programId) {
             }
 
         } catch (ApiException e) {
-            log.error(e.getResponseBody(), e);;
+            log.error(e.getResponseBody(), e);
         }
 
         String seasonDbId = (targetSeason==null) ? null : targetSeason.getSeasonDbId();
@@ -743,9 +754,9 @@ private String seasonDbIdToYearFromDatabase(String seasonDbId, UUID programId) {
         try {
             season = this.brAPISeasonDAO.getSeasonById (seasonDbId, programId);
         } catch (ApiException e) {
-            log.error(e.getResponseBody(), e);;
+            log.error(e.getResponseBody(), e);
         }
-        Integer yearInt = (season == null) ? null : season.getYear();;
+        Integer yearInt = (season == null) ? null : season.getYear();
         String yearStr = (yearInt==null) ? "" : yearInt.toString();
         return yearStr;
     }

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

@@ -23,6 +23,7 @@
 import org.breedinginsight.brapps.importer.model.response.ImportPreviewStatistics;
 import org.breedinginsight.model.Program;
 import org.breedinginsight.model.User;
+import org.breedinginsight.services.exceptions.MissingRequiredInfoException;
 import org.breedinginsight.services.exceptions.ValidatorException;
 
 import java.util.List;
@@ -49,7 +50,7 @@ public interface Processor {
      */
     Map<String, ImportPreviewStatistics> process(List<BrAPIImport> importRows,
                                                  Map<Integer, PendingImport> mappedBrAPIImport,
-                                                 Program program, User user, boolean commit) throws ValidatorException;
+                                                 Program program, User user, boolean commit) throws ValidatorException, MissingRequiredInfoException;
 
     /**
      * Given mapped brapi import with updates from prior dependencies, check if have everything needed

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

@@ -27,6 +27,7 @@
 import org.breedinginsight.brapps.importer.services.ImportStatusService;
 import org.breedinginsight.model.Program;
 import org.breedinginsight.model.User;
+import org.breedinginsight.services.exceptions.MissingRequiredInfoException;
 import org.breedinginsight.services.exceptions.ValidatorException;
 
 import javax.inject.Inject;
@@ -40,9 +41,9 @@
 public class ProcessorManager {
 
     private List<Processor> processors;
-    private Map<Integer, PendingImport> mappedBrAPIImport;
-    private ImportStatusService statusService;
-    private Map<String, ImportPreviewStatistics> statistics = new HashMap<>();
+    private final Map<Integer, PendingImport> mappedBrAPIImport;
+    private final ImportStatusService statusService;
+    private final Map<String, ImportPreviewStatistics> statistics = new HashMap<>();
 
     @Inject
     public ProcessorManager(ImportStatusService statusService) {
@@ -51,7 +52,7 @@ public ProcessorManager(ImportStatusService statusService) {
         this.statusService = statusService;
     }
 
-    public ImportPreviewResponse process(List<BrAPIImport> importRows, List<Processor> processors, Program program, ImportUpload upload, User user, boolean commit) throws ValidatorException, ApiException {
+    public ImportPreviewResponse process(List<BrAPIImport> importRows, List<Processor> processors, Program program, ImportUpload upload, User user, boolean commit) throws ValidatorException, ApiException, MissingRequiredInfoException {
 
         this.processors = processors;
         statusService.setUpload(upload);

src/main/resources/db/migration/V1.0.5__experiment_template_obsUnitID.sql

@@ -0,0 +1,27 @@
+/*
+ * See the NOTICE file distributed with this work for additional information
+ * regarding copyright ownership.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+DO $$
+
+BEGIN
+-- fix file mapping for ObsUnitID
+update importer_mapping
+set
+    mapping = '[{"id": "726a9f10-4892-4204-9e52-bd2b1d735f65", "value": {"fileFieldName": "Germplasm Name"}, "objectId": "germplasmName"}, {"id": "98774e20-6f89-4d6a-a7c9-f88887228ed6", "value": {"fileFieldName": "Germplasm GID"}, "objectId": "gid"}, {"id": "880ef0c9-4e3e-42d4-9edc-667684a91889", "value": {"fileFieldName": "Test (T) or Check (C )"}, "objectId": "test_or_check"}, {"id": "b693eca7-efcd-4518-a9d3-db0b037a76ee", "value": {"fileFieldName": "Exp Title"}, "objectId": "exp_title"}, {"id": "df340215-db6e-4219-a3b7-119f297b81c3", "value": {"fileFieldName": "Exp Description"}, "objectId": "expDescription"}, {"id": "9ca7cc81-562c-43a7-989a-41da309f603d", "value": {"fileFieldName": "Exp Unit"}, "objectId": "expUnit"}, {"id": "27215777-c8f9-4fe7-a7ac-918d6168b0dd", "value": {"fileFieldName": "Exp Type"}, "objectId": "expType"}, {"id": "19d220e2-dff0-4a3a-ad6e-32f4d8602b5c", "value": {"fileFieldName": "Env"}, "objectId": "env"}, {"id": "861518b9-5c9e-4fe5-b31e-baf16e27155d", "value": {"fileFieldName": "Env Location"}, "objectId": "envLocation"}, {"id": "667355c3-dae1-4a64-94c8-ac2d543bd474", "value": {"fileFieldName": "Env Year"}, "objectId": "envYear"}, {"id": "ad11f2df-c5b4-4a05-8e52-c57625140061", "value": {"fileFieldName": "Exp Unit ID"}, "objectId": "expUnitId"}, {"id": "639b40ec-20f8-4659-8464-6a4be997ac7a", "value": {"fileFieldName": "Exp Replicate #"}, "objectId": "expReplicateNo"}, {"id": "2a62a80f-d8ba-42c4-9997-3b4ac8a965aa", "value": {"fileFieldName": "Exp Block #"}, "objectId": "expBlockNo"}, {"id": "f3e7de69-21ad-4cda-b1cc-a5e1987fb931", "value": {"fileFieldName": "Row"}, "objectId": "row"}, {"id": "251c5bbd-fc4d-4371-a4ce-4e2686f6837e", "value": {"fileFieldName": "Column"}, "objectId": "column"}, {"id": "ce5f61f2-f1de-45a4-8baf-e2471a5d863d", "value": {"fileFieldName": "Treatment Factors"}, "objectId": "treatmentFactors"}, {"id": "c5b8276f-e777-4385-a80f-5199abe63aac", "value": {"fileFieldName": "ObsUnitID"}, "objectId": "obsUnitID"}]',
+    file    = '[{"Env": "New Study", "Row": 4, "Column": 5, "Env Year": 2012, "Exp Type": "phenotyping", "Exp Unit": "plot", "Exp Title": "New Trial", "ObsUnitID": "", "Exp Block #": 2, "Exp Unit ID": 3, "Env Location": "New Location", "Germplasm GID": 1, "Germplasm Name": "Test", "Exp Description": "A new trial", "Exp Replicate #": 0, "Treatment Factors": "Jam application", "Test (T) or Check (C )": true}]'
+where import_type_id = 'ExperimentImport';
+END $$;