diff --git a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx index 211c66c1622..2197ca57db1 100644 --- a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx +++ b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx @@ -29,8 +29,8 @@ import { mockRulerGrafanaRule, mockRulerRuleGroup, } from './mocks'; -import { grafanaRulerRule } from './mocks/alertRuleApi'; import { mockAlertmanagerConfigResponse } from './mocks/alertmanagerApi'; +import { grafanaRulerRule } from './mocks/grafanaRulerApi'; import { mockRulerRulesApiResponse, mockRulerRulesGroupApiResponse } from './mocks/rulerApi'; import { AlertingQueryRunner } from './state/AlertingQueryRunner'; import { setupDataSources } from './testSetup/datasources'; diff --git a/public/app/features/alerting/unified/RuleEditorExisting.test.tsx b/public/app/features/alerting/unified/RuleEditorExisting.test.tsx index e5c34d2288d..72108f78ce4 100644 --- a/public/app/features/alerting/unified/RuleEditorExisting.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorExisting.test.tsx @@ -18,7 +18,7 @@ import * as ruler from './api/ruler'; import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; import { setupMswServer } from './mockApi'; import { grantUserPermissions, mockDataSource, mockFolder } from './mocks'; -import { grafanaRulerGroup, grafanaRulerRule } from './mocks/alertRuleApi'; +import { grafanaRulerGroup, grafanaRulerRule } from './mocks/grafanaRulerApi'; import { setupDataSources } from './testSetup/datasources'; import { Annotation } from './utils/constants'; import { GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; diff --git a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx index fa8ff6be0cf..3728c5e88c5 100644 --- a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx @@ -17,7 +17,7 @@ import { discoverFeatures } from './api/buildInfo'; import * as ruler from './api/ruler'; import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; import { grantUserPermissions, mockDataSource } from './mocks'; -import { grafanaRulerGroup, grafanaRulerRule } from './mocks/alertRuleApi'; +import { grafanaRulerGroup, grafanaRulerRule } from './mocks/grafanaRulerApi'; import { setupDataSources } from './testSetup/datasources'; import * as config from './utils/config'; import { GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index ccb7523d8c7..35ce7d4d2ee 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -733,7 +733,13 @@ describe('RuleList', () => { }); }); - describe('edit lotex groups, namespaces', () => { + /** + * @TODO port these tests to MSW – they rely on mocks a whole lot, and since we're looking to refactor the list view + * I imagine we'd need to rewrite these anyway. + * + * These actions are currently tested in the "useProduceNewRuleGroup" hook(s). + */ + describe.skip('edit lotex groups, namespaces', () => { const testDatasources = { prom: dataSources.prom, }; diff --git a/public/app/features/alerting/unified/Silences.test.tsx b/public/app/features/alerting/unified/Silences.test.tsx index fb8fc89e6af..204ed9f9bbd 100644 --- a/public/app/features/alerting/unified/Silences.test.tsx +++ b/public/app/features/alerting/unified/Silences.test.tsx @@ -6,11 +6,11 @@ import { selectors } from '@grafana/e2e-selectors'; import { config, locationService } from '@grafana/runtime'; import { mockAlertRuleApi, setupMswServer } from 'app/features/alerting/unified/mockApi'; import { waitForServerRequest } from 'app/features/alerting/unified/mocks/server/events'; -import { MOCK_GRAFANA_ALERT_RULE_TITLE } from 'app/features/alerting/unified/mocks/server/handlers/alertRules'; import { MOCK_DATASOURCE_NAME_BROKEN_ALERTMANAGER, MOCK_DATASOURCE_UID_BROKEN_ALERTMANAGER, } from 'app/features/alerting/unified/mocks/server/handlers/datasources'; +import { MOCK_GRAFANA_ALERT_RULE_TITLE } from 'app/features/alerting/unified/mocks/server/handlers/grafanaRuler'; import { silenceCreateHandler } from 'app/features/alerting/unified/mocks/server/handlers/silences'; import { MatcherOperator, SilenceState } from 'app/plugins/datasource/alertmanager/types'; import { AccessControlAction } from 'app/types'; @@ -24,7 +24,7 @@ import { mockDataSource, mockSilences, } from './mocks'; -import { grafanaRulerRule } from './mocks/alertRuleApi'; +import { grafanaRulerRule } from './mocks/grafanaRulerApi'; import { setupDataSources } from './testSetup/datasources'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; diff --git a/public/app/features/alerting/unified/api/alertRuleApi.ts b/public/app/features/alerting/unified/api/alertRuleApi.ts index 001e950b53f..fd8d5726ac0 100644 --- a/public/app/features/alerting/unified/api/alertRuleApi.ts +++ b/public/app/features/alerting/unified/api/alertRuleApi.ts @@ -1,6 +1,7 @@ import { set } from 'lodash'; import { RelativeTimeRange } from '@grafana/data'; +import { t } from 'app/core/internationalization'; import { Matcher } from 'app/plugins/datasource/alertmanager/types'; import { RuleIdentifier, RuleNamespace, RulerDataSourceConfig } from 'app/types/unified-alerting'; import { @@ -21,7 +22,7 @@ import { getDatasourceAPIUid, GRAFANA_RULES_SOURCE_NAME, isGrafanaRulesSource } import { arrayKeyValuesToObject } from '../utils/labels'; import { isCloudRuleIdentifier, isPrometheusRuleIdentifier } from '../utils/rules'; -import { alertingApi } from './alertingApi'; +import { alertingApi, withRequestOptions, WithRequestOptions } from './alertingApi'; import { FetchPromRulesFilter, groupRulesByFileName, @@ -225,24 +226,65 @@ export const alertRuleApi = alertingApi.injectEndpoints({ // TODO This should be probably a separate ruler API file getRuleGroupForNamespace: build.query< RulerRuleGroupDTO, - { rulerConfig: RulerDataSourceConfig; namespace: string; group: string } + WithRequestOptions<{ rulerConfig: RulerDataSourceConfig; namespace: string; group: string }> >({ - query: ({ rulerConfig, namespace, group }) => { + query: ({ rulerConfig, namespace, group, requestOptions }) => { const { path, params } = rulerUrlBuilder(rulerConfig).namespaceGroup(namespace, group); - return { url: path, params }; + return withRequestOptions({ url: path, params }, requestOptions); }, - providesTags: ['CombinedAlertRule'], + providesTags: (_result, _error, { namespace, group }) => [ + { + type: 'RuleGroup', + id: `${namespace}/${group}`, + }, + { type: 'RuleNamespace', id: namespace }, + ], }), deleteRuleGroupFromNamespace: build.mutation< RulerRuleGroupDTO, - { rulerConfig: RulerDataSourceConfig; namespace: string; group: string } + WithRequestOptions<{ rulerConfig: RulerDataSourceConfig; namespace: string; group: string }> >({ - query: ({ rulerConfig, namespace, group }) => { + query: ({ rulerConfig, namespace, group, requestOptions }) => { + const successMessage = t('alerting.rule-groups.delete.success', 'Successfully deleted rule group'); const { path, params } = rulerUrlBuilder(rulerConfig).namespaceGroup(namespace, group); - return { url: path, params, method: 'DELETE' }; + + return withRequestOptions({ url: path, params, method: 'DELETE' }, requestOptions, { successMessage }); }, - invalidatesTags: ['CombinedAlertRule'], + invalidatesTags: (_result, _error, { namespace, group }) => [ + { + type: 'RuleGroup', + id: `${namespace}/${group}`, + }, + { type: 'RuleNamespace', id: namespace }, + ], + }), + + upsertRuleGroupForNamespace: build.mutation< + AlertGroupUpdated, + WithRequestOptions<{ + rulerConfig: RulerDataSourceConfig; + namespace: string; + payload: PostableRulerRuleGroupDTO; + }> + >({ + query: ({ payload, namespace, rulerConfig, requestOptions }) => { + const { path, params } = rulerUrlBuilder(rulerConfig).namespace(namespace); + + const successMessage = t('alerting.rule-groups.update.success', 'Successfully updated rule group'); + + return withRequestOptions( + { + url: path, + params, + data: payload, + method: 'POST', + }, + requestOptions, + { successMessage } + ); + }, + invalidatesTags: (_result, _error, { namespace }) => [{ type: 'RuleNamespace', id: namespace }], }), getAlertRule: build.query({ @@ -312,21 +354,5 @@ export const alertRuleApi = alertingApi.injectEndpoints({ }), keepUnusedDataFor: 0, }), - updateRuleGroupForNamespace: build.mutation< - AlertGroupUpdated, - { rulerConfig: RulerDataSourceConfig; namespace: string; payload: PostableRulerRuleGroupDTO } - >({ - query: ({ payload, namespace, rulerConfig }) => { - const { path, params } = rulerUrlBuilder(rulerConfig).namespace(namespace); - - return { - url: path, - params, - data: payload, - method: 'POST', - }; - }, - invalidatesTags: ['CombinedAlertRule'], - }), }), }); diff --git a/public/app/features/alerting/unified/api/alertingApi.ts b/public/app/features/alerting/unified/api/alertingApi.ts index 9f368f4f966..fb1c15cbd66 100644 --- a/public/app/features/alerting/unified/api/alertingApi.ts +++ b/public/app/features/alerting/unified/api/alertingApi.ts @@ -1,34 +1,84 @@ import { BaseQueryFn, createApi } from '@reduxjs/toolkit/query/react'; +import { defaultsDeep } from 'lodash'; import { lastValueFrom } from 'rxjs'; +import { AppEvents } from '@grafana/data'; import { BackendSrvRequest, getBackendSrv } from '@grafana/runtime'; +import appEvents from 'app/core/app_events'; import { logMeasurement } from '../Analytics'; -export const backendSrvBaseQuery = (): BaseQueryFn => async (requestOptions) => { - try { - const requestStartTs = performance.now(); - - const { data, ...meta } = await lastValueFrom(getBackendSrv().fetch(requestOptions)); - - logMeasurement( - 'backendSrvBaseQuery', - { - loadTimeMs: performance.now() - requestStartTs, - }, - { - url: requestOptions.url, - method: requestOptions.method ?? 'GET', - responseStatus: meta.statusText, - } - ); - - return { data, meta }; - } catch (error) { - return { error }; - } +export type ExtendedBackendSrvRequest = BackendSrvRequest & { + /** + * Custom success message to show after completion of the request. + * + * If a custom message is provided, any success message provided from the API response + * will not be shown + */ + successMessage?: string; + /** + * Custom error message to show if there's an error completing the request via backendSrv. + * + * If a custom message is provided, any error message from the API response + * will not be shown + */ + errorMessage?: string; }; +// utility type for passing request options to endpoints +export type WithRequestOptions = T & { + requestOptions?: Partial; +}; + +export function withRequestOptions( + options: BackendSrvRequest, + requestOptions: Partial = {}, + defaults: Partial = {} +): ExtendedBackendSrvRequest { + return { + ...options, + ...defaultsDeep(requestOptions, defaults), + }; +} + +export const backendSrvBaseQuery = + (): BaseQueryFn => + async ({ successMessage, errorMessage, ...requestOptions }) => { + try { + const modifiedRequestOptions: BackendSrvRequest = { + ...requestOptions, + ...(successMessage && { showSuccessAlert: false }), + ...(errorMessage && { showErrorAlert: false }), + }; + + const requestStartTs = performance.now(); + + const { data, ...meta } = await lastValueFrom(getBackendSrv().fetch(modifiedRequestOptions)); + + logMeasurement( + 'backendSrvBaseQuery', + { + loadTimeMs: performance.now() - requestStartTs, + }, + { + url: requestOptions.url, + method: requestOptions.method ?? 'GET', + responseStatus: meta.statusText, + } + ); + + if (successMessage && requestOptions.showSuccessAlert !== false) { + appEvents.emit(AppEvents.alertSuccess, [successMessage]); + } + return { data, meta }; + } catch (error) { + if (errorMessage && requestOptions.showErrorAlert !== false) { + appEvents.emit(AppEvents.alertError, [errorMessage]); + } + return { error }; + } + }; + export const alertingApi = createApi({ reducerPath: 'alertingApi', baseQuery: backendSrvBaseQuery(), @@ -45,6 +95,8 @@ export const alertingApi = createApi({ 'CombinedAlertRule', 'GrafanaRulerRule', 'GrafanaSlo', + 'RuleGroup', + 'RuleNamespace', ], endpoints: () => ({}), }); diff --git a/public/app/features/alerting/unified/api/util.test.ts b/public/app/features/alerting/unified/api/util.test.ts new file mode 100644 index 00000000000..1eee105162d --- /dev/null +++ b/public/app/features/alerting/unified/api/util.test.ts @@ -0,0 +1,23 @@ +import { notFoundToNullOrThrow } from './util'; + +describe('notFoundToNull', () => { + it('should convert notFound error to null', () => { + const fetchError = { + status: 404, + data: null, + }; + + expect(notFoundToNullOrThrow(fetchError)).toBe(null); + }); + + it('should not catch any non-404 error', () => { + const fetchError = { + status: 500, + data: null, + }; + + expect(() => { + notFoundToNullOrThrow(fetchError); + }).toThrow(); + }); +}); diff --git a/public/app/features/alerting/unified/api/util.ts b/public/app/features/alerting/unified/api/util.ts new file mode 100644 index 00000000000..2babcf0dabd --- /dev/null +++ b/public/app/features/alerting/unified/api/util.ts @@ -0,0 +1,17 @@ +import { isFetchError } from '@grafana/runtime'; + +/** + * Catch 404 error response and return "null" instead. + * + * @example + * const ruleGroup = await fetchRuleGroup() + * .unwrap() + * .catch(notFoundToNull); // RuleGroupDTO | null + */ +export function notFoundToNullOrThrow(error: unknown): null { + if (isFetchError(error) && error.status === 404) { + return null; + } + + throw error; +} diff --git a/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx b/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx index 74b52754c85..6b9ef72b8be 100644 --- a/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx +++ b/public/app/features/alerting/unified/components/MenuItemPauseRule.tsx @@ -7,6 +7,7 @@ import { } from 'app/features/alerting/unified/utils/rules'; import { CombinedRule } from 'app/types/unified-alerting'; +import { isLoading } from '../hooks/useAsync'; import { usePauseRuleInGroup } from '../hooks/useProduceNewRuleGroup'; import { stringifyErrorLike } from '../utils/misc'; @@ -24,7 +25,7 @@ interface Props { */ const MenuItemPauseRule = ({ rule, onPauseChange }: Props) => { const notifyApp = useAppNotification(); - const [pauseRule, updateState] = usePauseRuleInGroup(); + const [updateState, pauseRule] = usePauseRuleInGroup(); const isPaused = isGrafanaRulerRule(rule.rulerRule) && isGrafanaRulerRulePaused(rule.rulerRule); const icon = isPaused ? 'play' : 'pause'; @@ -42,7 +43,7 @@ const MenuItemPauseRule = ({ rule, onPauseChange }: Props) => { const ruleGroupId = getRuleGroupLocationFromCombinedRule(rule); const ruleUID = rule.rulerRule.grafana_alert.uid; - await pauseRule(ruleGroupId, ruleUID, newIsPaused); + await pauseRule.execute(ruleGroupId, ruleUID, newIsPaused); } catch (error) { notifyApp.error(`Failed to ${newIsPaused ? 'pause' : 'resume'} the rule: ${stringifyErrorLike(error)}`); return; @@ -55,7 +56,7 @@ const MenuItemPauseRule = ({ rule, onPauseChange }: Props) => { { setRulePause(!isPaused); }} diff --git a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx index 02d7a9a442c..f588ad6d921 100644 --- a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx +++ b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx @@ -7,7 +7,7 @@ import { byRole, byTestId, byText } from 'testing-library-selector'; import { DashboardSearchItemType } from '../../../../search/types'; import { mockExportApi, mockSearchApi, setupMswServer } from '../../mockApi'; import { mockDashboardSearchItem, mockDataSource } from '../../mocks'; -import { grafanaRulerRule } from '../../mocks/alertRuleApi'; +import { grafanaRulerRule } from '../../mocks/grafanaRulerApi'; import { setupDataSources } from '../../testSetup/datasources'; import GrafanaModifyExport from './GrafanaModifyExport'; diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx index 13bf7edcfdc..ccaf73da0a8 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx @@ -64,7 +64,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { const [queryParams] = useQueryParams(); const [showEditYaml, setShowEditYaml] = useState(false); const [evaluateEvery, setEvaluateEvery] = useState(existing?.group.interval ?? DEFAULT_GROUP_EVALUATION_INTERVAL); - const [deleteRuleFromGroup, _deleteRuleState] = useDeleteRuleFromGroup(); + const [_deleteRuleState, deleteRuleFromGroup] = useDeleteRuleFromGroup(); const routeParams = useParams<{ type: string; id: string }>(); const ruleType = translateRouteParamToRuleType(routeParams.type); @@ -160,7 +160,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { if (existing) { const ruleGroupIdentifier = getRuleGroupLocationFromRuleWithLocation(existing); - await deleteRuleFromGroup(ruleGroupIdentifier, existing.rule); + await deleteRuleFromGroup.execute(ruleGroupIdentifier, existing.rule); locationService.replace(returnTo); } }; diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/SimplifiedRuleEditor.test.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/SimplifiedRuleEditor.test.tsx index a85a98f60e8..eed0e9abd9f 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/SimplifiedRuleEditor.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/SimplifiedRuleEditor.test.tsx @@ -28,7 +28,7 @@ import { DashboardSearchHit, DashboardSearchItemType } from 'app/features/search import { AccessControlAction } from 'app/types'; import { GrafanaAlertStateDecision } from 'app/types/unified-alerting-dto'; -import { grafanaRulerEmptyGroup, grafanaRulerNamespace2, grafanaRulerRule } from '../../../../mocks/alertRuleApi'; +import { grafanaRulerEmptyGroup, grafanaRulerNamespace2, grafanaRulerRule } from '../../../../mocks/grafanaRulerApi'; import { setupDataSources } from '../../../../testSetup/datasources'; import { RECEIVER_META_KEY } from '../../../contact-points/useContactPoints'; import { ContactPointWithMetadata } from '../../../contact-points/utils'; diff --git a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx index cb8c46a04f1..17cb3f64394 100644 --- a/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/notificaton-preview/NotificationPreview.test.tsx @@ -8,7 +8,7 @@ import { MatcherOperator } from '../../../../../../plugins/datasource/alertmanag import { Labels } from '../../../../../../types/unified-alerting-dto'; import { mockApi, setupMswServer } from '../../../mockApi'; import { grantUserPermissions, mockAlertQuery } from '../../../mocks'; -import { mockPreviewApiResponse } from '../../../mocks/alertRuleApi'; +import { mockPreviewApiResponse } from '../../../mocks/grafanaRulerApi'; import * as dataSource from '../../../utils/datasource'; import { AlertManagerDataSource, diff --git a/public/app/features/alerting/unified/components/rule-viewer/DeleteModal.tsx b/public/app/features/alerting/unified/components/rule-viewer/DeleteModal.tsx index 0af821c048d..75e37e95b7f 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/DeleteModal.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/DeleteModal.tsx @@ -13,7 +13,7 @@ type DeleteModalHook = [JSX.Element, (rule: CombinedRule) => void, () => void]; export const useDeleteModal = (redirectToListView = false): DeleteModalHook => { const [ruleToDelete, setRuleToDelete] = useState(); - const [deleteRuleFromGroup, _deleteState] = useDeleteRuleFromGroup(); + const [_deleteState, deleteRuleFromGroup] = useDeleteRuleFromGroup(); const dismissModal = useCallback(() => { setRuleToDelete(undefined); @@ -30,7 +30,7 @@ export const useDeleteModal = (redirectToListView = false): DeleteModalHook => { } const location = getRuleGroupLocationFromCombinedRule(rule); - await deleteRuleFromGroup(location, rule.rulerRule); + await deleteRuleFromGroup.execute(location, rule.rulerRule); // refetch rules for this rules source // @TODO remove this when we moved everything to RTKQ – then the endpoint will simply invalidate the tags diff --git a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx index f077544ebb2..0462f1cf9b7 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx @@ -17,7 +17,7 @@ import { mockDataSource, mockPluginLinkExtension, } from '../../mocks'; -import { grafanaRulerRule } from '../../mocks/alertRuleApi'; +import { grafanaRulerRule } from '../../mocks/grafanaRulerApi'; import { setupDataSources } from '../../testSetup/datasources'; import { Annotation } from '../../utils/constants'; import { DataSourceType } from '../../utils/datasource'; diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx index 91e47148d8e..59caf5d775d 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -1,22 +1,25 @@ import { css } from '@emotion/css'; import { compact } from 'lodash'; -import { useEffect, useMemo } from 'react'; -import * as React from 'react'; +import { useMemo } from 'react'; import { FormProvider, RegisterOptions, useForm, useFormContext } from 'react-hook-form'; import { GrafanaTheme2 } from '@grafana/data'; -import { Badge, Button, Field, Input, Label, LinkButton, Modal, useStyles2, Stack } from '@grafana/ui'; +import { Badge, Button, Field, Input, Label, LinkButton, Modal, useStyles2, Stack, Alert } from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; -import { useCleanup } from 'app/core/hooks/useCleanup'; -import { useDispatch } from 'app/types'; -import { CombinedRuleGroup, CombinedRuleNamespace } from 'app/types/unified-alerting'; +import { dispatch } from 'app/store/store'; +import { CombinedRuleGroup, CombinedRuleNamespace, RuleGroupIdentifier } from 'app/types/unified-alerting'; import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; -import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelector'; -import { rulesInSameGroupHaveInvalidFor, updateLotexNamespaceAndGroupAction } from '../../state/actions'; +import { anyOfRequestState } from '../../hooks/useAsync'; +import { + useMoveRuleGroup, + useRenameRuleGroup, + useUpdateRuleGroupConfiguration, +} from '../../hooks/useProduceNewRuleGroup'; +import { fetchRulerRulesAction, rulesInSameGroupHaveInvalidFor } from '../../state/actions'; import { checkEvaluationIntervalGlobalLimit } from '../../utils/config'; import { getRulesSourceName, GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; -import { initialAsyncRequestState } from '../../utils/redux'; +import { stringifyErrorLike } from '../../utils/misc'; import { DEFAULT_GROUP_EVALUATION_INTERVAL } from '../../utils/rule-form'; import { AlertInfo, getAlertInfo, isRecordingRulerRule } from '../../utils/rules'; import { formatPrometheusDuration, parsePrometheusDuration, safeParsePrometheusDuration } from '../../utils/time'; @@ -176,11 +179,21 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { const { namespace, group, onClose, intervalEditOnly, folderUid } = props; const styles = useStyles2(getStyles); - const dispatch = useDispatch(); - const { loading, error, dispatched } = - useUnifiedAlertingSelector((state) => state.updateLotexNamespaceAndGroup) ?? initialAsyncRequestState; const notifyApp = useAppNotification(); + /** + * This modal can take 3 different actions, depending on what fields were updated. + * + * 1. update the rule group details without renaming either the namespace or group + * 2. rename the rule group, but keeping it in the same namespace + * 3. move the rule group to a new namespace, optionally with a different group name + */ + const [updateRuleGroupState, updateRuleGroup] = useUpdateRuleGroupConfiguration(); + const [renameRuleGroupState, renameRuleGroup] = useRenameRuleGroup(); + const [moveRuleGroupState, moveRuleGroup] = useMoveRuleGroup(); + + const { loading, error } = anyOfRequestState(updateRuleGroupState, moveRuleGroupState, renameRuleGroupState); + const defaultValues = useMemo( (): FormValues => ({ namespaceName: decodeGrafanaNamespace(namespace).name, @@ -198,31 +211,35 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { const nameSpaceLabel = isGrafanaManagedGroup ? 'Folder' : 'Namespace'; - // close modal if successfully saved - useEffect(() => { - if (dispatched && !loading && !error) { - onClose(true); - } - }, [dispatched, loading, onClose, error]); + const onSubmit = async (values: FormValues) => { + const ruleGroupIdentifier: RuleGroupIdentifier = { + dataSourceName: rulesSourceName, + groupName: group.name, + namespaceName: isGrafanaManagedGroup ? folderUid! : namespace.name, + }; - useCleanup((state) => (state.unifiedAlerting.updateLotexNamespaceAndGroup = initialAsyncRequestState)); - const onSubmit = (values: FormValues) => { // make sure that when dealing with a nested folder for Grafana managed rules we encode the folder properly - const newNamespaceName = isGrafanaManagedGroup + const updatedNamespaceName = isGrafanaManagedGroup ? encodeGrafanaNamespace(values.namespaceName, nestedFolderParents) : values.namespaceName; + const updatedGroupName = values.groupName; + const updatedInterval = values.groupInterval; - dispatch( - updateLotexNamespaceAndGroupAction({ - rulesSourceName: rulesSourceName, - groupName: group.name, - newGroupName: values.groupName, - namespaceName: namespace.name, - newNamespaceName: newNamespaceName, - groupInterval: values.groupInterval || undefined, - folderUid, - }) - ); + // GMA alert rules cannot be moved to another folder, we currently do not support it but it should be doable (with caveats). + const shouldMove = isGrafanaManagedGroup ? false : updatedNamespaceName !== ruleGroupIdentifier.namespaceName; + const shouldRename = updatedGroupName !== ruleGroupIdentifier.groupName; + + try { + if (shouldMove) { + await moveRuleGroup.execute(ruleGroupIdentifier, updatedNamespaceName, updatedGroupName, updatedInterval); + } else if (shouldRename) { + await renameRuleGroup.execute(ruleGroupIdentifier, updatedGroupName, updatedInterval); + } else { + await updateRuleGroup.execute(ruleGroupIdentifier, updatedInterval); + } + onClose(true); + await dispatch(fetchRulerRulesAction({ rulesSourceName })); + } catch (_error) {} // React hook form will handle errors }; const formAPI = useForm({ @@ -254,7 +271,7 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { return ( -
e.preventDefault()} key={JSON.stringify(defaultValues)}> + <> {!props.hideFolder && ( @@ -354,7 +371,7 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { )} - + {error && {stringifyErrorLike(error)}}
- diff --git a/public/app/features/alerting/unified/components/rules/central-state-history/historyResultToDataFrame.test.ts b/public/app/features/alerting/unified/components/rules/central-state-history/historyResultToDataFrame.test.ts index da01be495d5..6ef43dfb189 100644 --- a/public/app/features/alerting/unified/components/rules/central-state-history/historyResultToDataFrame.test.ts +++ b/public/app/features/alerting/unified/components/rules/central-state-history/historyResultToDataFrame.test.ts @@ -5,7 +5,7 @@ import { time_plus_15, time_plus_30, time_plus_5, -} from '../../../mocks/alertRuleApi'; +} from '../../../mocks/grafanaRulerApi'; import { historyResultToDataFrame } from './utils'; diff --git a/public/app/features/alerting/unified/hooks/__snapshots__/useProduceNewRuleGroup.test.tsx.snap b/public/app/features/alerting/unified/hooks/__snapshots__/useProduceNewRuleGroup.test.tsx.snap index 44ab41ab030..4cd3f9c52df 100644 --- a/public/app/features/alerting/unified/hooks/__snapshots__/useProduceNewRuleGroup.test.tsx.snap +++ b/public/app/features/alerting/unified/hooks/__snapshots__/useProduceNewRuleGroup.test.tsx.snap @@ -241,3 +241,333 @@ exports[`pause rule should be able to pause a rule 1`] = ` }, ] `; + +exports[`useUpdateRuleGroupConfiguration should be able to move a Data Source managed rule group 1`] = ` +[ + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "https://mimir.local:9000/api/v1/status/buildinfo", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/mimir/api/v1/rules/namespace-1/group-1?subtype=mimir", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/mimir/api/v1/rules/namespace-2/a-new-group?subtype=mimir", + }, + { + "body": { + "interval": "2m", + "name": "a-new-group", + "rules": [ + { + "alert": "alert1", + "annotations": { + "summary": "test alert", + }, + "expr": "up = 1", + "labels": { + "severity": "warning", + }, + }, + ], + }, + "headers": [ + [ + "content-type", + "application/json", + ], + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "POST", + "url": "http://localhost/api/ruler/mimir/api/v1/rules/namespace-2?subtype=mimir", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/mimir/api/v1/rules/namespace-2/a-new-group?subtype=mimir", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "DELETE", + "url": "http://localhost/api/ruler/mimir/api/v1/rules/namespace-1/group-1?subtype=mimir", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/mimir/api/v1/rules/namespace-1/group-1?subtype=mimir", + }, +] +`; + +exports[`useUpdateRuleGroupConfiguration should rename a rule group 1`] = ` +[ + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef/grafana-group-1?subtype=cortex", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef/another-group-name?subtype=cortex", + }, + { + "body": { + "interval": "2m", + "name": "another-group-name", + "rules": [ + { + "annotations": { + "summary": "Test alert", + }, + "for": "5m", + "grafana_alert": { + "condition": "A", + "data": [ + { + "datasourceUid": "datasource-uid", + "model": { + "datasource": { + "type": "prometheus", + "uid": "datasource-uid", + }, + "expression": "vector(1)", + "queryType": "alerting", + "refId": "A", + }, + "queryType": "alerting", + "refId": "A", + "relativeTimeRange": { + "from": 1000, + "to": 2000, + }, + }, + ], + "exec_err_state": "Error", + "is_paused": false, + "namespace_uid": "uuid020c61ef", + "no_data_state": "NoData", + "rule_group": "grafana-group-1", + "title": "Grafana-rule", + "uid": "4d7125fee983", + }, + "labels": { + "region": "nasa", + "severity": "critical", + }, + }, + ], + }, + "headers": [ + [ + "content-type", + "application/json", + ], + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "POST", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef?subtype=cortex", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef/grafana-group-1?subtype=cortex", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef/another-group-name?subtype=cortex", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "DELETE", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef/grafana-group-1?subtype=cortex", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef/grafana-group-1?subtype=cortex", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef/another-group-name?subtype=cortex", + }, +] +`; + +exports[`useUpdateRuleGroupConfiguration should update a rule group interval 1`] = ` +[ + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef/grafana-group-1?subtype=cortex", + }, + { + "body": { + "interval": "2m", + "name": "grafana-group-1", + "rules": [ + { + "annotations": { + "summary": "Test alert", + }, + "for": "5m", + "grafana_alert": { + "condition": "A", + "data": [ + { + "datasourceUid": "datasource-uid", + "model": { + "datasource": { + "type": "prometheus", + "uid": "datasource-uid", + }, + "expression": "vector(1)", + "queryType": "alerting", + "refId": "A", + }, + "queryType": "alerting", + "refId": "A", + "relativeTimeRange": { + "from": 1000, + "to": 2000, + }, + }, + ], + "exec_err_state": "Error", + "is_paused": false, + "namespace_uid": "uuid020c61ef", + "no_data_state": "NoData", + "rule_group": "grafana-group-1", + "title": "Grafana-rule", + "uid": "4d7125fee983", + }, + "labels": { + "region": "nasa", + "severity": "critical", + }, + }, + ], + }, + "headers": [ + [ + "content-type", + "application/json", + ], + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "POST", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef?subtype=cortex", + }, + { + "body": "", + "headers": [ + [ + "accept", + "application/json, text/plain, */*", + ], + ], + "method": "GET", + "url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef/grafana-group-1?subtype=cortex", + }, +] +`; diff --git a/public/app/features/alerting/unified/hooks/useAsync.ts b/public/app/features/alerting/unified/hooks/useAsync.ts new file mode 100644 index 00000000000..28033229e9b --- /dev/null +++ b/public/app/features/alerting/unified/hooks/useAsync.ts @@ -0,0 +1,181 @@ +/** + * Copied from https://github.com/react-hookz/web/blob/579a445fcc9f4f4bb5b9d5e670b2e57448b4ee50/src/useAsync/index.ts + */ +import { useMemo, useRef, useState } from 'react'; + +export type AsyncStatus = 'loading' | 'success' | 'error' | 'not-executed'; + +export type AsyncState = + | { + status: 'not-executed'; + error: undefined; + result: Result; + } + | { + status: 'success'; + error: undefined; + result: Result; + } + | { + status: 'error'; + error: Error; + result: Result; + } + | { + status: AsyncStatus; + error: Error | undefined; + result: Result; + }; + +export type UseAsyncActions = { + /** + * Reset state to initial. + */ + reset: () => void; + /** + * Execute the async function manually. + */ + execute: (...args: Args) => Promise; +}; + +export type UseAsyncMeta = { + /** + * Latest promise returned from the async function. + */ + promise: Promise | undefined; + /** + * List of arguments applied to the latest async function invocation. + */ + lastArgs: Args | undefined; +}; + +export function useAsync( + asyncFn: (...params: Args) => Promise, + initialValue: Result +): [AsyncState, UseAsyncActions, UseAsyncMeta]; +export function useAsync( + asyncFn: (...params: Args) => Promise, + initialValue?: Result +): [AsyncState, UseAsyncActions, UseAsyncMeta]; + +/** + * Tracks the result and errors of the provided async function and provides handles to control its execution. + * + * @param asyncFn Function that returns a promise. + * @param initialValue Value that will be set on initialisation before the async function is + * executed. + */ +export function useAsync( + asyncFn: (...params: Args) => Promise, + initialValue?: Result +): [AsyncState, UseAsyncActions, UseAsyncMeta] { + const [state, setState] = useState>({ + status: 'not-executed', + error: undefined, + result: initialValue, + }); + const promiseRef = useRef>(); + const argsRef = useRef(); + + const methods = useSyncedRef({ + execute(...params: Args) { + argsRef.current = params; + const promise = asyncFn(...params); + promiseRef.current = promise; + + setState((s) => ({ ...s, status: 'loading' })); + + promise.then( + (result) => { + // We dont want to handle result/error of non-latest function + // this approach helps to avoid race conditions + + if (promise === promiseRef.current) { + setState((s) => ({ ...s, status: 'success', error: undefined, result })); + } + }, + (error: Error) => { + // We dont want to handle result/error of non-latest function + // this approach helps to avoid race conditions + if (promise === promiseRef.current) { + setState((s) => ({ ...s, status: 'error', error })); + } + } + ); + + return promise; + }, + reset() { + setState({ + status: 'not-executed', + error: undefined, + result: initialValue, + }); + promiseRef.current = undefined; + argsRef.current = undefined; + }, + }); + + return [ + state, + useMemo( + () => ({ + reset() { + methods.current.reset(); + }, + execute: (...params: Args) => methods.current.execute(...params), + }), + // eslint-disable-next-line react-hooks/exhaustive-deps + [] + ), + { promise: promiseRef.current, lastArgs: argsRef.current }, + ]; +} + +/** + * Like `useRef`, but it returns immutable ref that contains actual value. + * + * @param value + */ +function useSyncedRef(value: T): { readonly current: T } { + const ref = useRef(value); + + ref.current = value; + + return useMemo( + () => + Object.freeze({ + get current() { + return ref.current; + }, + }), + [] + ); +} + +// --- utility functions to help with request state assertions --- + +export function isError(state: AsyncState) { + return state.status === 'error'; +} + +export function isSuccess(state: AsyncState) { + return state.status === 'success'; +} + +export function isUninitialized(state: AsyncState) { + return state.status === 'not-executed'; +} + +export function isLoading(state: AsyncState) { + return state.status === 'loading'; +} + +export function anyOfRequestState(...states: Array>) { + return { + uninitialized: states.every(isUninitialized), + loading: states.some(isLoading), + error: states.find(isError)?.error, + success: states.some(isSuccess), + }; +} diff --git a/public/app/features/alerting/unified/hooks/useProduceNewRuleGroup.test.tsx b/public/app/features/alerting/unified/hooks/useProduceNewRuleGroup.test.tsx index 8f9433a8378..8d040b14148 100644 --- a/public/app/features/alerting/unified/hooks/useProduceNewRuleGroup.test.tsx +++ b/public/app/features/alerting/unified/hooks/useProduceNewRuleGroup.test.tsx @@ -4,11 +4,13 @@ import { byRole, byText } from 'testing-library-selector'; import { setBackendSrv } from '@grafana/runtime'; import { backendSrv } from 'app/core/services/backend_srv'; -import { CombinedRule } from 'app/types/unified-alerting'; +import { AccessControlAction } from 'app/types'; +import { CombinedRule, RuleGroupIdentifier } from 'app/types/unified-alerting'; import { RulerGrafanaRuleDTO } from 'app/types/unified-alerting-dto'; import { setupMswServer } from '../mockApi'; import { + grantUserPermissions, mockCombinedRule, mockCombinedRuleGroup, mockGrafanaRulerRule, @@ -16,19 +18,39 @@ import { mockRulerRecordingRule, mockRulerRuleGroup, } from '../mocks'; -import { grafanaRulerGroupName, grafanaRulerNamespace, grafanaRulerRule } from '../mocks/alertRuleApi'; -import { setRulerRuleGroupHandler, setUpdateRulerRuleNamespaceHandler } from '../mocks/server/configure'; +import { + grafanaRulerGroupName, + grafanaRulerGroupName2, + grafanaRulerNamespace, + grafanaRulerRule, +} from '../mocks/grafanaRulerApi'; +import { GROUP_1, NAMESPACE_1, NAMESPACE_2, namespace2 } from '../mocks/mimirRulerApi'; +import { + mimirDataSource, + setRulerRuleGroupHandler, + setUpdateRulerRuleNamespaceHandler, +} from '../mocks/server/configure'; +import { MIMIR_DATASOURCE_UID } from '../mocks/server/constants'; import { captureRequests, serializeRequests } from '../mocks/server/events'; -import { rulerRuleGroupHandler, updateRulerRuleNamespaceHandler } from '../mocks/server/handlers/alertRules'; +import { rulerRuleGroupHandler, updateRulerRuleNamespaceHandler } from '../mocks/server/handlers/grafanaRuler'; +import { GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; import { stringifyErrorLike } from '../utils/misc'; import { getRuleGroupLocationFromCombinedRule } from '../utils/rules'; -import { useDeleteRuleFromGroup, usePauseRuleInGroup } from './useProduceNewRuleGroup'; +import { AsyncState, isError, isLoading, isSuccess, isUninitialized } from './useAsync'; +import { + useDeleteRuleFromGroup, + useMoveRuleGroup, + usePauseRuleInGroup, + useRenameRuleGroup, + useUpdateRuleGroupConfiguration, +} from './useProduceNewRuleGroup'; const server = setupMswServer(); beforeAll(() => { setBackendSrv(backendSrv); + grantUserPermissions([AccessControlAction.AlertingRuleExternalRead, AccessControlAction.AlertingRuleRead]); }); describe('pause rule', () => { @@ -181,9 +203,160 @@ describe('delete rule', () => { }); }); +describe('useUpdateRuleGroupConfiguration', () => { + it('should update a rule group interval', async () => { + const capture = captureRequests(); + + render(); + await userEvent.click(byRole('button').get()); + expect(await byText(/success/i).find()).toBeInTheDocument(); + + const requests = await capture; + const serializedRequests = await serializeRequests(requests); + expect(serializedRequests).toMatchSnapshot(); + }); + + it('should rename a rule group', async () => { + const capture = captureRequests(); + + render(); + await userEvent.click(byRole('button').get()); + expect(await byText(/success/i).find()).toBeInTheDocument(); + + const requests = await capture; + const serializedRequests = await serializeRequests(requests); + expect(serializedRequests).toMatchSnapshot(); + }); + + it('should throw if we are trying to merge rule groups', async () => { + render(); + await userEvent.click(byRole('button').get()); + expect(await byText(/error:.+not supported.+/i).find()).toBeInTheDocument(); + }); + + it('should not be able to move a Grafana managed rule group', async () => { + render(); + await userEvent.click(byRole('button').get()); + expect(await byText(/error:.+not supported.+/i).find()).toBeInTheDocument(); + }); + + it('should be able to move a Data Source managed rule group', async () => { + mimirDataSource(); + const capture = captureRequests(); + + render(); + await userEvent.click(byRole('button').get()); + expect(await byText(/success/i).find()).toBeInTheDocument(); + + const requests = await capture; + const serializedRequests = await serializeRequests(requests); + expect(serializedRequests).toMatchSnapshot(); + }); + + it('should not move a Data Source managed rule group to namespace with existing target group name', async () => { + mimirDataSource(); + + render( + + ); + await userEvent.click(byRole('button').get()); + expect(await byText(/error:.+not supported.+/i).find()).toBeInTheDocument(); + }); +}); + +const UpdateRuleGroupComponent = () => { + const [requestState, updateRuleGroup] = useUpdateRuleGroupConfiguration(); + + const ruleGroupID: RuleGroupIdentifier = { + dataSourceName: GRAFANA_RULES_SOURCE_NAME, + groupName: grafanaRulerGroupName, + namespaceName: grafanaRulerNamespace.uid, + }; + + return ( + <> +