From 40fd867b65071c987b8db35351c7635d5bd90cf7 Mon Sep 17 00:00:00 2001 From: Melissa Alvarez Date: Fri, 29 Oct 2021 13:48:42 -0400 Subject: [PATCH] [ML] Data Frame Analytics Wizard: ensure includes updated correctly on dependent variable change (#116381) * ensure included fields not overwritten + reduce unnecessary renders. * ensure editor validation works * ensure depVar always in includes * ensure selected runtimeField depVar option is shown --- .../analysis_fields_table.tsx | 326 +++++++++--------- .../configuration_step_form.tsx | 1 - .../use_create_analytics_form/reducer.ts | 10 +- 3 files changed, 176 insertions(+), 161 deletions(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/components/configuration_step/analysis_fields_table.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/components/configuration_step/analysis_fields_table.tsx index 9dd4c5c42cca..8b7109d87a86 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/components/configuration_step/analysis_fields_table.tsx +++ b/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>; -}> = ({ - 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 ( - - - - - {tableItems.length > 0 && minimumFieldsRequiredMessage === undefined && ( - - {i18n.translate('xpack.ml.dataframe.analytics.create.includedFieldsCount', { - defaultMessage: - '{numFields, plural, one {# field} other {# fields}} included in the analysis', - values: { numFields: includes.length }, - })} - - )} - {tableItems.length === 0 && ( - + - - - )} - {tableItems.length > 0 && ( - - { - // 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'} - /> - - )} - - - ); -}; + + + {tableItems.length > 0 && minimumFieldsRequiredMessage === undefined && ( + + {i18n.translate('xpack.ml.dataframe.analytics.create.includedFieldsCount', { + defaultMessage: + '{numFields, plural, one {# field} other {# fields}} included in the analysis', + values: { numFields: includes.length }, + })} + + )} + {tableItems.length === 0 && ( + + + + )} + {tableItems.length > 0 && ( + + { + // 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'} + /> + + )} + + + ); + }, + (prevProps, nextProps) => { + return ( + prevProps.dependentVariable === nextProps.dependentVariable && + isEqual(prevProps.includes, nextProps.includes) && + isEqual(prevProps.tableItems, nextProps.tableItems) && + prevProps.unsupportedFieldsError === nextProps.unsupportedFieldsError + ); + } +); diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/components/configuration_step/configuration_step_form.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/components/configuration_step/configuration_step_form.tsx index df42c5b8eb1c..9bf751eec797 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_creation/components/configuration_step/configuration_step_form.tsx +++ b/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}`, }); } }); diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts index 9137ce42a465..c49653c5a75c 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts +++ b/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;