BI-2100 - BrAPI Hackathon Integration of Helium with DeltaBreed #337
Conversation
nickpalladino
changed the title
nickpalladino
requested review from
a team,
davedrp and
dmeidlin
and removed request for
a team
| @Controller("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2) | ||
| @Secured(SecurityRule.IS_AUTHENTICATED) | ||
| public class BrAPIPedigreeController { | ||
| private final String referenceSource; |
There was a problem hiding this comment.
I do not see this being used, except in the constructor). Could it be removed?
There was a problem hiding this comment.
Thanks for catching that, removed and pushed changes.
| * @return A List of BrAPIPedigreeNode objects that match the search criteria. | ||
| * @throws ApiException If an error occurs while searching for pedigree nodes. | ||
| */ | ||
| /* |
There was a problem hiding this comment.
I think this is commenting-out everything down to line-183. Is that the intention? If so, should those lines be removed.
There was a problem hiding this comment.
I commented it out but left it here is case we want to add the search endpoints at some point we have some work done. I added a comment in the method documentation stating this and pushed changes.
| * @param pedigreeNodes The list of pedigree nodes. | ||
| * @param programKey The program key to be removed. | ||
| */ | ||
| private void stripProgramKeys(List<BrAPIPedigreeNode> pedigreeNodes, String programKey) { |
There was a problem hiding this comment.
Added justification for keeping it in the method documentation and pushed changes.
| pedigreeNodes.forEach(node -> { | ||
| node.setGermplasmName(Utilities.removeProgramKeyAnyAccession(node.getGermplasmName(), programKey)); | ||
| // TODO: pedigree stripping not working right | ||
| //node.setPedigreeString(Utilities.removeProgramKeyAnyAccession(node.getPedigreeString(), programKey)); |
There was a problem hiding this comment.
I left that commented line in with a TODO to fix in the future as that was what was desired but it wasn't working correctly so needs to be looked into more if we decide to strip the program keys in the future.
Description
Story: https://breedinginsight.atlassian.net/browse/BI-2100?atlOrigin=eyJpIjoiNzcxNDBiMTNmYWM2NGNjMmJiYzlmZGU0NzI3OTI5ZGMiLCJwIjoiaiJ9
Dependencies
Testing
Checklist: