From 7e34c015eee2077138cbe0b07504a11423be601a Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Tue, 5 Nov 2024 09:33:54 +0000 Subject: [PATCH] Alerting: Load alerting state view based on prom primary feature toggle (#95045) --- .../components/rules/RuleListStateView.tsx | 4 +- .../components/rules/RulesTable.test.tsx | 33 +++++++++------- .../unified/components/rules/RulesTable.tsx | 39 ++++++++++--------- 3 files changed, 42 insertions(+), 34 deletions(-) diff --git a/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx b/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx index 03db5349c7e..01199866c97 100644 --- a/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleListStateView.tsx @@ -71,7 +71,7 @@ const STATE_TITLES: Record = { const RulesByState = ({ state, rules }: { state: PromAlertingRuleState; rules: CombinedRule[] }) => { const { page, pageItems, numberOfPages, onPageChange } = usePagination(rules, 1, DEFAULT_PER_PAGE_PAGINATION); - const isFiringState = state !== PromAlertingRuleState.Firing; + const isNotFiringState = state !== PromAlertingRuleState.Firing; const hasRulesMatchingState = rules.length > 0; return ( @@ -82,7 +82,7 @@ const RulesByState = ({ state, rules }: { state: PromAlertingRuleState; rules: C } - collapsed={isFiringState || hasRulesMatchingState} + collapsed={isNotFiringState || hasRulesMatchingState} pagination={ { + beforeEach(() => { + jest.clearAllMocks(); + jest.restoreAllMocks(); + jest.resetAllMocks(); + }); + describe('Grafana rules action buttons', () => { const grafanaRule = getGrafanaRule({ name: 'Grafana' }); @@ -45,7 +51,7 @@ describe('RulesTable RBAC', () => { render(); - expect(ui.actionButtons.edit.query()).not.toBeInTheDocument(); + await waitFor(() => expect(ui.actionButtons.edit.query()).not.toBeInTheDocument()); }); it('Should not render Delete button for users without the delete permission', async () => { @@ -55,18 +61,18 @@ describe('RulesTable RBAC', () => { render(); - await user.click(ui.actionButtons.more.get()); + await user.click(await ui.actionButtons.more.find()); expect(ui.moreActionItems.delete.query()).not.toBeInTheDocument(); }); - it('Should render Edit button for users with the update permission', () => { + it('Should render Edit button for users with the update permission', async () => { mocks.useAlertRuleAbility.mockImplementation((_rule, action) => { return action === AlertRuleAction.Update ? [true, true] : [false, false]; }); render(); - expect(ui.actionButtons.edit.get()).toBeInTheDocument(); + expect(await ui.actionButtons.edit.find()).toBeInTheDocument(); }); it('Should render Delete button for users with the delete permission', async () => { @@ -76,8 +82,7 @@ describe('RulesTable RBAC', () => { render(); - expect(ui.actionButtons.more.get()).toBeInTheDocument(); - await user.click(ui.actionButtons.more.get()); + await user.click(await ui.actionButtons.more.find()); expect(ui.moreActionItems.delete.get()).toBeInTheDocument(); }); @@ -129,14 +134,14 @@ describe('RulesTable RBAC', () => { describe('Cloud rules action buttons', () => { const cloudRule = getCloudRule({ name: 'Cloud' }); - it('Should not render Edit button for users without the update permission', () => { + it('Should not render Edit button for users without the update permission', async () => { mocks.useAlertRuleAbility.mockImplementation((_rule, action) => { return action === AlertRuleAction.Update ? [true, false] : [true, true]; }); render(); - expect(ui.actionButtons.edit.query()).not.toBeInTheDocument(); + await waitFor(() => expect(ui.actionButtons.edit.query()).not.toBeInTheDocument()); }); it('Should not render Delete button for users without the delete permission', async () => { @@ -146,18 +151,18 @@ describe('RulesTable RBAC', () => { render(); - await user.click(ui.actionButtons.more.get()); + await user.click(await ui.actionButtons.more.find()); expect(ui.moreActionItems.delete.query()).not.toBeInTheDocument(); }); - it('Should render Edit button for users with the update permission', () => { + it('Should render Edit button for users with the update permission', async () => { mocks.useAlertRuleAbility.mockImplementation((_rule, action) => { return action === AlertRuleAction.Update ? [true, true] : [false, false]; }); render(); - expect(ui.actionButtons.edit.get()).toBeInTheDocument(); + expect(await ui.actionButtons.edit.find()).toBeInTheDocument(); }); it('Should render Delete button for users with the delete permission', async () => { @@ -167,8 +172,8 @@ describe('RulesTable RBAC', () => { render(); - await user.click(ui.actionButtons.more.get()); - expect(ui.moreActionItems.delete.get()).toBeInTheDocument(); + await user.click(await ui.actionButtons.more.find()); + expect(await ui.moreActionItems.delete.find()).toBeInTheDocument(); }); }); }); diff --git a/public/app/features/alerting/unified/components/rules/RulesTable.tsx b/public/app/features/alerting/unified/components/rules/RulesTable.tsx index 418afa0801b..25523ea81b1 100644 --- a/public/app/features/alerting/unified/components/rules/RulesTable.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesTable.tsx @@ -3,7 +3,7 @@ import { useEffect, useMemo } from 'react'; import Skeleton from 'react-loading-skeleton'; import { GrafanaTheme2 } from '@grafana/data'; -import { Pagination, Tooltip, useStyles2 } from '@grafana/ui'; +import { LoadingPlaceholder, Pagination, Tooltip, useStyles2 } from '@grafana/ui'; import { CombinedRule } from 'app/types/unified-alerting'; import { DEFAULT_PER_PAGE_PAGINATION } from '../../../../../core/constants'; @@ -63,9 +63,18 @@ export const RulesTable = ({ const { pageItems, page, numberOfPages, onPageChange } = usePagination(rules, 1, DEFAULT_PER_PAGE_PAGINATION); - const { result: rulesWithRulerDefinitions, status: rulerRulesLoadingStatus } = useLazyLoadRulerRules(pageItems); + const [lazyLoadRules, { result: rulesWithRulerDefinitions, status: rulerRulesLoadingStatus }] = + useLazyLoadRulerRules(pageItems); + const isLoadingRulerGroup = useMemo( + () => !rulerRulesLoadingStatus || rulerRulesLoadingStatus === 'loading', + [rulerRulesLoadingStatus] + ); - const isLoadingRulerGroup = rulerRulesLoadingStatus === 'loading'; + useEffect(() => { + if (pageItems.length > 0) { + lazyLoadRules.execute(); + } + }, [lazyLoadRules, pageItems, rulerRulesLoadingStatus]); const items = useMemo((): RuleTableItemProps[] => { return rulesWithRulerDefinitions.map((rule, ruleIdx) => { @@ -82,6 +91,10 @@ export const RulesTable = ({ return
{emptyMessage}
; } + if (isLoadingRulerGroup) { + return ; + } + const TableComponent = showGuidelines ? DynamicTableWithGuidelines : DynamicTable; return ( @@ -114,8 +127,11 @@ function useLazyLoadRulerRules(rules: CombinedRule[]) { const [fetchRulerRuleGroup] = useLazyGetRuleGroupForNamespaceQuery(); const [fetchDsFeatures] = useLazyDiscoverDsFeaturesQuery(); - const [actions, state] = useAsync(async () => { - const result = Promise.all( + return useAsync(async () => { + if (!prometheusRulesPrimary) { + return rules; + } + return Promise.all( rules.map(async (rule) => { const dsFeatures = await fetchDsFeatures( { rulesSourceName: getRulesSourceName(rule.namespace.rulesSource) }, @@ -140,20 +156,7 @@ function useLazyLoadRulerRules(rules: CombinedRule[]) { return rule; }) ); - return result; }, rules); - - useEffect(() => { - if (prometheusRulesPrimary) { - actions.execute(); - } else { - // We need to reset the actions to update the rules if they changed - // Otherwise useAsync acts like a cache and always return the first rules passed to it - actions.reset(); - } - }, [rules, actions]); - - return state; } export const getStyles = (theme: GrafanaTheme2) => ({