Repository navigation
fix(platform): show legal hold picker read failures - #4302
Conversation
yannickmonney
left a comment
There was a problem hiding this comment.
TALE-660 independent review: ACCEPT
PR: #4302
Exact reviewed head: fff89ccf63a301dc7d1e992e1e1c21a10388a569, unchanged on final readback. Base/main: 9e8f87f6832cf352eb70a0ad666bbec6fd9d1a57.
Reviewer: Codex #13 7155f7ae-b6f7-427b-9541-2edaf36abfdb, run 26569832-6763-4fb2-b930-6c45a5afe415; distinct from author Codex #14 c0c99cd0, run 6e3a18e1. This is an independent source/jsdom acceptance receipt, not native/protected approval or merge authorization.
Findings and contract
No blocking finding in this PR. Both pickers consume settled error state and retained first-read failure state through readStateOf; failed/loading pickers cannot open an ordinary empty choice list. Each failed read has localized visible failure copy and its own Retry action. Retry remains focusable and busy during first-read retry, blocks repeated keyboard activation, retains focus through another failure, and restores it to the recovered picker only when focus was still in that notice. Unaffected choices and organization-wide/custodian-without-matter submission remain usable.
EN/DE/FR add both failure messages and reuse existing translated Try again/Loading labels. Error details use failureDetail, not serialized error payloads. The new Button is explicitly type="button".
The PR body closes #3834 and #3835. Their descriptions/reproduction/root cause are identical; closing both is appropriate for this single dialog-scoped fix. #4276 changes active holds, release requests/history and the manual coverage register; #4302 changes only PlaceHoldDialog, its new test and three locale catalogs. Modified files are disjoint. This verdict does not close or supersede any finding on #4276.
Independently observed evidence
Installed /opt/node/bin/node v24.21.0, Vitest 4.1.11, jsdom, one worker, 1536 MiB heap cap; no dependency install/update.
- Exact-head picker regression plus existing
legal-hold-dialog.test.tsximporter suite: 16/16 pass. Includes EN/DE/FR, isolated retry, pending/repeated-failure/recovery focus, loading, empty-success control, cached failure, open-picker failure, unaffected submissions and axe checks (contrast disabled in the existing jsdom helper). - Reviewer-added keyboard probe: Tab reaches Retry; Enter retries once; Enter while pending cannot retry again; Tab moves away; recovery does not steal that moved focus. Pass.
- Reviewer-added cached-selection probe: selected custodian and optional matter survive both read failures and recovery; subsequent submission retains both identifiers and reason. Pass. Reviewer probes: 2/2 pass, 12 original cases intentionally skipped in that invocation.
- Production-only main negative control: replace only PlaceHoldDialog with its exact main version, retaining the identical PR tests/catalogs/dependencies. 9 fail / 3 pass, exit 1. Failures include absent alerts/retry, enabled unavailable pickers, loading shown as usable, and an open failed picker retaining empty copy. Passing controls include successful-empty and unaffected submissions. This is not a claim to have run the entire main suite.
- Targeted non-type-aware oxlint on both changed TSX files and direct importer: exit 0. Oxfmt check: exit 0 (two TSX files matched; YAML was not checked by that formatter).
git diff --check: pass.
Unrun proof and handoff
Real browser/native focus behavior, screen-reader announcements, contrast/layout/visual-analyzer score, live HTTP/backend failure/retry, full-stack/E2E/manual flows, independent TypeScript/type-aware lint, locale/guard suites and full workspace gates were not run. Query hooks and mutation outcomes in these component tests are controlled mocks, not backend transport proof. Author-reported broader checks are not promoted to independently observed evidence.
Passive exact-head CI readback showed five pending Candidate source / Resolve source checks; none is counted as passed. No CI rerun/cancellation, push, merge, task movement or native review decision occurred. The shared checkout's unrelated automation edits were preserved.
Root's 08:15Z rule (4d56e1f7) applies: a blocking finding overrides ACCEPT until explicitly independently closed at the exact head with evidence. No existing reviews or review comments were present on #4302 at the final readback. Acceptance is scoped to this head and this evidence; TALE-359 retains exact-head CI and authorized merge disposition, and any new finding or head requires reconciliation.
Artifacts: this delivery box contains head/main/probe logs, reviewed patch, author/reviewer regression sources, reviewer probe patch, lint/format logs, execution notes and the posting receipt.
What changed
readStateOfso a first-read retry cannot remove the error notice when react-query returns to pending. Retry stays keyboard-focusable and busy, suppresses duplicate activation, and returns focus to the recovered picker.Verification
git diff --checkpass. Type diagnostics include the existing UI-test setup; no whole-workspace typecheck or platform suite runs locally.Duplicate finding
Issues #3834 (TALE-266) and #3835 (TALE-267) have identical descriptions, reproduction steps and root cause. This single dialog-scoped fix covers both; it does not change the legal-hold list tables.
Closes #3834
Closes #3835