From 87884f4d416299c04c038e60ce68fd0401ea2045 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Tue, 20 Jun 2023 11:28:24 +0200 Subject: [PATCH] Alerting: Allow selecting the same custom group when swapping folders (#70337) --- .../alerting/unified/CloneRuleEditor.test.tsx | 5 +- .../components/rule-editor/AlertRuleForm.tsx | 1 - .../components/rule-editor/FolderAndGroup.tsx | 142 ++++++++---------- .../rule-editor/GrafanaEvaluationBehavior.tsx | 13 +- 4 files changed, 64 insertions(+), 97 deletions(-) diff --git a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx index da84f5cb16b..8179d22313a 100644 --- a/public/app/features/alerting/unified/CloneRuleEditor.test.tsx +++ b/public/app/features/alerting/unified/CloneRuleEditor.test.tsx @@ -1,4 +1,4 @@ -import { render, waitFor, waitForElementToBeRemoved } from '@testing-library/react'; +import { render, waitFor, waitForElementToBeRemoved, within } from '@testing-library/react'; import { setupServer } from 'msw/node'; import React from 'react'; import { FormProvider, useForm } from 'react-hook-form'; @@ -64,7 +64,6 @@ const ui = { labelValue: (idx: number) => byTestId(`label-value-${idx}`), }, loadingIndicator: byText('Loading the rule'), - loadingGroupIndicator: byText('Loading...'), }; function getProvidersWrapper() { @@ -149,7 +148,7 @@ describe('CloneRuleEditor', function () { }); await waitForElementToBeRemoved(ui.loadingIndicator.query()); - await waitForElementToBeRemoved(ui.loadingGroupIndicator.query(), { container: ui.inputs.group.get() }); + await waitForElementToBeRemoved(within(ui.inputs.group.get()).getByTestId('Spinner')); await waitFor(() => { expect(ui.inputs.name.get()).toHaveValue('First Grafana Rule (copy)'); diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx index 737ba5db488..5785be51c95 100644 --- a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx @@ -244,7 +244,6 @@ export const AlertRuleForm = ({ existing, prefill, id }: Props) => { <> {type === RuleFormType.grafana ? ( { + const dispatch = useDispatch(); + + // fetch the ruler rules from the database so we can figure out what other "groups" are already defined + // for our folders + useEffect(() => { + dispatch(fetchRulerRulesAction({ rulesSourceName: GRAFANA_RULES_SOURCE_NAME })); + }, [dispatch]); + const rulerRuleRequests = useUnifiedAlertingSelector((state) => state.rulerRules); const groupfoldersForGrafana = rulerRuleRequests[GRAFANA_RULES_SOURCE_NAME]; @@ -58,56 +62,33 @@ const sortByLabel = (a: SelectableValue, b: SelectableValue) => return a.label?.localeCompare(b.label ?? '') || 0; }; -export function FolderAndGroup({ initialFolder }: FolderAndGroupProps) { +const findGroupMatchingLabel = (group: SelectableValue, query: string) => { + return group.label?.toLowerCase().includes(query.toLowerCase()); +}; + +export function FolderAndGroup() { const { formState: { errors }, watch, + setValue, control, } = useFormContext(); const styles = useStyles2(getStyles); - const dispatch = useDispatch(); const folder = watch('folder'); const group = watch('group'); - const [selectedGroup, setSelectedGroup] = useState>({ label: group, title: group }); - const initialRender = useRef(true); const { groupOptions, loading } = useGetGroupOptionsFromFolder(folder?.title ?? ''); - useEffect(() => setSelectedGroup({ label: group, title: group }), [group, setSelectedGroup]); - - useEffect(() => { - dispatch(fetchRulerRulesIfNotFetchedYet(GRAFANA_RULES_SOURCE_NAME)); - }, [dispatch]); - const resetGroup = useCallback(() => { - if (group && !initialRender.current && folder?.title) { - setSelectedGroup({ label: '', title: '' }); - } - initialRender.current = false; - }, [group, folder?.title]); - - const groupIsInGroupOptions = useCallback( - (group_: string) => { - return groupOptions.includes((groupInList: SelectableValue) => groupInList.label === group_); - }, - [groupOptions] - ); - const sliceResults = (list: Array>) => list.slice(0, SLICE_GROUP_RESULTS_TO); + setValue('group', ''); + }, [setValue]); const getOptions = useCallback( async (query: string) => { - const results = query - ? sliceResults( - groupOptions.filter((el) => { - const label = el.label ?? ''; - return label.toLowerCase().includes(query.toLowerCase()); - }) - ) - : sliceResults(groupOptions); - - return results; + const results = query ? groupOptions.filter((group) => findGroupMatchingLabel(group, query)) : groupOptions; + return take(results, MAX_GROUP_RESULTS); }, [groupOptions] ); @@ -116,6 +97,8 @@ export function FolderAndGroup({ initialFolder }: FolderAndGroupProps) { return debounce(getOptions, 300, { leading: true }); }, [getOptions]); + const defaultGroupValue = group ? { value: group, label: group } : undefined; + return (
{ field.onChange({ title, uid }); - if (!groupIsInGroupOptions(selectedGroup.value ?? '')) { - resetGroup(); - } + resetGroup(); }} /> )} @@ -170,43 +151,40 @@ export function FolderAndGroup({ initialFolder }: FolderAndGroupProps) { invalid={!!errors.group?.message} > - loading ? ( - - ) : ( - ) => ( -
- {option.label} - {/* making the assumption here that it's provisioned when it's disabled, should probably change this */} - {option.isDisabled && ( - <> - {' '} - - - )} -
- )} - placeholder={'Evaluation group name'} - onChange={(value) => { - field.onChange(value.label ?? ''); - }} - value={selectedGroup} - allowCustomValue - formatCreateLabel={(_) => '+ Add new '} - noOptionsMessage="Start typing to create evaluation group" - /> - ) - } + render={({ field: { ref, ...field }, fieldState }) => ( + { + field.onChange(group.label ?? ''); + }} + isLoading={loading} + invalid={Boolean(folder) && !group && Boolean(fieldState.error)} + loadOptions={debouncedSearch} + cacheOptions + loadingMessage={'Loading groups...'} + defaultValue={defaultGroupValue} + defaultOptions={groupOptions} + getOptionLabel={(option: SelectableValue) => ( +
+ {option.label} + {/* making the assumption here that it's provisioned when it's disabled, should probably change this */} + {option.isDisabled && ( + <> + {' '} + + + )} +
+ )} + placeholder={'Evaluation group name'} + allowCustomValue + formatCreateLabel={(_) => '+ Add new '} + noOptionsMessage="Start typing to create evaluation group" + /> + )} name="group" control={control} rules={{ 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 2e8c3fe90fa..0cf8f23b4ac 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx @@ -21,7 +21,6 @@ import { EditCloudGroupModal, evaluateEveryValidationOptions } from '../rules/Ed import { FolderAndGroup, useGetGroupOptionsFromFolder } from './FolderAndGroup'; import { GrafanaAlertStatePicker } from './GrafanaAlertStatePicker'; import { RuleEditorSection } from './RuleEditorSection'; -import { Folder } from './RuleFolderPicker'; export const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds @@ -115,11 +114,9 @@ export const EvaluateEveryNewGroup = ({ rules }: { rules: RulerRulesConfigDTO | }; function FolderGroupAndEvaluationInterval({ - initialFolder, evaluateEvery, setEvaluateEvery, }: { - initialFolder: Folder | null; evaluateEvery: string; setEvaluateEvery: (value: string) => void; }) { @@ -167,7 +164,7 @@ function FolderGroupAndEvaluationInterval({ return (
- + {folderName && isEditingGroup && ( void; existing: boolean; @@ -270,11 +265,7 @@ export function GrafanaEvaluationBehavior({ // TODO remove "and alert condition" for recording rules - + {existing && (