refactoring and fixing

2026-01-12 11:41:25 +00:00
parent 8c0e09cc2d
commit d9e9d52028
6 changed files with 126 additions and 44 deletions
@@ -1,13 +1,19 @@
 import { css } from '@emotion/css';
+import { useEffect, useMemo } from 'react';
 import { FormProvider, useForm } from 'react-hook-form';

 import { GrafanaTheme2 } from '@grafana/data';
 import { t } from '@grafana/i18n';
-import { Drawer, useStyles2 } from '@grafana/ui';
+import { Button, Drawer, Stack, useStyles2 } from '@grafana/ui';
+import { useAppNotification } from 'app/core/copy/appNotification';
 import { RuleDefinitionSection } from 'app/features/alerting/unified/components/RuleDefinitionSection';

+import { isGrafanaGroupUpdatedResponse } from '../api/alertRuleModel';
+import { useAddRuleToRuleGroup } from '../hooks/ruleGroup/useUpsertRuleFromRuleGroup';
 import { getDefaultFormValues } from '../rule-editor/formDefaults';
 import { RuleFormType, RuleFormValues } from '../types/rule-form';
+import { formValuesToRulerGrafanaRuleDTO } from '../utils/rule-form';
+import { getRuleGroupLocationFromFormValues } from '../utils/rules';

 import { RuleConditionSection } from './RuleConditionSection';
 import { RuleNotificationSection } from './RuleNotificationSection';
@@ -16,16 +22,53 @@ export interface AlertRuleDrawerFormProps {
  isOpen: boolean;
  onClose: () => void;
  title?: string;
+  onContinueInAlerting?: () => void;
+  prefill?: Partial<RuleFormValues>;
 }

-export function AlertRuleDrawerForm({ isOpen, onClose, title }: AlertRuleDrawerFormProps) {
-  const methods = useForm<RuleFormValues>({ defaultValues: getDefaultFormValues(RuleFormType.grafana) });
+export function AlertRuleDrawerForm({ isOpen, onClose, title, onContinueInAlerting, prefill }: AlertRuleDrawerFormProps) {
+  const baseDefaults = useMemo(() => getDefaultFormValues(RuleFormType.grafana), []);
+  const methods = useForm<RuleFormValues>({
+    defaultValues: prefill ? { ...baseDefaults, ...(prefill as Partial<RuleFormValues>) } : baseDefaults,
+  });
  const styles = useStyles2(getStyles);
+  const [addRuleToRuleGroup] = useAddRuleToRuleGroup();
+  const notifyApp = useAppNotification();
+
+  // Keep form in sync if prefill changes between openings
+  useEffect(() => {
+    if (prefill) {
+      methods.reset({ ...baseDefaults, ...(prefill as Partial<RuleFormValues>) });
+    }
+  }, [prefill, methods]);

  if (!isOpen) {
    return null;
  }

+  const submit = async (values: RuleFormValues) => {
+    try {
+      const groupName = values.group && values.group.trim().length > 0 ? values.group : (values.name?.trim() || 'default');
+      const effectiveValues: RuleFormValues = { ...values, group: groupName };
+
+      const dto = formValuesToRulerGrafanaRuleDTO(effectiveValues);
+      const groupIdentifier = getRuleGroupLocationFromFormValues(effectiveValues);
+      const result = await addRuleToRuleGroup.execute(groupIdentifier, dto, effectiveValues.evaluateEvery);
+      if (isGrafanaGroupUpdatedResponse(result)) {
+        onClose();
+        return;
+      }
+      notifyApp.error('Failed to create rule', 'The rule was not created. Please review the form and try again.');
+    } catch (err: any) {
+      const msg = err?.data?.message || err?.message || 'Unknown error while creating the rule.';
+      notifyApp.error('Failed to create rule', msg);
+    }
+  };
+
+  const onInvalid = () => {
+    notifyApp.error('There are errors in the form. Please correct them and try again!');
+  };
+
  return (
    <Drawer
      title={title ?? t('alerting.new-rule-from-panel-button.new-alert-rule', 'New alert rule')}
@@ -37,7 +80,34 @@ export function AlertRuleDrawerForm({ isOpen, onClose, title }: AlertRuleDrawerF
        <RuleConditionSection type={RuleFormType.grafana} />
        <div className={styles.divider} aria-hidden="true" />
        <RuleNotificationSection />
-        <div className={styles.divider} aria-hidden="true" />
+        <div className={styles.footer}>
+          <Stack direction="row" gap={1} alignItems="center" justifyContent="flex-end">
+            <Button
+              variant="secondary"
+              type="button"
+              onClick={() => {
+                methods.reset(getDefaultFormValues(RuleFormType.grafana));
+                onClose();
+              }}
+            >
+              {t('alerting.common.cancel', 'Cancel')}
+            </Button>
+            {onContinueInAlerting && (
+              <Button variant="secondary" type="button" onClick={onContinueInAlerting}>
+                {t('alerting.simplified.continue-in-alerting', 'Continue in Alerting')}
+              </Button>
+            )}
+            <Button
+              variant="primary"
+              type="button"
+              onClick={methods.handleSubmit((values) => submit(values), onInvalid)}
+              disabled={methods.formState.isSubmitting}
+              icon={methods.formState.isSubmitting ? 'spinner' : undefined}
+            >
+              {t('alerting.simplified.create', 'Create')}
+            </Button>
+          </Stack>
+        </div>
      </FormProvider>
    </Drawer>
  );
@@ -50,5 +120,8 @@ function getStyles(theme: GrafanaTheme2) {
      margin: `${theme.spacing(3)} 0`,
      width: '100%',
    }),
+    footer: css({
+      marginTop: theme.spacing(3),
+    }),
  };
 }
