FINERACT-2455: Working Capital Loan Transaction Undo - Generic#5937
FINERACT-2455: Working Capital Loan Transaction Undo - Generic#5937somasorosdpc wants to merge 1 commit into
Conversation
153fdcd to
e393d96
Compare
e393d96 to
91dbe49
Compare
| generated during loan approval/disbursement from the loan and product data.""") | ||
| @ApiResponses({ @ApiResponse(responseCode = "200", description = "OK"), | ||
| @ApiResponse(responseCode = "404", description = "Working Capital Loan not found") }) | ||
| @Deprecated(forRemoval = true) |
There was a problem hiding this comment.
We have repayment, no need for this.
Should be deleted
| throw new IllegalStateException("payment not found: date=" + paymentDate + " with amount=" + amount); | ||
| } | ||
| actualPayments.remove(first.get()); | ||
| rebuildPayments(); |
There was a problem hiding this comment.
No need to rebuild everything... just the things after this operation...
| WorkingCapitalLoanConstants.paymentDetailsParamName, WorkingCapitalLoanConstants.externalIdParameterName, | ||
| WorkingCapitalLoanConstants.discountExternalIdParameterName, WorkingCapitalLoanConstants.classificationIdParamName)); | ||
|
|
||
| private static final Set<String> UNDO_TRANSACTION_SUPPORTED_PARAMETERS = new HashSet<>(Arrays.asList("locale", "dateFormat", |
There was a problem hiding this comment.
id there is no transaction date or any other date, dateFormat parameter is not needed
| .orElseThrow(() -> new IllegalStateException("Projected amortization schedule is not found for loan " + loan.getId())); | ||
|
|
||
| model.undoPayment(transactionDate, repaymentAmount); | ||
| model.recalculateNetAmortizationAndDeferredBalanceFrom(transactionDate); |
There was a problem hiding this comment.
This should not be standalone operation but as side-effect of model.undoPayment
| period.setBreach(null); | ||
| } | ||
| } | ||
| repository.saveAndFlush(period); |
| period.setDelinquentAmount(null); | ||
| period.setDelinquentDays(null); | ||
| } | ||
| loanDelinquencyRangeScheduleRepository.saveAndFlush(period); |
91dbe49 to
64f5c38
Compare
64f5c38 to
75c93bc
Compare
galovics
left a comment
There was a problem hiding this comment.
resolves a loan id from a loan external id (loanReadPlatformService.getResolvedLoanId). In executeWorkingCapitalLoanTransactionCommand you're passing the transaction external id into it for resolvedTransactionId, so the two ...TransactionExternalId endpoints will try to look up a loan by the transaction's external id and throw WorkingCapitalLoanNotFoundException. Don't we need a separate resolver for the transaction-by-external-id path? The undo feature only exercises the numeric id so this path isn't covered - worth a test case too.
galovics
left a comment
There was a problem hiding this comment.
resolveLoanId resolves a loan id from a loan external id (loanReadPlatformService.getResolvedLoanId). In executeWorkingCapitalLoanTransactionCommand you're passing the transaction external id into it for resolvedTransactionId, so the two ...TransactionExternalId endpoints will try to look up a loan by the transaction's external id and throw WorkingCapitalLoanNotFoundException. Don't we need a separate resolver for the transaction-by-external-id path? The undo feature only exercises the numeric id so this path isn't covered - worth a test case too.
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!
Your assigned reviewer(s) will follow our guidelines for code reviews.