diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index b9bbe3ea32a..2b330551ac0 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -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, diff --git a/public/app/features/alerting/unified/components/rules/ReorderRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/ReorderRuleGroupModal.tsx index 063f0b6d6ac..21b2d6b5ac9 100644 --- a/public/app/features/alerting/unified/components/rules/ReorderRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/ReorderRuleGroupModal.tsx @@ -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(false); - const [rulesList, setRulesList] = useState(group.rules); + const [rulesList, setRulesList] = useState(matchedGroup?.rules || []); const styles = useStyles2(getStyles); @@ -129,6 +140,7 @@ const ListItem = ({ provided, rule, isClone = false, isDragging = false }: ListI return (
setIsReorderingGroup(true)} /> diff --git a/public/app/features/alerting/unified/mocks.ts b/public/app/features/alerting/unified/mocks.ts index 3683e9f56ba..11f9c7732f6 100644 --- a/public/app/features/alerting/unified/mocks.ts +++ b/public/app/features/alerting/unified/mocks.ts @@ -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' })] })], diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts index 377877f84a4..4aecb6f0b9a 100644 --- a/public/app/features/alerting/unified/state/actions.ts +++ b/public/app/features/alerting/unified/state/actions.ts @@ -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,