From c443fc606af25b71a5526aea14a18464eaf5923b Mon Sep 17 00:00:00 2001 From: hsiegeln <37154749+hsiegeln@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:48:33 +0200 Subject: [PATCH] fix(alerts/ui): bell position, content tabs hidden, filters, novice labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaced during second smoke: 1. Notification bell moved — was first child of TopBar (left of breadcrumb); now rendered inside the `environment` slot so it sits between the env selector and the user menu, matching user expectations. 2. Content tabs (Exchanges/Dashboard/Runtime/Deployments) hidden on `/alerts/*` — the operational tabs don't apply there. 3. Inbox / All alerts filters now actually filter. `AlertController.list` accepts only `limit` — `state`/`severity` query params are dropped server-side. Move `useAlerts` to fetch once per env (limit 200) and apply filters client-side via react-query `select`, with a stable queryKey so filter toggles are instant and don't re-request. True server-side filter needs a backend change (follow-up). 4. Novice-friendly labels: - Inbox subtitle: "99 firing · 100 total" → "99 need attention · 100 total in inbox" - All alerts filter: Open/Firing/Acked/All → "Currently open"/"Firing now"/"Acknowledged"/"All states" - All alerts subtitle: "N shown" → "N matching your filter" - History subtitle: "N resolved" → "N resolved alert(s) in range" - Rules subtitle: "N total" → "N rule(s) configured" - Silences subtitle: "N active" → "N active silence(s)" or "Nothing silenced right now" - Column headers: "State" → "Status", rules "Kind" → "Type", rules "Targets" → "Notifies" - Buttons: "Ack" → "Acknowledge", silence "End" → "End early" Updated alerts.test.tsx and e2e selector to match new behavior/labels. Co-Authored-By: Claude Opus 4.7 (1M context) --- ui/src/api/queries/alerts.test.tsx | 28 ++++++++++++++++----- ui/src/api/queries/alerts.ts | 36 +++++++++++++++++++++------ ui/src/components/LayoutShell.tsx | 16 ++++++------ ui/src/pages/Alerts/AllAlertsPage.tsx | 12 ++++----- ui/src/pages/Alerts/HistoryPage.tsx | 8 ++++-- ui/src/pages/Alerts/InboxPage.tsx | 8 +++--- ui/src/pages/Alerts/RulesListPage.tsx | 8 +++--- ui/src/pages/Alerts/SilencesPage.tsx | 8 ++++-- ui/src/test/e2e/alerting.spec.ts | 2 +- 9 files changed, 87 insertions(+), 39 deletions(-) diff --git a/ui/src/api/queries/alerts.test.tsx b/ui/src/api/queries/alerts.test.tsx index b8eb3e66..0da5266c 100644 --- a/ui/src/api/queries/alerts.test.tsx +++ b/ui/src/api/queries/alerts.test.tsx @@ -22,7 +22,10 @@ describe('useAlerts', () => { useEnvironmentStore.setState({ environment: 'dev' }); }); - it('fetches alerts for selected env and passes filter query params', async () => { + it('fetches up to 200 alerts for selected env (no server-side filter params)', async () => { + // Backend AlertController.list accepts only `limit`; state/severity are + // dropped server-side. We therefore fetch once per env and filter + // client-side via react-query `select`. (apiClient.GET as any).mockResolvedValue({ data: [], error: null }); const { result } = renderHook( () => useAlerts({ state: 'FIRING', severity: ['CRITICAL', 'WARNING'] }), @@ -34,16 +37,29 @@ describe('useAlerts', () => { expect.objectContaining({ params: expect.objectContaining({ path: { envSlug: 'dev' }, - query: expect.objectContaining({ - state: ['FIRING'], - severity: ['CRITICAL', 'WARNING'], - limit: 100, - }), + query: { limit: 200 }, }), }), ); }); + it('applies state + severity filters client-side via select', async () => { + const dataset = [ + { id: '1', state: 'FIRING', severity: 'CRITICAL', title: 'a' }, + { id: '2', state: 'FIRING', severity: 'WARNING', title: 'b' }, + { id: '3', state: 'ACKNOWLEDGED', severity: 'CRITICAL', title: 'c' }, + { id: '4', state: 'RESOLVED', severity: 'INFO', title: 'd' }, + ]; + (apiClient.GET as any).mockResolvedValue({ data: dataset, error: null }); + const { result } = renderHook( + () => useAlerts({ state: ['FIRING'], severity: ['CRITICAL', 'WARNING'] }), + { wrapper }, + ); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + const ids = (result.current.data ?? []).map((a: any) => a.id); + expect(ids).toEqual(['1', '2']); + }); + it('does not fetch when no env is selected', () => { useEnvironmentStore.setState({ environment: undefined }); const { result } = renderHook(() => useAlerts(), { wrapper }); diff --git a/ui/src/api/queries/alerts.ts b/ui/src/api/queries/alerts.ts index 63b167e7..a13c5a9a 100644 --- a/ui/src/api/queries/alerts.ts +++ b/ui/src/api/queries/alerts.ts @@ -28,11 +28,28 @@ function toArray(v: T | T[] | undefined): T[] | undefined { // openapi-fetch regardless of what the TS types say; we therefore cast the // call options to `any` to bypass the generated type oddity. -/** List alert instances in the current env. Polls every 30s (pauses in background). */ +/** List alert instances in the current env. Polls every 30s (pauses in background). + * + * The backend's `AlertController.list` accepts only `limit` — `state` / + * `severity` query params are dropped. We fetch up to 200 alerts once per + * env (cached under a stable key) and apply filters client-side via + * react-query's `select` so filter switches on the All / History / Inbox + * pages are instant and don't each fire their own request. True server-side + * filtering needs a backend change (follow-up). + */ export function useAlerts(filter: AlertsFilter = {}) { const env = useSelectedEnv(); + const fetchLimit = 200; + const stateSet = filter.state === undefined + ? undefined + : new Set(toArray(filter.state)); + const severitySet = filter.severity === undefined + ? undefined + : new Set(toArray(filter.severity)); + const applyLimit = filter.limit; + const ruleIdFilter = filter.ruleId; return useQuery({ - queryKey: ['alerts', env, filter], + queryKey: ['alerts', env, 'list', fetchLimit], enabled: !!env, refetchInterval: 30_000, refetchIntervalInBackground: false, @@ -43,18 +60,21 @@ export function useAlerts(filter: AlertsFilter = {}) { { params: { path: { envSlug: env }, - query: { - state: toArray(filter.state), - severity: toArray(filter.severity), - ruleId: filter.ruleId, - limit: filter.limit ?? 100, - }, + query: { limit: fetchLimit }, }, } as any, ); if (error) throw error; return data as AlertDto[]; }, + select: (all) => { + let out = all; + if (stateSet) out = out.filter((a) => a.state && stateSet.has(a.state)); + if (severitySet) out = out.filter((a) => a.severity && severitySet.has(a.severity)); + if (ruleIdFilter) out = out.filter((a) => a.ruleId === ruleIdFilter); + if (applyLimit !== undefined) out = out.slice(0, applyLimit); + return out; + }, }); } diff --git a/ui/src/components/LayoutShell.tsx b/ui/src/components/LayoutShell.tsx index 5f017d21..4ae0be9f 100644 --- a/ui/src/components/LayoutShell.tsx +++ b/ui/src/components/LayoutShell.tsx @@ -957,18 +957,20 @@ function LayoutContent() { + <> + + + } user={username ? { name: username } : undefined} userMenuItems={userMenuItems} onLogout={handleLogout} onNavigate={navigate} > - setPaletteOpen(true)} /> - {!isAdminPage && ( + {!isAdminPage && !isAlertsPage && ( )} diff --git a/ui/src/pages/Alerts/AllAlertsPage.tsx b/ui/src/pages/Alerts/AllAlertsPage.tsx index f607e1b1..7de47066 100644 --- a/ui/src/pages/Alerts/AllAlertsPage.tsx +++ b/ui/src/pages/Alerts/AllAlertsPage.tsx @@ -21,10 +21,10 @@ import tableStyles from '../../styles/table-section.module.css'; type AlertState = NonNullable; const STATE_FILTERS: Record = { - open: { label: 'Open', values: ['PENDING', 'FIRING', 'ACKNOWLEDGED'] }, - firing: { label: 'Firing', values: ['FIRING'] }, - acked: { label: 'Acked', values: ['ACKNOWLEDGED'] }, - all: { label: 'All', values: ['PENDING', 'FIRING', 'ACKNOWLEDGED', 'RESOLVED'] }, + open: { label: 'Currently open', values: ['PENDING', 'FIRING', 'ACKNOWLEDGED'] }, + firing: { label: 'Firing now', values: ['FIRING'] }, + acked: { label: 'Acknowledged', values: ['ACKNOWLEDGED'] }, + all: { label: 'All states', values: ['PENDING', 'FIRING', 'ACKNOWLEDGED', 'RESOLVED'] }, }; export default function AllAlertsPage() { @@ -41,7 +41,7 @@ export default function AllAlertsPage() { render: (_, row) => row.severity ? : null, }, { - key: 'state', header: 'State', width: '140px', + key: 'state', header: 'Status', width: '140px', render: (_, row) => row.state ? : null, }, { @@ -74,7 +74,7 @@ export default function AllAlertsPage() {

All alerts

- {rows.length} shown + {rows.length} matching your filter
-

History

- {filtered.length} resolved +

Alert history

+ + {filtered.length === 0 + ? 'No resolved alerts in range' + : `${filtered.length} resolved alert${filtered.length === 1 ? '' : 's'} in range`} +
diff --git a/ui/src/pages/Alerts/InboxPage.tsx b/ui/src/pages/Alerts/InboxPage.tsx index 4d55c0fa..2ce460a0 100644 --- a/ui/src/pages/Alerts/InboxPage.tsx +++ b/ui/src/pages/Alerts/InboxPage.tsx @@ -80,7 +80,7 @@ export default function InboxPage() { row.severity ? : null, }, { - key: 'state', header: 'State', width: '140px', + key: 'state', header: 'Status', width: '140px', render: (_, row) => row.state ? : null, }, @@ -108,11 +108,11 @@ export default function InboxPage() { ) : '—', }, { - key: 'ack', header: '', width: '70px', + key: 'ack', header: '', width: '120px', render: (_, row) => row.state === 'FIRING' ? ( ) : null, }, @@ -126,7 +126,7 @@ export default function InboxPage() { const subtitle = selectedIds.length > 0 ? `${selectedIds.length} selected` - : `${unreadIds.length} firing · ${rows.length} total`; + : `${unreadIds.length} need attention · ${rows.length} total in inbox`; return (
diff --git a/ui/src/pages/Alerts/RulesListPage.tsx b/ui/src/pages/Alerts/RulesListPage.tsx index 2f502ae6..890a087a 100644 --- a/ui/src/pages/Alerts/RulesListPage.tsx +++ b/ui/src/pages/Alerts/RulesListPage.tsx @@ -67,7 +67,7 @@ export default function RulesListPage() { render: (_, r) => {r.name}, }, { - key: 'conditionKind', header: 'Kind', width: '160px', + key: 'conditionKind', header: 'Type', width: '160px', render: (_, r) => , }, { @@ -85,7 +85,7 @@ export default function RulesListPage() { ), }, { - key: 'targets', header: 'Targets', width: '90px', + key: 'targets', header: 'Notifies', width: '90px', render: (_, r) => String(r.targets?.length ?? 0), }, { @@ -114,7 +114,9 @@ export default function RulesListPage() {

Alert rules

- {rows.length} total + + {rows.length === 0 ? 'No rules yet' : `${rows.length} rule${rows.length === 1 ? '' : 's'} configured`} +
diff --git a/ui/src/pages/Alerts/SilencesPage.tsx b/ui/src/pages/Alerts/SilencesPage.tsx index 1294031f..52665235 100644 --- a/ui/src/pages/Alerts/SilencesPage.tsx +++ b/ui/src/pages/Alerts/SilencesPage.tsx @@ -84,7 +84,7 @@ export default function SilencesPage() { key: 'actions', header: '', width: '90px', render: (_, s) => ( ), }, @@ -95,7 +95,11 @@ export default function SilencesPage() {

Alert silences

- {rows.length} active + + {rows.length === 0 + ? 'Nothing silenced right now' + : `${rows.length} active silence${rows.length === 1 ? '' : 's'}`} +
diff --git a/ui/src/test/e2e/alerting.spec.ts b/ui/src/test/e2e/alerting.spec.ts index 90aaa1a8..396cc884 100644 --- a/ui/src/test/e2e/alerting.spec.ts +++ b/ui/src/test/e2e/alerting.spec.ts @@ -101,7 +101,7 @@ test.describe('alerting UI smoke', () => { await page .getByRole('row', { name: new RegExp(unique) }) - .getByRole('button', { name: /^end$/i }) + .getByRole('button', { name: /^end early$/i }) .click(); const confirmEnd = page.getByRole('dialog'); await expect(confirmEnd.getByText(/end silence/i)).toBeVisible();