FINERACT-1968: Fix overpayment calculation

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java

@@ -111,6 +111,7 @@
 import org.apache.fineract.portfolio.loanaccount.data.LoanTermVariationsData;
 import org.apache.fineract.portfolio.loanaccount.data.ScheduleGeneratorDTO;
 import org.apache.fineract.portfolio.loanaccount.domain.transactionprocessor.LoanRepaymentScheduleTransactionProcessor;
+import org.apache.fineract.portfolio.loanaccount.domain.transactionprocessor.MoneyHolder;
 import org.apache.fineract.portfolio.loanaccount.exception.ExceedingTrancheCountException;
 import org.apache.fineract.portfolio.loanaccount.exception.InvalidLoanStateTransitionException;
 import org.apache.fineract.portfolio.loanaccount.exception.InvalidLoanTransactionTypeException;
@@ -797,7 +798,7 @@ private void handleChargePaidTransaction(final LoanCharge charge, final LoanTran
         final Set<LoanCharge> loanCharges = new HashSet<>(1);
         loanCharges.add(charge);
         loanRepaymentScheduleTransactionProcessor.processLatestTransaction(chargesPayment, getCurrency(), chargePaymentInstallments,
-                loanCharges, getTotalOverpaidAsMoney());
+                loanCharges, new MoneyHolder(getTotalOverpaidAsMoney()));
 
         updateLoanSummaryDerivedFields();
         doPostLoanTransactionChecks(chargesPayment.getTransactionDate(), loanLifecycleStateMachine);
