Aman-Mittal commented on code in PR #5428:
URL: https://github.com/apache/fineract/pull/5428#discussion_r2749672532


##########
fineract-e2e-tests-runner/src/test/resources/features/LoanCharge.feature:
##########
@@ -4097,20 +4095,20 @@ Feature: LoanCharge
     Then Loan status has changed to "Approved"
     Then Loan Repayment schedule has 6 periods, with the following data for 
periods:
       | Nr | Days | Date             | Paid date | Balance of loan | Principal 
due | Interest | Fees | Penalties | Due   | Paid | In advance | Late | 
Outstanding |
-      |    |      | 01 January 2024  |           | 100.0           |           
    |          | 0.04 |           | 0.04  |      |            |      | 0.02     
    |
-      | 1  | 31   | 01 February 2024 |           | 83.57           | 16.43     
    | 0.58     | 0.0  | 0.0       | 17.01 | 0.0  | 0.0        | 0.0  | 17.01    
   |
-      | 2  | 29   | 01 March 2024    |           | 67.05           | 16.52     
    | 0.49     | 0.0  | 0.0       | 17.01 | 0.0  | 0.0        | 0.0  | 17.01    
   |
-      | 3  | 31   | 01 April 2024    |           | 50.43           | 16.62     
    | 0.39     | 0.0  | 0.0       | 17.01 | 0.0  | 0.0        | 0.0  | 17.01    
   |
-      | 4  | 30   | 01 May 2024      |           | 33.71           | 16.72     
    | 0.29     | 0.0  | 0.0       | 17.01 | 0.0  | 0.0        | 0.0  | 17.01    
   |
-      | 5  | 31   | 01 June 2024     |           | 16.9            | 16.81     
    | 0.2      | 0.0  | 0.0       | 17.01 | 0.0  | 0.0        | 0.0  | 17.01    
   |
-      | 6  | 30   | 01 July 2024     |           | 0.0             | 16.9      
    | 0.1      | 0.0  | 0.0       | 17.0  | 0.0  | 0.0        | 0.0  | 17.0     
   |
+      |    |      | 01 January 2024  |           |  70.0           |           
    |          | 0.03 |           | 0.03  |      |            |      | 0.03     
   |
+      | 1  | 31   | 01 February 2024 |           | 58.5            | 11.5      
    | 0.41     | 0.0  | 0.0       | 11.91 | 0.0  | 0.0        | 0.0  | 11.91    
   |
+      | 2  | 29   | 01 March 2024    |           | 46.93           | 11.57     
    | 0.34     | 0.0  | 0.0       | 11.91 | 0.0  | 0.0        | 0.0  | 11.91    
   |
+      | 3  | 31   | 01 April 2024    |           | 35.29           | 11.64     
    | 0.27     | 0.0  | 0.0       | 11.91 | 0.0  | 0.0        | 0.0  | 11.91    
   |
+      | 4  | 30   | 01 May 2024      |           | 23.59           | 11.7      
    | 0.21     | 0.0  | 0.0       | 11.91 | 0.0  | 0.0        | 0.0  | 11.91    
   |
+      | 5  | 31   | 01 June 2024     |           | 11.82           | 11.77     
    | 0.14     | 0.0  | 0.0       | 11.91 | 0.0  | 0.0        | 0.0  | 11.91    
   |
+      | 6  | 30   | 01 July 2024     |           | 0.0             | 11.82     
    | 0.07     | 0.0  | 0.0       | 11.89 | 0.0  | 0.0        | 0.0  | 11.89    
   |
     Then Loan Repayment schedule has the following data in Total row:
       | Principal due | Interest | Fees | Penalties | Due    | Paid  | In 
advance | Late | Outstanding |
-      | 100.0         | 2.05     | 0.04 | 0.0       | 102.09 | 0.0   | 0.0     
   | 0.0  | 102.09      |
+      | 70.0          | 1.44     | 0.03 | 0.0       | 71.47  | 0.00 | 0.0      
  | 0.0  | 71.47        |
     Then Loan Transactions tab has none transaction
     Then Loan Charges tab has the following data:
       | Name                | isPenalty | Payment due at | Due as of | 
Calculation type | Due  | Paid | Waived | Outstanding |
-      | Disbursement Charge | false     | Disbursement   |           | % 
Interest       | 0.04 | 0.04 | 0.0    | 0.0         |
+      | Disbursement Charge | false     | Disbursement   |           | % 
Interest       | 0.03 | 0.0  | 0.0    | 0.03        |
 
   @Skip

Review Comment:
   Is this test skipped due to flaky test? just curious



##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanChargeService.java:
##########
@@ -850,14 +850,20 @@ private void update(final LoanCharge loanCharge, final 
BigDecimal amount, final
         }
     }
 
-    private Money getTotalAllTrancheDisbursementAmount(final Loan loan) {
-        Money amount = Money.zero(loan.getCurrency());
+    private Money sumMultiDisbursementAmounts(final Loan loan) {
         if (loan.isMultiDisburmentLoan()) {
-            for (final LoanDisbursementDetails loanDisbursementDetail : 
loan.getDisbursementDetails()) {
-                amount = amount.plus(loanDisbursementDetail.principal());
+            final List<LoanDisbursementDetails> loanDisbursementDetails = 
loan.getDisbursementDetails();
+            if (loan.isSubmittedAndPendingApproval()) {
+                return Money.of(loan.getCurrency(), 
loan.getProposedPrincipal());
+            } else if (loan.isApproved()) {
+                return Money.of(loan.getCurrency(), 
loan.getApprovedPrincipal());
+            } else if (loan.isOpen() || !loanDisbursementDetails.isEmpty()) {
+                return Money.of(loan.getCurrency(), 
loanDisbursementDetails.stream() //
+                        .map(LoanDisbursementDetails::principal) //
+                        .reduce(BigDecimal.ZERO, BigDecimal::add));
             }
         }
-        return amount;
+        return Money.zero(loan.getCurrency());
     }

Review Comment:
   here is suggestion this may be more cleaner
   
   private Money sumMultiDisbursementAmounts(final Loan loan) {
       if (!loan.isMultiDisburmentLoan()) {
           return Money.zero(loan.getCurrency());
       }
   
       final List<LoanDisbursementDetails> disbursements = 
loan.getDisbursementDetails();
       final String currency = loan.getCurrency();
   
       if (loan.isSubmittedAndPendingApproval()) {
           return Money.of(currency, loan.getProposedPrincipal());
       }
   
       if (loan.isApproved()) {
           return Money.of(currency, loan.getApprovedPrincipal());
       }
   
       // For open loans or any loans with disbursement details
       BigDecimal totalPrincipal = disbursements.stream()
               .map(LoanDisbursementDetails::principal)
               .reduce(BigDecimal.ZERO, BigDecimal::add);
   
       return Money.of(currency, totalPrincipal);
   }
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to