FINERACT-2326: Add the ability to reschedule non-interest-bearing progressive loans to have interest #4884
Conversation
d5cbd96 to
c4a74ea
Compare
|
@Cocoa-Puffs Please check the failing tests! |
e448001 to
745288a
Compare
dc9d8f5 to
14e8136
Compare
| | 3 | 0 | 16 January 2024 | 16 January 2024 | 625.0 | 125.0 | 0.0 | 0.0 | 0.0 | 125.0 | 125.0 | 0.0 | 0.0 | 0.0 | | ||
| | 4 | 15 | 31 January 2024 | | 313.0 | 312.0 | 0.0 | 0.0 | 0.0 | 312.0 | 125.0 | 125.0 | 0.0 | 187.0 | | ||
| | 5 | 15 | 15 February 2024 | | 0.0 | 313.0 | 0.0 | 0.0 | 0.0 | 313.0 | 125.0 | 125.0 | 0.0 | 188.0 | | ||
| | 4 | 15 | 31 January 2024 | | 312.5 | 312.5 | 0.0 | 0.0 | 0.0 | 312.5 | 125.0 | 125.0 | 0.0 | 187.5 | |
There was a problem hiding this comment.
Why these balances got changed?
There was a problem hiding this comment.
Because this is a non-interest bearing loan, previously disbursements were handled without using the EMI model. In the recalculateRepaymentInstallmentsWithoutEMICalculation method we have some rounding logic based on installmentAmountInMultiplesOf of the loan. So previously the periods got rounded to whole numbers (in case of installmentAmountInMultiplesOf = 1). Now with the EMI calculator we can just calculate the exact emi for the periods.
| | 4 | 15 | 28 April 2024 | 24 March 2024 | 0.0 | 121.58 | 0.0 | 0.0 | 0.0 | 121.58 | 121.58 | 121.58 | 0.0 | 0.0 | | ||
| | Nr | Days | Date | Paid date | Balance of loan | Principal due | Interest | Fees | Penalties | Due | Paid | In advance | Late | Outstanding | | ||
| | | | 14 March 2024 | | 487.58 | | | 0.0 | | 0.0 | 0.0 | | | | | ||
| | 1 | 0 | 14 March 2024 | 14 March 2024 | 365.58 | 122.0 | 0.0 | 0.0 | 0.0 | 122.0 | 122.0 | 0.0 | 0.0 | 0.0 | |
There was a problem hiding this comment.
Why these balances got changed?
There was a problem hiding this comment.
Because this is a non-interest bearing loan, previously disbursements were handled without using the EMI model. In the recalculateRepaymentInstallmentsWithoutEMICalculation method we have some rounding logic based on installmentAmountInMultiplesOf of the loan. So previously the periods got rounded to whole numbers (in case of installmentAmountInMultiplesOf = 1). Now with the EMI calculator we can just calculate the exact emi for the periods.
There was a problem hiding this comment.
Thats not how it should work... it doesnt matter whether you are using emi model or not... installment amount in multiples of should be respected...
|
|
||
| private void handleCapitalizedIncome(LoanTransaction capitalizedIncomeTransaction, TransactionCtx transactionCtx) { | ||
| if (capitalizedIncomeTransaction.getLoan().isInterestBearing()) { | ||
| // TODO: Fix this and enhance EMICalculator to support reamortization and reaging |
There was a problem hiding this comment.
What this comment to do with capitalized income?
There was a problem hiding this comment.
In my understanding the scope of the issue was to move away from non EMI methods as much as possible as part of this ticket. This could not be done completely in a timely fashion as there were multiple operations that would need to be included in the emi calculator for everything to work. Reaging, reamortization, charges. I added the todo so that in the future if we implement these operations in the emi calculator we can remove the non emi calculator using method, same as with disbursements.
|
|
||
| @Setter | ||
| @Getter | ||
| private BigDecimal totalCapitalizedIncomeAmount; |
There was a problem hiding this comment.
Was there some issue with capitalized income handling? Should this to be extracted into a separate PR?
There was a problem hiding this comment.
There was a problem specifically with 0% interest loans that have FLAT interest method, with capitalized income transactions. The method that provides the principal calculation in these cases is (now called) RepaymentPeriod.calculateTotalDisbursedAndCapitalizedIncomeAmountTillGivenPeriod, it also calculates the EMI values for flat interest loans in ProgressiveEMICalculator.calculateEMIOnActualModelWithFlatInterestMethod, previously the calculate method didn't include the capitalized income amounts in the total and because of this the principal from capitalized income transactions were not calculated into the EMI. I made it a part of this PR because the problem arose because the non-emi capitalized income handling didn't have this issue.
becd20a to
7f1053d
Compare
…gressive loans to have interest
…e non-interest-bearing progressive loans to have interest
7f1053d to
0072998
Compare
| | 3 | 0 | 01 April 2024 | 01 April 2024 | 478.31 | 78.17 | 0.0 | 0.0 | 0.0 | 78.17 | 78.17 | 0.0 | 0.0 | 0.0 | | ||
| | 4 | 15 | 13 April 2024 | | 239.16 | 239.15 | 0.0 | 0.0 | 0.0 | 239.15 | 121.89 | 121.89 | 0.0 | 117.26 | | ||
| | 5 | 15 | 28 April 2024 | | 0.0 | 239.16 | 0.0 | 0.0 | 0.0 | 239.16 | 121.9 | 121.9 | 0.0 | 117.26 | | ||
| | 4 | 15 | 13 April 2024 | | 239.15 | 239.16 | 0.0 | 0.0 | 0.0 | 239.16 | 121.89 | 121.89 | 0.0 | 117.27 | |
There was a problem hiding this comment.
@Cocoa-Puffs @ruzeynalov Any reason why these changed?
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!
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.