INF-138 - Include data in response even if not populated#28
INF-138 - Include data in response even if not populated#28
Conversation
nickpalladino
changed the title
nickpalladino
left a comment
nickpalladino
left a comment
There was a problem hiding this comment.
Looks good, tests pass and manual testing with insomnia has data array responses include the empty data array now.
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public class DataResponse<T> { | ||
| @JsonInclude() |
There was a problem hiding this comment.
After looking up @JsonInclude, I see that it defaults to JsonInclude.Include.ALWAYS. For me personally seeing JsonInclude.Include.ALWAYS would make it more clear but IntelliJ doesn't like that and says it's redundant. Didn't know if anyone else had any thoughts on this? I'm ok with it either way just didn't know if we had a preferred style we should follow.
There was a problem hiding this comment.
I don't see a problem with leaving it as @JsonInclude
There was a problem hiding this comment.
I was using JsonInclude.Include.ALWAYS at first in the other models, but I think I lean toward not including it now. Just having the @JsonInclude seems explicit enough to me. But, thats a good point on consistency, because some of our models will still have the JsonInclude.Include.ALWAYS so we should update those at some point.
dmeidlin
left a comment
dmeidlin
left a comment
There was a problem hiding this comment.
The GET /programs endpoint returns an empty array as expected. I'm unable to run the tests using bi-docker-stack. From our offline discussions it seems like a bind mount to the docker daemon socket needs to be added to the bi-docker-stack docker-compose.yml under the dev-biapi service, and the installation of the docker daemon (or maybe just docker-cli) needs to be added to the biapi Dockerfile.
4 participants
One Line of code to fix it all, One line to find them,
One Line of code to bring them all and in the Jackson bind them
Testing