feat(transactions): port manual bank-transaction import to SSR #8

Closed
notid wants to merge 133 commits from integreat-add-transaction-manual into master
Owner

Summary

Ports the master-branch manual bank-transaction import to the SSR/alpinejs/htmx stack. The route names ::external-import-page/parse/import were already declared in routes/transactions.cljc but no handler served them — this fills that gap, modeled on the existing SSR ledger import.

Flow: a dedicated admin page (/transaction2/external-import-new) where you paste the exact same Yodlee positional-column TSV as before → parsed rows render in an editable review grid with per-row error/warning badges → import. Every master validation is preserved, and the existing import.transactions engine is reused unchanged (via import.manual/import-batch).

What changed

  • New ns auto-ap.ssr.transaction.import — page, clipboard/paste form, parse handler, editable form-cursor grid, two-tier validation, and import handler. Wired into ssr/transaction.clj key->handler; admin-only via wrap-must {:activity :import :subject :transaction} + assert-admin.
  • Two-tier validation. Tier 1 (malli, positional Yodlee columns) parses shape; Tier 2 attaches [message status] per row. Hard errors (bad date, unparseable amount, unknown client/bank-account code, missing fields) block the whole batch (throw+ :field-validation re-renders the grid). Warnings (before bank-account start-date, before client locked-until, already-imported) skip just that row. Warn conditions are computed from the engine's own categorize-transaction, so the grid preview matches the import result.
  • Transactions "Import" nav entry (admin-only) in ssr/components/aside.clj.
  • Testse2e/transaction-import.spec.ts (3 Playwright tests, committed failing-first then made green) and ssr/transaction/import_test.clj (10 clojure.test deftests, 33 assertions, against in-memory Datomic). Deterministic bank-account code added to the test-server seed.

Validation

  • Unit/integration: 10 tests / 33 assertions green (parsing, each validation clause, clean import, block-on-hard-error, idempotent re-import → extant, warn-skip).
  • e2e (real Chromium): renders the page, paste→parse→grid→import a valid row (appears on the list), and a blocking-error batch creates nothing.
  • clj-kondo: 0 warnings. cljfmt: clean.
  • The 2 unrelated failures in the existing e2e suite (transaction-navigation date-range persistence, transaction-edit shared-location) were verified pre-existing — they fail on the baseline with this branch's changes stashed.

Residual Review Findings

From the automated code review (advisory; not blocking this PR):

