Skip to content

[BI-2009] - Dynamic Concatenation of Entity + ObsUnitID (Exp UI)#458

Merged
HMS17 merged 37 commits intofuture/1.2from
feature/BI-2009
May 7, 2025
Merged

[BI-2009] - Dynamic Concatenation of Entity + ObsUnitID (Exp UI)#458
HMS17 merged 37 commits intofuture/1.2from
feature/BI-2009

Conversation

@HMS17
Copy link
Contributor

@HMS17 HMS17 commented Apr 22, 2025

Description

Story: BI-2009 - Dynamic Concatenation of Entity + ObsUnitID (Exp UI)

Modifications to allow appending of respective observation level to ObsUnitID column when generating experiment file for export.

Added dynamicUpdateObsUnitIDLabel method to BrAPITrialService.java to prefix the column header in the exported file with the observation level
Overrode equals and hashCode for Column class to aid in modifying the correct column
Overrode hashCode for Country class due to noticing it lacking one when working on the Column hashCode override

Updated unit tests to account for changed column name in export

Dependencies

bi-web: feature/BI-2009

Testing

see bi-web

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>

@HMS17 HMS17 marked this pull request as ready for review May 1, 2025 16:47
@HMS17 HMS17 requested review from a team, davedrp and dmeidlin and removed request for a team May 1, 2025 16:47
davedrp

This comment was marked as outdated.

@HMS17 HMS17 requested a review from davedrp May 5, 2025 15:57
Copy link
Contributor

@dmeidlin dmeidlin left a comment

Choose a reason for hiding this comment

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

The code in place looks good and fulfills the requirements of the card; however, changes to be made in 1.2 to the importer to support sub-entities will cause the export to break as implemented.

The issue is that neither the Create or Append workflows for importing experiments will use a column named ObsUnitID, so it will be removed from ExperimentObservation. For the export feature then, the dynamic observation unit column will need to be handled in a similar way to other dynamic columns like observation variables since ObsUnitID will no longer be a required static column.

ObsUnitID will be removed from ExpFileCOlumns and from ExperimentObservation.Columns, so in BrAPITrialService#ExportObservations the ObsUnitId column will not appear as an expected column here, and also in BrAPITrialService#createExportRow here.

BrAPITrtialService#dynamicUpdateObsUnitIDLabel will need to be changed from a method that mutates an existing column to a method that creates a new column, similar to BrAPITrialService#addObsVarColumns.

@dmeidlin
Copy link
Contributor

dmeidlin commented May 5, 2025

This branch doesn't have the new page size limit of 65,000 set in BrAPIDAOUtil that is needed for the brapi server develop branch, so I'm getting errors while running bi-api. We should probable rebase future1.2 so that it has Jason's changes.

@HMS17
Copy link
Contributor Author

HMS17 commented May 6, 2025

The code in place looks good and fulfills the requirements of the card; however, changes to be made in 1.2 to the importer to support sub-entities will cause the export to break as implemented.

The issue is that neither the Create or Append workflows for importing experiments will use a column named ObsUnitID, so it will be removed from ExperimentObservation. For the export feature then, the dynamic observation unit column will need to be handled in a similar way to other dynamic columns like observation variables since ObsUnitID will no longer be a required static column.

ObsUnitID will be removed from ExpFileCOlumns and from ExperimentObservation.Columns, so in BrAPITrialService#ExportObservations the ObsUnitId column will not appear as an expected column here, and also in BrAPITrialService#createExportRow here.

BrAPITrtialService#dynamicUpdateObsUnitIDLabel will need to be changed from a method that mutates an existing column to a method that creates a new column, similar to BrAPITrialService#addObsVarColumns.

https://breedinginsight.atlassian.net/browse/BI-2631 new card made to handle these changes to limit scope of this card

@HMS17 HMS17 merged commit faa5e69 into future/1.2 May 7, 2025
1 check passed
@HMS17 HMS17 deleted the feature/BI-2009 branch May 7, 2025 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants