Repository navigation
fix(loans,savings): withhold transaction controls the platform will refuse - #667
Merged
Aman-Mittal merged 1 commit intoOct 2, 2026
Conversation
…efuse
The loan screen offered REPAYMENT whatever state the loan was in, and the
savings screen offered SAVINGS DEPOSIT and WITHDRAW whatever state the account
was in. Both open a transaction form, and both can only end in a refusal unless
the record is in the handful of states the platform accepts:
POST /loans/2/transactions?command=repayment
400 error.msg.loan.must.be.active.fully.paid.or.overpaid
'Loan must be Active, Fully Paid or Overpaid'
POST /savingsaccounts/23/transactions?command=deposit
400 error.msg.savingsaccount.transaction.account.is.not.active
'Transaction is not allowed. Account is not active.'
Both were inconsistencies inside their own button row. APPROVE and DISBURSE
beside the repayment button already check loan state; CLOSE and the whole
ACTIONS menu beside the deposit and withdrawal buttons already check
account state. Only these three were exempt.
Withheld rather than disabled, per the convention in DOCS/RBAC.md: a control
that leads somewhere else is removed, and these lead to a separate form.
Reproduced through the UI against a live instance: a savings account awaiting
approval accepted an amount and a payment type and was refused on submit.
Contributor
E2E — mocked backend🎭 E2E Tests✅ All green — 409 passed · 0 failed · 1 skipped, across 38 spec files in 6m 55s. By spec file
All 410 tests — click to expand
Slowest 10 — what the shard counts should be tuned against
📼 Download the HTML report, videos and traces — see the Generated by run 37009277300 from |
Contributor
E2E — real Fineract🎭 E2E Tests✅ All green — 121 passed · 0 failed · 0 skipped, across 36 spec files in 7m 15s. By spec file
All 121 tests — click to expand
Slowest 10 — what the shard counts should be tuned against
📼 Download the HTML report, videos and traces — see the Generated by run 37009277300 from |
Aman-Mittal
added a commit
that referenced
this pull request
Oct 2, 2026
…cepts #667 withheld the loan Repayment button and the savings Deposit and Withdraw buttons unless the record is in a state the platform will act on. That landed with unit tests over a stubbed loan and account, and nothing against the platform that defines the rule — so a future template edit could reintroduce either defect and every check would stay green. Two backend specs, each with two halves that fail for different reasons: - A walk through the states. A loan awaiting approval offers no Repayment; approved and awaiting disbursal, still none; active, it is back. A savings account awaiting approval offers neither Deposit nor Withdraw; activated, both return. Each negative assertion is paired with a control on the same screen — the Approve button — so a screen that simply failed to render cannot pass the test. - A direct probe of the platform rule, so the gate is pinned to the behaviour it mirrors rather than to itself: POST /loans/{id}/transactions?command=repayment 400 error.msg.loan.must.be.active.fully.paid.or.overpaid POST /savingsaccounts/{id}/transactions?command=deposit 400 error.msg.savingsaccount.transaction.account.is.not.active Without these the specs would still pass if Fineract began accepting those commands, at which point the gates would be wrong rather than right. Both were mutation tested before being trusted. Reintroducing each defect failed exactly the assertion that describes it: getByTestId('loan-repayment-action') expected 0, received 1 getByTestId('savings-deposit-action') expected 0, received 1 and in both runs the API probe kept passing while the UI assertion failed, which is the division of labour working: a probe alone would have looked thorough and caught nothing. Seeding gains seedSubmittedLoan and seedSubmittedSavingsAccount, which stop before approval, plus approveLoan, disburseLoan and activateSavingsAccount so a spec can watch a record change state. seedActiveLoan and seedSavingsAccountWithTransactions are rebuilt on them rather than duplicating the application bodies. 5 tests, all passing against a live instance. Both specs are registered in BACKEND_SPECS; the backend project goes from 119 tests to 123.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #123. Same defect class as #663, reached from the other direction: there the control was withheld by permission, here it needs to be withheld by record state.
What was wrong
Two button rows each had one member exempt from the state checks its neighbours already carried.
Loan view.
APPROVEis wrapped in@if (isLoanPendingApproval)andDISBURSEin@if (isLoanApproved).REPAYMENTwas wrapped in nothing, so a loan awaiting approval offered it, and the platform refuses:Savings account view.
CLOSEand the entireACTIONSmenu are wrapped in@if (isActive()).SAVINGS DEPOSITandWITHDRAWwere not:Both land the operator on a full transaction form — date, amount, payment type, receipt number, bank and cheque numbers, routing code, note — and only tell them it was never possible after they fill it in and submit.
How it was found
Browsing a live instance, not reading code. On a savings account in Submitted and pending approval the deposit form accepted an amount and a payment type and answered on submit with:
Each refusal above was then reproduced directly against the platform before being written into a test, so the three accepted states are the platform's rule rather than a guess at it.
The change
Withheld, not disabled — the convention in
DOCS/RBAC.mdis that a control leading to another screen is removed, and both of these open a separate form.canAcceptRepaymenton the loan view:status.active || status.overpaid || status.closedObligationsMet, which is exactly what the platform's repayment validator accepts.@if (isActive()), which also removes a redundant second copy of the same condition on the line below.8 new unit tests cover both: offered in each accepted state, withheld awaiting approval, withheld awaiting disbursal, withheld once written off or closed. 279 files / 1835 tests pass.
Not fixed here
The platform's error object for the savings refusal has two fields transposed —
defaultUserMessageholds"transactionDate"(a parameter name) andparameterNameholds"02 October 2026"(the submitted value). That belongs in ASF Jira, not this repo; it is noted only because it explains the odd[2026-10-02]prefix in the toast above.