Skip to content

[BI-2028] Duplicated accession names getting assigned duplicated GIDs after Exp&Obs upload#320

Merged
dmeidlin merged 3 commits intodevelopfrom
bug/BI-2028
Dec 20, 2023
Merged

[BI-2028] Duplicated accession names getting assigned duplicated GIDs after Exp&Obs upload#320
dmeidlin merged 3 commits intodevelopfrom
bug/BI-2028

Conversation

@dmeidlin
Copy link
Contributor

@dmeidlin dmeidlin commented Dec 8, 2023

Description

Story: BI-2028

During experiment import, existing germplasm dbid was being assigned to OU PIO by looking up the germplasm name stored in the OU PIO. When there are multiple germplasm with the same name, this was causing the same germplasm to be assigned to multiple OUs.

  • Changed the OU PIO creation to include the germplasm GID as additional info
  • mapped the germplasm to the OU PIO using the unique GID instead of the non-unique germplasm name

Dependencies

none

Testing

  1. Import multiple germplasm with the same name
  2. Import an experiment using GIDs from step 1.
  3. After import successful, go to Experiment view and select the imported Experiment details
  4. Verify that the correct GID is displayed.
    See the card for sample import files.

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>

@dmeidlin dmeidlin requested review from a team, davedrp and nickpalladino and removed request for a team December 8, 2023 16:45
@github-actions github-actions bot added the bug Something isn't working label Dec 8, 2023
.filter(obsUnit -> germplasm.getAccessionNumber() != null &&
germplasm.getAccessionNumber().equals(obsUnit
.getBrAPIObject()
.getAdditionalInfo().getAsJsonObject()
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a migration to add gids to existing observation units additionalinfo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most existing OUs have the correct gemplasm dbId assigned, so a migration adding the GID wouldn't matter either way for them. It's a question more for Alex and Shawn to estimate how many cases of existing OUs using germplasm there are. They haven't com across this until now, so I'm guessing it's a small number where manually fixing it in the species database would make more sense. I'm not sure what a migration would look like. It wouldn't be a case of just adding the GID to the OU additional info since the existing id in the OU germplasmDbId field could be wrong. You couldn't easily tell if it was right or wrong, and if it's wrong, it's not clear which germplasmDbId would be correct? So a migration just looks like re-importing the experiment with bug fix in place.

Copy link
Contributor

@davedrp davedrp left a comment

Choose a reason for hiding this comment

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

passed developer test

@dmeidlin dmeidlin merged commit 93f983d into develop Dec 20, 2023
@dmeidlin dmeidlin deleted the bug/BI-2028 branch December 20, 2023 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants