Skip to content

Commit

Permalink
ensure at least one field besides depVar included in analysis
Browse files Browse the repository at this point in the history
  • Loading branch information
alvarezmelissa87 committed May 5, 2020
1 parent f2d1095 commit cb5fdc4
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ import {
} from '../../../../common/analytics';
import { shouldAddAsDepVarOption, OMIT_FIELDS } from './form_options_validation';

const requiredFieldsErrorText = 'At least one field must be included in the analysis.';

export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, state }) => {
const {
services: { docLinks },
Expand Down Expand Up @@ -96,6 +98,7 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, sta
numTopFeatureImportanceValuesValid,
previousJobType,
previousSourceIndex,
requiredFieldsError,
sourceIndex,
sourceIndexNameEmpty,
sourceIndexNameValid,
Expand Down Expand Up @@ -158,6 +161,8 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, sta
};

const debouncedGetExplainData = debounce(async () => {
const jobTypeOrIndexChanged =
previousSourceIndex !== sourceIndex || previousJobType !== jobType;
const shouldUpdateModelMemoryLimit = !firstUpdate.current || !modelMemoryLimit;
const shouldUpdateEstimatedMml =
!firstUpdate.current || !modelMemoryLimit || estimatedModelMemoryLimit === '';
Expand All @@ -167,7 +172,7 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ 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,
});
Expand All @@ -186,8 +191,21 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, sta
setEstimatedModelMemoryLimit(expectedMemoryWithoutDisk);
}

const fieldSelection: FieldSelectionItem[] = 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) {
Expand All @@ -204,21 +222,24 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ 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
Expand Down Expand Up @@ -321,6 +342,7 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, sta
excludesOptions: [],
previousSourceIndex: sourceIndex,
sourceIndex: selectedOptions[0].label || '',
requiredFieldsError: undefined,
});
};

Expand Down Expand Up @@ -565,7 +587,9 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, sta
<Fragment>
<EuiFormRow
fullWidth
isInvalid={maxDistinctValuesError !== undefined}
isInvalid={
maxDistinctValuesError !== undefined || requiredFieldsError !== undefined
}
error={[
...(fieldOptionsFetchFail === true && maxDistinctValuesError !== undefined
? [
Expand All @@ -580,6 +604,14 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, sta
</Fragment>,
]
: []),
...(requiredFieldsError !== undefined
? [
i18n.translate('xpack.ml.dataframe.analytics.create.requiredFieldsError', {
defaultMessage: 'Invalid. {message}',
values: { message: requiredFieldsError },
}),
]
: []),
]}
>
<Fragment />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export const JobType: FC<Props> = ({ type, setFormState }) => {
previousJobType: type,
jobType: value,
excludes: [],
requiredFieldsError: undefined,
});
}}
data-test-subj="mlAnalyticsCreateJobFlyoutJobTypeSelect"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export const validateAdvancedEditor = (state: State): State => {
createIndexPattern,
excludes,
maxDistinctValuesError,
requiredFieldsError,
} = state.form;
const { jobConfig } = state;

Expand Down Expand Up @@ -330,6 +331,7 @@ export const validateAdvancedEditor = (state: State): State => {

state.isValid =
maxDistinctValuesError === undefined &&
requiredFieldsError === undefined &&
excludesValid &&
trainingPercentValid &&
state.form.modelMemoryLimitUnitValid &&
Expand Down Expand Up @@ -397,6 +399,7 @@ const validateForm = (state: State): State => {
maxDistinctValuesError,
modelMemoryLimit,
numTopFeatureImportanceValuesValid,
requiredFieldsError,
} = state.form;
const { estimatedModelMemoryLimit } = state;

Expand All @@ -412,6 +415,7 @@ const validateForm = (state: State): State => {

state.isValid =
maxDistinctValuesError === undefined &&
requiredFieldsError === undefined &&
!jobTypeEmpty &&
!mmlValidationResult &&
!jobIdEmpty &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export interface State {
numTopFeatureImportanceValuesValid: boolean;
previousJobType: null | AnalyticsJobType;
previousSourceIndex: EsIndexName | undefined;
requiredFieldsError: string | undefined;
sourceIndex: EsIndexName;
sourceIndexNameEmpty: boolean;
sourceIndexNameValid: boolean;
Expand Down Expand Up @@ -133,6 +134,7 @@ export const getInitialState = (): State => ({
numTopFeatureImportanceValuesValid: true,
previousJobType: null,
previousSourceIndex: undefined,
requiredFieldsError: undefined,
sourceIndex: '',
sourceIndexNameEmpty: true,
sourceIndexNameValid: false,
Expand Down

0 comments on commit cb5fdc4

Please sign in to comment.