Alerting: Fix wrong use of empty list in times field in the UI (#84179)

* Fix wrong use of empty list in times field in the UI * Add tooltip for disable switch * Show disabled badge in mute timings * Disable time ranges when disabling time interval in the UI * PR review comments * remove tooltip for the field as it does not register it correctly * remove wrong code line * Add comment * Address PR review comments
2025-02-25 18:55:37 -06:00 · 2024-03-15 15:48:16 +01:00 · 2024-03-15 15:48:16 +01:00 · 1f13a14815
commit 1f13a14815
parent 4753948262
7 changed files with 196 additions and 75 deletions
--- a/public/app/features/alerting/unified/MuteTimings.test.tsx
+++ b/public/app/features/alerting/unified/MuteTimings.test.tsx
@ -351,7 +351,6 @@ describe('Mute timings', () => {
            name: 'default-mute',
            time_intervals: [
              {
-                times: [],
                weekdays: ['monday'],
                days_of_month: ['-7:-1'],
                months: ['3', '6', '9', '12'],
--- a/public/app/features/alerting/unified/components/mute-timings/MuteTimingForm.tsx
+++ b/public/app/features/alerting/unified/components/mute-timings/MuteTimingForm.tsx
@ -14,7 +14,7 @@ import { MuteTimingFields } from '../../types/mute-timing-form';
 import { renameMuteTimings } from '../../utils/alertmanager';
 import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource';
 import { makeAMLink } from '../../utils/misc';
-import { createMuteTiming, defaultTimeInterval } from '../../utils/mute-timings';
+import { createMuteTiming, defaultTimeInterval, isTimeIntervalDisabled } from '../../utils/mute-timings';
 import { ProvisionedResource, ProvisioningAlert } from '../Provisioning';

 import { MuteTimingTimeInterval } from './MuteTimingTimeInterval';
@ -38,12 +38,13 @@ const useDefaultValues = (muteTiming?: MuteTimeInterval): MuteTimingFields => {
  }

  const intervals = muteTiming.time_intervals.map((interval) => ({
-    times: interval.times ?? defaultTimeInterval.times,
-    weekdays: interval.weekdays?.join(', ') ?? defaultTimeInterval.weekdays,
-    days_of_month: interval.days_of_month?.join(', ') ?? defaultTimeInterval.days_of_month,
-    months: interval.months?.join(', ') ?? defaultTimeInterval.months,
-    years: interval.years?.join(', ') ?? defaultTimeInterval.years,
+    times: interval.times,
+    weekdays: interval.weekdays?.join(', '),
+    days_of_month: interval.days_of_month?.join(', '),
+    months: interval.months?.join(', '),
+    years: interval.years?.join(', '),
    location: interval.location ?? defaultTimeInterval.location,
+    disable: isTimeIntervalDisabled(interval),
  }));

  return {
--- a/public/app/features/alerting/unified/components/mute-timings/MuteTimingTimeInterval.tsx
+++ b/public/app/features/alerting/unified/components/mute-timings/MuteTimingTimeInterval.tsx
@ -4,8 +4,9 @@ import React, { useEffect, useState } from 'react';
 import { useFieldArray, useFormContext } from 'react-hook-form';

 import { GrafanaTheme2 } from '@grafana/data';
-import { Button, Field, FieldSet, Icon, Input, useStyles2, Stack } from '@grafana/ui';
+import { Button, Field, FieldSet, Icon, InlineSwitch, Input, Stack, useStyles2 } from '@grafana/ui';

+import { useAlertmanager } from '../../state/AlertmanagerContext';
 import { MuteTimingFields } from '../../types/mute-timing-form';
 import { DAYS_OF_THE_WEEK, defaultTimeInterval, MONTHS, validateArrayField } from '../../utils/mute-timings';

@ -22,6 +23,7 @@ export const MuteTimingTimeInterval = () => {
  } = useFieldArray({
    name: 'time_intervals',
  });
+  const { isGrafanaAlertmanager } = useAlertmanager();

  return (
    <FieldSet label="Time intervals">
@ -131,15 +133,30 @@ export const MuteTimingTimeInterval = () => {
                    data-testid="mute-timing-years"
                  />
                </Field>
-                <Button
-                  type="button"
-                  variant="destructive"
-                  fill="outline"
-                  icon="trash-alt"
-                  onClick={() => removeTimeInterval(timeIntervalIndex)}
-                >
-                  Remove time interval
-                </Button>
+                <Stack direction="row" gap={2}>
+                  <Button
+                    type="button"
+                    variant="destructive"
+                    fill="outline"
+                    icon="trash-alt"
+                    onClick={() => removeTimeInterval(timeIntervalIndex)}
+                  >
+                    Remove time interval
+                  </Button>
+                  {/* 
+                    This switch is only available for Grafana Alertmanager, as for now, Grafana alert manager doesn't support this feature
+                    It hanldes empty list as undefined making impossible the use of an empty list for disabling time interval
+                  */}
+                  {!isGrafanaAlertmanager && (
+                    <InlineSwitch
+                      id={`time_intervals.${timeIntervalIndex}.disable`}
+                      label="Disable"
+                      showLabel
+                      transparent
+                      {...register(`time_intervals.${timeIntervalIndex}.disable`)}
+                    />
+                  )}
+                </Stack>
              </div>
            );
          })}
--- a/public/app/features/alerting/unified/components/mute-timings/MuteTimingTimeRange.tsx
+++ b/public/app/features/alerting/unified/components/mute-timings/MuteTimingTimeRange.tsx
@ -3,9 +3,10 @@ import React from 'react';
 import { useFieldArray, useFormContext } from 'react-hook-form';

 import { GrafanaTheme2 } from '@grafana/data';
-import { Button, Field, Icon, IconButton, InlineField, InlineFieldRow, Input, useStyles2 } from '@grafana/ui';
+import { Button, Field, Icon, IconButton, InlineField, InlineFieldRow, Input, Tooltip, useStyles2 } from '@grafana/ui';

 import { MuteTimingFields } from '../../types/mute-timing-form';
+import ConditionalWrap from '../ConditionalWrap';

 import { isValidStartAndEndTime, isvalidTimeFormat } from './util';

@ -17,7 +18,8 @@ const INVALID_FORMAT_MESSAGE = 'Times must be between 00:00 and 24:00 UTC';

 export const MuteTimingTimeRange = ({ intervalIndex }: Props) => {
  const styles = useStyles2(getStyles);
-  const { register, formState, getValues } = useFormContext<MuteTimingFields>();
+  const { register, formState, getValues, watch } = useFormContext<MuteTimingFields>();
+  const isDisabled = watch(`time_intervals.${intervalIndex}.disable`);

  const {
    fields: timeRanges,
@ -81,6 +83,7 @@ export const MuteTimingTimeRange = ({ intervalIndex }: Props) => {
                      })}
                      className={styles.timeRangeInput}
                      maxLength={5}
+                      readOnly={isDisabled}
                      suffix={<Icon name="clock-nine" />}
                      // @ts-ignore react-hook-form doesn't handle nested field arrays well
                      defaultValue={timeRange.start_time}
@ -112,6 +115,7 @@ export const MuteTimingTimeRange = ({ intervalIndex }: Props) => {
                      })}
                      className={styles.timeRangeInput}
                      maxLength={5}
+                      readOnly={isDisabled}
                      suffix={<Icon name="clock-nine" />}
                      // @ts-ignore react-hook-form doesn't handle nested field arrays well
                      defaultValue={timeRange.end_time}
@ -135,15 +139,25 @@ export const MuteTimingTimeRange = ({ intervalIndex }: Props) => {
          })}
        </>
      </Field>
