BI-2344 - Incorrect Germplasm Export in Case of Duplicate Germplasm Names#419
Merged
mlm483 merged 3 commits intorelease/1.0from Oct 30, 2024
Merged
BI-2344 - Incorrect Germplasm Export in Case of Duplicate Germplasm Names#419mlm483 merged 3 commits intorelease/1.0from
mlm483 merged 3 commits intorelease/1.0from
Conversation
full germplasmName includes programKey and GID
HMS17
approved these changes
Oct 29, 2024
Contributor
HMS17
left a comment
There was a problem hiding this comment.
Passed testing with multiple large germplasm lists with duplicates in a program
| // 1. the BrAPI list items are full names, and | ||
| // 2. germplasmNames alone are not unique within a program, this led to unexpected behavior, see BI-2344. | ||
| String uniqueGermplasmName = String.format("%s [%s-%s]", g.getGermplasmName(), program.getKey(), g.getAccessionNumber()); | ||
| g.setGermplasmName(uniqueGermplasmName); // For later. |
Contributor
There was a problem hiding this comment.
could you clarify what the "for later" comment means?
Contributor
Author
There was a problem hiding this comment.
I updated the comment to be more clear. Inside that loop, g is an element of the germplasm collection which is used again on line 111, and needs to have the full germplasmName with programKey and accession number later in the method.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Story: https://breedinginsight.atlassian.net/browse/BI-2344
I introduced this regression in BI-2304.
Because germplasm names are not guaranteed to be unique within a program or list, code that used germplasm name as a key for lookups had unexpected behavior in the case of multiple distinct germplasm in a list that shared the same name (different GIDs, same name is allowed).
The solution was to use the "{germplasmName} [{programKey}-{accessionNumber}]" format as the key for lookups instead, which is what is stored in the backing BrAPI service. Finally, the program key and accession number are stripped from the germplasm name when building the output file data (we don't want to display that to the user).
Testing
germplasmDuplicateNames.xlsx
The screenshot below shows an example upload file, erroneous output (release/1.0 branch) and correct output (bug/BI-2344 branch).
Checklist: