Annotations: Adds loading indicators and cancellation (#33404)

public/app/features/dashboard/components/SubMenu/AnnotationPicker.tsx

@@ -0,0 +1,72 @@
+import { AnnotationQuery, EventBus, GrafanaThemeV2 } from '@grafana/data';
+import React, { useEffect, useState } from 'react';
+import { getDashboardQueryRunner } from '../../../query/state/DashboardQueryRunner/DashboardQueryRunner';
+import { AnnotationQueryFinished, AnnotationQueryStarted } from '../../../../types/events';
+import { InlineField, InlineSwitch, useStyles2 } from '@grafana/ui';
+import { LoadingIndicator } from '@grafana/ui/src/components/PanelChrome/LoadingIndicator';
+import { css } from '@emotion/css';
+
+export interface AnnotationPickerProps {
+  events: EventBus;
+  annotation: AnnotationQuery;
+  onEnabledChanged: (annotation: AnnotationQuery) => void;
+}
+
+export const AnnotationPicker = ({ annotation, events, onEnabledChanged }: AnnotationPickerProps): JSX.Element => {
+  const [loading, setLoading] = useState(false);
+  const styles = useStyles2(getStyles);
+  const onCancel = () => getDashboardQueryRunner().cancel(annotation);
+
+  useEffect(() => {
+    const started = events.subscribe(AnnotationQueryStarted, (event) => {
+      if (event.payload === annotation) {
+        setLoading(true);
+      }
+    });
+    const stopped = events.subscribe(AnnotationQueryFinished, (event) => {
+      if (event.payload === annotation) {
+        setLoading(false);
+      }
+    });
+
+    return () => {
+      started.unsubscribe();
+      stopped.unsubscribe();
+    };
+  });
+
+  return (
+    <div key={annotation.name} className={styles.annotation}>
+      <InlineField label={annotation.name} disabled={loading}>
+        <>
+          <InlineSwitch value={annotation.enable} onChange={() => onEnabledChanged(annotation)} disabled={loading} />
+          <div className={styles.indicator}>
+            <LoadingIndicator loading={loading} onCancel={onCancel} />
+          </div>
+        </>
+      </InlineField>
+    </div>
+  );
+};
+
+function getStyles(theme: GrafanaThemeV2) {
+  return {
+    annotation: css`
+      display: inline-block;
+      margin-right: ${theme.spacing(1)};
+
+      .fa-caret-down {
+        font-size: 75%;
+        padding-left: ${theme.spacing(1)};
+      }
+
+      .gf-form-inline .gf-form {
+        margin-bottom: 0;
+      }
+    `,
+    indicator: css`
+      align-self: center;
+      padding: 0 ${theme.spacing(0.5)};
+    `,
+  };
+}

public/app/features/dashboard/components/SubMenu/Annotations.tsx

@@ -1,14 +1,15 @@
 import React, { FunctionComponent, useEffect, useState } from 'react';
-import { AnnotationQuery } from '@grafana/data';
+import { AnnotationQuery, EventBus } from '@grafana/data';
-import { InlineField, InlineSwitch } from '@grafana/ui';
+import { AnnotationPicker } from './AnnotationPicker';
 
 interface Props {
+  events: EventBus;
   annotations: AnnotationQuery[];
   onAnnotationChanged: (annotation: any) => void;
 }
 
-export const Annotations: FunctionComponent<Props> = ({ annotations, onAnnotationChanged }) => {
+export const Annotations: FunctionComponent<Props> = ({ annotations, onAnnotationChanged, events }) => {
-  const [visibleAnnotations, setVisibleAnnotations] = useState<any>([]);
+  const [visibleAnnotations, setVisibleAnnotations] = useState<AnnotationQuery[]>([]);
   useEffect(() => {
     setVisibleAnnotations(annotations.filter((annotation) => annotation.hide !== true));
   }, [annotations]);
@@ -19,15 +20,14 @@ export const Annotations: FunctionComponent<Props> = ({ annotations, onAnnotatio
 
   return (
     <>
-      {visibleAnnotations.map((annotation: any) => {
+      {visibleAnnotations.map((annotation) => (
-        return (
+        <AnnotationPicker
-          <div key={annotation.name} className={'submenu-item'}>
+          events={events}
-            <InlineField label={annotation.name}>
+          annotation={annotation}
-              <InlineSwitch value={annotation.enable} onChange={() => onAnnotationChanged(annotation)} />
+          onEnabledChanged={onAnnotationChanged}
-            </InlineField>
+          key={annotation.name}
-          </div>
+        />
-        );
+      ))}
-      })}
     </>
   );
 };

public/app/features/dashboard/components/SubMenu/SubMenu.tsx

@@ -48,7 +48,11 @@ class SubMenuUnConnected extends PureComponent<Props> {
     return (
       <div className="submenu-controls">
         <SubMenuItems variables={variables} />
-        <Annotations annotations={annotations} onAnnotationChanged={this.onAnnotationStateChanged} />
+        <Annotations
+          annotations={annotations}
+          onAnnotationChanged={this.onAnnotationStateChanged}
+          events={dashboard.events}
+        />
         <div className="gf-form gf-form--grow" />
         {dashboard && <DashboardLinks dashboard={dashboard} links={links} />}
         <div className="clearfix" />

public/app/features/dashboard/state/initDashboard.test.ts

@@ -1,10 +1,12 @@
+import { Subject } from 'rxjs';
 import configureMockStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
+import { locationService, setEchoSrv } from '@grafana/runtime';
+
 import { initDashboard, InitDashboardArgs } from './initDashboard';
 import { DashboardInitPhase, DashboardRoutes } from 'app/types';
 import { getBackendSrv } from 'app/core/services/backend_srv';
 import { dashboardInitCompleted, dashboardInitFetching, dashboardInitServices } from './reducers';
-import { locationService, setEchoSrv } from '@grafana/runtime';
 import { Echo } from '../../../core/services/echo/Echo';
 import { variableAdapters } from 'app/features/variables/adapters';
 import { createConstantVariableAdapter } from 'app/features/variables/constant/adapter';
@@ -90,6 +92,7 @@ function describeInitScenario(description: string, scenarioFn: ScenarioFn) {
       getResult: emptyResult,
       run: jest.fn(),
       cancel: () => undefined,
+      cancellations: () => new Subject<any>(),
       destroy: () => undefined,
     }));
 

public/app/features/query/state/DashboardQueryRunner/AnnotationsWorker.test.ts

@@ -1,13 +1,27 @@
-import { throwError } from 'rxjs';
+import { Subject, throwError } from 'rxjs';
 import { setDataSourceSrv } from '@grafana/runtime';
 
 import { AnnotationsWorker } from './AnnotationsWorker';
 import * as annotationsSrv from '../../../annotations/annotations_srv';
 import { getDefaultOptions, LEGACY_DS_NAME, NEXT_GEN_DS_NAME, toAsyncOfResult } from './testHelpers';
 import { silenceConsoleOutput } from '../../../../../test/core/utils/silenceConsoleOutput';
+import { createDashboardQueryRunner, setDashboardQueryRunnerFactory } from './DashboardQueryRunner';
+import { emptyResult } from './utils';
+import { DashboardQueryRunnerOptions, DashboardQueryRunnerWorkerResult } from './types';
+import { AnnotationQuery } from '@grafana/data';
+import { delay } from 'rxjs/operators';
 
 function getTestContext() {
   jest.clearAllMocks();
+  const cancellations = new Subject<AnnotationQuery>();
+  setDashboardQueryRunnerFactory(() => ({
+    getResult: emptyResult,
+    run: () => undefined,
+    cancel: () => undefined,
+    cancellations: () => cancellations,
+    destroy: () => undefined,
+  }));
+  createDashboardQueryRunner({} as any);
   const executeAnnotationQueryMock = jest
     .spyOn(annotationsSrv, 'executeAnnotationQuery')
     .mockReturnValue(toAsyncOfResult({ events: [{ id: 'NextGen' }] }));
@@ -32,7 +46,28 @@ function getTestContext() {
   setDataSourceSrv(dataSourceSrvMock);
   const options = getDefaultOptions();
 
-  return { options, annotationQueryMock, executeAnnotationQueryMock };
+  return { options, annotationQueryMock, executeAnnotationQueryMock, cancellations };
+}
+
+function expectOnResults(args: {
+  worker: AnnotationsWorker;
+  options: DashboardQueryRunnerOptions;
+  done: jest.DoneCallback;
+  expect: (results: DashboardQueryRunnerWorkerResult) => void;
+}) {
+  const { worker, done, options, expect: expectCallback } = args;
+  const subscription = worker.work(options).subscribe({
+    next: (value) => {
+      try {
+        expectCallback(value);
+        subscription.unsubscribe();
+        done();
+      } catch (err) {
+        subscription.unsubscribe();
+        done.fail(err);
+      }
+    },
+  });
 }
 
 describe('AnnotationsWorker', () => {
@@ -142,17 +177,21 @@ describe('AnnotationsWorker', () => {
         expect(annotationQueryMock).toHaveBeenCalledTimes(1);
       });
     });
+  });
 
-    describe('when run is called with correct props and nextgen worker fails', () => {
+  describe('when run is called with correct props and a worker is cancelled', () => {
-      silenceConsoleOutput();
+    it('then it should return the correct results', (done) => {
-      it('then it should return the correct results', async () => {
+      const { options, executeAnnotationQueryMock, annotationQueryMock, cancellations } = getTestContext();
-        const { options, executeAnnotationQueryMock, annotationQueryMock } = getTestContext();
+      executeAnnotationQueryMock.mockReturnValueOnce(
-        executeAnnotationQueryMock.mockReturnValue(throwError({ message: 'An error' }));
+        toAsyncOfResult({ events: [{ id: 'NextGen' }] }).pipe(delay(10000))
+      );
 
-        await expect(worker.work(options)).toEmitValuesWith((received) => {
+      expectOnResults({
-          expect(received).toHaveLength(1);
+        worker,
-          const result = received[0];
+        options,
-          expect(result).toEqual({
+        done,
+        expect: (results) => {
+          expect(results).toEqual({
             alertStates: [],
             annotations: [
               {
@@ -173,24 +212,63 @@ describe('AnnotationsWorker', () => {
           });
           expect(executeAnnotationQueryMock).toHaveBeenCalledTimes(1);
           expect(annotationQueryMock).toHaveBeenCalledTimes(1);
+        },
+      });
+
+      setTimeout(() => {
+        // call to async needs to be async or the cancellation will be called before any of the runners have started
+        cancellations.next(options.dashboard.annotations.list[1]);
+      }, 100);
+    });
+  });
+
+  describe('when run is called with correct props and nextgen worker fails', () => {
+    silenceConsoleOutput();
+    it('then it should return the correct results', async () => {
+      const { options, executeAnnotationQueryMock, annotationQueryMock } = getTestContext();
+      executeAnnotationQueryMock.mockReturnValue(throwError({ message: 'An error' }));
+
+      await expect(worker.work(options)).toEmitValuesWith((received) => {
+        expect(received).toHaveLength(1);
+        const result = received[0];
+        expect(result).toEqual({
+          alertStates: [],
+          annotations: [
+            {
+              id: 'Legacy',
+              source: {
+                enable: true,
+                hide: false,
+                name: 'Test',
+                iconColor: 'pink',
+                snapshotData: undefined,
+                datasource: 'Legacy',
+              },
+              color: 'pink',
+              type: 'Test',
+              isRegion: false,
+            },
+          ],
         });
+        expect(executeAnnotationQueryMock).toHaveBeenCalledTimes(1);
+        expect(annotationQueryMock).toHaveBeenCalledTimes(1);
       });
     });
+  });
 
-    describe('when run is called with correct props and both workers fail', () => {
+  describe('when run is called with correct props and both workers fail', () => {
-      silenceConsoleOutput();
+    silenceConsoleOutput();
-      it('then it should return the correct results', async () => {
+    it('then it should return the correct results', async () => {
-        const { options, executeAnnotationQueryMock, annotationQueryMock } = getTestContext();
+      const { options, executeAnnotationQueryMock, annotationQueryMock } = getTestContext();
-        annotationQueryMock.mockRejectedValue({ message: 'Some error' });
+      annotationQueryMock.mockRejectedValue({ message: 'Some error' });
-        executeAnnotationQueryMock.mockReturnValue(throwError({ message: 'An error' }));
+      executeAnnotationQueryMock.mockReturnValue(throwError({ message: 'An error' }));
 
-        await expect(worker.work(options)).toEmitValuesWith((received) => {
+      await expect(worker.work(options)).toEmitValuesWith((received) => {
-          expect(received).toHaveLength(1);
+        expect(received).toHaveLength(1);
-          const result = received[0];
+        const result = received[0];
-          expect(result).toEqual({ alertStates: [], annotations: [] });
+        expect(result).toEqual({ alertStates: [], annotations: [] });
-          expect(executeAnnotationQueryMock).toHaveBeenCalledTimes(1);
+        expect(executeAnnotationQueryMock).toHaveBeenCalledTimes(1);
-          expect(annotationQueryMock).toHaveBeenCalledTimes(1);
+        expect(annotationQueryMock).toHaveBeenCalledTimes(1);
-        });
       });
     });
   });

public/app/features/query/state/DashboardQueryRunner/AnnotationsWorker.ts

@@ -1,6 +1,6 @@
 import { cloneDeep } from 'lodash';
 import { from, merge, Observable, of } from 'rxjs';
-import { map, mergeAll, mergeMap, reduce } from 'rxjs/operators';
+import { filter, finalize, map, mergeAll, mergeMap, reduce, takeUntil } from 'rxjs/operators';
 import { getDataSourceSrv } from '@grafana/runtime';
 import { AnnotationQuery, DataSourceApi } from '@grafana/data';
 
@@ -13,6 +13,8 @@ import {
 import { emptyResult, translateQueryResult } from './utils';
 import { LegacyAnnotationQueryRunner } from './LegacyAnnotationQueryRunner';
 import { AnnotationsQueryRunner } from './AnnotationsQueryRunner';
+import { AnnotationQueryFinished, AnnotationQueryStarted } from '../../../../types/events';
+import { getDashboardQueryRunner } from './DashboardQueryRunner';
 
 export class AnnotationsWorker implements DashboardQueryRunnerWorker {
   constructor(
@@ -43,7 +45,14 @@ export class AnnotationsWorker implements DashboardQueryRunnerWorker {
             return of([]);
           }
 
+          dashboard.events.publish(new AnnotationQueryStarted(annotation));
+
           return runner.run({ annotation, datasource, dashboard, range }).pipe(
+            takeUntil(
+              getDashboardQueryRunner()
+                .cancellations()
+                .pipe(filter((a) => a === annotation))
+            ),
             map((results) => {
               // store response in annotation object if this is a snapshot call
               if (dashboard.snapshot) {
@@ -51,6 +60,9 @@ export class AnnotationsWorker implements DashboardQueryRunnerWorker {
               }
               // translate result
               return translateQueryResult(annotation, results);
+            }),
+            finalize(() => {
+              dashboard.events.publish(new AnnotationQueryFinished(annotation));
             })
           );
         })

public/app/features/query/state/DashboardQueryRunner/DashboardQueryRunner.test.ts

@@ -206,7 +206,7 @@ describe('DashboardQueryRunnerImpl', () => {
   });
 
   describe('when calling cancel', () => {
-    it('then it should cancel previous run', (done) => {
+    it('then it should cancel matching workers', (done) => {
       const { runner, options, annotationQueryMock, executeAnnotationQueryMock, getMock } = getTestContext();
       executeAnnotationQueryMock.mockReturnValueOnce(
         toAsyncOfResult({ events: [{ id: 'NextGen' }] }).pipe(delay(10000))
@@ -219,16 +219,19 @@ describe('DashboardQueryRunnerImpl', () => {
         expect: (results) => {
           // should have one alert state, one snapshot, one legacy and one next gen result
           // having both snapshot and legacy/next gen is a imaginary example for testing purposes and doesn't exist for real
-          const expected = { alertState: undefined, annotations: [] };
+          const { alertState, annotations } = getExpectedForAllResult();
-          expect(results).toEqual(expected);
+          expect(results).toEqual({ alertState, annotations: [annotations[0], annotations[2]] });
-          expect(annotationQueryMock).toHaveBeenCalledTimes(0);
+          expect(annotationQueryMock).toHaveBeenCalledTimes(1);
-          expect(executeAnnotationQueryMock).toHaveBeenCalledTimes(0);
+          expect(executeAnnotationQueryMock).toHaveBeenCalledTimes(1);
           expect(getMock).toHaveBeenCalledTimes(1);
         },
       });
 
       runner.run(options);
-      runner.cancel();
+      setTimeout(() => {
+        // call to async needs to be async or the cancellation will be called before any of the workers have started
+        runner.cancel(options.dashboard.annotations.list[1]);
+      }, 100);
     });
   });
 });

public/app/features/query/state/DashboardQueryRunner/DashboardQueryRunner.ts

@@ -1,5 +1,6 @@
-import { merge, Observable, race, Subject, Unsubscribable } from 'rxjs';
+import { merge, Observable, Subject, Unsubscribable } from 'rxjs';
 import { map, mergeAll, reduce, share, takeUntil } from 'rxjs/operators';
+import { AnnotationQuery } from '@grafana/data';
 
 import { dedupAnnotations } from 'app/features/annotations/events_processing';
 import {
@@ -20,7 +21,7 @@ import { RefreshEvent } from '../../../../types/events';
 class DashboardQueryRunnerImpl implements DashboardQueryRunner {
   private readonly results: Subject<DashboardQueryRunnerWorkerResult>;
   private readonly runs: Subject<DashboardQueryRunnerOptions>;
-  private readonly cancellations: Subject<any>;
+  private readonly cancellationStream: Subject<AnnotationQuery>;
   private readonly runsSubscription: Unsubscribable;
   private readonly eventsSubscription: Unsubscribable;
 
@@ -40,7 +41,7 @@ class DashboardQueryRunnerImpl implements DashboardQueryRunner {
     this.executeRun = this.executeRun.bind(this);
     this.results = new Subject<DashboardQueryRunnerWorkerResult>();
     this.runs = new Subject<DashboardQueryRunnerOptions>();
-    this.cancellations = new Subject<any>();
+    this.cancellationStream = new Subject<any>();
     this.runsSubscription = this.runs.subscribe((options) => this.executeRun(options));
     this.eventsSubscription = dashboard.events.subscribe(RefreshEvent, (event) => {
       this.run({ dashboard: this.dashboard, range: this.timeSrv.timeRange() });
@@ -70,7 +71,7 @@ class DashboardQueryRunnerImpl implements DashboardQueryRunner {
 
     merge(observables)
       .pipe(
-        takeUntil(race(this.runs.asObservable(), this.cancellations.asObservable())),
+        takeUntil(this.runs.asObservable()),
         mergeAll(),
         reduce((acc, value) => {
           // should we use scan or reduce here
@@ -87,15 +88,18 @@ class DashboardQueryRunnerImpl implements DashboardQueryRunner {
       });
   }
 
-  cancel(): void {
+  cancel(annotation: AnnotationQuery): void {
-    this.cancellations.next(1);
+    this.cancellationStream.next(annotation);
-    this.results.next({ annotations: [], alertStates: [] });
+  }
+
+  cancellations(): Observable<AnnotationQuery> {
+    return this.cancellationStream.asObservable().pipe(share());
   }
 
   destroy(): void {
     this.results.complete();
     this.runs.complete();
-    this.cancellations.complete();
+    this.cancellationStream.complete();
     this.runsSubscription.unsubscribe();
     this.eventsSubscription.unsubscribe();
   }

public/app/features/query/state/DashboardQueryRunner/testHelpers.ts

@@ -1,4 +1,4 @@
-import { asyncScheduler, Observable, of, scheduled, Subject } from 'rxjs';
+import { asyncScheduler, Observable, of, scheduled } from 'rxjs';
 import { DashboardQueryRunnerOptions } from './types';
 import { AnnotationEvent, getDefaultTimeRange } from '@grafana/data';
 
@@ -48,7 +48,10 @@ export function getDefaultOptions(): DashboardQueryRunnerOptions {
         getAnnotation({ enable: false, useSnapshotData: true }),
       ],
     },
-    events: new Subject(),
+    events: {
+      subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }),
+      publish: jest.fn(),
+    },
   };
   const range = getDefaultTimeRange();
 

public/app/features/query/state/DashboardQueryRunner/types.ts

@@ -16,7 +16,8 @@ export interface DashboardQueryRunnerResult {
 export interface DashboardQueryRunner {
   run: (options: DashboardQueryRunnerOptions) => void;
   getResult: (panelId?: number) => Observable<DashboardQueryRunnerResult>;
-  cancel: () => void;
+  cancel: (annotation: AnnotationQuery) => void;
+  cancellations: () => Observable<AnnotationQuery>;
   destroy: () => void;
 }
 

public/app/features/query/state/PanelQueryRunner.test.ts

@@ -1,5 +1,6 @@
 const applyFieldOverridesMock = jest.fn(); // needs to be first in this file
 
+import { Subject } from 'rxjs';
 // Importing this way to be able to spy on grafana/data
 import * as grafanaData from '@grafana/data';
 import { DashboardModel } from '../../dashboard/state/index';
@@ -95,6 +96,7 @@ function describeQueryRunnerScenario(
       getResult: emptyResult,
       run: () => undefined,
       cancel: () => undefined,
+      cancellations: () => new Subject<any>(),
       destroy: () => undefined,
     }));
     createDashboardQueryRunner({} as any);

public/app/types/events.ts

@@ -1,4 +1,11 @@
-import { BusEventBase, BusEventWithPayload, eventFactory, GrafanaThemeV2, TimeRange } from '@grafana/data';
+import {
+  AnnotationQuery,
+  BusEventBase,
+  BusEventWithPayload,
+  eventFactory,
+  GrafanaThemeV2,
+  TimeRange,
+} from '@grafana/data';
 import { IconName } from '@grafana/ui';
 
 /**
@@ -184,3 +191,11 @@ export class HideModalEvent extends BusEventBase {
 export class DashboardSavedEvent extends BusEventBase {
   static type = 'dashboard-saved';
 }
+
+export class AnnotationQueryStarted extends BusEventWithPayload<AnnotationQuery> {
+  static type = 'annotation-query-started';
+}
+
+export class AnnotationQueryFinished extends BusEventWithPayload<AnnotationQuery> {
+  static type = 'annotation-query-finished';
+}