Skip to content

[BI-2778] Deprecate additionalinfo usage for level and use obs unit lvl #500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HMS17 wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

[BI-2778] Deprecate additionalinfo usage for level and use obs unit lvl #500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ddaf750
[BI-2778] Deprecate additionalinfo usage for level and use obs unit l…
HMS17 Mar 6, 2026
5379e4a
[BI-2778] Tests casing fix
HMS17 Mar 6, 2026
7074247
[BI-2778] Export file casing fix
HMS17 Mar 6, 2026
d5e532c
[BI-2778] Fixes
HMS17 Mar 6, 2026
81a7a85
[BI-2778] Casing test
HMS17 Mar 6, 2026
e4cae76
[BI-2778] Another casing fix
HMS17 Mar 6, 2026
341c310
[BI-2778] Null pointer fix re treatment factors
HMS17 Mar 6, 2026
7db5470
[BI-2778] Test fix
HMS17 Mar 9, 2026
a80a381
[BI-2778] Unit test experiment
HMS17 Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ public final class BrAPIAdditionalInfoFields {
public static final String GERMPLASM_BREEDING_METHOD = "breedingMethod";
public static final String CREATED_DATE = "createdDate";
public static final String DEFAULT_OBSERVATION_LEVEL = "defaultObservationLevel";
public static final String OBSERVATION_LEVEL = "observationLevel";
public static final String RTK = "rtk";
public static final String EXPERIMENT_TYPE = "experimentType";
public static final String EXPERIMENT_NUMBER = "experimentNumber";
Expand Down
22 changes: 8 additions & 14 deletions src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public DownloadFile exportObservations(
columns = dynamicUpdateObsUnitIDLabel(columns, topObservationLvl);
}
}
String observationLvl = ous.get(0).getAdditionalInfo().get(BrAPIAdditionalInfoFields.OBSERVATION_LEVEL).getAsString();
String observationLvl = StringUtils.capitalize(ous.get(0).getObservationUnitPosition().getObservationLevel().getLevelName());
columns = dynamicUpdateObsUnitIDLabel(columns, observationLvl);

