[AAASM-5146] ✨ (dashboard/alerts): Add free-text search to the filter bar - #1779
Conversation
The mock's primary find affordance (design/v1/hi-fi/alerts.jsx:180-184) matches free text over message + agent_id + id; the shipped Agent field only ever matched agentId + ruleName. Add the q predicate alongside it so an alert can be found by its rule name or its own id, not just its agent.
Mirrors design/v1/hi-fi/alerts.jsx:180-184's primary find affordance, placed ahead of the Agent field per the mock's filter-bar ordering.
Confirms the search box renders with the mock's placeholder text and that typing narrows the alert list by rule name, agent id, and alert id — verified against a stubbed backend since aa-gateway is out of scope for local frontend dev.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Chisanan232
left a comment
Chisanan232
left a comment
There was a problem hiding this comment.
✅ LGTM — reviewed and approved by Claude Code
(COMMENT review because GitHub blocks self-approval; this is the sign-off of record.)
Requirement correctness — AAASM-5146
Adds free-text search to the Alerts filter bar per design/v1/hi-fi/alerts.jsx:180-184. Verified: q added to AlertFilters + DEFAULT_ALERT_FILTERS (+ the ALL_ALERTS literal in OverviewPage, avoiding a tsc break); URL round-trip via ?q=; the search input uses the mock's exact placeholder "search message or agent…"; applyClientFilters matches q over ruleName + agentId + id case-insensitively, kept as a separate predicate from the existing agentQuery (no behavior change to the latter). Tests cover all three match fields (it.each), the input emitting q changes, and the URL round-trip. Correctly client-side only (the mock's primary find affordance), no server param invented.
CI — all checks green (CI Success pass).
Security — client-side filter over already-fetched, already-authorized alert data; no new request, no injection surface.
Merging as org admin with a merge commit.
🤖 Reviewed with Claude Code
1 participant
Description
Design-QA re-audit finding (AAASM-5146): the hi-fi mock (
design/v1/hi-fi/alerts.jsx:180-184) ships a "search message or agent…" free-text input as the Alerts page's primary find affordance, matching overmessage + agent_id + id. The shippedAlertFilterBaronly had an Agent field (bound to the server-ignoredagentparam), so there was no way to find an alert by its rule name or its own id.Adds a
qfield toAlertFilters/ the URL filter state, a new Search input in the filter bar (placed ahead of the existing Agent field, matching the mock's ordering and placeholder text), and a client-side predicate that matches`${ruleName} ${agentId ?? ''} ${id}`.toLowerCase()— mirroring the mock'shaystackconstruction but with the app's actualAlertfield names (ruleName/idin place of the mock'smessage/id, since the shippedAlerttype has no free-textmessagefield).Type of Change
Breaking Changes
Does this PR introduce any breaking changes to public APIs or behaviour?
Related Issues
Testing
alertFilters.test.ts(q matches rule name / agent id / alert id, case-insensitively),AlertFilterBar.test.tsx(search input emitsqchanges),urlFilters.test.ts(round-trip now exercisesq)pnpm dev+ Playwright against a stubbed/api/v1/alertsbackend (no aa-gateway available locally); screenshots indashboard/verify/5146/:01-search-field-visible.png— Search field rendered with the mock's placeholder ("search message or agent…"), positioned before the Agent field, 3 unfiltered rows02-filtered-by-rule-name.png— typing "anomaly" narrows to the "Anomaly spike" row03-filtered-by-agent-id.png— typing "aa-001" narrows to that agent's row04-filtered-by-alert-id.png— typing "alert-003" narrows to that alert's rowLocal checks (Node v22, pnpm, in
dashboard/):pnpm exec tsc --noEmitclean,pnpm exec eslint .clean,pnpm exec vitest run— 275 files / 3063 tests passed.Checklist
cargo fmt,cargo clippy) — N/A, TypeScript change; ESLint/Prettier equivalent is cleanSecurity note
Per ticket scope: this is a client-side filter over an already-fetched page (same pattern as the existing
agentQuerypredicate inalertFilters.ts) — no new backend call, no new injection surface. No other security-relevant findings during this change.