[v11.0.x] Alerting: Fix rules deleting when reordering whilst filtered (#88285)

Alerting: Fix rules deleting when reordering whilst filtered (#88221)

(cherry picked from commit ebdad80dfa)

Co-authored-by: Tom Ratcliffe <tom.ratcliffe@grafana.com>
This commit is contained in:
Gilles De Mey
2024-05-24 15:24:33 +03:00
committed by GitHub
co-authored by Tom Ratcliffe
parent 0d5818a717
commit 9cdfa6bf8a
5 changed files with 87 additions and 6 deletions
@@ -1,5 +1,5 @@
import { SerializedError } from '@reduxjs/toolkit';
import { prettyDOM, render, screen, waitFor } from '@testing-library/react';
import { prettyDOM, render, screen, waitFor, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';
import { TestProvider } from 'test/helpers/TestProvider';
@@ -555,6 +555,63 @@ describe('RuleList', () => {
await waitFor(() => expect(ui.ruleGroup.get()).toHaveTextContent('group-2'));
});
it('uses entire group when reordering after filtering', async () => {
const user = userEvent.setup();
mocks.getAllDataSourcesMock.mockReturnValue([dataSources.prom]);
setDataSourceSrv(new MockDataSourceSrv({ prom: dataSources.prom }));
mocks.api.discoverFeatures.mockResolvedValue({
application: PromApplication.Cortex,
features: {
rulerApiEnabled: true,
},
});
mocks.api.fetchRulerRules.mockImplementation(() => Promise.resolve(someRulerRules));
mocks.api.fetchRules.mockImplementation((dataSourceName: string) => {
if (dataSourceName === GRAFANA_RULES_SOURCE_NAME) {
return Promise.resolve([
mockPromRuleNamespace({
name: 'foofolder',
dataSourceName: GRAFANA_RULES_SOURCE_NAME,
groups: [
mockPromRuleGroup({
name: 'grafana-group',
rules: [
mockPromAlertingRule({
query: '[]',
}),
],
}),
],
}),
]);
} else {
return Promise.resolve([]);
}
});
renderRuleList();
const [firstReorderButton] = await screen.findAllByLabelText(/reorder/i);
const filterInput = ui.rulesFilterInput.get();
await userEvent.type(filterInput, 'alert1a{Enter}');
await user.click(firstReorderButton);
const reorderDialog = await screen.findByRole('dialog');
const alertsInReorder = within(reorderDialog).getAllByTestId('reorder-alert-rule');
// We've filtered down to one rule, but the reorder dialog should still
// have everything in the group visible for reordering
// If this were not the case, rules could be deleted ⚠️
expect(alertsInReorder).toHaveLength(2);
});
describe('edit lotex groups, namespaces', () => {
const testDatasources = {
prom: dataSources.prom,
@@ -13,6 +13,7 @@ import {
import { GrafanaTheme2 } from '@grafana/data';
import { Badge, Icon, Modal, Tooltip, useStyles2 } from '@grafana/ui';
import { useCombinedRuleNamespaces } from 'app/features/alerting/unified/hooks/useCombinedRuleNamespaces';
import { dispatch } from 'app/store/store';
import { CombinedRule, CombinedRuleGroup, CombinedRuleNamespace } from 'app/types/unified-alerting';
@@ -34,8 +35,18 @@ type CombinedRuleWithUID = { uid: string } & CombinedRule;
export const ReorderCloudGroupModal = (props: ModalProps) => {
const { group, namespace, onClose, folderUid } = props;
// The list of rules might have been filtered before we get to this reordering modal
// We need to grab the full (unfiltered) list so we are able to reorder via the API without
// deleting any rules (as they otherwise would have been omitted from the payload)
const unfilteredNamespaces = useCombinedRuleNamespaces();
const matchedNamespace = unfilteredNamespaces.find(
(ns) => ns.rulesSource === namespace.rulesSource && ns.name === namespace.name
);
const matchedGroup = matchedNamespace?.groups.find((g) => g.name === group.name);
const [pending, setPending] = useState<boolean>(false);
const [rulesList, setRulesList] = useState<CombinedRule[]>(group.rules);
const [rulesList, setRulesList] = useState<CombinedRule[]>(matchedGroup?.rules || []);
const styles = useStyles2(getStyles);
@@ -129,6 +140,7 @@ const ListItem = ({ provided, rule, isClone = false, isDragging = false }: ListI
return (
<div
data-testid="reorder-alert-rule"
className={cx(styles.listItem, isClone && 'isClone', isDragging && 'isDragging')}
ref={provided.innerRef}
{...provided.draggableProps}
@@ -105,7 +105,6 @@ export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }:
);
actionIcons.push(
<ActionIcon
aria-label="re-order rules"
data-testid="reorder-group"
key="reorder"
icon="exchange-alt"
@@ -181,11 +180,10 @@ export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }:
);
actionIcons.push(
<ActionIcon
aria-label="re-order rules"
data-testid="reorder-group"
key="reorder"
icon="exchange-alt"
tooltip="re-order rules"
tooltip="reorder rules"
className={styles.rotate90}
onClick={() => setIsReorderingGroup(true)}
/>
@@ -545,7 +545,10 @@ export const somePromRules = (dataSourceName = 'Prometheus'): RuleNamespace[] =>
];
export const someRulerRules: RulerRulesConfigDTO = {
namespace1: [
mockRulerRuleGroup({ name: 'group1', rules: [mockRulerAlertingRule({ alert: 'alert1' })] }),
mockRulerRuleGroup({
name: 'group1',
rules: [mockRulerAlertingRule({ alert: 'alert1' }), mockRulerAlertingRule({ alert: 'alert1a' })],
}),
mockRulerRuleGroup({ name: 'group2', rules: [mockRulerAlertingRule({ alert: 'alert2' })] }),
],
namespace2: [mockRulerRuleGroup({ name: 'group3', rules: [mockRulerAlertingRule({ alert: 'alert3' })] })],
@@ -35,6 +35,7 @@ import {
import { backendSrv } from '../../../../core/services/backend_srv';
import {
logError,
logInfo,
LogMessages,
trackSwitchToPoliciesRouting,
@@ -930,6 +931,16 @@ export const updateRulesOrder = createAsyncThunk(
throw new Error(`Group "${groupName}" not found.`);
}
// We're unlikely to have this happen, as any user of this action should have already ensured
// that the entire group was fetched before sending a new order.
// But as a final safeguard we should fail if we somehow ended up here with a mismatched rules count
// This would indicate an accidental deletion of rules following a frontend bug
if (existingGroup.rules.length !== newRules.length) {
const err = new Error('Rules count mismatch. Please refresh the page and try again.');
logError(err, { namespaceName, groupName });
throw err;
}
const payload: PostableRulerRuleGroupDTO = {
name: existingGroup.name,
interval: existingGroup.interval,