[ML] DF Analytics creation wizard: ensure user can switch back to for…

…m from JSON editor (#73752) (#73950) * wip: add reducer action to switch to form * rename getFormStateFromJobConfig * wip: types fix * show destIndex input when switching back from editor * ensure validation up to date when switching to form * cannot switch back to form if advanced config * update types * localization fix
elastic · Jul 31, 2020 · 3e97a4e · 3e97a4e
1 parent 88d1b2e
commit 3e97a4e
Show file tree

Hide file tree

Showing 9 changed files with 131 additions and 57 deletions.
diff --git a/...ta_frame_analytics/pages/analytics_creation/components/details_step/details_step_form.tsx b/...ta_frame_analytics/pages/analytics_creation/components/details_step/details_step_form.tsx
@@ -44,7 +44,7 @@ export const DetailsStepForm: FC<CreateAnalyticsStepProps> = ({
   const { ELASTIC_WEBSITE_URL, DOC_LINK_VERSION } = docLinks;
 
   const { setFormState } = actions;
-  const { form, cloneJob, isJobCreated } = state;
+  const { form, cloneJob, hasSwitchedToEditor, isJobCreated } = state;
   const {
     createIndexPattern,
     description,
@@ -61,7 +61,9 @@ export const DetailsStepForm: FC<CreateAnalyticsStepProps> = ({
     resultsField,
   } = form;
 
-  const [destIndexSameAsId, setDestIndexSameAsId] = useState<boolean>(cloneJob === undefined);
+  const [destIndexSameAsId, setDestIndexSameAsId] = useState<boolean>(
+    cloneJob === undefined && hasSwitchedToEditor === false
+  );
 
   const forceInput = useRef<HTMLInputElement | null>(null);
 
@@ -90,7 +92,11 @@ export const DetailsStepForm: FC<CreateAnalyticsStepProps> = ({
   useEffect(() => {
     if (destinationIndexNameValid === true) {
       debouncedIndexCheck();
-    } else if (destinationIndex.trim() === '' && destinationIndexNameExists === true) {
+    } else if (
+      typeof destinationIndex === 'string' &&
+      destinationIndex.trim() === '' &&
+      destinationIndexNameExists === true
+    ) {
       setFormState({ destinationIndexNameExists: false });
     }
 
@@ -102,7 +108,7 @@ export const DetailsStepForm: FC<CreateAnalyticsStepProps> = ({
   useEffect(() => {
     if (destIndexSameAsId === true && !jobIdEmpty && jobIdValid) {
       setFormState({ destinationIndex: jobId });
-    } else if (destIndexSameAsId === false) {
+    } else if (destIndexSameAsId === false && hasSwitchedToEditor === false) {
       setFormState({ destinationIndex: '' });
     }
   }, [destIndexSameAsId, jobId]);

diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/page.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/page.tsx
@@ -6,7 +6,6 @@
 
 import React, { FC, useEffect, useState } from 'react';
 import {
-  EuiButtonEmpty,
   EuiFlexGroup,
   EuiFlexItem,
   EuiFormRow,
@@ -16,7 +15,7 @@ import {
   EuiSpacer,
   EuiSteps,
   EuiStepStatus,
-  EuiText,
+  EuiSwitch,
   EuiTitle,
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
@@ -48,9 +47,15 @@ export const Page: FC<Props> = ({ jobId }) => {
   const { currentIndexPattern } = mlContext;
 
   const createAnalyticsForm = useCreateAnalyticsForm();
-  const { isAdvancedEditorEnabled } = createAnalyticsForm.state;
-  const { jobType } = createAnalyticsForm.state.form;
-  const { initiateWizard, setJobClone, switchToAdvancedEditor } = createAnalyticsForm.actions;
+  const { state } = createAnalyticsForm;
+  const { isAdvancedEditorEnabled, disableSwitchToForm } = state;
+  const { jobType } = state.form;
+  const {
+    initiateWizard,
+    setJobClone,
+    switchToAdvancedEditor,
+    switchToForm,
+  } = createAnalyticsForm.actions;
 
   useEffect(() => {
     initiateWizard();
@@ -170,34 +175,40 @@ export const Page: FC<Props> = ({ jobId }) => {
                 </EuiFlexItem>
               </EuiFlexGroup>
             </EuiFlexItem>
-            {isAdvancedEditorEnabled === false && (
-              <EuiFlexItem grow={false}>
-                <EuiFormRow
-                  helpText={i18n.translate(
-                    'xpack.ml.dataframe.analytics.create.enableJsonEditorHelpText',
+
+            <EuiFlexItem grow={false}>
+              <EuiFormRow
+                helpText={
+                  disableSwitchToForm &&
+                  i18n.translate(
+                    'xpack.ml.dataframe.analytics.create.jsonEditorDisabledSwitchText',
+                    {
+                      defaultMessage:
+                        'Configuration contains advanced fields not supported by the form. You cannot switch back to the form.',
+                    }
+                  )
+                }
+              >
+                <EuiSwitch
+                  disabled={jobType === undefined || disableSwitchToForm}
+                  label={i18n.translate(
+                    'xpack.ml.dataframe.analytics.create.switchToJsonEditorSwitch',
                     {
-                      defaultMessage: 'You cannot switch back to this form from the json editor.',
+                      defaultMessage: 'Switch to json editor',
                     }
                   )}
-                >
-                  <EuiButtonEmpty
-                    isDisabled={jobType === undefined}
-                    iconType="link"
-                    onClick={switchToAdvancedEditor}
-                    data-test-subj="mlAnalyticsCreateJobWizardAdvancedEditorSwitch"
-                  >
-                    <EuiText size="s" grow={false}>
-                      {i18n.translate(
-                        'xpack.ml.dataframe.analytics.create.switchToJsonEditorSwitch',
-                        {
-                          defaultMessage: 'Switch to json editor',
-                        }
-                      )}
-                    </EuiText>
-                  </EuiButtonEmpty>
-                </EuiFormRow>
-              </EuiFlexItem>
-            )}
+                  checked={isAdvancedEditorEnabled}
+                  onChange={(e) => {
+                    if (e.target.checked === true) {
+                      switchToAdvancedEditor();
+                    } else {
+                      switchToForm();
+                    }
+                  }}
+                  data-test-subj="mlAnalyticsCreateJobWizardAdvancedEditorSwitch"
+                />
+              </EuiFormRow>
+            </EuiFlexItem>
           </EuiFlexGroup>
           <EuiSpacer />
           {isAdvancedEditorEnabled === true && (

diff --git a/...ata_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/actions.ts b/...ata_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/actions.ts
@@ -25,6 +25,7 @@ export enum ACTION {
   SET_JOB_CONFIG,
   SET_JOB_IDS,
   SWITCH_TO_ADVANCED_EDITOR,
+  SWITCH_TO_FORM,
   SET_ESTIMATED_MODEL_MEMORY_LIMIT,
   SET_JOB_CLONE,
 }
@@ -38,7 +39,8 @@ export type Action =
         | ACTION.OPEN_MODAL
         | ACTION.RESET_ADVANCED_EDITOR_MESSAGES
         | ACTION.RESET_FORM
-        | ACTION.SWITCH_TO_ADVANCED_EDITOR;
+        | ACTION.SWITCH_TO_ADVANCED_EDITOR
+        | ACTION.SWITCH_TO_FORM;
     }
   // Actions with custom payloads:
   | { type: ACTION.ADD_REQUEST_MESSAGE; requestMessage: FormMessage }
@@ -71,6 +73,7 @@ export interface ActionDispatchers {
   setJobConfig: (payload: State['jobConfig']) => void;
   startAnalyticsJob: () => void;
   switchToAdvancedEditor: () => void;
+  switchToForm: () => void;
   setEstimatedModelMemoryLimit: (value: State['estimatedModelMemoryLimit']) => void;
   setJobClone: (cloneJob: DeepReadonly<DataFrameAnalyticsConfig>) => Promise<void>;
 }
diff --git a/...ata_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts b/...ata_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts
@@ -8,13 +8,17 @@ import { i18n } from '@kbn/i18n';
 import { memoize } from 'lodash';
 // @ts-ignore
 import numeral from '@elastic/numeral';
-import { isEmpty } from 'lodash';
 import { isValidIndexName } from '../../../../../../../common/util/es_utils';
 
 import { collapseLiteralStrings } from '../../../../../../../../../../src/plugins/es_ui_shared/public';
 
 import { Action, ACTION } from './actions';
-import { getInitialState, getJobConfigFromFormState, State } from './state';
+import {
+  getInitialState,
+  getFormStateFromJobConfig,
+  getJobConfigFromFormState,
+  State,
+} from './state';
 import {
   isJobIdValid,
   validateModelMemoryLimitUnits,
@@ -41,6 +45,7 @@ import {
   TRAINING_PERCENT_MAX,
 } from '../../../../common/analytics';
 import { indexPatterns } from '../../../../../../../../../../src/plugins/data/public';
+import { isAdvancedConfig } from '../../components/action_clone/clone_button';
 
 const mmlAllowedUnitsStr = `${ALLOWED_DATA_UNITS.slice(0, ALLOWED_DATA_UNITS.length - 1).join(
   ', '
@@ -458,13 +463,16 @@ export function reducer(state: State, action: Action): State {
 
     case ACTION.SET_ADVANCED_EDITOR_RAW_STRING:
       let resultJobConfig;
+      let disableSwitchToForm = false;
       try {
         resultJobConfig = JSON.parse(collapseLiteralStrings(action.advancedEditorRawString));
+        disableSwitchToForm = isAdvancedConfig(resultJobConfig);
       } catch (e) {
         return {
           ...state,
           advancedEditorRawString: action.advancedEditorRawString,
           isAdvancedEditorValidJson: false,
+          disableSwitchToForm: true,
           advancedEditorMessages: [],
         };
       }
@@ -473,6 +481,7 @@ export function reducer(state: State, action: Action): State {
         ...validateAdvancedEditor({ ...state, jobConfig: resultJobConfig }),
         advancedEditorRawString: action.advancedEditorRawString,
         isAdvancedEditorValidJson: true,
+        disableSwitchToForm,
       };
 
     case ACTION.SET_FORM_STATE:
@@ -538,17 +547,53 @@ export function reducer(state: State, action: Action): State {
 
     case ACTION.SWITCH_TO_ADVANCED_EDITOR:
       let { jobConfig } = state;
-      const isJobConfigEmpty = isEmpty(state.jobConfig);
-      if (isJobConfigEmpty) {
-        jobConfig = getJobConfigFromFormState(state.form);
-      }
+      jobConfig = getJobConfigFromFormState(state.form);
+      const shouldDisableSwitchToForm = isAdvancedConfig(jobConfig);
+
       return validateAdvancedEditor({
         ...state,
         advancedEditorRawString: JSON.stringify(jobConfig, null, 2),
         isAdvancedEditorEnabled: true,
+        disableSwitchToForm: shouldDisableSwitchToForm,
+        hasSwitchedToEditor: true,
         jobConfig,
       });
 
+    case ACTION.SWITCH_TO_FORM:
+      const { jobConfig: config, jobIds } = state;
+      const { jobId } = state.form;
+      // @ts-ignore
+      const formState = getFormStateFromJobConfig(config, false);
+
+      if (typeof jobId === 'string' && jobId.trim() !== '') {
+        formState.jobId = jobId;
+      }
+
+      formState.jobIdExists = jobIds.some((id) => formState.jobId === id);
+      formState.jobIdEmpty = jobId === '';
+      formState.jobIdValid = isJobIdValid(jobId);
+      formState.jobIdInvalidMaxLength = !!maxLengthValidator(JOB_ID_MAX_LENGTH)(jobId);
+
+      formState.destinationIndexNameEmpty = formState.destinationIndex === '';
+      formState.destinationIndexNameValid = isValidIndexName(formState.destinationIndex || '');
+      formState.destinationIndexPatternTitleExists =
+        state.indexPatternsMap[formState.destinationIndex || ''] !== undefined;
+
+      if (formState.numTopFeatureImportanceValues !== undefined) {
+        formState.numTopFeatureImportanceValuesValid = validateNumTopFeatureImportanceValues(
+          formState.numTopFeatureImportanceValues
+        );
+      }
+
+      return validateForm({
+        ...state,
+        // @ts-ignore
+        form: formState,
+        isAdvancedEditorEnabled: false,
+        advancedEditorRawString: JSON.stringify(config, null, 2),
+        jobConfig: config,
+      });
+
     case ACTION.SET_ESTIMATED_MODEL_MEMORY_LIMIT:
       return {
         ...state,

diff --git a/..._frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.test.ts b/..._frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.test.ts
@@ -4,11 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import {
-  getCloneFormStateFromJobConfig,
-  getInitialState,
-  getJobConfigFromFormState,
-} from './state';
+import { getFormStateFromJobConfig, getInitialState, getJobConfigFromFormState } from './state';
 
 const regJobConfig = {
   id: 'reg-test-01',
@@ -96,8 +92,8 @@ describe('useCreateAnalyticsForm', () => {
     ]);
   });
 
-  test('state: getCloneFormStateFromJobConfig() regression', () => {
-    const clonedState = getCloneFormStateFromJobConfig(regJobConfig);
+  test('state: getFormStateFromJobConfig() regression', () => {
+    const clonedState = getFormStateFromJobConfig(regJobConfig);
 
     expect(clonedState?.sourceIndex).toBe('reg-test-index');
     expect(clonedState?.includes).toStrictEqual([]);
@@ -112,8 +108,8 @@ describe('useCreateAnalyticsForm', () => {
     expect(clonedState?.jobId).toBe(undefined);
   });
 
-  test('state: getCloneFormStateFromJobConfig() outlier detection', () => {
-    const clonedState = getCloneFormStateFromJobConfig(outlierJobConfig);
+  test('state: getFormStateFromJobConfig() outlier detection', () => {
+    const clonedState = getFormStateFromJobConfig(outlierJobConfig);
 
     expect(clonedState?.sourceIndex).toBe('outlier-test-index');
     expect(clonedState?.includes).toStrictEqual(['field', 'other_field']);

diff --git a/.../data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts b/.../data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts
@@ -12,6 +12,7 @@ import {
   DataFrameAnalyticsId,
   DataFrameAnalyticsConfig,
   ANALYSIS_CONFIG_TYPE,
+  defaultSearchQuery,
 } from '../../../../common/analytics';
 import { CloneDataFrameAnalyticsConfig } from '../../components/action_clone';
 
@@ -44,6 +45,7 @@ export interface FormMessage {
 export interface State {
   advancedEditorMessages: FormMessage[];
   advancedEditorRawString: string;
+  disableSwitchToForm: boolean;
   form: {
     computeFeatureInfluence: string;
     createIndexPattern: boolean;
@@ -97,6 +99,7 @@ export interface State {
   indexPatternsMap: SourceIndexMap;
   isAdvancedEditorEnabled: boolean;
   isAdvancedEditorValidJson: boolean;
+  hasSwitchedToEditor: boolean;
   isJobCreated: boolean;
   isJobStarted: boolean;
   isValid: boolean;
@@ -110,6 +113,7 @@ export interface State {
 export const getInitialState = (): State => ({
   advancedEditorMessages: [],
   advancedEditorRawString: '',
+  disableSwitchToForm: false,
   form: {
     computeFeatureInfluence: 'true',
     createIndexPattern: true,
@@ -131,7 +135,7 @@ export const getInitialState = (): State => ({
     jobIdInvalidMaxLength: false,
     jobIdValid: false,
     jobType: undefined,
-    jobConfigQuery: { match_all: {} },
+    jobConfigQuery: defaultSearchQuery,
     jobConfigQueryString: undefined,
     lambda: undefined,
     loadingFieldOptions: false,
@@ -167,6 +171,7 @@ export const getInitialState = (): State => ({
   indexPatternsMap: {},
   isAdvancedEditorEnabled: false,
   isAdvancedEditorValidJson: true,
+  hasSwitchedToEditor: false,
   isJobCreated: false,
   isJobStarted: false,
   isValid: false,
@@ -283,8 +288,9 @@ function toCamelCase(property: string): string {
  * Extracts form state for a job clone from the analytics job configuration.
  * For cloning we keep job id and destination index empty.
  */
-export function getCloneFormStateFromJobConfig(
-  analyticsJobConfig: Readonly<CloneDataFrameAnalyticsConfig>
+export function getFormStateFromJobConfig(
+  analyticsJobConfig: Readonly<CloneDataFrameAnalyticsConfig>,
+  isClone: boolean = true
 ): Partial<State['form']> {
   const jobType = Object.keys(analyticsJobConfig.analysis)[0] as ANALYSIS_CONFIG_TYPE;
 
@@ -300,6 +306,10 @@ export function getCloneFormStateFromJobConfig(
     includes: analyticsJobConfig.analyzed_fields.includes,
   };
 
+  if (isClone === false) {
+    resultState.destinationIndex = analyticsJobConfig?.dest.index ?? '';
+  }
+
   const analysisConfig = analyticsJobConfig.analysis[jobType];
 
   for (const key in analysisConfig) {