diff --git a/.betterer.results b/.betterer.results index 3d15c748b2e..c08d07d5459 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1555,19 +1555,6 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"], [0, 0, 0, "No untranslated strings. Wrap text with ", "4"] ], - "public/app/features/alerting/unified/CloneRuleEditor.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"] - ], - "public/app/features/alerting/unified/ExistingRuleEditor.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "4"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "5"] - ], "public/app/features/alerting/unified/GrafanaRuleQueryViewer.tsx:5381": [ [0, 0, 0, "\'@grafana/data/src/datetime/rangeutil\' import is restricted from being used by a pattern. Import from the public export instead.", "0"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], @@ -1606,12 +1593,6 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "6"], [0, 0, 0, "No untranslated strings. Wrap text with ", "7"] ], - "public/app/features/alerting/unified/RuleEditor.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "3"] - ], "public/app/features/alerting/unified/RuleViewer.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"] ], @@ -2563,7 +2544,8 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "7"], [0, 0, 0, "No untranslated strings. Wrap text with ", "8"], [0, 0, 0, "No untranslated strings. Wrap text with ", "9"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "10"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "10"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "11"] ], "public/app/features/alerting/unified/components/rules/Filter/RulesFilter.v1.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], @@ -2925,6 +2907,25 @@ exports[`better eslint`] = { "public/app/features/alerting/unified/plugins/PluginOriginBadge.tsx:5381": [ [0, 0, 0, "\'@grafana/ui/src/components/Icon/utils\' import is restricted from being used by a pattern. Import from the public export instead.", "0"] ], + "public/app/features/alerting/unified/rule-editor/CloneRuleEditor.tsx:5381": [ + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"] + ], + "public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx:5381": [ + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "4"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "5"] + ], + "public/app/features/alerting/unified/rule-editor/RuleEditor.tsx:5381": [ + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "3"] + ], "public/app/features/alerting/unified/rule-list/FilterView.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"] ], diff --git a/public/app/features/alerting/routes.tsx b/public/app/features/alerting/routes.tsx index 839d5db06c4..490d264c5fa 100644 --- a/public/app/features/alerting/routes.tsx +++ b/public/app/features/alerting/routes.tsx @@ -214,7 +214,7 @@ export function getAlertingRoutes(cfg = config): RouteDescriptor[] { pageClass: 'page-alerting', roles: evaluateAccess([AccessControlAction.AlertingRuleCreate, AccessControlAction.AlertingRuleExternalWrite]), component: importAlertingComponent( - () => import(/* webpackChunkName: "AlertingRuleForm"*/ 'app/features/alerting/unified/RuleEditor') + () => import(/* webpackChunkName: "AlertingRuleForm"*/ 'app/features/alerting/unified/rule-editor/RuleEditor') ), }, { @@ -222,7 +222,7 @@ export function getAlertingRoutes(cfg = config): RouteDescriptor[] { pageClass: 'page-alerting', roles: evaluateAccess([AccessControlAction.AlertingRuleUpdate, AccessControlAction.AlertingRuleExternalWrite]), component: importAlertingComponent( - () => import(/* webpackChunkName: "AlertingRuleForm"*/ 'app/features/alerting/unified/RuleEditor') + () => import(/* webpackChunkName: "AlertingRuleForm"*/ 'app/features/alerting/unified/rule-editor/RuleEditor') ), }, { diff --git a/public/app/features/alerting/unified/api/alertRuleApi.ts b/public/app/features/alerting/unified/api/alertRuleApi.ts index f483df956e7..806b782a2bf 100644 --- a/public/app/features/alerting/unified/api/alertRuleApi.ts +++ b/public/app/features/alerting/unified/api/alertRuleApi.ts @@ -248,6 +248,7 @@ export const alertRuleApi = alertingApi.injectEndpoints({ const { path, params } = rulerUrlBuilder(rulerConfig).namespace(namespace); return { url: path, params }; }, + providesTags: (_result, _error, { namespace }) => [{ type: 'RuleNamespace', id: namespace }], }), // TODO This should be probably a separate ruler API file diff --git a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.tsx b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.tsx index 108532e5e41..5aca1d77824 100644 --- a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.tsx +++ b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.tsx @@ -7,8 +7,8 @@ import { Alert, LoadingPlaceholder } from '@grafana/ui'; import { RuleIdentifier } from '../../../../../types/unified-alerting'; import { useRuleWithLocation } from '../../hooks/useCombinedRule'; +import { formValuesFromExistingRule } from '../../rule-editor/formDefaults'; import { stringifyErrorLike } from '../../utils/misc'; -import { formValuesFromExistingRule } from '../../utils/rule-form'; import * as ruleId from '../../utils/rule-id'; import { isGrafanaRulerRule } from '../../utils/rules'; import { createRelativeUrl } from '../../utils/url'; diff --git a/public/app/features/alerting/unified/components/expressions/ExpressionStatusIndicator.test.tsx b/public/app/features/alerting/unified/components/expressions/ExpressionStatusIndicator.test.tsx index 3b76b141690..c82097c0df0 100644 --- a/public/app/features/alerting/unified/components/expressions/ExpressionStatusIndicator.test.tsx +++ b/public/app/features/alerting/unified/components/expressions/ExpressionStatusIndicator.test.tsx @@ -1,8 +1,8 @@ import { render, screen } from '@testing-library/react'; import { FormProvider, useForm } from 'react-hook-form'; +import { getDefaultFormValues } from '../../rule-editor/formDefaults'; import { RuleFormValues } from '../../types/rule-form'; -import { getDefaultFormValues } from '../../utils/rule-form'; import { ExpressionStatusIndicator } from './ExpressionStatusIndicator'; diff --git a/public/app/features/alerting/unified/components/rule-editor/AnnotationsStep.test.tsx b/public/app/features/alerting/unified/components/rule-editor/AnnotationsStep.test.tsx index df1ee304e11..9f1067761ca 100644 --- a/public/app/features/alerting/unified/components/rule-editor/AnnotationsStep.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/AnnotationsStep.test.tsx @@ -6,9 +6,9 @@ import { byRole, byTestId } from 'testing-library-selector'; import { DashboardSearchItemType } from '../../../../search/types'; import { mockDashboardApi, setupMswServer } from '../../mockApi'; import { mockDashboardDto, mockDashboardSearchItem } from '../../mocks'; +import { getDefaultFormValues } from '../../rule-editor/formDefaults'; import { RuleFormValues } from '../../types/rule-form'; import { Annotation } from '../../utils/constants'; -import { getDefaultFormValues } from '../../utils/rule-form'; import AnnotationsStep from './AnnotationsStep'; diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx index 3a918eb6369..6104c3b2fa6 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx @@ -1,12 +1,11 @@ import { css } from '@emotion/css'; -import { debounce, take, uniqueId } from 'lodash'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { uniqueId } from 'lodash'; +import { useEffect, useMemo, useState } from 'react'; import { Controller, FormProvider, RegisterOptions, useForm, useFormContext } from 'react-hook-form'; import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { - AsyncSelect, Box, Button, Field, @@ -15,6 +14,7 @@ import { Input, Label, Modal, + Select, Stack, Switch, Text, @@ -22,17 +22,13 @@ import { useStyles2, } from '@grafana/ui'; import { Trans, t } from 'app/core/internationalization'; -import { CombinedRuleGroup, CombinedRuleNamespace } from 'app/types/unified-alerting'; import { RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; -import { LogMessages, logInfo } from '../../Analytics'; import { alertRuleApi } from '../../api/alertRuleApi'; import { GRAFANA_RULER_CONFIG } from '../../api/featureDiscoveryApi'; -import { useCombinedRuleNamespaces } from '../../hooks/useCombinedRuleNamespaces'; -import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelector'; +import { DEFAULT_GROUP_EVALUATION_INTERVAL } from '../../rule-editor/formDefaults'; import { RuleFormValues } from '../../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; -import { DEFAULT_GROUP_EVALUATION_INTERVAL } from '../../utils/rule-form'; import { isGrafanaAlertingRuleByType, isGrafanaManagedRuleByType, @@ -53,48 +49,38 @@ import { RuleEditorSection } from './RuleEditorSection'; export const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds export const MAX_GROUP_RESULTS = 1000; -export const useFolderGroupOptions = (folderUid: string, enableProvisionedGroups: boolean) => { +const useFetchGroupsForFolder = (folderUid: string) => { // fetch the ruler rules from the database so we can figure out what other "groups" are already defined // for our folders - const { isLoading: isLoadingRulerNamespace, currentData: rulerNamespace } = - alertRuleApi.endpoints.rulerNamespace.useQuery( - { - namespace: folderUid, - rulerConfig: GRAFANA_RULER_CONFIG, - }, - { - skip: !folderUid, - refetchOnMountOrArgChange: true, - } - ); - - // There should be only one entry in the rulerNamespace object - // However it uses folder name as key, so to avoid fetching folder name, we use Object.values - const groupOptions = useMemo(() => { - if (!rulerNamespace) { - // still waiting for namespace information to be fetched - return []; + return alertRuleApi.endpoints.rulerNamespace.useQuery( + { + namespace: folderUid, + rulerConfig: GRAFANA_RULER_CONFIG, + }, + { + refetchOnMountOrArgChange: true, + skip: !folderUid, } + ); +}; - const folderGroups = Object.values(rulerNamespace).flat() ?? []; +const namespaceToGroupOptions = (rulerNamespace: RulerRulesConfigDTO, enableProvisionedGroups: boolean) => { + const folderGroups = Object.values(rulerNamespace).flat(); - return folderGroups - .map>((group) => { - const isProvisioned = isProvisionedGroup(group); - return { - label: group.name, - value: group.name, - description: group.interval ?? DEFAULT_GROUP_EVALUATION_INTERVAL, - // we include provisioned folders, but disable the option to select them - isDisabled: !enableProvisionedGroups ? isProvisioned : false, - isProvisioned: isProvisioned, - }; - }) + return folderGroups + .map>((group) => { + const isProvisioned = isProvisionedGroup(group); + return { + label: group.name, + value: group.name, + description: group.interval ?? DEFAULT_GROUP_EVALUATION_INTERVAL, + // we include provisioned folders, but disable the option to select them + isDisabled: !enableProvisionedGroups ? isProvisioned : false, + isProvisioned: isProvisioned, + }; + }) - .sort(sortByLabel); - }, [rulerNamespace, enableProvisionedGroups]); - - return { groupOptions, loading: isLoadingRulerNamespace }; + .sort(sortByLabel); }; const isProvisionedGroup = (group: RulerRuleGroupDTO) => { @@ -105,10 +91,6 @@ const sortByLabel = (a: SelectableValue, b: SelectableValue) => return a.label?.localeCompare(b.label ?? '') || 0; }; -const findGroupMatchingLabel = (group: SelectableValue, query: string) => { - return group.label?.toLowerCase().includes(query.toLowerCase()); -}; - const forValidationOptions = (evaluateEvery: string): RegisterOptions<{ evaluateFor: string }> => ({ required: { value: true, @@ -149,24 +131,10 @@ const forValidationOptions = (evaluateEvery: string): RegisterOptions<{ evaluate }, }); -const useIsNewGroup = (folder: string, group: string) => { - const { groupOptions } = useFolderGroupOptions(folder, false); - - const groupIsInGroupOptions = useCallback( - (group_: string) => groupOptions.some((groupInList: SelectableValue) => groupInList.label === group_), - [groupOptions] - ); - return !groupIsInGroupOptions(group); -}; - export function GrafanaEvaluationBehaviorStep({ - evaluateEvery, - setEvaluateEvery, existing, enableProvisionedGroups, }: { - evaluateEvery: string; - setEvaluateEvery: (value: string) => void; existing: boolean; enableProvisionedGroups: boolean; }) { @@ -181,51 +149,39 @@ export function GrafanaEvaluationBehaviorStep({ control, } = useFormContext(); - const [folder, group, type, isPaused, folderUid, folderName] = watch([ - 'folder', + const [group, type, isPaused, folder, evaluateEvery] = watch([ 'group', 'type', 'isPaused', - 'folder.uid', - 'folder.title', + 'folder', + 'evaluateEvery', ]); const isGrafanaAlertingRule = isGrafanaAlertingRuleByType(type); const isGrafanaRecordingRule = isGrafanaRecordingRuleByType(type); - const { groupOptions, loading } = useFolderGroupOptions(folder?.uid ?? '', enableProvisionedGroups); + const { currentData: rulerNamespace, isLoading: loadingGroups } = useFetchGroupsForFolder(folder?.uid ?? ''); const [isEditingGroup, setIsEditingGroup] = useState(false); - const rulerRuleRequests = useUnifiedAlertingSelector((state) => state.rulerRules); - const groupfoldersForGrafana = rulerRuleRequests[GRAFANA_RULES_SOURCE_NAME]; + const groupOptions = useMemo(() => { + return rulerNamespace ? namespaceToGroupOptions(rulerNamespace, enableProvisionedGroups) : []; + }, [enableProvisionedGroups, rulerNamespace]); - const grafanaNamespaces = useCombinedRuleNamespaces(GRAFANA_RULES_SOURCE_NAME); - const existingNamespace = grafanaNamespaces.find((ns) => ns.uid === folderUid); - const existingGroup = existingNamespace?.groups.find((g) => g.name === group); - - const isNewGroup = useIsNewGroup(folderUid ?? '', group); + const existingGroup = Object.values(rulerNamespace ?? {}) + .flat() + .find((ruleGroup) => ruleGroup.name === group); + const isNewGroup = !existingGroup && !loadingGroups; + // synchronize the evaluation interval with the group name when it's an existing group useEffect(() => { - if (!isNewGroup && existingGroup?.interval) { - setEvaluateEvery(existingGroup.interval); + if (existingGroup) { + setValue('evaluateEvery', existingGroup.interval ?? DEFAULT_GROUP_EVALUATION_INTERVAL); } - }, [setEvaluateEvery, isNewGroup, setValue, existingGroup]); - - const closeEditGroupModal = (saved = false) => { - if (!saved) { - logInfo(LogMessages.leavingRuleGroupEdit); - } - setIsEditingGroup(false); - }; + }, [existingGroup, setValue]); + const closeEditGroupModal = () => setIsEditingGroup(false); const onOpenEditGroupModal = () => setIsEditingGroup(true); - const editGroupDisabled = groupfoldersForGrafana?.loading || isNewGroup || !folderUid || !group; - const emptyNamespace: CombinedRuleNamespace = { - name: folderName, - rulesSource: GRAFANA_RULES_SOURCE_NAME, - groups: [], - }; - const emptyGroup: CombinedRuleGroup = { name: group, interval: evaluateEvery, rules: [], totals: {} }; + const editGroupDisabled = loadingGroups || isNewGroup || !folder?.uid || !group; const [isCreatingEvaluationGroup, setIsCreatingEvaluationGroup] = useState(false); @@ -235,18 +191,6 @@ export function GrafanaEvaluationBehaviorStep({ setIsCreatingEvaluationGroup(false); }; - const getOptions = useCallback( - async (query: string) => { - const results = query ? groupOptions.filter((group) => findGroupMatchingLabel(group, query)) : groupOptions; - return take(results, MAX_GROUP_RESULTS); - }, - [groupOptions] - ); - - const debouncedSearch = useMemo(() => { - return debounce(getOptions, 300, { leading: true }); - }, [getOptions]); - const defaultGroupValue = group ? { value: group, label: group } : undefined; const pauseContentText = isGrafanaRecordingRule @@ -257,7 +201,7 @@ export function GrafanaEvaluationBehaviorStep({ const step = isGrafanaManagedRuleByType(type) ? 4 : 3; const label = - isGrafanaManagedRuleByType(type) && !folder + isGrafanaManagedRuleByType(type) && !folder?.uid ? t( 'alerting.rule-form.evaluation.select-folder-before', 'Select a folder before setting evaluation group and interval' @@ -284,21 +228,20 @@ export function GrafanaEvaluationBehaviorStep({ > ( - { field.onChange(group.label ?? ''); }} - isLoading={loading} - invalid={Boolean(folder) && !group && Boolean(fieldState.error)} - loadOptions={debouncedSearch} + isLoading={loadingGroups} + invalid={Boolean(folder?.uid) && !group && Boolean(fieldState.error)} cacheOptions loadingMessage={'Loading groups...'} defaultValue={defaultGroupValue} - defaultOptions={groupOptions} + options={groupOptions} getOptionLabel={(option: SelectableValue) => (
{option.label} @@ -329,7 +272,7 @@ export function GrafanaEvaluationBehaviorStep({ icon="plus" fill="outline" variant="secondary" - disabled={!folder} + disabled={!folder?.uid} data-testid={selectors.components.AlertRules.newEvaluationGroupButton} > New evaluation group @@ -339,22 +282,25 @@ export function GrafanaEvaluationBehaviorStep({ setIsCreatingEvaluationGroup(false)} - groupfoldersForGrafana={groupfoldersForGrafana?.result} + groupfoldersForGrafana={rulerNamespace} /> )} - {folderName && isEditingGroup && ( + {folder?.uid && isEditingGroup && ( closeEditGroupModal()} intervalEditOnly hideFolder={true} /> )} - {folderName && group && ( + {folder?.title && group && (
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 54d22db90ac..3d46e0873e7 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 @@ -1,5 +1,5 @@ import { css } from '@emotion/css'; -import { useEffect, useMemo, useState } from 'react'; +import { useMemo, useState } from 'react'; import { FormProvider, SubmitErrorHandler, UseFormWatch, useForm } from 'react-hook-form'; import { useParams } from 'react-router-dom-v5-compat'; @@ -22,7 +22,6 @@ import { isGrafanaRulerRulePaused, isRecordingRuleByType, } from 'app/features/alerting/unified/utils/rules'; -import { isExpressionQuery } from 'app/features/expressions/guards'; import { RuleGroupIdentifier, RuleIdentifier, RuleWithLocation } from 'app/types/unified-alerting'; import { PostableRuleGrafanaRuleDTO, RulerRuleDTO } from 'app/types/unified-alerting-dto'; @@ -40,24 +39,21 @@ import { shouldUsePrometheusRulesPrimary } from '../../../featureToggles'; import { useDeleteRuleFromGroup } from '../../../hooks/ruleGroup/useDeleteRuleFromGroup'; import { useAddRuleToRuleGroup, useUpdateRuleInRuleGroup } from '../../../hooks/ruleGroup/useUpsertRuleFromRuleGroup'; import { useReturnTo } from '../../../hooks/useReturnTo'; -import { useURLSearchParams } from '../../../hooks/useURLSearchParams'; -import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; -import { DataSourceType } from '../../../utils/datasource'; import { - DEFAULT_GROUP_EVALUATION_INTERVAL, + defaultFormValuesForRuleType, + formValuesFromExistingRule, + formValuesFromPrefill, + translateRouteParamToRuleType, +} from '../../../rule-editor/formDefaults'; +import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; +import { MANUAL_ROUTING_KEY, SIMPLIFIED_QUERY_EDITOR_KEY, - formValuesFromExistingRule, formValuesToRulerGrafanaRuleDTO, formValuesToRulerRuleDTO, - getDefaultFormValues, - getDefaultQueries, - ignoreHiddenQueries, - normalizeDefaultAnnotations, } from '../../../utils/rule-form'; import * as ruleId from '../../../utils/rule-id'; import { fromRulerRule, fromRulerRuleAndRuleGroupIdentifier, stringifyIdentifier } from '../../../utils/rule-id'; -import { isGrafanaRecordingRuleByType } from '../../../utils/rules'; import { createRelativeUrl } from '../../../utils/url'; import { GrafanaRuleExporter } from '../../export/GrafanaRuleExporter'; import { AlertRuleNameAndMetric } from '../AlertRuleNameInput'; @@ -68,12 +64,7 @@ import { GrafanaFolderAndLabelsStep } from '../GrafanaFolderAndLabelsStep'; import { NotificationsStep } from '../NotificationsStep'; import { RecordingRulesNameSpaceAndGroupStep } from '../RecordingRulesNameSpaceAndGroupStep'; import { RuleInspector } from '../RuleInspector'; -import { - QueryAndExpressionsStep, - areQueriesTransformableToSimpleCondition, - isExpressionQueryInAlert, -} from '../query-and-alert-condition/QueryAndExpressionsStep'; -import { translateRouteParamToRuleType } from '../util'; +import { QueryAndExpressionsStep } from '../query-and-alert-condition/QueryAndExpressionsStep'; type Props = { existing?: RuleWithLocation; @@ -85,9 +76,7 @@ const prometheusRulesPrimary = shouldUsePrometheusRulesPrimary(); export const AlertRuleForm = ({ existing, prefill }: Props) => { const styles = useStyles2(getStyles); const notifyApp = useAppNotification(); - const [queryParams] = useURLSearchParams(); const [showEditYaml, setShowEditYaml] = useState(false); - const [evaluateEvery, setEvaluateEvery] = useState(existing?.group.interval ?? DEFAULT_GROUP_EVALUATION_INTERVAL); const [deleteRuleFromGroup] = useDeleteRuleFromGroup(); const [addRuleToRuleGroup] = useAddRuleToRuleGroup(); @@ -110,19 +99,10 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { return formValuesFromPrefill(prefill); } - if (queryParams.has('defaults')) { - return formValuesFromQueryParams(queryParams.get('defaults') ?? '', ruleType); - } const defaultRuleType = ruleType || RuleFormType.grafana; - return { - ...getDefaultFormValues(), - condition: 'C', - queries: getDefaultQueries(isGrafanaRecordingRuleByType(defaultRuleType)), - type: defaultRuleType, - evaluateEvery: evaluateEvery, - }; - }, [existing, prefill, queryParams, evaluateEvery, ruleType]); + return defaultFormValuesForRuleType(defaultRuleType); + }, [existing, prefill, ruleType]); const formAPI = useForm({ mode: 'onSubmit', @@ -151,6 +131,8 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { // @todo why is error not propagated to form? const submit = async (values: RuleFormValues, exitOnSave: boolean) => { + const { type, evaluateEvery } = values; + if (conditionErrorMsg !== '') { notifyApp.error(conditionErrorMsg); if (!existing && grafanaTypeRule) { @@ -160,7 +142,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { return; } - trackAlertRuleFormSaved({ formAction: existing ? 'update' : 'create', ruleType: values.type }); + trackAlertRuleFormSaved({ formAction: existing ? 'update' : 'create', ruleType: type }); const ruleDefinition = grafanaTypeRule ? formValuesToRulerGrafanaRuleDTO(values) : formValuesToRulerRuleDTO(values); @@ -206,12 +188,11 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { const deleteRule = async () => { if (existing) { - const returnTo = queryParams.get('returnTo') || '/alerting/list'; const ruleGroupIdentifier = getRuleGroupLocationFromRuleWithLocation(existing); const ruleIdentifier = fromRulerRuleAndRuleGroupIdentifier(ruleGroupIdentifier, existing.rule); await deleteRuleFromGroup.execute(ruleGroupIdentifier, ruleIdentifier); - locationService.replace(returnTo); + locationService.replace(returnTo ?? '/alerting/list'); } }; @@ -236,9 +217,6 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { locationService.getHistory().goBack(); }; - const evaluateEveryInForm = watch('evaluateEvery'); - useEffect(() => setEvaluateEvery(evaluateEveryInForm), [evaluateEveryInForm]); - const actionButtons = ( {existing && ( @@ -314,12 +292,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { {/* Step 4 & 5 & 6*/} {isGrafanaManagedRuleByType(type) && ( - + )} {/* Notifications step*/} @@ -379,101 +352,17 @@ const isCortexLokiOrRecordingRule = (watch: UseFormWatch) => { return (ruleType === RuleFormType.cloudAlerting || ruleType === RuleFormType.cloudRecording) && dataSourceName !== ''; }; -function formValuesFromQueryParams(ruleDefinition: string, type: RuleFormType): RuleFormValues { - let ruleFromQueryParams: Partial; - - try { - ruleFromQueryParams = JSON.parse(ruleDefinition); - } catch (err) { - return { - ...getDefaultFormValues(), - queries: getDefaultQueries(), - }; - } - - return setQueryEditorSettings( - setInstantOrRange( - ignoreHiddenQueries({ - ...getDefaultFormValues(), - ...ruleFromQueryParams, - annotations: normalizeDefaultAnnotations(ruleFromQueryParams.annotations ?? []), - queries: ruleFromQueryParams.queries ?? getDefaultQueries(), - type: type || RuleFormType.grafana, - evaluateEvery: DEFAULT_GROUP_EVALUATION_INTERVAL, - }) - ) - ); -} - -function formValuesFromPrefill(rule: Partial): RuleFormValues { - return ignoreHiddenQueries({ - ...getDefaultFormValues(), - ...rule, - }); -} - -function setQueryEditorSettings(values: RuleFormValues): RuleFormValues { - const isQuerySwitchModeEnabled = config.featureToggles.alertingQueryAndExpressionsStepMode ?? false; - - if (!isQuerySwitchModeEnabled) { - return { - ...values, - editorSettings: { - simplifiedQueryEditor: false, - simplifiedNotificationEditor: true, // actually it doesn't matter in this case - }, - }; - } - - // data queries only - const dataQueries = values.queries.filter((query) => !isExpressionQuery(query.model)); - - // expression queries only - const expressionQueries = values.queries.filter((query) => isExpressionQueryInAlert(query)); - - const queryParamsAreTransformable = areQueriesTransformableToSimpleCondition(dataQueries, expressionQueries); - return { - ...values, - editorSettings: { - simplifiedQueryEditor: queryParamsAreTransformable, - simplifiedNotificationEditor: true, - }, - }; -} - -function setInstantOrRange(values: RuleFormValues): RuleFormValues { - return { - ...values, - queries: values.queries?.map((query) => { - if (isExpressionQuery(query.model)) { - return query; - } - // data query - const defaultToInstant = - query.model.datasource?.type === DataSourceType.Loki || - query.model.datasource?.type === DataSourceType.Prometheus; - const isInstant = - 'instant' in query.model && query.model.instant !== undefined ? query.model.instant : defaultToInstant; - return { - ...query, - model: { - ...query.model, - instant: isInstant, - range: !isInstant, // we cannot have both instant and range queries in alerting - }, - }; - }), - }; -} - function storeInLocalStorageValues(values: RuleFormValues) { - if (values.manualRouting) { + const { manualRouting, editorSettings } = values; + + if (manualRouting) { localStorage.setItem(MANUAL_ROUTING_KEY, 'true'); } else { localStorage.setItem(MANUAL_ROUTING_KEY, 'false'); } - if (values.editorSettings) { - if (values.editorSettings.simplifiedQueryEditor) { + + if (editorSettings) { + if (editorSettings.simplifiedQueryEditor) { localStorage.setItem(SIMPLIFIED_QUERY_EDITOR_KEY, 'true'); } else { localStorage.setItem(SIMPLIFIED_QUERY_EDITOR_KEY, 'false'); diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx index 38598f08784..72093ecc490 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx @@ -16,14 +16,10 @@ import { alertRuleApi } from '../../../api/alertRuleApi'; import { fetchRulerRulesGroup } from '../../../api/ruler'; import { useDataSourceFeatures } from '../../../hooks/useCombinedRule'; import { useReturnTo } from '../../../hooks/useReturnTo'; +import { DEFAULT_GROUP_EVALUATION_INTERVAL, getDefaultFormValues } from '../../../rule-editor/formDefaults'; import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME } from '../../../utils/datasource'; -import { - DEFAULT_GROUP_EVALUATION_INTERVAL, - formValuesToRulerGrafanaRuleDTO, - getDefaultFormValues, - getDefaultQueries, -} from '../../../utils/rule-form'; +import { formValuesToRulerGrafanaRuleDTO, getDefaultQueries } from '../../../utils/rule-form'; import { isGrafanaRulerRule } from '../../../utils/rules'; import { FileExportPreview } from '../../export/FileExportPreview'; import { GrafanaExportDrawer } from '../../export/GrafanaExportDrawer'; @@ -64,9 +60,7 @@ export function ModifyExportRuleForm({ ruleForm, alertUid }: ModifyExportRuleFor const { returnTo } = useReturnTo('/alerting/list'); const [exportData, setExportData] = useState(undefined); - const [conditionErrorMsg, setConditionErrorMsg] = useState(''); - const [evaluateEvery, setEvaluateEvery] = useState(ruleForm?.evaluateEvery ?? DEFAULT_GROUP_EVALUATION_INTERVAL); const onInvalid = (): void => { notifyApp.error('There are errors in the form. Please correct them and try again!'); @@ -112,12 +106,7 @@ export function ModifyExportRuleForm({ ruleForm, alertUid }: ModifyExportRuleFor {/* Step 4 & 5 */} - + {/* Notifications step*/} {/* Annotations only for cloud and Grafana */} diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/getPayloadToExport.test.ts b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/getPayloadToExport.test.ts index 6fc5ba37a35..2a1e67855e3 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/getPayloadToExport.test.ts +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/getPayloadToExport.test.ts @@ -1,9 +1,9 @@ import { RulerRuleDTO, RulerRuleGroupDTO } from 'app/types/unified-alerting-dto'; import { mockRulerGrafanaRecordingRule, mockRulerGrafanaRule } from '../../../mocks'; +import { getDefaultFormValues } from '../../../rule-editor/formDefaults'; import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; import { Annotation } from '../../../utils/constants'; -import { getDefaultFormValues } from '../../../utils/rule-form'; import { getPayloadToExport } from './ModifyExportRuleForm'; 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 11b3a0f5dfc..116374475da 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 @@ -106,8 +106,10 @@ describe('Can create a new grafana managed alert using simplified routing', () = it('simplified routing is not available when Grafana AM is not enabled', async () => { setAlertmanagerChoices(AlertmanagerChoice.External, 1); - renderRuleEditor(); + const { user } = renderRuleEditor(); + // Just to make sure all dropdowns have been loaded + await selectFolderAndGroup(user); await waitFor(() => expect(ui.inputs.simplifiedRouting.contactPointRouting.query()).not.toBeInTheDocument()); }); @@ -147,6 +149,7 @@ describe('Can create a new grafana managed alert using simplified routing', () = expect(await screen.findByText('Email')).toBeInTheDocument(); }); }); + describe('switch modes enabled', () => { testWithFeatureToggles(['alertingQueryAndExpressionsStepMode', 'alertingNotificationsStepMode']); @@ -168,6 +171,7 @@ describe('Can create a new grafana managed alert using simplified routing', () = const serializedRequests = await serializeRequests(requests); expect(serializedRequests).toMatchSnapshot(); }); + it('can create the new grafana-managed rule with advanced modes', async () => { const capture = captureRequests((r) => r.method === 'POST' && r.url.includes('/api/ruler/')); @@ -185,6 +189,7 @@ describe('Can create a new grafana managed alert using simplified routing', () = const serializedRequests = await serializeRequests(requests); expect(serializedRequests).toMatchSnapshot(); }); + it('can create the new grafana-managed rule with only notifications step advanced mode', async () => { const capture = captureRequests((r) => r.method === 'POST' && r.url.includes('/api/ruler/')); @@ -202,6 +207,7 @@ describe('Can create a new grafana managed alert using simplified routing', () = const serializedRequests = await serializeRequests(requests); expect(serializedRequests).toMatchSnapshot(); }); + it('can create the new grafana-managed rule with only query step advanced mode', async () => { const contactPointName = 'lotsa-emails'; const capture = captureRequests((r) => r.method === 'POST' && r.url.includes('/api/ruler/')); @@ -221,6 +227,7 @@ describe('Can create a new grafana managed alert using simplified routing', () = const serializedRequests = await serializeRequests(requests); expect(serializedRequests).toMatchSnapshot(); }); + it('switch modes are intiallized depending on the local storage - 1', async () => { localStorage.setItem(SIMPLIFIED_QUERY_EDITOR_KEY, 'false'); localStorage.setItem(MANUAL_ROUTING_KEY, 'true'); @@ -231,6 +238,7 @@ describe('Can create a new grafana managed alert using simplified routing', () = expect(ui.inputs.switchModeAdvanced(GrafanaRuleFormStep.Query).get()).toBeInTheDocument(); expect(ui.inputs.switchModeBasic(GrafanaRuleFormStep.Notification).get()).toBeInTheDocument(); }); + it('switch modes are intiallized depending on the local storage - 2', async () => { localStorage.setItem(SIMPLIFIED_QUERY_EDITOR_KEY, 'true'); localStorage.setItem(MANUAL_ROUTING_KEY, 'false'); diff --git a/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.test.tsx b/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.test.tsx index 752d9e71101..a0983cba07f 100644 --- a/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.test.tsx @@ -136,6 +136,7 @@ describe('LabelsField with suggestions', () => { expect(screen.getByTestId('labelsInSubform-key-2')).toHaveTextContent('key3'); expect(screen.getByTestId('labelsInSubform-value-2')).toHaveTextContent('value3'); }); + it('Should be able to write new keys and values using the dropdowns, case sensitive', async () => { const { user } = await renderLabelsWithSuggestions(); diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx index f43e3a14b67..2ecd8131b3b 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx @@ -23,16 +23,14 @@ import { import { Text } from '@grafana/ui/src/components/Text/Text'; import { Trans, t } from 'app/core/internationalization'; import { isExpressionQuery } from 'app/features/expressions/guards'; -import { - ExpressionDatasourceUID, - ExpressionQuery, - ExpressionQueryType, - ReducerMode, - expressionTypes, -} from 'app/features/expressions/types'; -import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto'; +import { ExpressionDatasourceUID, ExpressionQueryType, expressionTypes } from 'app/features/expressions/types'; +import { AlertQuery } from 'app/types/unified-alerting-dto'; import { useRulesSourcesWithRuler } from '../../../hooks/useRuleSourcesWithRuler'; +import { + areQueriesTransformableToSimpleCondition, + isExpressionQueryInAlert, +} from '../../../rule-editor/formProcessing'; import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; import { getDefaultOrFirstCompatibleDataSource } from '../../../utils/datasource'; import { PromOrLokiQuery, isPromOrLokiQuery } from '../../../utils/rule-form'; @@ -76,49 +74,6 @@ import { import { useAdvancedMode } from './useAdvancedMode'; import { useAlertQueryRunner } from './useAlertQueryRunner'; -export function areQueriesTransformableToSimpleCondition( - dataQueries: Array>, - expressionQueries: Array> -) { - if (dataQueries.length !== 1) { - return false; - } - const singleReduceExpressionInInstantQuery = - 'instant' in dataQueries[0].model && dataQueries[0].model.instant && expressionQueries.length === 1; - - if (expressionQueries.length !== 2 && !singleReduceExpressionInInstantQuery) { - return false; - } - - const query = dataQueries[0]; - - if (query.refId !== SimpleConditionIdentifier.queryId) { - return false; - } - - const reduceExpressionIndex = expressionQueries.findIndex( - (query) => query.model.type === ExpressionQueryType.reduce && query.refId === SimpleConditionIdentifier.reducerId - ); - const reduceExpression = expressionQueries.at(reduceExpressionIndex); - const reduceOk = - reduceExpression && - reduceExpressionIndex === 0 && - (reduceExpression.model.settings?.mode === ReducerMode.Strict || - reduceExpression.model.settings?.mode === undefined); - - const thresholdExpressionIndex = expressionQueries.findIndex( - (query) => - query.model.type === ExpressionQueryType.threshold && query.refId === SimpleConditionIdentifier.thresholdId - ); - const thresholdExpression = expressionQueries.at(thresholdExpressionIndex); - const conditions = thresholdExpression?.model.conditions ?? []; - const thresholdIndexOk = singleReduceExpressionInInstantQuery - ? thresholdExpressionIndex === 0 - : thresholdExpressionIndex === 1; - const thresholdOk = thresholdExpression && thresholdIndexOk && conditions[0]?.unloadEvaluator === undefined; - return (Boolean(reduceOk) || Boolean(singleReduceExpressionInInstantQuery)) && Boolean(thresholdOk); -} - interface Props { editingExistingRule: boolean; onDataChange: (error: string) => void; @@ -777,9 +732,3 @@ const useSetExpressionAndDataSource = () => { } }; }; - -export function isExpressionQueryInAlert( - query: AlertQuery -): query is AlertQuery { - return isExpressionQuery(query.model); -} diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/__snapshots__/areQueriesTransformableToSimpleCondition.test.ts b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/__snapshots__/areQueriesTransformableToSimpleCondition.test.ts index 208342735e6..2a9294ada7d 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/__snapshots__/areQueriesTransformableToSimpleCondition.test.ts +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/__snapshots__/areQueriesTransformableToSimpleCondition.test.ts @@ -2,11 +2,10 @@ import { produce } from 'immer'; import { EvalFunction } from 'app/features/alerting/state/alertDef'; import { dataQuery, reduceExpression, thresholdExpression } from 'app/features/alerting/unified/mocks'; +import { areQueriesTransformableToSimpleCondition } from 'app/features/alerting/unified/rule-editor/formProcessing'; import { ExpressionQuery, ReducerMode } from 'app/features/expressions/types'; import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto'; -import { areQueriesTransformableToSimpleCondition } from '../QueryAndExpressionsStep'; - const expressionQueries: Array> = [reduceExpression, thresholdExpression]; describe('areQueriesTransformableToSimpleCondition', () => { diff --git a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/useAdvancedMode.ts b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/useAdvancedMode.ts index 0c793dfdaac..08e9d889bbf 100644 --- a/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/useAdvancedMode.ts +++ b/public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/useAdvancedMode.ts @@ -5,7 +5,8 @@ import { EvalFunction } from 'app/features/alerting/state/alertDef'; import { ExpressionQuery } from 'app/features/expressions/types'; import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto'; -import { areQueriesTransformableToSimpleCondition } from './QueryAndExpressionsStep'; +import { areQueriesTransformableToSimpleCondition } from '../../../rule-editor/formProcessing'; + import { SimpleCondition, getSimpleConditionFromExpressions } from './SimpleCondition'; function initializeSimpleCondition( @@ -30,7 +31,7 @@ export function determineAdvancedMode(simplifiedQueryEditor: boolean | undefined } /* - This hook is used mantain the state of the advanced mode, and the simple condition, + This hook is used mantain the state of the advanced mode, and the simple condition, depending on the editor settings, the alert type, and the queries. */ export const useAdvancedMode = ( diff --git a/public/app/features/alerting/unified/components/rule-editor/util.ts b/public/app/features/alerting/unified/components/rule-editor/util.ts index 1c432344b1c..3933de4f48e 100644 --- a/public/app/features/alerting/unified/components/rule-editor/util.ts +++ b/public/app/features/alerting/unified/components/rule-editor/util.ts @@ -15,8 +15,6 @@ import { isExpressionQuery } from 'app/features/expressions/guards'; import { ClassicCondition, ExpressionQueryType } from 'app/features/expressions/types'; import { AlertQuery } from 'app/types/unified-alerting-dto'; -import { RuleFormType } from '../../types/rule-form'; - import { createDagFromQueries, getOriginOfRefId } from './dag'; export function queriesWithUpdatedReferences( @@ -312,18 +310,6 @@ export function getStatusMessage(data: PanelData): string | undefined { return data.error?.message ?? genericErrorMessage; } -export function translateRouteParamToRuleType(param = ''): RuleFormType { - if (param === 'recording') { - return RuleFormType.cloudRecording; - } - - if (param === 'grafana-recording') { - return RuleFormType.grafanaRecording; - } - - return RuleFormType.grafana; -} - /** * This function finds what refIds have been updated given the previous Array of queries and an Array of updated data queries. * All expression queries are discarded from the arrays, since we have separate handlers for those (see "onUpdateRefId") of the ExpressionEditor 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 be7153bc131..7f41a873797 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 @@ -4,7 +4,6 @@ import { byRole, byText } from 'testing-library-selector'; import { setPluginLinksHook } from '@grafana/runtime'; import { setupMswServer } from 'app/features/alerting/unified/mockApi'; -import { setFolderAccessControl } from 'app/features/alerting/unified/mocks/server/configure'; import { AlertManagerDataSourceJsonData } from 'app/plugins/datasource/alertmanager/types'; import { AccessControlAction } from 'app/types'; import { CombinedRule, RuleIdentifier } from 'app/types/unified-alerting'; @@ -20,6 +19,7 @@ import { mockPromAlertingRule, } from '../../mocks'; import { grafanaRulerRule } from '../../mocks/grafanaRulerApi'; +import { grantPermissionsHelper } from '../../test/test-utils'; import { setupDataSources } from '../../testSetup/datasources'; import { Annotation } from '../../utils/constants'; import { DataSourceType } from '../../utils/datasource'; @@ -76,16 +76,6 @@ setPluginLinksHook(() => ({ isLoading: false, })); -/** - * "Grants" permissions via contextSrv mock, and additionally sets folder access control - * API response to match - */ -const grantPermissionsHelper = (permissions: AccessControlAction[]) => { - const permissionsHash = permissions.reduce((hash, permission) => ({ ...hash, [permission]: true }), {}); - grantUserPermissions(permissions); - setFolderAccessControl(permissionsHash); -}; - const openSilenceDrawer = async () => { const user = userEvent.setup(); await user.click(ELEMENTS.actions.more.button.get()); diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx index dcbc646ae03..f755d6ff9e3 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx @@ -1,17 +1,17 @@ +import { HttpResponse } from 'msw'; import { render } from 'test/test-utils'; import { byLabelText, byTestId, byText, byTitle } from 'testing-library-selector'; -import { CombinedRuleNamespace } from 'app/types/unified-alerting'; +import { AccessControlAction } from 'app/types'; +import { RuleGroupIdentifier } from 'app/types/unified-alerting'; -import { - mockCombinedRule, - mockCombinedRuleNamespace, - mockDataSource, - mockPromAlertingRule, - mockPromRecordingRule, - mockRulerAlertingRule, - mockRulerRecordingRule, -} from '../../mocks'; +import { GRAFANA_RULER_CONFIG } from '../../api/featureDiscoveryApi'; +import server, { setupMswServer } from '../../mockApi'; +import { mimirDataSource } from '../../mocks/server/configure'; +import { alertingFactory } from '../../mocks/server/db'; +import { rulerRuleGroupHandler as grafanaRulerRuleGroupHandler } from '../../mocks/server/handlers/grafanaRuler'; +import { rulerRuleGroupHandler } from '../../mocks/server/handlers/mimirRuler'; +import { grantPermissionsHelper } from '../../test/test-utils'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; import { EditRuleGroupModal } from './EditRuleGroupModal'; @@ -29,133 +29,165 @@ const ui = { }; const noop = () => jest.fn(); +setupMswServer(); -describe('EditGroupModal', () => { +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + useReturnToPrevious: jest.fn(), +})); + +describe('EditGroupModal component on cloud alert rules', () => { it('Should disable all inputs but interval when intervalEditOnly is set', async () => { - const namespace = mockCombinedRuleNamespace({ - name: 'my-alerts', - rulesSource: mockDataSource(), - groups: [{ name: 'default-group', interval: '90s', rules: [], totals: {} }], + const { rulerConfig } = mimirDataSource(); + + const group = alertingFactory.ruler.group.build({ + rules: [alertingFactory.ruler.alertingRule.build(), alertingFactory.ruler.recordingRule.build()], }); - const group = namespace.groups[0]; + // @TODO need to simplify this a bit I think, ideally there would be a higher-level function that simply sets up a few rules + // and attaches the ruler and prometheus endpoint(s) – including the namespaces and group endpoints. + server.use( + rulerRuleGroupHandler({ + response: HttpResponse.json(group), + }) + ); - render(); + const rulerGroupIdentifier: RuleGroupIdentifier = { + dataSourceName: rulerConfig.dataSourceName, + groupName: 'default-group', + namespaceName: 'my-namespace', + }; + + render( + + ); expect(await ui.input.namespace.find()).toHaveAttribute('readonly'); expect(ui.input.group.get()).toHaveAttribute('readonly'); expect(ui.input.interval.get()).not.toHaveAttribute('readonly'); }); -}); - -describe('EditGroupModal component on cloud alert rules', () => { - const promDsSettings = mockDataSource({ name: 'Prometheus-1', uid: 'Prometheus-1' }); - - const alertingRule = mockCombinedRule({ - namespace: undefined, - promRule: mockPromAlertingRule({ name: 'alerting-rule-cpu' }), - rulerRule: mockRulerAlertingRule({ alert: 'alerting-rule-cpu' }), - }); - - const recordingRule1 = mockCombinedRule({ - namespace: undefined, - promRule: mockPromRecordingRule({ name: 'recording-rule-memory' }), - rulerRule: mockRulerRecordingRule({ record: 'recording-rule-memory' }), - }); - - const recordingRule2 = mockCombinedRule({ - namespace: undefined, - promRule: mockPromRecordingRule({ name: 'recording-rule-cpu' }), - rulerRule: mockRulerRecordingRule({ record: 'recording-rule-cpu' }), - }); it('Should show alert table in case of having some non-recording rules in the group', async () => { - const promNs = mockCombinedRuleNamespace({ - name: 'prometheus-ns', - rulesSource: promDsSettings, - groups: [ - { name: 'default-group', interval: '90s', rules: [alertingRule, recordingRule1, recordingRule2], totals: {} }, - ], + const { dataSource, rulerConfig } = mimirDataSource(); + + const group = alertingFactory.ruler.group.build({ + rules: [alertingFactory.ruler.alertingRule.build(), alertingFactory.ruler.recordingRule.build()], }); - const group = promNs.groups[0]; + // @TODO need to simplify this a bit I think, ideally there would be a higher-level function that simply sets up a few rules + // and attaches the ruler and prometheus endpoint(s) – including the namespaces and group endpoints. + server.use( + rulerRuleGroupHandler({ + response: HttpResponse.json(group), + }) + ); - render(); + const ruleGroupIdentifier: RuleGroupIdentifier = { + dataSourceName: dataSource.name, + groupName: group.name, + namespaceName: 'ns1', + }; - expect(await ui.input.namespace.find()).toHaveValue('prometheus-ns'); + render(); + + expect(await ui.input.namespace.find()).toHaveValue('ns1'); expect(ui.input.namespace.get()).not.toHaveAttribute('readonly'); - expect(ui.input.group.get()).toHaveValue('default-group'); + expect(ui.input.group.get()).toHaveValue(group.name); + + // @ts-ignore + const ruleName = group.rules.at(0).alert; expect(ui.tableRows.getAll()).toHaveLength(1); // Only one rule is non-recording - expect(ui.tableRows.getAll()[0]).toHaveTextContent('alerting-rule-cpu'); + expect(ui.tableRows.getAll().at(0)).toHaveTextContent(ruleName); }); it('Should not show alert table in case of having exclusively recording rules in the group', async () => { - const promNs = mockCombinedRuleNamespace({ - name: 'prometheus-ns', - rulesSource: promDsSettings, - groups: [{ name: 'default-group', interval: '90s', rules: [recordingRule1, recordingRule2], totals: {} }], + const { dataSource, rulerConfig } = mimirDataSource(); + + const group = alertingFactory.ruler.group.build({ + rules: [alertingFactory.ruler.recordingRule.build(), alertingFactory.ruler.recordingRule.build()], }); - const group = promNs.groups[0]; + // @TODO need to simplify this a bit I think + server.use( + rulerRuleGroupHandler({ + response: HttpResponse.json(group), + }) + ); - render(); + const ruleGroupIdentifier: RuleGroupIdentifier = { + dataSourceName: dataSource.name, + groupName: group.name, + namespaceName: 'ns1', + }; + + render(); expect(ui.table.query()).not.toBeInTheDocument(); expect(await ui.noRulesText.find()).toBeInTheDocument(); }); }); describe('EditGroupModal component on grafana-managed alert rules', () => { - const grafanaNamespace: CombinedRuleNamespace = { - name: 'namespace1', - rulesSource: GRAFANA_RULES_SOURCE_NAME, - groups: [ - { - name: 'grafanaGroup1', - interval: '30s', - rules: [ - mockCombinedRule({ - namespace: undefined, - promRule: mockPromAlertingRule({ name: 'high-cpu-1' }), - rulerRule: mockRulerAlertingRule({ alert: 'high-cpu-1' }), - }), - mockCombinedRule({ - namespace: undefined, - promRule: mockPromAlertingRule({ name: 'high-memory' }), - rulerRule: mockRulerAlertingRule({ alert: 'high-memory' }), - }), - ], - totals: {}, - }, - ], + // @TODO simplify folder stuff, should also have a higher-level function to set these up + const folder = alertingFactory.folder.build(); + const NAMESPACE_UID = folder.uid; + + const group = alertingFactory.ruler.group.build({ + rules: [alertingFactory.ruler.alertingRule.build(), alertingFactory.ruler.alertingRule.build()], + }); + + const ruleGroupIdentifier: RuleGroupIdentifier = { + dataSourceName: GRAFANA_RULES_SOURCE_NAME, + groupName: group.name, + namespaceName: NAMESPACE_UID, }; - const grafanaGroup1 = grafanaNamespace.groups[0]; + beforeEach(() => { + grantPermissionsHelper([ + AccessControlAction.AlertingRuleCreate, + AccessControlAction.AlertingRuleRead, + AccessControlAction.AlertingRuleUpdate, + ]); + + server.use( + grafanaRulerRuleGroupHandler({ + response: HttpResponse.json(group), + }) + ); + }); const renderWithGrafanaGroup = () => - render(); + render( + + ); it('Should show alert table', async () => { renderWithGrafanaGroup(); - expect(await ui.input.namespace.find()).toHaveValue('namespace1'); - expect(ui.input.group.get()).toHaveValue('grafanaGroup1'); - expect(ui.input.interval.get()).toHaveValue('30s'); + expect(await ui.input.namespace.find()).toHaveValue(NAMESPACE_UID); + expect(ui.input.group.get()).toHaveValue(group.name); + expect(ui.input.interval.get()).toHaveValue(group.interval); expect(ui.tableRows.getAll()).toHaveLength(2); - expect(ui.tableRows.getAll()[0]).toHaveTextContent('high-cpu-1'); - expect(ui.tableRows.getAll()[1]).toHaveTextContent('high-memory'); + // @ts-ignore + expect(ui.tableRows.getAll().at(0)).toHaveTextContent(group.rules.at(0).alert); + // @ts-ignore + expect(ui.tableRows.getAll().at(1)).toHaveTextContent(group.rules.at(1).alert); }); it('Should have folder input in readonly mode', async () => { renderWithGrafanaGroup(); - expect(await ui.input.namespace.find()).toHaveAttribute('readonly'); }); it('Should not display folder link if no folderUrl provided', async () => { renderWithGrafanaGroup(); - expect(await ui.input.namespace.find()).toHaveValue('namespace1'); + expect(await ui.input.namespace.find()).toHaveValue(NAMESPACE_UID); expect(ui.folderLink.query()).not.toBeInTheDocument(); }); }); diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx index 76427e7d5fa..828920a7e52 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -4,32 +4,46 @@ import { useMemo } from 'react'; import { FieldValues, FormProvider, RegisterOptions, useForm, useFormContext } from 'react-hook-form'; import { GrafanaTheme2 } from '@grafana/data'; -import { Alert, Badge, Button, Field, Input, Label, LinkButton, Modal, Stack, useStyles2 } from '@grafana/ui'; +import { + Alert, + Badge, + Button, + Field, + Input, + Label, + LinkButton, + LoadingPlaceholder, + Modal, + Stack, + useStyles2, +} from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; -import { Trans } from 'app/core/internationalization'; +import { Trans, t } from 'app/core/internationalization'; import { dispatch } from 'app/store/store'; -import { CombinedRuleGroup, CombinedRuleNamespace, RuleGroupIdentifier } from 'app/types/unified-alerting'; -import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; +import { RuleGroupIdentifier, RulerDataSourceConfig } from 'app/types/unified-alerting'; +import { RulerRuleDTO, RulerRuleGroupDTO } from 'app/types/unified-alerting-dto'; +import { alertRuleApi } from '../../api/alertRuleApi'; import { useMoveRuleGroup, useRenameRuleGroup, useUpdateRuleGroupConfiguration, } from '../../hooks/ruleGroup/useUpdateRuleGroup'; import { anyOfRequestState } from '../../hooks/useAsync'; +import { DEFAULT_GROUP_EVALUATION_INTERVAL } from '../../rule-editor/formDefaults'; import { fetchRulerRulesAction, rulesInSameGroupHaveInvalidFor } from '../../state/actions'; import { checkEvaluationIntervalGlobalLimit } from '../../utils/config'; -import { GRAFANA_RULES_SOURCE_NAME, getRulesSourceName } from '../../utils/datasource'; +import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; import { stringifyErrorLike } from '../../utils/misc'; -import { DEFAULT_GROUP_EVALUATION_INTERVAL } from '../../utils/rule-form'; import { AlertInfo, getAlertInfo, isGrafanaOrDataSourceRecordingRule } from '../../utils/rules'; import { formatPrometheusDuration, parsePrometheusDuration, safeParsePrometheusDuration } from '../../utils/time'; import { DynamicTable, DynamicTableColumnProps, DynamicTableItemProps } from '../DynamicTable'; import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning'; -import { decodeGrafanaNamespace, encodeGrafanaNamespace } from '../expressions/util'; import { EvaluationGroupQuickPick } from '../rule-editor/EvaluationGroupQuickPick'; import { MIN_TIME_RANGE_STEP_S } from '../rule-editor/GrafanaEvaluationBehavior'; +const useRuleGroupDefinition = alertRuleApi.endpoints.getRuleGroupForNamespace.useQuery; + const ITEMS_PER_PAGE = 10; function ForBadge({ message, error }: { message: string; error?: boolean }) { @@ -170,17 +184,59 @@ export const evaluateEveryValidationOptions = (rules: Rul }); export interface ModalProps { - namespace: CombinedRuleNamespace; - group: CombinedRuleGroup; + ruleGroupIdentifier: RuleGroupIdentifier; + folderTitle?: string; + rulerConfig: RulerDataSourceConfig; onClose: (saved?: boolean) => void; intervalEditOnly?: boolean; folderUrl?: string; - folderUid?: string; hideFolder?: boolean; } -export function EditRuleGroupModal(props: ModalProps): React.ReactElement { - const { namespace, group, onClose, intervalEditOnly, folderUid } = props; +export interface ModalFormProps { + ruleGroupIdentifier: RuleGroupIdentifier; + folderTitle?: string; // used to display the GMA folder title + ruleGroup: RulerRuleGroupDTO; + onClose: (saved?: boolean) => void; + intervalEditOnly?: boolean; + folderUrl?: string; + hideFolder?: boolean; +} + +// this component just wraps the modal with some loading state for grabbing rules and such +export function EditRuleGroupModal(props: ModalProps) { + const { ruleGroupIdentifier, rulerConfig, intervalEditOnly, onClose } = props; + const rulesSourceName = ruleGroupIdentifier.dataSourceName; + const isGrafanaManagedGroup = rulesSourceName === GRAFANA_RULES_SOURCE_NAME; + + const modalTitle = + intervalEditOnly || isGrafanaManagedGroup ? 'Edit evaluation group' : 'Edit namespace or evaluation group'; + + const styles = useStyles2(getStyles); + + const { + data: ruleGroup, + error, + isLoading, + } = useRuleGroupDefinition({ + group: ruleGroupIdentifier.groupName, + namespace: ruleGroupIdentifier.namespaceName, + rulerConfig, + }); + + const loadingText = t('alerting.common.loading', 'Loading...'); + + return ( + + {isLoading && } + {error ? stringifyErrorLike(error) : null} + {ruleGroup && } + + ); +} + +export function EditRuleGroupModalForm(props: ModalFormProps): React.ReactElement { + const { ruleGroup, ruleGroupIdentifier, folderTitle, onClose, intervalEditOnly } = props; const styles = useStyles2(getStyles); const notifyApp = useAppNotification(); @@ -200,32 +256,21 @@ export function EditRuleGroupModal(props: ModalProps): React.ReactElement { const defaultValues = useMemo( (): FormValues => ({ - namespaceName: decodeGrafanaNamespace(namespace).name, - groupName: group.name, - groupInterval: group.interval ?? DEFAULT_GROUP_EVALUATION_INTERVAL, + namespaceName: ruleGroupIdentifier.namespaceName, + groupName: ruleGroupIdentifier.groupName, + groupInterval: ruleGroup?.interval ?? DEFAULT_GROUP_EVALUATION_INTERVAL, }), - [namespace, group.name, group.interval] + [ruleGroup?.interval, ruleGroupIdentifier.groupName, ruleGroupIdentifier.namespaceName] ); - const rulesSourceName = getRulesSourceName(namespace.rulesSource); + const rulesSourceName = ruleGroupIdentifier.dataSourceName; const isGrafanaManagedGroup = rulesSourceName === GRAFANA_RULES_SOURCE_NAME; - // parse any parent folders the alert rule might be stored in - const nestedFolderParents = decodeGrafanaNamespace(namespace).parents; - const nameSpaceLabel = isGrafanaManagedGroup ? 'Folder' : 'Namespace'; const onSubmit = async (values: FormValues) => { - const ruleGroupIdentifier: RuleGroupIdentifier = { - dataSourceName: rulesSourceName, - groupName: group.name, - namespaceName: isGrafanaManagedGroup ? folderUid! : namespace.name, - }; - // make sure that when dealing with a nested folder for Grafana managed rules we encode the folder properly - const updatedNamespaceName = isGrafanaManagedGroup - ? encodeGrafanaNamespace(values.namespaceName, nestedFolderParents) - : values.namespaceName; + const updatedNamespaceName = values.namespaceName; const updatedGroupName = values.groupName; const updatedInterval = values.groupInterval; @@ -266,136 +311,133 @@ export function EditRuleGroupModal(props: ModalProps): React.ReactElement { }; const rulesWithoutRecordingRules = compact( - group.rules.map((r) => r.rulerRule).filter((rule) => !isGrafanaOrDataSourceRecordingRule(rule)) + ruleGroup?.rules.filter((rule) => !isGrafanaOrDataSourceRecordingRule(rule)) ); const hasSomeNoRecordingRules = rulesWithoutRecordingRules.length > 0; - const modalTitle = - intervalEditOnly || isGrafanaManagedGroup ? 'Edit evaluation group' : 'Edit namespace or evaluation group'; return ( - - -
- <> - {!props.hideFolder && ( - - - {nameSpaceLabel} - - } - invalid={Boolean(errors.namespaceName) ? true : undefined} - error={errors.namespaceName?.message} - > - - - {isGrafanaManagedGroup && props.folderUrl && ( - - )} - - )} - - Evaluation group - - } - invalid={!!errors.groupName} - error={errors.groupName?.message} - > - - - - Evaluation interval - - } - invalid={Boolean(errors.groupInterval) ? true : undefined} - error={errors.groupInterval?.message} - > - + + + <> + {!props.hideFolder && ( + + + {nameSpaceLabel} + + } + invalid={Boolean(errors.namespaceName) ? true : undefined} + error={errors.namespaceName?.message} + > - setValue('groupInterval', value, { shouldValidate: true, shouldDirty: true })} - /> - - - - {/* if we're dealing with a Grafana-managed group, check if the evaluation interval is valid / permitted */} - {isGrafanaManagedGroup && checkEvaluationIntervalGlobalLimit(watch('groupInterval')).exceedsLimit && ( - - )} - - {!hasSomeNoRecordingRules &&
This group does not contain alert rules.
} - {hasSomeNoRecordingRules && ( - <> -
List of rules that belong to this group
-
- #Eval column represents the number of evaluations needed before alert starts firing. -
- - - )} - {error && {stringifyErrorLike(error)}} -
- - - - -
- - -
-
+ icon="folder-open" + target="_blank" + /> + )} +
+ )} + + Evaluation group + + } + invalid={!!errors.groupName} + error={errors.groupName?.message} + > + + + + Evaluation interval + + } + invalid={Boolean(errors.groupInterval) ? true : undefined} + error={errors.groupInterval?.message} + > + + + setValue('groupInterval', value, { shouldValidate: true, shouldDirty: true })} + /> + + + + {/* if we're dealing with a Grafana-managed group, check if the evaluation interval is valid / permitted */} + {isGrafanaManagedGroup && checkEvaluationIntervalGlobalLimit(watch('groupInterval')).exceedsLimit && ( + + )} + + {!hasSomeNoRecordingRules &&
This group does not contain alert rules.
} + {hasSomeNoRecordingRules && ( + <> +
List of rules that belong to this group
+
+ #Eval column represents the number of evaluations needed before alert starts firing. +
+ + + )} + {error && {stringifyErrorLike(error)}} +
+ + + + +
+ + + ); } diff --git a/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx b/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx index d79e630ffb5..9baab7600cd 100644 --- a/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx @@ -7,12 +7,14 @@ import { byRole, byTestId, byText } from 'testing-library-selector'; import { contextSrv } from 'app/core/services/context_srv'; import { configureStore } from 'app/store/configureStore'; import { AccessControlAction } from 'app/types'; -import { CombinedRuleGroup, CombinedRuleNamespace } from 'app/types/unified-alerting'; +import { CombinedRuleGroup, CombinedRuleNamespace, RulerDataSourceConfig } from 'app/types/unified-alerting'; import * as analytics from '../../Analytics'; +import { GRAFANA_RULER_CONFIG } from '../../api/featureDiscoveryApi'; import { useHasRuler } from '../../hooks/useHasRuler'; import { mockExportApi, mockFolderApi, setupMswServer } from '../../mockApi'; import { grantUserPermissions, mockCombinedRule, mockDataSource, mockFolder, mockGrafanaRulerRule } from '../../mocks'; +import { mimirDataSource } from '../../mocks/server/configure'; import { RulesGroup } from './RulesGroup'; @@ -38,10 +40,10 @@ const mocks = { useHasRuler: jest.mocked(useHasRuler), }; -function mockUseHasRuler(hasRuler: boolean, rulerRulesLoaded: boolean) { +function mockUseHasRuler(hasRuler: boolean, rulerConfig: RulerDataSourceConfig) { mocks.useHasRuler.mockReturnValue({ hasRuler, - rulerRulesLoaded, + rulerConfig, }); } @@ -107,7 +109,7 @@ describe('Rules group tests', () => { it('Should hide delete and edit group buttons', async () => { // Act - mockUseHasRuler(true, true); + mockUseHasRuler(true, GRAFANA_RULER_CONFIG); mockFolderApi(server).folder('cpu-usage', mockFolder({ uid: 'cpu-usage', canSave: false })); renderRulesGroup(namespace, group); expect(await screen.findByTestId('rule-group')).toBeInTheDocument(); @@ -119,7 +121,7 @@ describe('Rules group tests', () => { it('Should allow exporting rules group', async () => { // Arrange - mockUseHasRuler(true, true); + mockUseHasRuler(true, GRAFANA_RULER_CONFIG); mockFolderApi(server).folder('cpu-usage', mockFolder({ uid: 'cpu-usage' })); mockExportApi(server).exportRulesGroup('cpu-usage', 'TestGroup', { yaml: 'Yaml Export Content', @@ -151,6 +153,8 @@ describe('Rules group tests', () => { }); describe('Cloud rules', () => { + const { rulerConfig } = mimirDataSource(); + beforeEach(() => { contextSrv.isEditor = true; }); @@ -169,7 +173,7 @@ describe('Rules group tests', () => { it('When ruler enabled should display delete and edit group buttons', () => { // Arrange - mockUseHasRuler(true, true); + mockUseHasRuler(true, rulerConfig); // Act renderRulesGroup(namespace, group); @@ -182,7 +186,7 @@ describe('Rules group tests', () => { it('When ruler disabled should hide delete and edit group buttons', () => { // Arrange - mockUseHasRuler(false, false); + mockUseHasRuler(false, rulerConfig); // Act renderRulesGroup(namespace, group); @@ -195,7 +199,7 @@ describe('Rules group tests', () => { it('Delete button click should display confirmation modal', async () => { // Arrange - mockUseHasRuler(true, true); + mockUseHasRuler(true, rulerConfig); // Act renderRulesGroup(namespace, group); @@ -206,36 +210,4 @@ describe('Rules group tests', () => { expect(ui.confirmDeleteModal.confirmButton.get()).toBeInTheDocument(); }); }); - - describe('Analytics', () => { - beforeEach(() => { - contextSrv.isEditor = true; - }); - - const group: CombinedRuleGroup = { - name: 'TestGroup', - rules: [mockCombinedRule()], - totals: {}, - }; - - const namespace: CombinedRuleNamespace = { - name: 'TestNamespace', - rulesSource: mockDataSource(), - groups: [group], - }; - - it('Should log info when closing the edit group rule modal without saving', async () => { - mockUseHasRuler(true, true); - renderRulesGroup(namespace, group); - - await userEvent.click(ui.editGroupButton.get()); - - expect(screen.getByText('Cancel')).toBeInTheDocument(); - - await userEvent.click(screen.getByText('Cancel')); - - expect(screen.queryByText('Cancel')).not.toBeInTheDocument(); - expect(analytics.logInfo).toHaveBeenCalledWith(analytics.LogMessages.leavingRuleGroupEdit); - }); - }); }); diff --git a/public/app/features/alerting/unified/components/rules/RulesGroup.tsx b/public/app/features/alerting/unified/components/rules/RulesGroup.tsx index b1a5bddd43c..ccf725b5f99 100644 --- a/public/app/features/alerting/unified/components/rules/RulesGroup.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesGroup.tsx @@ -1,6 +1,6 @@ import { css } from '@emotion/css'; import pluralize from 'pluralize'; -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useMemo, useState } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; @@ -26,7 +26,7 @@ import { ActionIcon } from './ActionIcon'; import { EditRuleGroupModal } from './EditRuleGroupModal'; import { ReorderCloudGroupModal } from './ReorderRuleGroupModal'; import { RuleGroupStats } from './RuleStats'; -import { RulesTable } from './RulesTable'; +import { RulesTable, useIsRulesLoading } from './RulesTable'; type ViewMode = 'grouped' | 'list'; @@ -42,6 +42,7 @@ const { useDiscoverDsFeaturesQuery } = featureDiscoveryApi; export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }: Props) => { const { rulesSource } = namespace; const rulesSourceName = getRulesSourceName(rulesSource); + const rulerRulesLoaded = useIsRulesLoading(rulesSource); const [deleteRuleGroup] = useDeleteRuleGroup(); const styles = useStyles2(getStyles); @@ -58,7 +59,7 @@ export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }: setIsCollapsed(!expandAll); }, [expandAll]); - const { hasRuler, rulerRulesLoaded } = useHasRuler(namespace.rulesSource); + const { hasRuler, rulerConfig } = useHasRuler(namespace.rulesSource); const { currentData: dsFeatures } = useDiscoverDsFeaturesQuery({ rulesSourceName }); const rulerRule = group.rules[0]?.rulerRule; @@ -78,12 +79,15 @@ export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }: const isListView = viewMode === 'list'; const isGroupView = viewMode === 'grouped'; - const deleteGroup = async () => { - const namespaceName = decodeGrafanaNamespace(namespace).name; + const ruleGroupIdentifier = useMemo(() => { + const namespaceName = namespace.uid ?? namespace.name; const groupName = group.name; const dataSourceName = getRulesSourceName(namespace.rulesSource); - const ruleGroupIdentifier: RuleGroupIdentifier = { namespaceName, groupName, dataSourceName }; + return { namespaceName, groupName, dataSourceName }; + }, [namespace, group.name]); + + const deleteGroup = async () => { await deleteRuleGroup.execute(ruleGroupIdentifier); setIsDeletingGroup(false); }; @@ -274,13 +278,13 @@ export const RulesGroup = React.memo(({ group, namespace, expandAll, viewMode }: rules={group.rules} /> )} - {isEditingGroup && ( + {isEditingGroup && rulerConfig && ( closeEditModal()} folderUrl={folder?.canEdit ? makeFolderSettingsLink(folder.uid) : undefined} - folderUid={folderUID} /> )} {isReorderingGroup && dsFeatures?.rulerConfig && ( diff --git a/public/app/features/alerting/unified/components/rules/RulesTable.tsx b/public/app/features/alerting/unified/components/rules/RulesTable.tsx index 38eb1045e40..1f7c6c44b11 100644 --- a/public/app/features/alerting/unified/components/rules/RulesTable.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesTable.tsx @@ -4,7 +4,7 @@ import Skeleton from 'react-loading-skeleton'; import { GrafanaTheme2 } from '@grafana/data'; import { Pagination, Tooltip, useStyles2 } from '@grafana/ui'; -import { CombinedRule } from 'app/types/unified-alerting'; +import { CombinedRule, RulesSource } from 'app/types/unified-alerting'; import { DEFAULT_PER_PAGE_PAGINATION } from '../../../../../core/constants'; import { alertRuleApi } from '../../api/alertRuleApi'; @@ -14,6 +14,7 @@ import { useAsync } from '../../hooks/useAsync'; import { attachRulerRuleToCombinedRule } from '../../hooks/useCombinedRuleNamespaces'; import { useHasRuler } from '../../hooks/useHasRuler'; import { usePagination } from '../../hooks/usePagination'; +import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelector'; import { PluginOriginBadge } from '../../plugins/PluginOriginBadge'; import { calculateNextEvaluationEstimate } from '../../rule-list/components/util'; import { Annotation } from '../../utils/constants'; @@ -328,8 +329,20 @@ function RuleActionsCell({ rule, isLoadingRuler }: { rule: CombinedRule; isLoadi ); } +export function useIsRulesLoading(rulesSource: RulesSource) { + const rulerRules = useUnifiedAlertingSelector((state) => state.rulerRules); + const rulesSourceName = getRulesSourceName(rulesSource); + + const rulerRulesLoaded = Boolean(rulerRules[rulesSourceName]?.result); + return rulerRulesLoaded; +} + function useRuleStatus(rule: CombinedRule) { - const { hasRuler, rulerRulesLoaded } = useHasRuler(rule.namespace.rulesSource); + const rulesSource = rule.namespace.rulesSource; + + const rulerRulesLoaded = useIsRulesLoading(rulesSource); + const { hasRuler } = useHasRuler(rulesSource); + const { promRule, rulerRule } = rule; // If prometheusRulesPrimary is enabled, we don't fetch rules from the Ruler API (except for Grafana managed rules) diff --git a/public/app/features/alerting/unified/hooks/ruleGroup/useProduceNewRuleGroup.ts b/public/app/features/alerting/unified/hooks/ruleGroup/useProduceNewRuleGroup.ts index f7c54c1121b..6d8d890e737 100644 --- a/public/app/features/alerting/unified/hooks/ruleGroup/useProduceNewRuleGroup.ts +++ b/public/app/features/alerting/unified/hooks/ruleGroup/useProduceNewRuleGroup.ts @@ -7,7 +7,7 @@ import { alertRuleApi } from '../../api/alertRuleApi'; import { featureDiscoveryApi } from '../../api/featureDiscoveryApi'; import { notFoundToNullOrThrow } from '../../api/util'; import { ruleGroupReducer } from '../../reducers/ruler/ruleGroups'; -import { DEFAULT_GROUP_EVALUATION_INTERVAL } from '../../utils/rule-form'; +import { DEFAULT_GROUP_EVALUATION_INTERVAL } from '../../rule-editor/formDefaults'; const { useLazyGetRuleGroupForNamespaceQuery } = alertRuleApi; const { useLazyDiscoverDsFeaturesQuery } = featureDiscoveryApi; diff --git a/public/app/features/alerting/unified/hooks/useHasRuler.ts b/public/app/features/alerting/unified/hooks/useHasRuler.ts index db2e85f09ee..399aca6081f 100644 --- a/public/app/features/alerting/unified/hooks/useHasRuler.ts +++ b/public/app/features/alerting/unified/hooks/useHasRuler.ts @@ -3,19 +3,14 @@ import { RulesSource } from 'app/types/unified-alerting'; import { featureDiscoveryApi } from '../api/featureDiscoveryApi'; import { getRulesSourceName } from '../utils/datasource'; -import { useUnifiedAlertingSelector } from './useUnifiedAlertingSelector'; - const { useDiscoverDsFeaturesQuery } = featureDiscoveryApi; // datasource has ruler if the discovery api returns a rulerConfig export function useHasRuler(rulesSource: RulesSource) { - const rulerRules = useUnifiedAlertingSelector((state) => state.rulerRules); const rulesSourceName = getRulesSourceName(rulesSource); const { currentData: dsFeatures } = useDiscoverDsFeaturesQuery({ rulesSourceName }); - const hasRuler = Boolean(dsFeatures?.rulerConfig); - const rulerRulesLoaded = Boolean(rulerRules[rulesSourceName]?.result); - return { hasRuler, rulerRulesLoaded }; + return { hasRuler, rulerConfig: dsFeatures?.rulerConfig }; } diff --git a/public/app/features/alerting/unified/hooks/useUnifiedAlertingSelector.ts b/public/app/features/alerting/unified/hooks/useUnifiedAlertingSelector.ts index 7b50599934c..5e6daa81e8b 100644 --- a/public/app/features/alerting/unified/hooks/useUnifiedAlertingSelector.ts +++ b/public/app/features/alerting/unified/hooks/useUnifiedAlertingSelector.ts @@ -4,6 +4,9 @@ import { StoreState, useSelector } from 'app/types'; import { UnifiedAlertingState } from '../state/reducers'; +/** + * @deprecated: DO NOT USE THIS; when using this you are INCORRECTLY assuming that we already have dispatched an action to populate the redux store values + */ export function useUnifiedAlertingSelector( selector: (state: UnifiedAlertingState) => TSelected, equalityFn?: (left: TSelected, right: TSelected) => boolean diff --git a/public/app/features/alerting/unified/mocks.ts b/public/app/features/alerting/unified/mocks.ts index 5676dbb9a04..278178b395e 100644 --- a/public/app/features/alerting/unified/mocks.ts +++ b/public/app/features/alerting/unified/mocks.ts @@ -196,15 +196,12 @@ export const mockRulerAlertingRule = (partial: Partial = { ...partial, }); -export const mockRulerRecordingRule = (partial: Partial = {}): RulerAlertingRuleDTO => ({ - alert: 'alert1', +export const mockRulerRecordingRule = (partial: Partial = {}): RulerRecordingRuleDTO => ({ + record: 'alert1', expr: 'up = 1', labels: { severity: 'warning', }, - annotations: { - summary: 'test alert', - }, ...partial, }); @@ -735,7 +732,7 @@ export function mockStore(recipe: (state: StoreState) => void) { return configureStore(produce(defaultState, recipe)); } -export function mockAlertQuery(query: Partial): AlertQuery { +export function mockAlertQuery(query: Partial = {}): AlertQuery { return { datasourceUid: '--uid--', refId: 'A', diff --git a/public/app/features/alerting/unified/mocks/server/configure.ts b/public/app/features/alerting/unified/mocks/server/configure.ts index 063a070a10f..bc638438e14 100644 --- a/public/app/features/alerting/unified/mocks/server/configure.ts +++ b/public/app/features/alerting/unified/mocks/server/configure.ts @@ -20,6 +20,7 @@ import { SupportedPlugin } from 'app/features/alerting/unified/types/pluginBridg import { clearPluginSettingsCache } from 'app/features/plugins/pluginSettings'; import { AlertmanagerChoice } from 'app/plugins/datasource/alertmanager/types'; import { FolderDTO } from 'app/types'; +import { RulerDataSourceConfig } from 'app/types/unified-alerting'; import { PromRuleGroupDTO } from 'app/types/unified-alerting-dto'; import { setupDataSources } from '../../testSetup/datasources'; @@ -108,9 +109,15 @@ export function mimirDataSource() { { alerting: true, module: 'core:plugin/prometheus' } ); + const rulerConfig: RulerDataSourceConfig = { + apiVersion: 'config', + dataSourceUid: dataSource.uid, + dataSourceName: dataSource.name, + }; + setupDataSources(dataSource); - return { dataSource }; + return { dataSource, rulerConfig }; } export function setPrometheusRules(ds: DataSourceInstanceSettings, groups: PromRuleGroupDTO[]) { diff --git a/public/app/features/alerting/unified/mocks/server/db.ts b/public/app/features/alerting/unified/mocks/server/db.ts index 55e76f78208..ba78b0be31a 100644 --- a/public/app/features/alerting/unified/mocks/server/db.ts +++ b/public/app/features/alerting/unified/mocks/server/db.ts @@ -1,18 +1,23 @@ import { Factory } from 'fishery'; +import { uniqueId } from 'lodash'; import { DataSourceInstanceSettings, PluginType } from '@grafana/data'; import { config, setDataSourceSrv } from '@grafana/runtime'; +import { FolderDTO } from 'app/types'; import { PromAlertingRuleDTO, PromAlertingRuleState, PromRuleGroupDTO, PromRuleType, + RulerAlertingRuleDTO, + RulerRecordingRuleDTO, + RulerRuleGroupDTO, } from 'app/types/unified-alerting-dto'; import { MockDataSourceSrv } from '../../mocks'; import { DataSourceType } from '../../utils/datasource'; -const ruleFactory = Factory.define(({ sequence }) => ({ +const prometheusRuleFactory = Factory.define(({ sequence }) => ({ name: `test-rule-${sequence}`, query: 'test-query', state: PromAlertingRuleState.Inactive, @@ -21,15 +26,35 @@ const ruleFactory = Factory.define(({ sequence }) => ({ labels: { team: 'infra' }, })); -const groupFactory = Factory.define(({ sequence }) => { +const rulerAlertingRuleFactory = Factory.define(({ sequence }) => ({ + alert: `ruler-alerting-rule-${sequence}`, + expr: 'vector(0)', + annotations: { 'annotation-key-1': 'annotation-value-1' }, + labels: { 'label-key-1': 'label-value-1' }, + for: '5m', +})); + +const rulerRecordingRuleFactory = Factory.define(({ sequence }) => ({ + record: `ruler-recording-rule-${sequence}`, + expr: 'vector(0)', + labels: { 'label-key-1': 'label-value-1' }, +})); + +const rulerRuleGroupFactory = Factory.define(({ sequence }) => ({ + name: `ruler-rule-group-${sequence}`, + rules: [], + interval: '1m', +})); + +const prometheusRuleGroupFactory = Factory.define(({ sequence }) => { const group = { name: `test-group-${sequence}`, file: `test-namespace`, interval: 10, - rules: ruleFactory.buildList(10), + rules: prometheusRuleFactory.buildList(10), }; - ruleFactory.rewindSequence(); + prometheusRuleFactory.rewindSequence(); return group; }); @@ -72,8 +97,33 @@ const dataSourceFactory = Factory.define(({ sequence }; }); +const grafanaFolderFactory = Factory.define(({ sequence }) => ({ + id: sequence, + uid: uniqueId(), + title: `Mock Folder ${sequence}`, + version: 1, + url: '', + canAdmin: true, + canDelete: true, + canEdit: true, + canSave: true, + created: '', + createdBy: '', + hasAcl: false, + updated: '', + updatedBy: '', +})); + export const alertingFactory = { - group: groupFactory, - rule: ruleFactory, + folder: grafanaFolderFactory, + prometheus: { + group: prometheusRuleGroupFactory, + rule: prometheusRuleFactory, + }, + ruler: { + group: rulerRuleGroupFactory, + alertingRule: rulerAlertingRuleFactory, + recordingRule: rulerRecordingRuleFactory, + }, dataSource: dataSourceFactory, }; diff --git a/public/app/features/alerting/unified/reducers/ruler/__snapshots__/ruleGroups.test.ts.snap b/public/app/features/alerting/unified/reducers/ruler/__snapshots__/ruleGroups.test.ts.snap index 95329f16627..42aac7884e8 100644 --- a/public/app/features/alerting/unified/reducers/ruler/__snapshots__/ruleGroups.test.ts.snap +++ b/public/app/features/alerting/unified/reducers/ruler/__snapshots__/ruleGroups.test.ts.snap @@ -104,10 +104,6 @@ exports[`removing a rule should remove a Data source managed ruler rule without }, }, { - "alert": "alert1", - "annotations": { - "summary": "test alert", - }, "expr": "up = 1", "labels": { "severity": "warning", diff --git a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx b/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.test.tsx similarity index 93% rename from public/app/features/alerting/unified/CloneRuleEditor.test.tsx rename to public/app/features/alerting/unified/rule-editor/CloneRuleEditor.test.tsx index 7399bcae26b..298be9b1529 100644 --- a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.test.tsx @@ -4,36 +4,36 @@ import { getWrapper, render, waitFor, waitForElementToBeRemoved, within } from ' import { byRole, byTestId, byText } from 'testing-library-selector'; import { MIMIR_DATASOURCE_UID } from 'app/features/alerting/unified/mocks/server/constants'; +import { AccessControlAction } from 'app/types'; import { RuleWithLocation } from 'app/types/unified-alerting'; - -import { AccessControlAction } from '../../../types'; import { RulerAlertingRuleDTO, RulerGrafanaRuleDTO, RulerRecordingRuleDTO, RulerRuleDTO, -} from '../../../types/unified-alerting-dto'; +} from 'app/types/unified-alerting-dto'; -import { CloneRuleEditor, cloneRuleDefinition } from './CloneRuleEditor'; -import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; -import { setupMswServer } from './mockApi'; +import { ExpressionEditorProps } from '../components/rule-editor/ExpressionEditor'; +import { setupMswServer } from '../mockApi'; import { grantUserPermissions, mockDataSource, mockRulerAlertingRule, mockRulerGrafanaRule, mockRulerRuleGroup, -} from './mocks'; -import { grafanaRulerRule } from './mocks/grafanaRulerApi'; -import { mockRulerRulesApiResponse, mockRulerRulesGroupApiResponse } from './mocks/rulerApi'; -import { AlertingQueryRunner } from './state/AlertingQueryRunner'; -import { setupDataSources } from './testSetup/datasources'; -import { RuleFormValues } from './types/rule-form'; -import { Annotation } from './utils/constants'; -import { getDefaultFormValues } from './utils/rule-form'; -import { hashRulerRule } from './utils/rule-id'; +} from '../mocks'; +import { grafanaRulerRule } from '../mocks/grafanaRulerApi'; +import { mockRulerRulesApiResponse, mockRulerRulesGroupApiResponse } from '../mocks/rulerApi'; +import { AlertingQueryRunner } from '../state/AlertingQueryRunner'; +import { setupDataSources } from '../testSetup/datasources'; +import { RuleFormValues } from '../types/rule-form'; +import { Annotation } from '../utils/constants'; +import { hashRulerRule } from '../utils/rule-id'; -jest.mock('./components/rule-editor/ExpressionEditor', () => ({ +import { CloneRuleEditor, cloneRuleDefinition } from './CloneRuleEditor'; +import { getDefaultFormValues } from './formDefaults'; + +jest.mock('../components/rule-editor/ExpressionEditor', () => ({ // eslint-disable-next-line react/display-name ExpressionEditor: ({ value, onChange }: ExpressionEditorProps) => ( onChange(e.target.value)} /> diff --git a/public/app/features/alerting/unified/CloneRuleEditor.tsx b/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.tsx similarity index 72% rename from public/app/features/alerting/unified/CloneRuleEditor.tsx rename to public/app/features/alerting/unified/rule-editor/CloneRuleEditor.tsx index 92745f20bde..e92ad4b46e6 100644 --- a/public/app/features/alerting/unified/CloneRuleEditor.tsx +++ b/public/app/features/alerting/unified/rule-editor/CloneRuleEditor.tsx @@ -1,18 +1,17 @@ import { cloneDeep } from 'lodash'; -import { locationService } from '@grafana/runtime/src'; -import { Alert, LoadingPlaceholder } from '@grafana/ui/src'; +import { locationService } from '@grafana/runtime'; +import { Alert, LoadingPlaceholder } from '@grafana/ui'; +import { RuleIdentifier, RuleWithLocation } from 'app/types/unified-alerting'; +import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; -import { RuleIdentifier, RuleWithLocation } from '../../../types/unified-alerting'; -import { RulerRuleDTO } from '../../../types/unified-alerting-dto'; - -import { AlertRuleForm } from './components/rule-editor/alert-rule-form/AlertRuleForm'; -import { useRuleWithLocation } from './hooks/useCombinedRule'; -import { generateCopiedName } from './utils/duplicate'; -import { stringifyErrorLike } from './utils/misc'; -import { rulerRuleToFormValues } from './utils/rule-form'; -import { getRuleName, isAlertingRulerRule, isGrafanaRulerRule, isRecordingRulerRule } from './utils/rules'; -import { createRelativeUrl } from './utils/url'; +import { AlertRuleForm } from '../components/rule-editor/alert-rule-form/AlertRuleForm'; +import { useRuleWithLocation } from '../hooks/useCombinedRule'; +import { generateCopiedName } from '../utils/duplicate'; +import { stringifyErrorLike } from '../utils/misc'; +import { rulerRuleToFormValues } from '../utils/rule-form'; +import { getRuleName, isAlertingRulerRule, isGrafanaRulerRule, isRecordingRulerRule } from '../utils/rules'; +import { createRelativeUrl } from '../utils/url'; export function CloneRuleEditor({ sourceRuleId }: { sourceRuleId: RuleIdentifier }) { const { loading, result: rule, error } = useRuleWithLocation({ ruleIdentifier: sourceRuleId }); diff --git a/public/app/features/alerting/unified/ExistingRuleEditor.tsx b/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx similarity index 71% rename from public/app/features/alerting/unified/ExistingRuleEditor.tsx rename to public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx index 17dc0d74753..7d1b9d92fcf 100644 --- a/public/app/features/alerting/unified/ExistingRuleEditor.tsx +++ b/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx @@ -1,19 +1,18 @@ import { Alert, LoadingPlaceholder } from '@grafana/ui'; import { RuleIdentifier } from 'app/types/unified-alerting'; -import { AlertWarning } from './AlertWarning'; -import { AlertRuleForm } from './components/rule-editor/alert-rule-form/AlertRuleForm'; -import { useRuleWithLocation } from './hooks/useCombinedRule'; -import { useIsRuleEditable } from './hooks/useIsRuleEditable'; -import { stringifyErrorLike } from './utils/misc'; -import * as ruleId from './utils/rule-id'; +import { AlertWarning } from '../AlertWarning'; +import { AlertRuleForm } from '../components/rule-editor/alert-rule-form/AlertRuleForm'; +import { useRuleWithLocation } from '../hooks/useCombinedRule'; +import { useIsRuleEditable } from '../hooks/useIsRuleEditable'; +import { stringifyErrorLike } from '../utils/misc'; +import * as ruleId from '../utils/rule-id'; interface ExistingRuleEditorProps { identifier: RuleIdentifier; - id?: string; } -export function ExistingRuleEditor({ identifier, id }: ExistingRuleEditorProps) { +export function ExistingRuleEditor({ identifier }: ExistingRuleEditorProps) { const { loading: loadingAlertRule, result: ruleWithLocation, diff --git a/public/app/features/alerting/unified/RuleEditor.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditor.tsx similarity index 69% rename from public/app/features/alerting/unified/RuleEditor.tsx rename to public/app/features/alerting/unified/rule-editor/RuleEditor.tsx index bdda3e5d28b..b5adba57cf2 100644 --- a/public/app/features/alerting/unified/RuleEditor.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditor.tsx @@ -5,14 +5,16 @@ import { NavModelItem } from '@grafana/data'; import { withErrorBoundary } from '@grafana/ui'; import { RuleIdentifier } from 'app/types/unified-alerting'; -import { AlertWarning } from './AlertWarning'; +import { AlertWarning } from '../AlertWarning'; +import { AlertingPageWrapper } from '../components/AlertingPageWrapper'; +import { AlertRuleForm } from '../components/rule-editor/alert-rule-form/AlertRuleForm'; +import { useURLSearchParams } from '../hooks/useURLSearchParams'; +import { useRulesAccess } from '../utils/accessControlHooks'; +import * as ruleId from '../utils/rule-id'; + import { CloneRuleEditor } from './CloneRuleEditor'; import { ExistingRuleEditor } from './ExistingRuleEditor'; -import { AlertingPageWrapper } from './components/AlertingPageWrapper'; -import { AlertRuleForm } from './components/rule-editor/alert-rule-form/AlertRuleForm'; -import { useURLSearchParams } from './hooks/useURLSearchParams'; -import { useRulesAccess } from './utils/accessControlHooks'; -import * as ruleId from './utils/rule-id'; +import { formValuesFromQueryParams, translateRouteParamToRuleType } from './formDefaults'; type RuleEditorPathParams = { id?: string; @@ -44,14 +46,8 @@ const getPageNav = (identifier?: RuleIdentifier, type?: RuleEditorPathParams['ty }; const RuleEditor = () => { - const [searchParams] = useURLSearchParams(); - const params = useParams(); - const { type } = params; - const id = ruleId.getRuleIdFromPathname(params); - const identifier = ruleId.tryParse(id, true); - - const copyFromId = searchParams.get('copyFrom') ?? undefined; - const copyFromIdentifier = ruleId.tryParse(copyFromId); + const { identifier, type } = useRuleEditorPathParams(); + const { copyFromIdentifier, queryDefaults } = useRuleEditorQueryParams(); const { canCreateGrafanaRules, canCreateCloudRules, canEditRules } = useRulesAccess(); @@ -65,15 +61,15 @@ const RuleEditor = () => { } if (identifier) { - return ; + return ; } if (copyFromIdentifier) { return ; } // new alert rule - return ; - }, [canCreateCloudRules, canCreateGrafanaRules, canEditRules, copyFromIdentifier, id, identifier]); + return ; + }, [canCreateCloudRules, canCreateGrafanaRules, canEditRules, copyFromIdentifier, identifier, queryDefaults]); return ( @@ -83,3 +79,28 @@ const RuleEditor = () => { }; export default withErrorBoundary(RuleEditor, { style: 'page' }); + +function useRuleEditorPathParams() { + const params = useParams(); + const { type } = params; + const id = ruleId.getRuleIdFromPathname(params); + const identifier = ruleId.tryParse(id, true); + + return { identifier, type }; +} + +function useRuleEditorQueryParams() { + const { type } = useParams(); + + const [searchParams] = useURLSearchParams(); + const copyFromId = searchParams.get('copyFrom') ?? undefined; + const copyFromIdentifier = ruleId.tryParse(copyFromId); + + const ruleType = translateRouteParamToRuleType(type); + + const queryDefaults = searchParams.has('defaults') + ? formValuesFromQueryParams(searchParams.get('defaults') ?? '', ruleType) + : undefined; + + return { copyFromIdentifier, queryDefaults }; +} diff --git a/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditorCloudOnlyAllowed.test.tsx similarity index 88% rename from public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx rename to public/app/features/alerting/unified/rule-editor/RuleEditorCloudOnlyAllowed.test.tsx index e99c56856d2..3671d13d232 100644 --- a/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditorCloudOnlyAllowed.test.tsx @@ -6,24 +6,24 @@ import { contextSrv } from 'app/core/services/context_srv'; import { AccessControlAction } from 'app/types'; import { PromApiFeatures, PromApplication } from 'app/types/unified-alerting-dto'; -import { discoverFeaturesByUid } from './api/buildInfo'; -import { fetchRulerRulesGroup } from './api/ruler'; -import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; -import { setupMswServer } from './mockApi'; -import { grantUserPermissions, mockDataSource } from './mocks'; -import { setupDataSources } from './testSetup/datasources'; -import { DataSourceType, GRAFANA_DATASOURCE_NAME, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; +import { discoverFeaturesByUid } from '../api/buildInfo'; +import { fetchRulerRulesGroup } from '../api/ruler'; +import { ExpressionEditorProps } from '../components/rule-editor/ExpressionEditor'; +import { setupMswServer } from '../mockApi'; +import { grantUserPermissions, mockDataSource } from '../mocks'; +import { setupDataSources } from '../testSetup/datasources'; +import { DataSourceType, GRAFANA_DATASOURCE_NAME, GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; -jest.mock('./components/rule-editor/ExpressionEditor', () => ({ +jest.mock('../components/rule-editor/ExpressionEditor', () => ({ // eslint-disable-next-line react/display-name ExpressionEditor: ({ value, onChange }: ExpressionEditorProps) => ( onChange(e.target.value)} /> ), })); -jest.mock('./api/buildInfo'); -jest.mock('./api/ruler', () => ({ - rulerUrlBuilder: jest.requireActual('./api/ruler').rulerUrlBuilder, +jest.mock('../api/buildInfo'); +jest.mock('../api/ruler', () => ({ + rulerUrlBuilder: jest.requireActual('../api/ruler').rulerUrlBuilder, fetchRulerRules: jest.fn(), fetchRulerRulesGroup: jest.fn(), fetchRulerRulesNamespace: jest.fn(), @@ -36,8 +36,8 @@ jest.mock('app/features/query/components/QueryEditorRow', () => ({ QueryEditorRow: () =>

hi

, })); -jest.mock('./components/rule-editor/util', () => { - const originalModule = jest.requireActual('./components/rule-editor/util'); +jest.mock('../components/rule-editor/util', () => { + const originalModule = jest.requireActual('../components/rule-editor/util'); return { ...originalModule, getThresholdsForQueries: jest.fn(() => ({})), diff --git a/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditorCloudRules.test.tsx similarity index 85% rename from public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx rename to public/app/features/alerting/unified/rule-editor/RuleEditorCloudRules.test.tsx index 21ba09cb12f..d70177fc7a9 100644 --- a/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditorCloudRules.test.tsx @@ -5,15 +5,15 @@ import { screen } from 'test/test-utils'; import { selectors } from '@grafana/e2e-selectors'; import { AccessControlAction } from 'app/types'; -import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; -import { setupMswServer } from './mockApi'; -import { grantUserPermissions } from './mocks'; -import { GROUP_3, NAMESPACE_2 } from './mocks/mimirRulerApi'; -import { mimirDataSource } from './mocks/server/configure'; -import { MIMIR_DATASOURCE_UID } from './mocks/server/constants'; -import { captureRequests, serializeRequests } from './mocks/server/events'; +import { ExpressionEditorProps } from '../components/rule-editor/ExpressionEditor'; +import { setupMswServer } from '../mockApi'; +import { grantUserPermissions } from '../mocks'; +import { GROUP_3, NAMESPACE_2 } from '../mocks/mimirRulerApi'; +import { mimirDataSource } from '../mocks/server/configure'; +import { MIMIR_DATASOURCE_UID } from '../mocks/server/constants'; +import { captureRequests, serializeRequests } from '../mocks/server/events'; -jest.mock('./components/rule-editor/ExpressionEditor', () => ({ +jest.mock('../components/rule-editor/ExpressionEditor', () => ({ // eslint-disable-next-line react/display-name ExpressionEditor: ({ value, onChange }: ExpressionEditorProps) => ( onChange(e.target.value)} /> diff --git a/public/app/features/alerting/unified/RuleEditorExisting.test.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx similarity index 95% rename from public/app/features/alerting/unified/RuleEditorExisting.test.tsx rename to public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx index 15ecae4e3ec..1781ab75d2b 100644 --- a/public/app/features/alerting/unified/RuleEditorExisting.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx @@ -7,15 +7,15 @@ import { setFolderResponse } from 'app/features/alerting/unified/mocks/server/co import { MIMIR_DATASOURCE_UID } from 'app/features/alerting/unified/mocks/server/constants'; import { captureRequests } from 'app/features/alerting/unified/mocks/server/events'; import { DashboardSearchItemType } from 'app/features/search/types'; +import { AccessControlAction } from 'app/types'; -import { AccessControlAction } from '../../../types'; +import { setupMswServer } from '../mockApi'; +import { grantUserPermissions, mockDataSource, mockFolder } from '../mocks'; +import { grafanaRulerRule } from '../mocks/grafanaRulerApi'; +import { setupDataSources } from '../testSetup/datasources'; +import { Annotation } from '../utils/constants'; import RuleEditor from './RuleEditor'; -import { setupMswServer } from './mockApi'; -import { grantUserPermissions, mockDataSource, mockFolder } from './mocks'; -import { grafanaRulerRule } from './mocks/grafanaRulerApi'; -import { setupDataSources } from './testSetup/datasources'; -import { Annotation } from './utils/constants'; jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({ AppChromeUpdate: ({ actions }: { actions: React.ReactNode }) =>
{actions}
, diff --git a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditorGrafanaRules.test.tsx similarity index 94% rename from public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx rename to public/app/features/alerting/unified/rule-editor/RuleEditorGrafanaRules.test.tsx index e07f62cd7c1..94888d4c9b1 100644 --- a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditorGrafanaRules.test.tsx @@ -9,10 +9,10 @@ import { setupMswServer } from 'app/features/alerting/unified/mockApi'; import { PROMETHEUS_DATASOURCE_UID } from 'app/features/alerting/unified/mocks/server/constants'; import { AccessControlAction } from 'app/types'; -import { grantUserPermissions, mockDataSource } from './mocks'; -import { grafanaRulerGroup } from './mocks/grafanaRulerApi'; -import { captureRequests, serializeRequests } from './mocks/server/events'; -import { setupDataSources } from './testSetup/datasources'; +import { grantUserPermissions, mockDataSource } from '../mocks'; +import { grafanaRulerGroup } from '../mocks/grafanaRulerApi'; +import { captureRequests, serializeRequests } from '../mocks/server/events'; +import { setupDataSources } from '../testSetup/datasources'; jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({ AppChromeUpdate: ({ actions }: { actions: React.ReactNode }) =>
{actions}
, diff --git a/public/app/features/alerting/unified/RuleEditorRecordingRule.test.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditorRecordingRule.test.tsx similarity index 88% rename from public/app/features/alerting/unified/RuleEditorRecordingRule.test.tsx rename to public/app/features/alerting/unified/rule-editor/RuleEditorRecordingRule.test.tsx index 766253febf1..8e979828aa9 100644 --- a/public/app/features/alerting/unified/RuleEditorRecordingRule.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditorRecordingRule.test.tsx @@ -7,14 +7,14 @@ import { byText } from 'testing-library-selector'; import { setupMswServer } from 'app/features/alerting/unified/mockApi'; import { AccessControlAction } from 'app/types'; -import { RecordingRuleEditorProps } from './components/rule-editor/RecordingRuleEditor'; -import { grantUserPermissions } from './mocks'; -import { GROUP_3, NAMESPACE_2 } from './mocks/mimirRulerApi'; -import { mimirDataSource } from './mocks/server/configure'; -import { MIMIR_DATASOURCE_UID } from './mocks/server/constants'; -import { captureRequests, serializeRequests } from './mocks/server/events'; +import { RecordingRuleEditorProps } from '../components/rule-editor/RecordingRuleEditor'; +import { grantUserPermissions } from '../mocks'; +import { GROUP_3, NAMESPACE_2 } from '../mocks/mimirRulerApi'; +import { mimirDataSource } from '../mocks/server/configure'; +import { MIMIR_DATASOURCE_UID } from '../mocks/server/constants'; +import { captureRequests, serializeRequests } from '../mocks/server/events'; -jest.mock('./components/rule-editor/RecordingRuleEditor', () => ({ +jest.mock('../components/rule-editor/RecordingRuleEditor', () => ({ RecordingRuleEditor: ({ queries, onChangeQuery }: Pick) => { const onChange = (expr: string) => { const query = queries[0]; diff --git a/public/app/features/alerting/unified/__snapshots__/RuleEditorCloudRules.test.tsx.snap b/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorCloudRules.test.tsx.snap similarity index 100% rename from public/app/features/alerting/unified/__snapshots__/RuleEditorCloudRules.test.tsx.snap rename to public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorCloudRules.test.tsx.snap diff --git a/public/app/features/alerting/unified/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap b/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap similarity index 100% rename from public/app/features/alerting/unified/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap rename to public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap diff --git a/public/app/features/alerting/unified/__snapshots__/RuleEditorRecordingRule.test.tsx.snap b/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorRecordingRule.test.tsx.snap similarity index 100% rename from public/app/features/alerting/unified/__snapshots__/RuleEditorRecordingRule.test.tsx.snap rename to public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorRecordingRule.test.tsx.snap diff --git a/public/app/features/alerting/unified/rule-editor/formDefaults.test.ts b/public/app/features/alerting/unified/rule-editor/formDefaults.test.ts new file mode 100644 index 00000000000..800b95a6193 --- /dev/null +++ b/public/app/features/alerting/unified/rule-editor/formDefaults.test.ts @@ -0,0 +1,196 @@ +import { config } from '@grafana/runtime'; + +import { mockAlertQuery, mockDataSource, reduceExpression, thresholdExpression } from '../mocks'; +import { testWithFeatureToggles } from '../test/test-utils'; +import { RuleFormType } from '../types/rule-form'; +import { Annotation } from '../utils/constants'; +import { DataSourceType, getDefaultOrFirstCompatibleDataSource } from '../utils/datasource'; +import { MANUAL_ROUTING_KEY, getDefaultQueries } from '../utils/rule-form'; + +import { formValuesFromQueryParams, getDefaultFormValues, getDefautManualRouting } from './formDefaults'; +import { isAlertQueryOfAlertData } from './formProcessing'; + +jest.mock('../utils/datasource'); + +const mocks = { + getDefaultOrFirstCompatibleDataSource: jest.mocked(getDefaultOrFirstCompatibleDataSource), +}; + +// Setup mock implementation +mocks.getDefaultOrFirstCompatibleDataSource.mockReturnValue( + mockDataSource({ + type: DataSourceType.Prometheus, + }) +); + +// TODO Not sure why queries are an empty array in the default form values +const defaultFormValues = { + ...getDefaultFormValues(), + queries: getDefaultQueries(), +}; + +describe('formValuesFromQueryParams', () => { + it('should return default values when given invalid JSON', () => { + const result = formValuesFromQueryParams('invalid json', RuleFormType.grafana); + + expect(result).toEqual(defaultFormValues); + }); + + it('should normalize annotations', () => { + const ruleDefinition = JSON.stringify({ + annotations: [ + { key: 'custom', value: 'my custom annotation' }, + { key: Annotation.runbookURL, value: 'runbook annotation' }, + { key: 'custom-2', value: 'custom annotation v2' }, + { key: Annotation.summary, value: 'summary annotation' }, + { key: 'custom-3', value: 'custom annotation v3' }, + { key: Annotation.description, value: 'description annotation' }, + ], + }); + + const result = formValuesFromQueryParams(ruleDefinition, RuleFormType.grafana); + + const [summary, description, runbookURL, ...rest] = result.annotations; + + expect(summary).toEqual({ key: Annotation.summary, value: 'summary annotation' }); + expect(description).toEqual({ key: Annotation.description, value: 'description annotation' }); + expect(runbookURL).toEqual({ key: Annotation.runbookURL, value: 'runbook annotation' }); + expect(rest).toContainEqual({ key: 'custom', value: 'my custom annotation' }); + expect(rest).toContainEqual({ key: 'custom-2', value: 'custom annotation v2' }); + expect(rest).toContainEqual({ key: 'custom-3', value: 'custom annotation v3' }); + }); + + it('should disable simplified query editor when query switch mode is disabled', () => { + const result = formValuesFromQueryParams(JSON.stringify({}), RuleFormType.grafana); + + expect(result.editorSettings).toBeDefined(); + expect(result.editorSettings!.simplifiedQueryEditor).toBe(false); + }); + + describe('when simplified query editor is enabled', () => { + testWithFeatureToggles(['alertingQueryAndExpressionsStepMode']); + + it('should enable simplified query editor if queries are transformable to simple condition', () => { + const result = formValuesFromQueryParams( + JSON.stringify({ + queries: [mockAlertQuery(), reduceExpression, thresholdExpression], + }), + RuleFormType.grafana + ); + + expect(result.editorSettings).toBeDefined(); + expect(result.editorSettings!.simplifiedQueryEditor).toBe(true); + }); + + it('should disable simplified query editor if queries are not transformable to simple condition', () => { + const result = formValuesFromQueryParams( + JSON.stringify({ + queries: [mockAlertQuery(), mockAlertQuery(), thresholdExpression], + }), + RuleFormType.grafana + ); + + expect(result.editorSettings).toBeDefined(); + expect(result.editorSettings!.simplifiedQueryEditor).toBe(false); + }); + }); + + it('should default to instant queries for loki and prometheus if not specified', () => { + const result = formValuesFromQueryParams( + JSON.stringify({ + queries: [ + mockAlertQuery({ datasourceUid: 'loki', model: { refId: 'A', datasource: { type: DataSourceType.Loki } } }), + mockAlertQuery({ + datasourceUid: 'prometheus', + model: { refId: 'B', datasource: { type: DataSourceType.Prometheus } }, + }), + ], + }), + RuleFormType.grafana + ); + + const [lokiQuery, prometheusQuery] = result.queries.filter(isAlertQueryOfAlertData); + + expect(lokiQuery.model.instant).toBe(true); + expect(lokiQuery.model.range).toBe(false); + expect(prometheusQuery.model.instant).toBe(true); + expect(prometheusQuery.model.range).toBe(false); + }); + + it('should preserver instant and range values if specified', () => { + const result = formValuesFromQueryParams( + JSON.stringify({ + queries: [ + mockAlertQuery({ + datasourceUid: 'loki', + model: { refId: 'A', datasource: { type: DataSourceType.Loki }, instant: true, range: false }, + }), + mockAlertQuery({ + datasourceUid: 'prometheus', + model: { refId: 'B', datasource: { type: DataSourceType.Prometheus }, instant: false, range: true }, + }), + ], + }), + RuleFormType.grafana + ); + + const [lokiQuery, prometheusQuery] = result.queries.filter(isAlertQueryOfAlertData); + + expect(lokiQuery.model.instant).toBe(true); + expect(lokiQuery.model.range).toBe(false); + expect(prometheusQuery.model.range).toBe(true); + expect(prometheusQuery.model.instant).toBe(false); + }); + + it('should reveal hidden queries', () => { + const ruleDefinition = JSON.stringify({ + queries: [ + { refId: 'A', model: { refId: 'A', hide: true } }, + { refId: 'B', model: { refId: 'B', hide: false } }, + { refId: 'C', model: { refId: 'C' } }, + ], + }); + + const result = formValuesFromQueryParams(ruleDefinition, RuleFormType.grafana); + + expect(result.queries.length).toBe(3); + + const [q1, q2, q3] = result.queries; + expect(q1.refId).toBe('A'); + expect(q2.refId).toBe('B'); + expect(q3.refId).toBe('C'); + expect(q1.model).not.toHaveProperty('hide'); + expect(q2.model).not.toHaveProperty('hide'); + expect(q3.model).not.toHaveProperty('hide'); + }); +}); + +describe('getDefaultManualRouting', () => { + afterEach(() => { + window.localStorage.clear(); + }); + + it('returns false if the feature toggle is not enabled', () => { + config.featureToggles.alertingSimplifiedRouting = false; + expect(getDefautManualRouting()).toBe(false); + }); + + it('returns true if the feature toggle is enabled and localStorage is not set', () => { + config.featureToggles.alertingSimplifiedRouting = true; + expect(getDefautManualRouting()).toBe(true); + }); + + it('returns false if the feature toggle is enabled and localStorage is set to "false"', () => { + config.featureToggles.alertingSimplifiedRouting = true; + localStorage.setItem(MANUAL_ROUTING_KEY, 'false'); + expect(getDefautManualRouting()).toBe(false); + }); + + it('returns true if the feature toggle is enabled and localStorage is set to any value other than "false"', () => { + config.featureToggles.alertingSimplifiedRouting = true; + localStorage.setItem(MANUAL_ROUTING_KEY, 'true'); + expect(getDefautManualRouting()).toBe(true); + localStorage.removeItem(MANUAL_ROUTING_KEY); + expect(getDefautManualRouting()).toBe(true); + }); +}); diff --git a/public/app/features/alerting/unified/rule-editor/formDefaults.ts b/public/app/features/alerting/unified/rule-editor/formDefaults.ts new file mode 100644 index 00000000000..7c6a0c824c0 --- /dev/null +++ b/public/app/features/alerting/unified/rule-editor/formDefaults.ts @@ -0,0 +1,157 @@ +import { clamp } from 'lodash'; + +import { config } from '@grafana/runtime'; +import { RuleWithLocation } from 'app/types/unified-alerting'; +import { GrafanaAlertStateDecision, RulerRuleDTO } from 'app/types/unified-alerting-dto'; + +import { RuleFormType, RuleFormValues } from '../types/rule-form'; +// TODO Ideally all of these should be moved here +import { getRulesAccess } from '../utils/access-control'; +import { defaultAnnotations } from '../utils/constants'; +import { GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; +import { + MANUAL_ROUTING_KEY, + SIMPLIFIED_QUERY_EDITOR_KEY, + getDefaultQueries, + rulerRuleToFormValues, +} from '../utils/rule-form'; +import { isGrafanaRecordingRuleByType } from '../utils/rules'; +import { formatPrometheusDuration, safeParsePrometheusDuration } from '../utils/time'; + +import { + normalizeDefaultAnnotations, + revealHiddenQueries, + setInstantOrRange, + setQueryEditorSettings, +} from './formProcessing'; + +// even if the min interval is < 1m we should default to 1m, but allow arbitrary values for minInterval > 1m +const GROUP_EVALUATION_MIN_INTERVAL_MS = safeParsePrometheusDuration(config.unifiedAlerting?.minInterval ?? '10s'); +const GROUP_EVALUATION_INTERVAL_LOWER_BOUND = safeParsePrometheusDuration('1m'); +const GROUP_EVALUATION_INTERVAL_UPPER_BOUND = Infinity; + +export const DEFAULT_GROUP_EVALUATION_INTERVAL = formatPrometheusDuration( + clamp(GROUP_EVALUATION_MIN_INTERVAL_MS, GROUP_EVALUATION_INTERVAL_LOWER_BOUND, GROUP_EVALUATION_INTERVAL_UPPER_BOUND) +); +export const getDefaultFormValues = (): RuleFormValues => { + const { canCreateGrafanaRules, canCreateCloudRules } = getRulesAccess(); + + return Object.freeze({ + name: '', + uid: '', + labels: [{ key: '', value: '' }], + annotations: defaultAnnotations, + dataSourceName: GRAFANA_RULES_SOURCE_NAME, // let's use Grafana-managed alert rule by default + type: canCreateGrafanaRules ? RuleFormType.grafana : canCreateCloudRules ? RuleFormType.cloudAlerting : undefined, // viewers can't create prom alerts + group: '', + + // grafana + folder: undefined, + queries: [], + recordingRulesQueries: [], + condition: '', + noDataState: GrafanaAlertStateDecision.NoData, + execErrState: GrafanaAlertStateDecision.Error, + evaluateFor: DEFAULT_GROUP_EVALUATION_INTERVAL, + evaluateEvery: DEFAULT_GROUP_EVALUATION_INTERVAL, + manualRouting: getDefautManualRouting(), // we default to true if the feature toggle is enabled and the user hasn't set local storage to false + contactPoints: {}, + overrideGrouping: false, + overrideTimings: false, + muteTimeIntervals: [], + editorSettings: getDefaultEditorSettings(), + + // cortex / loki + namespace: '', + expression: '', + forTime: 1, + forTimeUnit: 'm', + }); +}; + +export const getDefautManualRouting = () => { + // first check if feature toggle for simplified routing is enabled + const simplifiedRoutingToggleEnabled = config.featureToggles.alertingSimplifiedRouting ?? false; + if (!simplifiedRoutingToggleEnabled) { + return false; + } + //then, check in local storage if the user has enabled simplified routing + // if it's not set, we'll default to true + const manualRouting = localStorage.getItem(MANUAL_ROUTING_KEY); + return manualRouting !== 'false'; +}; + +function getDefaultEditorSettings() { + const editorSettingsEnabled = config.featureToggles.alertingQueryAndExpressionsStepMode ?? false; + if (!editorSettingsEnabled) { + return undefined; + } + //then, check in local storage if the user has saved last rule with sections simplified + const queryEditorSettings = localStorage.getItem(SIMPLIFIED_QUERY_EDITOR_KEY); + const notificationStepSettings = localStorage.getItem(MANUAL_ROUTING_KEY); + return { + simplifiedQueryEditor: queryEditorSettings !== 'false', + simplifiedNotificationEditor: notificationStepSettings !== 'false', + }; +} + +export function formValuesFromQueryParams(ruleDefinition: string, type: RuleFormType): RuleFormValues { + let ruleFromQueryParams: Partial; + + try { + ruleFromQueryParams = JSON.parse(ruleDefinition); + } catch (err) { + return { + ...getDefaultFormValues(), + queries: getDefaultQueries(), + }; + } + + return setQueryEditorSettings( + setInstantOrRange( + revealHiddenQueries({ + ...getDefaultFormValues(), + ...ruleFromQueryParams, + annotations: normalizeDefaultAnnotations(ruleFromQueryParams.annotations ?? []), + queries: ruleFromQueryParams.queries ?? getDefaultQueries(), + type: type || RuleFormType.grafana, + evaluateEvery: DEFAULT_GROUP_EVALUATION_INTERVAL, + }) + ) + ); +} + +export function formValuesFromPrefill(rule: Partial): RuleFormValues { + return revealHiddenQueries({ + ...getDefaultFormValues(), + ...rule, + }); +} + +export function formValuesFromExistingRule(rule: RuleWithLocation) { + return revealHiddenQueries(rulerRuleToFormValues(rule)); +} + +export function defaultFormValuesForRuleType(ruleType: RuleFormType): RuleFormValues { + return { + ...getDefaultFormValues(), + condition: 'C', + queries: getDefaultQueries(isGrafanaRecordingRuleByType(ruleType)), + type: ruleType, + evaluateEvery: DEFAULT_GROUP_EVALUATION_INTERVAL, + }; +} + +// TODO This function is not 100% valid. There is no support for cloud form type because +// it's not valid from the path param point of view. +export function translateRouteParamToRuleType(param = ''): RuleFormType { + if (param === 'recording') { + return RuleFormType.cloudRecording; + } + + if (param === 'grafana-recording') { + return RuleFormType.grafanaRecording; + } + + return RuleFormType.grafana; +} diff --git a/public/app/features/alerting/unified/rule-editor/formProcessing.ts b/public/app/features/alerting/unified/rule-editor/formProcessing.ts new file mode 100644 index 00000000000..c9fa1951417 --- /dev/null +++ b/public/app/features/alerting/unified/rule-editor/formProcessing.ts @@ -0,0 +1,150 @@ +import { omit } from 'lodash'; + +import { config } from '@grafana/runtime'; +import { isExpressionQuery } from 'app/features/expressions/guards'; +import { ExpressionQuery, ExpressionQueryType, ReducerMode } from 'app/features/expressions/types'; +import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto'; + +import { SimpleConditionIdentifier } from '../components/rule-editor/query-and-alert-condition/SimpleCondition'; +import { KVObject, RuleFormValues } from '../types/rule-form'; +import { defaultAnnotations } from '../utils/constants'; +import { DataSourceType } from '../utils/datasource'; + +export function setQueryEditorSettings(values: RuleFormValues): RuleFormValues { + const isQuerySwitchModeEnabled = config.featureToggles.alertingQueryAndExpressionsStepMode ?? false; + + if (!isQuerySwitchModeEnabled) { + return { + ...values, + editorSettings: { + simplifiedQueryEditor: false, + simplifiedNotificationEditor: true, // actually it doesn't matter in this case + }, + }; + } + + // data queries only + const dataQueries = values.queries.filter((query) => !isExpressionQuery(query.model)); + + // expression queries only + const expressionQueries = values.queries.filter((query) => isExpressionQueryInAlert(query)); + + const queryParamsAreTransformable = areQueriesTransformableToSimpleCondition(dataQueries, expressionQueries); + return { + ...values, + editorSettings: { + simplifiedQueryEditor: queryParamsAreTransformable, + simplifiedNotificationEditor: true, + }, + }; +} + +export function setInstantOrRange(values: RuleFormValues): RuleFormValues { + return { + ...values, + queries: values.queries?.map((query) => { + if (isExpressionQuery(query.model)) { + return query; + } + // data query + const defaultToInstant = + query.model.datasource?.type === DataSourceType.Loki || + query.model.datasource?.type === DataSourceType.Prometheus; + const isInstant = + 'instant' in query.model && query.model.instant !== undefined ? query.model.instant : defaultToInstant; + return { + ...query, + model: { + ...query.model, + instant: isInstant, + range: !isInstant, // we cannot have both instant and range queries in alerting + }, + }; + }), + }; +} + +export function areQueriesTransformableToSimpleCondition( + dataQueries: Array>, + expressionQueries: Array> +) { + if (dataQueries.length !== 1) { + return false; + } + const singleReduceExpressionInInstantQuery = + 'instant' in dataQueries[0].model && dataQueries[0].model.instant && expressionQueries.length === 1; + + if (expressionQueries.length !== 2 && !singleReduceExpressionInInstantQuery) { + return false; + } + + const query = dataQueries[0]; + + if (query.refId !== SimpleConditionIdentifier.queryId) { + return false; + } + + const reduceExpressionIndex = expressionQueries.findIndex( + (query) => query.model.type === ExpressionQueryType.reduce && query.refId === SimpleConditionIdentifier.reducerId + ); + const reduceExpression = expressionQueries.at(reduceExpressionIndex); + const reduceOk = + reduceExpression && + reduceExpressionIndex === 0 && + (reduceExpression.model.settings?.mode === ReducerMode.Strict || + reduceExpression.model.settings?.mode === undefined); + + const thresholdExpressionIndex = expressionQueries.findIndex( + (query) => + query.model.type === ExpressionQueryType.threshold && query.refId === SimpleConditionIdentifier.thresholdId + ); + const thresholdExpression = expressionQueries.at(thresholdExpressionIndex); + const conditions = thresholdExpression?.model.conditions ?? []; + const thresholdIndexOk = singleReduceExpressionInInstantQuery + ? thresholdExpressionIndex === 0 + : thresholdExpressionIndex === 1; + const thresholdOk = thresholdExpression && thresholdIndexOk && conditions[0]?.unloadEvaluator === undefined; + return (Boolean(reduceOk) || Boolean(singleReduceExpressionInInstantQuery)) && Boolean(thresholdOk); +} + +export function isExpressionQueryInAlert( + query: AlertQuery +): query is AlertQuery { + return isExpressionQuery(query.model); +} + +export function isAlertQueryOfAlertData( + query: AlertQuery +): query is AlertQuery { + return !isExpressionQuery(query.model); +} + +// the backend will always execute "hidden" queries, so we have no choice but to remove the property in the front-end +// to avoid confusion. The query editor shows them as "disabled" and that's a different semantic meaning. +// furthermore the "AlertingQueryRunner" calls `filterQuery` on each data source and those will skip running queries that are "hidden"." +// It seems like we have no choice but to act like "hidden" queries don't exist in alerting. +export const revealHiddenQueries = (ruleDefinition: RuleFormValues): RuleFormValues => { + return { + ...ruleDefinition, + queries: ruleDefinition.queries?.map((query) => omit(query, 'model.hide')), + }; +}; + +export function normalizeDefaultAnnotations(annotations: KVObject[]) { + const orderedAnnotations = [...annotations]; + const defaultAnnotationKeys = defaultAnnotations.map((annotation) => annotation.key); + + defaultAnnotationKeys.forEach((defaultAnnotationKey, index) => { + const fieldIndex = orderedAnnotations.findIndex((field) => field.key === defaultAnnotationKey); + + if (fieldIndex === -1) { + //add the default annotation if abstent + const emptyValue = { key: defaultAnnotationKey, value: '' }; + orderedAnnotations.splice(index, 0, emptyValue); + } else if (fieldIndex !== index) { + //move it to the correct position if present + orderedAnnotations.splice(index, 0, orderedAnnotations.splice(fieldIndex, 1)[0]); + } + }); + return orderedAnnotations; +} diff --git a/public/app/features/alerting/unified/rule-list/FilterView.test.tsx b/public/app/features/alerting/unified/rule-list/FilterView.test.tsx index b436e84941a..de51e6dde8b 100644 --- a/public/app/features/alerting/unified/rule-list/FilterView.test.tsx +++ b/public/app/features/alerting/unified/rule-list/FilterView.test.tsx @@ -19,9 +19,9 @@ grantUserPermissions([AccessControlAction.AlertingRuleExternalRead]); setupMswServer(); -const mimirGroups = alertingFactory.group.buildList(5000, { file: 'test-mimir-namespace' }); -alertingFactory.group.rewindSequence(); -const prometheusGroups = alertingFactory.group.buildList(200, { file: 'test-prometheus-namespace' }); +const mimirGroups = alertingFactory.prometheus.group.buildList(5000, { file: 'test-mimir-namespace' }); +alertingFactory.prometheus.group.rewindSequence(); +const prometheusGroups = alertingFactory.prometheus.group.buildList(200, { file: 'test-prometheus-namespace' }); const mimirDs = alertingFactory.dataSource.build({ name: 'Mimir', uid: 'mimir' }); const prometheusDs = alertingFactory.dataSource.build({ name: 'Prometheus', uid: 'prometheus' }); diff --git a/public/app/features/alerting/unified/rule-list/GroupedView.test.tsx b/public/app/features/alerting/unified/rule-list/GroupedView.test.tsx index adb8b83c077..953bce50ead 100644 --- a/public/app/features/alerting/unified/rule-list/GroupedView.test.tsx +++ b/public/app/features/alerting/unified/rule-list/GroupedView.test.tsx @@ -19,9 +19,9 @@ grantUserPermissions([AccessControlAction.AlertingRuleExternalRead]); setupMswServer(); -const mimirGroups = alertingFactory.group.buildList(500, { file: 'test-mimir-namespace' }); -alertingFactory.group.rewindSequence(); -const prometheusGroups = alertingFactory.group.buildList(130, { file: 'test-prometheus-namespace' }); +const mimirGroups = alertingFactory.prometheus.group.buildList(500, { file: 'test-mimir-namespace' }); +alertingFactory.prometheus.group.rewindSequence(); +const prometheusGroups = alertingFactory.prometheus.group.buildList(130, { file: 'test-prometheus-namespace' }); const mimirDs = alertingFactory.dataSource.build({ name: 'Mimir', uid: 'mimir' }); const prometheusDs = alertingFactory.dataSource.build({ name: 'Prometheus', uid: 'prometheus' }); diff --git a/public/app/features/alerting/unified/test/test-utils.ts b/public/app/features/alerting/unified/test/test-utils.ts index 88de7e27984..66598dea083 100644 --- a/public/app/features/alerting/unified/test/test-utils.ts +++ b/public/app/features/alerting/unified/test/test-utils.ts @@ -2,6 +2,10 @@ import { act } from '@testing-library/react'; import { FeatureToggles } from '@grafana/data'; import { config } from '@grafana/runtime'; +import { AccessControlAction } from 'app/types'; + +import { grantUserPermissions } from '../mocks'; +import { setFolderAccessControl } from '../mocks/server/configure'; /** * Flushes out microtasks so we don't get warnings from `@floating-ui/react` @@ -47,3 +51,13 @@ export const testWithLicenseFeatures = (features: string[]) => { config.licenseInfo.enabledFeatures = originalFeatures; }); }; + +/** + * "Grants" permissions via contextSrv mock, and additionally sets folder access control + * API response to match + */ +export const grantPermissionsHelper = (permissions: AccessControlAction[]) => { + const permissionsHash = permissions.reduce((hash, permission) => ({ ...hash, [permission]: true }), {}); + grantUserPermissions(permissions); + setFolderAccessControl(permissionsHash); +}; diff --git a/public/app/features/alerting/unified/utils/rule-form.test.ts b/public/app/features/alerting/unified/utils/rule-form.test.ts index bfc7f86ea5c..80442e87984 100644 --- a/public/app/features/alerting/unified/utils/rule-form.test.ts +++ b/public/app/features/alerting/unified/utils/rule-form.test.ts @@ -1,20 +1,17 @@ import { PromQuery } from '@grafana/prometheus'; -import { config } from '@grafana/runtime'; import { GrafanaAlertStateDecision, GrafanaRuleDefinition, RulerAlertingRuleDTO } from 'app/types/unified-alerting-dto'; +import { getDefaultFormValues } from '../rule-editor/formDefaults'; import { AlertManagerManualRouting, RuleFormType, RuleFormValues } from '../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME } from './datasource'; import { - MANUAL_ROUTING_KEY, alertingRulerRuleToRuleForm, cleanAnnotations, cleanLabels, formValuesToRulerGrafanaRuleDTO, formValuesToRulerRuleDTO, getContactPointsFromDTO, - getDefaultFormValues, - getDefautManualRouting, getNotificationSettingsForDTO, } from './rule-form'; @@ -227,36 +224,6 @@ describe('getNotificationSettingsForDTO', () => { }); }); -describe('getDefautManualRouting', () => { - afterEach(() => { - window.localStorage.clear(); - }); - - it('returns false if the feature toggle is not enabled', () => { - config.featureToggles.alertingSimplifiedRouting = false; - expect(getDefautManualRouting()).toBe(false); - }); - - it('returns true if the feature toggle is enabled and localStorage is not set', () => { - config.featureToggles.alertingSimplifiedRouting = true; - expect(getDefautManualRouting()).toBe(true); - }); - - it('returns false if the feature toggle is enabled and localStorage is set to "false"', () => { - config.featureToggles.alertingSimplifiedRouting = true; - localStorage.setItem(MANUAL_ROUTING_KEY, 'false'); - expect(getDefautManualRouting()).toBe(false); - }); - - it('returns true if the feature toggle is enabled and localStorage is set to any value other than "false"', () => { - config.featureToggles.alertingSimplifiedRouting = true; - localStorage.setItem(MANUAL_ROUTING_KEY, 'true'); - expect(getDefautManualRouting()).toBe(true); - localStorage.removeItem(MANUAL_ROUTING_KEY); - expect(getDefautManualRouting()).toBe(true); - }); -}); - describe('cleanAnnotations', () => { it('should remove falsy KVs', () => { const output = cleanAnnotations([{ key: '', value: '' }]); diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index de5ee071296..ecea384f9e7 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -1,5 +1,3 @@ -import { clamp, omit } from 'lodash'; - import { DataQuery, DataSourceInstanceSettings, @@ -31,7 +29,6 @@ import { AlertDataQuery, AlertQuery, Annotations, - GrafanaAlertStateDecision, GrafanaNotificationSettings, GrafanaRuleDefinition, Labels, @@ -42,6 +39,8 @@ import { } from 'app/types/unified-alerting-dto'; import { EvalFunction } from '../../state/alertDef'; +import { getDefaultFormValues } from '../rule-editor/formDefaults'; +import { normalizeDefaultAnnotations } from '../rule-editor/formProcessing'; import { AlertManagerManualRouting, ContactPoint, @@ -51,8 +50,7 @@ import { SimplifiedEditor, } from '../types/rule-form'; -import { getRulesAccess } from './access-control'; -import { Annotation, defaultAnnotations } from './constants'; +import { Annotation } from './constants'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME, @@ -68,84 +66,13 @@ import { isGrafanaRulerRule, isRecordingRulerRule, } from './rules'; -import { formatPrometheusDuration, parseInterval, safeParsePrometheusDuration } from './time'; +import { parseInterval } from './time'; export type PromOrLokiQuery = PromQuery | LokiQuery; export const MANUAL_ROUTING_KEY = 'grafana.alerting.manualRouting'; export const SIMPLIFIED_QUERY_EDITOR_KEY = 'grafana.alerting.simplifiedQueryEditor'; -// even if the min interval is < 1m we should default to 1m, but allow arbitrary values for minInterval > 1m -const GROUP_EVALUATION_MIN_INTERVAL_MS = safeParsePrometheusDuration(config.unifiedAlerting?.minInterval ?? '10s'); -const GROUP_EVALUATION_INTERVAL_LOWER_BOUND = safeParsePrometheusDuration('1m'); -const GROUP_EVALUATION_INTERVAL_UPPER_BOUND = Infinity; - -export const DEFAULT_GROUP_EVALUATION_INTERVAL = formatPrometheusDuration( - clamp(GROUP_EVALUATION_MIN_INTERVAL_MS, GROUP_EVALUATION_INTERVAL_LOWER_BOUND, GROUP_EVALUATION_INTERVAL_UPPER_BOUND) -); - -export const getDefaultFormValues = (): RuleFormValues => { - const { canCreateGrafanaRules, canCreateCloudRules } = getRulesAccess(); - - return Object.freeze({ - name: '', - uid: '', - labels: [{ key: '', value: '' }], - annotations: defaultAnnotations, - dataSourceName: GRAFANA_RULES_SOURCE_NAME, // let's use Grafana-managed alert rule by default - type: canCreateGrafanaRules ? RuleFormType.grafana : canCreateCloudRules ? RuleFormType.cloudAlerting : undefined, // viewers can't create prom alerts - group: '', - - // grafana - folder: undefined, - queries: [], - recordingRulesQueries: [], - condition: '', - noDataState: GrafanaAlertStateDecision.NoData, - execErrState: GrafanaAlertStateDecision.Error, - evaluateFor: DEFAULT_GROUP_EVALUATION_INTERVAL, - evaluateEvery: DEFAULT_GROUP_EVALUATION_INTERVAL, - manualRouting: getDefautManualRouting(), // we default to true if the feature toggle is enabled and the user hasn't set local storage to false - contactPoints: {}, - overrideGrouping: false, - overrideTimings: false, - muteTimeIntervals: [], - editorSettings: getDefaultEditorSettings(), - - // cortex / loki - namespace: '', - expression: '', - forTime: 1, - forTimeUnit: 'm', - }); -}; - -export const getDefautManualRouting = () => { - // first check if feature toggle for simplified routing is enabled - const simplifiedRoutingToggleEnabled = config.featureToggles.alertingSimplifiedRouting ?? false; - if (!simplifiedRoutingToggleEnabled) { - return false; - } - //then, check in local storage if the user has enabled simplified routing - // if it's not set, we'll default to true - const manualRouting = localStorage.getItem(MANUAL_ROUTING_KEY); - return manualRouting !== 'false'; -}; - -function getDefaultEditorSettings() { - const editorSettingsEnabled = config.featureToggles.alertingQueryAndExpressionsStepMode ?? false; - if (!editorSettingsEnabled) { - return undefined; - } - //then, check in local storage if the user has saved last rule with sections simplified - const queryEditorSettings = localStorage.getItem(SIMPLIFIED_QUERY_EDITOR_KEY); - const notificationStepSettings = localStorage.getItem(MANUAL_ROUTING_KEY); - return { - simplifiedQueryEditor: queryEditorSettings !== 'false', - simplifiedNotificationEditor: notificationStepSettings !== 'false', - }; -} - export function formValuesToRulerRuleDTO(values: RuleFormValues): RulerRuleDTO { const { name, expression, forTime, forTimeUnit, keepFiringForTime, keepFiringForTimeUnit, type } = values; @@ -184,26 +111,6 @@ export function listifyLabelsOrAnnotations(item: Labels | Annotations | undefine return list; } -//make sure default annotations are always shown in order even if empty -export function normalizeDefaultAnnotations(annotations: KVObject[]) { - const orderedAnnotations = [...annotations]; - const defaultAnnotationKeys = defaultAnnotations.map((annotation) => annotation.key); - - defaultAnnotationKeys.forEach((defaultAnnotationKey, index) => { - const fieldIndex = orderedAnnotations.findIndex((field) => field.key === defaultAnnotationKey); - - if (fieldIndex === -1) { - //add the default annotation if abstent - const emptyValue = { key: defaultAnnotationKey, value: '' }; - orderedAnnotations.splice(index, 0, emptyValue); - } else if (fieldIndex !== index) { - //move it to the correct position if present - orderedAnnotations.splice(index, 0, orderedAnnotations.splice(fieldIndex, 1)[0]); - } - }); - return orderedAnnotations; -} - export function getNotificationSettingsForDTO( manualRouting: boolean, contactPoints?: AlertManagerManualRouting @@ -902,21 +809,6 @@ export function isPromOrLokiQuery(model: AlertDataQuery): model is PromOrLokiQue return 'expr' in model; } -// the backend will always execute "hidden" queries, so we have no choice but to remove the property in the front-end -// to avoid confusion. The query editor shows them as "disabled" and that's a different semantic meaning. -// furthermore the "AlertingQueryRunner" calls `filterQuery` on each data source and those will skip running queries that are "hidden"." -// It seems like we have no choice but to act like "hidden" queries don't exist in alerting. -export const ignoreHiddenQueries = (ruleDefinition: RuleFormValues): RuleFormValues => { - return { - ...ruleDefinition, - queries: ruleDefinition.queries?.map((query) => omit(query, 'model.hide')), - }; -}; - -export function formValuesFromExistingRule(rule: RuleWithLocation) { - return ignoreHiddenQueries(rulerRuleToFormValues(rule)); -} - export function getInstantFromDataQuery(model: AlertDataQuery, type: string): boolean | undefined { // if the datasource is not prometheus or loki, instant is defined in the model or defaults to undefined if (type !== DataSourceType.Prometheus && type !== DataSourceType.Loki) { diff --git a/public/test/helpers/alertingRuleEditor.tsx b/public/test/helpers/alertingRuleEditor.tsx index 1f1dc3552f2..4b50dce768e 100644 --- a/public/test/helpers/alertingRuleEditor.tsx +++ b/public/test/helpers/alertingRuleEditor.tsx @@ -4,7 +4,8 @@ import { byRole, byTestId, byText } from 'testing-library-selector'; import { selectors } from '@grafana/e2e-selectors'; import { AppNotificationList } from 'app/core/components/AppNotifications/AppNotificationList'; -import RuleEditor from 'app/features/alerting/unified/RuleEditor'; +import RuleEditor from 'app/features/alerting/unified/rule-editor/RuleEditor'; + export enum GrafanaRuleFormStep { Query = 2, Notification = 5,