Alerting: Disable alerting incompatible panels in the dashboard picker (#65341)

* Hide dashboard hint when dashboard already selected * Disable panels of types other than graph and timeseries * Add a test checking disabled panels * Make all panels selectable * Fix tests
2024-11-25 18:30:41 -06:00 · 2023-04-03 10:01:30 +02:00 · 2023-04-03 10:01:30 +02:00 · bde77e4f79
commit bde77e4f79
parent bf4906ed34
2 changed files with 95 additions and 18 deletions
--- a/public/app/features/alerting/unified/components/rule-editor/AnnotationsField.test.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/AnnotationsField.test.tsx
@ -1,4 +1,4 @@
-import { findByText, findByTitle, render } from '@testing-library/react';
+import { findByRole, findByText, findByTitle, getByTestId, render } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { rest } from 'msw';
 import { setupServer } from 'msw/node';
@ -107,8 +107,8 @@ describe('AnnotationsField', function () {
          title: 'My dashboard',
          uid: 'dash-test-uid',
          panels: [
-            { id: 1, title: 'First panel' },
-            { id: 2, title: 'Second panel' },
+            { id: 1, title: 'First panel', type: 'timeseries' },
+            { id: 2, title: 'Second panel', type: 'timeseries' },
          ],
        })
      );
@ -137,8 +137,8 @@ describe('AnnotationsField', function () {
          title: 'My dashboard',
          uid: 'dash-test-uid',
          panels: [
-            { id: 1, title: 'First panel' },
-            { id: 2, title: 'Second panel' },
+            { id: 1, title: 'First panel', type: 'graph' },
+            { id: 2, title: 'Second panel', type: 'graph' },
          ],
        })
      );
