BI-1845 - add listType parameter to GET /lists calls#284
Conversation
| // get germplasm lists by default | ||
| BrAPIListTypes type = BrAPIListTypes.fromValue(queryParams.getListType()); | ||
| if (type == null) { | ||
| type = BrAPIListTypes.GERMPLASM; |
There was a problem hiding this comment.
I think that this goes against the BrAPI spec. I think we should probably return all lists if the type parameter is null. Worth confirming with @BrapiCoordinatorSelby
There was a problem hiding this comment.
The BrAPI Spec does not specifically define what should happen if listType is null.
I would expect it to return all lists of all types, in line with other GET query parameters. You might also make listType a required parameter for your implementation.
Defaulting listType to germplasm is unusual, and might cause some confusion if it is not well documented, but it is not technically against the BrAPI spec.
There was a problem hiding this comment.
Thanks for the input @BrapiCoordinatorSelby, all lists of all types are now returned.
functionality and tests complete
| if (type != null) { | ||
| switch (type) { |
There was a problem hiding this comment.
Prevents NullPointerException for switch on null object.
timparsons
left a comment
There was a problem hiding this comment.
One suggested change to existing code, but doesn't need to stop my approval of the main change.
| default: | ||
| String createdBy = germplasmDAO.getGermplasmByRawName(listItemNames, program.getId()).get(0) | ||
| .getAdditionalInfo() | ||
| .getAsJsonObject("createdBy") | ||
| .get("userName") | ||
| .getAsString(); | ||
| list.setListOwnerName(createdBy); | ||
| } |
There was a problem hiding this comment.
I know this wasn't a change you made, but this code should probably be moved inside the GERMPLASM case
There was a problem hiding this comment.
Done, d029f0d.
A switch doesn't really make sense at the moment, but I assume it will get more use in the future.
Description
Story: https://breedinginsight.atlassian.net/browse/BI-1845
Instead of defaulting to Germplasm if
listTypeis not provided, the list controller nowresponds with 400 BAD REQUESTresponds with all lists of all types iflistTypeis missing in GET calls to /lists. This only applies to the /lists endpoint, and not, for example to the /lists/{listDbId} endpoint.In order to reuse some code in ListControllerIntegrationTest.java, I moved
createTraitsandwriteDataToFilemethods from ExperimentFileImportTest.java to ImportTestUtils.java.Dependencies
Breeding-Insight/bi-web#324
Testing
Authenticated calls to {bi-api}/v1/programs/{programId}/brapi/v2/lists should
get a 400 responseget a 200 response with all lists of all types for the program. Authenticated calls to {bi-api}/v1/programs/{programId}/brapi/v2/lists?listType={type} should get a 200 response with only Germplasm lists for the program.Checklist: