Alerting: small rule form ux improvements (#36941)

* dedupe folder option for existing grafana rules * update test mocks * change toggle to chevron for expanding error state ui * fix some strict lint errors
2025-02-25 18:55:37 -06:00 · 2021-07-21 18:01:05 +03:00 · 2021-07-21 18:01:05 +03:00 · cac4b4b443
commit cac4b4b443
parent 5a0221a8c4
11 changed files with 37 additions and 26 deletions
--- a/public/app/core/components/Select/FolderPicker.tsx
+++ b/public/app/core/components/Select/FolderPicker.tsx
@ -58,7 +58,7 @@ export class FolderPicker extends PureComponent<Props, State> {
  };

  getOptions = async (query: string) => {
-    const { rootName, enableReset, initialTitle, permissionLevel, showRoot } = this.props;
+    const { rootName, enableReset, initialTitle, permissionLevel, initialFolderId, showRoot } = this.props;
    const params = {
      query,
      type: 'dash-folder',
@ -74,8 +74,13 @@ export class FolderPicker extends PureComponent<Props, State> {
      options.unshift({ label: rootName, value: 0 });
    }

-    if (enableReset && query === '' && initialTitle !== '') {
-      options.unshift({ label: initialTitle, value: undefined });
+    if (
+      enableReset &&
+      query === '' &&
+      initialTitle !== '' &&
+      !options.find((option) => option.label === initialTitle)
+    ) {
+      options.unshift({ label: initialTitle, value: initialFolderId });
    }

    return options;
@ -122,9 +127,8 @@ export class FolderPicker extends PureComponent<Props, State> {
      folder = options.find((option) => option.value === initialFolderId) || null;
    } else if (enableReset && initialTitle) {
      folder = resetFolder;
-    } else if (initialTitle && initialFolderId === -1) {
-      // @TODO temporary, we don't know the id for alerting rule folder in some cases
-      folder = options.find((option) => option.label === initialTitle) || null;
+    } else if (initialFolderId) {
+      folder = options.find((option) => option.id === initialFolderId) || null;
    }

    if (!folder && !this.props.allowEmpty) {
--- a/public/app/features/alerting/unified/RuleViewer.test.tsx
+++ b/public/app/features/alerting/unified/RuleViewer.test.tsx
@ -87,6 +87,7 @@ const mockGrafanaRule = {
    grafana_alert: {
      condition: 'B',
      exec_err_state: GrafanaAlertStateDecision.Alerting,
+      namespace_id: 11,
      namespace_uid: 'namespaceuid123',
      no_data_state: GrafanaAlertStateDecision.NoData,
      title: 'Test alert',
--- a/public/app/features/alerting/unified/components/CollapseToggle.tsx
+++ b/public/app/features/alerting/unified/components/CollapseToggle.tsx
@ -7,14 +7,16 @@ interface Props extends HTMLAttributes<HTMLButtonElement> {
  onToggle: (isCollapsed: boolean) => void;
  size?: IconSize;
  className?: string;
+  text?: string;
 }

-export const CollapseToggle: FC<Props> = ({ isCollapsed, onToggle, className, size = 'xl', ...restOfProps }) => {
+export const CollapseToggle: FC<Props> = ({ isCollapsed, onToggle, className, text, size = 'xl', ...restOfProps }) => {
  const styles = useStyles(getStyles);

  return (
    <button className={cx(styles.expandButton, className)} onClick={() => onToggle(!isCollapsed)} {...restOfProps}>
      <Icon size={size} name={isCollapsed ? 'angle-right' : 'angle-down'} />
+      {text}
    </button>
  );
 };
@ -26,6 +28,9 @@ export const getStyles = () => ({

    outline: none !important;

+    display: inline-flex;
+    align-items: center;
+
    svg {
      margin-bottom: 0;
    }
--- a/public/app/features/alerting/unified/components/rule-editor/ExpressionEditor.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/ExpressionEditor.tsx
@ -63,7 +63,7 @@ function useQueryMappers(dataSourceName: string): QueryMappers {
      case 'loki':
      case 'prometheus':
        return {
-          mapToValue: (query: PromQuery | LokiQuery) => query.expr,
+          mapToValue: (query: DataQuery) => (query as PromQuery | LokiQuery).expr,
          mapToQuery: (existing: DataQuery, value: string | undefined) => ({ ...existing, expr: value }),
        };
      default:
--- a/public/app/features/alerting/unified/components/rule-editor/GrafanaConditionsStep.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaConditionsStep.tsx
@ -1,7 +1,7 @@
 import React, { FC, useState } from 'react';
 import { css } from '@emotion/css';
-import { GrafanaTheme, parseDuration, durationToMilliseconds } from '@grafana/data';
-import { Field, InlineLabel, Input, InputControl, Switch, useStyles } from '@grafana/ui';
+import { parseDuration, durationToMilliseconds, GrafanaTheme2 } from '@grafana/data';
+import { Field, InlineLabel, Input, InputControl, useStyles2 } from '@grafana/ui';
 import { useFormContext, RegisterOptions } from 'react-hook-form';
 import { RuleFormValues } from '../../types/rule-form';
 import { positiveDurationValidationPattern, durationValidationPattern } from '../../utils/time';
@ -10,6 +10,7 @@ import { GrafanaAlertStatePicker } from './GrafanaAlertStatePicker';
 import { RuleEditorSection } from './RuleEditorSection';
 import { PreviewRule } from './PreviewRule';
 import { GrafanaConditionEvalWarning } from './GrafanaConditionEvalWarning';
+import { CollapseToggle } from '../CollapseToggle';

 const MIN_TIME_RANGE_STEP_S = 10; // 10 seconds

@ -43,7 +44,7 @@ const evaluateEveryValidationOptions: RegisterOptions = {
 };

 export const GrafanaConditionsStep: FC = () => {
-  const styles = useStyles(getStyles);
+  const styles = useStyles2(getStyles);
  const [showErrorHandling, setShowErrorHandling] = useState(false);
  const {
    register,
@ -83,9 +84,12 @@ export const GrafanaConditionsStep: FC = () => {
        </div>
      </Field>
      <GrafanaConditionEvalWarning />
-      <Field label="Configure no data and error handling" horizontal={true} className={styles.switchField}>
-        <Switch value={showErrorHandling} onChange={() => setShowErrorHandling(!showErrorHandling)} />
-      </Field>
+      <CollapseToggle
+        isCollapsed={!showErrorHandling}
+        onToggle={(collapsed) => setShowErrorHandling(!collapsed)}
+        text="Configure no data and error handling"
+        className={styles.collapseToggle}
+      />
      {showErrorHandling && (
        <>
          <Field label="Alert state if no data or all values are null">
@ -121,7 +125,7 @@ export const GrafanaConditionsStep: FC = () => {
  );
 };

-const getStyles = (theme: GrafanaTheme) => ({
+const getStyles = (theme: GrafanaTheme2) => ({
  inlineField: css`
    margin-bottom: 0;
  `,
@ -131,12 +135,7 @@ const getStyles = (theme: GrafanaTheme) => ({
    justify-content: flex-start;
    align-items: flex-start;
  `,
-  switchField: css`
-    display: inline-flex;
-    flex-direction: row-reverse;
-    margin-top: ${theme.spacing.md};
-    & > div:first-child {
-      margin-left: ${theme.spacing.sm};
-    }
+  collapseToggle: css`
+    margin: ${theme.spacing(2, 0, 2, -1)};
  `,
 });
--- a/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx
@ -63,7 +63,7 @@ export const QueryWrapper: FC<Props> = ({

  return (
    <div className={styles.wrapper}>
-      <QueryEditorRow
+      <QueryEditorRow<DataQuery>
        dataSource={dsSettings}
        onChangeDataSource={!isExpression ? (settings) => onChangeDataSource(settings, index) : undefined}
        id={query.refId}
--- a/public/app/features/alerting/unified/components/rules/RulesFilter.tsx
+++ b/public/app/features/alerting/unified/components/rules/RulesFilter.tsx
@ -98,7 +98,7 @@ const RulesFilter = () => {
            <Label>View as</Label>
            <RadioButtonGroup
              options={ViewOptions}
-              value={String(queryParams['view'] ?? 'group')}
+              value={String(queryParams['view'] || 'group')}
              onChange={handleViewChange}
            />
          </div>
--- a/public/app/features/alerting/unified/utils/amroutes.ts
+++ b/public/app/features/alerting/unified/utils/amroutes.ts
@ -9,7 +9,7 @@ import { isUndefined, omitBy } from 'lodash';
 const defaultValueAndType: [string, string] = ['', timeOptions[0].value];

 const matchersToArrayFieldMatchers = (matchers: Record<string, string> | undefined, isRegex: boolean): Matcher[] =>
-  Object.entries(matchers ?? {}).reduce(
+  Object.entries(matchers ?? {}).reduce<Matcher[]>(
    (acc, [name, value]) => [
      ...acc,
      {
--- a/public/app/features/alerting/unified/utils/query.test.ts
+++ b/public/app/features/alerting/unified/utils/query.test.ts
@ -78,6 +78,7 @@ describe('alertRuleToQueries', () => {
 const grafanaAlert = {
  condition: 'B',
  exec_err_state: GrafanaAlertStateDecision.Alerting,
+  namespace_id: 11,
  namespace_uid: 'namespaceuid123',
  no_data_state: GrafanaAlertStateDecision.NoData,
  title: 'Test alert',
--- a/public/app/features/alerting/unified/utils/rule-form.ts
+++ b/public/app/features/alerting/unified/utils/rule-form.ts
@ -106,7 +106,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF
        condition: ga.condition,
        annotations: listifyLabelsOrAnnotations(rule.annotations),
        labels: listifyLabelsOrAnnotations(rule.labels),
-        folder: { title: namespace, id: -1 },
+        folder: { title: namespace, id: ga.namespace_id },
      };
    } else {
      throw new Error('Unexpected type of rule for grafana rules source');
--- a/public/app/types/unified-alerting-dto.ts
+++ b/public/app/types/unified-alerting-dto.ts
@ -120,6 +120,7 @@ export interface PostableGrafanaRuleDefinition {
 export interface GrafanaRuleDefinition extends PostableGrafanaRuleDefinition {
  uid: string;
  namespace_uid: string;
+  namespace_id: number;
 }

 export interface RulerGrafanaRuleDTO {