-      <Button
-        className={styles.addTimeRange}
-        variant="secondary"
-        type="button"
-        icon="plus"
-        onClick={() => addTimeRange({ start_time: '', end_time: '' })}
+      <ConditionalWrap
+        shouldWrap={isDisabled}
+        wrap={(children) => (
+          <Tooltip content="This time interval is disabled" placement="right-start">
+            {children}
+          </Tooltip>
+        )}
      >
-        Add another time range
-      </Button>
+        <Button
+          className={styles.addTimeRange}
+          variant="secondary"
+          type="button"
+          icon="plus"
+          disabled={isDisabled}
+          onClick={() => addTimeRange({ start_time: '', end_time: '' })}
+        >
+          Add another time range
+        </Button>
+      </ConditionalWrap>
    </div>
  );
 };
--- a/public/app/features/alerting/unified/components/mute-timings/MuteTimingsTable.tsx
+++ b/public/app/features/alerting/unified/components/mute-timings/MuteTimingsTable.tsx
@ -3,7 +3,7 @@ import React, { useCallback, useMemo, useState } from 'react';
 import { useToggle } from 'react-use';

 import { GrafanaTheme2 } from '@grafana/data';
-import { Button, ConfirmModal, IconButton, Link, LinkButton, Menu, Stack, useStyles2 } from '@grafana/ui';
+import { Badge, Button, ConfirmModal, IconButton, Link, LinkButton, Menu, Stack, useStyles2 } from '@grafana/ui';
 import { MuteTimeInterval } from 'app/plugins/datasource/alertmanager/types';
 import { useDispatch } from 'app/types/store';

