Skip to content

feat: Multiple Bank Accounts (IBAN/BIC) by contract - #511

Open
aaronspring wants to merge 12 commits into
tuttle-dev:mainfrom
aaronspring:feat/510-multiple-bank-accounts
Open

feat: Multiple Bank Accounts (IBAN/BIC) by contract #511
aaronspring wants to merge 12 commits into
tuttle-dev:mainfrom
aaronspring:feat/510-multiple-bank-accounts

Conversation

@aaronspring

@aaronspring aaronspring commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Closes #510

What

Support multiple bank accounts (IBAN/BIC) per user, with a default account used for invoicing and the option to pin a specific account onto a contract.

  • Model: User.bank_accounts (list) replaces the single bank_account_id column; is_default flags the account used when a contract names none. Contract.bank_account_id selects the account invoices for that contract pay into.
  • Migration f87515d1d068: backfills bankaccount.user_id from the old 1:1 link and marks the previously linked account as default, then drops user.bank_account_id. FKs: bankaccount.user_id → CASCADE, contract.bank_account_id → SET NULL.
  • Invoicing: e-invoice payee and rendered invoices resolve the contract's bank account first, falling back to the user's default account (einvoice.py, rendering.py, all template scripts).
  • App: Settings → Profile has a multi-account editor (default radio, add/remove); the contract form offers an "Invoicing Bank Account" dropdown. Removing an account a contract still invoices from is refused by update_profile and surfaced in the profile status bar.
  • Demo: Harry Tuttle gets two accounts — a default one and a USD one; his USD security contract pays into the USD account.

Verification

  • uv run pytest558 passed, 1 skipped.
  • cd ui && npm run build (tsc + vite) — clean.
  • Migration, exercised against a DB seeded at the previous head (9cad5ae77a79) with the old 1:1 link, an unreferenced leftover account, and a contract:
    • the linked account becomes user_id=1, is_default=1; the unreferenced one is kept as user_id=NULL, is_default=0 (not deleted);
    • user.bank_account_id is dropped with no row loss, PRAGMA foreign_key_check is empty, no _alembic_tmp_* residue;
    • both new FKs survive the batch rebuild with CASCADE / SET NULL intact.
    • New tests in tuttle_tests/test_migrations.py pin all of the above; they were mutation-checked (breaking the backfill fails them).
  • UI smoke (Electron via Playwright): Settings → Profile lists both accounts with the default radio and "Add bank account" grows the list; Contracts → New offers both accounts in "Invoicing Bank Account".

Notes

  • Migration is append-only per tuttle/migrations/README.md; the generated revision was reviewed (no rename-as-drop traps, data backfill preserved).
  • Accounts orphaned by pre-migration profile edits keep user_id=NULL and are not shown in the UI. They are preserved in the DB, not deleted.
  • Account deletion goes through delete-orphan on User.bank_accounts, so the in-use check in update_profile is what protects contracts from losing their account.

Replace the single 1:1 bank account with a list owned by the user:
- User.bank_accounts (new relationship) replaces the bank_account_id column;
  is_default flags the account used when a contract names none.
- Contract gains bank_account_id so a contract can invoice into a
  specific account; e-invoice payee and rendered invoices resolve
  contract account first, then the user's default.
- Migration backfills ownership (bankaccount.user_id) and marks the
  previously linked account as default, dropping user.bank_account_id.
- Settings profile gains a multi-account editor (default radio, add/
  remove). Contract form offers an Invoicing Bank Account dropdown.
- Removing an account a contract still invoices from is refused.
- Demo user gets Giro (default) + Wise USD accounts; the USD contract
  invoices into the Wise account.

Closes tuttle-dev#510
@aaronspring
aaronspring force-pushed the feat/510-multiple-bank-accounts branch from 19f1475 to 6639a02 Compare August 10, 2026 18:11
@aaronspring

aaronspring commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

UI verification — multiple bank accounts

Focused Electron smoke on PR #511 using the Harry Tuttle demo user.