if (params.getDatasetId() != null) {
Expand Down Expand Up @@ -576,7 +576,7 @@ public BrAPIObservationUnit createSubObservationUnit(
}

// Set treatment factors.
if (!expUnit.getTreatments().isEmpty()) {
if (!(expUnit.getTreatments() == null || expUnit.getTreatments().isEmpty())) {
List<BrAPIObservationTreatment> treatmentFactors = new ArrayList<>();
for (BrAPIObservationTreatment t : expUnit.getTreatments()) {
BrAPIObservationTreatment treatment = new BrAPIObservationTreatment();
Expand All @@ -587,9 +587,6 @@ public BrAPIObservationUnit createSubObservationUnit(
observationUnit.setTreatments(treatmentFactors);
}

// Put level in additional info: keep this in case we decide to rename levels in future.
observationUnit.putAdditionalInfoItem(BrAPIAdditionalInfoFields.OBSERVATION_LEVEL, subEntityDatasetName);

// Put RTK in additional info.
JsonElement rtk = expUnit.getAdditionalInfo().get(BrAPIAdditionalInfoFields.RTK);
if (rtk != null) {
Expand Down Expand Up @@ -641,16 +638,13 @@ public BrAPIObservationUnit createSubObservationUnit(
}
// ObservationLevelRelationships for top-level Exp Unit linking.
BrAPIObservationUnitLevelRelationship expUnitLevel = new BrAPIObservationUnitLevelRelationship();
// TODO: switch to using level name in main obs unit properties once dynamic obs unit support is complete
expUnitLevel.setLevelName(expUnit.getAdditionalInfo().get(BrAPIAdditionalInfoFields.OBSERVATION_LEVEL).getAsString().toLowerCase());
expUnitLevel.setLevelName(expUnit.getObservationUnitPosition().getObservationLevel().getLevelName().toLowerCase());
String expUnitUUID = Utilities.getExternalReference(expUnit.getExternalReferences(), referenceSource, ExternalReferenceSource.OBSERVATION_UNITS).orElseThrow().getReferenceId();
expUnitLevel.setLevelCode(Utilities.appendProgramKey(expUnitUUID, program.getKey(), seqVal));
expUnitLevel.setLevelOrder(DatasetLevel.EXP_UNIT.getValue());
levelRelationships.add(expUnitLevel);
position.setObservationLevelRelationships(levelRelationships);

observationUnit.putAdditionalInfoItem(BrAPIAdditionalInfoFields.EXP_UNIT_ID, expUnit.getObservationUnitName());

// Set ObservationUnitPosition.
observationUnit.setObservationUnitPosition(position);

Expand Down Expand Up @@ -884,7 +878,7 @@ private Map<String, Object> createExportRow(
}

//Append observation level to obsUnitID
String observationLvl = ou.getAdditionalInfo().getAsJsonObject().get(BrAPIAdditionalInfoFields.OBSERVATION_LEVEL).getAsString();
String observationLvl = StringUtils.capitalize(ou.getObservationUnitPosition().getObservationLevel().getLevelName());
row.put(observationLvl + " " + OBSERVATION_UNIT_ID_SUFFIX, ouId);

if (isSubEntity) {
Expand All @@ -897,13 +891,13 @@ private Map<String, Object> createExportRow(
String topLvlOuId = Utilities.removeProgramKeyAndUnknownAdditionalData(topLevel.getLevelCode(), program.getKey());
row.put(topLvlName + " " + OBSERVATION_UNIT_ID_SUFFIX, topLvlOuId);
}
row.put(ExperimentObservation.Columns.EXP_UNIT_ID, ou.getAdditionalInfo().get(BrAPIAdditionalInfoFields.EXP_UNIT_ID).getAsString());
row.put(ExperimentObservation.Columns.EXP_UNIT_ID, StringUtils.capitalize(getTopLevel(ou).getLevelName()));
Copy link
Collaborator

@jloux-brapi jloux-brapi Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok to swap the logic for exp unit id with level name?


row.put(ExperimentObservation.Columns.SUB_OBS_UNIT, ou.getAdditionalInfo().getAsJsonObject().get(BrAPIAdditionalInfoFields.OBSERVATION_LEVEL).getAsString());
row.put(ExperimentObservation.Columns.SUB_OBS_UNIT, StringUtils.capitalize(ou.getObservationUnitPosition().getObservationLevel().getLevelName()));
row.put(ExperimentObservation.Columns.SUB_UNIT_ID, Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getObservationUnitName(), program.getKey()));

} else {
row.put(ExperimentObservation.Columns.EXP_UNIT, ou.getAdditionalInfo().getAsJsonObject().get(BrAPIAdditionalInfoFields.OBSERVATION_LEVEL).getAsString());
row.put(ExperimentObservation.Columns.EXP_UNIT, StringUtils.capitalize(ou.getObservationUnitPosition().getObservationLevel().getLevelName()));
row.put(ExperimentObservation.Columns.EXP_UNIT_ID, Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getObservationUnitName(), program.getKey()));
}

Expand Down Expand Up @@ -995,7 +989,7 @@ private void sortDefaultForObservationUnit(List<BrAPIObservationUnit> ous) {

if (isSubEntityDataset(ous)) {
Comparator<BrAPIObservationUnit> subUnitComparator = Comparator.comparing(BrAPIObservationUnit::getObservationUnitName, new IntOrderComparator());
Comparator<BrAPIObservationUnit> ouNameComparator = Comparator.comparing(row -> (row.getAdditionalInfo().get(BrAPIAdditionalInfoFields.EXP_UNIT_ID).toString()), new IntOrderComparator());
Comparator<BrAPIObservationUnit> ouNameComparator = Comparator.comparing(row -> (getTopLevel(row).getLevelName()), new IntOrderComparator());
Copy link
Collaborator

@jloux-brapi jloux-brapi Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok to swap the logic for exp unit id with level name?

ous.sort((studyNameComparator).thenComparing(ouNameComparator).thenComparing(subUnitComparator));
}
else {
Expand Down
2 changes: 0 additions & 2 deletions ...inginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,12 +280,10 @@ public BrAPIObservationUnit constructBrAPIObservationUnit(
// If expUnit is null, a validation error will be produced later on.
if (getExpUnit() != null)
{
// TODO: [BI-2219] BJTS only accepts hardcoded levels, need to handle dynamic levels.
level.setLevelName(getExpUnit().toLowerCase()); // HACK: toLowerCase() is needed to match BJTS hardcoded levels.
}
level.setLevelCode(Utilities.appendProgramKey(getExpUnitId(), program.getKey(), seqVal));
position.setObservationLevel(level);
observationUnit.putAdditionalInfoItem(BrAPIAdditionalInfoFields.OBSERVATION_LEVEL, getExpUnit());

// Exp Unit
List<BrAPIObservationUnitLevelRelationship> levelRelationships = new ArrayList<>();
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,7 @@ private Map<String, Object> assertRowSaved(Map<String, Object> expected, Program
assertEquals(expected.get(Columns.EXP_TITLE), Utilities.removeProgramKey(study.getTrialName(), program.getKey()));
assertEquals(expected.get(Columns.EXP_DESCRIPTION), trial.getTrialDescription());
assertEquals(expected.get(Columns.EXP_UNIT), trial.getAdditionalInfo().get(BrAPIAdditionalInfoFields.DEFAULT_OBSERVATION_LEVEL).getAsString());
assertEquals(expected.get(Columns.EXP_UNIT), ou.getAdditionalInfo().get(BrAPIAdditionalInfoFields.OBSERVATION_LEVEL).getAsString());
assertEquals(expected.get(Columns.EXP_UNIT).toString().toLowerCase(), ou.getObservationUnitPosition().getObservationLevel().getLevelName().toLowerCase());
assertEquals(expected.get(Columns.EXP_TYPE), trial.getAdditionalInfo().get(BrAPIAdditionalInfoFields.EXPERIMENT_TYPE).getAsString());
assertEquals(expected.get(Columns.EXP_TYPE), study.getStudyType());
assertEquals(expected.get(Columns.ENV), Utilities.removeProgramKeyAndUnknownAdditionalData(study.getStudyName(), program.getKey()));
Expand Down Expand Up @@ -1808,7 +1808,7 @@ private Map<String, Object> assertValidPreviewRow(Map<String, Object> expected,
assertEquals(expected.get(Columns.EXP_TITLE), Utilities.removeProgramKey(study.getTrialName(), program.getKey()));
assertEquals(expected.get(Columns.EXP_DESCRIPTION), trial.getTrialDescription());
assertEquals(expected.get(Columns.EXP_UNIT), trial.getAdditionalInfo().get(BrAPIAdditionalInfoFields.DEFAULT_OBSERVATION_LEVEL).getAsString());
assertEquals(expected.get(Columns.EXP_UNIT), ou.getAdditionalInfo().get(BrAPIAdditionalInfoFields.OBSERVATION_LEVEL).getAsString());
assertEquals(expected.get(Columns.EXP_UNIT).toString().toLowerCase(), ou.getObservationUnitPosition().getObservationLevel().getLevelName().toLowerCase());
assertEquals(expected.get(Columns.EXP_TYPE), trial.getAdditionalInfo().get(BrAPIAdditionalInfoFields.EXPERIMENT_TYPE).getAsString());
assertEquals(expected.get(Columns.EXP_TYPE), study.getStudyType());
assertEquals(expected.get(Columns.ENV), Utilities.removeProgramKeyAndUnknownAdditionalData(study.getStudyName(), program.getKey()));
Expand Down
2 changes: 1 addition & 1 deletion src/test/resources/sql/ImportControllerIntegrationTest.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ INSERT INTO public.importer_mapping (name,import_type_id,mapping,file,draft,crea
INSERT INTO public.importer_mapping (name,import_type_id,mapping,file,draft,created_by,updated_by) values
('ExperimentsTemplateMap','ExperimentImport',
'[{"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"}]',
'[{"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}]',
'[{"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}]',
false,user_id,user_id);

END $$;
Expand Down
Loading