[BI-2110] New Append Exp Validations #483

dmeidlin · 2025-08-08T14:50:08Z

Description

Story: BI-2110

Summarize these file changes made for a pull request:

This pull request introduces several major improvements and fixes related primarily to enhancing validation and import workflow handling for appending and overwriting experimental data, particularly around observation variables and observation units for datasets. Here's a summary of the key changes:

New Tests Added:
- Added new integration tests for appending observation data using prior variables from earlier datasets.
- Tests for appending data to multiple datasets and validating observation variables are unique within datasets.
- Various test updates modifying calls to import utilities to include new parameters indicating sub-entity level and observation unit ID presence.
Append Overwrite Validation Middleware Enhancements:
- Added a new middleware for validating phenotype (observation variable) columns (AppendOverwriteVariableValidation).
- Existing append overwrite ID validation middleware enhanced to execute observation unit ID validation after fetching observation units from the BrAPI service.
Observation Unit ID Validators:
- Refactored observation unit ID validators under a new interface DynamicObsUnitValidator (replacing DynamicColumnValidator for observation units).
- Validators for duplicate IDs, ID formats, blank IDs, and column name validation were updated to this new interface.
- Added a new validator ObservationUnitSingleDatasetValidator that ensures observation units used in the import belong to a single dataset (prevents mixing units from multiple datasets in one import).
- Validators are smartly skipped if observation units have already been fetched to avoid redundant checks.
Observation Variable Validators:
- Introduced a similar interface DynamicObsVarValidator for observation variable validators.
- Added validators for:
  - Ensuring observation variables are not reused from prior datasets in the same experiment (ObservationVariablePriorDatasetValidator).
  - Validating timestamp columns associated with observation variables (ObservationVariableTimestampValidator).
- Created a main ObservationVariableValidator that chains multiple dynamic observation variable validators.
Pending Dataset Entity Updates:
- Modified PendingDataset to fetch datasets by IDs and support more robust dataset retrieval with associated program scoping.
- Corrected handling when no pending dataset or trial entries are found, avoiding exceptions.
- Improved the lookup of pending datasets by observation unit IDs for validation.
Experiment Import Processing:
- Revised import table processing to:
  - Use cached data and previous validations for observation variables and timestamps.
  - Adopt more accurate retrieval of pending datasets and properly handle their phenotype lists.
  - Fix issues regarding missing or duplicated observation unit columns.
- Improved validation error handling during import preview stages in the append/overwrite workflows.
Bug Fixes and Refactorings:
- Fixed a bug in observation variable validator that incorrectly skipped some validations based on mappings.
- Adjusted constants and error messages for clarity and correct usage (e.g., consistent naming for "Sub Unit ID").
- Improved utility methods and error propagation to prevent crashing and better reporting.
- Renamed some classes and interfaces related to observation unit validators for clarity.
Workflow Integration:
- Integrated the new observation variable validation middleware into the append/overwrite phenotypes workflow to enforce all validations before import preview and commit.
- Removed redundant data fetching workflow initialization to avoid unnecessary API calls.

Overall, this PR significantly strengthens validation logic for experiment data import workflows, ensuring observation units and variables are properly validated for duplication, formatting, dataset association, and timestamp matching. It also improves test coverage for these scenarios and refines core import processing to handle them correctly.

Dependencies

biweb:develop
Note: you can optionally use the fix in biweb:feature/BI-2628-2 to display the correct number of obs var columns in the preview import statistics.

Testing

Setup

Import ontology BI-2110_ontology_import.xlsx.
Import germplasm BI-2110_germplasm_import.xlsx.
Using the experiment create workflow, create an experiment that includes a single observation variable BI-2110_plot_exp_import.xls.
In the experiments table of the UI, select the experiment.
In the experiment view, select Manage Experiment and select "Create Sub-Entity Dataset", then create a subentity and enter "plant" for the Sub-Entity Name and 2 for the Repeated Measures
Repeat step 5 but enter "tree" for the Sub-Entity Name.