Settings → Profile — multiple accounts

The new Bank Accounts card shows the default-account radio, account details, and the add-account flow.

Settings — bank accounts

Settings — added account row

Contracts → New — per-contract account selection

The contract form exposes “Invoicing Bank Account”, with “Use my default account” as the safe fallback.

Contract — invoicing bank account

Invoice — non-default account

The Harry Tuttle demo’s USD invoice for Ductwork Security Inc. renders with the dedicated Wise USD account rather than the default Giro account.

USD invoice using Wise USD account

Invoice 2026-06-26-1 · $1,000.00 · account Wise USD.

Checks

  • Launch: Electron via ui/driver.mjs
  • Navigation: Settings → Profile; Contracts → New; Invoicing → invoice 2026-06-26-1
  • Synthetic data only: Harry Tuttle demo user, Giro + Wise USD accounts
  • Existing PR checks: uv run pytest (446 passed) and cd ui && npm run build (clean)
  • Limitation: this was a focused macOS smoke; final UX review and other platform packaging paths remain for human review.

@aaronspring aaronspring changed the title feat: Multiple Bank Accounts (IBAN/BIC) feat: Multiple Bank Accounts (IBAN/BIC) by contract Aug 10, 2026
@aaronspring

Copy link
Copy Markdown
Contributor Author

Note: Account name shouldn't be set to giro or wise USD (internal account names) but to Account Owner/Business name.

@aaronspring
aaronspring requested a review from clstaudt August 10, 2026 19:35
Comment thread tuttle/demo.py Outdated
Comment thread tuttle/demo.py Outdated
aaronspring and others added 10 commits August 11, 2026 00:24
tuttle/demo.py names the demo accounts "Harry Tuttle" (the field is the
account holder / bank name); the test fixtures still said "Giro".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
update_profile accepted a single ``bank_account`` dict as a partial
update alongside the ``bank_accounts`` list. Nothing sends it: the
settings view sends the list and onboarding goes through users.create,
which keeps its own ``bank_account`` handling. Dropping the branch also
retires the _ACCOUNTS_UNCHANGED sentinel that only existed to tell it
apart from an absent key.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_validate_bank_account re-queried the account before every save to
produce a friendly error. The per-user engine runs with
PRAGMA foreign_keys = ON (abstractions.py), so SQLite already rejects a
bank_account_id that no longer exists and _describe_save_error turns the
IntegrityError into the same message. Widen that message to name the
bank account, since contract now has two foreign keys.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both User helpers were loops that a generator expression states in one
line, and the invoice payee lookup wrapped one attribute access in a
bare `except Exception`. Invoice.contract is lazy="subquery", so it is
loaded with the invoice and cannot raise on access; a None check is
enough.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- bank_accountsError was declared, initialised and rendered but never
  assigned: update_profile's refusal to remove an in-use account already
  arrives as res.error and shows up in profileStatus.
- The .map() after the remove .filter() was a no-op — with one account it
  mapped over an empty array, otherwise it re-set is_default to itself.
  The backend re-picks a default when none is flagged.
- Three identical setProfile spreads collapse into patchAccount().

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ui/driver.mjs was a local Playwright REPL used to smoke-test this
branch: no package.json script, no CI job, no test references it, and it
imports playwright-core while the project depends on playwright. Keep
that kind of scratch tooling out of the tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
f87515d1d068 rewrites data, and nothing pinned that behaviour: the
existing chain tests only prove row counts survive. Seed the old 1:1
link at the previous head and assert the linked account becomes the
owned default, an unreferenced account is kept but not promoted,
user.bank_account_id is gone, and both new FKs come back from the batch
rebuild with CASCADE / SET NULL intact.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removing the _ACCOUNTS_UNCHANGED sentinel in 702c626 collapsed the
two-line separator before _normalize_logo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@clstaudt clstaudt added this to the 4.3 milestone Aug 11, 2026
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.

Multiple Bank Accounts (IBAN/BIC)

2 participants