@ -11,7 +11,9 @@ import { Authorize } from '../../components/Authorize';
 import { AlertmanagerAction, useAlertmanagerAbilities, useAlertmanagerAbility } from '../../hooks/useAbilities';
 import { useAlertmanagerConfig } from '../../hooks/useAlertmanagerConfig';
 import { deleteMuteTimingAction } from '../../state/actions';
+import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource';
 import { makeAMLink } from '../../utils/misc';
+import { isDisabled } from '../../utils/mute-timings';
 import { DynamicTable, DynamicTableColumnProps, DynamicTableItemProps } from '../DynamicTable';
 import { EmptyAreaWithCTA } from '../EmptyAreaWithCTA';
 import { ProvisioningBadge } from '../Provisioning';
@ -175,9 +177,8 @@ function useColumns(
  ]);
  const showActions = !hideActions && (allowedToEdit || allowedToDelete);

-  // const [ExportDrawer, openExportDrawer] = useExportMuteTiming();
-  // const [_, openExportDrawer] = useExportMuteTiming();
  const [exportSupported, exportAllowed] = useAlertmanagerAbility(AlertmanagerAction.ExportMuteTimings);
+  const styles = useStyles2(getStyles);

  return useMemo((): Array<DynamicTableColumnProps<MuteTimeInterval>> => {
    const columns: Array<DynamicTableColumnProps<MuteTimeInterval>> = [
@ -204,43 +205,18 @@ function useColumns(
    if (showActions) {
      columns.push({
        id: 'actions',
-        label: 'Actions',
+        label: '',
        renderCell: function renderActions({ data }) {
-          if (data.provenance) {
-            return (
-              <div>
-                <Link
-                  href={makeAMLink(`/alerting/routes/mute-timing/edit`, alertManagerSourceName, {
-                    muteName: data.name,
-                  })}
-                >
-                  <IconButton name="file-alt" tooltip="View mute timing" />
-                </Link>
-              </div>
-            );
-          }
          return (
-            <div>
-              <Authorize actions={[AlertmanagerAction.UpdateMuteTiming]}>
-                <Link
-                  href={makeAMLink(`/alerting/routes/mute-timing/edit`, alertManagerSourceName, {
-                    muteName: data.name,
-                  })}
-                >
-                  <IconButton name="edit" tooltip="Edit mute timing" />
-                </Link>
-              </Authorize>
-              <Authorize actions={[AlertmanagerAction.DeleteMuteTiming]}>
-                <IconButton
-                  name="trash-alt"
-                  tooltip="Delete mute timing"
-                  onClick={() => setMuteTimingName(data.name)}
-                />
-              </Authorize>
-            </div>
+            <ActionsAndBadge
+              muteTiming={data}
+              alertManagerSourceName={alertManagerSourceName}
+              setMuteTimingName={setMuteTimingName}
+            />
          );
        },
-        size: '80px',
+        size: '150px',
+        className: styles.actionsColumn,
      });
    }
    if (exportSupported) {
@ -265,7 +241,62 @@ function useColumns(
      });
    }
    return columns;
-  }, [alertManagerSourceName, setMuteTimingName, showActions, exportSupported, exportAllowed, openExportDrawer]);
+  }, [
+    alertManagerSourceName,
+    setMuteTimingName,
+    showActions,
+    exportSupported,
+    exportAllowed,
+    openExportDrawer,
+    styles.actionsColumn,
+  ]);
+}
+
+interface ActionsAndBadgeProps {
+  muteTiming: MuteTimeInterval;
+  alertManagerSourceName: string;
+  setMuteTimingName: (name: string) => void;
+}
+
+function ActionsAndBadge({ muteTiming, alertManagerSourceName, setMuteTimingName }: ActionsAndBadgeProps) {
+  const styles = useStyles2(getStyles);
+  const isGrafanaDataSource = alertManagerSourceName === GRAFANA_RULES_SOURCE_NAME;
+
+  if (muteTiming.provenance) {
+    return (
+      <Stack direction="row" alignItems="center" justifyContent="flex-end">
+        {isDisabled(muteTiming) && !isGrafanaDataSource && (
+          <Badge text="Disabled" color="orange" className={styles.disabledBadge} />
+        )}
+        <Link
+          href={makeAMLink(`/alerting/routes/mute-timing/edit`, alertManagerSourceName, {
+            muteName: muteTiming.name,
+          })}
+        >
+          <IconButton name="file-alt" tooltip="View mute timing" />
+        </Link>
+      </Stack>
+    );
+  }
+  return (
+    <Stack direction="row" alignItems="center" justifyContent="flex-end">
+      {isDisabled(muteTiming) && !isGrafanaDataSource && (
+        <Badge text="Disabled" color="orange" className={styles.disabledBadge} />
+      )}
+      <Authorize actions={[AlertmanagerAction.UpdateMuteTiming]}>
+        <Link
+          href={makeAMLink(`/alerting/routes/mute-timing/edit`, alertManagerSourceName, {
+            muteName: muteTiming.name,
+          })}
+        >
+          <IconButton name="edit" tooltip="Edit mute timing" className={styles.editButton} />
+        </Link>
+      </Authorize>
+      <Authorize actions={[AlertmanagerAction.DeleteMuteTiming]}>
+        <IconButton name="trash-alt" tooltip="Delete mute timing" onClick={() => setMuteTimingName(muteTiming.name)} />
+      </Authorize>
+    </Stack>
+  );
 }

 const getStyles = (theme: GrafanaTheme2) => ({
@ -277,4 +308,13 @@ const getStyles = (theme: GrafanaTheme2) => ({
    margin-bottom: ${theme.spacing(2)};
    align-self: flex-end;
  `,
+  disabledBadge: css({
+    height: 'fit-content',
+  }),
+  editButton: css({
+    display: 'flex',
+  }),
+  actionsColumn: css({
+    justifyContent: 'flex-end',
+  }),
 });
--- a/public/app/features/alerting/unified/types/mute-timing-form.ts
+++ b/public/app/features/alerting/unified/types/mute-timing-form.ts
@ -6,10 +6,11 @@ export type MuteTimingFields = {
 };

 export type MuteTimingIntervalFields = {
-  times: TimeRange[];
-  weekdays: string;
-  days_of_month: string;
-  months: string;
-  years: string;
+  times?: TimeRange[];
+  weekdays?: string;
+  days_of_month?: string;
+  months?: string;
+  years?: string;
  location?: string;
+  disable: boolean;
 };
--- a/public/app/features/alerting/unified/utils/mute-timings.ts
+++ b/public/app/features/alerting/unified/utils/mute-timings.ts
@ -1,6 +1,6 @@
-import { omitBy, isUndefined } from 'lodash';
+import { isUndefined, omitBy } from 'lodash';

-import { MuteTimeInterval, TimeInterval } from 'app/plugins/datasource/alertmanager/types';
+import { MuteTimeInterval, TimeInterval, TimeRange } from 'app/plugins/datasource/alertmanager/types';

 import { MuteTimingFields, MuteTimingIntervalFields } from '../types/mute-timing-form';

@ -28,9 +28,14 @@ export const defaultTimeInterval: MuteTimingIntervalFields = {
  months: '',
  years: '',
  location: '',
+  disable: false,
 };

-export const validateArrayField = (value: string, validateValue: (input: string) => boolean, invalidText: string) => {
+export const validateArrayField = (
+  value: string | undefined,
+  validateValue: (input: string) => boolean,
+  invalidText: string
+) => {
  if (value) {
    return (
      value
@ -43,15 +48,15 @@ export const validateArrayField = (value: string, validateValue: (input: string)
  }
 };

-const convertStringToArray = (str: string) => {
+const convertStringToArray = (str?: string) => {
  return str ? str.split(',').map((s) => s.trim()) : undefined;
 };

 export const createMuteTiming = (fields: MuteTimingFields): MuteTimeInterval => {
  const timeIntervals: TimeInterval[] = fields.time_intervals.map(
-    ({ times, weekdays, days_of_month, months, years, location }) => {
+    ({ times, weekdays, days_of_month, months, years, location, disable }) => {
      const interval = {
-        times: times.filter(({ start_time, end_time }) => !!start_time && !!end_time),
+        times: convertTimesToDto(times, disable),
        weekdays: convertStringToArray(weekdays)?.map((v) => v.toLowerCase()),
        days_of_month: convertStringToArray(days_of_month),
        months: convertStringToArray(months),
@ -68,3 +73,47 @@ export const createMuteTiming = (fields: MuteTimingFields): MuteTimeInterval =>
    time_intervals: timeIntervals,
  };
 };
+
+/*
+ * Convert times from form to dto, if disable is true, then return an empty array as times
+ If the times array is empty and disable is false, then return undefined
+ * @param muteTimeInterval
+ * @returns MuteTimingFields
+ *
+ */
+function convertTimesToDto(times: TimeRange[] | undefined, disable: boolean) {
+  if (disable) {
+    return [];
+  }
+  const timesToReturn = times?.filter(({ start_time, end_time }) => !!start_time && !!end_time);
+  return timesToReturn?.length ? timesToReturn : undefined;
+}
+
+/*
+ * Get disable field from dto, if any of the lists is an empty array, then the disable field is true
+ * @param muteTimeInterval
+ * @returns MuteTimingFields
+ *
+ */
+
+export function isTimeIntervalDisabled(intervals: TimeInterval): boolean {
+  if (
+    intervals.times?.length === 0 ||
+    intervals.weekdays?.length === 0 ||
+    intervals.days_of_month?.length === 0 ||
+    intervals.months?.length === 0 ||
+    intervals.years?.length === 0
+  ) {
+    return true;
+  }
+  return false;
+}
+
+/*
+   Return true if all the time intervals are disabled
+  * @param muteTimeInterval
+  * @returns MuteTimingFields
+  * */
+export function isDisabled(muteTiming: MuteTimeInterval) {
+  return muteTiming.time_intervals.every((timeInterval) => isTimeIntervalDisabled(timeInterval));
+}