@ -171,7 +171,7 @@ describe('AnnotationsField', function () {

    // this test _should_ work in theory but something is stopping the 'onClick' function on the dashboard item
    // to trigger "handleDashboardChange" – skipping it for now but has been manually tested.
-    it.skip('should update existing dashboard and panel identifies', async function () {
+    it('should update existing dashboard and panel identifies', async function () {
      mockSearchApiResponse(server, [
        mockDashboardSearchItem({ title: 'My dashboard', uid: 'dash-test-uid', type: DashboardSearchItemType.DashDB }),
        mockDashboardSearchItem({
@ -186,8 +186,8 @@ describe('AnnotationsField', function () {
          title: 'My dashboard',
          uid: 'dash-test-uid',
          panels: [
-            { id: 1, title: 'First panel' },
-            { id: 2, title: 'Second panel' },
+            { id: 1, title: 'First panel', type: 'timeseries' },
+            { id: 2, title: 'Second panel', type: 'timeseries' },
          ],
        })
      );
@ -195,7 +195,7 @@ describe('AnnotationsField', function () {
        mockDashboardDto({
          title: 'My other dashboard',
          uid: 'dash-other-uid',
-          panels: [{ id: 3, title: 'Third panel' }],
+          panels: [{ id: 3, title: 'Third panel', type: 'timeseries' }],
        })
      );

@ -216,10 +216,12 @@ describe('AnnotationsField', function () {
      expect(annotationValueElements[0]).toHaveTextContent('dash-test-uid');
      expect(annotationValueElements[1]).toHaveTextContent('1');

+      const { confirmButton, dialog } = ui.dashboardPicker;
+
      await user.click(ui.setDashboardButton.get());
-      await user.click(await findByTitle(ui.dashboardPicker.dialog.get(), 'My other dashboard'));
-      await user.click(await findByText(ui.dashboardPicker.dialog.get(), 'Third panel'));
-      await user.click(ui.dashboardPicker.confirmButton.get());
+      await user.click(await findByRole(dialog.get(), 'button', { name: /My other dashboard/ }));
+      await user.click(await findByRole(dialog.get(), 'button', { name: /Third panel/ }));
+      await user.click(confirmButton.get());

      expect(ui.dashboardPicker.dialog.query()).not.toBeInTheDocument();

@ -235,6 +237,36 @@ describe('AnnotationsField', function () {
      expect(annotationKeyElements[1]).toHaveTextContent('Panel ID');
      expect(annotationValueElements[1]).toHaveTextContent('3');
    });
+
+    it('should render warning icon for panels of type other than graph and timeseries', async function () {
+      mockSearchApiResponse(server, [
+        mockDashboardSearchItem({ title: 'My dashboard', uid: 'dash-test-uid', type: DashboardSearchItemType.DashDB }),
+      ]);
+
+      mockGetDashboardResponse(
+        mockDashboardDto({
+          title: 'My dashboard',
+          uid: 'dash-test-uid',
+          panels: [
+            { id: 1, title: 'First panel', type: 'bar' },
+            { id: 2, title: 'Second panel', type: 'graph' },
+          ],
+        })
+      );
+
+      const user = userEvent.setup();
+
+      render(<FormWrapper formValues={{ annotations: [] }} />);
+
+      const { dialog } = ui.dashboardPicker;
+
+      await user.click(ui.setDashboardButton.get());
+      await user.click(await findByTitle(dialog.get(), 'My dashboard'));
+
+      const warnedPanel = await findByRole(dialog.get(), 'button', { name: /First panel/ });
+
+      expect(getByTestId(warnedPanel, 'warning-icon')).toBeInTheDocument();
+    });
  });
 });

--- a/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx
+++ b/public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx
@ -14,6 +14,7 @@ import {
  Button,
  Alert,
  clearButtonStyles,
+  Tooltip,
 } from '@grafana/ui';

 import { dashboardApi } from '../../api/dashboardApi';
@ -21,6 +22,7 @@ import { dashboardApi } from '../../api/dashboardApi';
 export interface PanelDTO {
  id: number;
  title?: string;
+  type: string;
 }

 function panelSort(a: PanelDTO, b: PanelDTO) {
@ -71,7 +73,7 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis

  const filteredPanels =
    dashboardResult?.dashboard?.panels
-      ?.filter((panel): panel is PanelDTO => typeof panel.id === 'number')
+      ?.filter((panel): panel is PanelDTO => typeof panel.id === 'number' && typeof panel.type === 'string')
      ?.filter((panel) => panel.title?.toLowerCase().includes(panelFilter.toLowerCase()))
      .sort(panelSort) ?? [];

@ -115,7 +117,7 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis
        className={cx(styles.rowButton, { [styles.rowOdd]: index % 2 === 1, [styles.rowSelected]: isSelected })}
        onClick={() => handleDashboardChange(dashboard.uid)}
      >
-        <div className={styles.dashboardTitle}>{dashboard.title}</div>
+        <div className={cx(styles.dashboardTitle, styles.rowButtonTitle)}>{dashboard.title}</div>
        <div className={styles.dashboardFolder}>
          <Icon name="folder" /> {dashboard.folderTitle ?? 'General'}
        </div>
@ -125,16 +127,28 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis

  const PanelRow = ({ index, style }: { index: number; style: CSSProperties }) => {
    const panel = filteredPanels[index];
+    const panelTitle = panel.title || '<No title>';
    const isSelected = selectedPanelId === panel.id.toString();
+    const isAlertingCompatible = panel.type === 'graph' || panel.type === 'timeseries';

    return (
      <button
        type="button"
        style={style}
-        className={cx(styles.rowButton, { [styles.rowOdd]: index % 2 === 1, [styles.rowSelected]: isSelected })}
+        className={cx(styles.rowButton, styles.panelButton, {
+          [styles.rowOdd]: index % 2 === 1,
+          [styles.rowSelected]: isSelected,
+        })}
        onClick={() => setSelectedPanelId(panel.id.toString())}
      >
-        {panel.title || '<No title>'}
+        <div className={styles.rowButtonTitle} title={panelTitle}>
+          {panelTitle}
+        </div>
+        {!isAlertingCompatible && (
+          <Tooltip content="Alert tab will be disabled for this panel. It is only supported on graph and timeseries panels">
+            <Icon name="exclamation-triangle" className={styles.warnIcon} data-testid="warning-icon" />
+          </Tooltip>
+        )}
      </button>
    );
  };
@ -195,12 +209,16 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis
        </div>

        <div className={styles.column}>
-          {!dashboardUid && !isDashboardFetching && <div>Select a dashboard to get a list of available panels</div>}
+          {!selectedDashboardUid && !isDashboardFetching && (
+            <div className={styles.selectDashboardPlaceholder}>
+              <div>Select a dashboard to get a list of available panels</div>
+            </div>
+          )}
          {isDashboardFetching && (
            <LoadingPlaceholder text="Loading dashboard..." className={styles.loadingPlaceholder} />
          )}

-          {!isDashboardFetching && (
+          {selectedDashboardUid && !isDashboardFetching && (
            <AutoSizer>
              {({ width, height }) => (
                <FixedSizeList itemSize={32} height={height} width={width} itemCount={filteredPanels.length}>
@ -269,6 +287,15 @@ const getPickerStyles = (theme: GrafanaTheme2) => {
      white-space: nowrap;
      cursor: pointer;
      border: 2px solid transparent;
+
+      &:disabled {
+        cursor: not-allowed;
+        color: ${theme.colors.text.disabled};
+      }
+    `,
+    rowButtonTitle: css`
+      text-overflow: ellipsis;
+      overflow: hidden;
    `,
    rowSelected: css`
      border-color: ${theme.colors.primary.border};
@ -276,12 +303,27 @@ const getPickerStyles = (theme: GrafanaTheme2) => {
    rowOdd: css`
      background-color: ${theme.colors.background.secondary};
    `,
+    panelButton: css`
+      display: flex;
+      gap: ${theme.spacing(1)};
+      justify-content: space-between;
+      align-items: center;
+    `,
    loadingPlaceholder: css`
      height: 100%;
      display: flex;
      justify-content: center;
      align-items: center;
    `,
+    selectDashboardPlaceholder: css`
+      width: 100%;
+      height: 100%;
+      display: flex;
+      flex-direction: column;
+      justify-content: center;
+      text-align: center;
+      font-weight: ${theme.typography.fontWeightBold};
+    `,
    modal: css`
      height: 100%;
    `,
@ -293,5 +335,8 @@ const getPickerStyles = (theme: GrafanaTheme2) => {
    modalAlert: css`
      flex-grow: 0;
    `,
+    warnIcon: css`
+      fill: ${theme.colors.warning.main};
+    `,
  };
 };