feat(transactions): port manual bank-transaction import to SSR
Implement the SSR/alpine/htmx manual transaction import, wiring the already-declared but unhandled ::external-import-page/parse/import routes. Mirrors the SSR ledger import: paste the exact master-branch Yodlee positional-column TSV, review parsed rows in an editable grid with per-row error/warning badges, and import. Every master validation is preserved and the existing import.transactions engine is reused unchanged (via import.manual/import-batch), so core components are untouched. - New ns auto-ap.ssr.transaction.import (page, paste/parse, editable grid, two-tier validation, import handler) + admin-only transactions Import nav. - Two-tier validation: fixable problems (bad date/amount, unknown client or bank-account code, missing fields) are hard errors that block the whole batch; inherent skip-conditions (non-POSTED, before start-date/locked, already-imported) are warnings computed from the engine's own categorize-transaction so the grid preview matches the import result. - Tests: failing-first Playwright e2e (e2e/transaction-import.spec.ts) plus unit/integration coverage (ssr/transaction/import_test.clj, 10 tests). - Deterministic bank-account code in the e2e seed. Plan: docs/plans/2026-06-01-001-feat-manual-transaction-import-ssr-plan.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,176 @@
|
||||
---
|
||||
date: 2026-06-01
|
||||
topic: manual-transaction-import-ssr
|
||||
focus: Port the master-branch "manual import transactions" feature to the SSR/alpine/htmx stack, modeled on the SSR ledger import, preserving all validations, starting from a failing e2e test, with minimal core-component change.
|
||||
mode: repo-grounded
|
||||
---
|
||||
|
||||
# Ideation: Porting Manual Bank-Transaction Import to SSR
|
||||
|
||||
## Grounding Context (Codebase)
|
||||
|
||||
Three reference points were read in full:
|
||||
|
||||
**1. The master feature (what we must reproduce).**
|
||||
- UI: `src/cljs/auto_ap/views/pages/transactions/manual.cljs` — a re-frame modal titled "Import Transactions" with a single `<textarea>` ("Yodlee manual import table"). User pastes tab-separated Yodlee data, clicks "Import". POSTs `(:data)` as EDN to `/api/transactions/batch-upload`.
|
||||
- Route/handler: `src/clj/auto_ap/routes/invoices.clj:241` `batch-upload-transactions` → `assert-admin`, then `manual/import-batch (manual/tabulate-data data)`.
|
||||
- Parsing: `src/clj/auto_ap/import/manual.clj` — `tabulate-data` reads CSV with `\tab` separator, drops the header row, and maps **fixed positional columns**: `[:status :raw-date :description-original :high-level-category nil nil :amount nil nil nil nil nil :bank-account-code :client-code]`.
|
||||
- Per-row mapping/validation: `manual->transaction` uses `import.manual.common/assoc-or-error` to accumulate errors for: **client lookup** (by bank-account-code → client), **bank-account lookup** (by code), **date parse** (`parse-date`, requires `MM/dd/yyyy`), **amount parse** (`parse-amount`).
|
||||
- Engine: `import.manual/import-batch` builds lookups, calls `t/start-import-batch :import-source/manual`, applies `t/apply-synthetic-ids` (dedupe key), imports only rows with no `:errors`, returns stats `{:import-batch/imported ... :failed-validation N :sample-error "..."}`.
|
||||
- Deeper validations live in `src/clj/auto_ap/import/transactions.clj` `categorize-transaction`: status must be `"POSTED"`, transaction date must be after `bank-account/start-date` and after `client/locked-until`, duplicate detection via `:transaction/id` (extant cache), missing client/bank-account/id → `:error`, plus suppression. Synthetic-id (`apply-synthetic-ids`/`synthetic-key`) gives idempotent re-import.
|
||||
|
||||
**2. The SSR ledger import (the pattern to emulate).**
|
||||
- `src/clj/auto_ap/ssr/ledger.clj` implements a **dedicated two-stage page**, not a modal:
|
||||
- `external-import-text-form*` (~L246): alpine `x-data {clipboard}`, a hidden `text-area` bound `x-model`, an `hx-post` to `::route/external-import-parse` triggered on a `"pasted"` event; a "Load from clipboard" button reads `navigator.clipboard`.
|
||||
- `external-import-parse` (~L373): re-renders `external-import-form*` with `:just-parsed? true`.
|
||||
- `external-import-table-form*` (~L117): renders parsed rows into an **editable** `com/data-grid-card` — each cell is a `fc/with-field` text/money input (form-cursor round-trips values + errors); per-row error/warning badge with tooltip; "Import" `hx-post`s to `::route/external-import-import`.
|
||||
- Validation: malli `parse-form-schema` (~L341) with `:decode/string tsv->import-data` + `:decode/arbitrary` (vector→map) enforces shape (min-length, `clj-date-schema`, `money`, location max 2). Business validation in `add-errors`/`table->entries` (~L380–504) accumulates `[message status]` pairs (`:error`/`:warn`) at entry and line-item level; `flatten-errors` maps them to form-cursor field paths `[[:table idx] message status]`.
|
||||
- `import-ledger` (~L554): splits good/ignored(warn-only)/bad entries; `throw+` `:field-validation` with `:form-errors` if any bad; upserts hidden vendors; retracts+re-inserts good entries (idempotent via `:journal-entry/external-id`); touches solr; returns `{:successful :ignored :form-errors}`. `external-import-import` wraps it in an `html-response` with an `hx-trigger` notification.
|
||||
|
||||
**3. THE KEY DISCOVERY — scaffolding already exists, handlers do not.**
|
||||
- `src/cljc/auto_ap/routes/transactions.cljc` **already declares** the route names, mirroring ledger exactly:
|
||||
```
|
||||
"/external-new" ::external-page
|
||||
"/external-import-new" {"" ::external-import-page
|
||||
"/parse" ::external-import-parse
|
||||
"/import" ::external-import-import}
|
||||
```
|
||||
- But `src/clj/auto_ap/ssr/transaction.clj` `key->handler` (L101) wires **only** `page/table/csv/bank-account-filter/bulk-delete` (+ `edit/` + `bulk-code/`). **No handler exists for any `external-import-*` route.** The aside nav (`ssr/components/aside.clj`) wires the *ledger* import nav (`::ledger-routes/external-import-page`) but there is no transaction-import nav entry. So the routes are declared dead ends; the gap is handlers + UI + validation + nav + tests.
|
||||
|
||||
**Test conventions.** `test/clj/auto_ap/ssr/ledger_test.clj` is the template: pure-fn tests (`tsv->import-data`, `trim-header`, `line->id`, `flatten-errors`), validation tests (`add-errors` per error type), tx-building tests (`entry->tx`), and end-to-end import tests (`import-ledger` against Datomic via `wrap-setup` + `setup-test-data` + `admin-token`). `test/clj/auto_ap/test-server.clj` is a Playwright/browser e2e harness with `wrap-test-auth` and seeded `setup-test-data`. Existing engine unit tests: `test/clj/auto_ap/import/transactions_test.clj` already covers `categorize-transaction`.
|
||||
|
||||
## Topic Axes
|
||||
|
||||
- **Input/paste fidelity** — keeping the exact Yodlee positional-column paste payload (req #1)
|
||||
- **UI flow & surface** — modal vs. dedicated two-stage paste→review→import page (req #2)
|
||||
- **Validation architecture** — where shape vs. business rules live, and how errors surface (req #3)
|
||||
- **Core/backend reuse** — how much of `import.transactions` / `import.manual` is reused vs. reimplemented (req #5)
|
||||
- **Test strategy** — failing-first e2e, then unit coverage (req #4)
|
||||
|
||||
## The Central Design Fork
|
||||
|
||||
Requirements #1 ("paste the exact same content") and #2 ("follow ledger patterns") pull in slightly different directions, but resolve cleanly:
|
||||
|
||||
- **Input format stays identical** — the user still copies the same Yodlee table and pastes the same tab-separated positional columns. We reuse `manual/columns` + `manual/tabulate-data` for the *paste shape*; we do **not** switch to ledger's named-header columns.
|
||||
- **Everything downstream follows ledger** — dedicated page, clipboard paste, editable review grid, per-row error/warning badges, re-validate loop, notification on import.
|
||||
|
||||
The one decision genuinely worth the user's input is **how much of the ledger UX to adopt**: the full editable review grid (idea #5, higher value, more work) vs. a lighter paste→validate→summary page closer to master (still SSR, less scope). Both are presented below as ranked survivors; this is the seed question for brainstorming.
|
||||
|
||||
## Ranked Ideas
|
||||
|
||||
### 1. Wire the pre-scaffolded routes into a dedicated two-stage SSR import page
|
||||
**Description:** Implement `external-import-page` / `external-import-parse` / `external-import-import` handlers in a new `src/clj/auto_ap/ssr/transaction/import.clj`, add them to `ssr/transaction.clj` `key->handler` (with the same middleware stack: `wrap-must {:activity :view :subject :transaction}`, `wrap-client-redirect-unauthenticated`, etc.), and add a transaction "Import" nav entry in `aside.clj` parallel to the ledger one. The page structure mirrors `ledger/external-import-page`: breadcrumb → clipboard script → paste form → review table.
|
||||
**Axis:** UI flow & surface
|
||||
**Basis:** `direct:` `routes/transactions.cljc` already declares `::external-import-page/parse/import` but `ssr/transaction.clj:101 key->handler` wires none of them; `ssr/ledger.clj:686 key->handler` shows the exact wiring shape to copy.
|
||||
**Rationale:** The routing contract already exists and is unused — the cheapest, lowest-risk foundation, and it's what makes req #2 ("like the ledger import") literally true at the routing/page level.
|
||||
**Downsides:** Adds a new namespace; needs a nav entry and middleware parity to avoid auth gaps.
|
||||
**Confidence:** 95%
|
||||
**Complexity:** Low
|
||||
**Status:** Unexplored
|
||||
|
||||
### 2. Preserve the exact Yodlee positional paste format (req #1)
|
||||
**Description:** Reuse `import.manual/columns` + `tabulate-data` (or a malli `:decode/string` wrapper around them) for parsing the pasted TSV, instead of ledger's named-header `tsv->import-data`. Keep the same column positions (`:status :raw-date :description-original ... :amount ... :bank-account-code :client-code`) so users paste identical content.
|
||||
**Axis:** Input/paste fidelity
|
||||
**Basis:** `direct:` Requirement #1 ("Paste the exact same type of content as was in the master branch version") + `import/manual.clj:10` fixed `columns` vector; the master textarea label is literally "Yodlee manual import table".
|
||||
**Rationale:** Users' upstream copy/paste habit (and the Yodlee export shape) is the contract that must not break; the ledger named-header approach would silently change what content is valid.
|
||||
**Downsides:** Positional columns are brittle if Yodlee changes column order — but that's already the status quo, not a regression.
|
||||
**Confidence:** 90%
|
||||
**Complexity:** Low
|
||||
**Status:** Unexplored
|
||||
|
||||
### 3. Reuse the `import.transactions` engine as the backend (req #5)
|
||||
**Description:** The import handler maps parsed rows → `:transaction/*` maps via the existing `manual->transaction` shape, then drives `import.transactions/start-import-batch :import-source/manual` + `import-transaction!` + `finish!` + `get-stats` — exactly as `import.manual/import-batch` does today. The SSR layer is presentation + pre-validation only; the categorization, rule-matching, payment/deposit clearing, synthetic-id dedupe, and audit-transact paths are untouched.
|
||||
**Axis:** Core/backend reuse
|
||||
**Basis:** `direct:` Requirement #5 ("Minimally, if at all, change any core components") + `import/manual.clj:32 import-batch` already encapsulates the whole engine call; `import/transactions.clj` is covered by `transactions_test.clj`.
|
||||
**Rationale:** This is the battle-tested core. Re-implementing it ledger-style would risk dropping validations (`categorize-transaction`) and duplicate a large, audited code path. Wrapping it keeps core change near zero.
|
||||
**Downsides:** `import-batch` returns summary stats, not per-row form-cursor errors — so the pre-validation layer (idea #4) must surface row errors *before* the engine runs.
|
||||
**Confidence:** 90%
|
||||
**Complexity:** Medium
|
||||
**Status:** Unexplored
|
||||
|
||||
### 4. Two-tier validation: malli shape-parse + a transaction `add-errors` business layer
|
||||
**Description:** Mirror ledger's split. Tier 1: a malli `parse-form-schema` decodes the pasted TSV and coerces/validates *shape* (date parses as `MM/dd/yyyy`, amount parses, required fields present) — reusing `manual.common/parse-date`/`parse-amount` as `:decode`/predicate fns. Tier 2: a transaction-specific `add-errors`/`table->entries` adds *business* errors/warnings by reusing the predicates already encoded in `categorize-transaction`: client-not-found (`:error`), bank-account-not-found (`:error`), date before `bank-account/start-date` (`:warn`/`:not-ready`), client locked-until (`:warn`), status not `POSTED` (`:not-ready`), already-imported/extant (`:warn`). Errors are `[message status]` pairs surfaced via `flatten-errors` → form-cursor field paths.
|
||||
**Axis:** Validation architecture
|
||||
**Basis:** `direct:` Requirement #3 ("every validation maintained… but doesn't have to follow the same structure — make it like the ledger import"). Maps master validations (`manual.clj:23-30`, `transactions.clj:191-225 categorize-transaction`) onto ledger's `add-errors`/`flatten-errors` shape (`ledger.clj:380-519`).
|
||||
**Rationale:** Gives the ledger-style inline error UX while guaranteeing 1:1 validation parity — each master check becomes an explicit `add-errors` clause, which is also directly unit-testable (one test per error type, like `ledger_test/add-errors-test`).
|
||||
**Downsides:** Some checks (extant/duplicate, start-date, locked-until) need a DB read at validation time that master does lazily inside the engine — must decide whether to pre-check or let the engine's stats report them. Risk of double-validation drift if engine and pre-validator disagree.
|
||||
**Confidence:** 80%
|
||||
**Complexity:** High
|
||||
**Status:** Unexplored
|
||||
|
||||
### 5. Editable review grid with per-row error/warning badges and a re-validate loop
|
||||
**Description:** After paste+parse, render rows into a `com/data-grid-card` where each field (date, amount, description, bank-account-code, client-code) is an editable `fc/with-field` input, with `com/validated-field` error display and a per-row alert badge+tooltip — exactly like `ledger/external-import-table-form*`. The user can fix a wrong client/bank-account code or date inline and re-submit; only clean rows import, warn-only rows are skipped, error rows block. A "Show table" toggle keeps the default view compact.
|
||||
**Axis:** UI flow & surface / validation surfacing
|
||||
**Basis:** `direct:` Requirement #2 ("follow slightly better design patterns, like how the ledger import works"). `ledger.clj:117-244` is the editable-grid implementation to copy; master has no inline correction at all (fire-and-forget modal + summary stats).
|
||||
**Rationale:** This is the concrete UX upgrade over master and the main reason to model on ledger — turning "paste, pray, read a stats blob" into "paste, see exactly which rows are wrong and why, fix them, import."
|
||||
**Downsides:** Highest-effort idea; form-cursor round-tripping of an editable grid is the trickiest part of the ledger code. If scope must shrink, a read-only review table + summary (lighter survivor) is the fallback.
|
||||
**Confidence:** 75%
|
||||
**Complexity:** High
|
||||
**Status:** Unexplored
|
||||
|
||||
### 6. Preserve idempotent re-import via synthetic-id duplicate detection, surfaced as "already imported"
|
||||
**Description:** Keep `apply-synthetic-ids`/`synthetic-key` so re-pasting the same export is idempotent (the engine categorizes extant rows as `:extant` and skips them). Surface this in the review grid as a `:warn`-level "already imported" badge rather than silently dropping it, so the user understands why a row didn't import.
|
||||
**Axis:** Validation architecture
|
||||
**Basis:** `direct:` `import/transactions.clj:405-421 apply-synthetic-ids` + `categorize-transaction:192-225` extant handling. This is an existing master behavior that req #3 requires us to maintain.
|
||||
**Rationale:** Duplicate-safety is an easy validation to lose in a port; making it visible (vs. master's opaque stats) is a small, high-trust UX win that costs almost nothing on top of idea #5.
|
||||
**Downsides:** Requires a DB read of existing `:transaction/id`s at validation time (or reading it back from engine stats post-import).
|
||||
**Confidence:** 80%
|
||||
**Complexity:** Low
|
||||
**Status:** Unexplored
|
||||
|
||||
### 7. Start with a failing Playwright e2e, then backfill ledger_test-style unit coverage (req #4)
|
||||
**Description:** First commit: a failing e2e (against `test-server`) that dev-logs in, navigates dashboard → transactions → Import, pastes a known-good Yodlee TSV into the paste box, asserts parsed rows render, clicks Import, and asserts the transactions appear / a "N imported" notification fires. It fails initially (no handler/nav). Then make it pass incrementally: route+page (idea #1) → parse (#2/#4 tier 1) → review grid (#5) → import via engine (#3) → business validation (#4 tier 2). Backstop with unit tests mirroring `ledger_test.clj`: `tabulate-data`/parse, each `add-errors` validation clause, and an end-to-end `import` test against Datomic. Reuse the existing `transactions_test.clj` `categorize-transaction` coverage as the validation-parity oracle.
|
||||
**Axis:** Test strategy
|
||||
**Basis:** `direct:` Requirement #4 ("Write detailed acceptance criteria, and start with a failing e2e test, making it pass over time"). `test-server.clj` already provides the browser harness + test auth; `ledger_test.clj` provides the unit-test template.
|
||||
**Rationale:** A red e2e pins down the acceptance contract before any handler exists and gives an unambiguous "done" signal; the unit layer locks in validation parity clause-by-clause so req #3 can't silently regress.
|
||||
**Downsides:** e2e clipboard paste may need a direct `type`-into-textarea path (or a test seam) since `navigator.clipboard.read()` is awkward to drive headless — plan a paste fallback the test can use.
|
||||
**Confidence:** 85%
|
||||
**Complexity:** Medium
|
||||
**Status:** Unexplored
|
||||
|
||||
## Draft Acceptance Criteria (seed for brainstorm/plan, per req #4)
|
||||
|
||||
**Routing & access**
|
||||
- [ ] `GET /transactions/external-import-new` renders an import page (admin-gated, same middleware as other transaction routes); 401/redirect for unauthenticated.
|
||||
- [ ] A "Import" nav entry appears under the transactions section, active on the import route.
|
||||
|
||||
**Paste & parse (req #1)**
|
||||
- [ ] Pasting the exact master Yodlee TSV (same positional columns) parses into the same field set as `manual/tabulate-data`.
|
||||
- [ ] Header row is dropped; blank rows ignored.
|
||||
- [ ] `POST …/parse` re-renders the page with a "N rows found" banner and the review table.
|
||||
|
||||
**Validation parity (req #3)** — each must produce a visible, row-attributed message:
|
||||
- [ ] Client not found for bank-account-code → error.
|
||||
- [ ] Bank account not found by code → error.
|
||||
- [ ] Date not `MM/dd/yyyy` / unparseable → error.
|
||||
- [ ] Amount unparseable → error.
|
||||
- [ ] Status ≠ `POSTED` → not-imported (warn/not-ready).
|
||||
- [ ] Date before `bank-account/start-date` → not-imported (not-ready).
|
||||
- [ ] Date on/before `client/locked-until` → not-imported (not-ready).
|
||||
- [ ] Already-imported (synthetic-id extant) row → skipped, surfaced as warn.
|
||||
- [ ] Missing client / bank-account / id → error.
|
||||
|
||||
**Import (req #5, minimal core change)**
|
||||
- [ ] `POST …/import` runs the existing `import.transactions` engine via the `:import-source/manual` batch path; no change to `categorize-transaction`/`import-transaction!`/`apply-synthetic-ids`.
|
||||
- [ ] Only clean rows import; warn-only rows skipped; any error blocks (or imports clean rows + reports errors — match master's "import valid, report failed-validation").
|
||||
- [ ] Success notification reports counts (imported / skipped / errors), mirroring master's stats.
|
||||
- [ ] Re-importing the same paste is idempotent (no duplicates).
|
||||
|
||||
**Tests (req #4)**
|
||||
- [ ] A Playwright e2e covering paste → review → import → assert, committed red first, green at the end.
|
||||
- [ ] Unit tests per validation clause + a Datomic-backed end-to-end import test, modeled on `ledger_test.clj`.
|
||||
|
||||
## Failing-First e2e: concrete starting point
|
||||
|
||||
Add `test/clj/auto_ap/ssr/transaction/import_test.clj` (unit) and a Playwright spec driven through `test-server`. The e2e is the first artifact and is expected to fail because no `external-import` handler is wired in `ssr/transaction.clj`. Make it green by walking ideas #1 → #2 → #5 → #3 → #4 in that order; the unit suite grows alongside #4.
|
||||
|
||||
## Rejection Summary
|
||||
|
||||
| # | Idea | Reason Rejected |
|
||||
|---|------|-----------------|
|
||||
| 1 | Switch paste format to ledger's named-header columns | Violates req #1 — users paste the exact Yodlee positional export; changing valid input is a silent regression |
|
||||
| 2 | Keep it a re-frame/CLJS modal | Branch eliminated the CLJS app; contradicts the whole port. (An SSR htmx *modal* was considered but rejected vs. the dedicated page — ledger uses a page and the editable review grid needs the room) |
|
||||
| 3 | Reimplement validation entirely ledger-style, ignoring `import.transactions` | Duplicates audited `categorize-transaction` logic, risks dropping validations (req #3), and churns core (violates req #5) |
|
||||
| 4 | Async/streaming import for large pastes | Scope overrun — master is synchronous; YAGNI for the manual paste workflow |
|
||||
| 5 | Add CSV file-upload alongside paste | Scope overrun — not part of the master manual-import feature |
|
||||
| 6 | Replace `import-batch` stats with a bespoke result type | Unnecessary core change; the existing stats map already carries imported/failed/sample-error |
|
||||
Reference in New Issue
Block a user