diff --git a/.betterer.results b/.betterer.results index 030ace237cb..137ef7c32d1 100644 --- a/.betterer.results +++ b/.betterer.results @@ -990,9 +990,7 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Unexpected any. Specify a different type.", "4"], - [0, 0, 0, "Unexpected any. Specify a different type.", "5"] + [0, 0, 0, "Unexpected any. Specify a different type.", "3"] ], "public/app/features/alerting/unified/components/receivers/form/fields/SubformArrayField.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], @@ -1033,8 +1031,7 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "0"] ], "public/app/features/alerting/unified/types/receiver-form.ts:5381": [ - [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"] + [0, 0, 0, "Unexpected any. Specify a different type.", "0"] ], "public/app/features/alerting/unified/utils/misc.test.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/public/app/features/alerting/unified/components/contact-points/EditContactPoint.test.tsx b/public/app/features/alerting/unified/components/contact-points/EditContactPoint.test.tsx index 3a7895c1a29..b43671ce874 100644 --- a/public/app/features/alerting/unified/components/contact-points/EditContactPoint.test.tsx +++ b/public/app/features/alerting/unified/components/contact-points/EditContactPoint.test.tsx @@ -44,7 +44,7 @@ beforeEach(() => { grantUserPermissions([AccessControlAction.AlertingNotificationsRead, AccessControlAction.AlertingNotificationsWrite]); }); -const getTemplatePreviewContent = async () => within(screen.getByTestId('template-preview')).getByTestId('mockeditor'); +const getTemplatePreviewContent = async () => within(screen.getByTestId('template-preview')).findByTestId('mockeditor'); const templatesSelectorTestId = 'existing-templates-selector'; diff --git a/public/app/features/alerting/unified/components/contact-points/__snapshots__/useContactPoints.test.tsx.snap b/public/app/features/alerting/unified/components/contact-points/__snapshots__/useContactPoints.test.tsx.snap index ddb0a25c9e2..b409ffd1b60 100644 --- a/public/app/features/alerting/unified/components/contact-points/__snapshots__/useContactPoints.test.tsx.snap +++ b/public/app/features/alerting/unified/components/contact-points/__snapshots__/useContactPoints.test.tsx.snap @@ -106,8 +106,8 @@ exports[`useContactPoints should return contact points with status 1`] = ` "type": "oncall", Symbol(receiver_status): undefined, Symbol(receiver_metadata): { - "description": "Sends notifications to Grafana OnCall", - "name": "Grafana OnCall", + "description": "Sends notifications to Grafana IRM", + "name": "Grafana IRM", }, Symbol(receiver_plugin_metadata): { "icon": "public/img/alerting/oncall_logo.svg", @@ -357,8 +357,8 @@ exports[`useContactPoints when having oncall plugin installed and no alert manag "type": "oncall", Symbol(receiver_status): undefined, Symbol(receiver_metadata): { - "description": "Sends notifications to Grafana OnCall", - "name": "Grafana OnCall", + "description": "Sends notifications to Grafana IRM", + "name": "Grafana IRM", }, Symbol(receiver_plugin_metadata): { "description": "grafana-integration", diff --git a/public/app/features/alerting/unified/components/contact-points/useContactPoints.ts b/public/app/features/alerting/unified/components/contact-points/useContactPoints.ts index e1e9cc3f8e8..4cdfb910356 100644 --- a/public/app/features/alerting/unified/components/contact-points/useContactPoints.ts +++ b/public/app/features/alerting/unified/components/contact-points/useContactPoints.ts @@ -3,7 +3,6 @@ * and (if available) it will also fetch the status from the Grafana Managed status endpoint */ -import { merge, set } from 'lodash'; import { useMemo } from 'react'; import { receiversApi } from 'app/features/alerting/unified/api/receiversK8sApi'; @@ -13,11 +12,7 @@ import { BaseAlertmanagerArgs, Skippable } from 'app/features/alerting/unified/t import { cloudNotifierTypes } from 'app/features/alerting/unified/utils/cloud-alertmanager-notifier-types'; import { GRAFANA_RULES_SOURCE_NAME } from 'app/features/alerting/unified/utils/datasource'; import { isK8sEntityProvisioned, shouldUseK8sApi } from 'app/features/alerting/unified/utils/k8s/utils'; -import { - GrafanaManagedContactPoint, - GrafanaManagedReceiverConfig, - Receiver, -} from 'app/plugins/datasource/alertmanager/types'; +import { GrafanaManagedContactPoint, Receiver } from 'app/plugins/datasource/alertmanager/types'; import { getAPINamespace } from '../../../../../api/utils'; import { alertmanagerApi } from '../../api/alertmanagerApi'; @@ -327,47 +322,6 @@ export function useDeleteContactPoint({ alertmanager }: BaseAlertmanagerArgs) { return useK8sApi ? deleteFromK8sAPI : deleteFromAlertmanagerConfiguration; } -/** - * Turns a Grafana Managed receiver config into a format that can be sent to the k8s API - * - * When updating secure settings, we need to send a value of `true` for any secure setting that we want to keep the same. - * - * Any other setting that has a value in `secureSettings` will correspond to a new value for that setting - - * so we should not tell the API that we want to preserve it. Those values will instead be sent within `settings` - */ -const mapIntegrationSettingsForK8s = (integration: GrafanaManagedReceiverConfig): GrafanaManagedReceiverConfig => { - const { secureSettings, settings, ...restOfIntegration } = integration; - const secureFields = Object.entries(secureSettings || {}).reduce((acc, [key, value]) => { - // If a secure field has no (changed) value, then we tell the backend to persist it - if (value === undefined) { - return { - ...acc, - [key]: true, - }; - } - return acc; - }, {}); - - const mappedSecureSettings = Object.entries(secureSettings || {}).reduce((acc, [key, value]) => { - // If the value is an empty string/falsy value, then we need to omit it from the payload - // so the backend knows to remove it - if (!value) { - return acc; - } - - // Otherwise, we send the value of the secure field - return set(acc, key, value); - }, {}); - - // Merge settings properly with lodash so we don't lose any information from nested keys/secure settings - const mergedSettings = merge({}, settings, mappedSecureSettings); - - return { - ...restOfIntegration, - secureFields, - settings: mergedSettings, - }; -}; const grafanaContactPointToK8sReceiver = ( contactPoint: GrafanaManagedContactPoint, id?: string, @@ -380,7 +334,7 @@ const grafanaContactPointToK8sReceiver = ( }, spec: { title: contactPoint.name, - integrations: (contactPoint.grafana_managed_receiver_configs || []).map(mapIntegrationSettingsForK8s), + integrations: contactPoint.grafana_managed_receiver_configs || [], }, }; }; diff --git a/public/app/features/alerting/unified/components/receivers/GlobalConfigForm.tsx b/public/app/features/alerting/unified/components/receivers/GlobalConfigForm.tsx index e932fc4ea5e..5431c2391f0 100644 --- a/public/app/features/alerting/unified/components/receivers/GlobalConfigForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/GlobalConfigForm.tsx @@ -87,6 +87,7 @@ export const GlobalConfigForm = ({ config, alertManagerSourceName }: Props) => { option={option} error={errors[option.propertyName]} pathPrefix={''} + secureFields={{}} /> ))}
diff --git a/public/app/features/alerting/unified/components/receivers/__snapshots__/NewReceiverView.test.tsx.snap b/public/app/features/alerting/unified/components/receivers/__snapshots__/NewReceiverView.test.tsx.snap index f4c781df037..15473192d14 100644 --- a/public/app/features/alerting/unified/components/receivers/__snapshots__/NewReceiverView.test.tsx.snap +++ b/public/app/features/alerting/unified/components/receivers/__snapshots__/NewReceiverView.test.tsx.snap @@ -17,7 +17,7 @@ exports[`new receiver should be able to test and save a receiver 1`] = ` { "disableResolveMessage": false, "name": "test", - "secureSettings": {}, + "secureFields": {}, "settings": { "addresses": "tester@grafana.com", "singleEmail": false, diff --git a/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx b/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx index 1c99524f434..0648a964d4a 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelOptions.tsx @@ -2,20 +2,29 @@ import * as React from 'react'; import { DeepMap, FieldError, FieldErrors, useFormContext } from 'react-hook-form'; import { Field, SecretInput } from '@grafana/ui'; -import { NotificationChannelOption, NotificationChannelSecureFields } from 'app/types'; +import { NotificationChannelOption, NotificationChannelSecureFields, OptionMeta } from 'app/types'; -import { ChannelValues, ReceiverFormValues } from '../../../types/receiver-form'; +import { + ChannelValues, + CloudChannelValues, + GrafanaChannelValues, + ReceiverFormValues, +} from '../../../types/receiver-form'; import { OptionField } from './fields/OptionField'; export interface Props { defaultValues: R; selectedChannelOptions: NotificationChannelOption[]; - secureFields: NotificationChannelSecureFields; onResetSecureField: (key: string) => void; + onDeleteSubform?: (propertyName: string) => void; errors?: FieldErrors; - pathPrefix?: string; + /** + * The path for the integration in the array of integrations. + * This is used to access the settings and secure fields for the integration in a type-safe way. + */ + integrationPrefix: `items.${number}`; readOnly?: boolean; customValidators?: Record['customValidator']>; @@ -25,14 +34,23 @@ export function ChannelOptions({ defaultValues, selectedChannelOptions, onResetSecureField, - secureFields, + onDeleteSubform, errors, - pathPrefix = '', + integrationPrefix, readOnly = false, customValidators = {}, }: Props): JSX.Element { - const { watch } = useFormContext>(); - const currentFormValues = watch(); // react hook form types ARE LYING! + const { watch } = useFormContext>(); + + const [settings, secureFields] = watch([`${integrationPrefix}.settings`, `${integrationPrefix}.secureFields`]); + + // Note: settingsPath includes a trailing dot for OptionField, unlike the path used in watch() + const settingsPath = `${integrationPrefix}.settings.` as const; + + const getOptionMeta = (option: NotificationChannelOption): OptionMeta => ({ + required: determineRequired(option, settings, secureFields), + readOnly: determineReadOnly(option, settings, secureFields), + }); return ( <> @@ -41,9 +59,8 @@ export function ChannelOptions({ // Some options can be dependent on other options, this determines what is selected in the dependency options // I think this needs more thought. // pathPrefix = items.index. - const paths = pathPrefix.split('.'); - const selectedOptionValue = - paths.length >= 2 ? currentFormValues.items?.[Number(paths[1])].settings?.[option.showWhen.field] : undefined; + // const paths = pathPrefix.split('.'); + const selectedOptionValue = settings?.[option.showWhen.field]; if (option.showWhen.field && selectedOptionValue !== option.showWhen.is) { return null; @@ -51,33 +68,77 @@ export function ChannelOptions({ if (secureFields && secureFields[option.propertyName]) { return ( - - onResetSecureField(option.propertyName)} isConfigured /> + + onResetSecureField(option.propertyName)} + isConfigured + /> ); } const error: FieldError | DeepMap | undefined = ( - (option.secure ? errors?.secureSettings : errors?.settings) as DeepMap | undefined + (option.secure ? errors?.secureFields : errors?.settings) as DeepMap | undefined )?.[option.propertyName]; const defaultValue = defaultValues?.settings?.[option.propertyName]; return ( ); })} ); } + +const determineRequired = ( + option: NotificationChannelOption, + settings: Record, + secureFields: NotificationChannelSecureFields +) => { + if (!option.required) { + return false; + } + + if (!option.dependsOn) { + return option.required ? 'Required' : false; + } + + const dependentOn = Boolean(settings[option.dependsOn]) || Boolean(secureFields[option.dependsOn]); + + if (dependentOn) { + return false; + } + + return 'Required'; +}; + +const determineReadOnly = ( + option: NotificationChannelOption, + settings: Record, + secureFields: NotificationChannelSecureFields +) => { + if (!option.dependsOn) { + return false; + } + + return Boolean(settings[option.dependsOn]) || Boolean(secureFields[option.dependsOn]); +}; diff --git a/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx b/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx index cda5ed53046..2d1e62f144d 100644 --- a/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx +++ b/public/app/features/alerting/unified/components/receivers/form/ChannelSubForm.tsx @@ -1,31 +1,36 @@ import { css } from '@emotion/css'; import { sortBy } from 'lodash'; import * as React from 'react'; -import { useCallback, useEffect, useMemo, useState } from 'react'; -import { Controller, FieldErrors, FieldValues, useFormContext } from 'react-hook-form'; +import { useEffect, useMemo } from 'react'; +import { Controller, FieldErrors, useFormContext, useWatch } from 'react-hook-form'; import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { Alert, Button, Field, Select, Stack, Text, useStyles2 } from '@grafana/ui'; import { Trans, t } from 'app/core/internationalization'; import { useUnifiedAlertingSelector } from '../../../hooks/useUnifiedAlertingSelector'; -import { ChannelValues, CommonSettingsComponentType } from '../../../types/receiver-form'; +import { + ChannelValues, + CloudChannelValues, + CommonSettingsComponentType, + GrafanaChannelValues, + ReceiverFormValues, +} from '../../../types/receiver-form'; import { OnCallIntegrationType } from '../grafanaAppReceivers/onCall/useOnCallIntegration'; import { ChannelOptions } from './ChannelOptions'; import { CollapsibleSection } from './CollapsibleSection'; import { Notifier } from './notifiers'; -interface Props { +interface Props { defaultValues: R; initialValues?: R; - pathPrefix: string; + pathPrefix: `items.${number}.`; + integrationIndex: number; notifiers: Notifier[]; onDuplicate: () => void; onTest?: () => void; commonSettingsComponent: CommonSettingsComponentType; - - secureFields?: Record; errors?: FieldErrors; onDelete?: () => void; isEditable?: boolean; @@ -38,74 +43,85 @@ export function ChannelSubForm({ defaultValues, initialValues, pathPrefix, + integrationIndex, onDuplicate, onDelete, onTest, notifiers, errors, - secureFields, commonSettingsComponent: CommonSettingsComponent, isEditable = true, isTestable, customValidators = {}, }: Props): JSX.Element { const styles = useStyles2(getStyles); + const { control, watch, register, trigger, formState, setValue, getValues } = + useFormContext>(); - const fieldName = useCallback((fieldName: string) => `${pathPrefix}${fieldName}`, [pathPrefix]); + const channelFieldPath = `items.${integrationIndex}` as const; + const typeFieldPath = `${channelFieldPath}.type` as const; + const settingsFieldPath = `${channelFieldPath}.settings` as const; - const { control, watch, register, trigger, formState, setValue } = useFormContext(); - const selectedType = watch(fieldName('type')) ?? defaultValues.type; // nope, setting "default" does not work at all. - const parse_mode = watch(fieldName('settings.parse_mode')); + const selectedType = watch(typeFieldPath) ?? defaultValues.type; + const parse_mode = watch(`${settingsFieldPath}.parse_mode`); const { loading: testingReceiver } = useUnifiedAlertingSelector((state) => state.testReceivers); // TODO I don't like integration specific code here but other ways require a bigger refactoring - const onCallIntegrationType = watch(fieldName('settings.integration_type')); + const onCallIntegrationType = watch(`${settingsFieldPath}.integration_type`); const isTestAvailable = onCallIntegrationType !== OnCallIntegrationType.NewIntegration; useEffect(() => { - register(`${pathPrefix}.__id`); + register(`${channelFieldPath}.__id`); /* Need to manually register secureFields or else they'll be lost when testing a contact point */ - register(`${pathPrefix}.secureFields`); - }, [register, pathPrefix]); + register(`${channelFieldPath}.secureFields`); + }, [register, channelFieldPath]); // Prevent forgetting about initial values when switching the integration type and the oncall integration type useEffect(() => { // Restore values when switching back from a changed integration to the default one - const subscription = watch((v, { name, type }) => { - const value = name ? v[name] : ''; - if (initialValues && name === fieldName('type') && value === initialValues.type && type === 'change') { - setValue(fieldName('settings'), initialValues.settings); + const subscription = watch((formValues, { name, type }) => { + // @ts-expect-error name is valid key for formValues + const value = name ? formValues[name] : ''; + if (initialValues && name === typeFieldPath && value === initialValues.type && type === 'change') { + setValue(settingsFieldPath, initialValues.settings); } // Restore initial value of an existing oncall integration if ( initialValues && - name === fieldName('settings.integration_type') && + name === `${settingsFieldPath}.integration_type` && value === OnCallIntegrationType.ExistingIntegration ) { - setValue(fieldName('settings.url'), initialValues.settings.url); + setValue(`${settingsFieldPath}.url`, initialValues.settings.url); } }); return () => subscription.unsubscribe(); - }, [selectedType, initialValues, setValue, fieldName, watch]); + }, [selectedType, initialValues, setValue, settingsFieldPath, typeFieldPath, watch]); - const [_secureFields, setSecureFields] = useState>(secureFields ?? {}); + // const [_secureFields, setSecureFields] = useState>(secureFields ?? {}); + const formSecureFields = useWatch({ control, name: `${channelFieldPath}.secureFields` }); const onResetSecureField = (key: string) => { - if (_secureFields[key]) { - const updatedSecureFields = { ..._secureFields }; - updatedSecureFields[key] = ''; - setSecureFields(updatedSecureFields); - setValue(`${pathPrefix}.secureFields`, updatedSecureFields); + // formSecureFields might not be up to date if this function is called multiple times in a row + const currentSecureFields = getValues(`${channelFieldPath}.secureFields`); + if (currentSecureFields[key]) { + setValue(`${channelFieldPath}.secureFields`, { ...currentSecureFields, [key]: '' }); } }; + const onDeleteSubform = (propertyName: string) => { + const relatedSecureFields = Object.keys(formSecureFields).filter((key) => key.startsWith(propertyName)); + relatedSecureFields.forEach((key) => { + onResetSecureField(key); + }); + setValue(`${channelFieldPath}.settings.${propertyName}`, undefined); + }; + const typeOptions = useMemo( (): SelectableValue[] => - sortBy(notifiers, ({ dto, meta }) => [meta?.order ?? 0, dto.name]) - // .notifiers.sort((a, b) => a.dto.name.localeCompare(b.dto.name)) - .map(({ dto: { name, type }, meta }) => ({ + sortBy(notifiers, ({ dto, meta }) => [meta?.order ?? 0, dto.name]).map( + ({ dto: { name, type }, meta }) => ({ // @ts-expect-error ReactNode is supported label: ( @@ -116,7 +132,8 @@ export function ChannelSubForm({ value: type, description: meta?.description, isDisabled: meta ? !meta.enabled : false, - })), + }) + ), [notifiers] ); @@ -137,8 +154,8 @@ export function ChannelSubForm({ const showTelegramWarning = isTelegram && !isParseModeNone; // if there are mandatory options defined, optional options will be hidden by a collapse // if there aren't mandatory options, all options will be shown without collapse - const mandatoryOptions = notifier?.dto.options.filter((o) => o.required); - const optionalOptions = notifier?.dto.options.filter((o) => !o.required); + const mandatoryOptions = notifier?.dto.options.filter((o) => o.required) ?? []; + const optionalOptions = notifier?.dto.options.filter((o) => !o.required) ?? []; const contactPointTypeInputId = `contact-point-type-${pathPrefix}`; return ( @@ -151,7 +168,8 @@ export function ChannelSubForm({ data-testid={`${pathPrefix}type`} > ( option.validationRule ? validateOption(v, option.validationRule, option.required) : true, @@ -233,7 +237,7 @@ const OptionInput: FC = ({ onSelectTemplate={onSelectTemplate} > {isEncryptedInput ? ( - onResetSecureField?.(nestedKey)} isConfigured /> + onResetSecureField?.(nestedKey)} isConfigured /> ) : (