[Mappings editor] Fix app crash when selecting "other" field type (#79434)

This commit is contained in:
Sébastien Loix 2020-10-05 15:15:38 +02:00 committed by GitHub
parent 43493bb0d9
commit 519d4905bc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 224 additions and 25 deletions

View file

@ -464,24 +464,58 @@ export const useField = <T>(
[errors]
);
/**
* Handler to update the state and make sure the component is still mounted.
* When resetting the form, some field might get unmounted (e.g. a toggle on "true" becomes "false" and now certain fields should not be in the DOM).
* In that scenario there is a race condition in the "reset" method below, because the useState() hook is not synchronous.
*
* A better approach would be to have the state in a reducer and being able to update all values in a single dispatch action.
*/
const updateStateIfMounted = useCallback(
(
state: 'isPristine' | 'isValidating' | 'isChangingValue' | 'isValidated' | 'errors' | 'value',
nextValue: any
) => {
if (isMounted.current === false) {
return;
}
switch (state) {
case 'value':
return setValue(nextValue);
case 'errors':
return setErrors(nextValue);
case 'isChangingValue':
return setIsChangingValue(nextValue);
case 'isPristine':
return setPristine(nextValue);
case 'isValidated':
return setIsValidated(nextValue);
case 'isValidating':
return setValidating(nextValue);
}
},
[setValue]
);
const reset: FieldHook<T>['reset'] = useCallback(
(resetOptions = { resetValue: true }) => {
const { resetValue = true, defaultValue: updatedDefaultValue } = resetOptions;
setPristine(true);
setValidating(false);
setIsChangingValue(false);
setIsValidated(false);
setErrors([]);
updateStateIfMounted('isPristine', true);
updateStateIfMounted('isValidating', false);
updateStateIfMounted('isChangingValue', false);
updateStateIfMounted('isValidated', false);
updateStateIfMounted('errors', []);
if (resetValue) {
hasBeenReset.current = true;
const newValue = deserializeValue(updatedDefaultValue ?? defaultValue);
setValue(newValue);
updateStateIfMounted('value', newValue);
return newValue;
}
},
[setValue, deserializeValue, defaultValue]
[updateStateIfMounted, deserializeValue, defaultValue]
);
// Don't take into account non blocker validation. Some are just warning (like trying to add a wrong ComboBox item)

View file

@ -0,0 +1,112 @@
/*
* 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 { act } from 'react-dom/test-utils';
import { componentHelpers, MappingsEditorTestBed } from '../helpers';
const { setup, getMappingsEditorDataFactory } = componentHelpers.mappingsEditor;
describe('Mappings editor: other datatype', () => {
/**
* Variable to store the mappings data forwarded to the consumer component
*/
let data: any;
let onChangeHandler: jest.Mock = jest.fn();
let getMappingsEditorData = getMappingsEditorDataFactory(onChangeHandler);
let testBed: MappingsEditorTestBed;
beforeAll(() => {
jest.useFakeTimers();
});
afterAll(() => {
jest.useRealTimers();
});
beforeEach(() => {
onChangeHandler = jest.fn();
getMappingsEditorData = getMappingsEditorDataFactory(onChangeHandler);
});
test('allow to add custom field type', async () => {
await act(async () => {
testBed = setup({ onChange: onChangeHandler });
});
testBed.component.update();
const {
component,
actions: { addField },
} = testBed;
await addField('myField', 'other', 'customType');
const mappings = {
properties: {
myField: {
type: 'customType',
},
},
};
({ data } = await getMappingsEditorData(component));
expect(data).toEqual(mappings);
});
test('allow to change a field type to a custom type', async () => {
const defaultMappings = {
properties: {
myField: {
type: 'text',
},
},
};
let updatedMappings = { ...defaultMappings };
await act(async () => {
testBed = setup({ value: defaultMappings, onChange: onChangeHandler });
});
testBed.component.update();
const {
component,
find,
form,
actions: { startEditField, updateFieldAndCloseFlyout },
} = testBed;
// Open the flyout to edit the field
await startEditField('myField');
// Change the field type
await act(async () => {
find('mappingsEditorFieldEdit.fieldType').simulate('change', [
{
label: 'other',
value: 'other',
},
]);
});
component.update();
form.setInputValue('mappingsEditorFieldEdit.fieldSubType', 'customType');
// Save the field and close the flyout
await updateFieldAndCloseFlyout();
updatedMappings = {
properties: {
myField: {
type: 'customType',
},
},
};
({ data } = await getMappingsEditorData(component));
expect(data).toEqual(updatedMappings);
});
});