Prior Observation Variable Validation

Using the append workflow, import observations for the plant sub-entity using the same observation variable that was used during the experiment creation BI-2110_plant_append_data.xlsx.
Verify that a banner error is displayed with the message, "Observation variable(s) are already associated with another dataset(s) in this experiment".

Single Dataset Validation

Using the append workflow, import observations for both plant and tree datasets using an observation variable that was not used during the experiment creation BI-2110_plant_and_tree_append.xlsx.
Verify that a banner error is displayed with the message, "Required field is blank". Note: this message conforms to the source-of-truth doc, but this doc is currently under review to update this message to something more accurate for this validation step.

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>

…asets

nickpalladino

I have a few questions about things I saw when testing.

If the "Sub Obs Unit" ObsUnitID was invalid (missing or not a UUID) I was getting an error banner with An unknown error has occurred when processing your import. I was expecting a more descriptive message, like something about the UUID being invalid. Here's the stack trace:

2025-08-19 15:39:02.094-0400 [ForkJoinPool.commonPool-worker-3] ERROR o.b.b.i.services.FileImportService - null
java.lang.NullPointerException: null
	at org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.factory.entity.PendingStudy.brapiRead(PendingStudy.java:78)
	at org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.factory.action.WorkflowReadInitialization.execute(WorkflowReadInitialization.java:51)
	at org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.middleware.initialize.WorkflowInitialization.process(WorkflowInitialization.java:59)
	at org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.middleware.initialize.WorkflowInitialization.process(WorkflowInitialization.java:39)
	at org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.AppendOverwritePhenotypesWorkflow.process(AppendOverwritePhenotypesWorkflow.java:127)
	at org.breedinginsight.brapps.importer.services.processors.experiment.ExperimentWorkflowNavigator.lambda$process$0(ExperimentWorkflowNavigator.java:60)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
	at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1599)
	at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:127)
	at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:502)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:488)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
	at java.base/java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:150)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.findFirst(ReferencePipeline.java:543)
	at org.breedinginsight.brapps.importer.services.processors.experiment.ExperimentWorkflowNavigator.process(ExperimentWorkflowNavigator.java:63)
	at org.breedinginsight.brapps.importer.model.imports.DomainImportService.process(DomainImportService.java:59)
	at org.breedinginsight.brapps.importer.services.FileImportService.lambda$processFile$8(FileImportService.java:423)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1771)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1763)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1016)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1665)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1598)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

