-
Notifications
You must be signed in to change notification settings - Fork 2.2k
FINERACT-2354: e2e tests for charge-off and contract termination trns after re-age #5317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
7802df0 to
f01002e
Compare
826de95 to
c58dc54
Compare
c58dc54 to
d2f69b0
Compare
| } | ||
|
|
||
| @Override | ||
| public Money calculateInterestForAccelerateMaturity(final ProgressiveLoanInterestScheduleModel scheduleModel, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this?
Also i really dont like the idea to get parts of the scheduleModel from outside as arguments...
|
|
||
| loanSchedule.updateLoanSchedule(loan, installmentsUpToTransactionDate); | ||
|
|
||
| final boolean hasReAgedInstallments = currentInstallment.isReAged() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does it matter? The calculator and model should be aware already... there should be no difference in the handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets rework this whole logic:
- AdvancedPaymentScheduleTransactionProcessor should do NO changes on the ProgressiveLoanInterestScheduleModel at all!
- The action is accelerateMaturityDate executed on the model, the model is updated by the EMICalculator
- The result is read from the model and repayment installment entities are updated.
- Model is the single source of truth for calculations and interest...! -> Action executed on the model, result fetched from the model...
| | 14 March 2024 | Accrual | 1.27 | 0.0 | 1.27 | 0.0 | 0.0 | 0.0 | false | false | | ||
| | 15 March 2024 | Re-age | 84.28 | 83.57 | 0.71 | 0.0 | 0.0 | 0.0 | false | false | | ||
| | 15 March 2024 | Accrual | 1.53 | 0.0 | 1.53 | 0.0 | 0.0 | 0.0 | false | false | | ||
| | 15 March 2024 | Charge-off | 85.79 | 83.57 | 2.22 | 0.0 | 0.0 | 0.0 | false | false | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be the payable interest till that date, which probably is 0.71
| final boolean hasReAgedInterest = hasReAgedInterest(lastPeriod, periodsToRemove); | ||
|
|
||
| if (hasReAgedInterest) { | ||
| final Money futureInterest = periodsToRemove.stream().map(RepaymentPeriod::getDueInterest).reduce(scheduleModel.zero(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is cannot be true for accelerate maturity date charge-off. We should not consider the future interest. Only the past due (including the reaged interest).
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.