View file

@ -149,7 +149,7 @@ const createActions = (testBed: TestBed<TestSubjects>) => {
return { field: find(testSubject as TestSubjects), testSubject };
};
const addField = async (name: string, type: string) => {
const addField = async (name: string, type: string, subType?: string) => {
await act(async () => {
form.setInputValue('nameParameterInput', name);
find('createFieldForm.fieldType').simulate('change', [
@ -160,6 +160,17 @@ const createActions = (testBed: TestBed<TestSubjects>) => {
]);
});
component.update();
if (subType !== undefined) {
await act(async () => {
if (type === 'other') {
// subType is a text input
form.setInputValue('createFieldForm.fieldSubType', subType);
}
});
}
await act(async () => {
find('createFieldForm.addButton').simulate('click');
});

View file

@ -4,13 +4,11 @@
* you may not use this file except in compliance with the Elastic License.
*/
import React from 'react';
import React, { useCallback } from 'react';
import { i18n } from '@kbn/i18n';
import { UseField, TextField, FieldConfig } from '../../../shared_imports';
import { fieldValidators } from '../../../shared_imports';
const { emptyField } = fieldValidators;
import { UseField, TextField, FieldConfig, FieldHook } from '../../../shared_imports';
import { getFieldConfig } from '../../../lib';
/**
* This is a special component that does not have an explicit entry in {@link PARAMETERS_DEFINITION}.
@ -18,25 +16,69 @@ const { emptyField } = fieldValidators;
* We use it to store the name of types unknown to the mappings editor in the "subType" path.
*/
type FieldType = [{ value: string }];
const typeParameterConfig = getFieldConfig('type');
const fieldConfig: FieldConfig = {
label: i18n.translate('xpack.idxMgmt.mappingsEditor.otherTypeNameFieldLabel', {
defaultMessage: 'Type Name',
}),
defaultValue: '',
deserializer: typeParameterConfig.deserializer,
serializer: typeParameterConfig.serializer,
validations: [
{
validator: emptyField(
i18n.translate(
'xpack.idxMgmt.mappingsEditor.parameters.validations.otherTypeNameIsRequiredErrorMessage',
{
defaultMessage: 'The type name is required.',
}
)
),
validator: ({ value: fieldValue }) => {
if ((fieldValue as FieldType)[0].value.trim() === '') {
return {
message: i18n.translate(
'xpack.idxMgmt.mappingsEditor.parameters.validations.otherTypeNameIsRequiredErrorMessage',
{
defaultMessage: 'The type name is required.',
}
),
};
}
},
},
],
};
interface Props {
field: FieldHook<FieldType>;
}
/**
* The "subType" parameter can be configured either with a ComboBox (when the type is known)
* or with a TextField (when the type is unknown). This causes its value to have different type
* (either an array of object either a string). In order to align both value and let the consumer of
* the value worry about a single type, we will create a custom TextField component that works with the
* array of object that the ComboBox works with.
*/
const CustomTextField = ({ field }: Props) => {
const { setValue } = field;
const transformedField: FieldHook<any> = {
...field,
value: field.value[0]?.value ?? '',
};
const onChange = useCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
setValue([{ value: e.target.value }]);
},
[setValue]
);
return (
<TextField
field={transformedField}
euiFieldProps={{ onChange, 'data-test-subj': 'fieldSubType' }}
/>
);
};
export const OtherTypeNameParameter = () => (
<UseField path="subType" config={fieldConfig} component={TextField} />
<UseField path="subType" config={fieldConfig} component={CustomTextField} />
);

View file

@ -91,8 +91,8 @@ export const EditField = React.memo(({ form, field, allFields, exitEdit, updateF
<FormDataProvider pathsToWatch={['type', 'subType']}>
{({ type, subType }) => {
const linkDocumentation =
documentationService.getTypeDocLink(subType?.[0].value) ||
documentationService.getTypeDocLink(type?.[0].value);
documentationService.getTypeDocLink(subType?.[0]?.value) ||
documentationService.getTypeDocLink(type?.[0]?.value);
if (!linkDocumentation) {
return null;

View file

@ -168,7 +168,7 @@ export const PARAMETERS_DEFINITION: { [key in ParameterName]: ParameterDefinitio
},
];
}
return [];
return [{ value: '' }];
},
serializer: (fieldType: ComboBoxOption[] | undefined) =>
fieldType && fieldType.length ? fieldType[0].value : fieldType,