management: the four dashboard rejection-rendering branches can all be deleted with the suite green #82

New issue

Closed

opened 2026-09-02 17:09:05 +00:00 by yuris · 1 comment

yuris commented

2026-09-02 17:09:05 +00:00

Owner

Found by Reviewer78r1 across five review rounds of PR #78, and deliberately not closed there because it needs test infrastructure that does not exist in this repo yet.

The gap

PR #78 made server actions return service rejections as { ok: false, message } instead of throwing, because Next redacts an uncaught server-action error in a production build. Four UI consumers read that result and render the message:

app/(dashboard)/stok/adjustment-modal.tsx
app/(dashboard)/pembelian/add-purchase-modal.tsx
app/(dashboard)/pembelian/purchase-table.tsx (two call sites — the edit and the receipt-URL change)
app/(dashboard)/order/add-order-modal.tsx

All four can have their if (!result.ok) branch deleted with the whole suite green. So the last link in the delivery chain — service rejects, runAction converts, action returns, modal shows the operator — has no automated defence, and dropping it reproduces exactly the silent failure #69 was filed about: the modal closes, the row is unchanged, and nothing is said.

Why it was not closed in #78

apps/management/vitest.config.ts sets environment: "node", and there is no jsdom, happy-dom, or @testing-library/* anywhere in package.json. Asserting that a modal renders result.message means introducing a DOM test environment and a component-testing library — a real, separate piece of scope, and the wrong thing to bolt onto a bounds-and-locking PR.

The evidence these links work today is manual: the reviewer drove all four in real production builds (next build + next start, authenticated) across rounds 2–5 and confirmed the toast text, the modal staying open, and the row unchanged. That is good evidence and it is not repeatable in CI.

What would close it

Add a DOM environment (happy-dom is the lighter of the two) and @testing-library/react.
One test per consumer: stub the action to return { ok: false, message }, render, assert the message is shown and the dialog did not close.
A negative control per test — stub { ok: true } and assert it closes — so the tests cannot pass on a component that renders the error text unconditionally.

Note for whoever picks this up: the mistake #78 made four times was letting a unit test stand in for wiring. Testing that a toast helper formats a string would repeat it. The assertion has to be that the component calls it on ok: false.

#78 established the mechanism and closed the equivalent gap at every other layer: service (service-wiring.test.ts), runAction (validators.test.ts), the actions (action-wiring.test.ts), the REST route (purchases.test.ts), the bot nodes (node-wiring.test.ts), and the MCP tool (orders-wiring.test.ts). This is the one layer left.

Found by `Reviewer78r1` across five review rounds of PR #78, and deliberately not closed there because it needs test infrastructure that does not exist in this repo yet. ## The gap PR #78 made server actions **return** service rejections as `{ ok: false, message }` instead of throwing, because Next redacts an uncaught server-action error in a production build. Four UI consumers read that result and render the message: - `app/(dashboard)/stok/adjustment-modal.tsx` - `app/(dashboard)/pembelian/add-purchase-modal.tsx` - `app/(dashboard)/pembelian/purchase-table.tsx` (two call sites — the edit and the receipt-URL change) - `app/(dashboard)/order/add-order-modal.tsx` **All four can have their `if (!result.ok)` branch deleted with the whole suite green.** So the last link in the delivery chain — service rejects, `runAction` converts, action returns, modal shows the operator — has no automated defence, and dropping it reproduces exactly the silent failure #69 was filed about: the modal closes, the row is unchanged, and nothing is said. ## Why it was not closed in #78 `apps/management/vitest.config.ts` sets `environment: "node"`, and there is no `jsdom`, `happy-dom`, or `@testing-library/*` anywhere in `package.json`. Asserting that a modal renders `result.message` means introducing a DOM test environment and a component-testing library — a real, separate piece of scope, and the wrong thing to bolt onto a bounds-and-locking PR. The evidence these links work today is manual: the reviewer drove all four in real production builds (`next build` + `next start`, authenticated) across rounds 2–5 and confirmed the toast text, the modal staying open, and the row unchanged. That is good evidence and it is not repeatable in CI. ## What would close it 1. Add a DOM environment (`happy-dom` is the lighter of the two) and `@testing-library/react`. 2. One test per consumer: stub the action to return `{ ok: false, message }`, render, assert the message is shown and the dialog did not close. 3. A negative control per test — stub `{ ok: true }` and assert it closes — so the tests cannot pass on a component that renders the error text unconditionally. Note for whoever picks this up: the mistake #78 made four times was letting a unit test stand in for wiring. Testing that a toast helper formats a string would repeat it. The assertion has to be that the **component calls it** on `ok: false`. ## Related - #78 established the mechanism and closed the equivalent gap at every other layer: service (`service-wiring.test.ts`), `runAction` (`validators.test.ts`), the actions (`action-wiring.test.ts`), the REST route (`purchases.test.ts`), the bot nodes (`node-wiring.test.ts`), and the MCP tool (`orders-wiring.test.ts`). This is the one layer left.

yuris referenced this issue from a commit

2026-09-02 17:21:44 +00:00

docs(management): narrow the one overclaiming lock clause, record the deploy invariant

yuris commented

2026-09-28 20:34:51 +00:00

Author

Owner

Closed as stale / YAGNI. The rejection delivery paths were verified in real production builds across rounds 2–5 of PR #78 (toast text renders, dialogs stay open, rows unchanged). Introducing @testing-library/react and happy-dom to apps/management solely to test UI wiring echoes is unnecessary test scaffolding.

Closed as stale / YAGNI. The rejection delivery paths were verified in real production builds across rounds 2–5 of PR #78 (toast text renders, dialogs stay open, rows unchanged). Introducing `@testing-library/react` and `happy-dom` to `apps/management` solely to test UI wiring echoes is unnecessary test scaffolding.

yuris closed this issue

2026-09-28 20:34:51 +00:00

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

yuris/garage#82

No description provided.

Rows
Columns