From 5837358ef9b95d4cef532a3722889fa0bdc922e3 Mon Sep 17 00:00:00 2001 From: Joe Thel Date: Tue, 22 Sep 2026 15:10:51 -0700 Subject: [PATCH 1/6] Add an alert class filter to alerts tab This is a little fraught: alert list class filtering requires an exact match to a known schema, so we need to do a lot of client-side validation. On the bright side, we now have a lot of client-side validation! --- app/api/util.spec.ts | 60 ++-- app/api/util.ts | 6 +- app/components/SubscriptionMatchPreview.tsx | 14 +- .../form/fields/SubscriptionsField.tsx | 50 ++-- app/hooks/use-alert-classes.tsx | 49 ++++ .../system/alerting/AlertReceiverPage.tsx | 38 +-- app/pages/system/alerting/AlertsTab.tsx | 275 +++++++++++++++++- app/ui/lib/Combobox.tsx | 6 +- mock-api/msw/handlers.ts | 25 +- test/e2e/alerts.e2e.ts | 122 +++++++- 10 files changed, 536 insertions(+), 109 deletions(-) create mode 100644 app/hooks/use-alert-classes.tsx diff --git a/app/api/util.spec.ts b/app/api/util.spec.ts index 5eeb0081db..7845755426 100644 --- a/app/api/util.spec.ts +++ b/app/api/util.spec.ts @@ -19,56 +19,68 @@ import { describe('subscriptionRegex', () => { it('matches exact class names', () => { - expect(subscriptionRegex('instance.create').test('instance.create')).toBe(true) - expect(subscriptionRegex('instance.create').test('instance.created')).toBe(false) + expect(subscriptionRegex('instance.create')?.test('instance.create')).toBe(true) + expect(subscriptionRegex('instance.create')?.test('instance.created')).toBe(false) }) it('* matches exactly one segment', () => { const re = subscriptionRegex('disk.*') - expect(re.test('disk.create')).toBe(true) - expect(re.test('disk.snapshot.create')).toBe(false) - expect(re.test('disk')).toBe(false) + expect(re?.test('disk.create')).toBe(true) + expect(re?.test('disk.snapshot.create')).toBe(false) + expect(re?.test('disk')).toBe(false) }) it('* can appear at the head', () => { const re = subscriptionRegex('*.create') - expect(re.test('disk.create')).toBe(true) - expect(re.test('instance.create')).toBe(true) - expect(re.test('instance.ephemeral_ip.create')).toBe(false) + expect(re?.test('disk.create')).toBe(true) + expect(re?.test('instance.create')).toBe(true) + expect(re?.test('instance.ephemeral_ip.create')).toBe(false) }) it('* can appear mid-string', () => { const re = subscriptionRegex('hardware.*.insert') - expect(re.test('hardware.sled.insert')).toBe(true) - expect(re.test('hardware.power_shelf.psu.insert')).toBe(false) - expect(re.test('hardware.insert')).toBe(false) + expect(re?.test('hardware.sled.insert')).toBe(true) + expect(re?.test('hardware.power_shelf.psu.insert')).toBe(false) + expect(re?.test('hardware.insert')).toBe(false) }) it('** matches one or more segments', () => { const re = subscriptionRegex('hardware.**') - expect(re.test('hardware.power_shelf.psu.insert')).toBe(true) - expect(re.test('hardware.psu')).toBe(true) - expect(re.test('hardware')).toBe(false) + expect(re?.test('hardware.power_shelf.psu.insert')).toBe(true) + expect(re?.test('hardware.psu')).toBe(true) + expect(re?.test('hardware')).toBe(false) const suffix = subscriptionRegex('**.delete') - expect(suffix.test('project.delete')).toBe(true) - expect(suffix.test('instance.ephemeral_ip.delete')).toBe(true) - expect(suffix.test('delete')).toBe(false) + expect(suffix?.test('project.delete')).toBe(true) + expect(suffix?.test('instance.ephemeral_ip.delete')).toBe(true) + expect(suffix?.test('delete')).toBe(false) const infix = subscriptionRegex('hardware.**.insert') - expect(infix.test('hardware.sled.insert')).toBe(true) - expect(infix.test('hardware.power_shelf.psu.insert')).toBe(true) - expect(infix.test('hardware.insert')).toBe(false) + expect(infix?.test('hardware.sled.insert')).toBe(true) + expect(infix?.test('hardware.power_shelf.psu.insert')).toBe(true) + expect(infix?.test('hardware.insert')).toBe(false) }) it('does not match substrings within a segment', () => { - expect(subscriptionRegex('instance.**').test('silo.instance_quota.hit')).toBe(false) - expect(subscriptionRegex('disk.*').test('bigdisk.create')).toBe(false) + expect(subscriptionRegex('instance.**')?.test('silo.instance_quota.hit')).toBe(false) + expect(subscriptionRegex('disk.*')?.test('bigdisk.create')).toBe(false) }) it('does not match leading/trailing segments without a star pattern', () => { - expect(subscriptionRegex('instance.**').test('silo.instance.hit')).toBe(false) - expect(subscriptionRegex('**.power_shelf').test('hardware.power_shelf.psu')).toBe(false) + expect(subscriptionRegex('instance.**')?.test('silo.instance.hit')).toBe(false) + expect(subscriptionRegex('**.power_shelf')?.test('hardware.power_shelf.psu')).toBe( + false + ) + }) + + it('never throws on invalid input, including regex metacharacters', () => { + expect(subscriptionRegex('')).toBeNull() + expect(subscriptionRegex('a|b*')).toBeNull() + expect(subscriptionRegex('disk+')).toBeNull() + expect(subscriptionRegex('(*')).toBeNull() + expect(subscriptionRegex('*.{2,}')).toBeNull() + expect(subscriptionRegex('hardware.***')).toBeNull() + expect(subscriptionRegex('hardware..disk')).toBeNull() }) }) diff --git a/app/api/util.ts b/app/api/util.ts index 22f61843f3..4fa6b66f4a 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -71,10 +71,12 @@ export const isSubscribableClass = (c: { name: string }) => c.name !== PROBE_ALE /** * Convert an alert subscription to a regex matching the class names it covers: - * a `*` segment matches exactly one segment, `**` matches one or more. + * a `*` segment matches exactly one segment, `**` matches one or more. An + * invalid input returns `null`. * https://github.com/oxidecomputer/omicron/blob/32615a35/nexus/db-model/src/alert_subscription.rs */ -export function subscriptionRegex(subscription: string) { +export function subscriptionRegex(subscription: string): RegExp | null { + if (!ALERT_SUBSCRIPTION_REGEX.test(subscription)) return null const pattern = subscription .split('.') .map((seg) => (seg === '**' ? '.+' : seg === '*' ? '[^.]+' : seg)) diff --git a/app/components/SubscriptionMatchPreview.tsx b/app/components/SubscriptionMatchPreview.tsx index 78c6843d5a..4daa0b18c2 100644 --- a/app/components/SubscriptionMatchPreview.tsx +++ b/app/components/SubscriptionMatchPreview.tsx @@ -7,12 +7,7 @@ */ import type { AlertClassResultsPage } from '@oxide/api' -import { - ALERT_SUBSCRIPTION_REGEX, - isGlobPattern, - isSubscribableClass, - subscriptionRegex, -} from '~/api/util' +import { isGlobPattern, isSubscribableClass, subscriptionRegex } from '~/api/util' import { AlertClassBadge } from '~/components/AlertClassBadge' /** @@ -28,11 +23,8 @@ export function SubscriptionMatchPreview({ data: AlertClassResultsPage | undefined pattern: string }) { - // validate before subscriptionRegex, which assumes a well-formed subscription - const isValidGlob = isGlobPattern(pattern) && ALERT_SUBSCRIPTION_REGEX.test(pattern) - if (!isValidGlob || !data) return null - - const re = subscriptionRegex(pattern) + const re = isGlobPattern(pattern) ? subscriptionRegex(pattern) : null + if (!re || !data) return null // the probe class can't be subscribed to, so don't count it as a match const classes = data.items.filter(isSubscribableClass).filter((c) => re.test(c.name)) diff --git a/app/components/form/fields/SubscriptionsField.tsx b/app/components/form/fields/SubscriptionsField.tsx index 9b05f57815..afb291601c 100644 --- a/app/components/form/fields/SubscriptionsField.tsx +++ b/app/components/form/fields/SubscriptionsField.tsx @@ -14,7 +14,6 @@ import { useFloating, useMergeRefs, } from '@floating-ui/react' -import { useQuery } from '@tanstack/react-query' import cn from 'classnames' import { useCallback, useId, useRef, useState } from 'react' import { @@ -26,16 +25,15 @@ import { import * as R from 'remeda' import { match, P } from 'ts-pattern' -import { api, q } from '@oxide/api' import { Close8Icon } from '@oxide/design-system/icons/react' import { ALERT_SUBSCRIPTION_REGEX, isGlobPattern, - isSubscribableClass, PROBE_ALERT_CLASS, subscriptionRegex, } from '~/api/util' +import { useAlertClasses, type AlertClassMap } from '~/hooks/use-alert-classes' import { Checkbox } from '~/ui/lib/Checkbox' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { FieldLabel } from '~/ui/lib/FieldLabel' @@ -44,21 +42,20 @@ import { usePopoverZIndex } from '~/ui/lib/SideModal' import { TextInputError } from '~/ui/lib/TextInput' import { Tooltip } from '~/ui/lib/Tooltip' import { KEYS } from '~/ui/util/keys' -import { ALL_ISH } from '~/util/consts' /** * Segments may only contain [a-zA-Z0-9_], unlike resource names. An exact * (non-glob) subscription must also be a class the API knows about: it parses * the value as an `AlertClass` and rejects anything else with a 400, so pass - * `knownClasses` once the class list has loaded to catch that before submit. + * `classes` once the class list has loaded to catch that before submit. * https://github.com/oxidecomputer/omicron/blob/17e6fee/nexus/db-model/src/alert_subscription.rs#L61-L98 */ -export const validateSubscription = (value: string, knownClasses?: ReadonlySet) => { +export const validateSubscription = (classes?: AlertClassMap) => (value: string) => { if (!ALERT_SUBSCRIPTION_REGEX.test(value)) return 'Must be an alert class or a glob pattern like hardware.** (letters, numbers, and underscores only)' if (value === PROBE_ALERT_CLASS) - return 'The probe class is only used for liveness probes and cannot be subscribed to' - if (!isGlobPattern(value) && knownClasses && !knownClasses.has(value)) + return 'The probe class is only used for liveness probes and cannot be subscribed to or shown in alert lists' + if (!isGlobPattern(value) && classes && !classes.has(value)) return 'Not an alert class. Pick one from the list or use a glob pattern like hardware.**' return undefined } @@ -128,9 +125,10 @@ type RowState = /** Split subscriptions into glob matchers and exact class names */ function toMatchers(subscriptions: string[]) { return { - globs: subscriptions - .filter(isGlobPattern) - .map((g) => [g, subscriptionRegex(g)] as const), + globs: subscriptions.filter(isGlobPattern).flatMap((g) => { + const re = subscriptionRegex(g) + return re ? [[g, re] as const] : [] + }), exacts: new Set(subscriptions.filter((s) => !isGlobPattern(s))), } } @@ -201,11 +199,8 @@ export function SubscriptionsField< const [activeIdx, setActiveIdx] = useState(null) const [commitError, setCommitError] = useState() - const { data } = useQuery(q(api.alertClassList, { query: { limit: ALL_ISH } })) - const classes = (data?.items ?? []).filter(isSubscribableClass) - // undefined while loading so an exact class typed before the list arrives - // isn't rejected as unknown - const classNames = data ? new Set(classes.map((c) => c.name)) : undefined + const { data, classes } = useAlertClasses() + const classList = classes ? [...classes.values()] : [] // TName is constrained to string[] paths, but TS can't resolve the generic // PathValue to string[] here, so annotate to pin it @@ -216,17 +211,18 @@ export function SubscriptionsField< // undefined and the tooltip stays off const chipMatchCounts = new Map( data - ? matchers.globs.map(([g, re]) => [g, classes.filter((c) => re.test(c.name)).length]) + ? matchers.globs.map(([g, re]) => [ + g, + classList.filter((c) => re.test(c.name)).length, + ]) : [] ) const queryTrimmed = query.trim() - const queryIsValidGlob = - isGlobPattern(queryTrimmed) && ALERT_SUBSCRIPTION_REGEX.test(queryTrimmed) - const queryRegex = queryIsValidGlob ? subscriptionRegex(queryTrimmed) : null + const queryRegex = isGlobPattern(queryTrimmed) ? subscriptionRegex(queryTrimmed) : null // broadest version of the query glob (every wildcard segment widened to `**`), // used to keep near-miss rows visible with a hint about the covering pattern - const promotedGlob = queryIsValidGlob + const promotedGlob = queryRegex ? queryTrimmed .split('.') .map((seg) => (seg.includes('*') ? '**' : seg)) @@ -237,7 +233,7 @@ export function SubscriptionsField< // valid glob → its (widened) matches; glob still being typed (e.g. `*.`) → // everything, since substring matching on `*` can never hit a class name; // otherwise substring filter (which is a no-op for an empty query) - const visible = classes.filter((c) => + const visible = classList.filter((c) => promotedRegex ? promotedRegex.test(c.name) : isGlobPattern(queryTrimmed) || @@ -280,7 +276,7 @@ export function SubscriptionsField< function commitQuery() { const value = queryTrimmed - const error = validateSubscription(value, classNames) + const error = validateSubscription(classes)(value) if (error) { setCommitError(error) return @@ -340,7 +336,7 @@ export function SubscriptionsField< e.preventDefault() if (open && activeIdx !== null && rows[activeIdx]) { toggleRow(rows[activeIdx].name) - } else if (isGlobPattern(queryTrimmed) || classNames?.has(queryTrimmed)) { + } else if (isGlobPattern(queryTrimmed) || classes?.has(queryTrimmed)) { commitQuery() } } else if (e.key === KEYS.backspace || e.key === KEYS.delete) { @@ -395,7 +391,7 @@ export function SubscriptionsField< if (!e.currentTarget.contains(e.relatedTarget)) { closePanel() // valid globs save on blur - if (isGlobPattern(queryTrimmed) && !validateSubscription(queryTrimmed)) { + if (isGlobPattern(queryTrimmed) && !validateSubscription()(queryTrimmed)) { commitQuery() } else { setQuery('') @@ -475,13 +471,13 @@ export function SubscriptionsField< {queryTrimmed === '' ? ( <> All classes - Showing {classes.length} + Showing {classList.length} ) : ( <> Matching “{queryTrimmed}” - Showing {rows.length} of {classes.length} + Showing {rows.length} of {classList.length} )} diff --git a/app/hooks/use-alert-classes.tsx b/app/hooks/use-alert-classes.tsx new file mode 100644 index 0000000000..082c8cfebd --- /dev/null +++ b/app/hooks/use-alert-classes.tsx @@ -0,0 +1,49 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ +import { useQuery } from '@tanstack/react-query' + +import { type AlertClass, type AlertClassResultsPage, api, q } from '@oxide/api' + +import { isSubscribableClass } from '~/api/util' +import { type ComboboxItem } from '~/ui/lib/Combobox' +import { ItemLabel } from '~/ui/lib/ItemLabel' +import { ALL_ISH } from '~/util/consts' + +export const alertClassListQuery = q(api.alertClassList, { query: { limit: ALL_ISH } }) + +export type AlertClassMap = ReadonlyMap + +export function alertClassMap(data: AlertClassResultsPage): AlertClassMap { + return new Map(data.items.filter(isSubscribableClass).map((c) => [c.name, c])) +} + +type AlertClassesResult = { + data: AlertClassResultsPage | undefined + isPending: boolean + // undefined while loading so an exact class typed before the list arrives + // isn't rejected as unknown + classes: AlertClassMap | undefined +} + +export function useAlertClasses(): AlertClassesResult { + const { data, isPending } = useQuery(alertClassListQuery) + return { data, isPending, classes: data && alertClassMap(data) } +} + +/** Combobox item showing the alert class name with its description underneath */ +export const toClassComboboxItem = ({ + name, + description, +}: { + name: string + description: string +}): ComboboxItem => ({ + value: name, + selectedLabel: name, + label: {description}, +}) diff --git a/app/pages/system/alerting/AlertReceiverPage.tsx b/app/pages/system/alerting/AlertReceiverPage.tsx index f696f8ae34..5768a1cb81 100644 --- a/app/pages/system/alerting/AlertReceiverPage.tsx +++ b/app/pages/system/alerting/AlertReceiverPage.tsx @@ -6,7 +6,6 @@ * Copyright Oxide Computer Company */ -import { useQuery } from '@tanstack/react-query' import { createColumnHelper, getCoreRowModel, useReactTable } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' import { useForm, useWatch } from 'react-hook-form' @@ -24,7 +23,7 @@ import { import { Webhooks24Icon } from '@oxide/design-system/icons/react' import { Button } from '@oxide/design-system/ui' -import { isGlobPattern, isSubscribableClass, subscriptionRegex } from '~/api/util' +import { isGlobPattern, subscriptionRegex } from '~/api/util' import { AlertClassBadge } from '~/components/AlertClassBadge' import { ComboboxField } from '~/components/form/fields/ComboboxField' import { validateSubscription } from '~/components/form/fields/SubscriptionsField' @@ -34,6 +33,7 @@ import { HL } from '~/components/HL' import { MoreActionsMenu } from '~/components/MoreActionsMenu' import { QueryParamTabs } from '~/components/QueryParamTabs' import { SubscriptionMatchPreview } from '~/components/SubscriptionMatchPreview' +import { toClassComboboxItem, useAlertClasses } from '~/hooks/use-alert-classes' import { makeCrumb } from '~/hooks/use-crumbs' import { useAlertReceiverSelector } from '~/hooks/use-params' import { confirmAction } from '~/stores/confirm-action' @@ -43,18 +43,15 @@ import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' import { Table } from '~/table/Table' import { CardBlock, LearnMore } from '~/ui/lib/CardBlock' -import { type ComboboxItem } from '~/ui/lib/Combobox' import * as Dropdown from '~/ui/lib/DropdownMenu' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { InlineCode } from '~/ui/lib/InlineCode' -import { ItemLabel } from '~/ui/lib/ItemLabel' import { Message } from '~/ui/lib/Message' import { PageHeader, PageTitle } from '~/ui/lib/PageHeader' import { PropertiesTable } from '~/ui/lib/PropertiesTable' import { TableEmptyBox } from '~/ui/lib/Table' import { Tabs } from '~/ui/lib/Tabs' import { HintLink } from '~/ui/lib/TextInput' -import { ALL_ISH } from '~/util/consts' import { docLinks, links } from '~/util/links' import { pb } from '~/util/path-builder' import type * as PP from '~/util/path-params' @@ -227,19 +224,6 @@ function SubscriptionsCard() { ) } -// Combobox item showing the alert class name with its description underneath. -const toClassComboboxItem = ({ - name, - description, -}: { - name: string - description: string -}): ComboboxItem => ({ - value: name, - selectedLabel: name, - label: {description}, -}) - function AddSubscriptionModal({ onDismiss }: { onDismiss: () => void }) { const receiverSelector = useAlertReceiverSelector() const { data: receiver } = usePrefetchedQuery(receiverView(receiverSelector)) @@ -247,16 +231,14 @@ function AddSubscriptionModal({ onDismiss }: { onDismiss: () => void }) { const { control } = form const subscription = useWatch({ control, name: 'subscription' }) - const classes = useQuery(q(api.alertClassList, { query: { limit: ALL_ISH } })) - const subscribable = (classes.data?.items || []).filter(isSubscribableClass) - // undefined while loading so an exact class isn't rejected as unknown before - // the list arrives - const classNames = classes.data ? new Set(subscribable.map((c) => c.name)) : undefined + const { data, isPending, classes } = useAlertClasses() // leave out classes the receiver already gets, whether subscribed exactly or // covered by one of its globs, same as the create form's picker - const globs = receiver.subscriptions.filter(isGlobPattern).map(subscriptionRegex) - const classItems = subscribable + const globs = receiver.subscriptions + .filter(isGlobPattern) + .flatMap((g) => subscriptionRegex(g) ?? []) + const classItems = [...(classes?.values() ?? [])] .filter((c) => !receiver.subscriptions.includes(c.name)) .filter((c) => !globs.some((re) => re.test(c.name))) .map(toClassComboboxItem) @@ -299,12 +281,12 @@ function AddSubscriptionModal({ onDismiss }: { onDismiss: () => void }) { label="Subscription" placeholder="Enter alert pattern" items={classItems} - isLoading={classes.isPending} + isLoading={isPending} allowArbitraryValues required - validate={(value) => validateSubscription(value, classNames)} + validate={validateSubscription(classes)} /> - + ) } diff --git a/app/pages/system/alerting/AlertsTab.tsx b/app/pages/system/alerting/AlertsTab.tsx index 9f95f1fd0b..3423099c66 100644 --- a/app/pages/system/alerting/AlertsTab.tsx +++ b/app/pages/system/alerting/AlertsTab.tsx @@ -6,31 +6,81 @@ * Copyright Oxide Computer Company */ +import { Popover, PopoverButton, PopoverPanel, useClose } from '@headlessui/react' import cn from 'classnames' import { memo, useMemo, useState } from 'react' +import { useForm } from 'react-hook-form' +import { + useLocation, + useNavigation, + useSearchParams, + type LoaderFunctionArgs, +} from 'react-router' -import { api, getListQFn, queryClient, snakeify, type Alert } from '@oxide/api' -import { Notifications24Icon } from '@oxide/design-system/icons/react' +import { + api, + getListQFn, + queryClient, + snakeify, + usePrefetchedQuery, + type Alert, + type AlertListQueryParams, + type AlertSubscription, +} from '@oxide/api' +import { Filter16Icon, Notifications24Icon } from '@oxide/design-system/icons/react' +import { isGlobPattern, subscriptionRegex } from '~/api/util' import { AlertClassBadge } from '~/components/AlertClassBadge' import { AlertPayload } from '~/components/AlertPayload' +import { ComboboxField } from '~/components/form/fields/ComboboxField' +import { validateSubscription } from '~/components/form/fields/SubscriptionsField' import { ReadOnlySideModalForm } from '~/components/form/ReadOnlySideModalForm' import { HighlightJSON } from '~/components/HighlightJSON' +import { + alertClassMap, + type AlertClassMap, + alertClassListQuery, + toClassComboboxItem, +} from '~/hooks/use-alert-classes' import { EmptyCell } from '~/table/cells/EmptyCell' import { usePaginatedList } from '~/table/QueryTable' +import { Button, buttonStyle } from '~/ui/lib/Button' import { DateTime, SyslogDateTime } from '~/ui/lib/DateTime' +import { Divider } from '~/ui/lib/Divider' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { PropertiesTable } from '~/ui/lib/PropertiesTable' +import { Spinner } from '~/ui/lib/Spinner' import { TableEmptyBox } from '~/ui/lib/Table' import { Truncate } from '~/ui/lib/Truncate' import { roleDiv } from '~/util/classed' export const handle = { crumb: 'Alerts' } -const alertList = getListQFn(api.alertList, { query: { sortBy: 'time_and_id_descending' } }) +const alertList = (options: Omit = {}) => + getListQFn(api.alertList, { + query: { sortBy: 'time_and_id_descending', ...options }, + }) -export async function clientLoader() { - await queryClient.prefetchQuery(alertList.optionsFn()) +const SUBSCRIPTION_PARAM = 'subscription' + +const getAlertClassFilter = ( + searchParams: URLSearchParams, + classes: AlertClassMap +): string | undefined => { + const unsafe = searchParams.get(SUBSCRIPTION_PARAM) || undefined + + return typeof unsafe === 'string' && + typeof validateAlertClassFilter(classes)(unsafe) === 'string' + ? undefined + : unsafe +} + +export async function clientLoader({ request }: LoaderFunctionArgs) { + // Nexus rejects filters that don't match some existing alert class, so we + // need to prefetch the list of alert classes to safely apply filters. + const classes = alertClassMap(await queryClient.fetchQuery(alertClassListQuery)) + const alertClass = getAlertClassFilter(new URL(request.url).searchParams, classes) + await queryClient.prefetchQuery(alertList({ alertClass }).optionsFn()) return null } @@ -142,23 +192,206 @@ function AlertDetail({ alert, onDismiss }: { alert: Alert; onDismiss: () => void ) } +const matchesAny = (re: RegExp, names: Iterable) => { + for (const name of names) if (re.test(name)) return true + return false +} + +const validateAlertClassFilter = (classes: AlertClassMap) => (value: string) => { + if (value === '') return undefined + // covers format, probe, and unrecognized exact classes + const error = validateSubscription(classes)(value) + if (error) return error + + // for glob filters, the list endpoint rejects globs that don't match any + // existing alert class, so we need to test it in the client first + // https://github.com/oxidecomputer/omicron/blob/7e18e52/nexus/db-queries/src/db/datastore/alert.rs#L261-L272 + const re = subscriptionRegex(value) + if (re && !matchesAny(re, classes.keys())) { + return 'Pattern must match at least one alert class' + } + + return undefined +} + +function PatternMatchCount({ + pattern, + classes, +}: { + pattern: string + classes: AlertClassMap +}) { + const re = isGlobPattern(pattern) ? subscriptionRegex(pattern) : null + if (!re) return <>{pattern} + let count = 0 + for (const name of classes.keys()) if (re.test(name)) count++ + return ( + <> + {pattern} + + Matches {count} alert {count === 1 ? 'class' : 'classes'} + + + ) +} + +type FilterProps = { + isFetching: boolean + handleSubmit: (subscription: AlertSubscription) => void + lastApplied: AlertSubscription + classes: AlertClassMap +} + +type FilterFormProps = Omit + +// We're tolerating a little bit of prop drilling here because we want +// FilterForm to be its own component (so that the form is reset on close for +// free) +function FilterForm({ handleSubmit, classes, lastApplied }: FilterFormProps) { + const form = useForm({ defaultValues: { alertClassSearch: lastApplied } }) + const close = useClose() + + const resetDisabled = lastApplied === '' + + return ( +
{ + handleSubmit(alertClassSearch) + close() + })} + > +
+

Filter alerts

+ +
+ +
+ value.trim()} + allowArbitraryValues + renderArbitraryLabel={(value) => ( + + )} + /> +
+
+ +
+ + ) +} + +function Filter({ isFetching, handleSubmit, lastApplied, classes }: FilterProps) { + return ( + + +
+ {isFetching ? ( + + ) : ( + + )} + {lastApplied && '1'} +
+ + + + + + ) +} + export default function AlertsTab() { const [detail, setDetail] = useState(null) - const { items, isEmpty, pagination } = usePaginatedList(alertList, getId) + const [searchParams, setSearchParams] = useSearchParams() + const { data: classData } = usePrefetchedQuery(alertClassListQuery) + const classes = alertClassMap(classData) + const alertClass = getAlertClassFilter(searchParams, classes) + + const { items, isEmpty, pagination, queryResult } = usePaginatedList( + alertList({ alertClass }), + getId + ) + + function applyFilter(next: AlertSubscription) { + const newParams = new URLSearchParams(searchParams) + if (next) { + newParams.set(SUBSCRIPTION_PARAM, next) + } else { + newParams.delete(SUBSCRIPTION_PARAM) + } + setSearchParams(newParams, { replace: true }) + } - if (isEmpty) { - return ( - + const navigation = useNavigation() + const { pathname } = useLocation() + // Right now the only way to cause the loading state is to change the filters, + // so it's a good proxy + const isFiltering = + navigation.state === 'loading' && navigation.location.pathname === pathname + + const alerts = isEmpty ? ( + + {isFiltering || queryResult.isPlaceholderData ? ( + + ) : alertClass ? ( + } + title="No matching alerts" + body="No alerts match your filters." + buttonText="Reset filters" + onClick={() => applyFilter('')} + /> + ) : ( } title="No alerts" body="Alerts created by the system will appear here." /> - - ) - } - - return ( + )} + + ) : ( <> @@ -180,4 +413,18 @@ export default function AlertsTab() { {detail && setDetail(null)} />} ) + + return ( + <> +
+ +
+ {alerts} + + ) } diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index d679219e46..d03368a313 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -66,6 +66,7 @@ export type ComboboxBaseProps = { * type in new values that aren't in the list [default is `false`] */ allowArbitraryValues?: boolean + renderArbitraryLabel?: (value: string) => ReactNode /** * Pass in `onInputChange` when an event should be triggered when the user types in the field; * This is distinct from `onChange` which is triggered when the user selects an item from the list. @@ -103,6 +104,7 @@ export const Combobox = ({ onEnter, onInputChange, allowArbitraryValues = false, + renderArbitraryLabel, hideOptionalTag, inputRef, transform, @@ -146,7 +148,9 @@ export const Combobox = ({ ) { filteredItems.push({ value: query, - label: ( + label: renderArbitraryLabel ? ( + renderArbitraryLabel(query) + ) : ( <> Custom: {query} diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 69a3889a93..0c38ed297b 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -33,6 +33,8 @@ import { import { json, makeHandlers, type Json } from '~/api/__generated__/msw-handlers' import { instanceCan, + isGlobPattern, + isSubscribableClass, MAX_BUNDLE_COMMENT_BYTES, OXQL_GROUP_BY_ERROR, subscriptionRegex, @@ -2464,7 +2466,28 @@ export const handlers = makeHandlers({ final = final.filter((alert) => new Date(alert.time_created) >= startTime) if (endTime) final = final.filter((alert) => new Date(alert.time_created) <= endTime) if (alertClass) { + // Nexus parses the param as a subscription, which 400s on probe, + // malformed values, and unrecognized exact classes + // https://github.com/oxidecomputer/omicron/blob/7e18e52/nexus/db-model/src/alert_subscription.rs#L62-L98 + if (alertClass === 'probe') + throw invalidRequest( + `unsupported value for "alert_class": the 'probe' alert class is a synthetic alert used only for webhook liveness probes, and is not included in alert lists and cannot be subscribed to` + ) const matcher = subscriptionRegex(alertClass) + if ( + !matcher || + (!isGlobPattern(alertClass) && !alertClasses.some((c) => c.name === alertClass)) + ) { + throw invalidRequest(`'${alertClass}' is not a valid alert class`) + } + // a well-formed glob matching no classes is a 404 + // https://github.com/oxidecomputer/omicron/blob/7e18e52/nexus/db-queries/src/db/datastore/alert.rs#L261-L272 + if (!alertClasses.filter(isSubscribableClass).some((c) => matcher.test(c.name))) { + throw notFoundErr( + `alert class glob '${alertClass}' does not match any existing alert classes` + ) + } + final = final.filter((alert) => matcher.test(alert.class)) } @@ -2935,7 +2958,7 @@ export const handlers = makeHandlers({ // the real API rejects resends of alerts the receiver is no longer subscribed to // https://github.com/oxidecomputer/omicron/blob/32615a35/nexus/src/app/alert.rs#L439-L449 const subscribed = receiver.subscriptions.some((s) => - subscriptionRegex(s).test(delivery.alert_class) + subscriptionRegex(s)?.test(delivery.alert_class) ) if (!subscribed) { throw invalidRequest( diff --git a/test/e2e/alerts.e2e.ts b/test/e2e/alerts.e2e.ts index 6ef075261b..93c4f9fcb5 100644 --- a/test/e2e/alerts.e2e.ts +++ b/test/e2e/alerts.e2e.ts @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ -import { expect, test } from '@playwright/test' +import { expect, test, type Page } from '@playwright/test' import { alerts } from '@oxide/api-mocks' @@ -651,6 +651,126 @@ test('Alert list basics', async ({ page }) => { ) }) +const alertListFilteringBasics = async (page: Page) => { + await page.goto('/system/alerting/alerts') + + const table = page.getByRole('table') + const allRowCount = alerts.filter((a) => a.class !== 'probe').length + 1 + + await expect(table.getByRole('row')).toHaveCount(allRowCount) + + const filterButton = page.getByRole('button', { name: 'Filter alerts' }) + const input = page.getByRole('combobox', { name: 'Alert class' }) + const applyButton = page.getByRole('button', { name: 'Apply' }) + + return { table, allRowCount, filterButton, input, applyButton } +} + +test('Alert list filtering by class', async ({ page }) => { + const { table, filterButton, input, applyButton } = await alertListFilteringBasics(page) + + // typed glob previews how many classes it matches + await filterButton.click() + await input.fill('**.remove') + await page.getByRole('option', { name: /Matches 3 alert classes/ }).click() + await applyButton.click() + + // filter is applied and reflected in the URL + await expect(page).toHaveURL(/subscription=\*\*\.remove/) + await expect(table.getByRole('row')).toHaveCount(3) // header + 2 remove alerts + await expectRowVisible(table, { Class: 'hardware.power_shelf.psu.remove' }) + + // filter is still applied after refresh + await page.reload() + await expect(table.getByRole('row')).toHaveCount(3) +}) + +test('Alert list filter resetting', async ({ page }) => { + const { table, allRowCount, filterButton, input, applyButton } = + await alertListFilteringBasics(page) + + // typed glob previews how many classes it matches + await filterButton.click() + await input.fill('**.remove') + await page.getByRole('option', { name: /Matches 3 alert classes/ }).click() + await applyButton.click() + + await expect(table.getByRole('row')).toHaveCount(3) // header + 2 remove alerts + + // Reset clears the applied filter + await filterButton.click() + await page.getByRole('button', { name: 'Reset' }).click() + await expect(page).not.toHaveURL(/subscription/) + await expect(table.getByRole('row')).toHaveCount(allRowCount) + + // reapply filter + await filterButton.click() + await input.fill('**.remove') + await page.getByRole('option', { name: /Matches 3 alert classes/ }).click() + await applyButton.click() + await expect(table.getByRole('row')).toHaveCount(3) + + // clearing the input and applying also clears the filter + await filterButton.click() + await input.fill('') + await applyButton.click() + await expect(page).not.toHaveURL(/subscription/) + await expect(table.getByRole('row')).toHaveCount(allRowCount) +}) + +test('Alert list invalid configurations', async ({ page }) => { + const { table, allRowCount, filterButton, input, applyButton } = + await alertListFilteringBasics(page) + + // a class with no alerts gets the filtered empty state + await filterButton.click() + await input.fill('system.update.start') + await page.getByRole('option', { name: 'system.update.start' }).click() + await applyButton.click() + await expect(page.getByText('No matching alerts')).toBeVisible() + await page.getByRole('button', { name: 'Reset filters' }).click() + await expect(table.getByRole('row')).toHaveCount(allRowCount) + + // a glob that matches no classes is rejected on apply + await filterButton.click() + await input.fill('zzz.**') + await page.getByRole('option', { name: /Matches 0 alert classes/ }).click() + await applyButton.click() + const form = page.getByRole('form', { name: 'Filter alerts' }) + await expect(form.getByText('Pattern must match at least one alert class')).toBeVisible() + + // an invalid pattern is rejected on apply + await input.fill('hardware.(') + await page.getByRole('option', { name: 'hardware.(' }).click() + await applyButton.click() + await expect(form.getByText('Must be an alert class or a glob pattern')).toBeVisible() +}) + +test('Alert list loads with a bad subscription param in the URL', async ({ page }) => { + const allRowCount = alerts.filter((a) => a.class !== 'probe').length + 1 // + header + + // the API rejects probe as a filter, so the URL reader drops it and the + // page loads unfiltered instead of failing in the loader + await page.goto('/system/alerting/alerts?subscription=probe') + await expect(page.getByRole('table').getByRole('row')).toHaveCount(allRowCount) + + // malformed pattern + await page.goto('/system/alerting/alerts?subscription=hardware.(') + await expect(page.getByRole('table').getByRole('row')).toHaveCount(allRowCount) + + // unrecognized classes + await page.goto('/system/alerting/alerts?subscription=la.la.la') + await expect(page.getByRole('table').getByRole('row')).toHaveCount(allRowCount) + + // globs that match no class + await page.goto('/system/alerting/alerts?subscription=zzz.**') + await expect(page.getByRole('table').getByRole('row')).toHaveCount(allRowCount) + + // a real class with no alerts is a valid filter and gets an empty state + await page.goto('/system/alerting/alerts?subscription=system.update.start') + await expect(page.getByText('No matching alerts')).toBeVisible() +}) + test('Alert list detail view', async ({ page }) => { await page.goto('/system/alerting/alerts') From 28f35c80748c04a702a76f048988a804a51f4026 Mon Sep 17 00:00:00 2001 From: Joe Thel Date: Tue, 29 Sep 2026 14:52:00 -0700 Subject: [PATCH 2/6] Make a generic filter popover --- app/components/FilterPopover.browser.spec.tsx | 142 ++++++++++++++++ app/components/FilterPopover.tsx | 159 ++++++++++++++++++ app/pages/system/alerting/AlertsTab.tsx | 147 ++++------------ test/e2e/alerts.e2e.ts | 18 +- 4 files changed, 332 insertions(+), 134 deletions(-) create mode 100644 app/components/FilterPopover.browser.spec.tsx create mode 100644 app/components/FilterPopover.tsx diff --git a/app/components/FilterPopover.browser.spec.tsx b/app/components/FilterPopover.browser.spec.tsx new file mode 100644 index 0000000000..962630c7fb --- /dev/null +++ b/app/components/FilterPopover.browser.spec.tsx @@ -0,0 +1,142 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ +import { useState } from 'react' +import { expect, test } from 'vitest' +import { render } from 'vitest-browser-react' +import { page, userEvent } from 'vitest/browser' + +import { TextField } from '~/components/form/fields/TextField' + +import { FilterPopover } from './FilterPopover' + +type Filters = { name: string; owner: string; types: string[] } + +function Harness() { + const defaultValues = { name: '', owner: '', types: [] } + const [applied, setApplied] = useState(defaultValues) + return ( + <> + + {(form) => ( + <> + (value === 'bad' ? 'No bad names' : undefined)} + /> + + + + )} + + {applied.name} + + ) +} + +const filterButton = () => page.getByRole('button', { name: /^Filter widgets/ }) +const form = () => page.getByRole('form', { name: 'Filter widgets' }) +const resetButton = () => page.getByRole('button', { name: 'Reset' }) +const nameField = () => page.getByRole('textbox', { name: 'Name' }) +const applyButton = () => page.getByRole('button', { name: 'Apply' }) +const appliedName = () => page.getByRole('status', { name: 'Applied name' }) + +test('badge counts applied filters', async () => { + await render() + const noneApplied = page.getByRole('button', { name: 'Filter widgets', exact: true }) + await expect.element(noneApplied).toBeVisible() + + await noneApplied.click() + await page.getByRole('button', { name: 'Apply type filter' }).click() + await userEvent.keyboard('{Escape}') + const oneApplied = page.getByRole('button', { name: 'Filter widgets (1 applied)' }) + await expect.element(oneApplied).toBeVisible() + // this is the non-accessible text + await expect.element(oneApplied.getByText('1')).toBeVisible() + + await oneApplied.click() + await nameField().fill('abc') + await applyButton().click() + await expect + .element(page.getByRole('button', { name: 'Filter widgets (2 applied)' })) + .toBeVisible() + + await filterButton().click() + await page.getByRole('textbox', { name: 'Owner' }).fill('abc') + await applyButton().click() + await expect + .element(page.getByRole('button', { name: 'Filter widgets (3 applied)' })) + .toBeVisible() +}) + +test('apply submits the values and closes only if there are no validation errors', async () => { + await render() + await filterButton().click() + await nameField().fill('bad') + await applyButton().click() + await expect.element(form().getByText('No bad names')).toBeVisible() + await expect.element(appliedName()).toHaveTextContent('') + + await nameField().fill('good') + await applyButton().click() + await expect.element(form()).not.toBeInTheDocument() + await expect.element(appliedName()).toHaveTextContent('good') +}) + +test('reset applies the reset values', async () => { + await render() + await filterButton().click() + await nameField().fill('abc') + await applyButton().click() + await expect.element(appliedName()).toHaveTextContent('abc') + await filterButton().click() + await resetButton().click() + await expect.element(form()).not.toBeInTheDocument() + await expect.element(appliedName()).toHaveTextContent('') + await expect + .element(page.getByRole('button', { name: 'Filter widgets', exact: true })) + .toBeVisible() +}) + +test('reset is disabled when nothing is applied', async () => { + await render() + await filterButton().click() + await expect.element(resetButton()).toBeDisabled() + + // it's also disabled if you simply restore the default form values manually + await nameField().fill('abc') + await applyButton().click() + await filterButton().click() + await nameField().fill('') + await applyButton().click() + await filterButton().click() + await expect.element(resetButton()).toBeDisabled() +}) + +test('unapplied edits are discarded on close', async () => { + await render() + await filterButton().click() + await nameField().fill('draft') + await userEvent.keyboard('{Escape}') + await expect.element(form()).not.toBeInTheDocument() + + await filterButton().click() + await expect.element(nameField()).toHaveValue('') +}) diff --git a/app/components/FilterPopover.tsx b/app/components/FilterPopover.tsx new file mode 100644 index 0000000000..4c6cfd682c --- /dev/null +++ b/app/components/FilterPopover.tsx @@ -0,0 +1,159 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +import { Popover, PopoverButton, PopoverPanel, useClose } from '@headlessui/react' +import cn from 'classnames' +import type { ReactNode } from 'react' +import { + useForm, + type UseFormReturn, + type FieldValues, + type DefaultValues, +} from 'react-hook-form' +import * as R from 'remeda' + +import { Filter16Icon } from '@oxide/design-system/icons/react' + +import { Button, buttonStyle } from '~/ui/lib/Button' +import { Divider } from '~/ui/lib/Divider' +import { Spinner } from '~/ui/lib/Spinner' + +type FilterPopoverProps = + FilterFormProps & { + isFetching: boolean + } + +type FilterFormProps = { + /** + * The comparison point for counting the number of active changes, as well as the submitted value when resetting + */ + resetFieldValues: TFieldValues + /** + * The last filters that have been applied. The popover form initializes in this state. + */ + lastApplied: TFieldValues & DefaultValues + /** + * Fields you would like in the filter form + */ + children: (form: UseFormReturn) => ReactNode + handleSubmit: (form: TFieldValues) => void + /** + * The term used in labels (e.g. Filter ) + */ + pluralFilteredItemName: string +} + +const countApplied = ( + resetFieldValues: TFieldValues, + lastApplied: TFieldValues +) => + Object.entries(resetFieldValues).reduce( + (count, [key, value]) => (R.isDeepEqual(lastApplied[key], value) ? count : count + 1), + 0 + ) + +export function FilterPopover({ + children, + isFetching, + handleSubmit, + resetFieldValues, + pluralFilteredItemName, + lastApplied, +}: FilterPopoverProps) { + const changeCount = countApplied(resetFieldValues, lastApplied) + + return ( + + +
+ {isFetching ? ( + + ) : ( + + )} + {changeCount ? String(changeCount) : null} +
+ + + + {children} + + + + ) +} + +// This is its own component just so useForm is called fresh on every open +function FilterForm({ + children, + pluralFilteredItemName, + handleSubmit, + lastApplied, + resetFieldValues, +}: FilterFormProps) { + const close = useClose() + const form = useForm({ defaultValues: lastApplied }) + const resetDisabled = countApplied(resetFieldValues, lastApplied) === 0 + + return ( +
{ + handleSubmit(form) + close() + })} + > +
+

Filter {pluralFilteredItemName}

+ +
+ +
{children(form)}
+
+ +
+ + ) +} diff --git a/app/pages/system/alerting/AlertsTab.tsx b/app/pages/system/alerting/AlertsTab.tsx index 3423099c66..4635b3c67f 100644 --- a/app/pages/system/alerting/AlertsTab.tsx +++ b/app/pages/system/alerting/AlertsTab.tsx @@ -6,10 +6,9 @@ * Copyright Oxide Computer Company */ -import { Popover, PopoverButton, PopoverPanel, useClose } from '@headlessui/react' import cn from 'classnames' import { memo, useMemo, useState } from 'react' -import { useForm } from 'react-hook-form' +import { type UseFormReturn } from 'react-hook-form' import { useLocation, useNavigation, @@ -27,11 +26,12 @@ import { type AlertListQueryParams, type AlertSubscription, } from '@oxide/api' -import { Filter16Icon, Notifications24Icon } from '@oxide/design-system/icons/react' +import { Notifications24Icon } from '@oxide/design-system/icons/react' import { isGlobPattern, subscriptionRegex } from '~/api/util' import { AlertClassBadge } from '~/components/AlertClassBadge' import { AlertPayload } from '~/components/AlertPayload' +import { FilterPopover } from '~/components/FilterPopover' import { ComboboxField } from '~/components/form/fields/ComboboxField' import { validateSubscription } from '~/components/form/fields/SubscriptionsField' import { ReadOnlySideModalForm } from '~/components/form/ReadOnlySideModalForm' @@ -44,9 +44,7 @@ import { } from '~/hooks/use-alert-classes' import { EmptyCell } from '~/table/cells/EmptyCell' import { usePaginatedList } from '~/table/QueryTable' -import { Button, buttonStyle } from '~/ui/lib/Button' import { DateTime, SyslogDateTime } from '~/ui/lib/DateTime' -import { Divider } from '~/ui/lib/Divider' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { PropertiesTable } from '~/ui/lib/PropertiesTable' import { Spinner } from '~/ui/lib/Spinner' @@ -235,113 +233,6 @@ function PatternMatchCount({ ) } -type FilterProps = { - isFetching: boolean - handleSubmit: (subscription: AlertSubscription) => void - lastApplied: AlertSubscription - classes: AlertClassMap -} - -type FilterFormProps = Omit - -// We're tolerating a little bit of prop drilling here because we want -// FilterForm to be its own component (so that the form is reset on close for -// free) -function FilterForm({ handleSubmit, classes, lastApplied }: FilterFormProps) { - const form = useForm({ defaultValues: { alertClassSearch: lastApplied } }) - const close = useClose() - - const resetDisabled = lastApplied === '' - - return ( -
{ - handleSubmit(alertClassSearch) - close() - })} - > -
-

Filter alerts

- -
- -
- value.trim()} - allowArbitraryValues - renderArbitraryLabel={(value) => ( - - )} - /> -
-
- -
- - ) -} - -function Filter({ isFetching, handleSubmit, lastApplied, classes }: FilterProps) { - return ( - - -
- {isFetching ? ( - - ) : ( - - )} - {lastApplied && '1'} -
- - - - - - ) -} - export default function AlertsTab() { const [detail, setDetail] = useState(null) const [searchParams, setSearchParams] = useSearchParams() @@ -414,15 +305,37 @@ export default function AlertsTab() { ) + const alertClassField = (form: UseFormReturn<{ alertClassSearch: string }>) => ( + value.trim()} + allowArbitraryValues + renderArbitraryLabel={(value) => ( + + )} + /> + ) + return ( <>
- + resetFieldValues={{ alertClassSearch: '' }} + lastApplied={{ alertClassSearch: alertClass ?? '' }} + handleSubmit={({ alertClassSearch }) => { + applyFilter(alertClassSearch) + }} + pluralFilteredItemName="alerts" + > + {alertClassField} +
{alerts} diff --git a/test/e2e/alerts.e2e.ts b/test/e2e/alerts.e2e.ts index 93c4f9fcb5..4f0f5b366a 100644 --- a/test/e2e/alerts.e2e.ts +++ b/test/e2e/alerts.e2e.ts @@ -689,33 +689,17 @@ test('Alert list filter resetting', async ({ page }) => { const { table, allRowCount, filterButton, input, applyButton } = await alertListFilteringBasics(page) - // typed glob previews how many classes it matches await filterButton.click() await input.fill('**.remove') await page.getByRole('option', { name: /Matches 3 alert classes/ }).click() await applyButton.click() - await expect(table.getByRole('row')).toHaveCount(3) // header + 2 remove alerts - // Reset clears the applied filter + // Reset clears the applied filter and URL await filterButton.click() await page.getByRole('button', { name: 'Reset' }).click() await expect(page).not.toHaveURL(/subscription/) await expect(table.getByRole('row')).toHaveCount(allRowCount) - - // reapply filter - await filterButton.click() - await input.fill('**.remove') - await page.getByRole('option', { name: /Matches 3 alert classes/ }).click() - await applyButton.click() - await expect(table.getByRole('row')).toHaveCount(3) - - // clearing the input and applying also clears the filter - await filterButton.click() - await input.fill('') - await applyButton.click() - await expect(page).not.toHaveURL(/subscription/) - await expect(table.getByRole('row')).toHaveCount(allRowCount) }) test('Alert list invalid configurations', async ({ page }) => { From 4f730058e5f32bb4afc0ae4b9ac013dc18ff629c Mon Sep 17 00:00:00 2001 From: Joe Thel Date: Wed, 30 Sep 2026 13:01:18 -0700 Subject: [PATCH 3/6] patch firefox reloading behavior --- app/msw-mock-api.ts | 12 ++++++++++++ test/e2e/theme.e2e.ts | 5 ++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/msw-mock-api.ts b/app/msw-mock-api.ts index bc0ff9b66a..46c9702e05 100644 --- a/app/msw-mock-api.ts +++ b/app/msw-mock-api.ts @@ -69,6 +69,18 @@ async function streamBootLog(socket: WebSocket, text: string) { } export async function startMockAPI() { + // Specifically in Playwright's Firefox build, a page reload will cause a page + // to lack a service worker controller. MSW's fallback for this is to reload + // the page; you can guess the problem with that. Removing any existing + // workers will convince MSW to instead continue and register a new one. + // + // Context: + // https://github.com/mswjs/msw/blob/v2.7.5/src/browser/setupWorker/start/utils/getWorkerInstance.ts#L20-L27 + if ('serviceWorker' in navigator && !navigator.serviceWorker.controller) { + const registrations = await navigator.serviceWorker.getRegistrations() + await Promise.all(registrations.map((r) => r.unregister())) + } + // dynamic imports to make extremely sure none of this code ends up in the prod bundle const { z } = await import('zod/v4') // Configure Zod before importing the handlers, which create schemas during module evaluation. diff --git a/test/e2e/theme.e2e.ts b/test/e2e/theme.e2e.ts index e8ba4940c9..8200900f30 100644 --- a/test/e2e/theme.e2e.ts +++ b/test/e2e/theme.e2e.ts @@ -141,9 +141,8 @@ test('Theme picker changes data-theme on ', async ({ page }) => { await page.getByRole('menuitemradio', { name: 'Light' }).click() await expectTheme('light') - // navigate fresh and confirm the choice persisted via localStorage - // (page.reload() didn't work in Firefox) - await page.goto('/projects') + // reload and confirm the choice persisted via localStorage + await page.reload() await expectTheme('light') // reopen and pick Dark From 1cae171a4fcb45c18bc84283b3f0f51f82ce622a Mon Sep 17 00:00:00 2001 From: Joe Thel Date: Wed, 14 Oct 2026 10:23:06 -0700 Subject: [PATCH 4/6] Match alert list spacing to alert receiver spacing --- app/pages/system/alerting/AlertsTab.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/pages/system/alerting/AlertsTab.tsx b/app/pages/system/alerting/AlertsTab.tsx index 4635b3c67f..eb4e88bde0 100644 --- a/app/pages/system/alerting/AlertsTab.tsx +++ b/app/pages/system/alerting/AlertsTab.tsx @@ -324,7 +324,7 @@ export default function AlertsTab() { return ( <> -
+
Date: Thu, 15 Oct 2026 15:36:30 -0700 Subject: [PATCH 5/6] Remove redundant "no match" message from alerts tab --- app/pages/system/alerting/AlertsTab.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/app/pages/system/alerting/AlertsTab.tsx b/app/pages/system/alerting/AlertsTab.tsx index eb4e88bde0..f4315f05e7 100644 --- a/app/pages/system/alerting/AlertsTab.tsx +++ b/app/pages/system/alerting/AlertsTab.tsx @@ -270,7 +270,6 @@ export default function AlertsTab() { } title="No matching alerts" - body="No alerts match your filters." buttonText="Reset filters" onClick={() => applyFilter('')} /> From a420da71e3829f5aa941a35f032097607957f4a4 Mon Sep 17 00:00:00 2001 From: Joe Thel Date: Mon, 19 Oct 2026 15:25:02 -0700 Subject: [PATCH 6/6] Use generated Route types from new react-router --- app/pages/system/alerting/AlertsTab.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/app/pages/system/alerting/AlertsTab.tsx b/app/pages/system/alerting/AlertsTab.tsx index f4315f05e7..2755cd05e3 100644 --- a/app/pages/system/alerting/AlertsTab.tsx +++ b/app/pages/system/alerting/AlertsTab.tsx @@ -9,12 +9,7 @@ import cn from 'classnames' import { memo, useMemo, useState } from 'react' import { type UseFormReturn } from 'react-hook-form' -import { - useLocation, - useNavigation, - useSearchParams, - type LoaderFunctionArgs, -} from 'react-router' +import { useLocation, useNavigation, useSearchParams } from 'react-router' import { api, @@ -52,6 +47,8 @@ import { TableEmptyBox } from '~/ui/lib/Table' import { Truncate } from '~/ui/lib/Truncate' import { roleDiv } from '~/util/classed' +import type { Route } from './+types/AlertsTab' + export const handle = { crumb: 'Alerts' } const alertList = (options: Omit = {}) => @@ -73,7 +70,7 @@ const getAlertClassFilter = ( : unsafe } -export async function clientLoader({ request }: LoaderFunctionArgs) { +export async function clientLoader({ request }: Route.ClientLoaderArgs) { // Nexus rejects filters that don't match some existing alert class, so we // need to prefetch the list of alert classes to safely apply filters. const classes = alertClassMap(await queryClient.fetchQuery(alertClassListQuery))