Non blocker (#202)

amitgalitz · ohltyler · commit 24518cf3ae8e · 2022-03-08T16:14:25.000-08:00
Signed-off-by: Amit Galitzky <amgalitz@amazon.com> (cherry picked from commit 9ae8917)
diff --git a/public/models/interfaces.ts b/public/models/interfaces.ts
@@ -231,9 +231,11 @@ export type EntityOptionsMap = {
 export type ValidationModelResponse = {
   message: String;
   sub_issues?: { [key: string]: string };
+  validationType: string;
 };
 
 export interface ValidationSettingResponse {
   issueType: string;
   message: string;
+  validationType: string;
 }
diff --git a/public/pages/DefineDetector/utils/constants.tsx b/public/pages/DefineDetector/utils/constants.tsx
@@ -12,7 +12,6 @@
 import { FILTER_TYPES, UIFilter } from '../../../models/interfaces';
 import { OPERATORS_MAP } from '../../DefineDetector/components/DataFilterList/utils/constant';
 import { DetectorDefinitionFormikValues } from '../../DefineDetector/models/interfaces';
-
 export const EMPTY_UI_FILTER: UIFilter = {
   filterType: FILTER_TYPES.SIMPLE,
   fieldInfo: [],
diff --git a/public/pages/ReviewAndCreate/components/DetectorDefinitionFields/DetectorDefinitionFields.tsx b/public/pages/ReviewAndCreate/components/DetectorDefinitionFields/DetectorDefinitionFields.tsx
@@ -20,15 +20,14 @@ import {
   EuiText,
 } from '@elastic/eui';
 import React from 'react';
-import { get } from 'lodash';
+import { get, isEqual } from 'lodash';
 import {
   Detector,
   ValidationSettingResponse,
 } from '../../../../models/interfaces';
 import { FilterDisplayList } from '../FilterDisplayList';
 import { ConfigCell, FixedWidthRow } from '../../../../components/ConfigCell';
 import { toStringConfigCell } from '../../utils/helpers';
-
 interface DetectorDefinitionFieldsProps {
   detector: Detector;
   onEditDetectorDefinition(): void;
@@ -94,19 +93,38 @@ export const DetectorDefinitionFields = (
         !props.validDetectorSettings &&
         props.validationResponse.hasOwnProperty('message')
       ) {
-        return (
-          <EuiCallOut
-            title="Issues found in the detector settings"
-            color="danger"
-            iconType="alert"
-            size="s"
-            style={{ marginBottom: '10px' }}
-          >
-            <ul>
-              <li>{props.validationResponse.message}</li>
-            </ul>
-          </EuiCallOut>
-        );
+        if (
+          isEqual(get(props, 'validationResponse.validationType', ''), 'model')
+        ) {
+          return (
+            <EuiCallOut
+              title="We identified some areas that might improve your model"
+              color="warning"
+              iconType="iInCircle"
+              size="s"
+              style={{ marginBottom: '10px' }}
+            >
+              {JSON.stringify(props.validationResponse.message).replace(
+                /\"/g,
+                ''
+              )}
+            </EuiCallOut>
+          );
+        } else {
+          return (
+            <EuiCallOut
+              title="Issues found in the detector settings"
+              color="danger"
+              iconType="alert"
+              size="s"
+              style={{ marginBottom: '10px' }}
+            >
+              <ul>
+                <li>{props.validationResponse.message}</li>
+              </ul>
+            </EuiCallOut>
+          );
+        }
       } else {
         return null;
       }
diff --git a/public/pages/ReviewAndCreate/components/FilterDisplayList/FilterDisplayList.tsx b/public/pages/ReviewAndCreate/components/FilterDisplayList/FilterDisplayList.tsx
@@ -27,7 +27,7 @@ export const FilterDisplayList = (props: FilterDisplayListProps) => {
   let filters = get(props, 'uiMetadata.filters', []);
   const oldFilterType = get(props, 'uiMetadata.filterType', undefined);
 
-  // We want to show the custom filter if filters is empty and 
+  // We want to show the custom filter if filters is empty and
   // props.filterQuery isn't empty.
   // Two possible situations for the if branch:
   // First, old detectors with custom filters will have no filter list, but
diff --git a/public/pages/ReviewAndCreate/components/ModelConfigurationFields/ModelConfigurationFields.tsx b/public/pages/ReviewAndCreate/components/ModelConfigurationFields/ModelConfigurationFields.tsx
@@ -26,7 +26,7 @@ import {
   FeatureAttributes,
   ValidationModelResponse,
 } from '../../../../models/interfaces';
-import { get, sortBy } from 'lodash';
+import { get, sortBy, isEqual } from 'lodash';
 import ContentPanel from '../../../../components/ContentPanel/ContentPanel';
 import { CodeModal } from '../../../../components/CodeModal/CodeModal';
 import { AdditionalSettings } from '../AdditionalSettings/AdditionalSettings';
@@ -253,26 +253,50 @@ export const ModelConfigurationFields = (
       !props.validModel &&
       props.validationFeatureResponse.hasOwnProperty('message')
     ) {
-      return (
-        <EuiCallOut
-          title="issues found in the model configuration"
-          color="danger"
-          iconType="alert"
-          size="s"
-          style={{ marginBottom: '10px' }}
-        >
-          {/* Callout can either display feature subissue which related to a specific 
-          feature issue or display a feature_attribute issue that is general like
-          more then x anomaly feature or dulicate feature names */}
-          {props.validationFeatureResponse.hasOwnProperty('sub_issues') ? (
-            getFeatureValidationCallout(props.validationFeatureResponse)
-          ) : (
-            <ul>
-              <li>{JSON.stringify(props.validationFeatureResponse.message)}</li>
-            </ul>
-          )}
-        </EuiCallOut>
-      );
+      if (
+        isEqual(
+          get(props, 'validationFeatureResponse.validationType', ''),
+          'model'
+        )
+      ) {
+        return (
+          <EuiCallOut
+            title="We identified some areas that might improve your model"
+            color="warning"
+            iconType="iInCircle"
+            size="s"
+            style={{ marginBottom: '10px' }}
+          >
+            {JSON.stringify(props.validationFeatureResponse.message).replace(
+              /\"/g,
+              ''
+            )}
+          </EuiCallOut>
+        );
+      } else {
+        return (
+          <EuiCallOut
+            title="Issues found in the model configuration"
+            color="danger"
+            iconType="alert"
+            size="s"
+            style={{ marginBottom: '10px' }}
+          >
+            {/* Callout can either display feature subissue which related to a specific 
+            feature issue or display a feature_attribute issue that is general like
+            more then x anomaly feature or dulicate feature names */}
+            {props.validationFeatureResponse.hasOwnProperty('sub_issues') ? (
+              getFeatureValidationCallout(props.validationFeatureResponse)
+            ) : (
+              <ul>
+                <li>
+                  {JSON.stringify(props.validationFeatureResponse.message)}
+                </li>
+              </ul>
+            )}
+          </EuiCallOut>
+        );
+      }
     } else {
       return null;
     }
diff --git a/public/pages/ReviewAndCreate/containers/ReviewAndCreate.tsx b/public/pages/ReviewAndCreate/containers/ReviewAndCreate.tsx
@@ -20,7 +20,6 @@ import {
   EuiTitle,
   EuiButtonEmpty,
   EuiSpacer,
-  EuiCallOut,
 } from '@elastic/eui';
 import {
   createDetector,
@@ -30,7 +29,7 @@ import {
   validateDetector,
 } from '../../../redux/reducers/ad';
 import { Formik, FormikHelpers } from 'formik';
-import { get } from 'lodash';
+import { get, isEmpty } from 'lodash';
 import React, { Fragment, useEffect, useState } from 'react';
 import { RouteComponentProps } from 'react-router';
 import { useDispatch, useSelector } from 'react-redux';
@@ -54,7 +53,6 @@ import {
   ValidationSettingResponse,
   VALIDATION_ISSUE_TYPES,
 } from '../../../models/interfaces';
-import { isEmpty } from 'lodash';
 
 interface ReviewAndCreateProps extends RouteComponentProps {
   setStep(stepNumber: number): void;
@@ -92,28 +90,49 @@ export function ReviewAndCreate(props: ReviewAndCreateProps) {
   // meaning validation has passed and succesful callout will display or validation has failed
   // and callouts displaying what the issue is will be displayed instead.
   useEffect(() => {
-    dispatch(validateDetector(formikToDetector(props.values)))
+    dispatch(validateDetector(formikToDetector(props.values), 'model'))
       .then((resp: any) => {
         if (isEmpty(Object.keys(resp.response))) {
           setValidDetectorSettings(true);
           setValidModelConfigurations(true);
         } else {
-          if (resp.response.hasOwnProperty('detector')) {
-            const issueType = Object.keys(resp.response.detector)[0];
-            if (resp.response.detector[issueType].hasOwnProperty('message')) {
+          if (
+            resp.response.hasOwnProperty('detector') ||
+            resp.response.hasOwnProperty('model')
+          ) {
+            const validationType = Object.keys(resp.response)[0];
+            const issueType = Object.keys(resp.response[validationType])[0];
+            if (
+              resp.response[validationType][issueType].hasOwnProperty('message')
+            ) {
               const validationMessage =
-                resp.response.detector[issueType].message;
+                resp.response[validationType][issueType].message;
               const detectorSettingIssue: ValidationSettingResponse = {
                 issueType: issueType,
                 message: validationMessage,
+                validationType: validationType,
               };
+
+              // These issue types only come up during non-blocker validation after blocker validation has passed
+              // This means that the configurations don't have any blocking issues but request either timed out during
+              // non blocking validation or due to an issue in core. This means we aren't able to provide any recommendation
+              // and user has no way of re-trying except re-rendering page which isn't straightforward. At the moment we will
+              // hide these failures instead of explaining both levels of validation being done in the backend.
+              if (issueType == 'aggregation' || issueType == 'timeout') {
+                setValidDetectorSettings(true);
+                setValidModelConfigurations(true);
+                return;
+              }
+
               switch (issueType) {
+                // need to handle model validation issue case seperatly
                 case VALIDATION_ISSUE_TYPES.FEATURE_ATTRIBUTES:
                 case VALIDATION_ISSUE_TYPES.CATEGORY:
                 case VALIDATION_ISSUE_TYPES.SHINGLE_SIZE_FIELD:
-                  const modelResp = resp.response.detector[
+                  const modelResp = resp.response[validationType][
                     issueType
                   ] as ValidationModelResponse;
+                  modelResp.validationType = validationType;
                   setFeatureResponse(modelResp);
                   setValidDetectorSettings(true);
                   setValidModelConfigurations(false);
@@ -313,7 +332,6 @@ export function ReviewAndCreate(props: ReviewAndCreateProps) {
                 iconType="arrowLeft"
                 fill={false}
                 data-test-subj="reviewAndCreatePreviousButton"
-                //@ts-ignore
                 onClick={() => {
                   props.setStep(3);
                 }}
diff --git a/public/redux/reducers/ad.ts b/public/redux/reducers/ad.ts
@@ -377,10 +377,13 @@ export const createDetector = (requestBody: Detector): APIAction => ({
     }),
 });
 
-export const validateDetector = (requestBody: Detector): APIAction => ({
+export const validateDetector = (
+  requestBody: Detector,
+  validationType: string
+): APIAction => ({
   type: VALIDATE_DETECTOR,
   request: (client: HttpSetup) =>
-    client.post(`..${AD_NODE_API.DETECTOR}/_validate`, {
+    client.post(`..${AD_NODE_API.DETECTOR}/_validate/${validationType}`, {
       body: JSON.stringify(requestBody),
     }),
 });
diff --git a/server/cluster/ad/adPlugin.ts b/server/cluster/ad/adPlugin.ts
@@ -45,7 +45,13 @@ export default function adPlugin(Client: any, config: any, components: any) {
   });
   ad.validateDetector = ca({
     url: {
-      fmt: `${API.DETECTOR_BASE}/_validate`,
+      fmt: `${API.DETECTOR_BASE}/_validate/<%=validationType%>`,
+      req: {
+        validationType: {
+          type: 'string',
+          required: true,
+        },
+      },
     },
     needBody: true,
     method: 'POST',
diff --git a/server/routes/ad.ts b/server/routes/ad.ts
@@ -95,7 +95,10 @@ export function registerADRoutes(apiRouter: Router, adService: AdService) {
     '/detectors/{detectorId}/_topAnomalies/{isHistorical}',
     adService.getTopAnomalyResults
   );
-  apiRouter.post('/detectors/_validate', adService.validateDetector);
+  apiRouter.post(
+    '/detectors/_validate/{validationType}',
+    adService.validateDetector
+  );
 }
 
 export default class AdService {
@@ -230,13 +233,17 @@ export default class AdService {
     opensearchDashboardsResponse: OpenSearchDashboardsResponseFactory
   ): Promise<IOpenSearchDashboardsResponse<any>> => {
     try {
+      let { validationType } = request.params as {
+        validationType: string;
+      };
       const requestBody = JSON.stringify(
         convertPreviewInputKeysToSnakeCase(request.body)
       );
       const response = await this.client
         .asScoped(request)
         .callAsCurrentUser('ad.validateDetector', {
           body: requestBody,
+          validationType: validationType,
         });
       return opensearchDashboardsResponse.ok({
         body: {
