FINERACT-2593: when loan product is null

[mansi75]

Description

JIRA: https://issues.apache.org/jira/browse/FINERACT-2593

Problem

DelinquencyReadPlatformServiceImpl.calculateLoanCollectionData contained two bugs that caused an unhandled NullPointerException (HTTP 500) when loan.getLoanProduct() returns null:

Bug 1 — Inverted null guard (pre-approval / pending state branch)
The condition loan.getLoanProduct() == null || used the wrong operator. When getLoanProduct() returns null, the left-hand operand evaluates to true, short-circuiting the || and routing execution directly into calculateAvailableDisbursementAmountWithOverApplied(loan). The helper immediately dereferences loanProduct with no null check, producing an NPE. The intended operator is != null &&.

Bug 2 — No null guard on active-loan branch
The active (open) loan branch called calculateAvailableDisbursementAmountWithOverApplied(loan) unconditionally with no null check on getLoanProduct() at all, giving a second independent crash surface from the same root cause.

Fix

• Changed == null || to != null && in the pre-approval branch so the helper is only invoked when a LoanProduct is present and over-apply is enabled.
• Wrapped the active-loan branch call with a loan.getLoanProduct() != null guard.
• Added four missing @Mock field declarations in DelinquencyReadPlatformServiceImplTest (ConfigurationDomainService, LoanTransactionRepository, PossibleNextRepaymentCalculationServiceDiscovery, LoanDelinquencyActionRepository) that were causing silent Mockito injection failure on any new tests written for this path.
• Added unit tests covering both crash surfaces to prevent regression.

Files Changed

File	Change
DelinquencyReadPlatformServiceImpl.java	Fix inverted operator + add missing null guard
DelinquencyReadPlatformServiceImplTest.java	Add 4 missing @Mock declarations + new regression tests

---

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 — Not applicable, no API contract changes. This fix corrects internal null handling only.
• This PR must not be a "code dump". This is a minimal targeted fix — 2 lines changed in the implementation, 4 mock declarations and regression tests added.

[Copilot]

Pull request overview

Fixes FINERACT-2593 by preventing NullPointerException in delinquency collection calculations when loan.getLoanProduct() returns null, and updates the unit test setup to improve Mockito injection for added dependencies.

Changes:

• Fix inverted null guard in the pre-approval/pending/cancelled branch.
• Add a null guard before computing availableDisbursementAmountWithOverApplied on the active-loan branch.
• Add missing @Mock fields in DelinquencyReadPlatformServiceImplTest to support constructor injection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
fineract-loan/src/main/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImpl.java	Corrects null-check logic and prevents NPE when loan product is missing.
fineract-provider/src/test/java/org/apache/fineract/portfolio/delinquency/service/DelinquencyReadPlatformServiceImplTest.java	Adds mocks for newly required constructor dependencies (but still needs one more mock for reliable injection) and has minor formatting issues.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[AshharAhmadKhan]

Good work @mansi75 ! The core fixes on lines 146 and 163 are correct and address the two most visible crash surfaces.

Two things worth addressing before merge:

1. Helper still has no null guard (line 202)

calculateAvailableDisbursementAmountWithOverApplied dereferences loanProduct without a null check. This matters because LoanReadPlatformServiceImpl calls it directly at lines 648 and 663 with no guard at either call site — verified on develop. Those two surfaces are still live crashes.

Fix:
```java
// before
if (loanProduct.isAllowApprovedDisbursedAmountsOverApplied()) {
// after
if (loanProduct != null && loanProduct.isAllowApprovedDisbursedAmountsOverApplied()) {
```

2. Tests assert null but don't cover the helper directly

Asserting getAvailableDisbursementAmountWithOverApplied() is null only confirms the field was never set. Since the helper is a public interface method and callable independently, it needs direct coverage — null product path, flag false, and the percentage calculation path.

[San-43]

Hi @mansi75, you may want to update the PR title to use uppercase FINERACT-. I believe Jira only references the PR correctly when the issue key is uppercase. I ran into the same issue recently, where the PR was not linked to the Jira ticket because the key was not uppercase.

[Aman-Mittal]

@mansi75 Please review failing e2e test cases

Then Loan has availableDisbursementAmountWithOverApplied field with value: 150 # org.apache.fineract.test.stepdef.loan.LoanStepDef.checkLoanDetailsAvailableDisbursementAmountWithOverAppliedField(double)
org.opentest4j.AssertionFailedError: [Wrong value in LoanDetails/availableDisbursementAmountWithOverApplied.
Actual value is: 0.0
Expected Value is: 150.0]
expected: 150.0
but was: 0.0
at org.apache.fineract.test.stepdef.loan.LoanStepDef.checkLoanDetailsAvailableDisbursementAmountWithOverAppliedField(LoanStepDef.java:3484)
at ✽.Loan has availableDisbursementAmountWithOverApplied field with value: 150(file:///home/runner/work/fineract/fineract/fineract-e2e-tests-runner/src/test/resources/features/LoanUpdateAvailableDisbursementAmount.feature:280)

[Execute tests for shard 10](https://github.com/apache/fineract/actions/runs/25077326923/job/73538174734#annotation:12:16062)

Test failed for src/test/resources/features/LoanUpdateApprovedAmount.feature

Execute tests for shard 9
Test failed for src/test/resources/features/LoanCapitalizedIncome-Part2.feature

[mansi75]

@San-43 Thank you for informing. I will update accordingly.

[adamsaghy]

@mansi75 Please review the failing test cases!

[adamsaghy]

@mansi75
Please review the failing checks:

Please run:
./gradlew spotlessApply spotbugsMain spotbugsTest checkstyleMain checkstyleTest
./gradlew --no-daemon build -x test -x cucumber -x doc

Before any PR or changes, please always run these two commands and make sure there is green build!

[mansi75]

@adamsaghy Got it! I am working on the failing tests

[Aman-Mittal]

Can you please explain when loanProduct is required in calculateAvailableDisbursementAmountWithOverApplied function ? When loanProduct is not present what part of loan product it utilize when calculating AvailableDisburementAmountWithOverApplied

[Aman-Mittal]

Seems ok to merge what i can see it Logic was fixed where || is causing problem but curious about AvailableDisbursementAmountWithOverapplied Logic