fix(ui): four defects surfaced by the UI coverage work

[chriskehayias]

Covering the app routes and presentational components (#81) surfaced four real defects. Each was landed there as a test pinning the broken behaviour, so that fixing it would show up as a failing test. This is that follow-up — all four pinning tests are replaced by tests asserting the fixed behaviour.

1. formatDateTime() could blank the whole page

contact-logs.tsx threw RangeError: Invalid time value for any Contact_Date its regex missed and new Date() couldn't parse ("", " ", "not-a-date"). It's called unguarded during row render, and this app has no error boundary anywhere — no error.tsx, no global-error.tsx, no ErrorBoundary in src/. So the throw escaped ContactLogs entirely and hit Next's default global error screen: one bad datetime took out the page, not one row.

Guarded at both entry and the new Date() fallback, returning an em dash.

The DTO was deliberately not widened to string | null — MP's generated model has Contact_Date: string / z.string().datetime(), a NOT NULL column, so this is defence-in-depth at the formatting boundary, not a type correction. The file had been contradicting itself: handleEditClick already guarded for a falsy Contact_Date, and that guard was unreachable precisely because formatDateTime threw first. It's now reachable and covered.

The regression guard worth keeping: a list where one row's date is bad asserts the other rows still render. Reverting the fix fails it loudly.

2. contact-lookup-search never cleared stale results

The empty-query early return in handleSearch was dead code — performSearch applied the same guard first and passed an already-trimmed term. After a search, clearing the box and pressing Enter left the previous results and count on screen, stale and misleading. performSearch now passes the empty term through so the existing guard fires.

Note the button path's safety now rests entirely on the disabled attribute, since performSearch no longer guards — there's a test pinning that. This also takes the file from 92.3% to 100% statements; the dead code was the only reason it sat below 100.

3. A failed sign-out was silent

handleItemClick had no try/catch, so a rejected handleSignOut escaped as an unhandled rejection — no alert, no retry — on the app's only sign-out path. onClose() fires first, so the menu was already shut when it failed.

The fix has a live trap, verified rather than assumed. handleSignOut ends in redirect(), and in Next 16 the server-action reducer explicitly rejects the action promise with the NEXT_REDIRECT error:

// If the action triggered a redirect, the action promise will be rejected with a redirect so that it's handled by RedirectBoundary
— router-reducer/reducers/server-action-reducer.js

So a plain try/catch alerts Error: NEXT_REDIRECT on every successful sign-out — confirmed by deleting the guard and watching the test fail with exactly that string. unstable_rethrow(err) must stay the first statement in the catch.

Tests assert both that a successful sign-out is silent and that the signal is re-thrown rather than reported. next/navigation is deliberately left unmocked, so the real Next signal recognition is what's under test rather than a mock of it.

4. Breadcrumbs rendered raw GUIDs

dynamic-breadcrumb.tsx had no label mapping — every label was upper-case-first plus hyphens-to-spaces — so /contactlookup/<guid> read Contactlookup / Ab12cd34 ef56 7890 abcd ef1234567890. Added a known-segment label map (derived from the actual folders under src/app/, not invented) and an anchored GUID pattern rendering Details.

Two deliberate choices:

• The map is a Map, not an object literal, because the key is a raw URL segment and /constructor against a plain object would return an inherited Object.prototype member as the label.
• The GUID pattern is not restricted to the RFC-4122 v4 form, because MP GUIDs aren't guaranteed to be v4 and a stricter pattern would fail open on a legitimate id.

A GUID renders as the generic Details rather than the contact's name because this component is mounted by the layout, which has no access to page data — resolving the name needs a context provider, not a better regex. That limit is commented in place so it reads as a considered boundary, not an oversight.

Coverage

Metric	Before	After
Statements	99.47%	99.73% (1144/1147)
Branches	96.65%	97.18% (587/604)
Functions	98.92%	99.29% (282/284)
Lines	99.73%	99.91% (1111/1112)

897 → 996 tests. All existing thresholds pass; npx tsc --noEmit and npx eslint . are clean.

No test reaches Ministry Platform: every component test mocks its co-located ./actions module wholesale.

Known follow-up, not included

Emptying the search box and pressing Enter now sets hasSearched: true, so the panel reads "No contacts found" rather than resetting to the pre-search state. Arguably a cleared box should reset entirely (setHasSearched(false)) — that's a container change and a UX decision, so it's left out deliberately.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!