Files
integreat/docs/plans/2026-06-27-001-fix-ssr-modal-regressions-plan.md
Bryce fc8ce2633e fix(ssr): bulk-edit empty-selection 500 + modal-footer EDN leak
- BUG D: clicking "Bulk Edit" with no invoices selected 500'd. selected->ids
  returns nil with no selection, and all-ids-not-locked fed that nil into a
  Datomic `:in $ [?i ...]` query ("Unable to find data source"). Guard the body
  with (when (seq all-ids) ...) so an empty selection yields [] and the modal
  opens cleanly. Verified live: no "Oh, drat" toast, no 500.

- BUG B: modal-footer- called (hx/alpine-appear ...) twice; the 2nd return value
  (an attribute map) landed in child position and rendered as literal EDN
  ({:x-show ...}) in the red error banner whenever unexpectedError flipped true.
  Delete the duplicate. Verified: rendered HTML now has one alpine-appear and no
  EDN-text child. (Pre-existing defect, also present on master.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-27 13:21:50 -07:00

209 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# SSR Modal Regression Fixes — QA Findings & Resumable Task List
> **Status:** BUG A, BUG C, BUG D, BUG B, BUG E/F, and the CSS size root cause are
> **FIXED + verified** (2026-06-27). Remaining: modal step animations (§3). Resume from §4/§3.
> **Owner:** Bryce
> **Branch:** `integreat-execute-refactor`
> **Date:** 2026-06-27
> **Context:** Follow-up to the SSR rendering-modernization migration
> (`2026-06-02-001-refactor-ssr-rendering-modernization-plan.md`, Phases 211 complete).
> The migration to simplified route rendering + whole-form/wizard swaps introduced
> modal **size**, **animation**, and a set of **HTTP 500** regressions. This doc records a
> full browser QA pass (agent-browser, 9 migrated modals) with root causes and fixes.
## 0. How to resume / reproduce
- App is running on the port in `.http-port` (was `42987`); nREPL in `nrepl-port` (was `35689`).
- Browser session: `agent-browser --session integreat ...`. Log in via
`http://localhost:<port>/dev-login` → "Continue to dashboard".
- **Test at a realistic viewport:** `agent-browser --session integreat set viewport 1440 900`
(a short viewport exaggerates modal overflow and gives false positives).
- Screenshots from this pass are in `./tmp/` (gitignored): `02..19-*.png`. Raw running notes:
`./tmp/findings.md`.
- After rebuilding CSS, the browser caches the old `output.css`; bust it by swapping the
`<link>` href (`?v=<n>`) or hard-reloading, or you'll keep seeing the old sizes.
## 1. Test matrix (9 migrated modals)
| Modal | File | Type | Result |
|-------|------|------|--------|
| Transaction Edit | `transaction/edit.clj` | form | Opens OK; **BUG C** — whole-form swap 500 |
| Transaction Bulk Code | `transaction/bulk_code.clj` | form | ✅ Works |
| Sales Summary Edit | `pos/sales_summaries.clj` | form | ✅ Works |
| Invoice Bulk Edit | `invoices.clj` | form | ✅ Works (empty-selection 500 = **BUG D**) |
| Transaction Rule | `admin/transaction_rules.clj` | wizard | ✅ Works |
| Invoice Pay | `invoices.clj` | wizard | ✅ Works (narrow until CSS rebuild — see §2) |
| New Invoice | `invoice/new_invoice_wizard.clj` | wizard | **BUG A** — choose-client 500 |
| Vendor | `admin/vendors.clj` | wizard | Size broken (§2) + **BUG E** inner overflow |
| Client | `admin/clients.clj` | wizard | Size broken (§2) + **BUG F** inner overflow |
Cross-cutting: **§2** (stale CSS → sizes), **BUG B** (footer EDN leak), **animations** (§3).
## 2. ★ ROOT CAUSE of broken modal SIZES — stale compiled Tailwind CSS
`resources/public/output.css` (committed, last built **Jun 24**) is **missing the migration's
newer arbitrary size classes**. Tailwind only compiles classes present in source at build time,
so any size value added/changed after Jun 24 isn't in the CSS and the element falls back to its
`w-full h-full` sibling class → the modal balloons to full width.
- **Present** (these modals size correctly today): `md:w-[750px]` (New Invoice), `md:w-[950px]`
(Transaction Edit / Bulk Code / Sales Summary), `md:h-[600px]`, `md:h-[650px]`, `w-[850px]`
(Transaction Rule).
- **Missing** (these balloon / mis-size): `md:w-[760px]` (Vendor), `md:w-[820px]` (Client),
`md:h-[520px]`, `md:h-[560px]`, `w-[50em]` (Invoice Pay).
**Verified:** rebuilding to a temp file generates all missing classes; after rebuild + cache-bust,
the Vendor card went **1257px → 760×520** and Client → **820×560** (exactly as declared).
**DEEPER ROOT CAUSE found while fixing (2026-06-27):** `tailwind.config.js` `content` only scanned
`./src/**/*.{cljs,clj,cljc}` — it **never scanned `resources/templates/**/*.html`** (the 46 Selmer
templates the migration introduced). So a naive rebuild *drops* every template-only class
(e.g. `md:w-[950px]` / `md:h-[650px]`, used only in `templates/transaction-edit/edit-modal.html`,
which would have re-broken Transaction Edit / Bulk Code / Sales Summary). The durable fix is to add
the templates glob to the config, then rebuild.
**FIX — DONE & verified:**
1. Added `"./resources/templates/**/*.html"` to `tailwind.config.js` `content`.
2. `npx tailwindcss -i resources/input.css -o resources/public/output.css` (kept unminified to
match the committed file; add `--minify` only if the prod pipeline minifies).
- Verified: all modal size classes now present (`md:w-[760px]`, `md:w-[820px]`, `md:h-[520px]`,
`md:h-[560px]`, `w-[50em]` **and** the template-sourced `md:w-[950px]`/`md:h-[650px]`). Class-diff
vs the old CSS shows the only removed classes are orphaned (the deleted `mm/*` modal-stack
`forward`/`backward`/`group/transition`/`htmx-*:translate-x-2/3` animation set + the unused
`lg:w-[900px]` sales size) — none still referenced in src/ or templates/. Vendor modal confirmed
live at **760×520** (was 1257px).
- **Still TODO:** confirm the prod/CI build runs this tailwind step (so output.css can't drift
again) — ideally wire it into `lein build` / buildspec.
> Note: during QA I rebuilt `output.css` to verify, then reverted it so the working tree is clean
> for review. The verified rebuild is preserved at `tmp/output-rebuilt.css`; backup at
> `tmp/output.css.bak`.
## 3. Modal ANIMATIONS
The new wizard step cards swap via `hx-target "this" hx-swap "outerHTML"` but their
`modal-card-advanced` carries **no transition classes**, so step→step and modal enter/leave do not
animate:
- New Invoice steps (`render-basic-details`, `render-accounts`): card class is only
`md:w-[750px] md:h-[600px] w-full h-full` — no `htmx-swapping:`/`htmx-added:` variants.
- `next-steps-modal` (new_invoice_wizard.clj ~678): only static `scale-100 translate-x-0
opacity-100` — missing the `htmx-swapping:`/`htmx-added:` swap variants.
- By contrast, `dialog/success-modal-` and the Invoice Pay card (`last-modal-step transition
duration-150`) keep transitions, so the intended pattern still exists to copy from.
**FIX:** add the swap-transition classes to the wizard step cards (mirror `success-modal-`'s
`transition duration-300 ease-in-out htmx-swapping:... htmx-added:...` string, or the lighter
`last-modal-step transition duration-150` used by the transaction-edit card). Confirm the
`htmx-swapping:`/`htmx-added:` custom variants survive the §2 CSS rebuild.
## 4. Bug list + fixes (prioritized task list)
### P0 — functional 500s
**BUG A — New Invoice: choosing a client → 500 from `/invoice/new/due-date`.**
- File: `src/clj/auto_ap/ssr/invoice/new_invoice_wizard.clj`, fn `due-date` (and same latent bug in
`scheduled-payment-date`).
- Exception: `ClassCastException: org.joda.time.DateTime cannot be cast to java.util.Date`.
- Cause: `form-vendor-client` decodes `:invoice/date`/`:invoice/due` to **clj-time `DateTime`**
(via `clj-date-schema`), but `due-date` then calls `(some-> date coerce/from-date)` —
`coerce/from-date` expects a `java.util.Date`. `scheduled-payment-date` has the same
`(some-> due coerce/from-date)` but is masked because `due` is nil-guarded by a `when`.
- Repro: open New Invoice, pick any client (vendor still empty) → red "unexpected error" banner;
network shows PUT `/invoice/new/due-date` = 500. REPL-confirmed the decoded date is already a
`DateTime` and `time/plus` works on it directly.
- **FIX — DONE & verified live:** dropped the `coerce/from-date` calls — `due-date` now uses the
decoded `date`/`due` `DateTime`s directly (removed the `date (some-> date coerce/from-date)`
rebind and the `due` coerce); same for `scheduled-payment-date`. Live: selecting a client now
fires PUT `/invoice/new/due-date` + `/scheduled-payment-date` → **200** (was 500).
**BUG C — Transaction Edit: any whole-form swap (mode toggle, vendor change, add/remove row, $/%
toggle) → 500 from `/transaction2/edit-form-changed`, whenever the txn has ≥1 autopay-invoice
match.** (HIGH severity — breaks the flagship modal's core swap doctrine.)
- File: `src/clj/auto_ap/ssr/transaction/edit.clj`.
- Exception: `ClassCastException: PersistentVector cannot be cast to Named` at **edit.clj:849**
(`links-body*` does `(name (:action step-params))`).
- Cause: the link-panels each render a hidden `<input name="action">`. The **unpaid** panel
(line 690) and **rule** panel (line 730) add `:form ""` to exclude that hidden from form
serialization. The **autopay** panel (**line 661**) is **missing `:form ""`**, so its
`action="link-autopay-invoices"` hidden is serialized alongside the main `action` hidden (line
862, value "manual"). Ring collapses the duplicate `action` params into a vector → `(name
vector)` throws.
- Repro: edit a transaction whose "Link to autopay invoices" tab shows a badge count ≥1, click
"Switch to advanced mode". REPL-confirmed: single `action` → 200; `action` as a vector → the
exact 500. (Txns with 0 autopay matches render `panel-empty*` with no action-hidden and swap
fine — hence intermittent.)
- **FIX — DONE & verified live:** added `:form ""` to the autopay panel's action-hidden
(**edit.clj:661**) to match the unpaid/rule panels. Live: on a txn with an autopay match,
"Switch to advanced mode" now POSTs `edit-form-changed` → **200** with a single `action=manual`
param (the duplicate `action=link-autopay-invoices` is gone) and the mode toggles correctly.
- (Optional hardening, not applied: make `links-body*` coerce a vector action via
`(some-> (:action step-params) (as-> a (if (coll? a) (last a) a)) name)`.)
### P1 — sizing / layout (after §2 CSS rebuild)
**§2 CSS rebuild** — do first; fixes Vendor/Client/Invoice-Pay widths.
**BUG E/F — Vendor + Client wizard step layout overflows the card** — **DONE & verified live.**
- **Actual root cause (found while fixing):** `step-card` places the step timeline in a *vertical*
left sidebar (`grow-0 ... self-stretch hidden md:block`), but `vendor-timeline`/`client-timeline`
rendered the **horizontal** `timeline/timeline` component. A horizontal `<ol>` in a shrink-to-fit
sidebar forced the sidebar to ~full card width, pushing the body off-screen → horizontal scroll,
clipped fields, the "huge grey left region". (Vendor additionally pinned its bodies to a fixed
`w-[600px]`, compounding it.)
- **FIX applied:**
- `vendors.clj` `vendor-timeline` + `clients.clj` `client-timeline`: use
`timeline/vertical-timeline` + `timeline/vertical-timeline-step` (the components already existed)
instead of the horizontal pair.
- `vendors.clj`: the 5 step bodies' `w-[600px] h-[350px]` → `w-full h-[350px]` so the body fills
the remaining width next to the now-narrow vertical timeline.
- **Verified live:** Vendor 760×520 and Client 820×560 with a proper vertical timeline, **no
horizontal overflow**, fields fill the body; forward nav (Info→Terms) works, the vendor-name chip
renders, and the validation-error re-render also lays out correctly.
- Note: edits made with the plain `Edit` tool (exact-string) — the clojure-mcp editors reformat the
whole file against a stricter config than the project's `lein cljfmt`, producing large spurious
diffs; both files pass `lein cljfmt check`.
### P2 — robustness / cosmetic
**BUG D — Invoice Bulk Edit with no selection → 500** — **DONE & verified live.** With no
selection, `selected->ids` returns `nil`, which `all-ids-not-locked` fed straight into a Datomic
`:in $ [?i ...]` query → "Unable to find data source" exception → global "Oh, drat!" toast.
- File: `src/clj/auto_ap/ssr/invoices.clj` — wrapped `all-ids-not-locked`'s body in
`(when (seq all-ids) ...)`, so an empty/nil selection yields `[]` and the modal opens cleanly
(also protects every other caller). Live: Bulk Edit with nothing selected now opens the modal,
no toast, no 500.
**BUG B — Footer leaks a raw Hiccup attr-map as text** in the red "unexpected error" banner
(`{:x-show "unexpectedError", ...}`). **Pre-existing in master** (NOT a migration regression), but
visible whenever `unexpectedError` flips true (e.g. it showed during BUG A). Cause: `modal-footer-`
calls `(hx/alpine-appear {...})` **twice** — the 2nd return value lands in child position and
renders as literal EDN.
- File: `src/clj/auto_ap/ssr/components/dialog.clj` ~line 59.
- **FIX — DONE & verified:** deleted the duplicate `(hx/alpine-appear ...)` line in `modal-footer-`.
Rendering the footer to HTML now shows a single `alpine-appear` (the legit attrs) and no literal
`{:x-show ...}` text child.
**Cosmetic — over-tall empty modals.** Invoice/Transaction Bulk Edit and similar use fixed
`md:h-[650px]`; with little content they show a large empty lower region. Consider letting height
hug content (cap with `max-h`) rather than a fixed height. Low priority.
## 5. Suggested order of work
1. **§2 CSS rebuild** (unblocks all width checks; commit `output.css`; verify prod build does this).
2. **BUG A** + **BUG C** (the two functional 500s; one-liners each, REPL-verified).
3. **BUG E / BUG F** (wizard step-body layout rework; needs visual iteration per step).
4. **§3 animations** (add swap-transition classes to wizard step cards).
5. **BUG D**, **BUG B** (robustness / cosmetic).
6. Re-run the full agent-browser pass (all 9 modals) + the Playwright e2e suite as the parity gate.
## 6. What was verified vs. inferred
- **Verified in browser + REPL:** BUG A (500 + exact exception + fix direction), BUG C (500 +
exact exception + vector repro + line 661 cause), §2 CSS root cause (missing classes + rebuild
fixes Vendor 1257→760 and Client→820), BUG D (500 on empty selection), Vendor/Client inner
overflow (screenshots 16/17), and that Bulk Code / Sales Summary / Transaction Rule / Invoice
Pay / Transaction Edit-open all render without errors.
- **Inferred (static analysis, not yet visually A/B'd against master):** §3 animation regression
(step cards lack `htmx-swapping:`/`htmx-added:` classes) and BUG B being pre-existing.