fixed some bugs and added unit tests for associated detectors

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

public/action/ad_dashboard_action.tsx

@@ -14,6 +14,9 @@ import {
 } from '../../../../src/plugins/ui_actions/public';
 import { isReferenceOrValueEmbeddable } from '../../../../src/plugins/embeddable/public';
 import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
+import { isEmpty } from 'lodash';
+import { VisualizeEmbeddable } from '../../../../src/plugins/visualizations/public';
+import { isEligibleForVisLayers } from '../../../../src/plugins/vis_augmenter/public';
 
 export const ACTION_AD = 'ad';
 
@@ -57,15 +60,13 @@ export const createADAction = ({
     type: ACTION_AD,
     grouping,
     isCompatible: async ({ embeddable }: ActionContext) => {
-      const paramsType = embeddable.vis?.params?.type;
-      const seriesParams = embeddable.vis?.params?.seriesParams || [];
-      const series = embeddable.vis?.params?.series || [];
-      const isLineGraph =
-        seriesParams.find((item) => item.type === 'line') ||
-        series.find((item) => item.chart_type === 'line');
-      const isValidVis = isLineGraph && paramsType !== 'table';
+      const vis = (embeddable as VisualizeEmbeddable).vis;
       return Boolean(
-        embeddable.parent && isDashboard(embeddable.parent) && isValidVis
+        embeddable.parent &&
+          isDashboard(embeddable.parent) &&
+          vis !== undefined &&
+          isEligibleForVisLayers(vis) &&
+          !isEmpty((embeddable as VisualizeEmbeddable).visLayers)
       );
     },
     execute: async ({ embeddable }: ActionContext) => {

public/components/FeatureAnywhereContextMenu/AssociatedDetectors/containers/AssociatedDetectors.tsx

@@ -93,8 +93,6 @@ function AssociatedDetectors({ embeddable, closeFlyout, setMode }) {
   const notifications = getNotifications();
 
   useEffect(() => {
-    console.log("errorGettingDetectors: " + errorGettingDetectors)
-    //console.log("state.ad.errorMessage: " + state.ad.errorMessage)
     if (
       errorGettingDetectors &&
       !errorGettingDetectors.includes(SINGLE_DETECTOR_NOT_FOUND_MSG)

public/components/FeatureAnywhereContextMenu/AssociatedDetectors/containers/__tests__/AssociatedDetectors.test.tsx

@@ -1,12 +1,6 @@
 /*
+ * Copyright OpenSearch Contributors
  * SPDX-License-Identifier: Apache-2.0
- *
- * The OpenSearch Contributors require contributions made to
- * this file be licensed under the Apache-2.0 license or a
- * compatible open source license.
- *
- * Modifications Copyright OpenSearch Contributors. See
- * GitHub history for details.
  */
 
 import React from 'react';
@@ -35,7 +29,7 @@ import {
   VisLayerTypes,
   createSavedAugmentVisLoader,
   setUISettings as setVisAugUISettings,
-   getMockAugmentVisSavedObjectClient,
+  getMockAugmentVisSavedObjectClient,
   SavedObjectLoaderAugmentVis,
 } from '../../../../../../../../src/plugins/vis_augmenter/public';
 import { getAugmentVisSavedObjs } from '../../../../../../../../src/plugins/vis_augmenter/public/utils';
@@ -96,7 +90,6 @@ jest.mock('../../../../../services', () => ({
   },
 }));
 
-
 jest.mock(
   '../../../../../../../../src/plugins/vis_augmenter/public/utils',
   () => ({
@@ -108,9 +101,7 @@ const visEmbeddable = createMockVisEmbeddable(
   'test-title',
   false
 );
-// const uiSettingsMock = uiSettingsServiceMock.createStartContract();
-// setUISettings(uiSettingsMock);
-// setVisAugUISettings(uiSettingsMock);
+
 const renderWithRouter = (visEmbeddable: VisualizeEmbeddable) => ({
   ...render(
     <Provider store={configureStore(httpClientMock)}>
@@ -177,91 +168,96 @@ describe('AssociatedDetectors spec', () => {
         response: { detectorList: [], totalDetectors: 0 },
       });
       (getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
-          Promise.resolve(savedObjects)
-        );
+        Promise.resolve(savedObjects)
+      );
       const { getByText } = renderWithRouter(visEmbeddable);
       getByText('Loading detectors...');
       getByText('Real-time state');
       getByText('Associate a detector');
     });
   });
 
-    describe('renders either one or zero detectors', () => {
-      test('renders one associated detector', async () => {
-        httpClientMock.get = jest.fn().mockResolvedValue({
-          ok: true,
-          response: {
-            detectorList: detectorsToAssociate,
-            totalDetectors: detectorsToAssociate.length,
-          },
-        });
-        (getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
-          Promise.resolve(savedObjects)
-        );
-        const { getByText, queryByText } = renderWithRouter(visEmbeddable);
-        getByText('Loading detectors...');
-        await waitFor(() => getByText('detector_name_0'));
-        getByText('5');
-        expect(queryByText('detector_name_1')).toBeNull();
-     });
-      test('renders no associated detectors', async () => {
-        // const existingDetectors = [detectorTwo];
-
-        httpClientMock.get = jest.fn().mockResolvedValue({
-          ok: true,
-          response: {
-            detectorList: [detectorsToAssociate[1]],
-            totalDetectors: 1,
-          },
-        });
-        (getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
-          Promise.resolve(savedObjects)
-        );
-        const { getByText, findByText } = renderWithRouter(visEmbeddable);
-        getByText('Loading detectors...');
-        await waitFor(() =>
+  describe('renders either one or zero detectors', () => {
+    test('renders one associated detector', async () => {
+      httpClientMock.get = jest.fn().mockResolvedValue({
+        ok: true,
+        response: {
+          detectorList: detectorsToAssociate,
+          totalDetectors: detectorsToAssociate.length,
+        },
+      });
+      (getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
+        Promise.resolve(savedObjects)
+      );
+      const { getByText, queryByText } = renderWithRouter(visEmbeddable);
+      getByText('Loading detectors...');
+      await waitFor(() => getByText('detector_name_0'));
+      getByText('5');
+      expect(queryByText('detector_name_1')).toBeNull();
+    }, 80000);
+    test('renders no associated detectors', async () => {
+      httpClientMock.get = jest.fn().mockResolvedValue({
+        ok: true,
+        response: {
+          detectorList: [detectorsToAssociate[1]],
+          totalDetectors: 1,
+        },
+      });
+      (getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
+        Promise.resolve(savedObjects)
+      );
+      const { getByText, findByText } = renderWithRouter(visEmbeddable);
+      getByText('Loading detectors...');
+      await waitFor(() =>
         findByText(
-            'There are no anomaly detectors associated with test-title visualization.'
-          , undefined, {timeout: 100000})
-        );
-      }, 150000);
-    });
+          'There are no anomaly detectors associated with test-title visualization.',
+          undefined,
+          { timeout: 100000 }
+        )
+      );
+    }, 150000);
+  });
 
-    describe('tests unlink functionality', () => {
-      test('unlinks a single detector', async () => {
-        //const existingDetectors = [detectorOne, detectorTwo];
-        httpClientMock.get = jest.fn().mockResolvedValue({
-          ok: true,
-          response: {
-            detectorList: detectorsToAssociate,
-            totalDetectors: detectorsToAssociate.length,
-          },
-        });
-        (getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
-          Promise.resolve(savedObjects)
-        );
-        const { getByText, queryByText, getAllByTestId } =
-          renderWithRouter(visEmbeddable);
-        getByText('Loading detectors...');
-        await waitFor(() => getByText('detector_name_0'));
-        getByText('5');
-        expect(queryByText('detector_name_1')).toBeNull();
-        userEvent.click(getAllByTestId('unlinkButton')[0]);
-        await waitFor(() =>
-          getByText(
-            'Removing association unlinks detector_name_0 detector from the visualization but does not delete it. The detector association can be restored.'
-          )
-        );
-        userEvent.click(getAllByTestId('confirmUnlinkButton')[0]);
-        expect(
-          (await getAugmentVisSavedObjs("valid-obj-id-0", augmentVisLoader, uiSettingsMock))
-            .length
-        ).toEqual(2);
-        await waitFor(() => expect(mockDeleteFn).toHaveBeenCalledTimes(1));
+  describe('tests unlink functionality', () => {
+    test('unlinks a single detector', async () => {
+      httpClientMock.get = jest.fn().mockResolvedValue({
+        ok: true,
+        response: {
+          detectorList: detectorsToAssociate,
+          totalDetectors: detectorsToAssociate.length,
+        },
       });
-    });
+      (getAugmentVisSavedObjs as jest.Mock).mockImplementation(() =>
+        Promise.resolve(savedObjects)
+      );
+      const { getByText, queryByText, getAllByTestId } =
+        renderWithRouter(visEmbeddable);
+      getByText('Loading detectors...');
+      await waitFor(() => getByText('detector_name_0'));
+      getByText('5');
+      expect(queryByText('detector_name_1')).toBeNull();
+      userEvent.click(getAllByTestId('unlinkButton')[0]);
+      await waitFor(() =>
+        getByText(
+          'Removing association unlinks detector_name_0 detector from the visualization but does not delete it. The detector association can be restored.'
+        )
+      );
+      userEvent.click(getAllByTestId('confirmUnlinkButton')[0]);
+      expect(
+        (
+          await getAugmentVisSavedObjs(
+            'valid-obj-id-0',
+            augmentVisLoader,
+            uiSettingsMock
+          )
+        ).length
+      ).toEqual(2);
+      await waitFor(() => expect(mockDeleteFn).toHaveBeenCalledTimes(1));
+    }, 100000);
+  });
 });
 
+//I have a new beforeEach because I making a lot more detectors and saved objects for these tests
 describe('test over 10 associated objects functionality', () => {
   let augmentVisLoader: SavedObjectLoaderAugmentVis;
   let mockDeleteFn: jest.Mock;
@@ -374,7 +370,7 @@ describe('test over 10 associated objects functionality', () => {
       Promise.resolve(savedObjects)
     );
 
-    const { getByText, queryByText, getAllByTestId, findByText } =
+    const { queryByText, getAllByTestId, findByText } =
       renderWithRouter(visEmbeddable);
 
     // initial load only first 10 detectors (string sort means detector_name_0 -> detector_name_9 show up)

public/components/FeatureAnywhereContextMenu/AssociatedDetectors/utils/helpers.tsx

@@ -62,7 +62,7 @@ export const getColumns = ({ handleUnlinkDetectorAction }) =>
           description: 'Remove association',
           icon: 'unlink',
           onClick: handleUnlinkDetectorAction,
-          "data-test-subj":"unlinkButton",
+          'data-test-subj': 'unlinkButton',
         },
       ],
     },

public/components/FeatureAnywhereContextMenu/CreateAnomalyDetector/AddAnomalyDetector.tsx

@@ -413,17 +413,11 @@ function AddAnomalyDetector({
             closeFlyout();
           })
           .catch((error) => {
-            notifications.toasts.addDanger(
-              prettifyErrorMessage(
-                `Error associating selected detector: ${error}`
-              )
-            );
+            notifications.toasts.addDanger(prettifyErrorMessage(error));
           });
       })
       .catch((error) => {
-        notifications.toasts.addDanger(
-          prettifyErrorMessage(`Error associating selected detector: ${error}`)
-        );
+        notifications.toasts.addDanger(prettifyErrorMessage(error));
       });
   };
 

public/components/FeatureAnywhereContextMenu/CreateAnomalyDetector/AssociateExisting/containers/AssociateExisting.tsx

@@ -206,7 +206,7 @@ export function AssociateExisting(
           options={options}
           selectedOptions={selectedOptions}
           onChange={(selectedOptions) => {
-            let detector = {} as DetectorListItem | undefined;
+            let detector = undefined as DetectorListItem | undefined;
 
             if (selectedOptions && selectedOptions.length) {
               const match = existingDetectorsAvailableToAssociate.find(
@@ -234,7 +234,7 @@ export function AssociateExisting(
               </EuiText>
               <EuiSpacer size="s" />
               <EuiHealth color={stateToColorMap.get(detector.curState)}>
-                {renderTime(detector.enabledTime)}
+                Running since {renderTime(detector.enabledTime)}
               </EuiHealth>
             </EuiFlexItem>
             <EuiFlexItem grow={false}>

public/expressions/__tests__/overlay_anomalies.test.ts

@@ -20,6 +20,7 @@ import {
 } from '../../pages/utils/__tests__/constants';
 import {
   DETECTOR_HAS_BEEN_DELETED,
+  PLUGIN_EVENT_TYPE,
   START_OR_END_TIME_INVALID_ERROR,
   VIS_LAYER_PLUGIN_TYPE,
 } from '../constants';
@@ -104,6 +105,7 @@ describe('overlay_anomalies spec', () => {
           type: 'Anomaly Detectors',
           urlPath: `anomaly-detection-dashboards#/detectors/${ANOMALY_RESULT_SUMMARY_DETECTOR_ID}/results`,
         },
+        pluginEventType: PLUGIN_EVENT_TYPE,
         type: 'PointInTimeEvents',
       };
       const pointInTimeEventsVisLayer =

public/expressions/helpers.ts

@@ -83,7 +83,7 @@ export const convertAnomaliesToPointInTimeEventsVisLayer = (
     type: VisLayerTypes.PointInTimeEvents,
     pluginResource: ADPluginResource,
     events: events,
-    pluginEventType: PLUGIN_EVENT_TYPE
+    pluginEventType: PLUGIN_EVENT_TYPE,
   } as PointInTimeEventsVisLayer;
 };
 

public/services.ts

@@ -43,5 +43,5 @@ export default {
   getEmbeddable,
   getNotifications,
   getOverlays,
-  setUISettings
+  setUISettings,
 };