Skip to content

[BI-1129] View Germplasm List#222

Merged
dmeidlin merged 7 commits intodevelopfrom
feature/BI-1129
Nov 14, 2022
Merged

[BI-1129] View Germplasm List#222
dmeidlin merged 7 commits intodevelopfrom
feature/BI-1129

Conversation

@dmeidlin
Copy link
Contributor

@dmeidlin dmeidlin commented Nov 2, 2022

Description

Story: BI-1129

  • a GET endpoint was added to brapi/v2/GermplasmController.java with the signature programs/<program-ID>/germplasm/lists/<list-ID>/records
  • /brapi/v2/services/BrAPIGermplasmService.getGermplasmByList() created to fetch germplasm records in a list

Dependencies

none

Testing

Send a GET request /v1/programs/<program-id>/germplasm/lists/<list-id>/records?sortField=importEntryNumber&sortOrder=ASC&page=0&pageSize=50
and verify the response contains all germplasm records stored for the given program and list ID.

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>

Copy link
Member

@timparsons timparsons left a comment

Choose a reason for hiding this comment

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

One small change, otherwise looks good!!


public List<BrAPIGermplasm> getGermplasmByList(UUID programId, String listId) throws ApiException {
// get list germplasm names
BrAPIListDetails listData = brAPIListDAO.getListById(listId, programId).getResult();
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be broken up in to multiple lines to first check that there was a list that was returned before calling the getResults() method. Otherwise, this could throw a NullPointerException

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timparsons Agreed. There is now a check in place prior to the call that a list was returned.

Copy link
Contributor

@HMS17 HMS17 left a comment

Choose a reason for hiding this comment

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

Approving on assumption Tim's comments are addressed.

BrAPIListsSingleResponse listResponse = brAPIListDAO.getListById(listId, programId);
if(Objects.nonNull(listResponse)) {
BrAPIListDetails listData = listResponse.getResult();
List<String> germplasmNames = listData.getData();
Copy link
Member

Choose a reason for hiding this comment

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

Sorry to be a stickler here, but should probably check that listData is not null before calling the getData() method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Np, you're right of course! I added the other check.

@dmeidlin dmeidlin merged commit 5502ddf into develop Nov 14, 2022
@dmeidlin dmeidlin deleted the feature/BI-1129 branch November 14, 2022 14:49
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.

3 participants