Skip to content

Commit

Permalink
[7.x] [Mappings editor] Fix app crash when selecting "other" field ty…
Browse files Browse the repository at this point in the history
…pe (#79434) (#79454)
  • Loading branch information
sebelga authored Oct 5, 2020
1 parent 67cfa50 commit 1cc6c10
Show file tree
Hide file tree
Showing 6 changed files with 224 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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', [
Expand All @@ -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');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,81 @@
* 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}.
*
* 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} />
);
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 1cc6c10

Please sign in to comment.