feat/select-all
needs attentionviewing older commitaf91420 · incrementalPR #281reviewed 2026-07-08 23:28 UTC0H · 8M · 10L · 2I- Purpose
- Gmail-style bulk selection for Contratos table: select-all-matching lets batch actions (Consultar CFE, CSV, ZIP) target the entire filtered set, not just the visible 25-row page.
- Goal
- Persistent batch-result panel with grouped per-RPU errors, narrow-to-failed selection, one-click retry — and fixes for pre-existing bugs surfaced by live validation.
- Sub-goals
- SG-1: New GET /utility-contracts/keys endpoint with atomic count(*) OVER () + shared CFE_BATCH_MAX_RECORDS cap
- SG-2: Client materializes key set; select-all banner; cross-page selection survives paging
- SG-3: Persistent batch-result panel: invalid vs partial outcome kinds, handleRetry targets only failed RPUs
- SG-4: ZIP cap (25 RPU limit) + honest popup-blocker detection
- SG-5: Epoch staleness guard for select-all-matching keys fetch
- What
- Incremental fix commit (af91420f): dispatchBatch/handleRetry refactor, invalid BatchOutcome kind, ZIP_BATCH_CAP, popup-blocker detection, status-code error classification, epoch staleness guard, CFE_BATCH_MAX_RECORDS constant, single count(*) OVER () query, toOverviewViewState helper, 3-shape validation rejection handler.
- Why
- Consolidated fixes from prior Loop C review + live validation on 50-contract preview dataset.
- Areas
- apps/platform+1158−115docs/development+120−0domains/utility+151−1packages/analytics+11−0packages/api+101−2
- Blast
- Full branch: 18 files +1541/−118 across apps/platform, domains/utility, packages/api, packages/analytics. Incremental: 11 files +416/−141.
Findings · 20
correctness4
handleRetry fallback re-dispatches full selection when failed-RPU subset is empty
apps/platform/src/app/[locale]/(dashboard)/bills/_components/BulkActionBar.tsx:283
When outcome.failures RPUs don't appear in records, the fallback dispatchBatch(records) re-dispatches the full selection including already-succeeded jobs. Fix: no-op when subset.length === 0.
ZIP loop invokes Lambda for all capped RPUs even when browser blocks popups
apps/platform/src/app/[locale]/(dashboard)/bills/_components/BulkActionBar.tsx:340
Intentional and documented; cap bounds blast radius at 25.
viewJson double-encoding correct but type signature allows easy misuse
apps/platform/src/app/[locale]/(dashboard)/bills/contratos/_lib/contract-keys-client.ts:19
No structural constraint on the string param; future caller passing raw object silently breaks filters.
listOverviewKeys no ORDER BY — intentional and documented
domains/utility/src/utility-contract-overview/utility-contract-overview.queries.ts
Selection is a set; truncation guard makes order irrelevant.
security2
Unvalidated server-supplied URL passed to window.open via as any cast
apps/platform/src/app/[locale]/(dashboard)/bills/_components/BulkActionBar.tsx:342
No URL scheme check before window.open. Add startsWith('https://') guard.
JSON.parse on bodyErrors.message is safe as implemented
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/cfe-job-client.ts
Gated by Array.isArray + path checks before use in React JSX text nodes.
conventions3
Partial i18n migration — pre-existing hardcoded strings remain un-migrated
apps/platform/src/app/[locale]/(dashboard)/bills/_components/BulkActionBar.tsx:405
PR wired useTranslations for new copy but left CSV/ZIP toast titles and button labels as hardcoded Spanish strings.
Dynamic i18n key loses next-intl static type checking
apps/platform/src/app/[locale]/(dashboard)/bills/_components/BulkActionBar.tsx:522
t(`errorLabels.${g.code}`) — use a typed record lookup instead.
listContractKeysHandler uses args: { query: any } — bypasses ts-rest narrowing
apps/platform/src/api/handlers/utility-contracts.handler.ts:524
Pre-existing but new handler was an opportunity to fix.
tests5
ZIP_BATCH_CAP guard path untested
apps/platform/src/app/[locale]/(dashboard)/bills/_components/__tests__/BulkActionBar.test.tsx
No test for the cap warning toast path.
Popup-blocker detection (window.open → null) untested
apps/platform/src/app/[locale]/(dashboard)/bills/_components/__tests__/BulkActionBar.test.tsx
Required by product-ux P5; no test mocks window.open returning null.
cfe-job-client bodyErrors.issues shape not directly unit-tested
apps/platform/src/app/[locale]/(dashboard)/bills/_lib/cfe-job-client.ts
No cfe-job-client.test.ts; bodyErrors.issues branch has no coverage.
Epoch staleness guard untested
apps/platform/src/app/[locale]/(dashboard)/bills/contratos/page.tsx
No test verifies stale keys response is discarded on epoch change.
pageFullySelected !isFetching gate has no test
apps/platform/src/app/[locale]/(dashboard)/bills/contratos/page.tsx
No test asserts pageFullySelected=false during page-flip transition.
improvement6
handleRefresh is a redundant one-liner wrapper
apps/platform/src/app/[locale]/(dashboard)/bills/_components/BulkActionBar.tsx:270
Does exactly what the requestFailed branch of handleRetry does. Unify.
CONTRACT_KEYS_LIMIT local alias is needless indirection
apps/platform/src/api/handlers/utility-contracts.handler.ts:513
Import says 'never restate the literal'. Use CFE_BATCH_MAX_RECORDS directly.
selectedMeta staleness check scans rpus twice per realtime patch
apps/platform/src/app/[locale]/(dashboard)/bills/contratos/page.tsx:496
Combine pre-scan + update loop into a single pass.
isRefreshing state mirrors isFetching through sync effect
apps/platform/src/app/[locale]/(dashboard)/bills/contratos/page.tsx:99
Derived state anti-pattern; use isFetching directly.
displayedRPUs is an identity alias
apps/platform/src/app/[locale]/(dashboard)/bills/contratos/page.tsx:451
const displayedRPUs = rpus — no transformation. Pass rpus directly.
isSearchOpen and searchInputRef declared but never used
apps/platform/src/app/[locale]/(dashboard)/bills/contratos/page.tsx:100
Dead state; remove both declarations.