-
Notifications
You must be signed in to change notification settings - Fork 1
BI-1616 #237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BI-1616 #237
Changes from all commits
fc96343
a666c46
d8beca1
0453792
4775a2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,9 +35,11 @@ | |
| import org.breedinginsight.api.auth.ProgramSecured; | ||
| import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; | ||
| import org.breedinginsight.api.auth.SecurityService; | ||
| import org.breedinginsight.api.model.v1.request.query.SearchRequest; | ||
| import org.breedinginsight.api.model.v1.validators.QueryValid; | ||
| import org.breedinginsight.brapi.v1.controller.BrapiVersion; | ||
| import org.breedinginsight.brapi.v1.model.request.query.BrapiQuery; | ||
| import org.breedinginsight.brapi.v2.model.request.query.ListQuery; | ||
| import org.breedinginsight.utilities.response.mappers.ListQueryMapper; | ||
| import org.breedinginsight.brapi.v2.services.BrAPIGermplasmService; | ||
| import org.breedinginsight.model.ProgramBrAPIEndpoints; | ||
|
|
@@ -90,10 +92,24 @@ public BrAPIServerInfoResponse serverinfo() { | |
| @Produces(MediaType.APPLICATION_JSON) | ||
| @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.ALL}) | ||
| public HttpResponse getLists(@PathVariable("programId") UUID programId, HttpRequest<String> request, | ||
| @QueryValue @QueryValid(using = ListQueryMapper.class) @Valid BrapiQuery queryParams | ||
| @QueryValue @QueryValid(using = ListQueryMapper.class) @Valid ListQuery queryParams | ||
| ) throws DoesNotExistException, ApiException { | ||
| List<BrAPIListSummary> brapiLists = germplasmService.getGermplasmListsByProgramId(programId, request); | ||
| return ResponseUtils.getBrapiQueryResponse(brapiLists, listQueryMapper, queryParams); | ||
| List<BrAPIListSummary> brapiLists; | ||
|
|
||
| if (queryParams.getListType() == null) { | ||
| // TODO: in future return all list types but for now just return germplasm | ||
| brapiLists = germplasmService.getGermplasmListsByProgramId(programId, request); | ||
| } else { | ||
| // TODO: return appropriate lists by type, only germplasm currently | ||
| switch(queryParams.getListType()) { | ||
| case "germplasm": | ||
| default: | ||
| brapiLists = germplasmService.getGermplasmListsByProgramId(programId, request); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to above, maybe a 400 should be returned here.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could see the case for a 400 here if there is a listType specified but it's not one of the available ones. |
||
| } | ||
| } | ||
|
|
||
| SearchRequest searchRequest = queryParams.constructSearchRequest(); | ||
| return ResponseUtils.getBrapiQueryResponse(brapiLists, listQueryMapper, queryParams, searchRequest); | ||
| } | ||
|
|
||
| @Get("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2 + "/{+path}") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package org.breedinginsight.brapi.v2.model.request.query; | ||
|
|
||
| import io.micronaut.core.annotation.Introspected; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
| import lombok.Setter; | ||
| import org.breedinginsight.api.model.v1.request.query.FilterRequest; | ||
| import org.breedinginsight.api.model.v1.request.query.SearchRequest; | ||
| import org.breedinginsight.brapi.v1.model.request.query.BrapiQuery; | ||
| import org.jooq.tools.StringUtils; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| @Getter | ||
| @Introspected | ||
| public class ListQuery extends BrapiQuery { | ||
| private String listType; | ||
| private String name; | ||
| private String description; | ||
| private String size; | ||
| private String dateCreated; | ||
| private String ownerName; | ||
|
|
||
| public SearchRequest constructSearchRequest() { | ||
| List<FilterRequest> filters = new ArrayList<>(); | ||
| if (!StringUtils.isBlank(getListType())) { | ||
| filters.add(constructFilterRequest("type", getListType())); | ||
| } | ||
| if (!StringUtils.isBlank(getName())) { | ||
| filters.add(constructFilterRequest("name", getName())); | ||
| } | ||
| if (!StringUtils.isBlank(getDescription())) { | ||
| filters.add(constructFilterRequest("description", getDescription())); | ||
| } | ||
| if (!StringUtils.isBlank(getSize())) { | ||
| filters.add(constructFilterRequest("size", getSize())); | ||
| } | ||
| if (!StringUtils.isBlank(getDateCreated())) { | ||
| filters.add(constructFilterRequest("dateCreated", getDateCreated())); | ||
| } | ||
| if (!StringUtils.isBlank(getOwnerName())) { | ||
| filters.add(constructFilterRequest("ownerName", getOwnerName())); | ||
| } | ||
| return new SearchRequest(filters); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just have this method return a 400 if the listType isn't specified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't think the listType parameter was required but I can change it if it should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah...good point. Looked back at the spec, and you're spot on. I'll approve this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool thanks!