Skip to content

Comments

FINERACT-2421: Disbursement charge amount incorrect calculation#5428

Merged
adamsaghy merged 1 commit intoapache:developfrom
openMF:FINERACT-2421/disbursement-charge-amount-incorrect-calculation
Feb 5, 2026
Merged

FINERACT-2421: Disbursement charge amount incorrect calculation#5428
adamsaghy merged 1 commit intoapache:developfrom
openMF:FINERACT-2421/disbursement-charge-amount-incorrect-calculation

Conversation

@alberto-art3ch
Copy link
Contributor

Description

Disbursement charge amount incorrect calculation for full/partial disbursement. Repayment schedule and charges Charges: values are not based on disb amount+interest

FINERACT-2421

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.

Your assigned reviewer(s) will follow our guidelines for code reviews.

@@ -853,8 +853,14 @@ private void update(final LoanCharge loanCharge, final BigDecimal amount, final
private Money getTotalAllTrancheDisbursementAmount(final Loan loan) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method can be refactored to improve readability and maintainability. Consider:

Flattening the nested conditionals and using early returns to clarify the control flow.

Assigning loan.getDisbursementDetails() to a local variable to avoid repeated calls.

Replacing the manual loop for summing disbursements with a stream-based reduce operation.

Structuring the “sum disbursements” and “fallback principal” paths separately to make the logic clearer.

Optionally, renaming the method to better reflect its behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code updated, please review @Aman-Mittal

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2421/disbursement-charge-amount-incorrect-calculation branch from 0c66de4 to 415b61f Compare January 31, 2026 15:12
Copy link
Contributor

@Aman-Mittal Aman-Mittal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my review

| 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test skipped due to flaky test? just curious

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was skipped now has been fixed and available for testing

}
return amount;
return Money.zero(loan.getCurrency());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! code updated

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2421/disbursement-charge-amount-incorrect-calculation branch 2 times, most recently from f4a5012 to e923c73 Compare February 3, 2026 14:39
@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2421/disbursement-charge-amount-incorrect-calculation branch from e923c73 to 76a73c6 Compare February 4, 2026 04:04
@adamsaghy adamsaghy merged commit 1d1876e into apache:develop Feb 5, 2026
36 checks passed
@adamsaghy adamsaghy deleted the FINERACT-2421/disbursement-charge-amount-incorrect-calculation branch February 5, 2026 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants