You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "somasorosdpc (via GitHub)" <gi...@apache.org> on 2025/10/14 08:38:38 UTC

[PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

somasorosdpc opened a new pull request, #5102:
URL: https://github.com/apache/fineract/pull/5102

   ## Description
   
   Describe the changes made and why they were made.
   
   Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
   
   ## Checklist
   
   Please make sure these boxes are checked before submitting your pull request - thanks!
   
   - [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
   - [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
   - [ ] Create/update unit or integration tests for verifying the changes made.
   - [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
   - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
   - [ ] Submission is not a "code dump".  (Large changes can be made "in repository" via a branch.  Ask on the developer mailing list for guidance, if required.)
   
   FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490159934


##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java:
##########
@@ -837,7 +839,7 @@ private Map<AllocationType, Money> calculateChargebackAllocationMapByCreditAlloc
     }
 
     protected void processCreditTransaction(LoanTransaction loanTransaction, TransactionCtx ctx) {
-        if (loanTransaction.getLoan().isInterestRecalculationEnabled()) {
+        if (loanTransaction.getLoan().isInterestRecalculationEnabled() && keepUsingEmiCalculatorBasedOnReAge(ctx)) {

Review Comment:
   `keepUsingEmiCalculatorBasedOnReAge`. Lets use the emi model and calculator for everything.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490146468


##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransactionRepository.java:
##########
@@ -481,4 +481,13 @@ boolean existsNonReversedByLoanAndTypeAndDate(@Param("loan") Loan loan, @Param("
             """)
     CodeValue fetchClassificationCodeValueByTransactionId(@Param("transactionId") Long transactionId);
 
+    @Query("""
+                    SELECT lt FROM LoanTransaction lt
+                    WHERE lt.loan = :loan
+                        AND lt.reversed = false
+                            AND lt.typeOf = :type
+                         AND lt.dateOf <= :onDate
+            """)
+    Optional<LoanTransaction> findNonReversedTransactionActiveOnDate(@Param("loan") Loan loan, @Param("type") LoanTransactionType type,

Review Comment:
   This might return multiple transactions. Please review the logic.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490481312


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -3061,9 +3063,19 @@ public void handleChargebackTransaction(final Loan loan, LoanTransaction chargeb
         loan.addLoanTransaction(chargebackTransaction);
         if (loan.isInterestBearing() && loan.isInterestRecalculationEnabled()) {
             final List<LoanTransaction> transactions = loanTransactionService.retrieveListOfTransactionsForReprocessing(loan);
-            loanTransactionProcessingService.reprocessLoanTransactions(loan.getTransactionProcessingStrategyCode(),
-                    loan.getDisbursementDate(), transactions, loan.getCurrency(), loan.getRepaymentScheduleInstallments(),
-                    loan.getActiveCharges());
+            if (transactions.stream().filter(LoanTransaction::isNotReversed).filter(LoanTransaction::isReAge)
+                    .anyMatch(lt -> !lt.getTransactionDate().isAfter(lt.getLoan().getMaturityDate()))) {
+                final ScheduleGeneratorDTO scheduleGeneratorDTO = loanUtilService.buildScheduleGeneratorDTO(loan, null);
+                loanScheduleService.regenerateRepaymentSchedule(loan, scheduleGeneratorDTO);
+            }
+            ChangedTransactionDetail changedTransactionDetail = loanTransactionProcessingService.reprocessLoanTransactions(
+                    loan.getTransactionProcessingStrategyCode(), loan.getDisbursementDate(), transactions, loan.getCurrency(),
+                    loan.getRepaymentScheduleInstallments(), loan.getActiveCharges());
+            // todo check code if we can simplify it
+            if (changedTransactionDetail.getTransactionChanges() != null && !changedTransactionDetail.getTransactionChanges().isEmpty()) {
+                changedTransactionDetail.getTransactionChanges().stream().filter(change -> change.getNewTransaction() != null)
+                        .map(TransactionChangeData::getNewTransaction).forEach(loan::addLoanTransaction);
+            }

Review Comment:
   I believe we already reschedule it before.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "MarianaDmytrivBinariks (via GitHub)" <gi...@apache.org>.
MarianaDmytrivBinariks commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2493812253


##########
fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/initializer/global/LoanProductGlobalInitializerStep.java:
##########
@@ -2221,6 +2221,79 @@ public void initialize() throws Exception {
                 TestContextKey.DEFAULT_LOAN_PRODUCT_CREATE_RESPONSE_LP2_ADV_PYMNT_INTEREST_DAILY_EMI_360_30_INTEREST_RECALC_DAILY_MULTIDISBURSE,
                 responseLoanProductsRequestLP2AdvancedpaymentInterestEmi36030InterestRecalcDailyMultiDisburseDisbursementCharge);
 
+        // LP2 with progressive loan schedule + horizontal + interest recalculation daily EMI + 360/30 +
+        // multidisbursement
+        // Frequency for recalculate Outstanding Principal: Daily, Frequency Interval for recalculation: 1
+        // chargeback - interest, fee, principal, penalty
+        String name150 = DefaultLoanProduct.LP2_ADV_PYMNT_INTEREST_DAILY_EMI_360_30_INTEREST_RECALC_DAILY_MULTIDISBURSE_EXPECT_TRANCHE_CHARGEBACK
+                .getName();
+        PostLoanProductsRequest loanProductsRequestLP2AdvancedpaymentInterestEmi36030InterestRecalcDailyMultiDisburseExpectTrancheChargeback = loanProductsRequestFactory
+                .defaultLoanProductsRequestLP2Emi()//
+                .name(name150)//
+                .daysInYearType(DaysInYearType.DAYS360.value)//
+                .daysInMonthType(DaysInMonthType.DAYS30.value)//
+                .isInterestRecalculationEnabled(true)//
+                .preClosureInterestCalculationStrategy(1)//
+                .rescheduleStrategyMethod(4)//
+                .interestRecalculationCompoundingMethod(0)//
+                .recalculationRestFrequencyType(2)//
+                .recalculationRestFrequencyInterval(1)//
+                .creditAllocation(List.of(//
+                        createCreditAllocation("CHARGEBACK", List.of("INTEREST", "FEE", "PRINCIPAL", "PENALTY"))//
+                ))//
+                .paymentAllocation(List.of(//
+                        createPaymentAllocation("DEFAULT", "NEXT_INSTALLMENT"), //
+                        createPaymentAllocation("GOODWILL_CREDIT", "LAST_INSTALLMENT"), //
+                        createPaymentAllocation("MERCHANT_ISSUED_REFUND", "REAMORTIZATION"), //
+                        createPaymentAllocation("PAYOUT_REFUND", "NEXT_INSTALLMENT")))//
+                .multiDisburseLoan(true)//
+                .disallowExpectedDisbursements(false)//

Review Comment:
   I thought about additional test scenarios with multidisb loan that expect tranches and re-aging between two disbursements



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490162487


##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java:
##########
@@ -2847,18 +2857,30 @@ private static final class Balances {
     }
 
     private void mergeReAgedInstallment(final LoanRepaymentScheduleInstallment target,
-            final LoanRepaymentScheduleInstallment reAgedInstallment, MonetaryCurrency currency, LocalDate transactionDate) {
+            final LoanRepaymentScheduleInstallment reAgedInstallment, MonetaryCurrency currency, LocalDate transactionDate,
+            boolean isEqualInstallmentForInterest, boolean isEqualInstallmentForFeesAndPenalties) {
         target.setAdditional(false);
         target.setReAged(true);
         target.setFromDate(reAgedInstallment.getFromDate());
         target.setDueDate(reAgedInstallment.getDueDate());
         target.setPrincipal(reAgedInstallment.getPrincipal().add(MathUtil.nullToZero(target.getPrincipalCompleted())));
-        target.setInterestCharged(MathUtil.add(reAgedInstallment.getInterestCharged(), target.getInterestPaid()));
+
+        if (isEqualInstallmentForInterest) {
+            target.setInterestCharged(MathUtil.add(reAgedInstallment.getInterestCharged(), target.getInterestPaid()));
+            target.setInterestAccrued(MathUtil.add(target.getInterestAccrued(), reAgedInstallment.getInterestAccrued()));
+        }
+        if (isEqualInstallmentForFeesAndPenalties) {
+            target.setFeeChargesCharged(MathUtil.add(reAgedInstallment.getFeeChargesCharged(), target.getFeeChargesPaid()));
+            target.setFeeAccrued(MathUtil.add(target.getInterestAccrued(), reAgedInstallment.getFeeAccrued()));
+            target.setPenaltyCharges(MathUtil.add(reAgedInstallment.getPenaltyCharges(), target.getPenaltyChargesPaid()));
+            target.setPenaltyAccrued(MathUtil.add(target.getInterestAccrued(), reAgedInstallment.getPenaltyAccrued()));
+        }

Review Comment:
   Lets keep the logic where we calculate the model and simply update the balance from there. I dont want to have N+1 logic



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490224159


##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java:
##########
@@ -1540,12 +1549,12 @@ private boolean shouldUseEmiCalculation(TransactionCtx transactionCtx, LocalDate
         if (transactionCtx instanceof ProgressiveTransactionCtx progressiveTransactionCtx) {
             boolean hasActiveReAmortization = progressiveTransactionCtx.getAlreadyProcessedTransactions().stream()
                     .anyMatch(t -> t.getTypeOf().isReAmortize() && t.isNotReversed());
-            boolean hasActiveReAge = progressiveTransactionCtx.getAlreadyProcessedTransactions().stream()
-                    .anyMatch(t -> t.getTypeOf().isReAge() && t.isNotReversed());
+            boolean hasReAge = progressiveTransactionCtx.getReAgeInterestHandlingType() != null;
             if (hasActiveReAmortization) {
                 return false;
             } else {
-                return !hasActiveReAge || !DateUtils.isAfter(transactionDate, progressiveTransactionCtx.getModel().getMaturityDate());
+                return !hasReAge || !(DateUtils.isAfter(transactionDate, progressiveTransactionCtx.getModel().getMaturityDate())
+                        || !keepUsingEmiCalculatorBasedOnReAge(progressiveTransactionCtx));

Review Comment:
   this should not be negated, no?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490165060


##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java:
##########
@@ -3281,12 +3257,152 @@ private static class HorizontalPaymentAllocationContext implements LoopContext {
     private boolean isInterestRecalculationSupported(TransactionCtx ctx, Loan loan) {
         if (ctx instanceof ProgressiveTransactionCtx progressiveTransactionCtx) {
             return loan.isInterestBearingAndInterestRecalculationEnabled() && !progressiveTransactionCtx.isChargedOff()
-                    && !progressiveTransactionCtx.isWrittenOff() && !progressiveTransactionCtx.isContractTerminated();
+                    && !progressiveTransactionCtx.isWrittenOff() && !progressiveTransactionCtx.isContractTerminated()
+                    && keepUsingEmiCalculatorBasedOnReAge(progressiveTransactionCtx);
         } else {
             return false;
         }
     }
 
+    private void handleReAgeWithCommonStrategy(LoanTransaction loanTransaction, CommonReAgeSettings settings, TransactionCtx ctx) {

Review Comment:
   This should be part of the calculator and the model, not the transaction processor.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490152840


##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +57,7 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
                         || !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
     }
 
+    public Optional<LoanTransaction> getActiveReageTransactionOnDate(Loan loan, LocalDate onDate) {

Review Comment:
   I think we are reprocessing all transactions if it was not the last txn. I dont think we need to involve reaging into this, or do we?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490495056


##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java:
##########
@@ -1540,12 +1549,12 @@ private boolean shouldUseEmiCalculation(TransactionCtx transactionCtx, LocalDate
         if (transactionCtx instanceof ProgressiveTransactionCtx progressiveTransactionCtx) {
             boolean hasActiveReAmortization = progressiveTransactionCtx.getAlreadyProcessedTransactions().stream()
                     .anyMatch(t -> t.getTypeOf().isReAmortize() && t.isNotReversed());
-            boolean hasActiveReAge = progressiveTransactionCtx.getAlreadyProcessedTransactions().stream()
-                    .anyMatch(t -> t.getTypeOf().isReAge() && t.isNotReversed());
+            boolean hasReAge = progressiveTransactionCtx.getReAgeInterestHandlingType() != null;
             if (hasActiveReAmortization) {
                 return false;
             } else {
-                return !hasActiveReAge || !DateUtils.isAfter(transactionDate, progressiveTransactionCtx.getModel().getMaturityDate());
+                return !hasReAge || !(DateUtils.isAfter(transactionDate, progressiveTransactionCtx.getModel().getMaturityDate())
+                        || !keepUsingEmiCalculatorBasedOnReAge(progressiveTransactionCtx));

Review Comment:
   I believe below is equivalent:
   ```
   return !hasReAge || (!DateUtils.isAfter(transactionDate, progressiveTransactionCtx.getModel().getMaturityDate())
                           && keepUsingEmiCalculatorBasedOnReAge(progressiveTransactionCtx));
              



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490490296


##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java:
##########
@@ -3357,4 +3473,121 @@ private void handleReAgeWithInterestRecalculationEnabled(final LoanTransaction l
                 Money.of(currency, interestFromZeroedInstallments.get()), Money.zero(currency), Money.zero(currency));
         reprocessInstallments(installments);
     }
+
+    private boolean keepUsingEmiCalculatorBasedOnReAge(ProgressiveTransactionCtx progressiveTransactionCtx) {
+        boolean hasActiveReAge = progressiveTransactionCtx.getReAgeInterestHandlingType() != null;
+        return !hasActiveReAge || progressiveTransactionCtx.getReAgeInterestHandlingType().equals(LoanReAgeInterestHandlingType.DEFAULT);
+    }
+
+    OutstandingBalances liftOutstandingBalances(List<LoanRepaymentScheduleInstallment> installments, LoanTransaction loanTransaction,
+            MonetaryCurrency currency, boolean skipDownPayments, boolean onlyPlayableInterest, boolean isEqualInstallmentForInterest,
+            boolean isEqualInstallmentForFeesAndPenalties, TransactionCtx ctx) {
+
+        AtomicReference<Money> outstandingPrincipalBalance = new AtomicReference<>(Money.zero(currency));
+        AtomicReference<Money> outstandingInterestBalance = new AtomicReference<>(Money.zero(currency));
+        AtomicReference<Money> outstandingFeesBalance = new AtomicReference<>(Money.zero(currency));
+        AtomicReference<Money> outstandingPenaltiesBalance = new AtomicReference<>(Money.zero(currency));
+        AtomicReference<Money> accruedInterestToMove = new AtomicReference<>(Money.zero(currency));
+        AtomicReference<Money> accruedFeeToMove = new AtomicReference<>(Money.zero(currency));
+        AtomicReference<Money> accruedPenaltyToMove = new AtomicReference<>(Money.zero(currency));
+        final List<LoanCharge> liftedLoanCharges = new ArrayList<>();
+
+        installments.stream().filter(i -> !skipDownPayments || !i.isDownPayment()).forEach(i -> {
+            Money principalOutstanding = i.getPrincipalOutstanding(currency);
+            if (principalOutstanding.isGreaterThanZero()) {
+                outstandingPrincipalBalance.set(outstandingPrincipalBalance.get().add(principalOutstanding));
+                i.addToPrincipal(loanTransaction.getTransactionDate(), principalOutstanding.negated());
+            }
+            Money interestOutstanding = i.getInterestOutstanding(currency);
+            if (isEqualInstallmentForInterest && interestOutstanding.isGreaterThanZero()) {
+                outstandingInterestBalance.set(outstandingInterestBalance.get().add(interestOutstanding));
+                i.addToInterest(loanTransaction.getTransactionDate(), interestOutstanding.negated());
+                BigDecimal paid = MathUtil.nullToZero(i.getInterestPaid());
+                BigDecimal accrued = MathUtil.nullToZero(i.getInterestAccrued());
+                if (paid.compareTo(accrued) < 0) {
+                    accruedInterestToMove.set(Money.of(currency, accrued.subtract(paid)));
+                    i.setInterestAccrued(paid);
+                }
+            }
+
+            if (isEqualInstallmentForFeesAndPenalties) {
+                getLoanChargesOfInstallment(ctx.getCharges(), i, 1)//
+                        .stream()//
+                        .filter(c -> MathUtil.isGreaterThanZero(c.getAmountOutstanding()))//
+                        .forEach(loanInstallmentCharge -> {
+                            // if (MathUtil.nullToZero(loanInstallmentCharge.getAmountPaid()).equals(ZERO)) {
+                            // ctx.getCharges().remove(loanInstallmentCharge);
+                            // loanTransaction.getLoan().getCharges().remove(loanInstallmentCharge);
+                            // }

Review Comment:
   if we dont need it, remove it.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2486088058


##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransactionRepository.java:
##########
@@ -481,4 +481,13 @@ boolean existsNonReversedByLoanAndTypeAndDate(@Param("loan") Loan loan, @Param("
             """)
     CodeValue fetchClassificationCodeValueByTransactionId(@Param("transactionId") Long transactionId);
 
+    @Query("""
+                    SELECT lt FROM LoanTransaction lt
+                    WHERE lt.loan = :loan
+                        AND lt.reversed = false
+                            AND lt.typeOf = :type
+                         AND lt.dateOf <= :onDate
+            """)
+    Optional<LoanTransaction> findNonReversedTransactionActiveOnDate(@Param("loan") Loan loan, @Param("type") LoanTransactionType type,

Review Comment:
   what `findNonReversedTransactionActiveOnDate` method is for?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490480175


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/ProgressiveLoanInterestRefundServiceImpl.java:
##########
@@ -62,8 +62,16 @@ public class ProgressiveLoanInterestRefundServiceImpl implements InterestRefundS
 
     private static void simulateRepaymentForDisbursements(LoanTransaction lt, final AtomicReference<BigDecimal> refundFinal,
             List<LoanTransaction> collect) {
-        collect.add(new LoanTransaction(lt.getLoan(), lt.getLoan().getOffice(), lt.getTypeOf(), lt.getDateOf(), lt.getAmount(),
-                BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, false, null, null));
+        LoanTransaction copy = new LoanTransaction(lt.getLoan(), lt.getLoan().getOffice(), lt.getTypeOf(), lt.getDateOf(), lt.getAmount(),
+                BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, false, null, null);
+        if (LoanTransactionType.CHARGE_PAYMENT.equals(copy.getTypeOf())
+                || LoanTransactionType.REPAYMENT_AT_DISBURSEMENT.equals(copy.getTypeOf())) {
+            copy.getLoanChargesPaid().addAll(copy.getLoanChargesPaid());
+        }
+        if (LoanTransactionType.REAGE.equals(copy.getTypeOf())) {
+            copy.setLoanReAgeParameter(lt.getLoanReAgeParameter().getCopy(copy));
+        }
+        collect.add(copy);

Review Comment:
   what about reamortization?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2486075587


##########
fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/initializer/global/LoanProductGlobalInitializerStep.java:
##########
@@ -2221,6 +2221,79 @@ public void initialize() throws Exception {
                 TestContextKey.DEFAULT_LOAN_PRODUCT_CREATE_RESPONSE_LP2_ADV_PYMNT_INTEREST_DAILY_EMI_360_30_INTEREST_RECALC_DAILY_MULTIDISBURSE,
                 responseLoanProductsRequestLP2AdvancedpaymentInterestEmi36030InterestRecalcDailyMultiDisburseDisbursementCharge);
 
+        // LP2 with progressive loan schedule + horizontal + interest recalculation daily EMI + 360/30 +
+        // multidisbursement
+        // Frequency for recalculate Outstanding Principal: Daily, Frequency Interval for recalculation: 1
+        // chargeback - interest, fee, principal, penalty
+        String name150 = DefaultLoanProduct.LP2_ADV_PYMNT_INTEREST_DAILY_EMI_360_30_INTEREST_RECALC_DAILY_MULTIDISBURSE_EXPECT_TRANCHE_CHARGEBACK
+                .getName();
+        PostLoanProductsRequest loanProductsRequestLP2AdvancedpaymentInterestEmi36030InterestRecalcDailyMultiDisburseExpectTrancheChargeback = loanProductsRequestFactory
+                .defaultLoanProductsRequestLP2Emi()//
+                .name(name150)//
+                .daysInYearType(DaysInYearType.DAYS360.value)//
+                .daysInMonthType(DaysInMonthType.DAYS30.value)//
+                .isInterestRecalculationEnabled(true)//
+                .preClosureInterestCalculationStrategy(1)//
+                .rescheduleStrategyMethod(4)//
+                .interestRecalculationCompoundingMethod(0)//
+                .recalculationRestFrequencyType(2)//
+                .recalculationRestFrequencyInterval(1)//
+                .creditAllocation(List.of(//
+                        createCreditAllocation("CHARGEBACK", List.of("INTEREST", "FEE", "PRINCIPAL", "PENALTY"))//
+                ))//
+                .paymentAllocation(List.of(//
+                        createPaymentAllocation("DEFAULT", "NEXT_INSTALLMENT"), //
+                        createPaymentAllocation("GOODWILL_CREDIT", "LAST_INSTALLMENT"), //
+                        createPaymentAllocation("MERCHANT_ISSUED_REFUND", "REAMORTIZATION"), //
+                        createPaymentAllocation("PAYOUT_REFUND", "NEXT_INSTALLMENT")))//
+                .multiDisburseLoan(true)//
+                .disallowExpectedDisbursements(false)//

Review Comment:
   why do we need "expected disbursements"?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490168941


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/ProgressiveLoanInterestRefundServiceImpl.java:
##########
@@ -62,8 +62,16 @@ public class ProgressiveLoanInterestRefundServiceImpl implements InterestRefundS
 
     private static void simulateRepaymentForDisbursements(LoanTransaction lt, final AtomicReference<BigDecimal> refundFinal,
             List<LoanTransaction> collect) {
-        collect.add(new LoanTransaction(lt.getLoan(), lt.getLoan().getOffice(), lt.getTypeOf(), lt.getDateOf(), lt.getAmount(),
-                BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, false, null, null));
+        LoanTransaction copy = new LoanTransaction(lt.getLoan(), lt.getLoan().getOffice(), lt.getTypeOf(), lt.getDateOf(), lt.getAmount(),
+                BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, false, null, null);
+        if (LoanTransactionType.CHARGE_PAYMENT.equals(copy.getTypeOf())
+                || LoanTransactionType.REPAYMENT_AT_DISBURSEMENT.equals(copy.getTypeOf())) {
+            copy.getLoanChargesPaid().addAll(copy.getLoanChargesPaid());
+        }
+        if (LoanTransactionType.REAGE.equals(copy.getTypeOf())) {
+            copy.setLoanReAgeParameter(lt.getLoanReAgeParameter().getCopy(copy));
+        }
+        collect.add(copy);

Review Comment:
   why do we need this?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490504240


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionProcessingServiceImpl.java:
##########
@@ -221,7 +222,11 @@ public LoanScheduleDTO getRecalculatedSchedule(final ScheduleGeneratorDTO genera
     public OutstandingAmountsDTO fetchPrepaymentDetail(final ScheduleGeneratorDTO scheduleGeneratorDTO, final LocalDate onDate, Loan loan) {
         OutstandingAmountsDTO outstandingAmounts;
 
-        if (loan.isInterestBearingAndInterestRecalculationEnabled() && !loan.isChargeOffOnDate(onDate)) {
+        Optional<LoanTransaction> activeReageTransactionOnDate = loanTransactionService.getActiveReageTransactionOnDate(loan, onDate);
+
+        if (loan.isInterestBearingAndInterestRecalculationEnabled() && !loan.isChargeOffOnDate(onDate)
+                && (activeReageTransactionOnDate.isEmpty() || LoanReAgeInterestHandlingType.DEFAULT

Review Comment:
   its incorrect. What you need is if the loan is reaged AND EQUAL AMORTIZATION, then use the `getTotalOutstandingLoan()`



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "somasorosdpc (via GitHub)" <gi...@apache.org>.
somasorosdpc commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2489586342


##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanTransactionRepository.java:
##########
@@ -481,4 +481,13 @@ boolean existsNonReversedByLoanAndTypeAndDate(@Param("loan") Loan loan, @Param("
             """)
     CodeValue fetchClassificationCodeValueByTransactionId(@Param("transactionId") Long transactionId);
 
+    @Query("""
+                    SELECT lt FROM LoanTransaction lt
+                    WHERE lt.loan = :loan
+                        AND lt.reversed = false
+                            AND lt.typeOf = :type
+                         AND lt.dateOf <= :onDate
+            """)
+    Optional<LoanTransaction> findNonReversedTransactionActiveOnDate(@Param("loan") Loan loan, @Param("type") LoanTransactionType type,

Review Comment:
   It is needed for fetch last active ReAge transaction.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490141160


##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java:
##########
@@ -1824,6 +1825,14 @@ public boolean hasContractTerminationTransaction() {
         return getLoanTransactions().stream().anyMatch(t -> t.isContractTermination() && t.isNotReversed());
     }
 
+    public LoanReAgeInterestHandlingType getReAgeInterestHandlingType() {

Review Comment:
   This is not correct. We can have multiple reaging on a loan.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "MarianaDmytrivBinariks (via GitHub)" <gi...@apache.org>.
MarianaDmytrivBinariks commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2493923795


##########
fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/initializer/global/LoanProductGlobalInitializerStep.java:
##########
@@ -2221,6 +2221,79 @@ public void initialize() throws Exception {
                 TestContextKey.DEFAULT_LOAN_PRODUCT_CREATE_RESPONSE_LP2_ADV_PYMNT_INTEREST_DAILY_EMI_360_30_INTEREST_RECALC_DAILY_MULTIDISBURSE,
                 responseLoanProductsRequestLP2AdvancedpaymentInterestEmi36030InterestRecalcDailyMultiDisburseDisbursementCharge);
 
+        // LP2 with progressive loan schedule + horizontal + interest recalculation daily EMI + 360/30 +
+        // multidisbursement
+        // Frequency for recalculate Outstanding Principal: Daily, Frequency Interval for recalculation: 1
+        // chargeback - interest, fee, principal, penalty
+        String name150 = DefaultLoanProduct.LP2_ADV_PYMNT_INTEREST_DAILY_EMI_360_30_INTEREST_RECALC_DAILY_MULTIDISBURSE_EXPECT_TRANCHE_CHARGEBACK
+                .getName();
+        PostLoanProductsRequest loanProductsRequestLP2AdvancedpaymentInterestEmi36030InterestRecalcDailyMultiDisburseExpectTrancheChargeback = loanProductsRequestFactory
+                .defaultLoanProductsRequestLP2Emi()//
+                .name(name150)//
+                .daysInYearType(DaysInYearType.DAYS360.value)//
+                .daysInMonthType(DaysInMonthType.DAYS30.value)//
+                .isInterestRecalculationEnabled(true)//
+                .preClosureInterestCalculationStrategy(1)//
+                .rescheduleStrategyMethod(4)//
+                .interestRecalculationCompoundingMethod(0)//
+                .recalculationRestFrequencyType(2)//
+                .recalculationRestFrequencyInterval(1)//
+                .creditAllocation(List.of(//
+                        createCreditAllocation("CHARGEBACK", List.of("INTEREST", "FEE", "PRINCIPAL", "PENALTY"))//
+                ))//
+                .paymentAllocation(List.of(//
+                        createPaymentAllocation("DEFAULT", "NEXT_INSTALLMENT"), //
+                        createPaymentAllocation("GOODWILL_CREDIT", "LAST_INSTALLMENT"), //
+                        createPaymentAllocation("MERCHANT_ISSUED_REFUND", "REAMORTIZATION"), //
+                        createPaymentAllocation("PAYOUT_REFUND", "NEXT_INSTALLMENT")))//
+                .multiDisburseLoan(true)//
+                .disallowExpectedDisbursements(false)//

Review Comment:
   removed



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "MarianaDmytrivBinariks (via GitHub)" <gi...@apache.org>.
MarianaDmytrivBinariks commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2493812253


##########
fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/initializer/global/LoanProductGlobalInitializerStep.java:
##########
@@ -2221,6 +2221,79 @@ public void initialize() throws Exception {
                 TestContextKey.DEFAULT_LOAN_PRODUCT_CREATE_RESPONSE_LP2_ADV_PYMNT_INTEREST_DAILY_EMI_360_30_INTEREST_RECALC_DAILY_MULTIDISBURSE,
                 responseLoanProductsRequestLP2AdvancedpaymentInterestEmi36030InterestRecalcDailyMultiDisburseDisbursementCharge);
 
+        // LP2 with progressive loan schedule + horizontal + interest recalculation daily EMI + 360/30 +
+        // multidisbursement
+        // Frequency for recalculate Outstanding Principal: Daily, Frequency Interval for recalculation: 1
+        // chargeback - interest, fee, principal, penalty
+        String name150 = DefaultLoanProduct.LP2_ADV_PYMNT_INTEREST_DAILY_EMI_360_30_INTEREST_RECALC_DAILY_MULTIDISBURSE_EXPECT_TRANCHE_CHARGEBACK
+                .getName();
+        PostLoanProductsRequest loanProductsRequestLP2AdvancedpaymentInterestEmi36030InterestRecalcDailyMultiDisburseExpectTrancheChargeback = loanProductsRequestFactory
+                .defaultLoanProductsRequestLP2Emi()//
+                .name(name150)//
+                .daysInYearType(DaysInYearType.DAYS360.value)//
+                .daysInMonthType(DaysInMonthType.DAYS30.value)//
+                .isInterestRecalculationEnabled(true)//
+                .preClosureInterestCalculationStrategy(1)//
+                .rescheduleStrategyMethod(4)//
+                .interestRecalculationCompoundingMethod(0)//
+                .recalculationRestFrequencyType(2)//
+                .recalculationRestFrequencyInterval(1)//
+                .creditAllocation(List.of(//
+                        createCreditAllocation("CHARGEBACK", List.of("INTEREST", "FEE", "PRINCIPAL", "PENALTY"))//
+                ))//
+                .paymentAllocation(List.of(//
+                        createPaymentAllocation("DEFAULT", "NEXT_INSTALLMENT"), //
+                        createPaymentAllocation("GOODWILL_CREDIT", "LAST_INSTALLMENT"), //
+                        createPaymentAllocation("MERCHANT_ISSUED_REFUND", "REAMORTIZATION"), //
+                        createPaymentAllocation("PAYOUT_REFUND", "NEXT_INSTALLMENT")))//
+                .multiDisburseLoan(true)//
+                .disallowExpectedDisbursements(false)//

Review Comment:
   I thought about additional test scenarios with multidisb loan that expect tranches and re-aging between two disbursements, will add this scenario



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490167655


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java:
##########
@@ -3061,9 +3063,19 @@ public void handleChargebackTransaction(final Loan loan, LoanTransaction chargeb
         loan.addLoanTransaction(chargebackTransaction);
         if (loan.isInterestBearing() && loan.isInterestRecalculationEnabled()) {
             final List<LoanTransaction> transactions = loanTransactionService.retrieveListOfTransactionsForReprocessing(loan);
-            loanTransactionProcessingService.reprocessLoanTransactions(loan.getTransactionProcessingStrategyCode(),
-                    loan.getDisbursementDate(), transactions, loan.getCurrency(), loan.getRepaymentScheduleInstallments(),
-                    loan.getActiveCharges());
+            if (transactions.stream().filter(LoanTransaction::isNotReversed).filter(LoanTransaction::isReAge)
+                    .anyMatch(lt -> !lt.getTransactionDate().isAfter(lt.getLoan().getMaturityDate()))) {
+                final ScheduleGeneratorDTO scheduleGeneratorDTO = loanUtilService.buildScheduleGeneratorDTO(loan, null);
+                loanScheduleService.regenerateRepaymentSchedule(loan, scheduleGeneratorDTO);
+            }
+            ChangedTransactionDetail changedTransactionDetail = loanTransactionProcessingService.reprocessLoanTransactions(
+                    loan.getTransactionProcessingStrategyCode(), loan.getDisbursementDate(), transactions, loan.getCurrency(),
+                    loan.getRepaymentScheduleInstallments(), loan.getActiveCharges());
+            // todo check code if we can simplify it
+            if (changedTransactionDetail.getTransactionChanges() != null && !changedTransactionDetail.getTransactionChanges().isEmpty()) {
+                changedTransactionDetail.getTransactionChanges().stream().filter(change -> change.getNewTransaction() != null)
+                        .map(TransactionChangeData::getNewTransaction).forEach(loan::addLoanTransaction);
+            }

Review Comment:
   Why do we need all these logic?



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "somasorosdpc (via GitHub)" <gi...@apache.org>.
somasorosdpc commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2542123641


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/ProgressiveLoanInterestRefundServiceImpl.java:
##########
@@ -62,8 +62,16 @@ public class ProgressiveLoanInterestRefundServiceImpl implements InterestRefundS
 
     private static void simulateRepaymentForDisbursements(LoanTransaction lt, final AtomicReference<BigDecimal> refundFinal,
             List<LoanTransaction> collect) {
-        collect.add(new LoanTransaction(lt.getLoan(), lt.getLoan().getOffice(), lt.getTypeOf(), lt.getDateOf(), lt.getAmount(),
-                BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, false, null, null));
+        LoanTransaction copy = new LoanTransaction(lt.getLoan(), lt.getLoan().getOffice(), lt.getTypeOf(), lt.getDateOf(), lt.getAmount(),
+                BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, BigDecimal.ZERO, false, null, null);
+        if (LoanTransactionType.CHARGE_PAYMENT.equals(copy.getTypeOf())
+                || LoanTransactionType.REPAYMENT_AT_DISBURSEMENT.equals(copy.getTypeOf())) {
+            copy.getLoanChargesPaid().addAll(copy.getLoanChargesPaid());
+        }
+        if (LoanTransactionType.REAGE.equals(copy.getTypeOf())) {
+            copy.setLoanReAgeParameter(lt.getLoanReAgeParameter().getCopy(copy));
+        }
+        collect.add(copy);

Review Comment:
   reamortization case added.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "somasorosdpc (via GitHub)" <gi...@apache.org>.
somasorosdpc commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2489603544


##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +57,7 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
                         || !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
     }
 
+    public Optional<LoanTransaction> getActiveReageTransactionOnDate(Loan loan, LocalDate onDate) {

Review Comment:
   At the moment ProgressiveTransactionContext holds the last applied ReAge transaction type or null if no reage is happend. To perform processLatestTransaction, we need to fetch this transaction to create the correct context for perform it.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490157440


##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/ProgressiveTransactionCtx.java:
##########
@@ -45,22 +46,27 @@ public class ProgressiveTransactionCtx extends TransactionCtx {
     @Setter
     private boolean isWrittenOff = false;
     @Setter
+    private LoanReAgeInterestHandlingType reAgeInterestHandlingType;

Review Comment:
   i dont think we need this. The reage transaction has all the required reage parameters. It should not be stored in the context as multiple reage txn can be on a loan.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490489009


##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/ProgressiveTransactionCtx.java:
##########
@@ -45,22 +46,27 @@ public class ProgressiveTransactionCtx extends TransactionCtx {
     @Setter
     private boolean isWrittenOff = false;
     @Setter
+    private LoanReAgeInterestHandlingType reAgeInterestHandlingType;

Review Comment:
   it should be a simple boolean which is set if the reage transaction was reprocessed and we should not use further the emi calculator



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2486089977


##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanTransactionService.java:
##########
@@ -56,4 +57,7 @@ private Predicate<LoanTransaction> loanTransactionForReprocessingPredicate() {
                         || !transaction.isNonMonetaryTransaction() || transaction.isContractTermination());
     }
 
+    public Optional<LoanTransaction> getActiveReageTransactionOnDate(Loan loan, LocalDate onDate) {

Review Comment:
   A loan can have multiple reage txn. The only restriction is we cannot have multiple reage on the same date, but otherwise we can have as many reage as many we need.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5102:
URL: https://github.com/apache/fineract/pull/5102#discussion_r2490165060


##########
fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java:
##########
@@ -3281,12 +3257,152 @@ private static class HorizontalPaymentAllocationContext implements LoopContext {
     private boolean isInterestRecalculationSupported(TransactionCtx ctx, Loan loan) {
         if (ctx instanceof ProgressiveTransactionCtx progressiveTransactionCtx) {
             return loan.isInterestBearingAndInterestRecalculationEnabled() && !progressiveTransactionCtx.isChargedOff()
-                    && !progressiveTransactionCtx.isWrittenOff() && !progressiveTransactionCtx.isContractTerminated();
+                    && !progressiveTransactionCtx.isWrittenOff() && !progressiveTransactionCtx.isContractTerminated()
+                    && keepUsingEmiCalculatorBasedOnReAge(progressiveTransactionCtx);
         } else {
             return false;
         }
     }
 
+    private void handleReAgeWithCommonStrategy(LoanTransaction loanTransaction, CommonReAgeSettings settings, TransactionCtx ctx) {

Review Comment:
   This should be part of the calculator and the model, not the transaction processor.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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


Re: [PR] FINERACT-2354: Re-aging Interest Handling Option: Equal Amortization [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #5102:
URL: https://github.com/apache/fineract/pull/5102


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

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