When adding a new Sub Obs Unit variable and appending new observation values the front-end (feature/BI-2628-2) shows 2 for observation variables in the header (think it should be 1 unless it's including the Exp Unit level variable as well?) and doesn't show the table. The api response for GET /v1/programs/8051df4f-d40f-43c9-8fc0-f6500eefebb0/import/mappings/c3a921ad-f35e-402c-b1f2-4678810c14d3/data/41d4401a-95f0-4ee3-96a2-7e918c581351?mapping=true has "Sub Unit ID" as one of the entries in the dynamicColumnNames array. I'm guessing that's because we don't have "Sub Unit ID" in the database import mapping. I was also seeing some stuff in the console:

[Vue warn]: Error in render: "TypeError: can't access property "state", row.data.observations[this.observationIndexMap.get(...)] is undefined"

found in

---> <BTable> at src/views/PageNotFound.vue
       <ExpandableTable> at src/components/tables/expandableTable/ExpandableTable.vue
         <ImportTemplate> at src/views/import/ImportTemplate.vue
           <ImportExperiment> at src/views/import/ImportExperiment.vue
             <ImportFile> at src/views/import/ImportFile.vue
               <SideBarMaster> at src/components/layouts/BaseSideBarLayout.vue
                 <UserSideBarLayout> at src/components/layouts/UserSideBarLayout.vue
                   <App> at src/App.vue
                     <Root> vue.runtime.esm.js:4662:21
TypeError: can't access property "state", row.data.observations[this.observationIndexMap.get(...)] is undefined
    cellClassIfExisting webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportExperiment.vue?vue&type=script&lang=ts&:256
    getRootClasses webpack-internal:///./node_modules/buefy/dist/esm/table.js:272
    2 webpack-internal:///./node_modules/buefy/dist/esm/table.js:1551
    renderList VueJS
    2 webpack-internal:///./node_modules/buefy/dist/esm/table.js:1551
    renderList VueJS
    __vue_render__$2 webpack-internal:///./node_modules/buefy/dist/esm/table.js:1548
    VueJS 13
    statisticsLoaded webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportExperiment.vue?vue&type=script&lang=ts&:228
    VueJS 4
    _callee8$ webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:697
    f webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    A webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    n webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    Babel 6
    getDataUpload webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:731
    _callee8$ webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:679
    f webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    A webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    n webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    Babel 8
    getDataUpload webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:731
    _callee7$ webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:626
    f webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    A webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    n webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    Babel 6
    updateDataUpload webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:637
    _callee2$ webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:283
    f webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    A webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    n webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    Babel 6
    upload webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:338
    actions webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:184
    s webpack-internal:///./node_modules/@xstate/fsm/es/index.js:20
vue.runtime.esm.js:3104:17
[Vue warn]: Error in render: "TypeError: can't access property "state", row.data.observations[this.observationIndexMap.get(...)] is undefined"

found in

---> <BTable> at src/views/PageNotFound.vue
       <ExpandableTable> at src/components/tables/expandableTable/ExpandableTable.vue
         <ImportTemplate> at src/views/import/ImportTemplate.vue
           <ImportExperiment> at src/views/import/ImportExperiment.vue
             <ImportFile> at src/views/import/ImportFile.vue
               <SideBarMaster> at src/components/layouts/BaseSideBarLayout.vue
                 <UserSideBarLayout> at src/components/layouts/UserSideBarLayout.vue
                   <App> at src/App.vue
                     <Root> vue.runtime.esm.js:4662:21
TypeError: can't access property "state", row.data.observations[this.observationIndexMap.get(...)] is undefined
    cellClassIfExisting webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportExperiment.vue?vue&type=script&lang=ts&:256
    getRootClasses webpack-internal:///./node_modules/buefy/dist/esm/table.js:272
    2 webpack-internal:///./node_modules/buefy/dist/esm/table.js:1551
    renderList VueJS
    2 webpack-internal:///./node_modules/buefy/dist/esm/table.js:1551
    renderList VueJS
    __vue_render__$2 webpack-internal:///./node_modules/buefy/dist/esm/table.js:1548
    VueJS 14
    _callee2$ webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:303
    f webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    A webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    n webpack-internal:///./node_modules/vue-breakpoint-component/dist/vue-breakpoint-component.umd.min.js:9
    Babel 8
    upload webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:338
    actions webpack-internal:///./node_modules/cache-loader/dist/cjs.js?!./node_modules/babel-loader/lib/index.js!./node_modules/ts-loader/index.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=script&lang=ts&:184
    s webpack-internal:///./node_modules/@xstate/fsm/es/index.js:20
    s webpack-internal:///./node_modules/@xstate/fsm/es/index.js:20
    send webpack-internal:///./node_modules/@xstate/fsm/es/index.js:20
    import webpack-internal:///./node_modules/cache-loader/dist/cjs.js?{"cacheDirectory":"node_modules/.cache/vue-loader","cacheIdentifier":"69a64f48-vue-loader-template"}!./node_modules/vue-loader/lib/loaders/templateLoader.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/views/import/ImportTemplate.vue?vue&type=template&id=755668df&:147
    VueJS 4
    click webpack-internal:///./node_modules/cache-loader/dist/cjs.js?{"cacheDirectory":"node_modules/.cache/vue-loader","cacheIdentifier":"69a64f48-vue-loader-template"}!./node_modules/vue-loader/lib/loaders/templateLoader.js?!./node_modules/cache-loader/dist/cjs.js?!./node_modules/vue-loader/lib/index.js?!./src/components/file-import/FileSelectMessageBox.vue?vue&type=template&id=42958905&:62
    VueJS 33
vue.runtime.esm.js:3104:17

HMS17 · 2025-09-15T21:29:06Z

Current status:

Added:

Fix to get unit tests running
Fix to the unknown error issue Nick mentioned upon using an invalid ObsUnitID
Fix to enable append for experiments with more than one germplasm (also fixes QA issue for BI-2628)

Still TODO:

Fix the issue with the lack of SubUnitID not being in the database import mapping
Investigate if the console error Nick noticed on the append workflow still exists after current fixes

nickpalladino · 2025-11-21T21:55:28Z

.../org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java

+        // Extract the environment and experimental unit ID and germplasm GID from the ExperimentObservation object
        // and pass them to the createObservationUnitKey method
-        return createObservationUnitKey(importRow.getEnv(), importRow.getExpUnitId());
+        return createObservationUnitKey(importRow.getEnv(), importRow.getExpUnitId(), importRow.getGid());


Wouldn't env, exp unit id, and gid all be the same for the sub entity case? If there were 5 repeats, ex:
env, exp unit id, gid
CA, 1, 1
CA, 1, 1
CA, 1, 1
CA, 1, 1
CA, 1, 1

We will have sub unit id available once it's added to the download file that would uniquely identify each row.

dmeidlin added 6 commits June 30, 2025 16:20

add test for appending obs data using prior variables

5af44b9

add test for appending data to multiple datasets

b053edf

create validation check for single dataset used in append workflow

bb70d3d

create validator for ensuring obs vars are not re-used from other dat…

f5b26d1

…asets

create interface for obs var validator

3ef1742

fix bug in obsvar validator

2302e6d

github-actions bot added the feature label Aug 8, 2025

dmeidlin requested review from a team, HMS17 and nickpalladino and removed request for a team August 8, 2025 15:01

dmeidlin assigned nickpalladino Aug 8, 2025

HMS17 approved these changes Aug 13, 2025

View reviewed changes

nickpalladino reviewed Aug 20, 2025

View reviewed changes

HMS17 and others added 4 commits August 29, 2025 11:29

Merge branch 'develop' into feature/BI-2110

fc318ce

[BI-2110] - Unit Test Fix

f13bbe7

[BI-2110] - Validation Fix, Enabled Append when more than one germplasm

6f760ae

[BI-2110] - Remove unneeded todos

9616246

davedrp requested a review from nickpalladino October 21, 2025 17:24

HMS17 and others added 5 commits November 6, 2025 11:23

[BI-2110] Missing check on validation

de0a598

[BI-2110] Added missing imports

d63ca33

[BI-2110] - Adding GID to key generation

a4bf18a

[BI-2110] - Changed validation handling to avoid nullpointerexception

adce131

Merge branch 'develop' into feature/BI-2110

e45604e

nickpalladino reviewed Nov 21, 2025

View reviewed changes

nickpalladino approved these changes Nov 24, 2025

View reviewed changes

nickpalladino merged commit 0b5b602 into develop Nov 25, 2025
6 checks passed

nickpalladino deleted the feature/BI-2110 branch November 25, 2025 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[BI-2110] New Append Exp Validations #483

[BI-2110] New Append Exp Validations #483

Uh oh!

dmeidlin commented Aug 8, 2025 •

edited

Loading

Uh oh!

nickpalladino left a comment

Uh oh!

HMS17 commented Sep 15, 2025

Uh oh!

nickpalladino Nov 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[BI-2110] New Append Exp Validations #483

[BI-2110] New Append Exp Validations #483

Uh oh!

Conversation

dmeidlin commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Summarize these file changes made for a pull request:

Dependencies

Testing

Setup

Prior Observation Variable Validation

Single Dataset Validation

Checklist:

Uh oh!

nickpalladino left a comment

Choose a reason for hiding this comment

Uh oh!

HMS17 commented Sep 15, 2025

Uh oh!

nickpalladino Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dmeidlin commented Aug 8, 2025 •

edited

Loading