Engineering follow-ups

  • [P2] client-code is an editable grid column the import never consumes (client is derived from bank-account-code) — remove or make read-only. (maintainability + correctness)
  • [P2] No unit-level test for the wrap-nested-form-params → :table coercion on the edit-then-import path (covered by e2e end-to-end, not at unit level). (testing + reliability)
  • [P2] classify-table re-runs the two full-table lookups that import-batch also runs (~4 queries/cycle); thread resolved lookups through. Minor at admin scale. (maintainability + performance)
  • [P2] classify-table DB queries lack try/catch → raw 500 on Datomic failure (mirrors ledger's existing pattern). (reliability)
  • [P3] Parse helpers partially duplicate import.manual/tabulate-data; private import-transactions helper name collides with the import.transactions (t) alias. (maintainability)

Product decisions (master-faithful as shipped)

  • [P2] Non-POSTED rows import as POSTED (status is forced, like master), so the "not posted → skip" warning sub-condition can't fire. Decide: keep master parity (drop the wording) or pass the row's real status through.
  • [P2] On import the grid is cleared even when some rows were skipped/errored; write-time errors surface only as an aggregate count (matches master's stats behavior).

Plan

docs/plans/2026-06-01-001-feat-manual-transaction-import-ssr-plan.md

🤖 Generated with Claude Code

## Summary Ports the master-branch **manual bank-transaction import** to the SSR/alpinejs/htmx stack. The route names `::external-import-page/parse/import` were already declared in `routes/transactions.cljc` but no handler served them — this fills that gap, modeled on the existing SSR ledger import. Flow: a dedicated admin page (`/transaction2/external-import-new`) where you paste the **exact same Yodlee positional-column TSV** as before → parsed rows render in an **editable review grid** with per-row error/warning badges → import. Every master validation is preserved, and the existing `import.transactions` engine is **reused unchanged** (via `import.manual/import-batch`). ## What changed - **New ns `auto-ap.ssr.transaction.import`** — page, clipboard/paste form, parse handler, editable form-cursor grid, two-tier validation, and import handler. Wired into `ssr/transaction.clj` `key->handler`; admin-only via `wrap-must {:activity :import :subject :transaction}` + `assert-admin`. - **Two-tier validation.** Tier 1 (malli, positional Yodlee columns) parses shape; Tier 2 attaches `[message status]` per row. *Hard errors* (bad date, unparseable amount, unknown client/bank-account code, missing fields) **block the whole batch** (`throw+ :field-validation` re-renders the grid). *Warnings* (before bank-account start-date, before client locked-until, already-imported) **skip just that row**. Warn conditions are computed from the engine's own `categorize-transaction`, so the grid preview matches the import result. - **Transactions "Import" nav entry** (admin-only) in `ssr/components/aside.clj`. - **Tests** — `e2e/transaction-import.spec.ts` (3 Playwright tests, committed failing-first then made green) and `ssr/transaction/import_test.clj` (10 clojure.test deftests, 33 assertions, against in-memory Datomic). Deterministic bank-account code added to the test-server seed. ## Validation - Unit/integration: 10 tests / 33 assertions green (parsing, each validation clause, clean import, block-on-hard-error, idempotent re-import → extant, warn-skip). - e2e (real Chromium): renders the page, paste→parse→grid→import a valid row (appears on the list), and a blocking-error batch creates nothing. - clj-kondo: 0 warnings. cljfmt: clean. - The 2 unrelated failures in the existing e2e suite (`transaction-navigation` date-range persistence, `transaction-edit` shared-location) were verified **pre-existing** — they fail on the baseline with this branch's changes stashed. ## Residual Review Findings From the automated code review (advisory; not blocking this PR): **Engineering follow-ups** - **[P2]** `client-code` is an editable grid column the import never consumes (client is derived from bank-account-code) — remove or make read-only. *(maintainability + correctness)* - **[P2]** No unit-level test for the `wrap-nested-form-params → :table` coercion on the edit-then-import path (covered by e2e end-to-end, not at unit level). *(testing + reliability)* - **[P2]** `classify-table` re-runs the two full-table lookups that `import-batch` also runs (~4 queries/cycle); thread resolved lookups through. Minor at admin scale. *(maintainability + performance)* - **[P2]** `classify-table` DB queries lack try/catch → raw 500 on Datomic failure (mirrors ledger's existing pattern). *(reliability)* - **[P3]** Parse helpers partially duplicate `import.manual/tabulate-data`; private `import-transactions` helper name collides with the `import.transactions` (`t`) alias. *(maintainability)* **Product decisions (master-faithful as shipped)** - **[P2]** Non-POSTED rows import as POSTED (status is forced, like master), so the "not posted → skip" warning sub-condition can't fire. Decide: keep master parity (drop the wording) or pass the row's real status through. - **[P2]** On import the grid is cleared even when some rows were skipped/errored; write-time errors surface only as an aggregate count (matches master's stats behavior). ## Plan `docs/plans/2026-06-01-001-feat-manual-transaction-import-ssr-plan.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code)
notid added 139 commits 2026-06-01 11:36:54 -07:00
- Added comprehensive test suite for account creation, validation, and grid views
- Documented Datomic entity reference handling patterns in test comments
- Created 5 test improvement todo documents addressing common test anti-patterns
- Todo items cover: removing debug statements, fixing test nesting, strengthening assertions, extracting helpers, and removing doc-only tests
Add 8 BDD-style tests for the vendors module covering grid/list
operations and vendor merge functionality. Tests follow established
patterns from accounts_test.clj and include proper database
verification.

Tests Implemented:
- vendor-grid-loads-with-empty-database
- vendor-fetch-ids-returns-correct-structure
- vendor-fetch-page-returns-vendors
- vendor-hydrate-results-works
- vendor-merge-transfers-references
- vendor-merge-same-vendor-rejected
- vendor-merge-invalid-vendor-handled
- vendor-hydration-includes-all-fields

Key Implementation Details:
- Uses setup-test-data helper with unique temp IDs
- Tests focus on public interface (fetch-page, merge-submit)
- Follows BDD Given/When/Then pattern
- All 8 tests passing (26 assertions)

Documentation:
- Created implementation plan in docs/plans/
- Documented solution patterns in docs/solutions/
- Created code review todos for future improvements

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add comprehensive tests for the SSR admin transaction rules module:
- Rule matching by description pattern
- Rule matching returns empty for no matches
- Validation accepts valid data with 100% account allocation
- Validation rejects invalid account totals
- Rule matching by amount range
- Rule matching by bank account
- Security tests for non-admin access
- Execute validation tests

All 8 tests passing with 9 assertions. Tests focus on the unique
rule matching engine functionality that differentiates transaction
rules from other admin modules.

Includes implementation plan documenting 23 test scenarios
and 6-phase approach for complete coverage.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add 4 new tests for route handlers:
- page-route-returns-html-response: Tests main page route
- table-route-returns-table-data: Tests HTMX table data route
- delete-route-deletes-rule: Tests delete functionality
- check-badges-route-works: Tests badge checking route

Total: 12 tests with 15 assertions, all passing.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add 3 additional route handler tests:
- edit-dialog-route-returns-dialog: Tests edit dialog for existing rules
- account-typeahead-route-works: Tests account typeahead functionality
- location-select-route-works: Tests location selector route

Total: 15 tests with 22 assertions, all passing.

Route coverage now includes:
- page, table, delete, check-badges
- edit-dialog, account-typeahead, location-select

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add 2 more route handler tests:
- execute-dialog-route-works: Tests execution dialog for rules
- new-dialog-route-returns-empty-form: Tests new rule form

Total: 17 tests with 26 assertions, all passing.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add require-approval schema validation for :manual action
- Fix keyword comparison to use :transaction-approval-status/approved
- Move require-approval function before schema definition
- Also fix save handler validation to use correct keyword
When validation errors occur at the root level (e.g., from :fn validators
in multi schemas), they are returned as a vector directly rather than a
map with :errors key. Update default-step-footer to handle both cases.
Ensure the validation error message shows up properly when users try
to approve a manual transaction without assigning financial accounts.
When a transaction account had 'Shared' location, spread-account was
creating multiple accounts with the same :db/id but different locations,
causing a datoms-conflict error in Datomic.

Now generates unique tempids for all spread accounts beyond the first,
preventing entity ID collisions while preserving the original account's
tempid for the first location.

Fixes: Two datoms in the same transaction conflict on :transaction-account/location
When a transaction is saved via the edit modal, return the updated row
HTML with {:flash? true} instead of an empty div. This makes the row
flash with the live-added animation, matching the behavior of other
pages like invoices and accounts.

Uses hx-retarget to swap the specific row in the table while also
triggering modalclose to close the modal.
When editing a transaction manually, users can now toggle between viewing
account amounts as dollar values or percentages. The toggle appears in the
table header as a radio button group ($ / %).

Key features:
- Global toggle switches all accounts simultaneously
- $→%: amounts are converted to percentages of the transaction total
- %→$: uses percentages->dollars with spread-cents for accurate cent distribution
- Form state preserves vendor, memo, approval status when toggling
- Save handler converts % back to $ before persisting to Datomic
- Uses HTMX to re-render only the account grid body on toggle

New route: /toggle-amount-mode
New functions: ->percentage, percentages->dollars, convert-accounts-mode,
              account-grid-body*, toggle-amount-mode
- Fix Math/abs nil error when adding new accounts by using (or value 0.0)
- Fix Math/abs nil in account-grid-body* and save-handler for safety
- Make $/% radio toggle display side-by-side using :orientation :horizontal
- Apply fixes to edit-wizard-new-account render, account-grid-body*, and save-handler
The assoc-in call had too many arguments, so the request state wasn't being
updated with the new mode or converted accounts. Using -> threading with
separate assoc-in calls ensures both the accounts and mode are properly set
before re-rendering the grid.
The radio-card component was ignoring HTMX attributes (:hx-post, :hx-target,
etc.) passed to it. Modified the component to extract these attributes and
merge them into each radio input element, so the $/% toggle now properly
triggers HTMX requests when changed.
The toggle-amount-mode handler was failing because account-grid-body*
uses fc/cursor-map which requires the form cursor context to be set up.
Added manual cursor binding in toggle-amount-mode to create a cursor
pointing to the transaction/accounts vector and bind it to fc/*current*
before rendering the grid.
The previous attempts to set up form cursor context in toggle-amount-mode
were failing because the cursor library's dynamic binding model is complex
and requires specific initialization through fc/start-form.

Instead of trying to recreate the cursor context, this fix:
1. Creates transaction-account-row-no-cursor* that renders rows with explicit
   field names and values (no cursor functions)
2. Rewrites toggle-amount-mode to directly construct the data-grid HTML
   using map-indexed over the accounts vector
3. Removes the broken manual cursor binding attempts
4. Removes unused auto-ap.cursor import

This ensures the toggle handler works independently of the wizard's cursor
context while still producing identical HTML output.
The format specifier $%,.2f requires floating-point values but
(reduce + 0 ...) can return a Long when all amounts sum to an integer.
Added explicit (double ...) casts and changed initial value to 0.0
to ensure the format call always receives a double.
- Create requirements document based on master cljs implementation
- Add Playwright e2e tests covering happy path, validation, and distribution
- Fix hiccup id syntax in SSR bulk code form (div#id.class order)
- Add missing account location validation to SSR bulk code submit
- Enhance test server with multiple transactions and fixed-location account
- Add vendor-changed HTMX handlers for both bulk code and individual edit
- Pre-populate default account at 100% when vendor is selected and no accounts exist
- Fix render-accounts-section to render from step-params correctly
- Change bulk code vendor-changed from hx-get to hx-post to include form data
- Add routes for vendor-changed endpoints
- Update e2e tests to cover vendor pre-population
- Run lein cljfmt fix across codebase
- vendor-default-account now uses raw vendor default account (not client-specific override)
- Account name is clientized via d-accounts/clientize only for single-client contexts
- Added single-client-id helper that returns client ID only when user has exactly one client
- Added multi-client e2e test verifying no pre-population across multiple clients
- Updated test server to support multi-client mode switching via /test-set-client-mode
- Test server now seeds a second client for multi-client scenarios
When typing in the company dropdown search, check for an exact match
on client code via Datomic before falling back to Solr name search.
This allows users to quickly find clients by typing their code (e.g. NGRV).
Fixes substring search in company dropdown. The search query was
using raw user input instead of the cleansed version that adds a
wildcard suffix (e.g. 'dough' -> 'dough*'). Without the wildcard,
Solr performs exact token matching, so searching 'dough' would not
match 'Doughballs'.
- Add new memo filter to transaction page (searches :transaction/memo)
- Enhance existing description filter to use case-insensitive regex
- Both filters support wildcard matching via .* pattern
- Add e2e tests for filter functionality
- Update test data with memo fields
The hx-vals attribute with a JavaScript IIFE was causing a SyntaxError
when navigating to the transactions page from any other page. Replaced
with hx-include="#transaction-filters" which correctly preserves
filter state across transaction sub-pages.
When a transaction is pre-coded, the snapshot stores :transaction-account/account
as a Datomic ref map {:db/id N} rather than a bare integer. simple-mode-fields*
and the simpleAccountId Alpine initializer both need the integer id, not the map,
to correctly populate the account typeahead value and the x-hx-val binding.
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>
- Alphabetize the import.clj :require block (AGENTS.md Import Formatting).
- Remove unused imports (digest, strip) flagged by clj-kondo.
- Make the client-not-found classify-table test independent: it previously
  reused the bank-account-not-found input and added zero marginal coverage;
  now seeds an orphan bank account so only the client error fires.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
notid closed this pull request 2026-06-01 19:58:03 -07:00

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: notid/integreat#8