Skip to content

Commit 3e6d620

Browse files
konrad147soniaAguilarPeironSonia Augilar
authored
Alerting: Remove ruler from alert list view2 (#106778)
* wip * Add working actions for GMA rules based on Prom-only API * Remove Ruler-loader related code for Grafana rules Co-authored-by: Sonia Augilar <[email protected]> * Remove outdated tests * add some comments * remove commented code * remove showLocation property * Add missing mocks in tests * Add showLocation to GrafanaRuleListItem, improve useAbilities, address PR feedback * Enhance GrafanaGroupLoader tests: Add permission checks and More button functionality - Introduced user permission grants for alerting actions in tests. - Added tests for rendering the More button with action menu options. - Verified that each rule has its own action buttons and handles permissions correctly. - Ensured the edit button is not rendered when user lacks edit permissions. - Confirmed the correct menu actions are displayed when the More button is clicked. * Update translations --------- Co-authored-by: Sonia Aguilar <[email protected]> Co-authored-by: Sonia Augilar <[email protected]>
1 parent acdb0e1 commit 3e6d620

File tree

14 files changed

+542
-418
lines changed

14 files changed

+542
-418
lines changed

public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx

Lines changed: 53 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@ import { useRulePluginLinkExtension } from 'app/features/alerting/unified/plugin
1010
import { Rule, RuleGroupIdentifierV2, RuleIdentifier } from 'app/types/unified-alerting';
1111
import { PromAlertingRuleState, RulerRuleDTO } from 'app/types/unified-alerting-dto';
1212

13-
import { AlertRuleAction, useRulerRuleAbility } from '../../hooks/useAbilities';
13+
import {
14+
AlertRuleAction,
15+
skipToken,
16+
useGrafanaPromRuleAbilities,
17+
useRulerRuleAbilities,
18+
} from '../../hooks/useAbilities';
1419
import { createShareLink, isLocalDevEnv, isOpenSourceEdition } from '../../utils/misc';
1520
import * as ruleId from '../../utils/rule-id';
1621
import { prometheusRuleType, rulerRuleType } from '../../utils/rules';
@@ -33,6 +38,8 @@ interface Props {
3338
/**
3439
* Get a list of menu items + divider elements for rendering in an alert rule's
3540
* dropdown menu
41+
* If the consumer of this component comes from the alert list view, we need to use promRule to check abilities and permissions,
42+
* as we have removed all requests to the ruler API in the list view.
3643
*/
3744
const AlertRuleMenu = ({
3845
promRule,
@@ -46,29 +53,51 @@ const AlertRuleMenu = ({
4653
buttonSize,
4754
fill,
4855
}: Props) => {
49-
// check all abilities and permissions
50-
const [pauseSupported, pauseAllowed] = useRulerRuleAbility(rulerRule, groupIdentifier, AlertRuleAction.Pause);
51-
const canPause = pauseSupported && pauseAllowed;
52-
53-
const [deleteSupported, deleteAllowed] = useRulerRuleAbility(rulerRule, groupIdentifier, AlertRuleAction.Delete);
54-
const canDelete = deleteSupported && deleteAllowed;
55-
56-
const [duplicateSupported, duplicateAllowed] = useRulerRuleAbility(
57-
rulerRule,
58-
groupIdentifier,
59-
AlertRuleAction.Duplicate
60-
);
61-
const canDuplicate = duplicateSupported && duplicateAllowed;
62-
63-
const [silenceSupported, silenceAllowed] = useRulerRuleAbility(rulerRule, groupIdentifier, AlertRuleAction.Silence);
64-
const canSilence = silenceSupported && silenceAllowed;
65-
66-
const [exportSupported, exportAllowed] = useRulerRuleAbility(
67-
rulerRule,
68-
groupIdentifier,
69-
AlertRuleAction.ModifyExport
70-
);
71-
const canExport = exportSupported && exportAllowed;
56+
// check all abilities and permissions using rulerRule
57+
const [rulerPauseAbility, rulerDeleteAbility, rulerDuplicateAbility, rulerSilenceAbility, rulerExportAbility] =
58+
useRulerRuleAbilities(rulerRule, groupIdentifier, [
59+
AlertRuleAction.Pause,
60+
AlertRuleAction.Delete,
61+
AlertRuleAction.Duplicate,
62+
AlertRuleAction.Silence,
63+
AlertRuleAction.ModifyExport,
64+
]);
65+
66+
// check all abilities and permissions using promRule
67+
const [
68+
grafanaPauseAbility,
69+
grafanaDeleteAbility,
70+
grafanaDuplicateAbility,
71+
grafanaSilenceAbility,
72+
grafanaExportAbility,
73+
] = useGrafanaPromRuleAbilities(prometheusRuleType.grafana.rule(promRule) ? promRule : skipToken, [
74+
AlertRuleAction.Pause,
75+
AlertRuleAction.Delete,
76+
AlertRuleAction.Duplicate,
77+
AlertRuleAction.Silence,
78+
AlertRuleAction.ModifyExport,
79+
]);
80+
81+
const [pauseSupported, pauseAllowed] = rulerPauseAbility;
82+
const [grafanaPauseSupported, grafanaPauseAllowed] = grafanaPauseAbility;
83+
const canPause = (pauseSupported && pauseAllowed) || (grafanaPauseSupported && grafanaPauseAllowed);
84+
85+
const [deleteSupported, deleteAllowed] = rulerDeleteAbility;
86+
const [grafanaDeleteSupported, grafanaDeleteAllowed] = grafanaDeleteAbility;
87+
const canDelete = (deleteSupported && deleteAllowed) || (grafanaDeleteSupported && grafanaDeleteAllowed);
88+
89+
const [duplicateSupported, duplicateAllowed] = rulerDuplicateAbility;
90+
const [grafanaDuplicateSupported, grafanaDuplicateAllowed] = grafanaDuplicateAbility;
91+
const canDuplicate =
92+
(duplicateSupported && duplicateAllowed) || (grafanaDuplicateSupported && grafanaDuplicateAllowed);
93+
94+
const [silenceSupported, silenceAllowed] = rulerSilenceAbility;
95+
const [grafanaSilenceSupported, grafanaSilenceAllowed] = grafanaSilenceAbility;
96+
const canSilence = (silenceSupported && silenceAllowed) || (grafanaSilenceSupported && grafanaSilenceAllowed);
97+
98+
const [exportSupported, exportAllowed] = rulerExportAbility;
99+
const [grafanaExportSupported, grafanaExportAllowed] = grafanaExportAbility;
100+
const canExport = (exportSupported && exportAllowed) || (grafanaExportSupported && grafanaExportAllowed);
72101

73102
const ruleExtensionLinks = useRulePluginLinkExtension(promRule, groupIdentifier);
74103

public/app/features/alerting/unified/components/rules/RuleDetails.test.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { setupMswServer } from 'app/features/alerting/unified/mockApi';
77

88
import { useIsRuleEditable } from '../../hooks/useIsRuleEditable';
99
import { getCloudRule, getGrafanaRule } from '../../mocks';
10+
import { mimirDataSource } from '../../mocks/server/configure';
1011

1112
import { RuleDetails } from './RuleDetails';
1213

@@ -32,6 +33,8 @@ const ui = {
3233

3334
setupMswServer();
3435

36+
const { dataSource: mimirDs } = mimirDataSource();
37+
3538
beforeAll(() => {
3639
jest.clearAllMocks();
3740
});
@@ -81,7 +84,7 @@ describe('RuleDetails RBAC', () => {
8184
});
8285

8386
describe('Cloud rules action buttons', () => {
84-
const cloudRule = getCloudRule({ name: 'Cloud' });
87+
const cloudRule = getCloudRule({ name: 'Cloud' }, { rulesSource: mimirDs });
8588

8689
it('Should not render Edit button for users with the update permission', async () => {
8790
// Arrange

public/app/features/alerting/unified/components/rules/RulesTable.test.tsx

Lines changed: 89 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,14 @@ import { byRole } from 'testing-library-selector';
44
import { setPluginLinksHook } from '@grafana/runtime';
55
import { setupMswServer } from 'app/features/alerting/unified/mockApi';
66

7-
import { AlertRuleAction, useAlertRuleAbility, useRulerRuleAbility } from '../../hooks/useAbilities';
7+
import {
8+
AlertRuleAction,
9+
useAlertRuleAbility,
10+
useGrafanaPromRuleAbilities,
11+
useGrafanaPromRuleAbility,
12+
useRulerRuleAbilities,
13+
useRulerRuleAbility,
14+
} from '../../hooks/useAbilities';
815
import { getCloudRule, getGrafanaRule } from '../../mocks';
916
import { mimirDataSource } from '../../mocks/server/configure';
1017

@@ -13,11 +20,15 @@ import { RulesTable } from './RulesTable';
1320
jest.mock('../../hooks/useAbilities');
1421

1522
const mocks = {
16-
// This is a bit unfortunate, but we need to mock both abilities
17-
// RuleActionButtons still needs to use the useAlertRuleAbility hook
18-
// whereas AlertRuleMenu has already been refactored to use useRulerRuleAbility
23+
// Mock the hooks that are actually used by the components:
24+
// RuleActionsButtons uses: useAlertRuleAbility (singular)
25+
// AlertRuleMenu uses: useRulerRuleAbilities and useGrafanaPromRuleAbilities (plural)
26+
// We can also use useGrafanaPromRuleAbility (singular) for simpler mocking
1927
useRulerRuleAbility: jest.mocked(useRulerRuleAbility),
2028
useAlertRuleAbility: jest.mocked(useAlertRuleAbility),
29+
useGrafanaPromRuleAbility: jest.mocked(useGrafanaPromRuleAbility),
30+
useRulerRuleAbilities: jest.mocked(useRulerRuleAbilities),
31+
useGrafanaPromRuleAbilities: jest.mocked(useGrafanaPromRuleAbilities),
2132
};
2233

2334
setPluginLinksHook(() => ({
@@ -46,30 +57,55 @@ describe('RulesTable RBAC', () => {
4657
jest.clearAllMocks();
4758
jest.restoreAllMocks();
4859
jest.resetAllMocks();
60+
61+
// Set up default neutral mocks for all hooks
62+
// Singular hooks (used by RuleActionsButtons and can simplify mocking)
63+
mocks.useAlertRuleAbility.mockReturnValue([false, false]);
64+
mocks.useRulerRuleAbility.mockReturnValue([false, false]);
65+
mocks.useGrafanaPromRuleAbility.mockReturnValue([false, false]);
66+
67+
// Plural hooks (used by AlertRuleMenu) - need to return arrays based on input actions
68+
mocks.useRulerRuleAbilities.mockImplementation((_rule, _groupIdentifier, actions) => {
69+
return actions.map(() => [false, false]);
70+
});
71+
mocks.useGrafanaPromRuleAbilities.mockImplementation((_rule, actions) => {
72+
return actions.map(() => [false, false]);
73+
});
4974
});
5075

5176
describe('Grafana rules action buttons', () => {
5277
const grafanaRule = getGrafanaRule({ name: 'Grafana' });
5378

5479
it('Should not render Edit button for users without the update permission', async () => {
55-
mocks.useRulerRuleAbility.mockImplementation((_rule, _groupIdentifier, action) => {
80+
// Mock the specific hooks needed for Grafana rules
81+
// Using singular hook for simpler mocking
82+
mocks.useAlertRuleAbility.mockImplementation((rule, action) => {
5683
return action === AlertRuleAction.Update ? [true, false] : [true, true];
5784
});
58-
mocks.useAlertRuleAbility.mockImplementation((_rule, action) => {
85+
mocks.useGrafanaPromRuleAbility.mockImplementation((rule, action) => {
5986
return action === AlertRuleAction.Update ? [true, false] : [true, true];
6087
});
88+
// Still need plural hook for AlertRuleMenu component
89+
mocks.useGrafanaPromRuleAbilities.mockImplementation((rule, actions) => {
90+
return actions.map((action) => {
91+
return action === AlertRuleAction.Update ? [true, false] : [true, true];
92+
});
93+
});
6194

6295
render(<RulesTable rules={[grafanaRule]} />);
6396

6497
await waitFor(() => expect(ui.actionButtons.edit.query()).not.toBeInTheDocument());
6598
});
6699

67100
it('Should not render Delete button for users without the delete permission', async () => {
68-
mocks.useRulerRuleAbility.mockImplementation((_rule, _groupIdentifier, action) => {
101+
// Mock the specific hooks needed for Grafana rules
102+
mocks.useAlertRuleAbility.mockImplementation((rule, action) => {
69103
return action === AlertRuleAction.Delete ? [true, false] : [true, true];
70104
});
71-
mocks.useAlertRuleAbility.mockImplementation((_rule, action) => {
72-
return action === AlertRuleAction.Delete ? [true, false] : [true, true];
105+
mocks.useGrafanaPromRuleAbilities.mockImplementation((rule, actions) => {
106+
return actions.map((action) => {
107+
return action === AlertRuleAction.Delete ? [true, false] : [true, true];
108+
});
73109
});
74110

75111
render(<RulesTable rules={[grafanaRule]} />);
@@ -80,11 +116,14 @@ describe('RulesTable RBAC', () => {
80116
});
81117

82118
it('Should render Edit button for users with the update permission', async () => {
83-
mocks.useRulerRuleAbility.mockImplementation((_rule, _groupIdentifier, action) => {
119+
// Mock the specific hooks needed for Grafana rules
120+
mocks.useAlertRuleAbility.mockImplementation((rule, action) => {
84121
return action === AlertRuleAction.Update ? [true, true] : [false, false];
85122
});
86-
mocks.useAlertRuleAbility.mockImplementation((_rule, action) => {
87-
return action === AlertRuleAction.Update ? [true, true] : [false, false];
123+
mocks.useGrafanaPromRuleAbilities.mockImplementation((rule, actions) => {
124+
return actions.map((action) => {
125+
return action === AlertRuleAction.Update ? [true, true] : [false, false];
126+
});
88127
});
89128

90129
render(<RulesTable rules={[grafanaRule]} />);
@@ -93,11 +132,14 @@ describe('RulesTable RBAC', () => {
93132
});
94133

95134
it('Should render Delete button for users with the delete permission', async () => {
96-
mocks.useRulerRuleAbility.mockImplementation((_rule, _groupIdentifier, action) => {
135+
// Mock the specific hooks needed for Grafana rules
136+
mocks.useAlertRuleAbility.mockImplementation((rule, action) => {
97137
return action === AlertRuleAction.Delete ? [true, true] : [false, false];
98138
});
99-
mocks.useAlertRuleAbility.mockImplementation((_rule, action) => {
100-
return action === AlertRuleAction.Delete ? [true, true] : [false, false];
139+
mocks.useGrafanaPromRuleAbilities.mockImplementation((rule, actions) => {
140+
return actions.map((action) => {
141+
return action === AlertRuleAction.Delete ? [true, true] : [false, false];
142+
});
101143
});
102144

103145
render(<RulesTable rules={[grafanaRule]} />);
@@ -123,11 +165,15 @@ describe('RulesTable RBAC', () => {
123165
};
124166

125167
beforeEach(() => {
126-
mocks.useRulerRuleAbility.mockImplementation(() => {
127-
return [true, true];
168+
// Mock all hooks needed for the creating/deleting state tests
169+
mocks.useRulerRuleAbility.mockImplementation(() => [true, true]);
170+
mocks.useAlertRuleAbility.mockImplementation(() => [true, true]);
171+
// Mock plural hooks for AlertRuleMenu
172+
mocks.useRulerRuleAbilities.mockImplementation((_rule, _groupIdentifier, actions) => {
173+
return actions.map(() => [true, true]);
128174
});
129-
mocks.useAlertRuleAbility.mockImplementation(() => {
130-
return [true, true];
175+
mocks.useGrafanaPromRuleAbilities.mockImplementation((_rule, actions) => {
176+
return actions.map(() => [true, true]);
131177
});
132178
});
133179

@@ -164,6 +210,12 @@ describe('RulesTable RBAC', () => {
164210
mocks.useAlertRuleAbility.mockImplementation((_rule, action) => {
165211
return action === AlertRuleAction.Update ? [true, false] : [true, true];
166212
});
213+
// Cloud rules only need useRulerRuleAbilities mock (useGrafanaPromRuleAbilities gets skipToken)
214+
mocks.useRulerRuleAbilities.mockImplementation((_rule, _groupIdentifier, actions) => {
215+
return actions.map((action) => {
216+
return action === AlertRuleAction.Update ? [true, false] : [true, true];
217+
});
218+
});
167219

168220
render(<RulesTable rules={[cloudRule]} />);
169221

@@ -177,6 +229,12 @@ describe('RulesTable RBAC', () => {
177229
mocks.useAlertRuleAbility.mockImplementation((_rule, action) => {
178230
return action === AlertRuleAction.Delete ? [true, false] : [true, true];
179231
});
232+
// Cloud rules only need useRulerRuleAbilities mock (useGrafanaPromRuleAbilities gets skipToken)
233+
mocks.useRulerRuleAbilities.mockImplementation((_rule, _groupIdentifier, actions) => {
234+
return actions.map((action) => {
235+
return action === AlertRuleAction.Delete ? [true, false] : [true, true];
236+
});
237+
});
180238

181239
render(<RulesTable rules={[cloudRule]} />);
182240

@@ -191,6 +249,12 @@ describe('RulesTable RBAC', () => {
191249
mocks.useAlertRuleAbility.mockImplementation((_rule, action) => {
192250
return action === AlertRuleAction.Update ? [true, true] : [false, false];
193251
});
252+
// Cloud rules only need useRulerRuleAbilities mock (useGrafanaPromRuleAbilities gets skipToken)
253+
mocks.useRulerRuleAbilities.mockImplementation((_rule, _groupIdentifier, actions) => {
254+
return actions.map((action) => {
255+
return action === AlertRuleAction.Update ? [true, true] : [false, false];
256+
});
257+
});
194258

195259
render(<RulesTable rules={[cloudRule]} />);
196260

@@ -204,6 +268,12 @@ describe('RulesTable RBAC', () => {
204268
mocks.useAlertRuleAbility.mockImplementation((_rule, action) => {
205269
return action === AlertRuleAction.Delete ? [true, true] : [false, false];
206270
});
271+
// Cloud rules only need useRulerRuleAbilities mock (useGrafanaPromRuleAbilities gets skipToken)
272+
mocks.useRulerRuleAbilities.mockImplementation((_rule, _groupIdentifier, actions) => {
273+
return actions.map((action) => {
274+
return action === AlertRuleAction.Delete ? [true, true] : [false, false];
275+
});
276+
});
207277

208278
render(<RulesTable rules={[cloudRule]} />);
209279

0 commit comments

Comments
 (0)