Skip to content

Commit

Permalink
[ML] Data frame analytics: Fix source index checks. (elastic#44479)
Browse files Browse the repository at this point in the history
The source index selection is based in Kibana index patterns. The form validation was too strict in that regard and used the same check as the destination index.
- Removes the check if the source index exists, it's not practical with index patterns containing wildcards.
- Changes the name validation to use the Kibana index pattern name check.
  • Loading branch information
walterra committed Sep 2, 2019
1 parent 813135b commit 9df52a2
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,15 @@ import {
import { i18n } from '@kbn/i18n';

import { metadata } from 'ui/metadata';
import { INDEX_PATTERN_ILLEGAL_CHARACTERS } from 'ui/index_patterns';

import { CreateAnalyticsFormProps } from '../../hooks/use_create_analytics_form';

// based on code used by `ui/index_patterns` internally
// remove the space character from the list of illegal characters
INDEX_PATTERN_ILLEGAL_CHARACTERS.pop();
const characterList = INDEX_PATTERN_ILLEGAL_CHARACTERS.join(', ');

export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, state }) => {
const { setFormState } = actions;

Expand All @@ -49,7 +55,6 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, sta
jobIdValid,
sourceIndex,
sourceIndexNameEmpty,
sourceIndexNameExists,
sourceIndexNameValid,
} = form;

Expand Down Expand Up @@ -119,39 +124,18 @@ export const CreateAnalyticsForm: FC<CreateAnalyticsFormProps> = ({ actions, sta
'This index pattern does not contain any numeric type fields. The analytics job may not be able to come up with any outliers.',
})
}
isInvalid={!sourceIndexNameEmpty && (!sourceIndexNameValid || !sourceIndexNameExists)}
isInvalid={!sourceIndexNameEmpty && !sourceIndexNameValid}
error={
(!sourceIndexNameEmpty &&
!sourceIndexNameValid && [
<Fragment>
{i18n.translate('xpack.ml.dataframe.analytics.create.sourceIndexInvalidError', {
defaultMessage: 'Invalid source index name.',
})}
<br />
<EuiLink
href={`https://www.elastic.co/guide/en/elasticsearch/reference/${metadata.branch}/indices-create-index.html#indices-create-index`}
target="_blank"
>
{i18n.translate(
'xpack.ml.dataframe.stepDetailsForm.sourceIndexInvalidErrorLink',
{
defaultMessage: 'Learn more about index name limitations.',
}
)}
</EuiLink>
</Fragment>,
]) ||
(!sourceIndexNameEmpty &&
!sourceIndexNameExists && [
<Fragment>
{i18n.translate(
'xpack.ml.dataframe.analytics.create.sourceIndexDoesNotExistError',
{
defaultMessage: 'An index with this name does not exist.',
}
)}
</Fragment>,
])
!sourceIndexNameEmpty &&
!sourceIndexNameValid && [
<Fragment>
{i18n.translate('xpack.ml.dataframe.analytics.create.sourceIndexInvalidError', {
defaultMessage:
'Invalid source index name, it cannot contain spaces or the characters: {characterList}',
values: { characterList },
})}
</Fragment>,
]
}
>
<Fragment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import { ACTION } from './actions';
import { reducer } from './reducer';
import { getInitialState } from './state';

jest.mock('ui/index_patterns', () => ({
validateIndexPattern: () => true,
}));

describe('useCreateAnalyticsForm', () => {
test('reducer(): provide a minimum required valid job config, then reset.', () => {
const initialState = getInitialState();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import { idx } from '@kbn/elastic-idx';
import { i18n } from '@kbn/i18n';

import { validateIndexPattern } from 'ui/index_patterns';

import { isValidIndexName } from '../../../../../../common/util/es_utils';

import { isAnalyticsIdValid } from '../../../../common';
Expand All @@ -22,7 +24,7 @@ const validateAdvancedEditor = (state: State): State => {

const sourceIndexName = idx(jobConfig, _ => _.source.index) || '';
const sourceIndexNameEmpty = sourceIndexName === '';
const sourceIndexNameValid = isValidIndexName(sourceIndexName);
const sourceIndexNameValid = validateIndexPattern(sourceIndexName);

const destinationIndexName = idx(jobConfig, _ => _.dest.index) || '';
const destinationIndexNameEmpty = destinationIndexName === '';
Expand Down Expand Up @@ -161,11 +163,9 @@ export function reducer(state: State, action: Action): State {
}

if (action.payload.sourceIndex !== undefined) {
newFormState.sourceIndexNameExists = state.indexNames.some(
name => newFormState.sourceIndex === name
);
newFormState.sourceIndexNameEmpty = newFormState.sourceIndex === '';
newFormState.sourceIndexNameValid = isValidIndexName(newFormState.sourceIndex);
const validationMessages = validateIndexPattern(newFormState.sourceIndex);
newFormState.sourceIndexNameValid = Object.keys(validationMessages).length === 0;
}

return state.isAdvancedEditorEnabled
Expand All @@ -177,9 +177,6 @@ export function reducer(state: State, action: Action): State {
newState.form.destinationIndexNameExists = newState.indexNames.some(
name => newState.form.destinationIndex === name
);
newState.form.sourceIndexNameExists = newState.indexNames.some(
name => newState.form.sourceIndex === name
);
return newState;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export interface State {
jobIdEmpty: boolean;
jobIdValid: boolean;
sourceIndex: EsIndexName;
sourceIndexNameExists: boolean;
sourceIndexNameEmpty: boolean;
sourceIndexNameValid: boolean;
};
Expand Down Expand Up @@ -68,7 +67,6 @@ export const getInitialState = (): State => ({
jobIdEmpty: true,
jobIdValid: false,
sourceIndex: '',
sourceIndexNameExists: false,
sourceIndexNameEmpty: true,
sourceIndexNameValid: false,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import { kibanaContextValueMock } from '../../../../../contexts/kibana/__mocks__

import { getErrorMessage, useCreateAnalyticsForm } from './use_create_analytics_form';

jest.mock('ui/index_patterns', () => ({
validateIndexPattern: () => true,
}));

const getMountedHook = () =>
mountHook(
() => useCreateAnalyticsForm(),
Expand Down

0 comments on commit 9df52a2

Please sign in to comment.