-
Notifications
You must be signed in to change notification settings - Fork 21
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
Study ws server v2 #126
Study ws server v2 #126
Conversation
45c90d5
to
03e943e
Compare
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.
oh, I thought I submitted this review yesterday, sorry
private ObjectMapper objectMapper; | ||
|
||
@Before | ||
public void setUp() throws Exception { |
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.
no need to keep this if it's empty
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.
Fixed :)
|
||
@RequestMapping(value = "/", method = RequestMethod.GET) | ||
public ResponseEntity getBrowsableStudies(@RequestParam("species") String species, | ||
@RequestParam("assembly") String assembly, |
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.
you can either copy the descriptions for these fields from #127 or add them here, whatever you prefer
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.
Added :)
eva-server/src/main/java/uk/ac/ebi/eva/server/ws/StudyWSServerV2.java
Outdated
Show resolved
Hide resolved
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.
Looks good! Just some minor comments
eva-server/src/main/java/uk/ac/ebi/eva/server/ws/StudyWSServerV2.java
Outdated
Show resolved
Hide resolved
eva-server/src/main/java/uk/ac/ebi/eva/server/ws/StudyWSServerV2.java
Outdated
Show resolved
Hide resolved
eva-server/src/main/java/uk/ac/ebi/eva/server/ws/StudyWSServerV2.java
Outdated
Show resolved
Hide resolved
eva-server/src/test/java/uk/ac/ebi/eva/server/ws/StudyWSServerV2Test.java
Outdated
Show resolved
Hide resolved
.read("$['page']['number']", Integer.class); | ||
Integer size = JsonPath.using(configuration).parse(response.getBody()) | ||
.read("$['page']['size']", Integer.class); | ||
Integer totalPages = JsonPath.using(configuration).parse(response.getBody()) |
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.
totalPages
is not being used, can you add the validation for it?
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.
Added 👍
be152ab
to
7e201bd
Compare
…V2.java Co-Authored-By: jmmut <jomutlo@gmail.com>
Co-Authored-By: andresfsilva <afsilva04@gmail.com>
7e201bd
to
3c5d27f
Compare
The purpose of this PR is to add a the V2 endpoint for ArchiveWSServer