-
Notifications
You must be signed in to change notification settings - Fork 2.2k
FINERACT-2354: Re-age - Excess payment handling - Default Behavior, interestRecalculation = true #5122
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
Conversation
bb14437 to
4d1ba4f
Compare
43cc49d to
f1939f8
Compare
| && DateUtils.isBefore(currentBusinessDate, expectedMaturityDate); | ||
| if (installment.isDownPayment()) { | ||
| isCurrentDateBeforeInstallmentAndLoanPeriod = DateUtils.isEqual(currentBusinessDate, installment.getDueDate()) | ||
| if (!installment.isPaidMovedPaidDuringReAging()) { |
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.
isPaidMovedPaidDuringReAging -> duplicate usage of the word of "paid"?
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.
Renamed
| && DateUtils.isBefore(currentBusinessDate, expectedMaturityDate); | ||
| if (installment.isDownPayment()) { | ||
| isCurrentDateBeforeInstallmentAndLoanPeriod = DateUtils.isEqual(currentBusinessDate, installment.getDueDate()) | ||
| if (!installment.isPaidMovedPaidDuringReAging()) { |
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 to modify this logic?
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 wanted just to exclude reAgePaidInstallment here
| private boolean isReAged; | ||
|
|
||
| @Column(name = "is_paid_moved_during_re_aging", nullable = false) | ||
| private boolean isPaidMovedPaidDuringReAging; |
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.
Can it be something more generic, like "isReageSpecialInstallment" or "isReagePaidInstallment"? isPaidMovedPaidDuringReAging sounds weird to me.
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.
Done
|
|
||
| loanTransaction.updateComponentsAndTotal(totalOutstandingPrincipal, interestFromZeroedInstallments, Money.zero(currency), | ||
| Money.zero(currency)); | ||
| reprocessInstallments(installments); |
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 to remove this?
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 returned it back, but modified the sorting
|
|
||
| @Getter | ||
| @Setter | ||
| private boolean isPeriodWithMovedPaidAmountDuringReAging = 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.
Can we use something similar like in the repayment installment entity which describe the nature of the period instead of the action?
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.
Done
f1939f8 to
4e7ac7d
Compare
5d2fe4b to
8b6945c
Compare
ec57729 to
88c44f4
Compare
…nterestRecalculation = true
…t to last edge case of re-aging
…nterestRecalculation = true, without dueDate change - missing testRailIds added
88c44f4 to
7c82a41
Compare
adamsaghy
left a comment
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.
LGTM
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.