Skip to content

feat(bank-account): update approved bank account — backend [ENG-509]#444

Merged
islandbitcoin merged 3 commits into
mainfrom
jabariennis/eng-509-bank-account-update-flow-let-users-edit-their-approved
Jul 13, 2026
Merged

feat(bank-account): update approved bank account — backend [ENG-509]#444
islandbitcoin merged 3 commits into
mainfrom
jabariennis/eng-509-bank-account-update-flow-let-users-edit-their-approved

Conversation

@islandbitcoin

Copy link
Copy Markdown
Contributor

What & why

Users submit their Jamaican bank details once, inside the account-upgrade (KYC) flow. After approval there was no way to change them. This adds a self-serve update path that goes through human review before taking effect.

Backend of 3 PRs for ENG-509 (see companion PRs in frappe-flash-admin and flash-mobile).

Changes

  • bankAccountUpdateRequest(input): BankAccountUpdateRequestPayload! mutation
  • BankAccount.pendingUpdate: BankAccountUpdateRequest read field (drives the app's "Under review" state)
  • ERPNext model BankAccountUpdateRequest + three ErpNext service methods (post / query-open / bulk-close)
  • App service createBankAccountUpdateRequest

Invariants enforced

  • Ownership — the target account must belong to the caller's erpParty
  • Currency locked — currency changes are rejected (currency drives the JMD-vs-USD cashout payout rail)
  • Server-side field validation — non-empty bank/branch/number and account_type ∈ {Chequing, Savings}, validated server-side rather than trusted from the client
  • Supersede — at most one open request per account; the replacement is created before prior requests are closed, so a failed create never destroys the user's existing pending request

Testing

yarn build ✓ · yarn test:unit (model round-trip, ErpNext HTTP methods, app-layer invariants incl. create-before-close safety and field validation) ✓ · eslint ✓ · SDL regenerated via make codegen.

Known follow-up

BankAccount.pendingUpdate resolves with one ERPNext call per account (N+1) and returns null on an ERPNext error. Fine for the handful of accounts a user has; a batch/dataloader is a deliberate follow-up.

🤖 Generated with Claude Code

Lets an approved user request a change to their Jamaican (ERPNext) bank
account details. The request is created as a Bank Account Update Request
DocType in ERPNext and reviewed by an admin before it takes effect;
cashouts keep settling to the current details until approval.

- New GraphQL mutation bankAccountUpdateRequest + BankAccount.pendingUpdate
  read field (regenerated SDL).
- New ERPNext model BankAccountUpdateRequest and three ErpNext service
  methods (post / query-open / bulk-close).
- App layer enforces the invariants: ownership (account must belong to the
  caller's erpParty), currency lock, non-empty/allowed proposed fields
  (validated server-side, not trusted from the client), and supersede
  (created before priors are closed so a failed create never destroys the
  existing request).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@linear

linear Bot commented Jul 13, 2026

Copy link
Copy Markdown

ENG-509

@islandbitcoin

Copy link
Copy Markdown
Contributor Author

Part of ENG-509 — this change spans three repos:

Merge order: flash#444 first (it defines the GraphQL contract), then the other two — the mobile change depends on the backend field to work end-to-end.

bobodread876 and others added 2 commits July 13, 2026 00:28
- pendingUpdate now surfaces Pending OR Rejected requests
  (getLatestBankAccountUpdateRequestForAccount), so a rejected change is
  shown to the user instead of silently vanishing.
- Remove the unreachable SetDocTypeValueError branch (+ orphaned imports)
  in the mutation resolver; close is best-effort and never returns that type.
- Dedup the bulk-close helper: parametrize setStatusForRequests(doctype,
  status) instead of a second near-identical method.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ENG-509]

The api-key scope map requires an entry for every authed root field
(deny-by-default regression test). Block bankAccountUpdateRequest for
API-key access, mirroring businessAccountUpgradeRequest.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@islandbitcoin
islandbitcoin merged commit 1fc2b06 into main Jul 13, 2026
15 checks passed
islandbitcoin added a commit that referenced this pull request Jul 14, 2026
… [ENG-509] (#447)

#444 added `pendingUpdate` to the public subgraph schema but never
regenerated the composed Apollo Router supergraph. The router validates
against that baked-in supergraph, so it rejects `bankAccounts { pendingUpdate }`
with GRAPHQL_VALIDATION_FAILED (400) even on images built from #444 — which
blocks the mobile cashout screen ("No bank account found") on TEST.

Ran `make codegen` to recompose the supergraph. Only the generated
supergraph.graphql changes (58 insertions, 0 deletions).

Co-authored-by: Dread <bobodread@bobodread.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
islandbitcoin added a commit that referenced this pull request Jul 15, 2026
#449)

`JMDAmount.dollars(d)` did `new JMDAmount(BigInt(d) * 100n)`, and `BigInt(d)`
throws on any non-integer `d`. The cashout offer wraps the live NCB rate
(essentially always fractional — 152.7, 155.5, ...) via this method, so
`getCashoutExchangeRate` returned `ExchangeRateQueryError` for every real
rate and `requestCashout` failed. Rewrote it to mirror `USDAmount.dollars`
(Money-lib, HALF_TO_EVEN) so fractional rates convert correctly.

Compounding it, `error-map.ts` had no `ExchangeRateQueryError` case, so the
failure fell through to the exhaustiveness fallback ("This should never
compile ...") and returned a `data: null` top-level GraphQL error — the app
therefore showed no message, just a spinner. Added a mapped case.

Only surfaces post-#444/#445 (live NCB rate); a deploy would break JMD
cashout in every env until this lands.

Co-authored-by: Dread <bobodread@bobodread.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

2 participants