@@ -83,7 +83,8 @@ export function RuleDefinitionSection({ type }: { type: RuleFormType }) {
                isOpen={showLabelsEditor}
                onClose={(labelsToUpdate) => {
                  if (labelsToUpdate) {
-                    setValue('labels', labelsToUpdate);
+                    const filtered = labelsToUpdate.filter((l) => (l?.key ?? '').length > 0 || (l?.value ?? '').length > 0);
+                    setValue('labels', filtered, { shouldDirty: true, shouldValidate: true });
                  }
                  setShowLabelsEditor(false);
                }}
@@ -4,13 +4,13 @@ import { useAsync } from 'react-use';

 import { urlUtil } from '@grafana/data';
 import { Trans, t } from '@grafana/i18n';
-import { config } from '@grafana/runtime';
+import { config, locationService, logInfo } from '@grafana/runtime';
 import { Alert, Button, LinkButton } from '@grafana/ui';
 import { DashboardModel } from 'app/features/dashboard/state/DashboardModel';
 import { PanelModel } from 'app/features/dashboard/state/PanelModel';
 import { useSelector } from 'app/types/store';

-import { LogMessages, logInfo } from '../../Analytics';
+import { LogMessages } from '../../Analytics';
 import { AlertRuleDrawerForm } from '../../components/AlertRuleDrawerForm';
 import { panelToRuleFormValues } from '../../utils/rule-form';

@@ -58,47 +58,48 @@ export const NewRuleFromPanelButton = ({ dashboard, panel, className }: Props) =
    );
  }

