Copilot commented on code in PR #5693:
URL: https://github.com/apache/fineract/pull/5693#discussion_r2986395824
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanDisbursementValidator.java:
##########
@@ -39,29 +40,59 @@ public void
compareDisbursedToApprovedOrProposedPrincipal(final Loan loan, final
final BigDecimal totalCapitalizedIncomeAdjustment =
MathUtil.nullToZero(loan.getSummary().getTotalCapitalizedIncomeAdjustment());
final BigDecimal netCapitalizedIncome =
totalCapitalizedIncome.subtract(totalCapitalizedIncomeAdjustment);
- if (loan.loanProduct().isAllowApprovedDisbursedAmountsOverApplied()) {
- // Validate total disbursed amount (after this transaction)
against max allowed
- validateOverMaximumAmount(loan, totalDisbursed,
netCapitalizedIncome);
- } else {
- if ((totalDisbursed.compareTo(loan.getApprovedPrincipal()) > 0)
- ||
(totalDisbursed.add(netCapitalizedIncome).compareTo(loan.getApprovedPrincipal())
> 0)) {
- final String errorMsg = "Loan can't be disbursed, disburse
amount is exceeding approved principal.";
- throw new LoanDisbursalException(errorMsg,
"disburse.amount.must.be.less.than.approved.principal", totalDisbursed,
- loan.getApprovedPrincipal());
+ // Always use the same logic for over-applied config and fallback
+ LoanProduct loanProduct = loan.getLoanProduct();
+ BigDecimal approvedPrincipal = loan.getApprovedPrincipal();
+ BigDecimal maxAllowed = approvedPrincipal;
+
+ if (loanProduct.getOverAppliedCalculationType() != null &&
loanProduct.getOverAppliedNumber() != null) {
+ BigDecimal overAppliedNumber =
BigDecimal.valueOf(loanProduct.getOverAppliedNumber());
+ if
("percentage".equalsIgnoreCase(loanProduct.getOverAppliedCalculationType())) {
+ BigDecimal extra =
approvedPrincipal.multiply(overAppliedNumber).divide(BigDecimal.valueOf(100));
+ maxAllowed = approvedPrincipal.add(extra);
+ } else {
+ // ABSOLUTE (flat)
+ maxAllowed = approvedPrincipal.add(overAppliedNumber);
}
}
+
+ BigDecimal total = totalDisbursed.add(netCapitalizedIncome);
+ if (maxAllowed == null) {
+ maxAllowed = approvedPrincipal;
+ }
Review Comment:
This over-applied limit calculation bypasses
`LoanProduct#isAllowApprovedDisbursedAmountsOverApplied()` and
`LoanApplicationValidator#getOverAppliedMax(loan)`. That changes behavior in at
least two cases: (1) if the allow flag is false but calculationType/number are
set, this now still permits over-applied disbursement; (2) if over-applied
config is absent, the previous path used `getOverAppliedMax()` (which falls
back to proposed principal and handles approved-amount modifications) but this
now falls back to approved principal only. Consider gating on the allow flag
and delegating max calculation to
`loanApplicationValidator.getOverAppliedMax(loan)` to preserve the established
rules.
```suggestion
LoanProduct loanProduct = loan.getLoanProduct();
BigDecimal maxAllowed;
if (loanProduct != null &&
loanProduct.isAllowApprovedDisbursedAmountsOverApplied()) {
// Delegate over-applied max calculation to
LoanApplicationValidator to preserve established rules
maxAllowed = loanApplicationValidator.getOverAppliedMax(loan);
} else {
// When over-applied disbursements are not allowed, cap at
approved principal
maxAllowed = loan.getApprovedPrincipal();
}
BigDecimal total = totalDisbursed.add(netCapitalizedIncome);
```
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanDisbursementValidator.java:
##########
@@ -39,29 +40,59 @@ public void
compareDisbursedToApprovedOrProposedPrincipal(final Loan loan, final
final BigDecimal totalCapitalizedIncomeAdjustment =
MathUtil.nullToZero(loan.getSummary().getTotalCapitalizedIncomeAdjustment());
final BigDecimal netCapitalizedIncome =
totalCapitalizedIncome.subtract(totalCapitalizedIncomeAdjustment);
- if (loan.loanProduct().isAllowApprovedDisbursedAmountsOverApplied()) {
- // Validate total disbursed amount (after this transaction)
against max allowed
- validateOverMaximumAmount(loan, totalDisbursed,
netCapitalizedIncome);
- } else {
- if ((totalDisbursed.compareTo(loan.getApprovedPrincipal()) > 0)
- ||
(totalDisbursed.add(netCapitalizedIncome).compareTo(loan.getApprovedPrincipal())
> 0)) {
- final String errorMsg = "Loan can't be disbursed, disburse
amount is exceeding approved principal.";
- throw new LoanDisbursalException(errorMsg,
"disburse.amount.must.be.less.than.approved.principal", totalDisbursed,
- loan.getApprovedPrincipal());
+ // Always use the same logic for over-applied config and fallback
+ LoanProduct loanProduct = loan.getLoanProduct();
+ BigDecimal approvedPrincipal = loan.getApprovedPrincipal();
+ BigDecimal maxAllowed = approvedPrincipal;
+
+ if (loanProduct.getOverAppliedCalculationType() != null &&
loanProduct.getOverAppliedNumber() != null) {
+ BigDecimal overAppliedNumber =
BigDecimal.valueOf(loanProduct.getOverAppliedNumber());
+ if
("percentage".equalsIgnoreCase(loanProduct.getOverAppliedCalculationType())) {
+ BigDecimal extra =
approvedPrincipal.multiply(overAppliedNumber).divide(BigDecimal.valueOf(100));
+ maxAllowed = approvedPrincipal.add(extra);
+ } else {
+ // ABSOLUTE (flat)
+ maxAllowed = approvedPrincipal.add(overAppliedNumber);
}
}
+
+ BigDecimal total = totalDisbursed.add(netCapitalizedIncome);
+ if (maxAllowed == null) {
+ maxAllowed = approvedPrincipal;
+ }
+ if (total.compareTo(maxAllowed) > 0) {
+ throw new LoanDisbursalException(
+ "Disbursement exceeds allowed limit including over-applied
threshold",
+ "disburse.amount.must.be.less.than.or.equal.to.max.allowed",
total, maxAllowed);
+ }
}
public void validateOverMaximumAmount(final Loan loan, final BigDecimal
totalDisbursed, final BigDecimal capitalizedIncome) {
- final BigDecimal maxDisbursedAmount =
loanApplicationValidator.getOverAppliedMax(loan);
- if
(totalDisbursed.add(capitalizedIncome).compareTo(maxDisbursedAmount) > 0) {
+ LoanProduct loanProduct = loan.getLoanProduct();
+ BigDecimal approvedPrincipal = loan.getApprovedPrincipal();
+ BigDecimal maxAllowed = approvedPrincipal;
+
+ if (loanProduct.getOverAppliedCalculationType() != null &&
loanProduct.getOverAppliedNumber() != null) {
+ BigDecimal overAppliedNumber =
BigDecimal.valueOf(loanProduct.getOverAppliedNumber());
+ if
("percentage".equalsIgnoreCase(loanProduct.getOverAppliedCalculationType())) {
+ BigDecimal extra =
approvedPrincipal.multiply(overAppliedNumber).divide(BigDecimal.valueOf(100));
+ maxAllowed = approvedPrincipal.add(extra);
+ } else {
+ // ABSOLUTE (flat)
+ maxAllowed = approvedPrincipal.add(overAppliedNumber);
+ }
+ }
+
Review Comment:
The max-allowed computation logic is duplicated here and in
`compareDisbursedToApprovedOrProposedPrincipal()`, and it no longer uses the
existing centralized helper (`LoanApplicationValidator#getOverAppliedMax`).
This increases the risk of inconsistencies (e.g., approved-amount modification
handling) and leaves `loanApplicationValidator` unused in this class. Suggest
extracting a single helper for max calculation (or delegating to
`getOverAppliedMax`) and calling it from both entry points; if this method is
no longer needed, consider removing it or making it private to avoid dead API
surface.
```suggestion
BigDecimal approvedPrincipal = loan.getApprovedPrincipal();
BigDecimal maxAllowed =
loanApplicationValidator.getOverAppliedMax(loan);
```
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanDisbursementValidator.java:
##########
@@ -39,29 +40,59 @@ public void
compareDisbursedToApprovedOrProposedPrincipal(final Loan loan, final
final BigDecimal totalCapitalizedIncomeAdjustment =
MathUtil.nullToZero(loan.getSummary().getTotalCapitalizedIncomeAdjustment());
final BigDecimal netCapitalizedIncome =
totalCapitalizedIncome.subtract(totalCapitalizedIncomeAdjustment);
- if (loan.loanProduct().isAllowApprovedDisbursedAmountsOverApplied()) {
- // Validate total disbursed amount (after this transaction)
against max allowed
- validateOverMaximumAmount(loan, totalDisbursed,
netCapitalizedIncome);
- } else {
- if ((totalDisbursed.compareTo(loan.getApprovedPrincipal()) > 0)
- ||
(totalDisbursed.add(netCapitalizedIncome).compareTo(loan.getApprovedPrincipal())
> 0)) {
- final String errorMsg = "Loan can't be disbursed, disburse
amount is exceeding approved principal.";
- throw new LoanDisbursalException(errorMsg,
"disburse.amount.must.be.less.than.approved.principal", totalDisbursed,
- loan.getApprovedPrincipal());
+ // Always use the same logic for over-applied config and fallback
+ LoanProduct loanProduct = loan.getLoanProduct();
+ BigDecimal approvedPrincipal = loan.getApprovedPrincipal();
+ BigDecimal maxAllowed = approvedPrincipal;
+
+ if (loanProduct.getOverAppliedCalculationType() != null &&
loanProduct.getOverAppliedNumber() != null) {
+ BigDecimal overAppliedNumber =
BigDecimal.valueOf(loanProduct.getOverAppliedNumber());
+ if
("percentage".equalsIgnoreCase(loanProduct.getOverAppliedCalculationType())) {
+ BigDecimal extra =
approvedPrincipal.multiply(overAppliedNumber).divide(BigDecimal.valueOf(100));
+ maxAllowed = approvedPrincipal.add(extra);
+ } else {
+ // ABSOLUTE (flat)
+ maxAllowed = approvedPrincipal.add(overAppliedNumber);
}
}
+
+ BigDecimal total = totalDisbursed.add(netCapitalizedIncome);
+ if (maxAllowed == null) {
+ maxAllowed = approvedPrincipal;
+ }
Review Comment:
`maxAllowed` is initialized from `approvedPrincipal`, so this
null-check/fallback is redundant. If `approvedPrincipal` can be null, assigning
it back to `maxAllowed` doesn’t prevent a later NPE in `compareTo`; otherwise
this block can be removed to simplify the flow.
--
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]
