address comments

Author: ylwu-amzn

.lintstagedrc

@@ -1,4 +1,4 @@
 {
-  "*.{ts,tsx,js,jsx,json,css,md}": ["prettier --write", "git add"]
+  "*.{js,jsx,json,css,md}": ["prettier --write", "git add"]
 }
 

index.js

@@ -22,7 +22,7 @@ import {
   MonitorService,
   AnomalyDetectorService,
 } from './server/services';
-import { createAlertingCluster, createAlertingAdCluster } from './server/clusters';
+import { createAlertingCluster, createAlertingADCluster } from './server/clusters';
 import { PLUGIN_NAME } from './utils/constants';
 
 export default function(kibana) {
@@ -49,7 +49,7 @@ export default function(kibana) {
     init(server, options) {
       // Create clusters
       createAlertingCluster(server);
-      createAlertingAdCluster(server);
+      createAlertingADCluster(server);
 
       // Initialize services
       const esDriver = server.plugins.elasticsearch;

public/pages/CreateMonitor/components/AnomalyDetectors/AnomaliesChart/AnomaliesChart.js

@@ -15,7 +15,6 @@
 
 import React from 'react';
 import PropTypes from 'prop-types';
-import DelayedLoader from '../../../../../components/DelayedLoader';
 import { EuiText, EuiSpacer } from '@elastic/eui';
 import {
   Chart,
@@ -30,14 +29,13 @@ import {
   LineAnnotation,
 } from '@elastic/charts';
 import { ChartContainer } from '../../../../../components/ChartContainer/ChartContainer';
+import DelayedLoader from '../../../../../components/DelayedLoader';
 
 const getAxisTitle = (displayGrade, displayConfidence) => {
   if (displayGrade && displayConfidence) {
     return 'Anomaly grade / confidence';
-  } else if (displayGrade) {
-    return 'Anomaly grade';
   }
-  return 'Anomaly confidence';
+  return displayGrade ? 'Anomaly grade' : 'Anomaly confidence';
 };
 
 const AnomaliesChart = props => {

public/pages/CreateMonitor/components/AnomalyDetectors/AnomaliesChart/AnomaliesChart.test.js

@@ -15,8 +15,8 @@
 
 import React from 'react';
 import { render } from 'enzyme';
-import { AnomaliesChart } from './AnomaliesChart';
 import moment from 'moment';
+import { AnomaliesChart } from './AnomaliesChart';
 
 const startTime = moment('2018-10-25T09:30:00').valueOf();
 const endTime = moment('2018-10-29T09:30:00').valueOf();

public/pages/CreateMonitor/components/AnomalyDetectors/EmptyFeaturesMessage/EmptyFeaturesMessage.js

@@ -36,8 +36,8 @@ const EmptyFeaturesMessage = props => (
         style={{ maxWidth: '45em' }}
         body={
           <EuiText>
-            No features have been added to this anomaly detector. A feature is a metric that used
-            for anomaly detection. A detector can discover anomalies across one or many features.
+            No features have been added to this anomaly detector. A feature is a metric that is used
+            for anomaly detection. A detector can discover anomalies across one or more features.
           </EuiText>
         }
         actions={[

public/pages/CreateMonitor/components/AnomalyDetectors/EmptyFeaturesMessage/__snapshots__/EmptyFeaturesMessage.test.js.snap

@@ -17,7 +17,7 @@ exports[`EmptyFeaturesMessage renders  1`] = `
         <div
           class="euiText euiText--medium"
         >
-          No features have been added to this anomaly detector. A feature is a metric that used for anomaly detection. A detector can discover anomalies across one or many features.
+          No features have been added to this anomaly detector. A feature is a metric that is used for anomaly detection. A detector can discover anomalies across one or more features.
         </div>
       </div>
     </span>

public/pages/CreateMonitor/containers/AnomalyDetectors/AnomalyDetectorData.js

@@ -17,6 +17,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import moment from 'moment';
 import { AppContext } from '../../../../utils/AppContext';
+import { AD_PREVIEW_DAYS } from '../../../../utils/constants';
 
 class AnomalyDetectorData extends React.Component {
   static contextType = AppContext;
@@ -39,15 +40,6 @@ class AnomalyDetectorData extends React.Component {
   async componentDidMount() {
     await this.getPreviewData();
   }
-  componentDidUpdate(prevProps) {
-    if (
-      this.props.detectorId != prevProps.detectorId ||
-      this.props.startTime != prevProps.startTime ||
-      this.props.endTime != prevProps.endTime
-    ) {
-      this.getPreviewData();
-    }
-  }
 
   async getPreviewData() {
     const { detectorId, startTime, endTime } = this.props;
@@ -58,7 +50,7 @@ class AnomalyDetectorData extends React.Component {
     if (!detectorId) return;
     const requestParams = {
       startTime: moment()
-        .subtract(5, 'd')
+        .subtract(AD_PREVIEW_DAYS, 'd')
         .valueOf(),
       startTime: startTime,
       endTime: endTime,

public/pages/CreateMonitor/containers/AnomalyDetectors/AnomalyDetectors.js

@@ -43,7 +43,6 @@ class AnomalyDetectors extends React.Component {
           label: detector.name,
           value: detector.id,
           features: detector.featureAttributes,
-          //TODO::Temp hack until backend fix
           interval: detector.detectionInterval,
         }));
         this.setState({ detectorOptions });

public/pages/CreateMonitor/containers/AnomalyDetectors/__tests__/AnomalyDetector.test.js

@@ -51,11 +51,6 @@ describe('AnomalyDetectors', () => {
     const wrapper = getMountWrapper();
     expect(wrapper).toMatchSnapshot();
   });
-  // test('must call renderEmptyMessage initially', () => {
-  //   httpClientMock.post.mockResolvedValue({ data: { ok: true, detectors: [] } });
-  //   getMountWrapper();
-  //   expect(renderEmptyMessage).toHaveBeenCalled();
-  // });
 
   test('should be able to select the detector', done => {
     httpClientMock.post.mockResolvedValueOnce({

public/pages/CreateMonitor/containers/AnomalyDetectors/__tests__/AnomalyDetectorPreviewData.test.js

@@ -42,16 +42,6 @@ describe('AnomalyDetectorData', () => {
     expect(getPreviewData).toHaveBeenCalled();
     expect(getPreviewData).toHaveBeenCalledTimes(1);
   });
-  test('calls preview api if detectorId changes', () => {
-    const mockedRender = jest.fn().mockImplementation(() => null);
-    const getPreviewData = jest.spyOn(AnomalyDetectorData.prototype, 'getPreviewData');
-    const wrapper = getMountWrapper();
-    wrapper.setProps({
-      children: <AnomalyDetectorData detectorId="another" render={mockedRender} />,
-    });
-    wrapper.update();
-    expect(getPreviewData).toHaveBeenCalledTimes(2);
-  });
   test('calls render with anomalyResult', () => {
     const wrapper = getMountWrapper();
     expect(mockedRender).toHaveBeenCalled();

public/pages/CreateMonitor/containers/CreateMonitor/utils/formikToMonitor.js

@@ -83,11 +83,6 @@ export function formikToAdQuery(values) {
       },
     },
     aggregations: {
-      // max_confidence: {
-      //   max: {
-      //     field: "confidence",
-      //   },
-      // },
       max_anomaly_grade: {
         max: {
           field: 'anomaly_grade',

public/pages/CreateMonitor/containers/CreateMonitor/utils/monitorToFormik.js

@@ -51,9 +51,7 @@ export default function monitorToFormik(monitor) {
     fieldName: fieldName ? [{ label: fieldName }] : [],
     timezone: timezone ? [{ label: timezone }] : [],
 
-    detectorId: isAD //&& _.get(inputs, '0.search.indices.0') === '.opendistro-anomaly-results*'
-      ? _.get(inputs, INPUTS_DETECTOR_ID)
-      : undefined,
+    detectorId: isAD ? _.get(inputs, INPUTS_DETECTOR_ID) : undefined,
     index: inputs[0].search.indices.map(index => ({ label: index })),
     query: JSON.stringify(inputs[0].search.query, null, 4),
   };

public/pages/CreateTrigger/containers/CreateTrigger/utils/formikToTrigger.js

@@ -90,7 +90,6 @@ export function getADCondition(values) {
     return {
       script: {
         lang: 'painless',
-        //TODO:: Check null check is required or not confirm with backend
         source: `return ${ANOMALY_GRADE_RESULT_PATH} != null && ${ANOMALY_GRADE_RESULT_PATH} ${anomalyGradeOperator} ${anomalyDetector.anomalyGradeThresholdValue} && ${ANOMALY_CONFIDENCE_RESULT_PATH} ${anomalyConfidenceOperator} ${anomalyDetector.anomalyConfidenceThresholdValue}`,
       },
     };

public/pages/Dashboard/containers/Dashboard.js

@@ -314,23 +314,24 @@ export default class Dashboard extends Component {
         selectable ? undefined : 'Only Active Alerts are Acknowledgeable',
     };
 
+    const actions = () => {
+      const actions = [<EuiButton onClick={this.acknowledgeAlert}>Acknowledge</EuiButton>];
+      if (detectorIds.length) {
+        actions.unshift(
+          <EuiButton href={`${ES_AD_PLUGIN}#/detectors/${detectorIds[0]}`} target="_blank">
+            View detector <EuiIcon type="popout" />
+          </EuiButton>
+        );
+      }
+      return actions;
+    };
+
     return (
       <ContentPanel
         title="Alerts"
         titleSize={monitorIds.length ? 's' : 'l'}
         bodyStyles={{ padding: 'initial' }}
-        actions={
-          detectorIds.length ? (
-            [
-              <EuiButtonEmpty href={`${ES_AD_PLUGIN}#/detectors/${detectorIds[0]}`} target="_blank">
-                View detector <EuiIcon type="popout" />
-              </EuiButtonEmpty>,
-              <EuiButton onClick={this.acknowledgeAlert}>Acknowledge</EuiButton>,
-            ]
-          ) : (
-            <EuiButton onClick={this.acknowledgeAlert}>Acknowledge</EuiButton>
-          )
-        }
+        actions={actions()}
       >
         <DashboardControls
           activePage={page}

public/pages/MonitorDetails/containers/AnomalyHistory/AnomalyHistory.js

@@ -1,3 +1,18 @@
+/*
+ *   Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License").
+ *   You may not use this file except in compliance with the License.
+ *   A copy of the License is located at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   or in the "license" file accompanying this file. This file is distributed
+ *   on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *   express or implied. See the License for the specific language governing
+ *   permissions and limitations under the License.
+ */
+
 import React, { Component } from 'react';
 import moment from 'moment';
 import { get } from 'lodash';

public/pages/MonitorDetails/containers/MonitorDetails.js

@@ -291,7 +291,7 @@ export default class MonitorDetails extends Component {
             {detector ? (
               <EuiFlexItem grow={false}>
                 <EuiText size="s">
-                  Created from detector{' '}
+                  Created from detector:{' '}
                   <EuiLink
                     href={`${ES_AD_PLUGIN}#/detectors/${detectorId}`}
                     external="true"

public/pages/MonitorDetails/containers/utils/__tests__/helper.test.js

@@ -1,3 +1,18 @@
+/*
+ *   Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License").
+ *   You may not use this file except in compliance with the License.
+ *   A copy of the License is located at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   or in the "license" file accompanying this file. This file is distributed
+ *   on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *   express or implied. See the License for the specific language governing
+ *   permissions and limitations under the License.
+ */
+
 import { migrateTriggerMetadata } from '../helpers';
 
 describe('helpers', () => {

public/pages/MonitorDetails/containers/utils/helpers.js

@@ -1,3 +1,18 @@
+/*
+ *   Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License").
+ *   You may not use this file except in compliance with the License.
+ *   A copy of the License is located at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   or in the "license" file accompanying this file. This file is distributed
+ *   on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *   express or implied. See the License for the specific language governing
+ *   permissions and limitations under the License.
+ */
+
 import { get, isEmpty } from 'lodash';
 export const migrateTriggerMetadata = monitor => {
   const uiMetadata = get(monitor, 'ui_metadata', {});

public/utils/AppContext.js

@@ -1,3 +1,18 @@
+/*
+ *   Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License").
+ *   You may not use this file except in compliance with the License.
+ *   A copy of the License is located at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   or in the "license" file accompanying this file. This file is distributed
+ *   on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *   express or implied. See the License for the specific language governing
+ *   permissions and limitations under the License.
+ */
+
 import React from 'react';
 
 export const AppContext = React.createContext({});

public/utils/constants.js

@@ -54,10 +54,10 @@ export const DATA_TYPES = {
   KEYWORD: 'keyword',
 };
 
-export const AD_PLUGIN_ID = 'aes-ad-plugin';
-
 export const ES_AD_PLUGIN = 'opendistro-anomaly-detection';
 
 export const INPUTS_DETECTOR_ID = '0.search.query.query.bool.filter[1].term.detector_id.value';
 
 export const MONITOR_INPUT_DETECTOR_ID = `inputs.${INPUTS_DETECTOR_ID}`;
+
+export const AD_PREVIEW_DAYS = 7;

public/utils/validate.js

@@ -93,7 +93,7 @@ export const ILLEGAL_CHARACTERS = ['\\', '/', '?', '"', '<', '>', '|', ',', ' ']
 export const validateDetector = (detectorId, selectedDetector) => {
   if (!detectorId) return 'Must select detector';
   if (selectedDetector && selectedDetector.features.length === 0)
-    return 'Detector with no features.';
+    return 'Must choose detector which has features';
 };
 
 export const validateIndex = options => {

server/clusters/alerting/createAlertingAdCluster.js

@@ -16,7 +16,7 @@
 import alertingADPlugin from './adPlugin';
 import { CLUSTER, DEFAULT_HEADERS } from '../../services/utils/constants';
 
-export default function createAlertingAdCluster(server) {
+export default function createAlertingADCluster(server) {
   const { customHeaders, ...rest } = server.config().get('elasticsearch');
   server.plugins.elasticsearch.createCluster(CLUSTER.AD_ALERTING, {
     plugins: [alertingADPlugin],

server/clusters/index.js

@@ -14,6 +14,6 @@
  */
 
 import createAlertingCluster from './alerting/createAlertingCluster';
-import createAlertingAdCluster from './alerting/createAlertingAdCluster';
+import createAlertingADCluster from './alerting/createAlertingADCluster';
 
-export { createAlertingCluster, createAlertingAdCluster };
+export { createAlertingCluster, createAlertingADCluster };

server/services/MonitorService.js

@@ -31,10 +31,6 @@ export default class MonitorService {
       return { ok: true, resp: createResponse };
     } catch (err) {
       console.error('Alerting - MonitorService - createMonitor:', err);
-      //FIXME: This is temporary as backend should send error message inside message instead of body
-      if (err.statusCode === 400) {
-        return { ok: false, resp: err.body };
-      }
       return { ok: false, resp: err.message };
     }
   };