From 5874b0a217ec9c1136fadf5205606e21118998a3 Mon Sep 17 00:00:00 2001 From: Melissa Alvarez Date: Thu, 7 May 2020 16:54:41 -0400 Subject: [PATCH] [ML] DFA: ensure at least one field is included in analysis before job can be created (#65320) * ensure at least one field besides depVar included in analysis * show requiredFieldsError above excluded fields * update jest test * update fieldSelection explainResponse type --- .../data_frame_analytics/common/analytics.ts | 2 +- .../create_analytics_form.test.tsx | 2 +- .../create_analytics_form.tsx | 61 ++++++++++++++++--- .../create_analytics_form/job_type.tsx | 1 + .../use_create_analytics_form/reducer.ts | 4 ++ .../hooks/use_create_analytics_form/state.ts | 2 + 6 files changed, 61 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts index fb3b2b3519947..7501fe3d82fc6 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/common/analytics.ts @@ -91,7 +91,7 @@ export interface FieldSelectionItem { } export interface DfAnalyticsExplainResponse { - field_selection: FieldSelectionItem[]; + field_selection?: FieldSelectionItem[]; memory_estimation: { expected_memory_without_disk: string; expected_memory_with_disk: string; diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.test.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.test.tsx index 92de5ad7be21e..85cd70912b41f 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.test.tsx +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.test.tsx @@ -53,7 +53,7 @@ describe('Data Frame Analytics: ', () => { ); const euiFormRows = wrapper.find('EuiFormRow'); - expect(euiFormRows.length).toBe(9); + expect(euiFormRows.length).toBe(10); const row1 = euiFormRows.at(0); expect(row1.find('label').text()).toBe('Job type'); diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx index 199100d8b5ab0..11052b171845d 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_form/create_analytics_form.tsx @@ -48,6 +48,13 @@ import { } from '../../../../common/analytics'; import { shouldAddAsDepVarOption, OMIT_FIELDS } from './form_options_validation'; +const requiredFieldsErrorText = i18n.translate( + 'xpack.ml.dataframe.analytics.create.requiredFieldsErrorMessage', + { + defaultMessage: 'At least one field must be included in the analysis.', + } +); + export const CreateAnalyticsForm: FC = ({ actions, state }) => { const { services: { docLinks }, @@ -96,6 +103,7 @@ export const CreateAnalyticsForm: FC = ({ actions, sta numTopFeatureImportanceValuesValid, previousJobType, previousSourceIndex, + requiredFieldsError, sourceIndex, sourceIndexNameEmpty, sourceIndexNameValid, @@ -158,6 +166,8 @@ export const CreateAnalyticsForm: FC = ({ actions, sta }; const debouncedGetExplainData = debounce(async () => { + const jobTypeOrIndexChanged = + previousSourceIndex !== sourceIndex || previousJobType !== jobType; const shouldUpdateModelMemoryLimit = !firstUpdate.current || !modelMemoryLimit; const shouldUpdateEstimatedMml = !firstUpdate.current || !modelMemoryLimit || estimatedModelMemoryLimit === ''; @@ -167,7 +177,7 @@ export const CreateAnalyticsForm: FC = ({ actions, sta } // Reset if sourceIndex or jobType changes (jobType requires dependent_variable to be set - // which won't be the case if switching from outlier detection) - if (previousSourceIndex !== sourceIndex || previousJobType !== jobType) { + if (jobTypeOrIndexChanged) { setFormState({ loadingFieldOptions: true, }); @@ -186,8 +196,21 @@ export const CreateAnalyticsForm: FC = ({ actions, sta setEstimatedModelMemoryLimit(expectedMemoryWithoutDisk); } + const fieldSelection: FieldSelectionItem[] | undefined = resp.field_selection; + + let hasRequiredFields = false; + if (fieldSelection) { + for (let i = 0; i < fieldSelection.length; i++) { + const field = fieldSelection[i]; + if (field.is_included === true && field.is_required === false) { + hasRequiredFields = true; + break; + } + } + } + // If sourceIndex has changed load analysis field options again - if (previousSourceIndex !== sourceIndex || previousJobType !== jobType) { + if (jobTypeOrIndexChanged) { const analyzedFieldsOptions: EuiComboBoxOptionOption[] = []; if (resp.field_selection) { @@ -204,21 +227,24 @@ export const CreateAnalyticsForm: FC = ({ actions, sta loadingFieldOptions: false, fieldOptionsFetchFail: false, maxDistinctValuesError: undefined, + requiredFieldsError: !hasRequiredFields ? requiredFieldsErrorText : undefined, }); } else { setFormState({ ...(shouldUpdateModelMemoryLimit ? { modelMemoryLimit: expectedMemoryWithoutDisk } : {}), + requiredFieldsError: !hasRequiredFields ? requiredFieldsErrorText : undefined, }); } } catch (e) { let errorMessage; if ( jobType === ANALYSIS_CONFIG_TYPE.CLASSIFICATION && - e.message !== undefined && - e.message.includes('status_exception') && - e.message.includes('must have at most') + e.body && + e.body.message !== undefined && + e.body.message.includes('status_exception') && + e.body.message.includes('must have at most') ) { - errorMessage = e.message; + errorMessage = e.body.message; } const fallbackModelMemoryLimit = jobType !== undefined @@ -321,6 +347,7 @@ export const CreateAnalyticsForm: FC = ({ actions, sta excludesOptions: [], previousSourceIndex: sourceIndex, sourceIndex: selectedOptions[0].label || '', + requiredFieldsError: undefined, }); }; @@ -368,6 +395,9 @@ export const CreateAnalyticsForm: FC = ({ actions, sta forceInput.current.dispatchEvent(evt); }, []); + const noSupportetdAnalysisFields = + excludesOptions.length === 0 && fieldOptionsFetchFail === false && !sourceIndexNameEmpty; + return ( @@ -715,18 +745,31 @@ export const CreateAnalyticsForm: FC = ({ actions, sta )} + + + = ({ type, setFormState }) => { previousJobType: type, jobType: value, excludes: [], + requiredFieldsError: undefined, }); }} data-test-subj="mlAnalyticsCreateJobFlyoutJobTypeSelect" 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 d55eb14a20e29..1cab42d8ee12d 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 @@ -124,6 +124,7 @@ export const validateAdvancedEditor = (state: State): State => { createIndexPattern, excludes, maxDistinctValuesError, + requiredFieldsError, } = state.form; const { jobConfig } = state; @@ -330,6 +331,7 @@ export const validateAdvancedEditor = (state: State): State => { state.isValid = maxDistinctValuesError === undefined && + requiredFieldsError === undefined && excludesValid && trainingPercentValid && state.form.modelMemoryLimitUnitValid && @@ -397,6 +399,7 @@ const validateForm = (state: State): State => { maxDistinctValuesError, modelMemoryLimit, numTopFeatureImportanceValuesValid, + requiredFieldsError, } = state.form; const { estimatedModelMemoryLimit } = state; @@ -412,6 +415,7 @@ const validateForm = (state: State): State => { state.isValid = maxDistinctValuesError === undefined && + requiredFieldsError === undefined && !jobTypeEmpty && !mmlValidationResult && !jobIdEmpty && diff --git a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts index 70840a442f6f6..8ca985a537b6e 100644 --- a/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts +++ b/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts @@ -76,6 +76,7 @@ export interface State { numTopFeatureImportanceValuesValid: boolean; previousJobType: null | AnalyticsJobType; previousSourceIndex: EsIndexName | undefined; + requiredFieldsError: string | undefined; sourceIndex: EsIndexName; sourceIndexNameEmpty: boolean; sourceIndexNameValid: boolean; @@ -133,6 +134,7 @@ export const getInitialState = (): State => ({ numTopFeatureImportanceValuesValid: true, previousJobType: null, previousSourceIndex: undefined, + requiredFieldsError: undefined, sourceIndex: '', sourceIndexNameEmpty: true, sourceIndexNameValid: false,