[ML] Data Frame Analytics Wizard: ensure includes updated correctly on dependent variable change (#116381) (#116805)

* ensure included fields not overwritten + reduce unnecessary renders.

* ensure editor validation works

* ensure depVar always in includes

* ensure selected runtimeField depVar option is shown

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/components/configuration_step/analysis_fields_table.tsx

@@ -7,6 +7,7 @@
 
 import React, { FC, Fragment, useEffect, useState } from 'react';
 import { EuiCallOut, EuiFormRow, EuiPanel, EuiSpacer, EuiText } from '@elastic/eui';
+import { isEqual } from 'lodash';
 // @ts-ignore no declaration
 import { LEFT_ALIGNMENT, CENTER_ALIGNMENT, SortableProperties } from '@elastic/eui/lib/services';
 import { i18n } from '@kbn/i18n';
@@ -90,167 +91,182 @@ export const AnalysisFieldsTable: FC<{
   tableItems: FieldSelectionItem[];
   unsupportedFieldsError?: string;
   setUnsupportedFieldsError: React.Dispatch<React.SetStateAction<any>>;
-}> = ({
-  dependentVariable,
-  includes,
-  setFormState,
-  minimumFieldsRequiredMessage,
-  setMinimumFieldsRequiredMessage,
-  tableItems,
-  unsupportedFieldsError,
-  setUnsupportedFieldsError,
-}) => {
-  const [sortableProperties, setSortableProperties] = useState();
-  const [currentPaginationData, setCurrentPaginationData] = useState<{
-    pageIndex: number;
-    itemsPerPage: number;
-  }>({ pageIndex: 0, itemsPerPage: 5 });
+}> = React.memo(
+  ({
+    dependentVariable,
+    includes,
+    setFormState,
+    minimumFieldsRequiredMessage,
+    setMinimumFieldsRequiredMessage,
+    tableItems,
+    unsupportedFieldsError,
+    setUnsupportedFieldsError,
+  }) => {
+    const [sortableProperties, setSortableProperties] = useState();
+    const [currentPaginationData, setCurrentPaginationData] = useState<{
+      pageIndex: number;
+      itemsPerPage: number;
+    }>({ pageIndex: 0, itemsPerPage: 5 });
 
-  useEffect(() => {
-    if (includes.length === 0 && tableItems.length > 0) {
-      const includedFields: string[] = [];
-      tableItems.forEach((field) => {
-        if (field.is_included === true) {
-          includedFields.push(field.name);
-        }
-      });
-      setFormState({ includes: includedFields });
-    } else if (includes.length > 0) {
-      setFormState({ includes });
-    }
-    setMinimumFieldsRequiredMessage(undefined);
-  }, [tableItems]);
+    useEffect(() => {
+      if (includes.length === 0 && tableItems.length > 0) {
+        const includedFields: string[] = [];
+        tableItems.forEach((field) => {
+          if (field.is_included === true) {
+            includedFields.push(field.name);
+          }
+        });
+        setFormState({ includes: includedFields });
+      } else if (includes.length > 0) {
+        setFormState({
+          includes:
+            dependentVariable && includes.includes(dependentVariable)
+              ? includes
+              : [...includes, dependentVariable],
+        });
+      }
+      setMinimumFieldsRequiredMessage(undefined);
+    }, [tableItems]);
 
-  useEffect(() => {
-    let sortablePropertyItems = [];
-    const defaultSortProperty = 'name';
+    useEffect(() => {
+      let sortablePropertyItems = [];
+      const defaultSortProperty = 'name';
 
-    sortablePropertyItems = [
+      sortablePropertyItems = [
+        {
+          name: 'name',
+          getValue: (item: any) => item.name.toLowerCase(),
+          isAscending: true,
+        },
+        {
+          name: 'is_included',
+          getValue: (item: any) => item.is_included,
+          isAscending: true,
+        },
+        {
+          name: 'is_required',
+          getValue: (item: any) => item.is_required,
+          isAscending: true,
+        },
+      ];
+      const sortableProps = new SortableProperties(sortablePropertyItems, defaultSortProperty);
+
+      setSortableProperties(sortableProps);
+    }, []);
+
+    const filters = [
       {
-        name: 'name',
-        getValue: (item: any) => item.name.toLowerCase(),
-        isAscending: true,
-      },
-      {
-        name: 'is_included',
-        getValue: (item: any) => item.is_included,
-        isAscending: true,
-      },
-      {
-        name: 'is_required',
-        getValue: (item: any) => item.is_required,
-        isAscending: true,
+        type: 'field_value_toggle_group',
+        field: 'is_included',
+        items: [
+          {
+            value: true,
+            name: i18n.translate('xpack.ml.dataframe.analytics.create.isIncludedOption', {
+              defaultMessage: 'Is included',
+            }),
+          },
+          {
+            value: false,
+            name: i18n.translate('xpack.ml.dataframe.analytics.create.isNotIncludedOption', {
+              defaultMessage: 'Is not included',
+            }),
+          },
+        ],
       },
     ];
-    const sortableProps = new SortableProperties(sortablePropertyItems, defaultSortProperty);
 
-    setSortableProperties(sortableProps);
-  }, []);
-
-  const filters = [
-    {
-      type: 'field_value_toggle_group',
-      field: 'is_included',
-      items: [
-        {
-          value: true,
-          name: i18n.translate('xpack.ml.dataframe.analytics.create.isIncludedOption', {
-            defaultMessage: 'Is included',
-          }),
-        },
-        {
-          value: false,
-          name: i18n.translate('xpack.ml.dataframe.analytics.create.isNotIncludedOption', {
-            defaultMessage: 'Is not included',
-          }),
-        },
-      ],
-    },
-  ];
-
-  return (
-    <Fragment>
-      <EuiFormRow
-        data-test-subj="mlAnalyticsCreateJobWizardIncludesTable"
-        label={i18n.translate('xpack.ml.dataframe.analytics.create.includedFieldsLabel', {
-          defaultMessage: 'Included fields',
-        })}
-        fullWidth
-        isInvalid={
-          minimumFieldsRequiredMessage !== undefined || unsupportedFieldsError !== undefined
-        }
-        error={[
-          ...(minimumFieldsRequiredMessage !== undefined ? [minimumFieldsRequiredMessage] : []),
-          ...(unsupportedFieldsError !== undefined
-            ? [
-                i18n.translate('xpack.ml.dataframe.analytics.create.unsupportedFieldsError', {
-                  defaultMessage: 'Invalid. {message}',
-                  values: { message: unsupportedFieldsError },
-                }),
-              ]
-            : []),
-        ]}
-      >
-        <Fragment />
-      </EuiFormRow>
-      {tableItems.length > 0 && minimumFieldsRequiredMessage === undefined && (
-        <EuiText size="xs">
-          {i18n.translate('xpack.ml.dataframe.analytics.create.includedFieldsCount', {
-            defaultMessage:
-              '{numFields, plural, one {# field} other {# fields}} included in the analysis',
-            values: { numFields: includes.length },
-          })}
-        </EuiText>
-      )}
-      {tableItems.length === 0 && (
-        <EuiCallOut
-          title={i18n.translate('xpack.ml.dataframe.analytics.create.calloutTitle', {
-            defaultMessage: 'Analysis fields not available',
+    return (
+      <Fragment>
+        <EuiFormRow
+          data-test-subj="mlAnalyticsCreateJobWizardIncludesTable"
+          label={i18n.translate('xpack.ml.dataframe.analytics.create.includedFieldsLabel', {
+            defaultMessage: 'Included fields',
           })}
+          fullWidth
+          isInvalid={
+            minimumFieldsRequiredMessage !== undefined || unsupportedFieldsError !== undefined
+          }
+          error={[
+            ...(minimumFieldsRequiredMessage !== undefined ? [minimumFieldsRequiredMessage] : []),
+            ...(unsupportedFieldsError !== undefined
+              ? [
+                  i18n.translate('xpack.ml.dataframe.analytics.create.unsupportedFieldsError', {
+                    defaultMessage: 'Invalid. {message}',
+                    values: { message: unsupportedFieldsError },
+                  }),
+                ]
+              : []),
+          ]}
         >
-          <FormattedMessage
-            id="xpack.ml.dataframe.analytics.create.calloutMessage"
-            defaultMessage="Additional data required to load analysis fields."
-          />
-        </EuiCallOut>
-      )}
-      {tableItems.length > 0 && (
-        <EuiPanel paddingSize="m" data-test-subj="mlAnalyticsCreateJobWizardIncludesSelect">
-          <CustomSelectionTable
-            currentPage={currentPaginationData.pageIndex}
-            data-test-subj="mlAnalyticsCreationAnalysisFieldsTable"
-            checkboxDisabledCheck={checkboxDisabledCheck}
-            columns={columns}
-            filters={filters}
-            items={tableItems}
-            itemsPerPage={currentPaginationData.itemsPerPage}
-            onTableChange={(selection: string[]) => {
-              // dependent variable must always be in includes
-              if (
-                dependentVariable !== undefined &&
-                dependentVariable !== '' &&
-                selection.length === 0
-              ) {
-                selection = [dependentVariable];
-              }
-              // If includes is empty show minimum fields required message and don't update form yet
-              if (selection.length === 0) {
-                setMinimumFieldsRequiredMessage(minimumFieldsMessage);
-                setUnsupportedFieldsError(undefined);
-              } else {
-                setMinimumFieldsRequiredMessage(undefined);
-                setFormState({ includes: selection });
-              }
-            }}
-            selectedIds={includes}
-            setCurrentPaginationData={setCurrentPaginationData}
-            singleSelection={false}
-            sortableProperties={sortableProperties}
-            tableItemId={'name'}
-          />
-        </EuiPanel>
-      )}
-      <EuiSpacer />
-    </Fragment>
-  );
-};
+          <Fragment />
+        </EuiFormRow>
+        {tableItems.length > 0 && minimumFieldsRequiredMessage === undefined && (
+          <EuiText size="xs">
+            {i18n.translate('xpack.ml.dataframe.analytics.create.includedFieldsCount', {
+              defaultMessage:
+                '{numFields, plural, one {# field} other {# fields}} included in the analysis',
+              values: { numFields: includes.length },
+            })}
+          </EuiText>
+        )}
+        {tableItems.length === 0 && (
+          <EuiCallOut
+            title={i18n.translate('xpack.ml.dataframe.analytics.create.calloutTitle', {
+              defaultMessage: 'Analysis fields not available',
+            })}
+          >
+            <FormattedMessage
+              id="xpack.ml.dataframe.analytics.create.calloutMessage"
+              defaultMessage="Additional data required to load analysis fields."
+            />
+          </EuiCallOut>
+        )}
+        {tableItems.length > 0 && (
+          <EuiPanel paddingSize="m" data-test-subj="mlAnalyticsCreateJobWizardIncludesSelect">
+            <CustomSelectionTable
+              currentPage={currentPaginationData.pageIndex}
+              data-test-subj="mlAnalyticsCreationAnalysisFieldsTable"
+              checkboxDisabledCheck={checkboxDisabledCheck}
+              columns={columns}
+              filters={filters}
+              items={tableItems}
+              itemsPerPage={currentPaginationData.itemsPerPage}
+              onTableChange={(selection: string[]) => {
+                // dependent variable must always be in includes
+                if (
+                  dependentVariable !== undefined &&
+                  dependentVariable !== '' &&
+                  selection.length === 0
+                ) {
+                  selection = [dependentVariable];
+                }
+                // If includes is empty show minimum fields required message and don't update form yet
+                if (selection.length === 0) {
+                  setMinimumFieldsRequiredMessage(minimumFieldsMessage);
+                  setUnsupportedFieldsError(undefined);
+                } else {
+                  setMinimumFieldsRequiredMessage(undefined);
+                  setFormState({ includes: selection });
+                }
+              }}
+              selectedIds={includes}
+              setCurrentPaginationData={setCurrentPaginationData}
+              singleSelection={false}
+              sortableProperties={sortableProperties}
+              tableItemId={'name'}
+            />
+          </EuiPanel>
+        )}
+        <EuiSpacer />
+      </Fragment>
+    );
+  },
+  (prevProps, nextProps) => {
+    return (
+      prevProps.dependentVariable === nextProps.dependentVariable &&
+      isEqual(prevProps.includes, nextProps.includes) &&
+      isEqual(prevProps.tableItems, nextProps.tableItems) &&
+      prevProps.unsupportedFieldsError === nextProps.unsupportedFieldsError
+    );
+  }
+);

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/components/configuration_step/configuration_step_form.tsx

@@ -88,7 +88,6 @@ function getRuntimeDepVarOptions(jobType: AnalyticsJobType, runtimeMappings: Run
     if (isRuntimeField(field) && shouldAddAsDepVarOption(id, field.type, jobType)) {
       runtimeOptions.push({
         label: id,
-        key: `runtime_mapping_${id}`,
       });
     }
   });

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts

@@ -144,7 +144,7 @@ export const validateNumTopFeatureImportanceValues = (
 };
 
 export const validateAdvancedEditor = (state: State): State => {
-  const { jobIdEmpty, jobIdValid, jobIdExists, jobType, createIndexPattern, includes } = state.form;
+  const { jobIdEmpty, jobIdValid, jobIdExists, jobType, createIndexPattern } = state.form;
   const { jobConfig } = state;
 
   state.advancedEditorMessages = [];
@@ -160,6 +160,8 @@ export const validateAdvancedEditor = (state: State): State => {
   const destinationIndexPatternTitleExists =
     state.indexPatternsMap[destinationIndexName] !== undefined;
 
+  const analyzedFields = jobConfig?.analyzed_fields?.includes || [];
+
   const resultsFieldEmptyString =
     typeof jobConfig?.dest?.results_field === 'string' &&
     jobConfig?.dest?.results_field.trim() === '';
@@ -189,12 +191,10 @@ export const validateAdvancedEditor = (state: State): State => {
   ) {
     const dependentVariableName = getDependentVar(jobConfig.analysis) || '';
     dependentVariableEmpty = dependentVariableName === '';
-
     if (
       !dependentVariableEmpty &&
-      includes !== undefined &&
-      includes.length > 0 &&
-      !includes.includes(dependentVariableName)
+      analyzedFields.length > 0 &&
+      !analyzedFields.includes(dependentVariableName)
     ) {
       includesValid = false;