From 7829fced9450062065bf1390f7cf64c1f684c858 Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Fri, 19 Jul 2024 10:55:12 +0100 Subject: [PATCH] Alerting: Hide edit/view rule buttons according to deleting/creating state (#90375) --- .../components/rules/RulesTable.test.tsx | 87 +++++++++++++------ .../unified/components/rules/RulesTable.tsx | 32 +++++-- 2 files changed, 85 insertions(+), 34 deletions(-) diff --git a/public/app/features/alerting/unified/components/rules/RulesTable.test.tsx b/public/app/features/alerting/unified/components/rules/RulesTable.test.tsx index a13c5e1e864..89cbc375386 100644 --- a/public/app/features/alerting/unified/components/rules/RulesTable.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesTable.test.tsx @@ -1,13 +1,8 @@ -import { render } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { Provider } from 'react-redux'; -import { MemoryRouter } from 'react-router-dom'; +import { render, userEvent, screen } from 'test/test-utils'; import { byRole } from 'testing-library-selector'; import { setPluginExtensionsHook } from '@grafana/runtime'; import { mockApi, setupMswServer } from 'app/features/alerting/unified/mockApi'; -import { configureStore } from 'app/store/configureStore'; -import { CombinedRule } from 'app/types/unified-alerting'; import { AlertRuleAction, useAlertRuleAbility } from '../../hooks/useAbilities'; import { getCloudRule, getGrafanaRule, getMockPluginMeta } from '../../mocks'; @@ -36,18 +31,6 @@ const ui = { }, }; -function renderRulesTable(rule: CombinedRule) { - const store = configureStore(); - - render( - - - - - - ); -} - const user = userEvent.setup(); const server = setupMswServer(); @@ -57,6 +40,7 @@ describe('RulesTable RBAC', () => { ...getMockPluginMeta('grafana-incident-app', 'Grafana Incident'), }); }); + describe('Grafana rules action buttons', () => { const grafanaRule = getGrafanaRule({ name: 'Grafana' }); @@ -64,7 +48,8 @@ describe('RulesTable RBAC', () => { mocks.useAlertRuleAbility.mockImplementation((_rule, action) => { return action === AlertRuleAction.Update ? [true, false] : [true, true]; }); - renderRulesTable(grafanaRule); + + render(); expect(ui.actionButtons.edit.query()).not.toBeInTheDocument(); }); @@ -74,7 +59,8 @@ describe('RulesTable RBAC', () => { return action === AlertRuleAction.Delete ? [true, false] : [true, true]; }); - renderRulesTable(grafanaRule); + render(); + await user.click(ui.actionButtons.more.get()); expect(ui.moreActionItems.delete.query()).not.toBeInTheDocument(); @@ -84,7 +70,8 @@ describe('RulesTable RBAC', () => { mocks.useAlertRuleAbility.mockImplementation((_rule, action) => { return action === AlertRuleAction.Update ? [true, true] : [false, false]; }); - renderRulesTable(grafanaRule); + render(); + expect(ui.actionButtons.edit.get()).toBeInTheDocument(); }); @@ -93,12 +80,56 @@ describe('RulesTable RBAC', () => { return action === AlertRuleAction.Delete ? [true, true] : [false, false]; }); - renderRulesTable(grafanaRule); + render(); expect(ui.actionButtons.more.get()).toBeInTheDocument(); await user.click(ui.actionButtons.more.get()); expect(ui.moreActionItems.delete.get()).toBeInTheDocument(); }); + + describe('rules in creating/deleting states', () => { + const { promRule, ...creatingRule } = grafanaRule; + const { rulerRule, ...deletingRule } = grafanaRule; + const rulesSource = 'grafana'; + + /** + * Preloaded state that implies the rulerRules have finished loading + * + * @todo Remove this state and test at a higher level to avoid mocking the store. + * We need to manually populate this, as the component hierarchy expects that we will + * have already called the necessary APIs to get the rulerRules data + */ + const preloadedState = { + unifiedAlerting: { rulerRules: { [rulesSource]: { result: {}, loading: false, dispatched: true } } }, + }; + + beforeEach(() => { + mocks.useAlertRuleAbility.mockImplementation(() => { + return [true, true]; + }); + }); + + it('does not render View button when rule is creating', async () => { + render(, { + // @ts-ignore + preloadedState, + }); + + expect(await screen.findByText('Creating')).toBeInTheDocument(); + expect(ui.actionButtons.view.query()).not.toBeInTheDocument(); + }); + + it('does not render View or Edit button when rule is deleting', async () => { + render(, { + // @ts-ignore + preloadedState, + }); + + expect(await screen.findByText('Deleting')).toBeInTheDocument(); + expect(ui.actionButtons.view.query()).not.toBeInTheDocument(); + expect(ui.actionButtons.edit.query()).not.toBeInTheDocument(); + }); + }); }); describe('Cloud rules action buttons', () => { @@ -109,7 +140,8 @@ describe('RulesTable RBAC', () => { return action === AlertRuleAction.Update ? [true, false] : [true, true]; }); - renderRulesTable(cloudRule); + render(); + expect(ui.actionButtons.edit.query()).not.toBeInTheDocument(); }); @@ -118,7 +150,8 @@ describe('RulesTable RBAC', () => { return action === AlertRuleAction.Delete ? [true, false] : [true, true]; }); - renderRulesTable(cloudRule); + render(); + await user.click(ui.actionButtons.more.get()); expect(ui.moreActionItems.delete.query()).not.toBeInTheDocument(); }); @@ -128,7 +161,8 @@ describe('RulesTable RBAC', () => { return action === AlertRuleAction.Update ? [true, true] : [false, false]; }); - renderRulesTable(cloudRule); + render(); + expect(ui.actionButtons.edit.get()).toBeInTheDocument(); }); @@ -137,7 +171,8 @@ describe('RulesTable RBAC', () => { return action === AlertRuleAction.Delete ? [true, true] : [false, false]; }); - renderRulesTable(cloudRule); + render(); + await user.click(ui.actionButtons.more.get()); expect(ui.moreActionItems.delete.get()).toBeInTheDocument(); }); diff --git a/public/app/features/alerting/unified/components/rules/RulesTable.tsx b/public/app/features/alerting/unified/components/rules/RulesTable.tsx index 8712d292fa9..dfadb75dd8b 100644 --- a/public/app/features/alerting/unified/components/rules/RulesTable.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesTable.tsx @@ -109,18 +109,25 @@ function useColumns(showSummaryColumn: boolean, showGroupColumn: boolean, showNe const { hasRuler, rulerRulesLoaded } = useHasRuler(); return useMemo((): RuleTableColumnProps[] => { + const ruleIsDeleting = (rule: CombinedRule) => { + const { namespace, promRule, rulerRule } = rule; + const { rulesSource } = namespace; + return Boolean(hasRuler(rulesSource) && rulerRulesLoaded(rulesSource) && promRule && !rulerRule); + }; + + const ruleIsCreating = (rule: CombinedRule) => { + const { namespace, promRule, rulerRule } = rule; + const { rulesSource } = namespace; + return Boolean(hasRuler(rulesSource) && rulerRulesLoaded(rulesSource) && rulerRule && !promRule); + }; + const columns: RuleTableColumnProps[] = [ { id: 'state', label: 'State', - // eslint-disable-next-line react/display-name renderCell: ({ data: rule }) => { - const { namespace } = rule; - const { rulesSource } = namespace; - const { promRule, rulerRule } = rule; - - const isDeleting = !!(hasRuler(rulesSource) && rulerRulesLoaded(rulesSource) && promRule && !rulerRule); - const isCreating = !!(hasRuler(rulesSource) && rulerRulesLoaded(rulesSource) && rulerRule && !promRule); + const isDeleting = ruleIsDeleting(rule); + const isCreating = ruleIsCreating(rule); const isPaused = isGrafanaRulerRule(rule.rulerRule) && isGrafanaRulerRulePaused(rule.rulerRule); return ; @@ -226,7 +233,16 @@ function useColumns(showSummaryColumn: boolean, showGroupColumn: boolean, showNe label: 'Actions', // eslint-disable-next-line react/display-name renderCell: ({ data: rule }) => { - return ; + const isDeleting = ruleIsDeleting(rule); + const isCreating = ruleIsCreating(rule); + return ( + + ); }, size: '200px', });