diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index ba2365e5a..b2327502a 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -246,10 +246,14 @@ config transport, defaults, and save/load orchestration, `frontend-modern/src/features/alerts/useAlertOverridesState.ts` for raw override normalization plus resource-backed override projection, and `frontend-modern/src/features/alerts/useAlertDestinationsState.ts` for -notification destination reload and persistence. Future config cleanup should -extend the config transport hook, the override-projection hook, or the -destinations hook based on which subsystem actually owns the behavior instead -of letting the broader configuration hook absorb all three concerns again. +notification destination reload and persistence. +`frontend-modern/src/features/alerts/useAlertDestinationsTabState.ts` now owns +webhook load/mutate/test flow plus destination test actions, while +`frontend-modern/src/features/alerts/tabs/DestinationsTab.tsx` stays the +destinations render shell. Future config cleanup should extend the config +transport hook, the override-projection hook, or the destinations runtime hook +based on which subsystem actually owns the behavior instead of letting the +broader configuration hook absorb all three concerns again. Alert filter metadata and grouped header consumers must also preserve the canonical `agent` and `node` header boundary when reusing shared filter diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 226d0ef22..9b75245d8 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -447,9 +447,13 @@ is the feature shell. The canonical runtime owner is now config transport, `frontend-modern/src/features/alerts/useAlertOverridesState.ts` for override projection and thresholds-facing resource selectors, and `frontend-modern/src/features/alerts/useAlertDestinationsState.ts` for -notification destination reload and persistence. Future cleanup should extend -the transport hook, override hook, or destinations hook based on the true -owner, not move config control flow back into the top-level page shell. +notification destination reload and persistence. +`frontend-modern/src/features/alerts/useAlertDestinationsTabState.ts` now owns +webhook runtime and destination test actions while +`frontend-modern/src/features/alerts/tabs/DestinationsTab.tsx` stays the +render shell. Future cleanup should extend the transport hook, override hook, +or destinations runtime hook based on the true owner, not move config control +flow back into the top-level page shell. The same rule now also covers cross-tab incident timelines: the shared runtime owner is `frontend-modern/src/features/alerts/useAlertIncidentTimelineState.ts`, while `frontend-modern/src/features/alerts/OverviewTab.tsx` and diff --git a/frontend-modern/src/features/alerts/__tests__/useAlertDestinationsTabState.test.tsx b/frontend-modern/src/features/alerts/__tests__/useAlertDestinationsTabState.test.tsx new file mode 100644 index 000000000..ab0926f41 --- /dev/null +++ b/frontend-modern/src/features/alerts/__tests__/useAlertDestinationsTabState.test.tsx @@ -0,0 +1,199 @@ +import { renderHook, waitFor } from '@solidjs/testing-library'; +import { createSignal } from 'solid-js'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { NotificationsAPI } from '@/api/notifications'; +import { notificationStore } from '@/stores/notifications'; +import { showErrorWithDetail } from '@/utils/toast'; + +import { useAlertDestinationsTabState } from '../useAlertDestinationsTabState'; +import type { UIAppriseConfig, UIEmailConfig } from '../types'; + +vi.mock('@/api/notifications', () => ({ + NotificationsAPI: { + createWebhook: vi.fn(), + deleteWebhook: vi.fn(), + getWebhooks: vi.fn(), + testNotification: vi.fn(), + testWebhook: vi.fn(), + updateWebhook: vi.fn(), + }, +})); + +vi.mock('@/stores/notifications', () => ({ + notificationStore: { + error: vi.fn(), + success: vi.fn(), + }, +})); + +vi.mock('@/utils/logger', () => ({ + logger: { + error: vi.fn(), + }, +})); + +vi.mock('@/utils/toast', () => ({ + showErrorWithDetail: vi.fn(), +})); + +const buildEmailConfig = (): UIEmailConfig => ({ + enabled: true, + from: 'pulse@example.com', + maxRetries: 3, + password: '', + port: 587, + provider: 'smtp', + rateLimit: 60, + replyTo: '', + retryDelay: 5, + server: 'smtp.example.com', + startTLS: true, + tls: true, + to: ['alerts@example.com'], + username: 'ops@example.com', +}); + +const buildAppriseConfig = (): UIAppriseConfig => ({ + apiKey: '', + apiKeyHeader: 'X-API-KEY', + cliPath: '/usr/local/bin/apprise', + configKey: '', + enabled: true, + mode: 'cli', + serverUrl: '', + skipTlsVerify: false, + targetsText: 'mailto://alerts@example.com', + timeoutSeconds: 20, +}); + +describe('useAlertDestinationsTabState', () => { + beforeEach(() => { + vi.mocked(NotificationsAPI.createWebhook).mockReset(); + vi.mocked(NotificationsAPI.deleteWebhook).mockReset(); + vi.mocked(NotificationsAPI.getWebhooks).mockReset(); + vi.mocked(NotificationsAPI.testNotification).mockReset(); + vi.mocked(NotificationsAPI.testWebhook).mockReset(); + vi.mocked(NotificationsAPI.updateWebhook).mockReset(); + vi.mocked(notificationStore.error).mockReset(); + vi.mocked(notificationStore.success).mockReset(); + vi.mocked(showErrorWithDetail).mockReset(); + }); + + it('owns webhook runtime and destination test actions separately from config load/save state', async () => { + const [emailConfig] = createSignal(buildEmailConfig()); + const [appriseConfig, setAppriseConfig] = createSignal(buildAppriseConfig()); + const [configLoadError] = createSignal(null); + const [isRetrying] = createSignal(false); + const [isLoadingDestinations] = createSignal(false); + const onRetryLoad = vi.fn(); + + vi.mocked(NotificationsAPI.getWebhooks).mockResolvedValue([ + { + enabled: true, + headers: {}, + id: 'hook-1', + method: 'POST', + name: 'Ops', + url: 'https://hooks.example.test/ops', + }, + ] as never); + vi.mocked(NotificationsAPI.testNotification).mockResolvedValue({ success: true } as never); + vi.mocked(NotificationsAPI.testWebhook).mockResolvedValue({ success: true } as never); + vi.mocked(NotificationsAPI.createWebhook).mockResolvedValue({ + enabled: true, + headers: {}, + id: 'hook-2', + method: 'POST', + name: 'Pager', + service: 'slack', + url: 'https://hooks.example.test/pager', + } as never); + vi.mocked(NotificationsAPI.updateWebhook).mockResolvedValue({ + enabled: false, + headers: {}, + id: 'hook-2', + method: 'POST', + name: 'Pager Updated', + service: 'slack', + url: 'https://hooks.example.test/pager', + } as never); + vi.mocked(NotificationsAPI.deleteWebhook).mockResolvedValue({ success: true } as never); + + const { result } = renderHook(() => + useAlertDestinationsTabState({ + appriseConfig, + configLoadError, + emailConfig, + isLoadingDestinations, + isRetrying, + onRetryLoad, + setAppriseConfig, + }), + ); + + await waitFor(() => expect(NotificationsAPI.getWebhooks).toHaveBeenCalledTimes(1)); + expect(result.webhooks()).toEqual([ + expect.objectContaining({ id: 'hook-1', service: 'generic' }), + ]); + + await result.testEmailConfig(); + expect(NotificationsAPI.testNotification).toHaveBeenCalledWith( + expect.objectContaining({ type: 'email' }), + ); + + await result.testApprise(); + expect(NotificationsAPI.testNotification).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'apprise', + config: expect.objectContaining({ + mode: 'cli', + targets: ['mailto://alerts@example.com'], + }), + }), + ); + + await result.addWebhook({ + enabled: true, + headers: {}, + method: 'POST', + name: 'Pager', + service: 'slack', + url: 'https://hooks.example.test/pager', + }); + expect(result.webhooks().map((hook) => hook.id)).toEqual(['hook-1', 'hook-2']); + + await result.updateWebhook({ + enabled: true, + headers: {}, + id: 'hook-2', + method: 'POST', + name: 'Pager', + service: 'slack', + url: 'https://hooks.example.test/pager', + }); + expect(result.webhooks().find((hook) => hook.id === 'hook-2')).toEqual( + expect.objectContaining({ enabled: false, name: 'Pager Updated' }), + ); + + await result.testWebhook('hook-2'); + expect(NotificationsAPI.testNotification).toHaveBeenCalledWith({ + type: 'webhook', + webhookId: 'hook-2', + }); + + await result.deleteWebhook('hook-1'); + expect(result.webhooks().map((hook) => hook.id)).toEqual(['hook-2']); + + result.updateApprise({ mode: 'http', serverUrl: 'https://apprise.internal' }); + expect(result.appriseState()).toEqual( + expect.objectContaining({ mode: 'http', serverUrl: 'https://apprise.internal' }), + ); + + result.handleRetry(); + expect(onRetryLoad).toHaveBeenCalledTimes(1); + await waitFor(() => expect(NotificationsAPI.getWebhooks).toHaveBeenCalledTimes(2)); + expect(notificationStore.success).toHaveBeenCalled(); + expect(showErrorWithDetail).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend-modern/src/features/alerts/tabs/DestinationsTab.tsx b/frontend-modern/src/features/alerts/tabs/DestinationsTab.tsx index 345c0042b..766b6a3ef 100644 --- a/frontend-modern/src/features/alerts/tabs/DestinationsTab.tsx +++ b/frontend-modern/src/features/alerts/tabs/DestinationsTab.tsx @@ -1,7 +1,6 @@ -import { createSignal, onMount, Show } from 'solid-js'; +import { Show } from 'solid-js'; import AlertTriangleIcon from 'lucide-solid/icons/alert-triangle'; -import { NotificationsAPI, type Webhook } from '@/api/notifications'; import { EmailProviderSelect } from '@/components/Alerts/EmailProviderSelect'; import { WebhookConfig } from '@/components/Alerts/WebhookConfig'; import { Card } from '@/components/shared/Card'; @@ -13,9 +12,6 @@ import { } from '@/components/shared/Form'; import { SettingsPanel } from '@/components/shared/SettingsPanel'; import { Toggle } from '@/components/shared/Toggle'; -import { notificationStore } from '@/stores/notifications'; -import { logger } from '@/utils/logger'; -import { showErrorWithDetail } from '@/utils/toast'; import { ALERT_DESTINATIONS_APPRISE_API_KEY_HEADER_HELP, ALERT_DESTINATIONS_APPRISE_API_KEY_HEADER_LABEL, @@ -48,180 +44,30 @@ import { ALERT_DESTINATIONS_EMAIL_PANEL_DESCRIPTION, ALERT_DESTINATIONS_EMAIL_PANEL_TITLE, getAlertDestinationsAppriseTargetsHelp, - getAlertDestinationsAppriseTestFailure, getAlertDestinationsAppriseTestLabel, - getAlertDestinationsAppriseTestSuccess, - getAlertDestinationsAppriseValidationError, - getAlertDestinationsEmailTestFailure, - getAlertDestinationsEmailTestSuccess, getAlertDestinationsLoadErrorBanner, getAlertDestinationsRetryLabel, getAlertDestinationsStatusLabel, - getAlertDestinationsWebhookLoadError, } from '@/utils/alertDestinationsPresentation'; import { - getAlertWebhookMutationFailure, - getAlertWebhookMutationSuccess, - getAlertWebhookTestFailure, - getAlertWebhookTestSuccess, getAlertWebhooksSectionDescription, getAlertWebhooksSectionTitle, } from '@/utils/alertWebhookPresentation'; -import { parseAppriseTargets } from '../helpers'; -import type { AppriseConfig } from '@/api/notifications'; -import type { UIAppriseConfig, UIEmailConfig } from '../types'; +import { useAlertDestinationsTabState, type AlertDestinationsTabStateProps } from '../useAlertDestinationsTabState'; -export interface DestinationsTabProps { +export interface DestinationsTabProps extends AlertDestinationsTabStateProps { setHasUnsavedChanges: (value: boolean) => void; - emailConfig: () => UIEmailConfig; - setEmailConfig: (config: UIEmailConfig) => void; - appriseConfig: () => UIAppriseConfig; - setAppriseConfig: (config: UIAppriseConfig) => void; - configLoadError: () => string | null; - isRetrying: () => boolean; - isLoadingDestinations: () => boolean; - onRetryLoad: () => void; + setEmailConfig: (config: ReturnType) => void; } export function DestinationsTab(props: DestinationsTabProps) { - const [webhooks, setWebhooks] = createSignal([]); - const [webhookLoadError, setWebhookLoadError] = createSignal(null); - const [isLoadingWebhooks, setIsLoadingWebhooks] = createSignal(true); - const [testingEmail, setTestingEmail] = createSignal(false); - const [testingApprise, setTestingApprise] = createSignal(false); - const [testingWebhook, setTestingWebhook] = createSignal(null); - - const isLoading = () => - props.isLoadingDestinations() || isLoadingWebhooks() || props.isRetrying(); - const appriseState = () => props.appriseConfig(); - - const updateApprise = (partial: Partial) => { - props.setAppriseConfig({ ...props.appriseConfig(), ...partial }); - }; - - const buildAppriseRequestConfig = (): AppriseConfig => { - const config = appriseState(); - const serverUrl = (config.serverUrl || '').trim(); - const apiKeyHeader = (config.apiKeyHeader || '').trim() || 'X-API-KEY'; - return { - enabled: config.enabled, - mode: config.mode, - targets: parseAppriseTargets(config.targetsText), - cliPath: config.cliPath?.trim() || 'apprise', - timeoutSeconds: config.timeoutSeconds, - serverUrl, - configKey: config.configKey.trim(), - apiKey: config.apiKey, - apiKeyHeader, - skipTlsVerify: config.skipTlsVerify, - }; - }; - - const loadWebhooks = async () => { - setWebhookLoadError(null); - setIsLoadingWebhooks(true); - try { - const hooks = await NotificationsAPI.getWebhooks(); - setWebhooks( - hooks.map((hook) => ({ - ...hook, - service: hook.service || 'generic', - })), - ); - } catch (error) { - logger.error('Failed to load webhooks:', error); - setWebhookLoadError(getAlertDestinationsWebhookLoadError()); - } finally { - setIsLoadingWebhooks(false); - } - }; - - onMount(() => { - void loadWebhooks(); - }); - - const testEmailConfig = async () => { - setTestingEmail(true); - try { - await NotificationsAPI.testNotification({ - type: 'email', - config: { ...props.emailConfig() } as Record, - }); - notificationStore.success(getAlertDestinationsEmailTestSuccess()); - } catch (error) { - logger.error(getAlertDestinationsEmailTestFailure(), error); - const message = - error instanceof Error ? error.message : getAlertDestinationsEmailTestFailure(); - const detail = (error as Error & { detail?: string })?.detail; - showErrorWithDetail(message, detail); - } finally { - setTestingEmail(false); - } - }; - - const testApprise = async () => { - setTestingApprise(true); - try { - const config = buildAppriseRequestConfig(); - - if (!config.enabled) { - throw new Error(getAlertDestinationsAppriseValidationError('disabled')); - } - - const targets = config.targets || []; - if (config.mode === 'cli' && targets.length === 0) { - throw new Error(getAlertDestinationsAppriseValidationError('missingTargets')); - } - if (config.mode === 'http' && !config.serverUrl) { - throw new Error(getAlertDestinationsAppriseValidationError('missingServerUrl')); - } - - await NotificationsAPI.testNotification({ - type: 'apprise', - config, - }); - notificationStore.success(getAlertDestinationsAppriseTestSuccess()); - } catch (error) { - logger.error(getAlertDestinationsAppriseTestFailure(), error); - const message = - error instanceof Error ? error.message : getAlertDestinationsAppriseTestFailure(); - const detail = (error as Error & { detail?: string })?.detail; - showErrorWithDetail(message, detail); - } finally { - setTestingApprise(false); - } - }; - - const testWebhook = async (webhookId: string, webhookData?: Omit) => { - setTestingWebhook(webhookId); - try { - if (webhookData) { - await NotificationsAPI.testWebhook(webhookData); - } else { - await NotificationsAPI.testNotification({ type: 'webhook', webhookId }); - } - notificationStore.success(getAlertWebhookTestSuccess()); - } catch (error) { - const message = error instanceof Error ? error.message : getAlertWebhookTestFailure(); - const detail = (error as Error & { detail?: string })?.detail; - showErrorWithDetail(message, detail); - } finally { - setTestingWebhook(null); - } - }; - - const hasLoadError = () => props.configLoadError() || webhookLoadError(); - - const handleRetry = () => { - props.onRetryLoad(); - void loadWebhooks(); - }; + const state = useAlertDestinationsTabState(props); return (
@@ -265,21 +111,21 @@ export function DestinationsTab(props: DestinationsTabProps) {
} > - +
{getAlertDestinationsLoadErrorBanner( - props.configLoadError() || webhookLoadError() || '', + props.configLoadError() || state.webhookLoadError() || '', )}
@@ -320,8 +166,8 @@ export function DestinationsTab(props: DestinationsTabProps) { props.setEmailConfig(config); props.setHasUnsavedChanges(true); }} - onTest={testEmailConfig} - testing={testingEmail()} + onTest={state.testEmailConfig} + testing={state.testingEmail()} />
@@ -332,23 +178,23 @@ export function DestinationsTab(props: DestinationsTabProps) { action={
{ - updateApprise({ enabled: event.currentTarget.checked }); + state.updateApprise({ enabled: event.currentTarget.checked }); props.setHasUnsavedChanges(true); }} label={ - {getAlertDestinationsStatusLabel(appriseState().enabled)} + {getAlertDestinationsStatusLabel(state.appriseState().enabled)} } />
} @@ -362,9 +208,9 @@ export function DestinationsTab(props: DestinationsTabProps) {