mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-19 02:28:59 +00:00
Fix off-toggle edge cases in threshold editors (#1642)
The per-metric Off toggles added in 085208a82 disagreed with the alert
engine in five ways. The engine disables a metric whenever its trigger is
<= 0, so a stored 0 (which an older help banner told operators to type) is
off, and an unset global default is off too. Edit-mode detection tested for
-1 exactly, so those rows rendered On while nothing was alerting, and the
global-defaults rows coerced an emptied input to 0, which disabled the
metric in the engine while the row still showed On. All of it now reads the
same <= 0 rule through isAlertResourceMetricOff in the shared resource
table model, and a cleared input is treated as mid-edit rather than as a
disable request.
Re-enabling was worse than cosmetic. The row editor seeds its state from
the defaults, so switching a metric back on wrote undefined, the save path
skipped it as unchanged, and the resource kept inheriting a globally
disabled default. Bulk edit had the same hole, where undefined means
unchanged, so re-enabling a metric across a selection was impossible. Both
now stage an explicit enabled threshold when there is nothing enabled to
inherit, and the row editor still clears the override when the inherited
default is already on.
The row editor's input saved and closed on blur, so tabbing to the Off
badge dismissed the editor before the badge could be reached. Blur now
ignores focus moving inside the same metric editor, which covers the badge
without changing mouse behavior.
Documents the off sentinel rule in the alerts contract, since these editors
are named there and the previous commit bypassed it as contract-neutral.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
courtmanr@gmail.com
parent
5c1c89a363
commit
7018e799de
8 changed files with 400 additions and 17 deletions
|
|
@ -79,6 +79,29 @@ the one whose identity alerting honours. Standalone Pulse-agent machines belong
|
|||
to Machines, so the Virtualization Hosts section is fed by provider-owned
|
||||
virtualization nodes rather than by every resource of type `agent`.
|
||||
|
||||
A per-metric threshold is off whenever its trigger is `<= 0`. That boundary is
|
||||
engine truth (`internal/alerts/canonical_metric.go`,
|
||||
`internal/alerts/config_runtime.go`), not a display convention: `-1` is the
|
||||
value Pulse writes, and `0` disables the metric just as completely because
|
||||
earlier builds advertised `0` as the disable value and those overrides are still
|
||||
on disk. Every threshold editor reads that same `<= 0` rule through
|
||||
`frontend-modern/src/components/Alerts/alertResourceTableModel.ts` rather than
|
||||
testing for `-1` inline: read cells, the row override editor, the global
|
||||
defaults row, and the bulk edit dialog, on both the desktop and the mobile
|
||||
surface. A surface that recognises only `-1` reports On for a metric the engine
|
||||
is not alerting on at all, and an unset global default carries the same
|
||||
obligation, because absent is disabled rather than enabled.
|
||||
|
||||
Turning a metric back on must write a value the save path will actually
|
||||
persist. Clearing an override only re-inherits the default, so it re-enables
|
||||
nothing when the inherited default is itself off, and an undefined entry in a
|
||||
bulk edit reads as unchanged rather than as on; in both cases the editor stages
|
||||
the metric's enabled default instead, and only clears the override when the
|
||||
inherited default is already enabled. Editors must also not manufacture an off
|
||||
state out of an empty input: a cleared box is mid-edit, and coercing it to `0`
|
||||
disabled the metric in the engine while the row still showed On. Disabling is
|
||||
the toggle's job, and the toggle writes the canonical `-1`.
|
||||
|
||||
## Canonical Files
|
||||
|
||||
1. `internal/alerts/specs/types.go`
|
||||
|
|
|
|||
|
|
@ -39,12 +39,14 @@ import {
|
|||
getAlertResourceTableNoResultsState,
|
||||
} from '@/utils/alertResourceTablePresentation';
|
||||
import {
|
||||
ALERT_RESOURCE_METRIC_OFF_VALUE,
|
||||
getAlertResourceColumnKind,
|
||||
getAlertResourceColumnHeaderTooltip,
|
||||
getAlertResourceEnabledDefault,
|
||||
getAlertResourceMetricBounds,
|
||||
getAlertResourceMetricDelayOverride,
|
||||
getAlertResourceMetricStep,
|
||||
isAlertResourceMetricOff,
|
||||
normalizeAlertResourceMetricKey,
|
||||
} from './alertResourceTableModel';
|
||||
import type { OfflineState, ResourceTableProps } from './ResourceTable';
|
||||
|
|
@ -223,7 +225,9 @@ export function AlertResourceTableDesktop(props: AlertResourceTableDesktopProps)
|
|||
const metric = normalizeAlertResourceMetricKey(column);
|
||||
const bounds = getAlertResourceMetricBounds(metric);
|
||||
const value = () => props.table.globalDefaults?.[metric] ?? 0;
|
||||
const isOff = () => value() === -1;
|
||||
// An unset default and a stored 0 are both disabled to the
|
||||
// alert engine, so neither may render as On.
|
||||
const isOff = () => isAlertResourceMetricOff(value());
|
||||
|
||||
return (
|
||||
<TableCell
|
||||
|
|
@ -241,9 +245,16 @@ export function AlertResourceTableDesktop(props: AlertResourceTableDesktopProps)
|
|||
disabled={isOff()}
|
||||
onInput={(e) => {
|
||||
const nextValue = parseFloat(e.currentTarget.value);
|
||||
// An empty box is mid-edit, not a disable request:
|
||||
// coercing it to 0 used to disable the metric in the
|
||||
// engine while the row still claimed On. Use the Off
|
||||
// toggle to disable.
|
||||
if (Number.isNaN(nextValue)) {
|
||||
return;
|
||||
}
|
||||
props.table.setGlobalDefaults?.((prev) => ({
|
||||
...prev,
|
||||
[metric]: Number.isNaN(nextValue) ? 0 : nextValue,
|
||||
[metric]: nextValue,
|
||||
}));
|
||||
props.table.setHasUnsavedChanges?.(true);
|
||||
}}
|
||||
|
|
@ -276,7 +287,9 @@ export function AlertResourceTableDesktop(props: AlertResourceTableDesktopProps)
|
|||
onToggle={() => {
|
||||
props.table.setGlobalDefaults?.((prev) => ({
|
||||
...prev,
|
||||
[metric]: isOff() ? getAlertResourceEnabledDefault(metric) : -1,
|
||||
[metric]: isOff()
|
||||
? getAlertResourceEnabledDefault(metric)
|
||||
: ALERT_RESOURCE_METRIC_OFF_VALUE,
|
||||
}));
|
||||
props.table.setHasUnsavedChanges?.(true);
|
||||
}}
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import {
|
|||
getAlertResourceTableRevertToDefaultsLabel,
|
||||
} from '@/utils/alertResourceTablePresentation';
|
||||
import {
|
||||
ALERT_RESOURCE_METRIC_OFF_VALUE,
|
||||
alertResourceSupportsMetric,
|
||||
buildAlertResourceEditPayload,
|
||||
getAlertResourceEnabledDefault,
|
||||
|
|
@ -26,8 +27,10 @@ import {
|
|||
getAlertResourceMetricBounds,
|
||||
getAlertResourceMetricDisplayValue,
|
||||
getAlertResourceMetricStep,
|
||||
isAlertResourceMetricOff,
|
||||
isAlertResourceMetricOverridden,
|
||||
normalizeAlertResourceMetricKey,
|
||||
resolveAlertResourceMetricEnableValue,
|
||||
} from './alertResourceTableModel';
|
||||
import type { Resource } from '@/features/alerts/thresholds/tableTypes';
|
||||
import type { ResourceTableProps } from './ResourceTable';
|
||||
|
|
@ -152,7 +155,9 @@ export function AlertResourceTableMobile(props: AlertResourceTableMobileProps) {
|
|||
const metric = normalizeAlertResourceMetricKey(column);
|
||||
const bounds = getAlertResourceMetricBounds(metric);
|
||||
const value = () => props.table.globalDefaults?.[metric] ?? 0;
|
||||
const isOff = () => value() === -1;
|
||||
// An unset default and a stored 0 are both disabled to the alert
|
||||
// engine, so neither may render as On.
|
||||
const isOff = () => isAlertResourceMetricOff(value());
|
||||
|
||||
return (
|
||||
<div class="p-2 bg-surface rounded border border-border-subtle flex flex-col gap-1">
|
||||
|
|
@ -170,9 +175,16 @@ export function AlertResourceTableMobile(props: AlertResourceTableMobileProps) {
|
|||
class={`w-full text-sm p-1 rounded border text-center ${isOff() ? 'bg-surface-hover' : ' border-border'}`}
|
||||
onInput={(e) => {
|
||||
const nextValue = parseFloat(e.currentTarget.value);
|
||||
// An empty box is mid-edit, not a disable request:
|
||||
// coercing it to 0 used to disable the metric in the
|
||||
// engine while the card still claimed On. Use the Off
|
||||
// toggle to disable.
|
||||
if (Number.isNaN(nextValue)) {
|
||||
return;
|
||||
}
|
||||
props.table.setGlobalDefaults?.((prev) => ({
|
||||
...prev,
|
||||
[metric]: Number.isNaN(nextValue) ? 0 : nextValue,
|
||||
[metric]: nextValue,
|
||||
}));
|
||||
props.table.setHasUnsavedChanges?.(true);
|
||||
}}
|
||||
|
|
@ -197,7 +209,9 @@ export function AlertResourceTableMobile(props: AlertResourceTableMobileProps) {
|
|||
onToggle={() => {
|
||||
props.table.setGlobalDefaults?.((prev) => ({
|
||||
...prev,
|
||||
[metric]: isOff() ? getAlertResourceEnabledDefault(metric) : -1,
|
||||
[metric]: isOff()
|
||||
? getAlertResourceEnabledDefault(metric)
|
||||
: ALERT_RESOURCE_METRIC_OFF_VALUE,
|
||||
}));
|
||||
props.table.setHasUnsavedChanges?.(true);
|
||||
}}
|
||||
|
|
@ -338,7 +352,8 @@ export function AlertResourceTableMobile(props: AlertResourceTableMobileProps) {
|
|||
const metric = normalizeAlertResourceMetricKey(column);
|
||||
if (!alertResourceSupportsMetric(resource.type, metric)) return null;
|
||||
|
||||
const isDisabled = () => thresholds()?.[metric] === -1;
|
||||
const isDisabled = () => isAlertResourceMetricOff(thresholds()?.[metric]);
|
||||
const inheritedDefault = () => resource.defaults?.[metric];
|
||||
const bounds = getAlertResourceMetricBounds(metric);
|
||||
|
||||
return (
|
||||
|
|
@ -388,7 +403,12 @@ export function AlertResourceTableMobile(props: AlertResourceTableMobileProps) {
|
|||
onToggle={() =>
|
||||
props.table.setEditingThresholds({
|
||||
...props.table.editingThresholds(),
|
||||
[metric]: isDisabled() ? undefined : -1,
|
||||
[metric]: isDisabled()
|
||||
? resolveAlertResourceMetricEnableValue(
|
||||
inheritedDefault(),
|
||||
metric,
|
||||
)
|
||||
: ALERT_RESOURCE_METRIC_OFF_VALUE,
|
||||
})
|
||||
}
|
||||
{...getAlertResourceTableMetricOffToggleProps()}
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import {
|
|||
getAlertResourceTableRevertToDefaultsLabel,
|
||||
} from '@/utils/alertResourceTablePresentation';
|
||||
import {
|
||||
ALERT_RESOURCE_METRIC_OFF_VALUE,
|
||||
ALERT_RESOURCE_TABLE_SLIDER_METRICS,
|
||||
alertResourceSupportsMetric,
|
||||
getAlertResourceColumnKind,
|
||||
|
|
@ -30,8 +31,10 @@ import {
|
|||
getAlertResourceMetricBounds,
|
||||
getAlertResourceMetricDisplayValue,
|
||||
getAlertResourceMetricStep,
|
||||
isAlertResourceMetricOff,
|
||||
isAlertResourceMetricOverridden,
|
||||
normalizeAlertResourceMetricKey,
|
||||
resolveAlertResourceMetricEnableValue,
|
||||
type AlertResourceThresholdMap,
|
||||
} from './alertResourceTableModel';
|
||||
import type { Alert } from '@/types/api';
|
||||
|
|
@ -314,8 +317,10 @@ export function AlertResourceTableRow(props: AlertResourceTableRowProps) {
|
|||
{(column) => {
|
||||
const metric = normalizeAlertResourceMetricKey(column);
|
||||
const bounds = getAlertResourceMetricBounds(metric);
|
||||
const isDisabled = () => getThresholds()?.[metric] === -1;
|
||||
const isDisabled = () => isAlertResourceMetricOff(getThresholds()?.[metric]);
|
||||
const inheritedDefault = () => props.resource.defaults?.[metric];
|
||||
const isSpecialToggle = metric === 'backup' || metric === 'snapshot';
|
||||
let metricEditor: HTMLDivElement | undefined;
|
||||
|
||||
if (isSpecialToggle) {
|
||||
const config = metric === 'backup' ? props.resource.backup : props.resource.snapshot;
|
||||
|
|
@ -439,7 +444,7 @@ export function AlertResourceTableRow(props: AlertResourceTableRowProps) {
|
|||
);
|
||||
})()}
|
||||
</Show>
|
||||
<div class="flex items-center justify-center gap-1.5">
|
||||
<div class="flex items-center justify-center gap-1.5" ref={metricEditor}>
|
||||
<input
|
||||
type="number"
|
||||
min={bounds.min}
|
||||
|
|
@ -471,10 +476,17 @@ export function AlertResourceTableRow(props: AlertResourceTableRowProps) {
|
|||
updateEditingThreshold(metric, val);
|
||||
}
|
||||
}}
|
||||
onBlur={() => {
|
||||
if (props.editingId() === props.resource.id) {
|
||||
saveEditing();
|
||||
onBlur={(event) => {
|
||||
if (props.editingId() !== props.resource.id) {
|
||||
return;
|
||||
}
|
||||
// Tabbing to the Off toggle must not save and close the
|
||||
// editor before the toggle can be operated.
|
||||
const nextFocus = event.relatedTarget;
|
||||
if (nextFocus instanceof Node && metricEditor?.contains(nextFocus)) {
|
||||
return;
|
||||
}
|
||||
saveEditing();
|
||||
}}
|
||||
class={`w-16 px-2 py-0.5 text-sm text-center border rounded ${
|
||||
isDisabled()
|
||||
|
|
@ -488,7 +500,12 @@ export function AlertResourceTableRow(props: AlertResourceTableRowProps) {
|
|||
<StatusBadge
|
||||
isEnabled={!isDisabled()}
|
||||
onToggle={() =>
|
||||
updateEditingThreshold(metric, isDisabled() ? undefined : -1)
|
||||
updateEditingThreshold(
|
||||
metric,
|
||||
isDisabled()
|
||||
? resolveAlertResourceMetricEnableValue(inheritedDefault(), metric)
|
||||
: ALERT_RESOURCE_METRIC_OFF_VALUE,
|
||||
)
|
||||
}
|
||||
{...getAlertResourceTableMetricOffToggleProps()}
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -6,6 +6,11 @@ import {
|
|||
getAlertResourceTableMetricOffToggleProps,
|
||||
getAlertResourceTableMetricPlaceholder,
|
||||
} from '@/utils/alertResourceTablePresentation';
|
||||
import {
|
||||
ALERT_RESOURCE_METRIC_OFF_VALUE,
|
||||
getAlertResourceEnabledDefault,
|
||||
isAlertResourceMetricOff,
|
||||
} from './alertResourceTableModel';
|
||||
import {
|
||||
ALERT_BULK_EDIT_CANCEL_LABEL,
|
||||
ALERT_BULK_EDIT_CLEAR_LABEL,
|
||||
|
|
@ -107,7 +112,7 @@ export function BulkEditDialog(props: BulkEditDialogProps) {
|
|||
if (metric === 'backup' || metric === 'snapshot') return null;
|
||||
const bounds = getMetricBounds(metric);
|
||||
const val = () => thresholds()[metric];
|
||||
const isOff = () => val() === -1;
|
||||
const isOff = () => isAlertResourceMetricOff(val());
|
||||
|
||||
return (
|
||||
<div class="space-y-2 pb-4 border-b border-border-subtle last:border-0">
|
||||
|
|
@ -126,7 +131,12 @@ export function BulkEditDialog(props: BulkEditDialogProps) {
|
|||
onToggle={() =>
|
||||
setThresholds((prev) => ({
|
||||
...prev,
|
||||
[metric]: isOff() ? undefined : -1,
|
||||
// Re-enabling has to stage a real threshold: an
|
||||
// undefined entry reads as "unchanged" on save, so
|
||||
// the selected resources would stay disabled.
|
||||
[metric]: isOff()
|
||||
? getAlertResourceEnabledDefault(metric)
|
||||
: ALERT_RESOURCE_METRIC_OFF_VALUE,
|
||||
}))
|
||||
}
|
||||
{...getAlertResourceTableMetricOffToggleProps()}
|
||||
|
|
|
|||
|
|
@ -12,7 +12,9 @@ import {
|
|||
alertResourceSupportsMetric,
|
||||
buildAlertResourceEditPayload,
|
||||
getAlertResourceMetricDisplayValue,
|
||||
isAlertResourceMetricOff,
|
||||
normalizeAlertResourceMetricKey,
|
||||
resolveAlertResourceMetricEnableValue,
|
||||
} from '@/components/Alerts/alertResourceTableModel';
|
||||
|
||||
// --- Mocks (must be before component import) ---
|
||||
|
|
@ -346,6 +348,16 @@ describe('ResourceTable', () => {
|
|||
expect(alertResourceTableModelSource).toContain(
|
||||
'export function alertResourceSupportsMetric',
|
||||
);
|
||||
// The off sentinel is engine truth, not per-surface trivia: every editor
|
||||
// reads it through the shared model instead of comparing against -1 inline.
|
||||
expect(alertResourceTableModelSource).toContain('export function isAlertResourceMetricOff');
|
||||
expect(alertResourceTableModelSource).toContain(
|
||||
'export function resolveAlertResourceMetricEnableValue',
|
||||
);
|
||||
const strictOffComparison = ['===', ' -1'].join('');
|
||||
expect(alertResourceTableDesktopSource).not.toContain(strictOffComparison);
|
||||
expect(alertResourceTableMobileSource).not.toContain(strictOffComparison);
|
||||
expect(alertResourceTableRowSource).not.toContain(strictOffComparison);
|
||||
});
|
||||
|
||||
it('resolves metric values and edit payloads through the shared model', () => {
|
||||
|
|
@ -390,6 +402,23 @@ describe('ResourceTable', () => {
|
|||
note: 'Investigating bursty load',
|
||||
});
|
||||
});
|
||||
|
||||
it('reads the engine off sentinel and resolves re-enable values in the shared model', () => {
|
||||
// internal/alerts/canonical_metric.go disables any metric whose trigger is
|
||||
// <= 0, so a legacy 0 override is off even though the UI now writes -1.
|
||||
expect(isAlertResourceMetricOff(-1)).toBe(true);
|
||||
expect(isAlertResourceMetricOff(0)).toBe(true);
|
||||
expect(isAlertResourceMetricOff(-5)).toBe(true);
|
||||
expect(isAlertResourceMetricOff(80)).toBe(false);
|
||||
expect(isAlertResourceMetricOff(undefined)).toBe(false);
|
||||
|
||||
// Re-enabling can only clear the override when the inherited default is
|
||||
// itself enabled; otherwise the resource stays disabled.
|
||||
expect(resolveAlertResourceMetricEnableValue(80, 'cpu')).toBeUndefined();
|
||||
expect(resolveAlertResourceMetricEnableValue(-1, 'cpu')).toBe(80);
|
||||
expect(resolveAlertResourceMetricEnableValue(0, 'cpu')).toBe(80);
|
||||
expect(resolveAlertResourceMetricEnableValue(undefined, 'restartCount')).toBe(3);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resource display and editing', () => {
|
||||
|
|
@ -757,6 +786,212 @@ describe('ResourceTable', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('metric off toggles', () => {
|
||||
// The alert engine disables a metric whenever its trigger is <= 0, so every
|
||||
// editor here has to agree on that reading and has to write a value the
|
||||
// engine will actually honour.
|
||||
function editorProps(overrides: Record<string, any> = {}) {
|
||||
return makeProps({
|
||||
columns: ['CPU %'],
|
||||
editingId: () => 'vm-1',
|
||||
...overrides,
|
||||
});
|
||||
}
|
||||
|
||||
it('reads a stored 0 override as Off in the row editor', () => {
|
||||
const props = editorProps({
|
||||
resources: [makeResource({ id: 'vm-1', thresholds: { cpu: 0 }, defaults: { cpu: 80 } })],
|
||||
editingThresholds: () => ({ cpu: 0 }),
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
expect(screen.getByTestId('status-badge')).toHaveTextContent('Off');
|
||||
});
|
||||
|
||||
it('stages an explicit threshold when the inherited default is itself off', () => {
|
||||
// Clearing the override would only re-inherit the disabled default, so
|
||||
// the save path would drop the change and the metric would stay off.
|
||||
const setEditingThresholds = vi.fn();
|
||||
const props = editorProps({
|
||||
resources: [makeResource({ id: 'vm-1', thresholds: {}, defaults: { cpu: -1 } })],
|
||||
editingThresholds: () => ({ cpu: -1 }),
|
||||
setEditingThresholds,
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
fireEvent.click(screen.getByTestId('status-badge'));
|
||||
|
||||
expect(setEditingThresholds).toHaveBeenCalledWith({ cpu: 80 });
|
||||
});
|
||||
|
||||
it('clears the override when the inherited default is already enabled', () => {
|
||||
const setEditingThresholds = vi.fn();
|
||||
const props = editorProps({
|
||||
resources: [makeResource({ id: 'vm-1', thresholds: { cpu: -1 }, defaults: { cpu: 80 } })],
|
||||
editingThresholds: () => ({ cpu: -1 }),
|
||||
setEditingThresholds,
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
fireEvent.click(screen.getByTestId('status-badge'));
|
||||
|
||||
expect(setEditingThresholds).toHaveBeenCalledWith({ cpu: undefined });
|
||||
});
|
||||
|
||||
it('turns a metric off with the canonical -1 sentinel', () => {
|
||||
const setEditingThresholds = vi.fn();
|
||||
const props = editorProps({
|
||||
resources: [makeResource({ id: 'vm-1', thresholds: { cpu: 80 }, defaults: { cpu: 80 } })],
|
||||
editingThresholds: () => ({ cpu: 80 }),
|
||||
setEditingThresholds,
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
fireEvent.click(screen.getByTestId('status-badge'));
|
||||
|
||||
expect(setEditingThresholds).toHaveBeenCalledWith({ cpu: -1 });
|
||||
});
|
||||
|
||||
it('keeps the editor open when focus moves from the input to the off toggle', () => {
|
||||
// Tabbing to the toggle must not save and close the row before the toggle
|
||||
// can be operated.
|
||||
const onSaveEdit = vi.fn();
|
||||
const props = editorProps({
|
||||
resources: [makeResource({ id: 'vm-1', thresholds: { cpu: 80 }, defaults: { cpu: 80 } })],
|
||||
editingThresholds: () => ({ cpu: 80 }),
|
||||
onSaveEdit,
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
const input = screen.getByRole('spinbutton');
|
||||
fireEvent.blur(input, { relatedTarget: screen.getByTestId('status-badge') });
|
||||
|
||||
expect(onSaveEdit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still saves when focus leaves the metric editor entirely', () => {
|
||||
const onSaveEdit = vi.fn();
|
||||
const props = editorProps({
|
||||
resources: [makeResource({ id: 'vm-1', thresholds: { cpu: 80 }, defaults: { cpu: 80 } })],
|
||||
editingThresholds: () => ({ cpu: 80 }),
|
||||
onSaveEdit,
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
fireEvent.blur(screen.getByRole('spinbutton'), { relatedTarget: null });
|
||||
|
||||
expect(onSaveEdit).toHaveBeenCalledWith('vm-1');
|
||||
});
|
||||
|
||||
it('shows an unset global default as Off and re-enables it explicitly', () => {
|
||||
const setGlobalDefaults = vi.fn();
|
||||
const props = makeProps({
|
||||
resources: [],
|
||||
columns: ['CPU %'],
|
||||
globalDefaults: {},
|
||||
setGlobalDefaults,
|
||||
setHasUnsavedChanges: vi.fn(),
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
expect(screen.getByTestId('status-badge')).toHaveTextContent('Off');
|
||||
|
||||
fireEvent.click(screen.getByTestId('status-badge'));
|
||||
const updater = setGlobalDefaults.mock.calls[0][0] as (
|
||||
prev: Record<string, number | undefined>,
|
||||
) => Record<string, number | undefined>;
|
||||
expect(updater({})).toEqual({ cpu: 80 });
|
||||
});
|
||||
|
||||
it('shows a stored 0 global default as Off', () => {
|
||||
const props = makeProps({
|
||||
resources: [],
|
||||
columns: ['CPU %'],
|
||||
globalDefaults: { cpu: 0 },
|
||||
setGlobalDefaults: vi.fn(),
|
||||
setHasUnsavedChanges: vi.fn(),
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
expect(screen.getByTestId('status-badge')).toHaveTextContent('Off');
|
||||
});
|
||||
|
||||
it('does not write a disabling 0 when the global default input is cleared', () => {
|
||||
const setGlobalDefaults = vi.fn();
|
||||
const setHasUnsavedChanges = vi.fn();
|
||||
const props = makeProps({
|
||||
resources: [],
|
||||
columns: ['CPU %'],
|
||||
globalDefaults: { cpu: 80 },
|
||||
setGlobalDefaults,
|
||||
setHasUnsavedChanges,
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
fireEvent.input(screen.getByDisplayValue('80'), { target: { value: '' } });
|
||||
|
||||
expect(setGlobalDefaults).not.toHaveBeenCalled();
|
||||
expect(setHasUnsavedChanges).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('turns a global default off with the canonical -1 sentinel', () => {
|
||||
const setGlobalDefaults = vi.fn();
|
||||
const props = makeProps({
|
||||
resources: [],
|
||||
columns: ['CPU %'],
|
||||
globalDefaults: { cpu: 80 },
|
||||
setGlobalDefaults,
|
||||
setHasUnsavedChanges: vi.fn(),
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
fireEvent.click(screen.getByTestId('status-badge'));
|
||||
const updater = setGlobalDefaults.mock.calls[0][0] as (
|
||||
prev: Record<string, number | undefined>,
|
||||
) => Record<string, number | undefined>;
|
||||
expect(updater({ cpu: 80 })).toEqual({ cpu: -1 });
|
||||
});
|
||||
|
||||
it('applies the same off reading to the mobile global defaults card', () => {
|
||||
mockIsMobile.mockReturnValue(true);
|
||||
const setGlobalDefaults = vi.fn();
|
||||
const props = makeProps({
|
||||
resources: [],
|
||||
columns: ['CPU %'],
|
||||
globalDefaults: { cpu: 0 },
|
||||
setGlobalDefaults,
|
||||
setHasUnsavedChanges: vi.fn(),
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
expect(screen.getByTestId('status-badge')).toHaveTextContent('Off');
|
||||
|
||||
fireEvent.click(screen.getByTestId('status-badge'));
|
||||
const updater = setGlobalDefaults.mock.calls[0][0] as (
|
||||
prev: Record<string, number | undefined>,
|
||||
) => Record<string, number | undefined>;
|
||||
expect(updater({ cpu: 0 })).toEqual({ cpu: 80 });
|
||||
});
|
||||
|
||||
it('stages an explicit threshold from the mobile row editor', () => {
|
||||
mockIsMobile.mockReturnValue(true);
|
||||
const setEditingThresholds = vi.fn();
|
||||
const props = makeProps({
|
||||
columns: ['CPU %'],
|
||||
resources: [makeResource({ id: 'vm-1', thresholds: {}, defaults: { cpu: -1 } })],
|
||||
editingId: () => 'vm-1',
|
||||
editingThresholds: () => ({ cpu: -1 }),
|
||||
setEditingThresholds,
|
||||
globalDefaults: undefined,
|
||||
});
|
||||
render(() => <ResourceTable {...props} />);
|
||||
|
||||
fireEvent.click(screen.getByTestId('status-badge'));
|
||||
|
||||
expect(setEditingThresholds).toHaveBeenCalledWith({ cpu: 80 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('grouped resources', () => {
|
||||
it('renders group headers for grouped resources', () => {
|
||||
const props = makeProps({
|
||||
|
|
|
|||
|
|
@ -246,7 +246,11 @@ describe('BulkEditDialog', () => {
|
|||
expect(props.onSave).toHaveBeenCalledWith({ restartCount: -1 });
|
||||
});
|
||||
|
||||
it('off toggle returns the metric to Unchanged when clicked again', () => {
|
||||
it('re-enabling a metric stages an explicit threshold the save path can apply', () => {
|
||||
// An undefined entry means "unchanged" to the bulk save path, so staging
|
||||
// undefined here would leave every selected resource disabled while the
|
||||
// badge claimed On. Re-enabling therefore stages the metric's enabled
|
||||
// default.
|
||||
const props = defaultProps();
|
||||
props.columns = ['Restart Count'];
|
||||
render(() => <BulkEditDialog {...props} />);
|
||||
|
|
@ -254,12 +258,45 @@ describe('BulkEditDialog', () => {
|
|||
fireEvent.click(screen.getByRole('button', { name: 'On' }));
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Off' }));
|
||||
|
||||
expect(screen.getByRole('button', { name: 'On' })).toBeInTheDocument();
|
||||
expect(screen.queryByText('Unchanged')).not.toBeInTheDocument();
|
||||
|
||||
fireEvent.click(screen.getByText(/Apply to 3 items/));
|
||||
expect(props.onSave).toHaveBeenCalledWith({ restartCount: 3 });
|
||||
});
|
||||
|
||||
it('Clear still returns a re-enabled metric to Unchanged', () => {
|
||||
const props = defaultProps();
|
||||
props.columns = ['Restart Count'];
|
||||
render(() => <BulkEditDialog {...props} />);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'On' }));
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Off' }));
|
||||
fireEvent.click(screen.getByText('Clear'));
|
||||
|
||||
expect(screen.getByText('Unchanged')).toBeInTheDocument();
|
||||
fireEvent.click(screen.getByText(/Apply to 3 items/));
|
||||
const saved = props.onSave.mock.calls[0][0] as Record<string, number | undefined>;
|
||||
expect(saved.restartCount).toBeUndefined();
|
||||
});
|
||||
|
||||
it('reads a staged 0 as Off, matching the alert engine', () => {
|
||||
// The engine disables any metric whose trigger is <= 0, and older builds
|
||||
// told operators to type 0, so 0 must not read as On here.
|
||||
const props = defaultProps();
|
||||
props.columns = ['Restart Count'];
|
||||
render(() => <BulkEditDialog {...props} />);
|
||||
|
||||
fireEvent.input(screen.getByRole('spinbutton'), { target: { value: '0' } });
|
||||
|
||||
expect(screen.getByRole('button', { name: 'Off' })).toBeInTheDocument();
|
||||
|
||||
// Toggling that stale 0 back on stages the canonical enabled default.
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Off' }));
|
||||
fireEvent.click(screen.getByText(/Apply to 3 items/));
|
||||
expect(props.onSave).toHaveBeenCalledWith({ restartCount: 3 });
|
||||
});
|
||||
|
||||
it('typing a value after turning a metric off replaces the staged -1', () => {
|
||||
const props = defaultProps();
|
||||
props.columns = ['Restart Count'];
|
||||
|
|
|
|||
|
|
@ -202,6 +202,34 @@ export function getAlertResourceEnabledDefault(metric: string): number {
|
|||
return 80;
|
||||
}
|
||||
|
||||
export const ALERT_RESOURCE_METRIC_OFF_VALUE = -1;
|
||||
|
||||
/**
|
||||
* The alert engine disables a metric whenever its trigger is `<= 0`
|
||||
* (`internal/alerts/canonical_metric.go`), so every editor surface has to read
|
||||
* `0` as Off too: older builds advertised `0` as the disable value and those
|
||||
* overrides are still on disk. New writes always use the canonical `-1`.
|
||||
*/
|
||||
export function isAlertResourceMetricOff(value: number | undefined | null): boolean {
|
||||
return typeof value === 'number' && Number.isFinite(value) && value <= 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Value an override editor must stage when a metric is switched back on.
|
||||
* Clearing the override (`undefined`) only re-inherits the default, so it
|
||||
* re-enables nothing when the inherited default is itself off; in that case the
|
||||
* editor has to stage an explicit enabled threshold instead.
|
||||
*/
|
||||
export function resolveAlertResourceMetricEnableValue(
|
||||
inheritedDefault: number | undefined,
|
||||
metric: string,
|
||||
): number | undefined {
|
||||
if (inheritedDefault === undefined || isAlertResourceMetricOff(inheritedDefault)) {
|
||||
return getAlertResourceEnabledDefault(metric);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export function getAlertResourceMetricDelayOverride(
|
||||
metricDelaySeconds: Record<string, number> | undefined,
|
||||
metric: string,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue