maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[Index management] Fix regression on mappings editor forms (#62437)

Browse source
This commit is contained in:
Sébastien Loix 2020-04-06 14:40:16 +02:00 committed by GitHub
parent 9e9928f35c
commit e732196238
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 329 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts
View file

@ -431,6 +431,7 @@ export const useField = (
errors,
form,
isPristine,
isValid: errors.length === 0,
isValidating,
isValidated,
isChangingValue,

21
src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts
View file

@ -129,8 +129,7 @@ export function useForm<T extends FormData = FormData>(
}, [] as string[]);
};
const isFieldValid = (field: FieldHook) =>
field.getErrorsMessages() === null && !field.isValidating;
const isFieldValid = (field: FieldHook) => field.isValid && !field.isValidating;
const updateFormValidity = () => {
const fieldsArray = fieldsToArray();
@ -167,14 +166,24 @@ export function useForm<T extends FormData = FormData>(
};
const validateAllFields = async (): Promise<boolean> => {
const fieldsToValidate = fieldsToArray().filter(field => !field.isValidated);
const fieldsArray = fieldsToArray();
const fieldsToValidate = fieldsArray.filter(field => !field.isValidated);
let isFormValid: boolean | undefined = isValid;
if (fieldsToValidate.length === 0) {
// Nothing left to validate, all fields are already validated.
return isValid!;
if (isFormValid === undefined) {
// We should never enter this condition as the form validity is updated each time
// a field is validated. But sometimes, during tests it does not happen and we need
// to wait the next tick (hooks lifecycle being tricky) to make sure the "isValid" state is updated.
// In order to avoid this unintentional behaviour, we add this if condition here.
isFormValid = fieldsArray.every(isFieldValid);
setIsValid(isFormValid);
}
return isFormValid;
}
const { isFormValid } = await validateFields(fieldsToValidate.map(field => field.path));
({ isFormValid } = await validateFields(fieldsToValidate.map(field => field.path)));
return isFormValid!;
};

1
src/plugins/es_ui_shared/static/forms/hook_form_lib/types.ts
View file

@ -94,6 +94,7 @@ export interface FieldHook {
readonly type: string;
readonly value: unknown;
readonly errors: ValidationError[];
readonly isValid: boolean;
readonly isPristine: boolean;
readonly isValidating: boolean;
readonly isValidated: boolean;

24
x-pack/plugins/index_management/__jest__/client_integration/helpers/template_form.helpers.ts
View file

@ -44,7 +44,7 @@ export const formSetup = async (initTestBed: SetupFunc<TestSubjects>) => {
testBed.find('editFieldUpdateButton').simulate('click');
};
const clickRemoveButtonAtField = (index: number) => {
const deleteMappingsFieldAt = (index: number) => {
testBed
.find('removeFieldButton')
.at(index)
@ -54,7 +54,7 @@ export const formSetup = async (initTestBed: SetupFunc<TestSubjects>) => {
};
const clickCancelCreateFieldButton = () => {
testBed.find('createFieldWrapper.cancelButton').simulate('click');
testBed.find('createFieldForm.cancelButton').simulate('click');
};
const completeStepOne = async ({
@ -154,7 +154,7 @@ export const formSetup = async (initTestBed: SetupFunc<TestSubjects>) => {
const { find, form, component } = testBed;
form.setInputValue('nameParameterInput', name);
find('createFieldWrapper.mockComboBox').simulate('change', [
find('createFieldForm.mockComboBox').simulate('change', [
{
label: type,
value: type,
@ -164,7 +164,7 @@ export const formSetup = async (initTestBed: SetupFunc<TestSubjects>) => {
await nextTick(50);
component.update();
find('createFieldWrapper.addButton').simulate('click');
find('createFieldForm.addButton').simulate('click');
await nextTick();
component.update();
@ -178,7 +178,7 @@ export const formSetup = async (initTestBed: SetupFunc<TestSubjects>) => {
clickSubmitButton,
clickEditButtonAtField,
clickEditFieldUpdateButton,
clickRemoveButtonAtField,
deleteMappingsFieldAt,
clickCancelCreateFieldButton,
completeStepOne,
completeStepTwo,
@ -196,16 +196,16 @@ export type TestSubjects =
| 'backButton'
| 'codeEditorContainer'
| 'confirmModalConfirmButton'
| 'createFieldWrapper.addPropertyButton'
| 'createFieldWrapper.addButton'
| 'createFieldWrapper.addFieldButton'
| 'createFieldWrapper.addMultiFieldButton'
| 'createFieldWrapper.cancelButton'
| 'createFieldWrapper.mockComboBox'
| 'createFieldForm.addPropertyButton'
| 'createFieldForm.addButton'
| 'createFieldForm.addFieldButton'
| 'createFieldForm.addMultiFieldButton'
| 'createFieldForm.cancelButton'
| 'createFieldForm.mockComboBox'
| 'editFieldButton'
| 'editFieldUpdateButton'
| 'fieldsListItem'
| 'fieldTypeComboBox'
| 'fieldType'
| 'indexPatternsField'
| 'indexPatternsWarning'
| 'indexPatternsWarningDescription'

2
x-pack/plugins/index_management/__jest__/client_integration/template_create.test.tsx
View file

@ -178,7 +178,7 @@ describe('<TemplateCreate />', () => {
actions.clickCancelCreateFieldButton();
// Remove first field
actions.clickRemoveButtonAtField(0);
actions.deleteMappingsFieldAt(0);
expect(find('fieldsListItem').length).toBe(1);
});

4
x-pack/plugins/index_management/public/application/components/mappings_editor/__jest__/client_integration/helpers/index.ts
View file

@ -3,7 +3,7 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { setup as mappingsEditorSetup } from './mappings_editor.helpers';
import { setup as mappingsEditorSetup, MappingsEditorTestBed } from './mappings_editor.helpers';
export {
nextTick,
@ -15,3 +15,5 @@ export {
export const componentHelpers = {
mappingsEditor: { setup: mappingsEditorSetup },
};
export { MappingsEditorTestBed };

16
x-pack/plugins/index_management/public/application/components/mappings_editor/__jest__/client_integration/helpers/mappings_editor.helpers.ts
View file

@ -1,16 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { registerTestBed } from '../../../../../../../../../test_utils';
import { MappingsEditor } from '../../../mappings_editor';
export const setup = (props: any) =>
registerTestBed(MappingsEditor, {
memoryRouter: {
wrapComponent: false,
},
defaultProps: props,
})();

132
x-pack/plugins/index_management/public/application/components/mappings_editor/__jest__/client_integration/helpers/mappings_editor.helpers.tsx Normal file
View file

@ -0,0 +1,132 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import React from 'react';
import { registerTestBed, TestBed, nextTick } from '../../../../../../../../../test_utils';
import { MappingsEditor } from '../../../mappings_editor';
jest.mock('@elastic/eui', () => ({
...jest.requireActual('@elastic/eui'),
// Mocking EuiComboBox, as it utilizes "react-virtualized" for rendering search suggestions,
// which does not produce a valid component wrapper
EuiComboBox: (props: any) => (
<input
data-test-subj={props['data-test-subj'] || 'mockComboBox'}
onChange={async (syntheticEvent: any) => {
props.onChange([syntheticEvent['0']]);
}}
/>
),
// Mocking EuiCodeEditor, which uses React Ace under the hood
EuiCodeEditor: (props: any) => (
<input
data-test-subj={props['data-test-subj'] || 'mockCodeEditor'}
data-currentvalue={props.value}
onChange={(e: any) => {
props.onChange(e.jsonContent);
}}
/>
),
}));
const createActions = (testBed: TestBed<TestSubjects>) => {
const { find, waitFor, form, component } = testBed;
const addField = async (name: string, type: string) => {
const currentCount = find('fieldsListItem').length;
form.setInputValue('nameParameterInput', name);
find('createFieldForm.fieldType').simulate('change', [
{
label: type,
value: type,
},
]);
await nextTick();
component.update();
find('createFieldForm.addButton').simulate('click');
// We wait until there is one more field in the DOM
await waitFor('fieldsListItem', currentCount + 1);
};
const selectTab = async (tab: 'fields' | 'templates' | 'advanced') => {
const index = ['fields', 'templates', 'advanced'].indexOf(tab);
const tabIdToContentMap: { [key: string]: TestSubjects } = {
fields: 'documentFields',
templates: 'dynamicTemplates',
advanced: 'advancedConfiguration',
};
const tabElement = find('formTab').at(index);
if (tabElement.length === 0) {
throw new Error(`Tab not found: "${tab}"`);
}
tabElement.simulate('click');
await waitFor(tabIdToContentMap[tab]);
};
const updateJsonEditor = async (testSubject: TestSubjects, value: object) => {
find(testSubject).simulate('change', { jsonContent: JSON.stringify(value) });
};
const getJsonEditorValue = (testSubject: TestSubjects) => {
const value = find(testSubject).props()['data-currentvalue'];
if (typeof value === 'string') {
try {
return JSON.parse(value);
} catch {
return { errorParsingJson: true, props: find(testSubject).props() };
}
}
return value;
};
return {
selectTab,
addField,
updateJsonEditor,
getJsonEditorValue,
};
};
export const setup = async (props: any = { onUpdate() {} }): Promise<MappingsEditorTestBed> => {
const testBed = await registerTestBed<TestSubjects>(MappingsEditor, {
memoryRouter: {
wrapComponent: false,
},
defaultProps: props,
})();
return {
...testBed,
actions: createActions(testBed),
};
};
export type MappingsEditorTestBed = TestBed<TestSubjects> & {
actions: ReturnType<typeof createActions>;
};
export type TestSubjects =
| 'formTab'
| 'mappingsEditor'
| 'fieldsListItem'
| 'fieldName'
| 'mappingTypesDetectedCallout'
| 'documentFields'
| 'dynamicTemplates'
| 'advancedConfiguration'
| 'advancedConfiguration.numericDetection'
| 'advancedConfiguration.numericDetection.input'
| 'advancedConfiguration.dynamicMappingsToggle'
| 'advancedConfiguration.dynamicMappingsToggle.input'
| 'dynamicTemplatesEditor'
| 'nameParameterInput'
| 'createFieldForm.fieldType'
| 'createFieldForm.addButton';

124
x-pack/plugins/index_management/public/application/components/mappings_editor/__jest__/client_integration/mappings_editor.test.tsx
View file

@ -3,8 +3,9 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { act } from 'react-dom/test-utils';
import { componentHelpers } from './helpers';
import { componentHelpers, MappingsEditorTestBed, nextTick, getRandomString } from './helpers';
const { setup } = componentHelpers.mappingsEditor;
const mockOnUpdate = () => undefined;
@ -52,4 +53,125 @@ describe('<MappingsEditor />', () => {
expect(exists('documentFields')).toBe(true);
});
});
describe('tabs', () => {
const defaultMappings = {
properties: {},
dynamic_templates: [{ before: 'foo' }],
};
let testBed: MappingsEditorTestBed;
beforeEach(async () => {
testBed = await setup({ defaultValue: defaultMappings, onUpdate() {} });
});
test('should keep the changes when switching tabs', async () => {
const {
actions: { addField, selectTab, updateJsonEditor, getJsonEditorValue },
component,
find,
exists,
form,
} = testBed;
// -------------------------------------
// Document fields Tab: add a new field
// -------------------------------------
expect(find('fieldsListItem').length).toEqual(0); // Check that we start with an empty list
const newField = { name: getRandomString(), type: 'text' };
await act(async () => {
await addField(newField.name, newField.type);
});
expect(find('fieldsListItem').length).toEqual(1);
let field = find('fieldsListItem').at(0);
expect(find('fieldName', field).text()).toEqual(newField.name);
// -------------------------------------
// Navigate to dynamic templates tab
// -------------------------------------
await act(async () => {
await selectTab('templates');
});
let templatesValue = getJsonEditorValue('dynamicTemplatesEditor');
expect(templatesValue).toEqual(defaultMappings.dynamic_templates);
// Update the dynamic templates editor value
const updatedValueTemplates = [{ after: 'bar' }];
await act(async () => {
await updateJsonEditor('dynamicTemplatesEditor', updatedValueTemplates);
await nextTick();
component.update();
});
templatesValue = getJsonEditorValue('dynamicTemplatesEditor');
expect(templatesValue).toEqual(updatedValueTemplates);
// ------------------------------------------------------
// Switch to advanced settings tab and make some changes
// ------------------------------------------------------
await act(async () => {
await selectTab('advanced');
});
let isDynamicMappingsEnabled = find(
'advancedConfiguration.dynamicMappingsToggle.input'
).props()['aria-checked'];
expect(isDynamicMappingsEnabled).toBe(true);
let isNumericDetectionVisible = exists('advancedConfiguration.numericDetection');
expect(isNumericDetectionVisible).toBe(true);
// Turn off dynamic mappings
await act(async () => {
form.toggleEuiSwitch('advancedConfiguration.dynamicMappingsToggle.input');
await nextTick();
component.update();
await nextTick();
});
isDynamicMappingsEnabled = find('advancedConfiguration.dynamicMappingsToggle.input').props()[
'aria-checked'
];
expect(isDynamicMappingsEnabled).toBe(false);
isNumericDetectionVisible = exists('advancedConfiguration.numericDetection');
expect(isNumericDetectionVisible).toBe(false);
// ----------------------------------------------------------------------------
// Go back to dynamic templates tab and make sure our changes are still there
// ----------------------------------------------------------------------------
await act(async () => {
await selectTab('templates');
});
templatesValue = getJsonEditorValue('dynamicTemplatesEditor');
expect(templatesValue).toEqual(updatedValueTemplates);
// -----------------------------------------------------------
// Go back to fields and make sure our created field is there
// -----------------------------------------------------------
await act(async () => {
await selectTab('fields');
});
field = find('fieldsListItem').at(0);
expect(find('fieldName', field).text()).toEqual(newField.name);
// Go back to advanced settings tab make sure dynamic mappings is disabled
await act(async () => {
await selectTab('advanced');
});
isDynamicMappingsEnabled = find('advancedConfiguration.dynamicMappingsToggle.input').props()[
'aria-checked'
];
expect(isDynamicMappingsEnabled).toBe(false);
isNumericDetectionVisible = exists('advancedConfiguration.numericDetection');
expect(isNumericDetectionVisible).toBe(false);
});
});
});

9
x-pack/plugins/index_management/public/application/components/mappings_editor/components/configuration_form/configuration_form.tsx
View file

@ -121,7 +121,7 @@ export const ConfigurationForm = React.memo(({ defaultValue }: Props) => {
// Avoid reseting the form on component mount.
didMountRef.current = true;
}
}, [defaultValue, form]);
}, [defaultValue]); // eslint-disable-line react-hooks/exhaustive-deps
useEffect(() => {
return () => {
@ -131,7 +131,12 @@ export const ConfigurationForm = React.memo(({ defaultValue }: Props) => {
}, [dispatch]);
return (
<Form form={form} isInvalid={form.isSubmitted && !form.isValid} error={form.getErrors()}>
<Form
form={form}
isInvalid={form.isSubmitted && !form.isValid}
error={form.getErrors()}
data-test-subj="advancedConfiguration"
>
<DynamicMappingSection />
<EuiSpacer size="xl" />
<MetaFieldSection />

11
x-pack/plugins/index_management/public/application/components/mappings_editor/components/configuration_form/dynamic_mapping_section/dynamic_mapping_section.tsx
View file

@ -43,7 +43,11 @@ export const DynamicMappingSection = () => (
}}
/>
<EuiSpacer size="m" />
<UseField path="dynamicMapping.enabled" component={ToggleField} />
<UseField
path="dynamicMapping.enabled"
component={ToggleField}
componentProps={{ 'data-test-subj': 'dynamicMappingsToggle' }}
/>
</>
}
>
@ -62,7 +66,10 @@ export const DynamicMappingSection = () => (
if (enabled) {
return (
<>
<UseField path="dynamicMapping.numeric_detection" />
<UseField
path="dynamicMapping.numeric_detection"
componentProps={{ 'data-test-subj': 'numericDetection' }}
/>
<UseField path="dynamicMapping.date_detection" />
{dateDetection && (
<UseField

2
x-pack/plugins/index_management/public/application/components/mappings_editor/components/document_fields/field_parameters/type_parameter.tsx
View file

@ -68,7 +68,7 @@ export const TypeParameter = ({ onTypeChange, isMultiField, docLink, isRootLevel
selectedOptions={typeField.value as ComboBoxOption[]}
onChange={onTypeChange}
isClearable={false}
data-test-subj="fieldTypeComboBox"
data-test-subj="fieldType"
/>
</EuiFormRow>
);

8
x-pack/plugins/index_management/public/application/components/mappings_editor/components/document_fields/fields/create_field/create_field.tsx
View file

@ -272,7 +272,12 @@ export const CreateField = React.memo(function CreateFieldComponent({
return (
<EuiOutsideClickDetector onOutsideClick={onClickOutside}>
<Form form={form} FormWrapper={formWrapper} onSubmit={submitForm}>
<Form
form={form}
FormWrapper={formWrapper}
onSubmit={submitForm}
data-test-subj="createFieldForm"
>
<div
className={classNames('mappingsEditor__createFieldWrapper', {
'mappingsEditor__createFieldWrapper--toggle':
@ -286,7 +291,6 @@ export const CreateField = React.memo(function CreateFieldComponent({
: paddingLeft
}px`,
}}
data-test-subj="createFieldWrapper"
>
<div className="mappingsEditor__createFieldContent">
<EuiFlexGroup gutterSize="s" alignItems="center">

6
x-pack/plugins/index_management/public/application/components/mappings_editor/components/document_fields/fields/fields_list_item.tsx
View file

@ -254,7 +254,11 @@ function FieldListItemComponent(
</EuiFlexItem>
)}
<EuiFlexItem grow={false} className="mappingsEditor__fieldsListItem__name">
<EuiFlexItem
grow={false}
className="mappingsEditor__fieldsListItem__name"
data-test-subj="fieldName"
>
{source.name}
</EuiFlexItem>

7
x-pack/plugins/index_management/public/application/components/mappings_editor/components/templates_form/templates_form.tsx
View file

@ -80,7 +80,7 @@ export const TemplatesForm = React.memo(({ defaultValue }: Props) => {
// Avoid reseting the form on component mount.
didMountRef.current = true;
}
}, [defaultValue, form]);
}, [defaultValue]); // eslint-disable-line react-hooks/exhaustive-deps
useEffect(() => {
return () => {
@ -90,7 +90,7 @@ export const TemplatesForm = React.memo(({ defaultValue }: Props) => {
}, [dispatch]);
return (
<>
<div data-test-subj="dynamicTemplates">
<EuiText size="s" color="subdued">
<FormattedMessage
id="xpack.idxMgmt.mappingsEditor.dynamicTemplatesDescription"
@ -113,6 +113,7 @@ export const TemplatesForm = React.memo(({ defaultValue }: Props) => {
component={JsonEditorField}
componentProps={{
euiCodeEditorProps: {
['data-test-subj']: 'dynamicTemplatesEditor',
height: '600px',
'aria-label': i18n.translate(
'xpack.idxMgmt.mappingsEditor.dynamicTemplatesEditorAriaLabel',
@ -124,6 +125,6 @@ export const TemplatesForm = React.memo(({ defaultValue }: Props) => {
}}
/>
</Form>
</>
</div>
);
});

3
x-pack/plugins/index_management/public/application/components/mappings_editor/mappings_editor.tsx
View file

@ -122,6 +122,7 @@ export const MappingsEditor = React.memo(({ onUpdate, defaultValue, indexSetting
<EuiTab
onClick={() => changeTab('fields', state)}
isSelected={selectedTab === 'fields'}
data-test-subj="formTab"
>
{i18n.translate('xpack.idxMgmt.mappingsEditor.fieldsTabLabel', {
defaultMessage: 'Mapped fields',
@ -130,6 +131,7 @@ export const MappingsEditor = React.memo(({ onUpdate, defaultValue, indexSetting
<EuiTab
onClick={() => changeTab('templates', state)}
isSelected={selectedTab === 'templates'}
data-test-subj="formTab"
>
{i18n.translate('xpack.idxMgmt.mappingsEditor.templatesTabLabel', {
defaultMessage: 'Dynamic templates',
@ -138,6 +140,7 @@ export const MappingsEditor = React.memo(({ onUpdate, defaultValue, indexSetting
<EuiTab
onClick={() => changeTab('advanced', state)}
isSelected={selectedTab === 'advanced'}
data-test-subj="formTab"
>
{i18n.translate('xpack.idxMgmt.mappingsEditor.advancedTabLabel', {
defaultMessage: 'Advanced options',

8
x-pack/test_utils/testbed/testbed.ts
View file

@ -104,7 +104,7 @@ export const registerTestBed = <T extends string = string>(
* ----------------------------------------------------------------
*/
const find: TestBed<T>['find'] = (testSubject: T) => {
const find: TestBed<T>['find'] = (testSubject: T, sourceReactWrapper = component) => {
const testSubjectToArray = testSubject.split('.');
return testSubjectToArray.reduce((reactWrapper, subject, i) => {
@ -117,7 +117,7 @@ export const registerTestBed = <T extends string = string>(
);
}
return target;
}, component);
}, sourceReactWrapper);
};
const exists: TestBed<T>['exists'] = (testSubject, count = 1) =>
@ -138,7 +138,7 @@ export const registerTestBed = <T extends string = string>(
});
};
const waitFor: TestBed<T>['waitFor'] = async (testSubject: T) => {
const waitFor: TestBed<T>['waitFor'] = async (testSubject: T, count = 1) => {
const triggeredAt = Date.now();
/**
@ -153,7 +153,7 @@ export const registerTestBed = <T extends string = string>(
const WAIT_INTERVAL = 100;
const process = async (): Promise<void> => {
const elemFound = exists(testSubject);
const elemFound = exists(testSubject, count);
if (elemFound) {
// Great! nothing else to do here.

4
x-pack/test_utils/testbed/types.ts
View file

@ -48,7 +48,7 @@ export interface TestBed<T = string> {
find('myForm.nameInput');
```
*/
find: (testSubject: T) => ReactWrapper<any>;
find: (testSubject: T, reactWrapper?: ReactWrapper) => ReactWrapper<any>;
/**
* Update the props of the mounted component
*
@ -60,7 +60,7 @@ export interface TestBed<T = string> {
* Useful when loading a component that fetches a resource from the server
* and we need to wait for the data to be fetched (and bypass any "loading" state).
*/
waitFor: (testSubject: T) => Promise<void>;
waitFor: (testSubject: T, count?: number) => Promise<void>;
form: {
/**
* Set the value of a form text input.