Skip to content

Commit fb28446

Browse files
feat(app): edit multiple notification channels on alerts (#2848)
1 parent d11f857 commit fb28446

13 files changed

Lines changed: 566 additions & 117 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@hyperdx/app': minor
3+
---
4+
5+
Alert forms for saved searches and dashboard tiles can now send to several webhooks. Add or remove notification channels inline (up to 10); webhooks already chosen by the alert are greyed out in the other pickers, since duplicates are rejected.

‎packages/app/src/DBSearchPageAlertModal.tsx‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
SearchConditionLanguage,
1717
validateAlertScheduleOffsetMinutes,
1818
validateAlertThresholdMax,
19-
zAlertChannel,
19+
zAlertChannels,
2020
} from '@hyperdx/common-utils/dist/types';
2121
import {
2222
Accordion,
@@ -53,6 +53,7 @@ import {
5353
ALERT_THRESHOLD_TYPE_OPTIONS,
5454
intervalToMinutes,
5555
normalizeNoOpAlertScheduleFields,
56+
toAlertChannels,
5657
} from '@/utils/alerts';
5758

5859
import { AlertNoteField } from './components/AlertNoteField';
@@ -76,7 +77,7 @@ const SavedSearchAlertFormSchema = z
7677
scheduleOffsetMinutes: z.number().int().min(0).default(0),
7778
scheduleStartAt: scheduleStartAtSchema,
7879
thresholdType: z.nativeEnum(AlertThresholdType),
79-
channel: zAlertChannel,
80+
channels: zAlertChannels,
8081
// nullish() (not optional()): persisted alerts store this as null, which
8182
// optional() would reject.
8283
numConsecutiveWindows: z.number().int().min(1).nullish(),
@@ -123,6 +124,7 @@ const AlertForm = ({
123124
defaultValues: defaultValues
124125
? {
125126
...defaultValues,
127+
channels: toAlertChannels(defaultValues),
126128
scheduleOffsetMinutes: defaultValues.scheduleOffsetMinutes ?? 0,
127129
scheduleStartAt: defaultValues.scheduleStartAt ?? null,
128130
// Persisted null -> undefined for the NumberInput.
@@ -136,18 +138,14 @@ const AlertForm = ({
136138
scheduleStartAt: null,
137139
thresholdType: AlertThresholdType.ABOVE,
138140
source: AlertSource.SAVED_SEARCH,
139-
channel: {
140-
type: 'webhook',
141-
webhookId: '',
142-
},
141+
channels: [{ type: 'webhook', webhookId: '' }],
143142
note: null,
144143
},
145144
resolver: zodResolver(SavedSearchAlertFormSchema),
146145
});
147146

148147
const groupBy = useWatch({ control, name: 'groupBy' });
149148
const thresholdType = useWatch({ control, name: 'thresholdType' });
150-
const channelType = useWatch({ control, name: 'channel.type' });
151149
const interval = useWatch({ control, name: 'interval' });
152150
const scheduleOffsetMinutes = useWatch({
153151
control,
@@ -178,7 +176,10 @@ const AlertForm = ({
178176
// this form was opened with. This binds the submission to the
179177
// originally-edited alert rather than whatever the parent's alerts
180178
// list happens to index at submit time.
181-
{ ...data, id: defaultValues?.id },
179+
// `channel` is cleared (JSON drops undefined): the API derives it
180+
// from channels[0], and echoing a stale value back would conflict
181+
// with an edited list.
182+
{ ...data, channel: undefined, id: defaultValues?.id },
182183
defaultValues,
183184
{
184185
preserveExplicitScheduleOffsetMinutes:
@@ -264,7 +265,7 @@ const AlertForm = ({
264265
</Text>
265266
<Controller
266267
control={control}
267-
name="channel.type"
268+
name="channels.0.type"
268269
render={({ field }) => (
269270
<NativeSelect
270271
data={optionsToSelectData(ALERT_CHANNEL_OPTIONS)}
@@ -299,7 +300,7 @@ const AlertForm = ({
299300
<Text size="xxs" opacity={0.5} mb={4}>
300301
Send to
301302
</Text>
302-
<AlertChannelForm control={control} type={channelType} />
303+
<AlertChannelForm control={control} channelsName="channels" />
303304
<AlertNoteField control={control} name="note" />
304305
{groupBy &&
305306
(thresholdType === AlertThresholdType.BELOW ||

‎packages/app/src/__tests__/DBSearchPageAlertModal.test.tsx‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,10 @@ describe('DBSearchPageAlertModal', () => {
134134
const alertTab = await screen.findByRole('tab', { name: /Alert 2/ });
135135
fireEvent.click(alertTab);
136136

137-
const saveButton = await screen.findByText('Save Alert');
138-
fireEvent.click(saveButton.closest('button') as HTMLButtonElement);
137+
const saveButton = await screen.findByRole('button', {
138+
name: 'Save Alert',
139+
});
140+
fireEvent.click(saveButton);
139141

140142
await waitFor(() => {
141143
expect(updateAlertMutateAsync).toHaveBeenCalledTimes(1);
@@ -151,4 +153,29 @@ describe('DBSearchPageAlertModal', () => {
151153
);
152154
expect(createAlertMutateAsync).not.toHaveBeenCalled();
153155
});
156+
157+
// The submitted payload must not carry the legacy singular `channel`: the API
158+
// rejects it when it disagrees with the edited channels list, and these
159+
// fixtures are legacy channel-only alerts.
160+
it('submits channels and drops the legacy channel field', async () => {
161+
renderModal();
162+
163+
const alertTab = await screen.findByRole('tab', { name: /Alert 2/ });
164+
fireEvent.click(alertTab);
165+
166+
const saveButton = await screen.findByRole('button', {
167+
name: 'Save Alert',
168+
});
169+
fireEvent.click(saveButton);
170+
171+
await waitFor(() => {
172+
expect(updateAlertMutateAsync).toHaveBeenCalledTimes(1);
173+
});
174+
175+
const payload = updateAlertMutateAsync.mock.calls[0][0];
176+
expect(payload.channel).toBeUndefined();
177+
expect(payload.channels).toEqual([
178+
{ type: 'webhook', webhookId: 'webhook-id' },
179+
]);
180+
});
154181
});

‎packages/app/src/components/Alerts.tsx‎

Lines changed: 105 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,108 +1,150 @@
11
import { useMemo } from 'react';
22
import {
3+
ArrayPath,
34
Control,
4-
Controller,
5+
FieldArray,
56
FieldValues,
67
Path,
7-
useController,
8+
useFieldArray,
9+
useWatch,
810
} from 'react-hook-form';
911
import { Label, ReferenceArea, ReferenceLine } from 'recharts';
1012
import {
11-
type AlertChannelType,
1213
AlertThresholdType,
14+
MAX_ALERT_CHANNELS,
1315
WebhookService,
1416
} from '@hyperdx/common-utils/dist/types';
15-
import { Button, ComboboxData, Group, Modal, Select } from '@mantine/core';
17+
import { ActionIcon, Button, Group, Modal, Stack, Text } from '@mantine/core';
1618
import { useDisclosure } from '@mantine/hooks';
19+
import { IconPlus, IconTrash, IconWebhook } from '@tabler/icons-react';
1720

1821
import api from '@/api';
22+
import { WebhookChannelForm } from '@/components/alerts/WebhookChannelForm';
1923
import { WebhookForm } from '@/components/TeamSettings/WebhookForm';
2024

21-
type Webhook = {
22-
_id: string;
23-
name: string;
24-
};
25-
26-
const WebhookChannelForm = <T extends FieldValues>({
27-
control,
28-
name,
29-
}: {
30-
control?: Control<T>;
31-
name?: string;
32-
}) => {
33-
const { data: webhooks, refetch: refetchWebhooks } = api.useWebhooks([
25+
const useAlertWebhooks = () =>
26+
api.useWebhooks([
3427
WebhookService.Slack,
3528
WebhookService.Generic,
3629
WebhookService.IncidentIO,
3730
]);
38-
const [opened, { open, close }] = useDisclosure(false);
39-
40-
const hasWebhooks = Array.isArray(webhooks?.data) && webhooks.data.length > 0;
41-
42-
const options = useMemo<ComboboxData>(() => {
43-
const webhookOptions =
44-
webhooks?.data.map((sw: Webhook) => ({
45-
value: sw._id,
46-
label: sw.name,
47-
})) || [];
4831

49-
return [
50-
{
51-
value: '',
52-
label: 'Select a Webhook',
53-
disabled: true,
54-
},
55-
...webhookOptions,
56-
];
57-
}, [webhooks]);
32+
// react-hook-form's `FieldArray<T, ArrayPath<T>>` depends on the caller's
33+
// generic form type, which TypeScript can't verify a plain object literal
34+
// against here. Centralized so that unavoidable assertion exists once
35+
// instead of at every call site.
36+
function makeWebhookChannel<T extends FieldValues>(
37+
webhookId: string,
38+
): FieldArray<T, ArrayPath<T>> {
39+
return { type: 'webhook', webhookId } as FieldArray<T, ArrayPath<T>>;
40+
}
5841

59-
const { field } = useController({
42+
export const AlertChannelForm = <T extends FieldValues>({
43+
control,
44+
channelsName,
45+
}: {
46+
control: Control<T>;
47+
/** Path of the alert's channels array, e.g. "channels" or "alert.channels". */
48+
channelsName: ArrayPath<T> & Path<T>;
49+
}) => {
50+
const { fields, append, remove, update } = useFieldArray<T>({
6051
control,
61-
name: name! as Path<T>,
52+
name: channelsName,
6253
});
54+
// `fields` holds the values from the last render, so watch for the live ones
55+
// the duplicate check (and the per-row type discriminant below) need.
56+
const channels = useWatch({ control, name: channelsName }) as
57+
| { type?: string | null; webhookId?: string }[]
58+
| undefined;
59+
const { refetch: refetchWebhooks } = useAlertWebhooks();
60+
const [opened, { open, close }] = useDisclosure(false);
61+
62+
const selectedWebhookIds = useMemo(
63+
() => (channels ?? []).map(c => c?.webhookId ?? ''),
64+
[channels],
65+
);
6366

67+
const newChannel = () => makeWebhookChannel<T>('');
68+
69+
// A webhook created from here lands in the first empty row, or a new one if
70+
// every row is already filled — so the user never has to re-pick it.
6471
const handleWebhookCreated = async (webhookId?: string) => {
6572
await refetchWebhooks();
6673
if (webhookId) {
67-
field.onChange(webhookId);
68-
field.onBlur();
74+
const emptyIndex = selectedWebhookIds.findIndex(id => !id);
75+
const value = makeWebhookChannel<T>(webhookId);
76+
if (emptyIndex >= 0) {
77+
update(emptyIndex, value);
78+
} else if (fields.length < MAX_ALERT_CHANNELS) {
79+
append(value);
80+
}
6981
}
7082
close();
7183
};
7284

7385
return (
74-
<div>
75-
<Group gap="md" justify="space-between" align="flex-start">
76-
<Controller
77-
control={control}
78-
name={name! as Path<T>}
79-
render={({ field, fieldState }) => (
80-
<Select
81-
data-testid="select-webhook"
82-
comboboxProps={{
83-
withinPortal: false,
84-
}}
85-
required
86-
size="xs"
87-
flex={1}
88-
placeholder={
89-
hasWebhooks ? 'Select a Webhook' : 'No Webhooks available'
90-
}
91-
data={options}
92-
{...field}
93-
error={fieldState.error?.message}
86+
<Stack gap="xs">
87+
{fields.map((field, index) => {
88+
// The discriminant is per-row, not per-array: a downstream fork can
89+
// mix channel types (e.g. webhook + email) in one alert. A row whose
90+
// type this repo doesn't render (anything but webhook) is skipped so
91+
// the rest of the list still works.
92+
const channelType = channels?.[index]?.type;
93+
if (channelType !== 'webhook') {
94+
return null;
95+
}
96+
return (
97+
<Group key={field.id} gap="md" align="flex-start" wrap="nowrap">
98+
<WebhookChannelForm
99+
control={control}
100+
namePrefix={`${channelsName}.${index}.`}
101+
takenWebhookIds={selectedWebhookIds.filter(
102+
(id, i) => i !== index && !!id,
103+
)}
94104
/>
95-
)}
96-
/>
105+
{/* A single channel is not removable: an alert with no target
106+
would fire into the void, and the API rejects it anyway. */}
107+
{fields.length > 1 && (
108+
<ActionIcon
109+
data-testid="remove-webhook-channel-button"
110+
aria-label="Remove notification channel"
111+
size="md"
112+
variant="danger"
113+
onClick={() => remove(index)}
114+
>
115+
<IconTrash size={14} />
116+
</ActionIcon>
117+
)}
118+
</Group>
119+
);
120+
})}
121+
<Group gap="xs">
122+
<Button
123+
data-testid="add-alert-channel-button"
124+
size="xs"
125+
variant="subtle"
126+
color="gray"
127+
leftSection={<IconPlus size={14} />}
128+
disabled={fields.length >= MAX_ALERT_CHANNELS}
129+
onClick={() => append(newChannel())}
130+
>
131+
Add another channel
132+
</Button>
97133
<Button
98134
data-testid="add-new-webhook-button"
99135
size="xs"
100136
variant="subtle"
101137
color="gray"
138+
leftSection={<IconWebhook size={14} />}
102139
onClick={open}
103140
>
104141
Add New Incoming Webhook
105142
</Button>
143+
{fields.length >= MAX_ALERT_CHANNELS && (
144+
<Text size="xs" opacity={0.5}>
145+
Limit of {MAX_ALERT_CHANNELS} channels reached
146+
</Text>
147+
)}
106148
</Group>
107149

108150
<Modal
@@ -116,31 +158,10 @@ const WebhookChannelForm = <T extends FieldValues>({
116158
>
117159
<WebhookForm onClose={close} onSuccess={handleWebhookCreated} />
118160
</Modal>
119-
</div>
161+
</Stack>
120162
);
121163
};
122164

123-
export const AlertChannelForm = <T extends FieldValues>({
124-
control,
125-
type,
126-
namePrefix = '',
127-
}: {
128-
control: Control<T>;
129-
type: AlertChannelType;
130-
namePrefix?: string;
131-
}) => {
132-
if (type === 'webhook') {
133-
return (
134-
<WebhookChannelForm
135-
control={control}
136-
name={`${namePrefix}channel.webhookId`}
137-
/>
138-
);
139-
}
140-
141-
return null;
142-
};
143-
144165
export const getAlertReferenceLines = ({
145166
thresholdType,
146167
threshold,

0 commit comments

Comments
 (0)