FINERACT-2169: Replace String json to DTO - for FloatingRatesApiResource #4347
Conversation
| import jakarta.ws.rs.Produces; | ||
| import jakarta.ws.rs.core.Context; | ||
| import jakarta.validation.Valid; | ||
| import jakarta.ws.rs.*; |
There was a problem hiding this comment.
Please, try to use setting like "import like single class"
| final CommandWrapper commandRequest = new CommandWrapperBuilder().createFloatingRate().withJson(apiRequestBodyAsJson).build(); | ||
| final CommandProcessingResult result = this.commandsSourceWritePlatformService.logCommandSource(commandRequest); | ||
| return this.toApiJsonSerializer.serialize(result); | ||
| public CommandProcessingResult createFloatingRate(@Parameter(hidden = true) final @Valid FloatingRateRequest floatingRateRequest) { |
There was a problem hiding this comment.
final Valid ?? Do you use jakarta annotations somewhere ?
There was a problem hiding this comment.
For now it not used, it reasonable to remove
There was a problem hiding this comment.
Ok, but why did you use Valid annotation here ?=)
| } | ||
|
|
||
| public List<FloatingRatePeriodResponse> getRatePeriods() { | ||
| return ratePeriods; |
There was a problem hiding this comment.
Why didn't you use Lombok here for getters/setters?
There was a problem hiding this comment.
Applied getter/setters for this field
oleksma
force-pushed
the
FINERACT-2169-Replace-String-json-to-DTO-in-FloatingRatesApiResource
branch
2 times, most recently
from
4e11298 to
d2ba4b0
Compare
oleksma
force-pushed
the
FINERACT-2169-Replace-String-json-to-DTO-in-FloatingRatesApiResource
branch
from
d2ba4b0 to
d86c795
Compare
| this.context.authenticatedUser().validateHasReadPermission(RESOURCE_NAME); | ||
| final List<FloatingRateData> floatingRates = this.floatingRatesReadPlatformService.retrieveAll(); | ||
| final ApiRequestJsonSerializationSettings settings = this.apiRequestParameterHelper.process(uriInfo.getQueryParameters()); | ||
| return this.toApiJsonSerializer.serialize(settings, floatingRates, FloatingRatesApiResource.LIST_FLOATING_RATES_PARAMETERS); | ||
| return floatingRates.stream().map(data -> new FloatingRateResponse(data.getId(), data.getName(), data.isBaseLendingRate(), |
There was a problem hiding this comment.
Why to remap to a different DTO?
There was a problem hiding this comment.
This is more specific DTO without field List interestRateFrequencyTypeOptions
This field is not used in result for this endpoint
There was a problem hiding this comment.
API should not remap the service response. Would you mind to revisit this logic and make sure the service provides the wanted response immediately?
There was a problem hiding this comment.
I will revisit logic and review my changes for response
Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated Apache Fineract JIRA ticket.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Write the commit message as per https://github.com/apache/fineract/#pull-requests
Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
Create/update unit or integration tests for verifying the changes made.
Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.