vidakovic commented on code in PR #5613:
URL: https://github.com/apache/fineract/pull/5613#discussion_r2924495968
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/api/FixedDepositProductsApiResource.java:
##########
@@ -125,14 +130,11 @@ public class FixedDepositProductsApiResource {
Mandatory Fields for Cash based accounting (accountingRule = 2):
savingsReferenceAccountId, savingsControlAccountId, interestOnSavingsAccountId,
incomeFromFeeAccountId, transfersInSuspenseAccountId,
incomeFromPenaltyAccountId""")
@RequestBody(required = true, content = @Content(schema =
@Schema(implementation =
FixedDepositProductsApiResourceSwagger.PostFixedDepositProductsRequest.class)))
@ApiResponse(responseCode = "200", description = "OK", content =
@Content(schema = @Schema(implementation =
FixedDepositProductsApiResourceSwagger.PostFixedDepositProductsResponse.class)))
- public String create(@Parameter(hidden = true) final String
apiRequestBodyAsJson) {
-
- final CommandWrapper commandRequest = new
CommandWrapperBuilder().createFixedDepositProduct().withJson(apiRequestBodyAsJson)
- .build();
-
- final CommandProcessingResult result =
this.commandsSourceWritePlatformService.logCommandSource(commandRequest);
-
- return this.toApiJsonSerializer.serialize(result);
+ public FixedDepositProductCreateResponse create(@Parameter(hidden = true)
final FixedDepositProductCreateRequest request) {
Review Comment:
This going too far. It's ok to adjust the code of anything that uses the
"fixed rate" services, because of the cleanup. But we are going overboard here
when we start refactoring the domain "savings" and migrate it to the new
command processing; this should be in another pull request. Otherwise the
amount of changed files (200) is too large.
Probably everything that comes next is related to savings and the migration
to new command processing... again, separate PR, then we can control the blast
radius of these changes. Please move those "savings" changes to a different PR
(I'm not sure actually if we have a ticket for this... not sure if I had this
on my list of "low impact" modules, sounds more like this is in the critical
path).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
