fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [fineract] maektwain commented on a change in pull request #926: FINERACT-977
Date Fri, 22 May 2020 20:15:40 GMT

maektwain commented on a change in pull request #926:
URL: https://github.com/apache/fineract/pull/926#discussion_r429436062



##########
File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/AbstractLoanScheduleGenerator.java
##########
@@ -517,9 +517,11 @@ private LoanScheduleModelPeriod handleRecalculationForTransactions(final
MathCon
                                 scheduleParams.getTotalCumulativePrincipal().plus(
                                         currentPeriodParams.getPrincipalForThisPeriod().minus(principalProcessed)),
                                 scheduleParams.getPeriodNumber() + 1, mc));
-                        if (loanApplicationTerms.getAmortizationMethod().isEqualInstallment()
-                                && fixedEmiAmount.compareTo(loanApplicationTerms.getFixedEmiAmount())
!= 0) {
-                            currentPeriodParams.setEmiAmountChanged(true);
+                        if (loanApplicationTerms.getAmortizationMethod().isEqualInstallment())
+                          if(fixedEmiAmount != null) {
+                            if (fixedEmiAmount.compareTo(loanApplicationTerms.getFixedEmiAmount())
!= 0) {

Review comment:
       I thought about this, @vorburger , in case where fixedAmount is null , then the third
condition will simply throw null exception. Don't you think?
   
   Let me simplify the case for you.
   
   Given a loan scenario where loan product type Amortisation is set as Equal Instalment so,
you could set the fixedEmi Amount which is your repayment as when you create a loan product
you can set this boolean value here 
   
   Allow fixing of the installment amount []
   
   if this and other condition is true of equal instalment then while making a loan you can
set the EMI of your own choice like in case of 100 USD loan you can choose to set every month
10 USD instalment then system is forced to have this as every month due.
   
   Now in the above case there are two parts, when we decide to set the FixedEMI or not and
when we don't then there is null value and then you can imagine what might happen.
   
   Now in code level, we should actually take care of null values since we know the cases,
but it was not taken care here.
   
   It won't trigger `setEmiAmountChanged (true)`if fixedEMI is null,  and in the edge case
its null, so better we check if fixedEmi is null and then apply the method .
   
   
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message