@@ -3321,7 +3322,7 @@ private ChangedTransactionDetail handleRepaymentOrRecoveryOrWaiverTransaction(fi
         if (isTransactionChronologicallyLatest && adjustedTransaction == null
                 && (!reprocess || !this.repaymentScheduleDetail().isInterestRecalculationEnabled()) && !isForeclosure()) {
             loanRepaymentScheduleTransactionProcessor.processLatestTransaction(loanTransaction, getCurrency(),
-                    getRepaymentScheduleInstallments(), getActiveCharges(), getTotalOverpaidAsMoney());
+                    getRepaymentScheduleInstallments(), getActiveCharges(), new MoneyHolder(getTotalOverpaidAsMoney()));
             reprocess = false;
             if (this.repaymentScheduleDetail().isInterestRecalculationEnabled()) {
                 if (currentInstallment == null || currentInstallment.isNotFullyPaidOff()) {
@@ -3914,7 +3915,7 @@ public ChangedTransactionDetail closeAsWrittenOff(final JsonCommand command, fin
 
             addLoanTransaction(loanTransaction);
             loanRepaymentScheduleTransactionProcessor.processLatestTransaction(loanTransaction, loanCurrency(),
-                    getRepaymentScheduleInstallments(), getActiveCharges(), getTotalOverpaidAsMoney());
+                    getRepaymentScheduleInstallments(), getActiveCharges(), new MoneyHolder(getTotalOverpaidAsMoney()));
 
             updateLoanSummaryDerivedFields();
         }
@@ -4019,7 +4020,7 @@ public ChangedTransactionDetail close(final JsonCommand command, final LoanLifec
 
                 addLoanTransaction(loanTransaction);
                 loanRepaymentScheduleTransactionProcessor.processLatestTransaction(loanTransaction, loanCurrency(),
-                        getRepaymentScheduleInstallments(), getActiveCharges(), getTotalOverpaidAsMoney());
+                        getRepaymentScheduleInstallments(), getActiveCharges(), new MoneyHolder(getTotalOverpaidAsMoney()));
 
                 updateLoanSummaryDerivedFields();
             } else if (totalOutstanding.isGreaterThanZero()) {
@@ -6373,7 +6374,7 @@ private ChangedTransactionDetail handleRefundTransaction(final LoanTransaction l
         // If is a refund
         if (adjustedTransaction == null) {
             loanRepaymentScheduleTransactionProcessor.processLatestTransaction(loanTransaction, getCurrency(),
-                    getRepaymentScheduleInstallments(), getActiveCharges(), getTotalOverpaidAsMoney());
+                    getRepaymentScheduleInstallments(), getActiveCharges(), new MoneyHolder(getTotalOverpaidAsMoney()));
         } else {
             final List<LoanTransaction> allNonContraTransactionsPostDisbursement = retrieveListOfTransactionsPostDisbursement();
             changedTransactionDetail = loanRepaymentScheduleTransactionProcessor.reprocessLoanTransactions(getDisbursementDate(),
@@ -6404,7 +6405,7 @@ public void handleChargebackTransaction(final LoanTransaction chargebackTransact
 
         addLoanTransaction(chargebackTransaction);
         loanRepaymentScheduleTransactionProcessor.processLatestTransaction(chargebackTransaction, getCurrency(),
-                getRepaymentScheduleInstallments(), getActiveCharges(), getTotalOverpaidAsMoney());
+                getRepaymentScheduleInstallments(), getActiveCharges(), new MoneyHolder(getTotalOverpaidAsMoney()));
 
         updateLoanSummaryDerivedFields();
         if (!doPostLoanTransactionChecks(chargebackTransaction.getTransactionDate(), loanLifecycleStateMachine)) {

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransaction.java

@@ -483,11 +483,6 @@ public void updateComponentsAndTotal(final Money principal, final Money interest
                 .plus(getPenaltyChargesPortion(currency)).getAmount();
     }
 
-    public void updateOverPayments(final Money overPayment) {
-        final MonetaryCurrency currency = overPayment.getCurrency();
-        this.overPaymentPortion = defaultToNullIfZero(getOverPaymentPortion(currency).plus(overPayment).getAmount());
-    }
-
     public void setOverPayments(final Money overPayment) {
         if (overPayment != null) {
             this.overPaymentPortion = defaultToNullIfZero(overPayment.getAmount());

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/AbstractLoanRepaymentScheduleTransactionProcessor.java

@@ -144,14 +144,15 @@ public ChangedTransactionDetail reprocessLoanTransactions(final LocalDate disbur
 
                 if (unprocessed.isGreaterThanZero()) {
                     onLoanOverpayment(loanTransaction, unprocessed);
-                    loanTransaction.updateOverPayments(unprocessed);
+                    loanTransaction.setOverPayments(unprocessed);
                 }
 
             } else {
                 transactionsToBeProcessed.add(loanTransaction);
             }
         }
 
+        MoneyHolder overpaymentHolder = new MoneyHolder(Money.zero(currency));
         for (final LoanTransaction loanTransaction : transactionsToBeProcessed) {
             // TODO: analyze and remove this
             if (!loanTransaction.getTypeOf().equals(LoanTransactionType.REFUND_FOR_ACTIVE_LOAN)) {
@@ -163,7 +164,7 @@ public ChangedTransactionDetail reprocessLoanTransactions(final LocalDate disbur
             if (loanTransaction.isRepaymentLikeType() || loanTransaction.isInterestWaiver() || loanTransaction.isRecoveryRepayment()) {
                 // pass through for new transactions
                 if (loanTransaction.getId() == null) {
-                    processLatestTransaction(loanTransaction, currency, installments, charges, null);
+                    processLatestTransaction(loanTransaction, currency, installments, charges, overpaymentHolder);
                     loanTransaction.adjustInterestComponent(currency);
                 } else {
                     /**
@@ -174,7 +175,7 @@ public ChangedTransactionDetail reprocessLoanTransactions(final LocalDate disbur
 
                     // Reset derived component of new loan transaction and
                     // re-process transaction
-                    processLatestTransaction(newLoanTransaction, currency, installments, charges, null);
+                    processLatestTransaction(newLoanTransaction, currency, installments, charges, overpaymentHolder);
                     newLoanTransaction.adjustInterestComponent(currency);
                     /**
                      * Check if the transaction amounts have changed. If so, reverse the original transaction and update
@@ -195,9 +196,11 @@ public ChangedTransactionDetail reprocessLoanTransactions(final LocalDate disbur
                 loanTransaction.resetDerivedComponents();
                 handleRefund(loanTransaction, currency, installments, charges);
             } else if (loanTransaction.isCreditBalanceRefund()) {
-                recalculateCreditTransaction(changedTransactionDetail, loanTransaction, currency, installments, transactionsToBeProcessed);
+                recalculateCreditTransaction(changedTransactionDetail, loanTransaction, currency, installments, transactionsToBeProcessed,
+                        overpaymentHolder);
             } else if (loanTransaction.isChargeback()) {
-                recalculateCreditTransaction(changedTransactionDetail, loanTransaction, currency, installments, transactionsToBeProcessed);
+                recalculateCreditTransaction(changedTransactionDetail, loanTransaction, currency, installments, transactionsToBeProcessed,
+                        overpaymentHolder);
                 reprocessChargebackTransactionRelation(changedTransactionDetail, transactionsToBeProcessed);
             } else if (loanTransaction.isChargeOff()) {
                 recalculateChargeOffTransaction(changedTransactionDetail, loanTransaction, currency, installments);
@@ -209,11 +212,11 @@ public ChangedTransactionDetail reprocessLoanTransactions(final LocalDate disbur
 
     @Override
     public void processLatestTransaction(final LoanTransaction loanTransaction, final MonetaryCurrency currency,
-            final List<LoanRepaymentScheduleInstallment> installments, final Set<LoanCharge> charges, Money overpaidAmount) {
+            final List<LoanRepaymentScheduleInstallment> installments, final Set<LoanCharge> charges, MoneyHolder overpaymentHolder) {
         switch (loanTransaction.getTypeOf()) {
             case WRITEOFF -> handleWriteOff(loanTransaction, currency, installments);
             case REFUND_FOR_ACTIVE_LOAN -> handleRefund(loanTransaction, currency, installments, charges);
-            case CHARGEBACK -> handleChargeback(loanTransaction, currency, overpaidAmount, installments);
+            case CHARGEBACK -> handleChargeback(loanTransaction, currency, installments, overpaymentHolder);
             default -> {
                 Money transactionAmountUnprocessed = handleTransactionAndCharges(loanTransaction, currency, installments, charges, null,
                         false);
@@ -223,8 +226,11 @@ public void processLatestTransaction(final LoanTransaction loanTransaction, fina
                                 transactionAmountUnprocessed.zero(), transactionAmountUnprocessed.zero());
                     } else {
                         onLoanOverpayment(loanTransaction, transactionAmountUnprocessed);
-                        loanTransaction.updateOverPayments(transactionAmountUnprocessed);
+                        loanTransaction.setOverPayments(transactionAmountUnprocessed);
                     }
+                    overpaymentHolder.setMoneyObject(transactionAmountUnprocessed);
+                } else {
+                    overpaymentHolder.setMoneyObject(Money.zero(currency));
                 }
             }
         }
@@ -435,17 +441,15 @@ private boolean isNotObligationsMet(LoanRepaymentScheduleInstallment loanRepayme
     }
 
     private void recalculateCreditTransaction(ChangedTransactionDetail changedTransactionDetail, LoanTransaction loanTransaction,
-            MonetaryCurrency currency, List<LoanRepaymentScheduleInstallment> installments,
-            List<LoanTransaction> transactionsToBeProcessed) {
+            MonetaryCurrency currency, List<LoanRepaymentScheduleInstallment> installments, List<LoanTransaction> transactionsToBeProcessed,
+            MoneyHolder overpaymentHolder) {
         // pass through for new transactions
         if (loanTransaction.getId() == null) {
             return;
         }
         final LoanTransaction newLoanTransaction = LoanTransaction.copyTransactionProperties(loanTransaction);
 
-        List<LoanTransaction> mergedList = getMergedTransactionList(transactionsToBeProcessed, changedTransactionDetail);
-        Money overpaidAmount = calculateOverpaidAmount(loanTransaction, mergedList, installments, currency);
-        processCreditTransaction(newLoanTransaction, overpaidAmount, currency, installments);
+        processCreditTransaction(newLoanTransaction, overpaymentHolder, currency, installments);
         if (!LoanTransaction.transactionAmountsMatch(currency, loanTransaction, newLoanTransaction)) {
             createNewTransaction(loanTransaction, newLoanTransaction, changedTransactionDetail);
         }
@@ -470,50 +474,18 @@ protected void createNewTransaction(LoanTransaction loanTransaction, LoanTransac
 
     }
 
-    private Money calculateOverpaidAmount(LoanTransaction loanTransaction, List<LoanTransaction> transactions,
-            List<LoanRepaymentScheduleInstallment> installments, MonetaryCurrency currency) {
-        Money totalPaidInRepayments = Money.zero(currency);
-
-        Money cumulativeTotalPaidOnInstallments = Money.zero(currency);
-        for (final LoanRepaymentScheduleInstallment scheduledRepayment : installments) {
-            cumulativeTotalPaidOnInstallments = cumulativeTotalPaidOnInstallments
-                    .plus(scheduledRepayment.getPrincipalCompleted(currency).plus(scheduledRepayment.getInterestPaid(currency)))
-                    .plus(scheduledRepayment.getFeeChargesPaid(currency)).plus(scheduledRepayment.getPenaltyChargesPaid(currency));
-        }
-
-        for (final LoanTransaction transaction : transactions) {
-            if (transaction.isReversed()) {
-                continue;
-            }
-            if (transaction.equals(loanTransaction)) {
-                // We want to process only the transactions prior to the actual one
-                break;
-            }
-            if (transaction.isRefund() || transaction.isRefundForActiveLoan()) {
-                totalPaidInRepayments = totalPaidInRepayments.minus(transaction.getAmount(currency));
-            } else if (transaction.isCreditBalanceRefund() || transaction.isChargeback()) {
-                totalPaidInRepayments = totalPaidInRepayments.minus(transaction.getOverPaymentPortion(currency));
-            } else if (transaction.isRepaymentLikeType()) {
-                totalPaidInRepayments = totalPaidInRepayments.plus(transaction.getAmount(currency));
-            }
-        }
-
-        // if total paid in transactions higher than repayment schedule then
-        // theres an overpayment.
-        return MathUtil.negativeToZero(totalPaidInRepayments.minus(cumulativeTotalPaidOnInstallments));
-    }
-
-    private void processCreditTransaction(LoanTransaction loanTransaction, Money overpaidAmount, MonetaryCurrency currency,
+    private void processCreditTransaction(LoanTransaction loanTransaction, MoneyHolder overpaymentHolder, MonetaryCurrency currency,
             List<LoanRepaymentScheduleInstallment> installments) {
         loanTransaction.resetDerivedComponents();
         List<LoanTransactionToRepaymentScheduleMapping> transactionMappings = new ArrayList<>();
         final Comparator<LoanRepaymentScheduleInstallment> byDate = Comparator.comparing(LoanRepaymentScheduleInstallment::getDueDate);
         installments.sort(byDate);
         final Money zeroMoney = Money.zero(currency);
         Money transactionAmount = loanTransaction.getAmount(currency);
-        Money principalPortion = MathUtil.negativeToZero(loanTransaction.getAmount(currency).minus(overpaidAmount));
+        Money principalPortion = MathUtil.negativeToZero(loanTransaction.getAmount(currency).minus(overpaymentHolder.getMoneyObject()));
         Money repaidAmount = MathUtil.negativeToZero(transactionAmount.minus(principalPortion));
-        loanTransaction.updateOverPayments(repaidAmount);
+        loanTransaction.setOverPayments(repaidAmount);
+        overpaymentHolder.setMoneyObject(overpaymentHolder.getMoneyObject().minus(repaidAmount));
         loanTransaction.updateComponents(principalPortion, zeroMoney, zeroMoney, zeroMoney);
 
         if (principalPortion.isGreaterThanZero()) {
@@ -770,14 +742,14 @@ protected void handleWriteOff(final LoanTransaction loanTransaction, final Monet
         loanTransaction.updateComponentsAndTotal(principalPortion, interestPortion, feeChargesPortion, penaltychargesPortion);
     }
 
-    protected void handleChargeback(LoanTransaction loanTransaction, MonetaryCurrency currency, Money overpaidAmount,
-            List<LoanRepaymentScheduleInstallment> installments) {
-        processCreditTransaction(loanTransaction, overpaidAmount, currency, installments);
+    protected void handleChargeback(LoanTransaction loanTransaction, MonetaryCurrency currency,
+            List<LoanRepaymentScheduleInstallment> installments, MoneyHolder overpaidAmountHolder) {
+        processCreditTransaction(loanTransaction, overpaidAmountHolder, currency, installments);
     }
 
-    protected void handleCreditBalanceRefund(LoanTransaction loanTransaction, MonetaryCurrency currency, Money overpaidAmount,
-            List<LoanRepaymentScheduleInstallment> installments) {
-        processCreditTransaction(loanTransaction, overpaidAmount, currency, installments);
+    protected void handleCreditBalanceRefund(LoanTransaction loanTransaction, MonetaryCurrency currency,
+            List<LoanRepaymentScheduleInstallment> installments, MoneyHolder overpaidAmountHolder) {
+        processCreditTransaction(loanTransaction, overpaidAmountHolder, currency, installments);
     }
 
     protected void handleRefund(LoanTransaction loanTransaction, MonetaryCurrency currency,

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/LoanRepaymentScheduleTransactionProcessor.java

@@ -41,7 +41,7 @@ public interface LoanRepaymentScheduleTransactionProcessor {
      * schedule.
      */
     void processLatestTransaction(LoanTransaction loanTransaction, MonetaryCurrency currency,
-            List<LoanRepaymentScheduleInstallment> installments, Set<LoanCharge> charges, Money overpaidAmount);
+            List<LoanRepaymentScheduleInstallment> installments, Set<LoanCharge> charges, MoneyHolder overpaymentHolder);
 
     /**
      * Provides support for passing all {@link LoanTransaction}'s so it will completely re-process the entire loan

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/MoneyHolder.java

@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.loanaccount.domain.transactionprocessor;
+
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.fineract.organisation.monetary.domain.Money;
+
+@Getter
+@Setter
+@AllArgsConstructor
+public class MoneyHolder {
+
+    private Money moneyObject;
+}