Skip to content

Extract shared → reuse on the standalone window

Progress

  • Phase 1a DONE (origin/nightly a3f37492) — extracted the 4 shared display helpers (formatCurrency/formatDate/getStatusColor/getStatusIcon) → components/accounting/ transactionWorkspace/format.tsx; BankTransactionsTab imports them (pure move, no behavior change; cleaned the 2 now-unused icon imports). Foundation so the relocated workspace components can import helpers without a cycle. tsc+lint clean, suite 325/325.
  • Phase 1b DONE (origin/nightly c326e09c) — relocated AttachmentsPanel (+ AttachmentDoc / AttachmentsPanelProps) verbatim → transactionWorkspace/AttachmentsPanel.tsx (638 lines out, via awk extract → assemble-with-header → remove → trim orphaned imports). Cleaned the new file's 2 inherited anys + empty catch. tsc+lint clean, suite 325/325.
  • Phase 1c DONE (origin/nightly 626a7bd0) → PHASE 1 COMPLETE. Relocated TransactionDetailsDrawer (~1553 lines) verbatim → transactionWorkspace/TransactionDetailsDrawer.tsx (assembled tsc-clean on the first try). Trimmed BankTransactionsTab's drawer-only imports (Drawer/Descriptions/Pagination/Checkbox/ Radio/Divider/Avatar/CheckboxChangeEvent/routingFocusForTransaction/NamePreviewBuilder/useCloudBillingMapping/ getGLTemplateCategoryForCode/MCC_CATEGORIES). New file inherits 3 callback anys + 1 dead var verbatim (faithful move). tsc+lint clean (no new errors), suite 325/325. Rebased onto origin/nightly (other agent's receipts work — no file overlap, clean rebase).
  • Phase 2 DONE (origin/nightly ff1329a3). Hoisted the 7 single-tx modals (detail drawer, TransactionLinkingModal, WopcRemovalPrompt, BillingLinkModal, CreateWOPCModal, payment-confirmation plugin host, ApprovalRequestModal) out of BankTransactionsTab's render into a thin <TransactionWorkspace> (components/accounting/transactionWorkspace/ TransactionWorkspace.tsx). Pure JSX relocation — still takes every handler + state as props; per-modal props grouped & typed via React.ComponentProps<typeof X> (no hand-authored 35-prop interface). The 5 reference drawers (Person/Client/Invoice/Student/Note) + AutoMatchPopup row STAY (table/list-level). Verified w/ 8GB-heap tsc (0 errors), eslint (no new; wrapper clean), suite 325/325, Firestore-read diff scan (only comments moved). Owner to verify drawer/modals on nightly before Phase 3.
  • Phase 3a DONE (origin/nightly 5a273c35). Routed all single-tx mutations through two new helpers — onTransactionUpdated(tx) [setTransactions(map) + setSelectedTransaction sync] + onTransactionDeleted(id) [setTransactions(filter) + clear selection] — instead of writing the list cache directly. 28 handler sites converted; optimistic-update + rollback (prev = transactions.find BEFORE; revert single tx on error) + server-response reconcile all flow through the helpers. setTransactions stays as the helper's cache-writer (still stamps the live-sync cooldown). This absorbs the BEHAVIORAL risk of the standalone-reuse refactor; what remains is mechanical. (4 remaining setTransactions = the 2 helper bodies + 2 parent closures.)
  • Phase 3b PLAN (the move — NOT yet done). Relocate the single-tx modal STATE + HANDLERS from BankTransactionsTab into TransactionWorkspace so the standalone window reuses them. MOVES: state (linkingModalOpen/InitialTab, wopcRemoval + wopcRemovalResolverRef, billingLinkModalOpen, createWopcModalOpen, activeMatchPlugin, paymentConfirmation{ModalOpen,Transaction,Template}, approvalPrompt) + handlers (handleOpenLinkingModal, handleMatch/Unmatch{Invoices,CoachingInvoice,Receipts}, handleAssign/UnassignAccount, handleDeleteTransaction, handleUpdateName, handlePaymentConfirmationRequired, handleSavePaymentConfirmation, interceptUnmatchForWopc [Promise bridge — move as ONE unit w/ onWopcRemovalDone + the resolver ref], handleUnmatch, retryApprovedPending + its on-mount useEffect, assertTransactionOpen/blockIfClosed). REWIRE: each handler's transactions.find(t=>t.id===id) → the single transaction prop (id===transaction.id in the workspace); guards use a passed getClosedPeriodMessage (STAYS in BankTransactionsTab — table uses it at 3235/3287/3645/3652). STAYS in parent: table/filters/stats, useTransactions, the setTransactions cache-writer, the onTransactionUpdated/onTransactionDeleted closures, Add-modal + handleCreateTransaction, reference drawers, autoMatch, selection state (selectedTransaction + detailsModalOpen). TABLE TRIGGER: line ~3290 handleOpenLinkingModal(record) → workspace imperative ref wsRef.openLinking(record, tab) (forwardRef + useImperativeHandle). LEAN PROPS: transaction, open, onClose, accounts, bankAccounts, expenseProjects, loadingProjects, subsidiaryId, onTransactionUpdated, onTransactionDeleted, refreshTransactions, getClosedPeriodMessage, onRequestOffBookEntry, onShowRouting. Then Phase 4 = standalone page renders <TransactionWorkspace transactionId=… />. ~600-line scattered move; behavioral risk already absorbed by 3a; verify 8GB-tsc + suite + owner nightly checklist (assign/unassign / match invoice+coaching+receipt / unmatch / delete / edit-title / WOPC void+delete / s.33 approval). Best done with fresh focus, not tail-of-marathon.
  • Phase 3b-i DONE (origin/nightly a8536a74). Relocated the workspace logic VERBATIM into a new hook components/accounting/transactionWorkspace/useTransactionWorkspace.tsx (1007 lines): all 11 modal state decls + WOPC resolver ref, both guards, all 17 handlers (incl. the WOPC Promise-bridge + s.33 approval retry) + the grouped <TransactionWorkspace> prop construction (returned as workspaceProps). Deps passed IN as params (transactions KEPT — no lean conversion yet). BankTransactionsTab −874 lines (3990→3116): calls the hook, renders <TransactionWorkspace {...ws.workspaceProps} />, table trigger → ws.openLinking(record). Built by a subagent, reviewed by me: 8GB-tsc 0 / vitest 325 / eslint no-new / 3 critical handlers (interceptUnmatchForWopc 55L, handleAssignAccount 81L, handleUnmatchInvoices 45L) byte-for-byte identical to HEAD; integration wiring confirmed. Owner regression-checks on nightly (behavior UNCHANGED). KNOWN: type-only cycle (hook import type {BankTransaction,…} from '../BankTransactionsTab') — erased at runtime, tsc/lint-clean; fixed in 3b-ii.
  • Phase 3b-ii DONE (origin/nightly 6b9cb3ca) — de-cycle only. Extracted BankTransaction / BankAccountInfo / ExpenseProject → components/accounting/transactionWorkspace/types.ts; BankTransactionsTab + the hook both import from there (hook no longer type-imports from ../BankTransactionsTabcycle broken). Trimmed 5 now-dead canonical-type imports in BankTransactionsTab (ExpenseMetadata/TransactionTitleData/PaymentConfirmation/WOPCReference/ MatchedCoachingInvoice). Compile-time only (types erased; ZERO runtime change). tsc 0 / vitest 325 / eslint no-new. Rebased onto nightly (other agent's expenseCategories Phase-0 — no file overlap). Lean conversion SKIPPED (transactions.find→transaction): cosmetic — ~11 money-path edits for no functional gain; Phase 4 reuses the hook fine via transactions={[transaction]}. The drawer keeps its own local-mirror BankAccountInfo (other agent's file, untouched) — reconciling that duplicate + full canonical unification is the Phase 5 item.
  • ⚠️ Phase 1c shipped BROKEN — root-caused + reconciled. My Phase 1c "tsc clean" was an OOM-crash misread (default-heap tsc dies exit 134 w/ empty log → looks like 0 errors). 626a7bd0 actually had ~13 type errors: 5 drawer imports left behind (formatMonthLabel/routingFocusForTransaction/generateCoachingPreviewClient/ getGLTemplateCategoryForCode/BankAccountInfo) + a local-vs-canonical BankTransaction mismatch. See [[feedback_tsc_needs_8gb_heap]]. The other agent's Vercel build (type-checks) caught it and fixed it on nightly (commits aabf5c35 + b26abeee): added the missing imports, kept the drawer on canonical BankTransaction + 5 as unknown as bridge-casts at the call site + a local-mirror BankAccountInfo. I first fixed it my own way (shared transactionWorkspace/types.ts, drawer on the tab-local shape, 1 cast) — cleaner/DRY-er but enshrines the legacy local shape; theirs points the right direction (canonical, per docs/api-catalog). Verdict: keep THEIRS. I discarded my Phase 1c fix, ff'd q3 to nightly, and redid only the hoist on top (carrying their casts into the wrapper's drawer={{}}). The real fix = full canonical unification (drop the tab-local BankTransaction) → that's the Phase 5 cleanup, supersedes both expedients. Coordinate it with the other agent (they're active in these files).
  • Pre-existing dead code — ALL REMOVED (2026-06-18). (1) GCPEvidencePanel cluster + pendingBillingLink: removed in 87843b2a (290 lines, pure deletion, behavior-identical). (2) drawer updatedTitleData: investigated + removed in f3bd17d4 — verdict was (a) dead code, NOT a bug. handleSaveTitle built updatedTitleData (+ the wopcData/WOPC-fetch feeding it) but the save only calls onUpdatePayer(id, trimmedTitle|null); titleData is NOT persisted by design (the read-time enricher rebuilds it — per the save-site comments). Vestige of the old persist-titleData design, superseded by the enricher. "Use Default" unchanged (clears title → enricher renders the template name). Removed ~149-line dead block + getGLTemplateCategoryForCode/TransactionTitleData imports. All verified 8GB-tsc 0 / vitest 325 / eslint clean.
  • Phase 4 DONE → on nightly 7c593289 (PENDING owner nightly verification). The standalone /transaction/[id] window now renders the interactive <TransactionWorkspace> via useTransactionWorkspace (built by a subagent, REVIEWED by me) — full match/assign-GL/WOPC/s.33 surface, same code as the accounting page. Added drawer variant: 'drawer' | 'page' ('page' = full-width to fill the window; threaded through the hook; the tab is unchanged, defaults 'drawer'). Window wiring: transactions=[tx]; canonical↔tab-local BankTransaction cast at the boundary; bankAccounts Record→BankAccountInfo[]; getClosedPeriodMessage→null (the SERVER enforces closed periods via assertTxPeriodOpen, so the client guard is UX-only); off-book Add-modal path no-op. My review caught + fixed a real bug: the subagent wired setDetailsModalOpen→window.history.back(), which would navigate AWAY on Match/Assign (both fire setDetailsModalOpen(false) before opening the linking modal). FIX: setDetailsModalOpen is now a no-op (the page stays, the linking modal opens on top); leaving is driven by setSelectedTransaction(null) (close-X / post-unmatch → navigate back). Retired the read-only TransactionDetailsView (deleted; the window was its only importer). Verified 8GB-tsc 0 / vitest 325. CANNOT browser-verify (auth-gated) — owner MUST test on nightly: open a tx in a new tab → full-page view; Match (invoice/coaching) + Assign GL open the linking modal ON TOP (must NOT navigate away); unmatch/delete; WOPC void/delete; s.33 approval; close-X leaves. Watch the full-width-drawer look + modal stacking — likely needs a small UX polish pass.
  • Phase 5 (optional cleanup, TODO). Full canonical-BankTransaction unification: drop the tab-local shape + the drawer's local-mirror BankAccountInfo + the handful of as unknown as bridges across the tab/hook/window/drawer boundaries. Pure type-debt, no behavior change. Low priority.
  • Phase 3a DONE (origin/nightly 5a273c35). In-place conversion: every single-tx handler (match/unmatch invoices + coaching + receipts, assign/unassign GL, delete, update-name, the Modal.confirm unmatch) now funnels its optimistic
  • rollback + server-reconcile writes through two new helpers — onTransactionUpdated(tx) / onTransactionDeleted(id) — instead of setTransactions(prev=>...) directly. Helpers keep the list-cache write (stamps the live-sync cooldown via setTransactions) + now sync the open drawer's selectedTransaction. ONE intended behavior narrowing: rollback restores the single affected tx (onTransactionUpdated(previousTx)) not the whole-list snapshot — equivalent for single-tx actions, safer re concurrent live-sync. tsc 0 (8GB), suite 325/325, no new lint, Firestore reads unchanged. The 2 parent-render closures (drawer.onTransactionUpdated ~3772, linking.onRefreshTransaction ~3802) still use setTransactions directly — Phase 3b reworks them.
  • Phase 3b NEXT (relocation). Move the now-list-agnostic handlers + modal state (linking/wopc/billing/createWopc/ payment/approval) into TransactionWorkspace; pass onTransactionUpdated/onTransactionDeleted/refreshTransactions/ data as props; re-scope assertTransactionOpen to the single transaction (currently reads the list to find the tx); expose imperative openLinking(tx, tab) for the TABLE trigger (BankTransactionsTab ~3290 handleOpenLinkingModal(record)). Lower-risk than 3a (pure relocation) but large (~700 lines). Then Phase 4 (standalone swap) + Phase 5 (canonical type unification — supersedes the local-vs-canonical casts).

Why

Owner reframed the standalone /transaction/[id] window as "an entry point to the Accounting page, but transaction-focused + match-modal-focused, without the other tabs/nav". They want FULL interactivity there (match, assign GL, the HKFRS s.33 approval flow, the WOPC Void/Delete prompt) — and chose to get it by REUSING the real BankTransactionsTab machinery (not reimplementing). Extracting a shared <TransactionWorkspace> used by BOTH the Accounting page and the standalone window gives zero divergence. (T-063's read-only TransactionDetailsView is the stepping stone this supersedes.)

Key findings (from a full dependency audit of BankTransactionsTab.tsx, 6363 lines)

  • TransactionDetailsDrawer (2144-3694) is already prop-clean — references NO parent state; every mutation funnels through props (mainly onTransactionUpdated(json.transaction)). AttachmentsPanel (1518-2138) is likewise self-contained (owns its own WOPC signing modal). So these are pure relocations.
  • The coupling lives in the handlers (4411-5100), via two things: (1) the custom setTransactions (4073) that writes the React-Query LIST cache + calls markLocalAccountingMutation (optimistic update + live-sync cooldown); (2) assertTransactionOpen/blockIfClosed (closed-period guards reading transactions).
  • THE FIX for the list coupling: replace every setTransactions(prev.map) with an internal single-tx setTx + an onTransactionUpdated(tx) callback. The parent's existing call-site closure (6089: setTransactions(map)+setSelectedTransaction) keeps the table's optimistic updates; the standalone page just does setTx(updated).
  • Approval retry (retryApprovedPending 4128-4174 + approvalPrompt + savePendingApproval): NO list coupling — moves cleanly. Bonus: opening the window after approving self-completes the action on mount. approvalPrompt.retryBody must stay the literal assign POST body.
  • WOPC cascade (interceptUnmatchForWopc 4952 + wopcRemovalResolverRef + WopcRemovalPrompt 6150 + onWopcRemovalDone): a Promise bridged across a modal — MUST move as one unit (PR #602 await-the-user contract).
  • Stays in BankTransactionsTab: table/filters/stats/pagination, useTransactions, the setTransactions cache-writer, AutoMatchPopup×5 + banner, Add Transaction modal + handleCreateTransaction + off-book wiring, selection state.
  • Data the workspace needs: accounts, bankAccounts (useBankAccounts, global), HK bank registry, templates (drawer self-fetches on open), closed-periods (per-subsidiary), expense projects (/api/accounting/expense-projects, lazy on linking-modal-open), match plugins.

Proposed interface

interface TransactionWorkspaceProps {
  transaction?: BankTransaction | null   // list context (hydrated)
  transactionId?: string                 // standalone context (self-fetch)
  subsidiaryId: string
  accounts?: {code;name;displayNameTemplate?}[]   // else self-fetch
  bankAccounts?: BankAccountInfo[]                 // else self-fetch
  variant?: 'drawer' | 'page'
  open?: boolean; onClose?: () => void             // drawer mode
  onTransactionUpdated: (tx: BankTransaction) => void   // replaces setTransactions writes
  onTransactionDeleted?: (txId: string) => void
  onShowRouting?: (focusNodeId: string) => void
}

Phased plan (BankTransactionsTab stays green at each step; verify on nightly)

  1. Relocate leaf components — move AttachmentsPanel (1518-2138) + TransactionDetailsDrawer (2144-3694) into components/accounting/transactionWorkspace/, import back. Pure move.
  2. Thin <TransactionWorkspace> wrapper — renders the drawer + all single-tx modal JSX (TransactionLinkingModal, WopcRemovalPrompt, BillingLinkModal, CreateWOPCModal, the payment-confirmation plugin host, ApprovalRequestModal, the 5 reference drawers), but STILL receives every handler as a prop from BankTransactionsTab. Pure JSX hoist, no logic moved.
  3. Move the handlers + WOPC cascade + approval machinery into the workspace; convert setTransactions(map) → internal setTx + onTransactionUpdated; re-scope assertTransactionOpen to the single tx. Parent shrinks to selection + the onTransactionUpdated closure + list concerns. Test match/unmatch/assign/unassign/delete/title/merchant/WOPC-void/approval on the Accounting page.
  4. Swap the standalone page — app/transaction/[id]/client.tsx renders <TransactionWorkspace transactionId={id} subsidiaryId variant="page" onTransactionUpdated={setTx} /> instead of TransactionDetailsView. Retire TransactionDetailsView once parity confirmed.
  5. Cleanup — dead parent state (pendingBillingLink ~3843), type unification at the plugin boundary (6209).

Owner decisions (already captured)

  • s.33 approval gate on the window: wire the full ApprovalRequestModal (comes for free via reuse).
  • WOPC-attached unmatch/unassign on the window: bring the full Void/Delete prompt (free via reuse).

Risk / note

Big refactor of the most-used accounting component. CANNOT browser-verify locally (auth-gated) and there are no component tests — so execute phase-by-phase with owner verification on nightly after each phase; don't land it all in one rushed pass.

DONE — Phases 2–4 + polish (2026-06-19)

  • Phase 2 (ff1329a3) — extracted <TransactionWorkspace> wrapper (renders the 7 single-tx modals); handlers still passed as props.
  • Phase 3a (5a273c35) — routed tx mutations through onTransactionUpdated (severed the optimistic setTransactions LIST writes).
  • Phase 3b-i (a8536a74) — extracted useTransactionWorkspace hook (verbatim move of state + ~17 handlers + WOPC bridge + approval retry).
  • Phase 3b-ii (6b9cb3ca) — broke the hook↔tab type cycle via transactionWorkspace/types.ts.
  • Phase 4 (7c593289) — standalone app/transaction/[id]/client.tsx now calls useTransactionWorkspace({…variant:'page'}) + renders <TransactionWorkspace>; deleted the read-only TransactionDetailsView. Window leave driven by setSelectedTransaction(null); setDetailsModalOpen is a no-op so Match/Assign-GL open the modal on top instead of blanking.
  • Phase 4 polish (538860da, owner feedback) — drawer close-X hidden in page mode (closable={variant!=='page'}); PeriodsTab opens the window as an EXTERNAL popup (window.open(url, 'tx-<id>', 'popup,width=560,height=900,…')) not a browser tab; window leaves via window.close() not history.back().

The standalone window and the Accounting page now run the SAME machinery — zero divergence. ✓ tsc 0 on current nightly; suite unaffected (UI changes).

Status / remaining

  • Phase 5 (full canonical-BankTransaction unification) — OPTIONAL/deferred. The local↔canonical bridge via as unknown as casts is the established pattern; unification is cleanup, not required.
  • Owner nightly verification still pending for the Phase 4 popup + close UX (can't browser-verify here — auth-gated).
  • The receipt auto-matcher "no status change" blocker raised here is RESOLVED separately → T-066.

T-063 (the read-only window this supersedes) · T-066 (receipt-matcher blocker, resolved).