From 5548e5976b7845c7c43e30604b0f06181366bc88 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Fri, 18 Oct 2024 14:28:35 +0200 Subject: [PATCH] Alerting: Fix templates editing validation (#94960) Fix title validation when editing template files --- .../alerting/unified/Templates.test.tsx | 36 +++++++++++++++++-- .../useNotificationTemplates.ts | 15 +++++++- .../components/receivers/TemplateForm.tsx | 2 +- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/public/app/features/alerting/unified/Templates.test.tsx b/public/app/features/alerting/unified/Templates.test.tsx index 9d048d60495..bd6ea96480c 100644 --- a/public/app/features/alerting/unified/Templates.test.tsx +++ b/public/app/features/alerting/unified/Templates.test.tsx @@ -2,8 +2,8 @@ import { InitialEntry } from 'history/createMemoryHistory'; import * as React from 'react'; import { Route, Routes } from 'react-router-dom-v5-compat'; import { Props } from 'react-virtualized-auto-sizer'; -import { render, screen, within } from 'test/test-utils'; -import { byRole } from 'testing-library-selector'; +import { render, screen, waitFor, within } from 'test/test-utils'; +import { byLabelText, byRole } from 'testing-library-selector'; import { CodeEditorProps } from '@grafana/ui/src/components/Monaco/types'; import { AppNotificationList } from 'app/core/components/AppNotifications/AppNotificationList'; @@ -42,6 +42,10 @@ jest.mock('@grafana/ui', () => ({ const ui = { templateForm: byRole('form', { name: 'Template form' }), + form: { + title: byLabelText(/Template name/), + saveButton: byRole('button', { name: 'Save' }), + }, }; const navUrl = { @@ -91,6 +95,34 @@ describe('Templates routes', () => { expect(form).toBeInTheDocument(); expect(within(form).getByRole('textbox', { name: /Template name/ })).toHaveValue(''); }); + + it('should pass name validation when editing existing template', async () => { + const { user } = setup([navUrl.edit('custom-email')]); + + const titleElement = await ui.form.title.find(); + await waitFor(() => { + expect(titleElement).toHaveValue('custom-email'); + }); + + await user.click(ui.form.saveButton.get()); + + // No error message should be displayed for a unique name + expect(screen.queryByText('Another template with this name already exists')).not.toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByText('Template saved')).toBeInTheDocument(); + }); + }); + + it('should display error message when creating new template with duplicate name', async () => { + const { user } = setup([navUrl.new]); + + const titleElement = await ui.form.title.find(); + await user.type(titleElement, 'custom-email'); + + await user.click(ui.form.saveButton.get()); + + expect(screen.getByText('Another template with this name already exists')).toBeInTheDocument(); + }); }); describe('Templates K8s API', () => { diff --git a/public/app/features/alerting/unified/components/contact-points/useNotificationTemplates.ts b/public/app/features/alerting/unified/components/contact-points/useNotificationTemplates.ts index f1d15f6dfe5..a640e936dcf 100644 --- a/public/app/features/alerting/unified/components/contact-points/useNotificationTemplates.ts +++ b/public/app/features/alerting/unified/components/contact-points/useNotificationTemplates.ts @@ -285,7 +285,15 @@ export function useDeleteNotificationTemplate({ alertmanager }: BaseAlertmanager return k8sApiSupported ? deleteUsingK8sApi : deleteUsingConfigFileApi; } -export function useValidateNotificationTemplate({ alertmanager }: BaseAlertmanagerArgs) { +interface ValidateNotificationTemplateParams { + alertmanager: string; + originalTemplate?: NotificationTemplate; +} + +export function useValidateNotificationTemplate({ + alertmanager, + originalTemplate, +}: ValidateNotificationTemplateParams) { const { useLazyGetAlertmanagerConfigurationQuery } = alertmanagerApi; const [fetchAmConfig] = useLazyGetAlertmanagerConfigurationQuery(); @@ -298,6 +306,11 @@ export function useValidateNotificationTemplate({ alertmanager }: BaseAlertmanag return true; } + if (originalTemplate?.title === name) { + // If original template is defined we update existing template so name will not be unique but it's ok + return true; + } + const amConfig = await fetchAmConfig(alertmanager).unwrap(); const templates = amConfigToTemplates(amConfig); const templateOfThisNameExists = templates.some((t) => t.title === name); diff --git a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx index eeb6890e744..e1e07f99e96 100644 --- a/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/TemplateForm.tsx @@ -92,7 +92,7 @@ export const TemplateForm = ({ originalTemplate, prefill, alertmanager }: Props) const createNewTemplate = useCreateNotificationTemplate({ alertmanager }); const updateTemplate = useUpdateNotificationTemplate({ alertmanager }); - const { titleIsUnique } = useValidateNotificationTemplate({ alertmanager }); + const { titleIsUnique } = useValidateNotificationTemplate({ alertmanager, originalTemplate }); useCleanup((state) => (state.unifiedAlerting.saveAMConfig = initialAsyncRequestState)); const formRef = useRef(null);