Skip to content

fix(platform): show legal hold picker read failures - #4302

Merged
yannickmonney merged 1 commit into
mainfrom
fix/legal-hold-picker-read-errors
Oct 4, 2026
Merged

yannickmonney merged 1 commit into
mainfrom
fix/legal-hold-picker-read-errors

Conversation

@yannickmonney

@yannickmonney yannickmonney commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

What changed

  • Treat failed member and open-matter reads as unavailable, not successful empty lists. Disable only the affected picker, close a picker whose read fails while open, and show a localized error with its own retry action.
  • Reuse readStateOf so 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.
  • Show initial loading feedback; preserve successful empty states, unaffected selections, organization-wide holds, and custodian holds without an optional matter.
  • Add EN/DE/FR error messages and a focused regression suite. No files or sections from PR fix(platform): surface legal-hold list read failures #4276 are changed.

Verification

  • Negative control on origin/main: the initial regression suite fails (4 failed, 2 passed).
  • Installed Node, jsdom, one worker: place-hold picker regression and existing active-holds dialog suite pass (16 tests), including EN/DE/FR, retry isolation, repeated failure, pending-state focus, recovery focus, initial loading, stale data, an open picker failing, unaffected submissions, and axe accessibility.
  • Platform message tests and error-description guard pass (48 tests). Single-failure-toast guard passes separately (51 tests); its first run under shared-host contention hit its 30-second timeout, not a failed assertion; isolated run completes in 12 seconds with a local 120-second ceiling.
  • Targeted oxlint, oxfmt check, TypeScript semantic/syntactic diagnostics for the changed TSX files plus their direct importer, and git diff --check pass. Type diagnostics include the existing UI-test setup; no whole-workspace typecheck or platform suite runs locally.
  • Pre-commit formatting and conflict-marker hooks pass. The local SAST hook exits successfully but skips the scan because its pinned binary is not cached; SAST execution remains a CI gate, not claimed as a local scan.
  • Browser/visual review is not claimed: this dispatch is light/jsdom-only with no native slot, and there is no running platform URL. CI and independent review retain the broader gates. No CI rerun or cancellation requested.

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

@yannickmonney yannickmonney left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.tsx importer 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.

@yannickmonney
yannickmonney merged commit 33900e4 into main Oct 4, 2026
63 checks passed
@yannickmonney
yannickmonney deleted the fix/legal-hold-picker-read-errors branch October 4, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant