From e32a25ff0bdb7f54668fe6e5191a7b45dbb754ea Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Thu, 3 Sep 2026 11:18:07 -0400 Subject: [PATCH] feat: Show and edit alert displayName and tags in UI --- packages/api/src/controllers/dashboard.ts | 28 ++- packages/api/src/controllers/savedSearch.ts | 8 +- .../api/__tests__/dashboard.int.test.ts | 24 ++ .../api/__tests__/savedSearch.int.test.ts | 27 ++ packages/api/src/utils/alerts.ts | 31 ++- packages/app/src/AlertDetailPage.tsx | 17 +- packages/app/src/AlertsPage.tsx | 15 +- packages/app/src/DBSearchPageAlertModal.tsx | 32 +++ .../__tests__/DBSearchPageAlertModal.test.tsx | 69 +++++- .../app/src/components/AlertDisplayFields.tsx | 109 +++++++++ .../ChartEditor/RawSqlChartEditor.tsx | 9 +- .../ChartEditorControls.tsx | 9 +- .../DBEditTimeChartForm/TileAlertEditor.tsx | 18 ++ .../__tests__/AlertDisplayFields.test.tsx | 175 +++++++++++++ .../src/components/alerts/AlertCardList.tsx | 3 +- .../alerts/AlertDetailProperties.tsx | 10 +- .../src/components/alerts/AlertDetails.tsx | 38 +-- .../src/components/alerts/AlertRowMenu.tsx | 4 +- .../src/components/alerts/EditAlertModal.tsx | 15 +- .../__tests__/AlertDetailProperties.test.tsx | 31 ++- .../alerts/__tests__/AlertRowMenu.test.tsx | 2 +- .../alerts/__tests__/EditAlertModal.test.tsx | 36 +++ .../app/src/utils/__tests__/alerts.test.ts | 53 ++++ packages/app/src/utils/alerts.ts | 18 +- .../e2e/components/ChartEditorComponent.ts | 27 +- .../SearchPageAlertModalComponent.ts | 22 ++ .../app/tests/e2e/features/alerts.spec.ts | 231 ++++++++++++++++++ .../app/tests/e2e/page-objects/AlertsPage.ts | 28 +++ packages/app/tests/e2e/utils/tags.ts | 52 ++++ packages/cli/src/api/client.ts | 2 + packages/cli/src/components/AlertsPage.tsx | 2 +- .../common-utils/src/__tests__/alerts.test.ts | 45 ++++ packages/common-utils/src/alerts.ts | 29 +++ 33 files changed, 1113 insertions(+), 106 deletions(-) create mode 100644 packages/app/src/components/AlertDisplayFields.tsx create mode 100644 packages/app/src/components/__tests__/AlertDisplayFields.test.tsx create mode 100644 packages/app/tests/e2e/utils/tags.ts create mode 100644 packages/common-utils/src/__tests__/alerts.test.ts create mode 100644 packages/common-utils/src/alerts.ts diff --git a/packages/api/src/controllers/dashboard.ts b/packages/api/src/controllers/dashboard.ts index 47fab50252..8fcc265613 100644 --- a/packages/api/src/controllers/dashboard.ts +++ b/packages/api/src/controllers/dashboard.ts @@ -17,6 +17,7 @@ import { import type { ObjectId } from '@/models'; import type { AlertDocument, IAlert } from '@/models/alert'; import Dashboard, { IDashboard } from '@/models/dashboard'; +import { resolveAlertDisplayFields } from '@/utils/alerts'; function pickAlertsByTile(tiles: Tile[]) { return tiles.reduce((acc, tile) => { @@ -117,6 +118,23 @@ async function syncDashboardAlerts( } } +/** + * Attach the resolved display name/tags so the chart editor prefills real + * values for alerts written before the fields existed. + */ +function withResolvedDisplayFields( + alert: AlertDocument | undefined, + dashboard: Pick, +) { + if (alert == null) { + return undefined; + } + return { + ...alert.toJSON(), + ...resolveAlertDisplayFields(alert, { dashboard }), + }; +} + export async function getDashboards(teamId: ObjectId) { const [_dashboards, alerts] = await Promise.all([ Dashboard.find({ team: teamId }) @@ -133,7 +151,10 @@ export async function getDashboards(teamId: ObjectId) { ...t, config: { ...t.config, - alert: alerts[`${d._id.toString()}:${t.id}`]?.[0], + alert: withResolvedDisplayFields( + alerts[`${d._id.toString()}:${t.id}`]?.[0], + d, + ), }, })), })) @@ -158,7 +179,10 @@ export async function getDashboard(dashboardId: string, teamId: ObjectId) { ..._dashboard.toJSON(), tiles: _dashboard.tiles.map(t => ({ ...t, - config: { ...t.config, alert: alerts[t.id]?.[0] }, + config: { + ...t.config, + alert: withResolvedDisplayFields(alerts[t.id]?.[0], _dashboard), + }, })), }); } diff --git a/packages/api/src/controllers/savedSearch.ts b/packages/api/src/controllers/savedSearch.ts index 53e773c545..ac66b1cb47 100644 --- a/packages/api/src/controllers/savedSearch.ts +++ b/packages/api/src/controllers/savedSearch.ts @@ -8,6 +8,7 @@ import { z } from 'zod'; import { deleteSavedSearchAlerts } from '@/controllers/alerts'; import Alert from '@/models/alert'; import { SavedSearch } from '@/models/savedSearch'; +import { resolveAlertDisplayFields } from '@/utils/alerts'; import logger from '@/utils/logger'; type SavedSearchWithoutId = Omit, 'id'>; @@ -27,9 +28,10 @@ export async function getSavedSearches( return savedSearches.map(savedSearch => ({ ...savedSearch.toJSON(), - alerts: alertsBySavedSearchId[savedSearch._id.toString()]?.map(alert => { - return alert.toJSON(); - }), + alerts: alertsBySavedSearchId[savedSearch._id.toString()]?.map(alert => ({ + ...alert.toJSON(), + ...resolveAlertDisplayFields(alert, { savedSearch }), + })), })); } diff --git a/packages/api/src/routers/api/__tests__/dashboard.int.test.ts b/packages/api/src/routers/api/__tests__/dashboard.int.test.ts index c5b05219b3..88d985856d 100644 --- a/packages/api/src/routers/api/__tests__/dashboard.int.test.ts +++ b/packages/api/src/routers/api/__tests__/dashboard.int.test.ts @@ -266,6 +266,30 @@ describe('dashboard router', () => { expect(storedAlert?.tags).toEqual(['ops']); }); + it('resolves displayName and tags on GET for a tile alert stored without them', async () => { + const mockAlert = makeMockAlert(webhook._id.toString()); + const dashboard = await agent + .post('/dashboards') + .send({ + name: 'Test Dashboard', + tiles: [makeTile({ alert: mockAlert })], + tags: ['ops'], + }) + .expect(200); + // Alerts written before the fields existed. + await Alert.updateMany( + { dashboard: dashboard.body.id }, + { $set: { displayName: null, tags: null } }, + ); + + const list = await agent.get('/dashboards').expect(200); + const fromList = list.body.find(d => d._id === dashboard.body.id); + expect(fromList.tiles[0].config.alert).toMatchObject({ + displayName: 'Test Dashboard - Test Chart', + tags: ['ops'], + }); + }); + it('persists a tile alert displayName sent through the dashboard PATCH', async () => { const mockAlert = makeMockAlert(webhook._id.toString()); const dashboard = await agent diff --git a/packages/api/src/routers/api/__tests__/savedSearch.int.test.ts b/packages/api/src/routers/api/__tests__/savedSearch.int.test.ts index d0c2a78b8e..6f0abfd317 100644 --- a/packages/api/src/routers/api/__tests__/savedSearch.int.test.ts +++ b/packages/api/src/routers/api/__tests__/savedSearch.int.test.ts @@ -221,4 +221,31 @@ describe('savedSearch router', () => { expect(savedSearches.body[0].alerts[0].createdBy).toBeDefined(); expect(savedSearches.body[0].alerts[0].createdBy.email).toBe(user.email); }); + + it('resolves displayName and tags on GET for an alert stored without them', async () => { + const savedSearch = await agent + .post('/saved-search') + .send({ ...MOCK_SAVED_SEARCH, tags: ['prod'] }) + .expect(200); + const alert = await agent + .post('/alerts') + .send( + makeSavedSearchAlertInput({ + savedSearchId: savedSearch.body._id, + webhookId: webhook._id.toString(), + }), + ) + .expect(200); + // Alerts written before the fields existed. + await Alert.updateOne( + { _id: alert.body.data._id }, + { $set: { displayName: null, tags: null } }, + ); + + const savedSearches = await agent.get('/saved-search').expect(200); + expect(savedSearches.body[0].alerts[0]).toMatchObject({ + displayName: MOCK_SAVED_SEARCH.name, + tags: ['prod'], + }); + }); }); diff --git a/packages/api/src/utils/alerts.ts b/packages/api/src/utils/alerts.ts index fc47fbf846..fdcf66ec6b 100644 --- a/packages/api/src/utils/alerts.ts +++ b/packages/api/src/utils/alerts.ts @@ -1,8 +1,8 @@ import { - MAX_ALERT_DISPLAY_NAME_LENGTH, - MAX_TAG_LENGTH, - MAX_TAGS, -} from '@hyperdx/common-utils/dist/types'; + clampAlertDisplayName, + clampAlertTags, + formatTileAlertDisplayName, +} from '@hyperdx/common-utils/dist/alerts'; import { Types } from 'mongoose'; import type { ObjectId } from '@/models'; @@ -44,7 +44,6 @@ type AlertDisplayInput = { }; const FALLBACK_DISPLAY_NAME = 'Alert'; -const FALLBACK_TILE_NAME = 'Tile'; /** * Referenced entities are read straight from Mongo and predate any of these @@ -59,19 +58,14 @@ function normalizeName(value: unknown): string | null { return trimmed === '' ? null : trimmed; } -/** - * Derived tags are persisted on the alert, which validates them with - * `alertTagsSchema`, but they are copied from a dashboard/saved search whose - * own schema caps neither length nor count. Apply the alert caps here. - */ +/** Untrusted Mongo value -> tags that satisfy `alertTagsSchema`. */ function normalizeTags(value: unknown): string[] { if (!Array.isArray(value)) { return []; } - return value - .filter((tag): tag is string => typeof tag === 'string' && tag !== '') - .slice(0, MAX_TAGS) - .map(tag => tag.slice(0, MAX_TAG_LENGTH)); + return clampAlertTags( + value.filter((tag): tag is string => typeof tag === 'string'), + ); } /** @@ -119,8 +113,10 @@ export function deriveAlertDisplayFields( const tile = alert.tileId ? tiles.find(t => t.id === alert.tileId) : undefined; - const tileName = normalizeName(tile?.config?.name); - displayName = `${dashboardName} - ${tileName ?? FALLBACK_TILE_NAME}`; + displayName = formatTileAlertDisplayName( + dashboardName, + normalizeName(tile?.config?.name), + ); } tags = dashboard ? normalizeTags(dashboard.tags) : null; break; @@ -147,7 +143,8 @@ export function deriveAlertDisplayFields( } return { - displayName: displayName?.slice(0, MAX_ALERT_DISPLAY_NAME_LENGTH) ?? null, + displayName: + displayName == null ? null : clampAlertDisplayName(displayName), tags, }; } diff --git a/packages/app/src/AlertDetailPage.tsx b/packages/app/src/AlertDetailPage.tsx index 99f3a583f5..3eb3553702 100644 --- a/packages/app/src/AlertDetailPage.tsx +++ b/packages/app/src/AlertDetailPage.tsx @@ -28,11 +28,7 @@ import EmptyState from '@/components/EmptyState'; import { PageHeader } from '@/components/PageHeader'; import { TimePicker } from '@/components/TimePicker'; import { IS_ALERT_DETAILS_ENABLED } from '@/config'; -import { - getAlertDisplayName, - getAlertSourceLabel, - getAlertSourceUrl, -} from '@/utils/alerts'; +import { getAlertSourceLabel, getAlertSourceUrl } from '@/utils/alerts'; import { useBrandDisplayName } from './theme/ThemeProvider'; import api from './api'; @@ -135,14 +131,16 @@ function AlertDetailBody({ alert }: { alert: AlertsPageItem }) { Alerts - {getAlertDisplayName(alert) || 'Alert'} + {alert.displayName || 'Alert'} } leading={ {alert.state != null && } - {getAlertDisplayName(alert)} + + {alert.displayName} + {alertUrl && ( /* Next to the name rather than in the actions row: it navigates to what the alert watches, so it belongs with the identity, @@ -174,7 +172,7 @@ function AlertDetailBody({ alert }: { alert: AlertsPageItem }) { this page carries that link next to the title instead. */} router.push('/alerts')} /> @@ -255,8 +253,7 @@ export default function AlertDetailPage() { > - {alert ? `${getAlertDisplayName(alert)} - Alerts` : 'Alerts'} -{' '} - {brandName} + {alert ? `${alert.displayName} - Alerts` : 'Alerts'} - {brandName} {isLoading && ( diff --git a/packages/app/src/AlertsPage.tsx b/packages/app/src/AlertsPage.tsx index 57dd92fff2..fac9ad1bed 100644 --- a/packages/app/src/AlertsPage.tsx +++ b/packages/app/src/AlertsPage.tsx @@ -19,12 +19,7 @@ import { import { AlertCardList } from '@/components/alerts/AlertCardList'; import EmptyState from '@/components/EmptyState'; import { PageHeader } from '@/components/PageHeader'; -import { - getAlertCreatorLabel, - getAlertDisplayName, - getAlertSourceLabel, - getAlertTags, -} from '@/utils/alerts'; +import { getAlertCreatorLabel, getAlertSourceLabel } from '@/utils/alerts'; import { useBrandDisplayName } from './theme/ThemeProvider'; import api from './api'; @@ -43,7 +38,7 @@ export default function AlertsPage() { const allTags = React.useMemo(() => { const tags = new Set(); - alerts.forEach(a => getAlertTags(a).forEach(t => tags.add(t))); + alerts.forEach(a => a.tags?.forEach(t => tags.add(t))); return Array.from(tags).sort(); }, [alerts]); @@ -70,7 +65,7 @@ export default function AlertsPage() { result = result.filter(a => getAlertSourceLabel(a) === sourceFilter); } if (tagFilter) { - result = result.filter(a => getAlertTags(a).includes(tagFilter)); + result = result.filter(a => a.tags?.includes(tagFilter)); } if (creatorFilter) { result = result.filter(a => getAlertCreatorLabel(a) === creatorFilter); @@ -79,14 +74,14 @@ export default function AlertsPage() { const q = search.toLowerCase(); result = result.filter( a => - getAlertDisplayName(a).toLowerCase().includes(q) || + a.displayName?.toLowerCase().includes(q) || // So "tile" / "saved search" narrow the list the same way the type // filter does, without having to reach for the dropdown. getAlertSourceLabel(a) .toLowerCase() .split(' ') .some(word => word.startsWith(q)) || - getAlertTags(a).some(t => t.toLowerCase().includes(q)), + a.tags?.some(t => t.toLowerCase().includes(q)), ); } return result; diff --git a/packages/app/src/DBSearchPageAlertModal.tsx b/packages/app/src/DBSearchPageAlertModal.tsx index 5cd2a96c10..516e20f730 100644 --- a/packages/app/src/DBSearchPageAlertModal.tsx +++ b/packages/app/src/DBSearchPageAlertModal.tsx @@ -3,11 +3,17 @@ import router from 'next/router'; import { Controller, useForm, useWatch } from 'react-hook-form'; import { z } from 'zod'; import { zodResolver } from '@hookform/resolvers/zod'; +import { + clampAlertDisplayName, + clampAlertTags, +} from '@hyperdx/common-utils/dist/alerts'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { type Alert, + alertDisplayNameSchema, AlertIntervalSchema, AlertSource, + alertTagsSchema, AlertThresholdType, Filter, isRangeThresholdType, @@ -56,6 +62,7 @@ import { toAlertChannels, } from '@/utils/alerts'; +import { AlertDisplayFields } from './components/AlertDisplayFields'; import { AlertNoteField } from './components/AlertNoteField'; import { AlertPreviewChart } from './components/AlertPreviewChart'; import { AlertChannelForm } from './components/Alerts'; @@ -78,6 +85,8 @@ const SavedSearchAlertFormSchema = z scheduleStartAt: scheduleStartAtSchema, thresholdType: z.nativeEnum(AlertThresholdType), channels: zAlertChannels, + displayName: alertDisplayNameSchema, + tags: alertTagsSchema, // nullish() (not optional()): persisted alerts store this as null, which // optional() would reject. numConsecutiveWindows: z.number().int().min(1).nullish(), @@ -93,9 +102,11 @@ const AlertForm = ({ filters, select, defaultValues, + prefill, loading, deleteLoading, hasSavedSearch, + savedSearchName, onDelete, onSubmit, onClose, @@ -106,9 +117,12 @@ const AlertForm = ({ filters?: Filter[] | null; select?: string | null; defaultValues?: null | AlertWithCreatedBy; + /** Seed values for a brand-new alert */ + prefill?: { displayName?: string; tags?: string[] }; loading?: boolean; deleteLoading?: boolean; hasSavedSearch?: boolean; + savedSearchName?: string; onDelete: (id: string) => void; onSubmit: (data: Alert) => void; onClose: () => void; @@ -140,6 +154,8 @@ const AlertForm = ({ source: AlertSource.SAVED_SEARCH, channels: [{ type: 'webhook', webhookId: '' }], note: null, + displayName: prefill?.displayName, + tags: prefill?.tags, }, resolver: zodResolver(SavedSearchAlertFormSchema), }); @@ -193,6 +209,13 @@ const AlertForm = ({ > + Trigger @@ -607,6 +630,15 @@ export const DBSearchPageAlertModal = ({ ({ data: [firstAlert, secondAlert].find(a => a.id === id) ?? firstAlert, }, }), + useTags: () => ({ + data: { data: ['prod'] }, + isLoading: false, + isError: false, + refetch: jest.fn(), + }), }, })); @@ -98,10 +109,30 @@ jest.mock('@/components/AlertPreviewChart', () => ({ __esModule: true, AlertPreviewChart: () =>
, })); -jest.mock('@/components/Alerts', () => ({ - __esModule: true, - AlertChannelForm: () =>
, -})); +// Stubbed, but it still has to register the webhook id: the form schema +// rejects an empty one, so a create submitted through an inert stub would +// never reach the mutation. +jest.mock('@/components/Alerts', () => { + return { + __esModule: true, + AlertChannelForm: ({ + control, + channelsName, + }: ComponentProps) => ( + ( + + )} + /> + ), + }; +}); jest.mock('@/components/SQLEditor/SQLInlineEditor', () => ({ __esModule: true, SQLInlineEditorControlled: () =>
, @@ -178,4 +209,32 @@ describe('DBSearchPageAlertModal', () => { { type: 'webhook', webhookId: 'webhook-id' }, ]); }); + + // A new alert opens prefilled from the saved search it will watch, so the + // create payload carries those values rather than relying on the server's + // derivation. + it('prefills the display name and tags from the saved search, clamped to the alert caps', async () => { + renderModal(); + + expect(await screen.findByTestId('alert-display-name-input')).toHaveValue( + 'My Search', + ); + + fireEvent.change(screen.getByTestId('webhook-id-input'), { + target: { value: 'webhook-id' }, + }); + fireEvent.click( + await screen.findByRole('button', { name: 'Create Alert' }), + ); + + await waitFor(() => { + expect(createAlertMutateAsync).toHaveBeenCalledTimes(1); + }); + expect(createAlertMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ + displayName: 'My Search', + tags: ['prod', 'checkout-service-payment-failure'], + }), + ); + }); }); diff --git a/packages/app/src/components/AlertDisplayFields.tsx b/packages/app/src/components/AlertDisplayFields.tsx new file mode 100644 index 0000000000..67d3e43c8c --- /dev/null +++ b/packages/app/src/components/AlertDisplayFields.tsx @@ -0,0 +1,109 @@ +import { + Control, + Controller, + FieldError, + FieldValues, + Merge, + Path, +} from 'react-hook-form'; +import { Button, Group, Stack, Text, TextInput } from '@mantine/core'; +import { IconTags } from '@tabler/icons-react'; + +import { Tags } from '@/components/Tags'; + +/** + * A tags error usually sits on one element (`tags.3`), not on the array, so + * fieldState.error is a sparse array of per-item errors with no message of + * its own. Surface the first one found. + */ +function firstErrorMessage( + error: Merge | FieldError | undefined, +): string | undefined { + if (error == null) return undefined; + if (error.message) return error.message; + if (Array.isArray(error)) { + return error.find(e => e?.message)?.message; + } + return undefined; +} + +export function AlertDisplayFields({ + control, + displayNameName, + tagsName, + derivedDisplayName, + labelMarginTop = 'xs', +}: { + control: Control; + displayNameName: Path; + tagsName: Path; + /** Name the server will derive when the field is left empty. */ + derivedDisplayName?: string | null; + labelMarginTop?: string; +}) { + return ( + +
+ + Name + + ( + field.onChange(e.target.value || null)} + /> + )} + /> +
+ +
+ + Tags + + { + const values: string[] = field.value ?? []; + const error = firstErrorMessage(fieldState.error); + return ( + + + + + {error && ( + + {error} + + )} + + ); + }} + /> +
+ + ); +} diff --git a/packages/app/src/components/ChartEditor/RawSqlChartEditor.tsx b/packages/app/src/components/ChartEditor/RawSqlChartEditor.tsx index 903cef223b..1e351ceecd 100644 --- a/packages/app/src/components/ChartEditor/RawSqlChartEditor.tsx +++ b/packages/app/src/components/ChartEditor/RawSqlChartEditor.tsx @@ -140,6 +140,7 @@ export default function RawSqlChartEditor({ const connection = useWatch({ control, name: 'connection' }); const source = useWatch({ control, name: 'source' }); const sqlTemplate = useWatch({ control, name: 'sqlTemplate' }); + const chartName = useWatch({ control, name: 'name' }); const sourceObject = sources?.find(s => s.id === source); const rawSqlConfig = useMemo( @@ -298,7 +299,12 @@ export default function RawSqlChartEditor({ data-testid="alert-button" size="sm" color={'gray'} - onClick={() => setValue('alert', DEFAULT_TILE_ALERT)} + onClick={() => + setValue('alert', { + ...DEFAULT_TILE_ALERT, + ...(chartName && { displayName: chartName }), + }) + } > Add Alert @@ -361,6 +367,7 @@ export default function RawSqlChartEditor({ control={control} setValue={setValue} alert={alert} + dashboardId={dashboardId} onRemove={() => setValue('alert', undefined)} error={alertErrorMessage} warning={alertWarningMessage} diff --git a/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx b/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx index 4f26493cf0..47bdcf97bd 100644 --- a/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx @@ -176,6 +176,7 @@ export function ChartEditorControls({ // Grouped ratios can divide two ways (see RatioModeSchema); the mode toggle // is only meaningful when a Group By is set, so gate it on a non-empty value. const groupBy = useWatch({ control, name: 'groupBy' }); + const chartName = useWatch({ control, name: 'name' }); const hasGroupBy = typeof groupBy === 'string' && groupBy.trim().length > 0; return ( @@ -490,7 +491,12 @@ export function ChartEditorControls({ variant="subtle" data-testid="alert-button" size="sm" - onClick={() => setValue('alert', DEFAULT_TILE_ALERT)} + onClick={() => + setValue('alert', { + ...DEFAULT_TILE_ALERT, + ...(chartName && { displayName: chartName }), + }) + } > Add Alert @@ -559,6 +565,7 @@ export function ChartEditorControls({ control={control} setValue={setValue} alert={alert} + dashboardId={dashboardId} onRemove={() => setValue('alert', undefined)} warning={ additionalWarnings?.length diff --git a/packages/app/src/components/DBEditTimeChartForm/TileAlertEditor.tsx b/packages/app/src/components/DBEditTimeChartForm/TileAlertEditor.tsx index 9df6027eac..62459a5939 100644 --- a/packages/app/src/components/DBEditTimeChartForm/TileAlertEditor.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/TileAlertEditor.tsx @@ -4,6 +4,7 @@ import { UseFormSetValue, useWatch, } from 'react-hook-form'; +import { formatTileAlertDisplayName } from '@hyperdx/common-utils/dist/alerts'; import { AlertThresholdType, isRangeThresholdType, @@ -31,12 +32,14 @@ import { } from '@tabler/icons-react'; import api from '@/api'; +import { AlertDisplayFields } from '@/components/AlertDisplayFields'; import { AlertNoteField } from '@/components/AlertNoteField'; import { AlertChannelForm } from '@/components/Alerts'; import { AckAlert } from '@/components/alerts/AckAlert'; import { AlertHistoryCardList } from '@/components/alerts/AlertHistoryCards'; import { AlertScheduleFields } from '@/components/AlertScheduleFields'; import { ChartEditorFormState } from '@/components/ChartEditor/types'; +import { useDashboards } from '@/dashboard'; import { optionsToSelectData } from '@/utils'; import { ALERT_CHANNEL_OPTIONS, @@ -49,6 +52,7 @@ export function TileAlertEditor({ control, setValue, alert, + dashboardId, onRemove, error, warning, @@ -57,6 +61,7 @@ export function TileAlertEditor({ control: Control; setValue: UseFormSetValue; alert: NonNullable; + dashboardId?: string; onRemove: () => void; error?: string; warning?: string; @@ -85,6 +90,13 @@ export function TileAlertEditor({ const { data: alertData } = api.useAlert(alert.id); const alertItem = alertData?.data; + const tileName = useWatch({ control, name: 'name' }); + const { data: dashboards } = useDashboards(); + const dashboardName = dashboards?.find(d => d.id === dashboardId)?.name; + const derivedDisplayName = dashboardName + ? formatTileAlertDisplayName(dashboardName, tileName) + : undefined; + return ( @@ -228,6 +240,12 @@ export function TileAlertEditor({ Created by {alert.createdBy.name || alert.createdBy.email} )} + ({ + __esModule: true, + default: { + useTags: () => ({ + data: { data: TAGS }, + isLoading: false, + isError: false, + refetch: jest.fn().mockResolvedValue({ data: { data: TAGS } }), + }), + }, +})); + +type FormValues = { + displayName?: string | null; + tags?: string[] | null; +}; + +const submitted = jest.fn(); + +// The real caps, so the error rendering is tested against the messages the +// alert forms actually produce. +const schema = z.object({ + displayName: alertDisplayNameSchema, + tags: alertTagsSchema, +}); + +const NO_VALUES: FormValues = {}; + +const Harness = ({ + initial = NO_VALUES, + derivedDisplayName, +}: { + initial?: FormValues; + derivedDisplayName?: string; +}) => { + const { control, handleSubmit } = useForm({ + defaultValues: initial, + resolver: zodResolver(schema), + }); + return ( +
+ + + + ); +}; + +const submit = async () => { + fireEvent.click(screen.getByText('Submit')); + await waitFor(() => expect(submitted).toHaveBeenCalledTimes(1)); + return submitted.mock.calls[0][0]; +}; + +beforeEach(() => { + submitted.mockClear(); +}); + +describe('AlertDisplayFields', () => { + it('emits null when the name is cleared so the server re-derives it', async () => { + renderWithMantine(); + + fireEvent.change(screen.getByTestId('alert-display-name-input'), { + target: { value: '' }, + }); + + expect((await submit()).displayName).toBeNull(); + }); + + it('shows the derived name as the placeholder', () => { + renderWithMantine(); + + expect(screen.getByTestId('alert-display-name-input')).toHaveAttribute( + 'placeholder', + 'Checkout - Error rate', + ); + }); + + it('falls back to a generic placeholder without a derived name', () => { + renderWithMantine(); + + expect(screen.getByTestId('alert-display-name-input')).toHaveAttribute( + 'placeholder', + 'Defaults to the saved search or dashboard tile name', + ); + }); + + it('emits the typed name', async () => { + renderWithMantine(); + + fireEvent.change(screen.getByTestId('alert-display-name-input'), { + target: { value: 'Checkout 5xx' }, + }); + + expect((await submit()).displayName).toBe('Checkout 5xx'); + }); + + it('shows the schema error for a whitespace-only name and blocks submit', async () => { + renderWithMantine(); + + fireEvent.change(screen.getByTestId('alert-display-name-input'), { + target: { value: ' ' }, + }); + fireEvent.click(screen.getByText('Submit')); + + expect( + await screen.findByText(/at least 1 character/i), + ).toBeInTheDocument(); + expect(submitted).not.toHaveBeenCalled(); + }); + + it('shows the schema error for an over-long tag and blocks submit', async () => { + renderWithMantine( + , + ); + + fireEvent.click(screen.getByText('Submit')); + + expect(await screen.findByTestId('alert-tags-error')).toHaveTextContent( + /at most 32 character/i, + ); + expect(submitted).not.toHaveBeenCalled(); + }); + + it('emits an array once a tag is picked', async () => { + renderWithMantine(); + + fireEvent.click(screen.getByTestId('alert-tags-button')); + fireEvent.click(await screen.findByText('CHECKOUT')); + + expect((await submit()).tags).toEqual(['checkout']); + }); + + it('leaves untouched tags undefined so the server derives them', async () => { + renderWithMantine(); + + // An unset list inherits, so the button must not claim zero tags. + expect(screen.getByTestId('alert-tags-button')).toHaveTextContent( + 'Inherited', + ); + expect((await submit()).tags).toBeUndefined(); + }); + + it('counts the tags once the list is set', async () => { + renderWithMantine(); + + expect(screen.getByTestId('alert-tags-button')).toHaveTextContent('1'); + }); + + it('counts zero for a deliberately emptied list', async () => { + renderWithMantine(); + + const button = screen.getByTestId('alert-tags-button'); + expect(button).toHaveTextContent('0'); + expect(button).not.toHaveTextContent('Inherited'); + }); +}); diff --git a/packages/app/src/components/alerts/AlertCardList.tsx b/packages/app/src/components/alerts/AlertCardList.tsx index 75b96c1847..dd2cb9ad3c 100644 --- a/packages/app/src/components/alerts/AlertCardList.tsx +++ b/packages/app/src/components/alerts/AlertCardList.tsx @@ -13,7 +13,6 @@ import EmptyState from '@/components/EmptyState'; import { useVirtualList } from '@/hooks/useVirtualList'; import { APP_CONTENT_SCROLL_CONTAINER_ID } from '@/layout'; import type { AlertsPageItem } from '@/types'; -import { getAlertTags } from '@/utils/alerts'; import styles from '@styles/AlertsPage.module.scss'; @@ -56,7 +55,7 @@ function estimateItemHeight(item: AlertListItem): number { default: return ( ROW_HEIGHT + - (getAlertTags(item.alert).length > 0 ? TAGS_HEIGHT : 0) + + (item.alert.tags?.length > 0 ? TAGS_HEIGHT : 0) + (item.alert.note ? NOTE_TOGGLE_HEIGHT : 0) ); } diff --git a/packages/app/src/components/alerts/AlertDetailProperties.tsx b/packages/app/src/components/alerts/AlertDetailProperties.tsx index f8284990eb..587508a915 100644 --- a/packages/app/src/components/alerts/AlertDetailProperties.tsx +++ b/packages/app/src/components/alerts/AlertDetailProperties.tsx @@ -30,12 +30,12 @@ function PropertyRow({ /** * Full alert metadata block for the alert detail page: the shared one-line * summary (threshold, schedule, targets) plus every other persisted - * property — name, message template, group-by, schedule anchor/offset, - * acknowledgement, tags, and created/updated timestamps. Rows render only - * when the field is set. + * property — notification title, message template, group-by, schedule + * anchor/offset, acknowledgement, tags, and created/updated timestamps. Rows + * render only when the field is set. */ export function AlertDetailProperties({ alert }: { alert: AlertsPageItem }) { - const tags = alert.dashboard?.tags ?? alert.savedSearch?.tags ?? []; + const tags = alert.tags ?? []; const hasScheduleAnchor = alert.scheduleStartAt != null; const hasScheduleOffset = alert.scheduleOffsetMinutes != null && alert.scheduleOffsetMinutes > 0; @@ -45,7 +45,7 @@ export function AlertDetailProperties({ alert }: { alert: AlertsPageItem }) {
{alert.name && ( - + {alert.name} )} diff --git a/packages/app/src/components/alerts/AlertDetails.tsx b/packages/app/src/components/alerts/AlertDetails.tsx index 9e8a3f1cf9..89104d5520 100644 --- a/packages/app/src/components/alerts/AlertDetails.tsx +++ b/packages/app/src/components/alerts/AlertDetails.tsx @@ -16,7 +16,6 @@ import { useDisclosure } from '@mantine/hooks'; import { IconChartLine, IconChevronDown, - IconChevronRight, IconHelpCircle, IconNote, IconTableRow, @@ -28,12 +27,7 @@ import { AlertPropertiesSummary } from '@/components/alerts/AlertPropertiesSumma import { AlertRowMenu } from '@/components/alerts/AlertRowMenu'; import { IS_ALERT_DETAILS_ENABLED } from '@/config'; import type { AlertsPageItem } from '@/types'; -import { - getAlertDisplayName, - getAlertSourceLabel, - getAlertSourceUrl, - getAlertTags, -} from '@/utils/alerts'; +import { getAlertSourceLabel, getAlertSourceUrl } from '@/utils/alerts'; import styles from '@styles/AlertsPage.module.scss'; @@ -99,29 +93,7 @@ export const AlertDetails = React.memo(function AlertDetails({ }: { alert: AlertsPageItem; }) { - const alertName = React.useMemo(() => { - if (alert.source === AlertSource.TILE && alert.dashboard) { - const tile = alert.dashboard?.tiles.find( - tile => tile.id === alert.tileId, - ); - const tileName = tile?.config.name || 'Tile'; - return ( - <> - {alert.dashboard?.name} - {tileName ? ( - <> - - {tileName} - - ) : null} - - ); - } - if (alert.source === AlertSource.SAVED_SEARCH && alert.savedSearch) { - return alert.savedSearch?.name; - } - return '–'; - }, [alert]); + const alertName = alert.displayName || '–'; const alertUrl = React.useMemo(() => getAlertSourceUrl(alert), [alert]); @@ -213,9 +185,9 @@ export const AlertDetails = React.memo(function AlertDetails({ Created by {alert.createdBy.name || alert.createdBy.email} )} - {getAlertTags(alert).length > 0 && ( + {alert.tags?.length > 0 && ( - {getAlertTags(alert).map(tag => ( + {alert.tags.map(tag => ( {tag} @@ -238,7 +210,7 @@ export const AlertDetails = React.memo(function AlertDetails({ alert={alert} alertUrl={IS_ALERT_DETAILS_ENABLED ? alertUrl : undefined} linkTitle={linkTitle} - alertName={getAlertDisplayName(alert)} + alertName={alert.displayName} />
diff --git a/packages/app/src/components/alerts/AlertRowMenu.tsx b/packages/app/src/components/alerts/AlertRowMenu.tsx index cdca8aa5fe..a743af338f 100644 --- a/packages/app/src/components/alerts/AlertRowMenu.tsx +++ b/packages/app/src/components/alerts/AlertRowMenu.tsx @@ -29,9 +29,7 @@ type AlertRowMenuProps = { /** Label for that source, e.g. "Saved search". */ linkTitle?: string; /** - * Display name, for the delete confirmation and the menu's accessible - * label. Both `getAlertDisplayName` and `linkTitle` return an empty string - * for an alert whose source can't be resolved, so callers may pass one. + * Display name, for the delete confirmation and the menu's accessible label. */ alertName?: string; /** diff --git a/packages/app/src/components/alerts/EditAlertModal.tsx b/packages/app/src/components/alerts/EditAlertModal.tsx index e08b4569ca..cfaeaf1de6 100644 --- a/packages/app/src/components/alerts/EditAlertModal.tsx +++ b/packages/app/src/components/alerts/EditAlertModal.tsx @@ -5,8 +5,10 @@ import { zodResolver } from '@hookform/resolvers/zod'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { type Alert, + alertDisplayNameSchema, AlertIntervalSchema, AlertSource, + alertTagsSchema, AlertThresholdType, isRangeThresholdType, scheduleStartAtSchema, @@ -32,6 +34,7 @@ import { IconChartLine, IconInfoCircleFilled } from '@tabler/icons-react'; import { useQueryClient } from '@tanstack/react-query'; import api from '@/api'; +import { AlertDisplayFields } from '@/components/AlertDisplayFields'; import { AlertNoteField } from '@/components/AlertNoteField'; import { AlertChannelForm } from '@/components/Alerts'; import { AlertDetailChart } from '@/components/alerts/AlertDetailChart'; @@ -42,7 +45,7 @@ import { useSource } from '@/source'; import { useBrandDisplayName } from '@/theme/ThemeProvider'; import type { AlertsPageItem } from '@/types'; import { optionsToSelectData } from '@/utils'; -import { toAlertChannels } from '@/utils/alerts'; +import { getDerivedAlertDisplayName, toAlertChannels } from '@/utils/alerts'; import { ALERT_CHANNEL_OPTIONS, ALERT_INTERVAL_OPTIONS, @@ -66,6 +69,8 @@ const EditAlertFormSchema = z thresholdType: z.nativeEnum(AlertThresholdType), channel: zAlertChannel.optional(), channels: zAlertChannels.optional(), + displayName: alertDisplayNameSchema, + tags: alertTagsSchema, // nullish() (not optional()): persisted alerts store this as null, which // optional() would reject. numConsecutiveWindows: z.number().int().min(1).nullish(), @@ -106,6 +111,8 @@ function alertToFormValues(alert: AlertsPageItem): Alert { name: alert.name ?? null, message: alert.message ?? null, note: alert.note ?? null, + displayName: alert.displayName, + tags: alert.tags, // Persisted null -> undefined for the NumberInput. numConsecutiveWindows: alert.numConsecutiveWindows ?? undefined, }; @@ -380,6 +387,12 @@ export function EditAlertModal({ )} />
+ ({ const baseAlert = { _id: 'alert-1', + tags: [], interval: '5m', threshold: 3, thresholdType: AlertThresholdType.ABOVE, @@ -59,6 +60,7 @@ describe('AlertDetailProperties', () => { // Far future so the silence reads as active, not expired. until: '2099-01-01T00:00:00.000Z', }, + tags: ['prod', 'payments'], savedSearch: { _id: 'saved-search-id', name: 'My Search', @@ -72,7 +74,7 @@ describe('AlertDetailProperties', () => { ); expect(screen.getByTestId('alert-property-name')).toHaveTextContent( - 'CPU alert', + 'Notification titleCPU alert', ); expect(screen.getByTestId('alert-property-message')).toHaveTextContent( 'CPU is high on {{group}}', @@ -96,6 +98,33 @@ describe('AlertDetailProperties', () => { expect(screen.getByText(/Team Slack/)).toBeInTheDocument(); }); + it("renders the alert's own tags, not the saved search's", () => { + renderWithMantine( + , + ); + + expect(screen.getByTestId('alert-property-tags')).toHaveTextContent( + 'own-tag', + ); + expect(screen.getByTestId('alert-property-tags')).not.toHaveTextContent( + 'prod', + ); + }); + it('omits rows for unset fields and marks expired silences', () => { renderWithMantine( { expect(mutateAsync).toHaveBeenCalledWith('alert-1'); }); - // getAlertDisplayName and linkTitle both return '' for an alert whose source + // linkTitle returns '' for an alert whose source // can't be resolved, and '' is not nullish — so `??` would have produced // "Delete ?" and "Open ". it('falls back to generic wording when the name and source are empty', async () => { diff --git a/packages/app/src/components/alerts/__tests__/EditAlertModal.test.tsx b/packages/app/src/components/alerts/__tests__/EditAlertModal.test.tsx index c388164166..c7054865f1 100644 --- a/packages/app/src/components/alerts/__tests__/EditAlertModal.test.tsx +++ b/packages/app/src/components/alerts/__tests__/EditAlertModal.test.tsx @@ -19,6 +19,12 @@ jest.mock('@/api', () => ({ }), getAlertQueryKey: (alertId: string | undefined) => ['alert', alertId], getAlertsQueryKey: () => ['alerts'], + useTags: () => ({ + data: { data: ['checkout'] }, + isLoading: false, + isError: false, + refetch: jest.fn(), + }), }, })); @@ -159,6 +165,8 @@ describe('EditAlertModal', () => { // clears any that are omitted. name: 'My alert', message: 'My message template', + displayName: 'Checkout errors', + tags: ['checkout'], }), ); }); @@ -183,6 +191,34 @@ describe('EditAlertModal', () => { ); }); + it('sends an edited display name in the PUT payload', async () => { + renderModal(savedSearchAlert); + + fireEvent.change(screen.getByTestId('alert-display-name-input'), { + target: { value: 'Checkout 5xx' }, + }); + + await submitForm(); + + expect(updateAlertMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ displayName: 'Checkout 5xx' }), + ); + }); + + it('clears the display name to null so the server re-derives it', async () => { + renderModal(savedSearchAlert); + + fireEvent.change(screen.getByTestId('alert-display-name-input'), { + target: { value: '' }, + }); + + await submitForm(); + + expect(updateAlertMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ displayName: null }), + ); + }); + it('updates edited threshold values in the PUT payload', async () => { renderModal(savedSearchAlert); diff --git a/packages/app/src/utils/__tests__/alerts.test.ts b/packages/app/src/utils/__tests__/alerts.test.ts index 21389c0a31..d7a24095b4 100644 --- a/packages/app/src/utils/__tests__/alerts.test.ts +++ b/packages/app/src/utils/__tests__/alerts.test.ts @@ -1,7 +1,9 @@ import { AlertSource } from '@hyperdx/common-utils/dist/types'; +import type { AlertsPageItem } from '@/types'; import { getAlertSourceLabel, + getDerivedAlertDisplayName, normalizeNoOpAlertScheduleFields, toAlertChannels, } from '@/utils/alerts'; @@ -184,3 +186,54 @@ describe('getAlertSourceLabel', () => { expect(getAlertSourceLabel({ source: null })).toBe('Unknown source'); }); }); + +// Only the fields the two helpers read; `any` keeps these off the +// no-unsafe-type-assertion budget without spelling out the full page item. +const asAlert = (partial: any): AlertsPageItem => partial; + +describe('getDerivedAlertDisplayName', () => { + it('formats a tile alert like the server does', () => { + expect( + getDerivedAlertDisplayName( + asAlert({ + displayName: 'Custom', + source: AlertSource.TILE, + tileId: 'tile-1', + dashboard: { + name: 'Checkout', + tiles: [{ id: 'tile-1', config: { name: 'Error rate' } }], + }, + }), + ), + ).toBe('Checkout - Error rate'); + }); + + it('falls back to a generic tile name', () => { + expect( + getDerivedAlertDisplayName( + asAlert({ + source: AlertSource.TILE, + tileId: 'tile-1', + dashboard: { name: 'Checkout', tiles: [] }, + }), + ), + ).toBe('Checkout - Tile'); + }); + + it('uses the saved search name', () => { + expect( + getDerivedAlertDisplayName( + asAlert({ + source: AlertSource.SAVED_SEARCH, + savedSearch: { name: 'Checkout 5xx' }, + }), + ), + ).toBe('Checkout 5xx'); + }); + + it('is undefined when the source is not embedded', () => { + expect( + getDerivedAlertDisplayName(asAlert({ source: AlertSource.SAVED_SEARCH })), + ).toBeUndefined(); + }); +}); diff --git a/packages/app/src/utils/alerts.ts b/packages/app/src/utils/alerts.ts index 0416ebbb46..6f2399fcfa 100644 --- a/packages/app/src/utils/alerts.ts +++ b/packages/app/src/utils/alerts.ts @@ -6,6 +6,7 @@ import { } from 'date-fns'; import _ from 'lodash'; import { z } from 'zod'; +import { formatTileAlertDisplayName } from '@hyperdx/common-utils/dist/alerts'; import { Granularity } from '@hyperdx/common-utils/dist/core/utils'; import { ALERT_INTERVAL_TO_MINUTES, @@ -256,16 +257,21 @@ export function getAlertSourceLabel(alert: { } } -export function getAlertDisplayName(alert: AlertsPageItem): string { +/** + * The name the server would derive if the alert had none of its own. Only for + * previewing (e.g. the name input's placeholder); + */ +export function getDerivedAlertDisplayName( + alert: AlertsPageItem, +): string | undefined { if (alert.source === AlertSource.TILE && alert.dashboard) { const tile = alert.dashboard.tiles.find(t => t.id === alert.tileId); - const tileName = tile?.config.name || 'Tile'; - return `${alert.dashboard.name} ${tileName}`; + return formatTileAlertDisplayName(alert.dashboard.name, tile?.config.name); } if (alert.source === AlertSource.SAVED_SEARCH && alert.savedSearch) { return alert.savedSearch.name; } - return ''; + return undefined; } /** URL of the saved search / dashboard tile the alert is watching. */ @@ -279,10 +285,6 @@ export function getAlertSourceUrl(alert: AlertsPageItem): string { return ''; } -export function getAlertTags(alert: AlertsPageItem): string[] { - return alert.dashboard?.tags ?? alert.savedSearch?.tags ?? []; -} - export function getAlertCreatorLabel( alert: AlertsPageItem, ): string | undefined { diff --git a/packages/app/tests/e2e/components/ChartEditorComponent.ts b/packages/app/tests/e2e/components/ChartEditorComponent.ts index d5e044c166..d5f7055aca 100644 --- a/packages/app/tests/e2e/components/ChartEditorComponent.ts +++ b/packages/app/tests/e2e/components/ChartEditorComponent.ts @@ -11,6 +11,7 @@ import { replaceEditorText, } from '../utils/locators'; import { switchWhereToLucene } from '../utils/lucene-autocomplete'; +import { addTagViaPicker, removeTagViaPicker } from '../utils/tags'; import { WebhookAlertModalComponent } from './WebhookAlertModalComponent'; @@ -798,8 +799,8 @@ export class ChartEditorComponent { */ async save() { await this.saveButton.click(); - // Wait for save button to disappear (modal closes) - await this.saveButton.waitFor({ state: 'hidden', timeout: 2000 }); + // The modal closes once the dashboard mutation resolves. + await this.saveButton.waitFor({ state: 'hidden', timeout: 10000 }); } /** @@ -922,6 +923,28 @@ export class ChartEditorComponent { await input.blur(); } + /** Set the alert's own display name in the tile alert editor. */ + async setTileAlertDisplayName(name: string) { + await this.page + .getByTestId('alert-details') + .getByTestId('alert-display-name-input') + .fill(name); + } + + private get tileAlertTagsButton() { + return this.page + .getByTestId('alert-details') + .getByTestId('alert-tags-button'); + } + + async addTileAlertTag(tag: string) { + await addTagViaPicker(this.page, this.tileAlertTagsButton, tag); + } + + async removeTileAlertTag(tag: string) { + await removeTagViaPicker(this.page, this.tileAlertTagsButton, tag); + } + /** * Set the note field in the tile alert editor. */ diff --git a/packages/app/tests/e2e/components/SearchPageAlertModalComponent.ts b/packages/app/tests/e2e/components/SearchPageAlertModalComponent.ts index 0432a7c31e..8ef7591f3f 100644 --- a/packages/app/tests/e2e/components/SearchPageAlertModalComponent.ts +++ b/packages/app/tests/e2e/components/SearchPageAlertModalComponent.ts @@ -1,5 +1,7 @@ import { Locator, Page } from '@playwright/test'; +import { addTagViaPicker, removeTagViaPicker } from '../utils/tags'; + import { WebhookAlertModalComponent } from './WebhookAlertModalComponent'; /** @@ -116,6 +118,26 @@ export class SearchPageAlertModalComponent { }); } + /** + * Set the alert's display name. Empty clears it, which makes the server + * re-derive it from the saved search. + */ + async setDisplayName(name: string) { + await this.modal.getByTestId('alert-display-name-input').fill(name); + } + + private get tagsButton() { + return this.modal.getByTestId('alert-tags-button'); + } + + async addTag(tag: string) { + await addTagViaPicker(this.page, this.tagsButton, tag); + } + + async removeTag(tag: string) { + await removeTagViaPicker(this.page, this.tagsButton, tag); + } + /** * Set the note field in the alert form. */ diff --git a/packages/app/tests/e2e/features/alerts.spec.ts b/packages/app/tests/e2e/features/alerts.spec.ts index ec91703dbf..b711097b9b 100644 --- a/packages/app/tests/e2e/features/alerts.spec.ts +++ b/packages/app/tests/e2e/features/alerts.spec.ts @@ -75,6 +75,117 @@ test.describe('Alert Creation', { tag: ['@alerts', '@full-stack'] }, () => { }, ); + test( + 'should create and update a saved-search alert with a custom display name and tags', + { tag: '@full-stack' }, + async () => { + // Two round trips through the alert detail page, which dev mode compiles + // on first hit. + test.setTimeout(120000); + const ts = Date.now(); + const savedSearchName = `E2E Named Alert Search ${ts}`; + const displayName = `E2E Custom Alert Name ${ts}`; + const tag = `e2e-tag-${ts}`; + const updatedDisplayName = `E2E Renamed Alert ${ts}`; + const updatedTag = `e2e-renamed-tag-${ts}`; + const webhookName = `E2E Webhook Named ${ts}`; + const webhookUrl = `https://example.com/named-${ts}`; + + await test.step('Create a saved search', async () => { + await searchPage.goto(); + await searchPage.openSaveSearchModal(); + await searchPage.savedSearchModal.saveSearchAndWaitForNavigation( + savedSearchName, + ); + }); + + await test.step('Open the alerts modal from the saved search page', async () => { + await expect(searchPage.alertsButton).toBeVisible(); + await searchPage.openAlertsModal(); + await expect(searchPage.alertModal.addNewWebhookButton).toBeVisible(); + }); + + await test.step('Set a custom display name and tag', async () => { + await searchPage.alertModal.setDisplayName(displayName); + await searchPage.alertModal.addTag(tag); + }); + + await test.step('Create a new incoming webhook for the alert channel', async () => { + await searchPage.alertModal.addWebhookAndWait( + 'Generic', + webhookName, + webhookUrl, + ); + }); + + await test.step('Create the alert (webhook is auto-selected after creation)', async () => { + await searchPage.alertModal.createAlert(); + }); + + await test.step('The alerts page finds it by its custom name and tag', async () => { + await alertsPage.goto(); + await expect(alertsPage.pageContainer).toBeVisible(); + await alertsPage.filterToAlert(displayName); + + const card = alertsPage.getAlertCardByName(displayName); + await expect(card).toBeVisible({ timeout: 10000 }); + await expect(card.getByText(tag)).toBeVisible(); + }); + + await test.step('The saved search name no longer surfaces it', async () => { + await alertsPage.searchByName(savedSearchName); + await expect( + alertsPage.getAlertCardByName(savedSearchName), + ).toHaveCount(0); + }); + + await test.step('The detail page shows the custom name and tag', async () => { + await alertsPage.filterToAlert(displayName); + await alertsPage.openDetails( + alertsPage.getAlertCardByName(displayName), + ); + await expect(alertsPage.detailName).toHaveText(displayName); + await expect(alertsPage.detailTags).toContainText(tag); + }); + + await test.step('Rename and retag the alert from the saved search page', async () => { + await alertsPage.detailSourceLink.click(); + await alertsPage.page.waitForURL(/\/search\/[a-f0-9]{24}/); + await expect(searchPage.alertsButton).toBeVisible(); + await searchPage.openAlertsModal(); + await searchPage.alertModal.selectExistingAlertTab(0); + await searchPage.alertModal.setDisplayName(updatedDisplayName); + await searchPage.alertModal.removeTag(tag); + await searchPage.alertModal.addTag(updatedTag); + await searchPage.alertModal.saveAlert(); + }); + + await test.step('The alerts page shows the updated name and tag', async () => { + await alertsPage.goto(); + await expect(alertsPage.pageContainer).toBeVisible(); + await alertsPage.filterToAlert(updatedDisplayName); + + const card = alertsPage.getAlertCardByName(updatedDisplayName); + await expect(card).toBeVisible({ timeout: 10000 }); + await expect(card.getByText(updatedTag)).toBeVisible(); + await expect(card.getByText(tag, { exact: true })).toHaveCount(0); + + await alertsPage.searchByName(displayName); + await expect(alertsPage.getAlertCardByName(displayName)).toHaveCount(0); + }); + + await test.step('The detail page shows the updated name and tag', async () => { + await alertsPage.filterToAlert(updatedDisplayName); + await alertsPage.openDetails( + alertsPage.getAlertCardByName(updatedDisplayName), + ); + await expect(alertsPage.detailName).toHaveText(updatedDisplayName); + await expect(alertsPage.detailTags).toContainText(updatedTag); + await expect(alertsPage.detailTags).not.toContainText(tag); + }); + }, + ); + test( 'should create an alert from a dashboard tile and verify on the alerts page', { tag: '@full-stack' }, @@ -140,6 +251,126 @@ test.describe('Alert Creation', { tag: ['@alerts', '@full-stack'] }, () => { }, ); + test( + 'should create and update a dashboard tile alert with a custom display name and tags', + { tag: '@full-stack' }, + async ({ page }) => { + test.setTimeout(120000); + const ts = Date.now(); + const tileName = `E2E Named Alert Tile ${ts}`; + const displayName = `E2E Custom Tile Alert ${ts}`; + const tag = `e2e-tile-tag-${ts}`; + const updatedDisplayName = `E2E Renamed Tile Alert ${ts}`; + const updatedTag = `e2e-tile-retag-${ts}`; + const webhookName = `E2E Webhook Named Tile ${ts}`; + const webhookUrl = `https://example.com/named-tile-${ts}`; + + await test.step('Create a new dashboard', async () => { + await dashboardPage.goto(); + await dashboardPage.createNewDashboard(); + }); + + await test.step('Add a tile to the dashboard', async () => { + await dashboardPage.addTile(); + await expect(dashboardPage.chartEditor.nameInput).toBeVisible(); + await dashboardPage.chartEditor.waitForDataToLoad(); + await dashboardPage.chartEditor.setChartName(tileName); + await dashboardPage.chartEditor.runQuery(); + }); + + await test.step('Enable an alert with a custom display name and tag', async () => { + await expect(dashboardPage.chartEditor.alertButton).toBeVisible(); + await dashboardPage.chartEditor.clickAddAlert(); + await expect( + dashboardPage.chartEditor.addNewWebhookButton, + ).toBeVisible(); + await dashboardPage.chartEditor.setTileAlertDisplayName(displayName); + await dashboardPage.chartEditor.addTileAlertTag(tag); + await dashboardPage.chartEditor.addNewWebhookButton.click(); + await expect(page.getByTestId('webhook-name-input')).toBeVisible(); + await dashboardPage.chartEditor.webhookAlertModal.addWebhook( + 'Generic', + webhookName, + webhookUrl, + ); + await expect(page.getByTestId('alert-modal')).toBeHidden(); + }); + + await test.step('Save the tile with the alert configured', async () => { + await dashboardPage.chartEditor.save(); + await expect(dashboardPage.getTiles()).toHaveCount(1, { + timeout: 10000, + }); + }); + + await test.step('The alerts page finds it by its custom name and tag', async () => { + await alertsPage.goto(); + await expect(alertsPage.pageContainer).toBeVisible(); + await alertsPage.filterToAlert(displayName); + + const card = alertsPage.getAlertCardByName(displayName); + await expect(card).toBeVisible({ timeout: 10000 }); + await expect(card.getByText(tag)).toBeVisible(); + + await alertsPage.searchByName(tileName); + await expect(alertsPage.getAlertCardByName(tileName)).toHaveCount(0); + }); + + await test.step('The detail page shows the custom name and tag', async () => { + await alertsPage.filterToAlert(displayName); + await alertsPage.openDetails( + alertsPage.getAlertCardByName(displayName), + ); + await expect(alertsPage.detailName).toHaveText(displayName); + await expect(alertsPage.detailTags).toContainText(tag); + }); + + await test.step('Rename and retag the alert from the tile editor', async () => { + await alertsPage.detailSourceLink.click(); + await page.waitForURL(/\/dashboards\/[a-f0-9]{24}/); + await expect(dashboardPage.getTiles()).toHaveCount(1, { + timeout: 10000, + }); + await dashboardPage.editTile(0); + await expect(dashboardPage.chartEditor.nameInput).toBeVisible(); + await dashboardPage.chartEditor.setTileAlertDisplayName( + updatedDisplayName, + ); + await dashboardPage.chartEditor.removeTileAlertTag(tag); + await dashboardPage.chartEditor.addTileAlertTag(updatedTag); + // The tile save issues a fire-and-forget PATCH; navigating away before + // it lands would drop the update. + const patch = dashboardPage.waitForDashboardPatch(); + await dashboardPage.chartEditor.save(); + await patch; + }); + + await test.step('The alerts page shows the updated name and tag', async () => { + await alertsPage.goto(); + await expect(alertsPage.pageContainer).toBeVisible(); + await alertsPage.filterToAlert(updatedDisplayName); + + const card = alertsPage.getAlertCardByName(updatedDisplayName); + await expect(card).toBeVisible({ timeout: 10000 }); + await expect(card.getByText(updatedTag)).toBeVisible(); + await expect(card.getByText(tag, { exact: true })).toHaveCount(0); + + await alertsPage.searchByName(displayName); + await expect(alertsPage.getAlertCardByName(displayName)).toHaveCount(0); + }); + + await test.step('The detail page shows the updated name and tag', async () => { + await alertsPage.filterToAlert(updatedDisplayName); + await alertsPage.openDetails( + alertsPage.getAlertCardByName(updatedDisplayName), + ); + await expect(alertsPage.detailName).toHaveText(updatedDisplayName); + await expect(alertsPage.detailTags).toContainText(updatedTag); + await expect(alertsPage.detailTags).not.toContainText(tag); + }); + }, + ); + test( 'should create an alert from a raw SQL dashboard tile and verify on the alerts page', { tag: '@full-stack' }, diff --git a/packages/app/tests/e2e/page-objects/AlertsPage.ts b/packages/app/tests/e2e/page-objects/AlertsPage.ts index e265965356..a002fcfe8f 100644 --- a/packages/app/tests/e2e/page-objects/AlertsPage.ts +++ b/packages/app/tests/e2e/page-objects/AlertsPage.ts @@ -183,6 +183,34 @@ export class AlertsPage { return this.page.locator('[data-testid="alert-detail-page"]'); } + /** The alert's name in the detail page header. */ + get detailName() { + return this.page.getByTestId('alert-detail-name'); + } + + /** The Tags row in the detail page properties. Absent when there are none. */ + get detailTags() { + return this.page.getByTestId('alert-property-tags'); + } + + /** Link on the detail page to the saved search / dashboard tile watched. */ + get detailSourceLink() { + return this.page.getByTestId('open-alert-source'); + } + + /** + * Follow a card's name link to /alerts/:id and wait for the page to render. + * Generous timeouts: in dev mode the first hit compiles the route. + */ + async openDetails(alertCard: Locator) { + await this.getDetailsLinkForAlertCard(alertCard).click(); + await this.page.waitForURL(/\/alerts\/[a-f0-9]{24}/, { timeout: 60000 }); + await this.detailPageContainer.waitFor({ + state: 'visible', + timeout: 15000, + }); + } + /** * The evaluation event-stream table on the alert detail page. */ diff --git a/packages/app/tests/e2e/utils/tags.ts b/packages/app/tests/e2e/utils/tags.ts new file mode 100644 index 0000000000..f11ef4c3ce --- /dev/null +++ b/packages/app/tests/e2e/utils/tags.ts @@ -0,0 +1,52 @@ +import { expect, Locator, Page } from '@playwright/test'; + +/** + * Drive the shared Tags popover (components/Tags.tsx) from its trigger button. + * The dropdown is portalled, so its search input is looked up on the page. + * Closing goes through the trigger rather than Escape, which would bubble to + * an enclosing modal and close that too. + */ +async function withTagPicker( + page: Page, + trigger: Locator, + fn: (search: Locator) => Promise, +) { + await trigger.click(); + const search = page.getByPlaceholder(/Search (or create )?tag/); + await search.waitFor({ state: 'visible' }); + await fn(search); + await trigger.click(); + await search.waitFor({ state: 'detached' }); +} + +/** Type a new tag and press Enter to create and select it. */ +export async function addTagViaPicker( + page: Page, + trigger: Locator, + tag: string, +) { + await withTagPicker(page, trigger, async search => { + await search.fill(tag); + await search.press('Enter'); + }); +} + +/** + * Deselect a tag. Labels render uppercased. Plain click rather than + * `uncheck()`: a tag known only to this picker leaves the list the moment it + * is deselected, and `uncheck()` never gets to confirm the new state. + */ +export async function removeTagViaPicker( + page: Page, + trigger: Locator, + tag: string, +) { + await withTagPicker(page, trigger, async search => { + await search.fill(tag); + const name = tag.toUpperCase(); + await page.getByRole('checkbox', { name }).click(); + await expect( + page.getByRole('checkbox', { name, checked: true }), + ).toHaveCount(0); + }); +} diff --git a/packages/cli/src/api/client.ts b/packages/cli/src/api/client.ts index d519d40a33..fa39266c94 100644 --- a/packages/cli/src/api/client.ts +++ b/packages/cli/src/api/client.ts @@ -551,6 +551,8 @@ export interface AlertItem { tileId?: string; name?: string | null; message?: string | null; + displayName?: string; + tags?: string[]; createdAt: string; updatedAt: string; history: AlertHistoryItem[]; diff --git a/packages/cli/src/components/AlertsPage.tsx b/packages/cli/src/components/AlertsPage.tsx index aca3482ed6..97f528cd07 100644 --- a/packages/cli/src/components/AlertsPage.tsx +++ b/packages/cli/src/components/AlertsPage.tsx @@ -39,7 +39,7 @@ function stateLabel(state?: string): string { } function alertName(alert: AlertItem): string { - if (alert.name) return alert.name; + if (alert.displayName) return alert.displayName; if (alert.dashboard) { const tile = alert.dashboard.tiles.find(t => t.id === alert.tileId); const tileName = tile?.config.name ?? alert.tileId ?? ''; diff --git a/packages/common-utils/src/__tests__/alerts.test.ts b/packages/common-utils/src/__tests__/alerts.test.ts new file mode 100644 index 0000000000..88cc1bb561 --- /dev/null +++ b/packages/common-utils/src/__tests__/alerts.test.ts @@ -0,0 +1,45 @@ +import { + clampAlertDisplayName, + clampAlertTags, + formatTileAlertDisplayName, +} from '@/alerts'; +import { + MAX_ALERT_DISPLAY_NAME_LENGTH, + MAX_TAG_LENGTH, + MAX_TAGS, +} from '@/types'; + +describe('formatTileAlertDisplayName', () => { + it('joins the dashboard and tile names', () => { + expect(formatTileAlertDisplayName('Checkout', 'Error rate')).toBe( + 'Checkout - Error rate', + ); + }); + + it('falls back for a blank tile name', () => { + expect(formatTileAlertDisplayName('Checkout', ' ')).toBe( + 'Checkout - Tile', + ); + }); +}); + +describe('clampAlertDisplayName', () => { + it('truncates to the alert cap', () => { + expect( + clampAlertDisplayName('x'.repeat(MAX_ALERT_DISPLAY_NAME_LENGTH + 5)), + ).toHaveLength(MAX_ALERT_DISPLAY_NAME_LENGTH); + }); +}); + +describe('clampAlertTags', () => { + it('drops empties, truncates each tag, and caps the count', () => { + const tags = [ + '', + 'a'.repeat(MAX_TAG_LENGTH + 1), + ...Array(MAX_TAGS).fill('t'), + ]; + const clamped = clampAlertTags(tags); + expect(clamped).toHaveLength(MAX_TAGS); + expect(clamped[0]).toBe('a'.repeat(MAX_TAG_LENGTH)); + }); +}); diff --git a/packages/common-utils/src/alerts.ts b/packages/common-utils/src/alerts.ts new file mode 100644 index 0000000000..b2144c4289 --- /dev/null +++ b/packages/common-utils/src/alerts.ts @@ -0,0 +1,29 @@ +import { + MAX_ALERT_DISPLAY_NAME_LENGTH, + MAX_TAG_LENGTH, + MAX_TAGS, +} from './types'; + +/** Display name derived for a dashboard tile alert with no explicit displayName. */ +export function formatTileAlertDisplayName( + dashboardName: string, + tileName?: string | null, +): string { + return `${dashboardName} - ${tileName?.trim() || 'Tile'}`; +} + +/** + * Alerts cap displayName and tags (alertDisplayNameSchema / alertTagsSchema), + * but the saved searches and dashboards they inherit from cap neither. Anything + * copied across has to be clamped first or the alert form and API reject it. + */ +export function clampAlertDisplayName(name: string): string { + return name.slice(0, MAX_ALERT_DISPLAY_NAME_LENGTH); +} + +export function clampAlertTags(tags: readonly string[]): string[] { + return tags + .filter(tag => tag !== '') + .slice(0, MAX_TAGS) + .map(tag => tag.slice(0, MAX_TAG_LENGTH)); +}