-  const ruleFormUrl = urlUtil.renderUrl('alerting/new', {
-    defaults: JSON.stringify(formValues),
-    returnTo: location.pathname + location.search,
-  });
+  const onContinueInAlerting = async () => {
+    logInfo(LogMessages.alertRuleFromPanel);
+    // Refresh values to ensure they're up-to-date with current panel state
+    const updateToDateFormValues = await panelToRuleFormValues(panel, dashboard);
+    const ruleFormUrl = urlUtil.renderUrl('alerting/new', {
+      defaults: JSON.stringify(updateToDateFormValues),
+      returnTo: location.pathname + location.search,
+    });
+    locationService.push(ruleFormUrl);
+  };

  const shouldUseDrawer = config.featureToggles.createAlertRuleFromPanel;

  if (shouldUseDrawer) {
-    if (isOpen) {
-      return (
-        <>
-          <Button
-            icon="bell"
-            className={className}
-            data-testid="create-alert-rule-button-drawer"
-            onClick={() => setIsOpen(true)}
-          >
-            <Trans i18nKey="alerting.new-rule-from-panel-button.new-alert-rule">New alert rule</Trans>
-          </Button>
-          <AlertRuleDrawerForm isOpen={isOpen} onClose={() => setIsOpen(false)} />
-        </>
-      );
-    }
-
    return (
-      <Button
-        icon="bell"
-        className={className}
-        data-testid="create-alert-rule-button-drawer"
-        onClick={() => setIsOpen(true)}
-      >
-        <Trans i18nKey="alerting.new-rule-from-panel-button.new-alert-rule">New alert rule</Trans>
-      </Button>
+      <>
+        <Button
+          icon="bell"
+          className={className}
+          data-testid="create-alert-rule-button-drawer"
+          onClick={() => setIsOpen(true)}
+        >
+          <Trans i18nKey="alerting.new-rule-from-panel-button.new-alert-rule">New alert rule</Trans>
+        </Button>
+        <AlertRuleDrawerForm
+          isOpen={isOpen}
+          onClose={() => setIsOpen(false)}
+          onContinueInAlerting={onContinueInAlerting}
+          prefill={formValues ?? undefined}
+        />
+      </>
    );
  }

  return (
    <LinkButton
      icon="bell"
-      onClick={() => logInfo(LogMessages.alertRuleFromPanel)}
-      href={ruleFormUrl}
+      onClick={onContinueInAlerting}
+      href={urlUtil.renderUrl('alerting/new', {
+        defaults: JSON.stringify(formValues),
+        returnTo: location.pathname + location.search,
+      })}
      className={className}
      data-testid="create-alert-rule-button"
    >
@@ -39,11 +39,12 @@ function mapLabelsToOptions(
 }

 export interface LabelsInRuleProps {
-  labels: Array<{ key: string; value: string }>;
+  labels: Array<{ key: string; value: string }> | undefined | null;
 }

 export const LabelsInRule = ({ labels }: LabelsInRuleProps) => {
-  const labelsObj: Record<string, string> = labels.reduce((acc: Record<string, string>, label) => {
+  const safeLabels = Array.isArray(labels) ? labels : [];
+  const labelsObj: Record<string, string> = safeLabels.reduce((acc: Record<string, string>, label) => {
    if (label.key) {
      acc[label.key] = label.value;
    }
@@ -1,4 +1,4 @@
-import { useFormContext } from 'react-hook-form';
+import { useFormContext, useWatch } from 'react-hook-form';

 import { Trans, t } from '@grafana/i18n';
 import { Button, Icon, Stack, Text, Tooltip } from '@grafana/ui';
@@ -20,9 +20,10 @@ export function LabelsFieldInForm({
  labelVariant = 'default',
  showHelpTooltip = false,
 }: LabelsFieldInFormProps) {
-  const { watch } = useFormContext<RuleFormValues>();
+  const { control, watch } = useFormContext<RuleFormValues>();

-  const labels = watch('labels');
+  // Subscribe to label changes so UI updates when modal saves
+  const labels = useWatch({ control, name: 'labels' }) ?? [];
  const type = watch('type');

  const isRecordingRule = type ? isRecordingRuleByType(type) : false;
@@ -35,7 +36,7 @@ export function LabelsFieldInForm({
        'Add labels to your rule for searching, silencing, or routing to a notification policy.'
      );

-  const hasLabels = Object.keys(labels).length > 0 && labels.some((label) => label.key || label.value);
+  const hasLabels = Array.isArray(labels) && labels.length > 0 && labels.some((label) => label?.key || label?.value);

  return (
    <Stack direction="column" gap={2}>
@@ -73,7 +73,12 @@ export const ScenesNewRuleFromPanelButton = ({ panel, className }: ScenesNewRule
        >
          <Trans i18nKey="alerting.new-rule-from-panel-button.new-alert-rule">New alert rule</Trans>
        </Button>
-        <AlertRuleDrawerForm isOpen={isOpen} onClose={() => setIsOpen(false)} />
+        <AlertRuleDrawerForm
+          isOpen={isOpen}
+          onClose={() => setIsOpen(false)}
+          onContinueInAlerting={onClick}
+          prefill={formValues ?? undefined}
+        />
      </>
    );
  }