From 34bc7d081e9e10a2ee627302e5dcab1ec3a0e383 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Tue, 3 Nov 2026 13:04:09 -0500 Subject: [PATCH] fix(frontend): branch incident copy on alert type and drop null placeholders --- .../monitoring/__tests__/alertCopy.test.mjs | 39 ++++++++++++ .../src/components/monitoring/alertCopy.js | 62 +++++++++++++++++++ frontend/src/i18n/locales/en.json | 23 +++++++ frontend/src/pages/Incidents.jsx | 35 +++++------ 4 files changed, 140 insertions(+), 19 deletions(-) create mode 100644 frontend/src/components/monitoring/__tests__/alertCopy.test.mjs create mode 100644 frontend/src/components/monitoring/alertCopy.js diff --git a/frontend/src/components/monitoring/__tests__/alertCopy.test.mjs b/frontend/src/components/monitoring/__tests__/alertCopy.test.mjs new file mode 100644 index 00000000..dcb8bce6 --- /dev/null +++ b/frontend/src/components/monitoring/__tests__/alertCopy.test.mjs @@ -0,0 +1,39 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { classifyAlertMetric, describeAlert, formatReading } from '../alertCopy.js'; + +// Minimal i18next stand-in: default string + {{var}} interpolation. +const t = (_key, fallback, vars = {}) => fallback.replace(/{{(\w+)}}/g, (_, k) => vars[k]); +const text = (alert) => JSON.stringify(describeAlert(t, alert)); + +test('classifies health signals apart from threshold metrics', () => { + assert.deepEqual(classifyAlertMetric('offline'), { kind: 'offline' }); + assert.deepEqual(classifyAlertMetric('svc:mysql'), { kind: 'service', service: 'mysql' }); + assert.deepEqual(classifyAlertMetric('cpu'), { kind: 'threshold' }); +}); + +test('offline and service alerts never claim a limit or render placeholders', () => { + const offline = describeAlert(t, { metric: 'offline', value: null, threshold: null, subject: 'web-1' }); + assert.equal(offline.title, 'web-1 is offline'); + const service = describeAlert(t, { metric: 'svc:mysql', value: null, threshold: null, subject: 'web-1' }); + assert.equal(service.title, 'mysql is down on web-1'); + assert.equal(service.detail, 'mysql is not running'); + for (const s of [text({ metric: 'offline', subject: 'x' }), text({ metric: 'svc:a', subject: 'x' })]) { + assert.doesNotMatch(s, /null|undefined|—|over limit/); + } +}); + +test('threshold alerts include the reading when present and omit it when not', () => { + const full = describeAlert(t, { metric: 'cpu', value: 93.456, threshold: 90, subject: 'db' }); + assert.equal(full.title, 'CPU % over limit on db'); + assert.equal(full.detail, 'CPU % 93.5 of 90 limit'); + const bare = describeAlert(t, { metric: 'disk', value: null, threshold: null, subject: 'db' }); + assert.equal(bare.detail, 'Disk %'); + assert.doesNotMatch(text({ metric: 'memory', subject: 'db' }), /null|undefined|—/); +}); + +test('unknown or missing metric falls back to a neutral title', () => { + assert.equal(describeAlert(t, { metric: undefined, subject: 'db' }).title, 'Alert on db'); + assert.equal(formatReading('x'), null); +}); diff --git a/frontend/src/components/monitoring/alertCopy.js b/frontend/src/components/monitoring/alertCopy.js new file mode 100644 index 00000000..a7d95413 --- /dev/null +++ b/frontend/src/components/monitoring/alertCopy.js @@ -0,0 +1,62 @@ +import { METRIC_LABELS } from './fleetMetrics.js'; + +// Plain-language title/detail for a metric alert. The backend's `metric` is +// either a threshold metric (cpu, memory, disk, network_*) or a health signal +// with no reading at all: `offline` (server stopped reporting) and +// `svc:` (a watched service is down). Only the first kind has a value and +// limit, so only it may talk about going "over limit". +const SERVICE_PREFIX = 'svc:'; + +export function classifyAlertMetric(metric) { + if (metric === 'offline') return { kind: 'offline' }; + if (typeof metric === 'string' && metric.startsWith(SERVICE_PREFIX)) { + return { kind: 'service', service: metric.slice(SERVICE_PREFIX.length) || null }; + } + return { kind: 'threshold' }; +} + +export function formatReading(value, digits = 1) { + return typeof value === 'number' && Number.isFinite(value) ? value.toFixed(digits) : null; +} + +export function metricLabel(metric) { + return METRIC_LABELS[metric] || metric || null; +} + +// `t` is i18next's translate function (injected so this stays testable). +export function describeAlert(t, { metric, value, threshold, subject }) { + const { kind, service } = classifyAlertMetric(metric); + if (kind === 'offline') { + return { + title: t('app.alertCopy.offlineTitle', '{{subject}} is offline', { subject }), + detail: t('app.alertCopy.offlineDetail', 'Stopped reporting'), + }; + } + if (kind === 'service') { + return service + ? { + title: t('app.alertCopy.serviceDownTitle', '{{service}} is down on {{subject}}', { service, subject }), + detail: t('app.alertCopy.serviceDownDetail', '{{service}} is not running', { service }), + } + : { + title: t('app.alertCopy.serviceDownAnyTitle', 'A service is down on {{subject}}', { subject }), + detail: t('app.alertCopy.serviceDownAnyDetail', 'Service not running'), + }; + } + const label = metricLabel(metric); + const reading = formatReading(value); + const limit = formatReading(threshold, 0); + if (!label) { + return { title: t('app.alertCopy.genericTitle', 'Alert on {{subject}}', { subject }), detail: '' }; + } + let detail = label; + if (reading && limit) { + detail = t('app.alertCopy.readingOfLimit', '{{metric}} {{reading}} of {{limit}} limit', { metric: label, reading, limit }); + } else if (reading) { + detail = t('app.alertCopy.readingOnly', '{{metric}} {{reading}}', { metric: label, reading }); + } + return { + title: t('app.alertCopy.overLimitTitle', '{{metric}} over limit on {{subject}}', { metric: label, subject }), + detail, + }; +} diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index e369fe45..df297b82 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -361,6 +361,18 @@ "decisionFailed": "Couldn't submit your decision.", "requestFailed": "Couldn't reach the assistant." }, + "alertCopy": { + "genericTitle": "Alert on {{subject}}", + "offlineDetail": "Stopped reporting", + "offlineTitle": "{{subject}} is offline", + "overLimitTitle": "{{metric}} over limit on {{subject}}", + "readingOfLimit": "{{metric}} {{reading}} of {{limit}} limit", + "readingOnly": "{{metric}} {{reading}}", + "serviceDownAnyDetail": "Service not running", + "serviceDownAnyTitle": "A service is down on {{subject}}", + "serviceDownDetail": "{{service}} is not running", + "serviceDownTitle": "{{service}} is down on {{subject}}" + }, "alertsTab": { "acknowledged": "Acknowledged", "allClear": "All clear", @@ -3316,7 +3328,9 @@ "rules": "Rules", "searchIncidentsAndAlerts": "Search incidents and alerts…", "subject": "Subject", + "thisServer": "This server", "timeline": "Timeline", + "unknownServer": "Unknown server", "whatChanged": "What changed?", "whatHappened": "What happened" }, @@ -4804,6 +4818,7 @@ "cpu": "CPU", "disk": "Disk", "featured": "Featured", + "headerLine": "One-click installers", "install": "Install", "lede": "Ready-to-go installers that run as real operations: probe, derive, apply, verify. Secrets are asked for mid-run and kept in the vault.", "loadFailed": "Couldn't load the recipe catalog.", @@ -4822,6 +4837,14 @@ "secretAsks_other": "{{count}} secret asks", "startFailed": "Couldn't start the recipe.", "started": "Recipe started on {{server}}", + "stepApply": "Apply", + "stepApplyInfo": "Derive settings and run the install as a real operation.", + "stepProbe": "Probe", + "stepProbeInfo": "Check the target server meets the requirements.", + "stepSecrets": "Secrets", + "stepSecretsInfo": "Secrets are asked for mid-run and kept in the vault.", + "stepVerify": "Verify", + "stepVerifyInfo": "Confirm the service came up healthy.", "stepsCount_one": "1 step", "stepsCount_other": "{{count}} steps", "unavailable": "Recipe catalog unavailable", diff --git a/frontend/src/pages/Incidents.jsx b/frontend/src/pages/Incidents.jsx index 02806416..d78276f2 100644 --- a/frontend/src/pages/Incidents.jsx +++ b/frontend/src/pages/Incidents.jsx @@ -27,7 +27,7 @@ import { Input } from '@/components/ui/input'; import { useTableSort } from '@/hooks/useTableSort'; import { useColumnVisibility } from '@/hooks/useColumnVisibility'; import { useTopbarActions, useTopbarChrome } from '@/hooks/useTopbarActions'; -import { METRIC_LABELS } from '../components/monitoring/fleetMetrics'; +import { describeAlert, formatReading, metricLabel } from '../components/monitoring/alertCopy'; import { impactTone, INCIDENT_STATES } from '../components/monitoring/monitorShared'; import { useTranslation } from 'react-i18next'; import { toastError } from '@/utils/errorMessage'; @@ -123,11 +123,6 @@ function formatWhen(iso) { return formatDateTime(date); } -function formatValue(value) { - if (typeof value !== 'number' || Number.isNaN(value)) return '—'; - return value.toFixed(1); -} - export default function Incidents() { const { t } = useTranslation(); const toast = useToast(); @@ -237,15 +232,16 @@ export default function Incidents() { }; }); + const thisServer = t('app.incidents.thisServer', 'This server'); const hostDetail = (alert) => (alert.type - ? `${alert.type} ${formatValue(alert.value)} / ${alert.threshold}` - : 'This server'); + ? describeAlert(t, { metric: alert.type, value: alert.value, threshold: alert.threshold, subject: thisServer }).detail + : thisServer); const fromActive = activeAlerts.map((alert, index) => ({ kind: 'alert', key: `active-${alert.type}-${index}`, title: alert.message, - subject: 'This server', + subject: thisServer, detail: hostDetail(alert), state: 'firing', tone: 'red', @@ -262,7 +258,7 @@ export default function Incidents() { kind: 'alert', key: `history-${alert.timestamp || index}-${index}`, title: alert.message, - subject: 'This server', + subject: thisServer, detail: hostDetail(alert), state: alert.severity, tone: statusKind(alert.severity), @@ -282,15 +278,16 @@ export default function Incidents() { // sources above describe the panel's own box and have no server-side // lifecycle to ack or resolve. const fromFleet = fleetAlerts.map((alert) => { - const metric = METRIC_LABELS[alert.metric] || alert.metric; - const subject = alert.server_name || 'Unknown server'; + const metric = metricLabel(alert.metric); + const subject = alert.server_name || t('app.incidents.unknownServer', 'Unknown server'); + const { title, detail } = describeAlert(t, { ...alert, subject }); return { kind: 'alert', key: `fleet-${alert.id}`, alertId: alert.id, - title: `${metric} over limit on ${subject}`, + title, subject, - detail: `${metric} ${formatValue(alert.value)} / ${alert.threshold}`, + detail, state: alert.status, tone: alertStatusKind(alert.status), impact: alert.severity, @@ -305,7 +302,7 @@ export default function Incidents() { return [...fromActive, ...fromFleet, ...fromIncidents, ...fromHistory] .sort((a, b) => new Date(b.when || 0) - new Date(a.when || 0)); - }, [incidents, activeAlerts, alertHistory, fleetAlerts, monitorsById]); + }, [incidents, activeAlerts, alertHistory, fleetAlerts, monitorsById, t]); // Search only. The active/resolved split used to live here as a segment // filter; it is a column rule now, applied inside the table. @@ -408,7 +405,7 @@ export default function Incidents() { headerKey: 'app.incidents.impact', header: 'Impact', sortable: true, type: 'enum', - value: (item) => item.impact || '—', + value: (item) => item.impact || '', }, { key: 'when', @@ -628,9 +625,9 @@ export default function Incidents() {
-
{t('app.incidents.metric', 'Metric')}
{selected.metric || '—'}
-
{t('app.incidents.reading', 'Reading')}
{formatValue(selected.value)}
-
{t('app.incidents.limit', 'Limit')}
{selected.threshold ?? '—'}
+ {selected.metric &&
{t('app.incidents.metric', 'Metric')}
{selected.metric}
} + {formatReading(selected.value) &&
{t('app.incidents.reading', 'Reading')}
{formatReading(selected.value)}
} + {formatReading(selected.threshold, 0) &&
{t('app.incidents.limit', 'Limit')}
{formatReading(selected.threshold, 0)}
}
{t('common.labels.when', 'When')}
{formatWhen(selected.when)}