-
Notifications
You must be signed in to change notification settings - Fork 2.2k
FINERACT-2354: Re-aging:- Accrual and Accrual Activity handling - Default Handling #5245
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
0c6592e to
9fb0aeb
Compare
9fb0aeb to
7281e99
Compare
|
@mariiaKraievska Please rebase |
| private void calculateAccrualActivity(LoanTransaction loanTransaction, MonetaryCurrency currency, | ||
| List<LoanRepaymentScheduleInstallment> installments) { | ||
|
|
||
| private boolean calculateAccrualActivity(LoanTransaction loanTransaction, MonetaryCurrency currency, |
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.
This function has cognitive complexity of 27, it should be broken down to smaller functions
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
7281e99 to
81450a0
Compare
81450a0 to
a8762c4
Compare
Aman-Mittal
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.
On Technical Level Seems OK
a8762c4 to
1fd4ade
Compare
1fd4ade to
98691af
Compare
|
|
||
| @Override | ||
| public void recalculateAccrualActivityTransaction(Loan loan, ChangedTransactionDetail changedTransactionDetail) { | ||
| public void recalculateAccrualActivityTransaction(Loan loan, ChangedTransactionDetail changedTransactionDetail, |
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.
Let's rework it without loan transactions argument and without reaging handling.
Accrual activities should be compared only with loan 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.
Done
| throw new IllegalStateException("Expected an AdvancedPaymentScheduleTransactionProcessor"); | ||
| } | ||
| if (installment.isAdditional() || installment.isDownPayment() || installment.isReAged()) { | ||
| if (installment.isAdditional() || installment.isDownPayment() || (installment.isReAged() && loanReAgeParameter != null |
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.
//TODO: Remove installment.isReAged() once EQUAL AMORTIZATION is implemented
| final LoanTransaction reAgeTransaction = loan.findReAgeTransaction(); | ||
| final LoanReAgeParameter loanReAgeParameter = reAgeTransaction != null ? reAgeTransaction.getLoanReAgeParameter() : null; | ||
|
|
||
| if (installment.isAdditional() || (installment.isReAged() && loanReAgeParameter != null |
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.
//TODO: Remove installment.isReAged() once EQUAL AMORTIZATION is implemented
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.
Kindly see my review!
98691af to
a06684e
Compare
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.