Skip to content

FINERACT-2721: Fix loan collateral quantity adjustment and modify-loan change detection - #6196

Open
Samer-Melhem-FOO wants to merge 1 commit into
apache:developfrom
foodeveloper:port/CBS-4-loan-collateral-quantity-adjustment
Open

FINERACT-2721: Fix loan collateral quantity adjustment and modify-loan change detection#6196
Samer-Melhem-FOO wants to merge 1 commit into
apache:developfrom
foodeveloper:port/CBS-4-loan-collateral-quantity-adjustment

Conversation

@Samer-Melhem-FOO

Copy link
Copy Markdown
Contributor

Description

Two related defects in loan collateral handling (LoanCollateralAssembler / LoanAssemblerImpl):

  1. Repeated quantity deduction. LoanCollateralAssembler.fromParsedJson(...) deducted client collateral quantity on every call, including from read-only paths (loan application validation, schedule preview), not just actual loan submission/update. Since the same parsing method is reused across both, a client's available collateral quantity could be decremented more than once for the same loan, and submitting a loan with linked collateral could fail with an "invalid amount of collateral quantity" error even with sufficient collateral available.

  2. Inverted change detection on loan modification. In LoanAssemblerImpl, the check that decides whether collateral changed on a Modify Loan Application request was inverted:

    possiblyModifedLoanCollateralItems.equals(loan.getLoanCollateralManagements())

    This recorded a "collateral changed" entry (and triggered loan.updateLoanCollateral(...)) only when the new and existing collateral sets were equal — i.e. when nothing changed — and skipped it when they genuinely differed. Real collateral changes on loan modification were silently dropped.

Fix: LoanCollateralAssembler.fromParsedJson now takes an explicit adjustClientCollateralQuantity flag, defaulting to false for read-only/validation callers (existing single-arg overload preserved) and true only for actual loan submission/update. The modify-loan condition is flipped to !equals(...) so genuine changes are detected and persisted.

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

Checklist

  • 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.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Note: no existing unit/integration test covers LoanCollateralAssembler/LoanAssemblerImpl's collateral-handling logic, so none were added — happy to add coverage if a reviewer points to the right harness (loan submission with collateral is normally exercised via integration tests).

@Samer-Melhem-FOO

Copy link
Copy Markdown
Contributor Author

The only failing check here (MariaDB Tests (Shard 14 of 15)) is CobPartitioningTest.testLoanCOBPartitioningQuery() failing with Expected status code <200> but was <403> — a permission/auth assertion in the Close-of-Business job partitioning test suite. This PR only touches LoanCollateralAssembler and LoanAssemblerImpl (collateral quantity handling), and doesn't touch permissions, job scheduling, or COB logic at all, so this looks like pre-existing test flakiness rather than something introduced here.

Would appreciate a re-run of that job when convenient — happy to investigate further if it fails again on a fresh run.

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.

1 participant