avi8pE7N : Ticket from trello by alejandro@quintero.com#50
avi8pE7N : Ticket from trello by alejandro@quintero.com#50aquintero2023 wants to merge 2 commits intoBravoLT:mainfrom
Conversation
| private final PaymentService paymentService; | ||
| private final UserValidator userValidator; | ||
|
|
||
| public PaymentController(PaymentService paymentService, UserValidator userValidator) { |
There was a problem hiding this comment.
Have you heard of @RequiredArgsConstructor from Lombok?
| @ApiResponse(responseCode = "200", content = { | ||
| @Content(schema = @Schema(implementation = List.class), mediaType = "application/json") | ||
| }) | ||
| @ApiResponse(responseCode = "400", content = {@Content(schema = @Schema())}) |
There was a problem hiding this comment.
The empty @Schema() is weird. Additionally the @ArraySchema seems better in this situation because you can list the PaymentDto type as well.
https://github.com/swagger-api/swagger-core/wiki/Swagger-2.X---Annotations#arrayschema
| { | ||
| log.info("Userid:{}", userId); | ||
| userValidator.validateId(userId); | ||
| return paymentService.retrieveByUserId(userId); |
There was a problem hiding this comment.
httpResponse is unused, and this log is vague and unnecessary.
|
|
||
| public List<PaymentDto> retrieveByUserId(final String userId) { | ||
| final List<Payment> paymentList = paymentRepository.findByUserId(userId); | ||
| log.info("found {} payment(s)", paymentList.size()); |
There was a problem hiding this comment.
Maybe the userId belongs in this log statement as well?
|
|
||
| @BeforeEach | ||
| public void beforeEach() { | ||
| log.info("Initializing payment list"); |
There was a problem hiding this comment.
I think logs in tests are usually unnecessary as there are methods built into the testing tools to provide more clear output. Additionally the public modifier is unnecessary.
| } | ||
|
|
||
| @Test | ||
| void getRetrieveByUserId() throws Exception { |
There was a problem hiding this comment.
Weird formatting. What IDE are you using?
| } | ||
|
|
||
| private PaymentDto createPaymentDto(String userId) { | ||
| final PaymentDto payment = new PaymentDto(); |
There was a problem hiding this comment.
@Builder from Lombok might make this code nicer.
| final List<Integer> userIds = IntStream.range(1, 10).boxed().toList(); | ||
|
|
||
| final List<Payment> daoPayments = userIds.stream() | ||
| .map(id -> createPayment(Integer.toString(id))).collect(Collectors.toList()); |
There was a problem hiding this comment.
If you update the createPayment method to accept a String then you can use method reference instead of this Lambda.
.map(this::createPayment)
| when(paymentRepository.findByUserId(anyString())).thenReturn(daoPayments); | ||
|
|
||
| this.dtoPayments = userIds.stream().map(id -> createPaymentDto(Integer.toString(id))) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
Does .toList() work instead of .collect(Collectors.toList())?
2 participants
You can check a demo of this on:
http://34.239.237.47:9999/swagger-ui/index.htmlThere are only 5 users with payment data, 2 cards for each user: