Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] [Mappings editor] Only add defined advanced options to request (#194148) #194795

Merged
merged 3 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,48 @@ With this option you can decide if you want empty string value to be returned by
"role": ""
}
```

#### stripUnsetFields

**Type:** `boolean`
**Default:** `false`

Sometimes, we only want to include fields that have a defined initial value or if their value has been set by the user.
In this case, set `stripUnsetFields` to `true`.

Suppose we have a toggle field `autocompleteEnabled`, which doesn't have a specified default value passed to `useForm`:

```js
const { form } = useForm({
defaultValue: {
darkModeEnabled: false,
accessibilityEnabled: true,
autocompleteEnabled: undefined,
},
options: { stripUnsetFields: true },
});
```

Initially, the form data includes only `darkModeEnabled` and `accessibilityEnabled` because `autocompleteEnabled` is stripped.

```js
{
"darkModeEnabled": false,
"accessibilityEnabled": true,
}
```

Then the user toggles the `autocompleteEnabled` field to `false`. Now the field is included in the form data:

```js
{
"darkModeEnabled": false,
"accessibilityEnabled": true,
"autocompleteEnabled": false,
}
```

Note: This option only considers the `defaultValue` config passed to `useForm()` to determine if the initial value is
undefined. If a default value has been specified as a prop to the `<UseField />` component or in the form schema,
but not in the `defaultValue` config for `useForm()`, the field would initially be populated with the specified default
value, but it won't be included in the form data until the user explicitly sets its value.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { createArrayItem, getInternalArrayFieldPath } from '../components/use_ar
const DEFAULT_OPTIONS = {
valueChangeDebounceTime: 500,
stripEmptyFields: true,
stripUnsetFields: false,
};

export interface UseFormReturn<T extends FormData, I extends FormData> {
Expand Down Expand Up @@ -66,13 +67,18 @@ export function useForm<T extends FormData = FormData, I extends FormData = T>(
return initDefaultValue(defaultValue);
}, [defaultValue, initDefaultValue]);

const { valueChangeDebounceTime, stripEmptyFields: doStripEmptyFields } = options ?? {};
const {
valueChangeDebounceTime,
stripEmptyFields: doStripEmptyFields,
stripUnsetFields,
} = options ?? {};
const formOptions = useMemo(
() => ({
stripEmptyFields: doStripEmptyFields ?? DEFAULT_OPTIONS.stripEmptyFields,
valueChangeDebounceTime: valueChangeDebounceTime ?? DEFAULT_OPTIONS.valueChangeDebounceTime,
stripUnsetFields: stripUnsetFields ?? DEFAULT_OPTIONS.stripUnsetFields,
}),
[valueChangeDebounceTime, doStripEmptyFields]
[valueChangeDebounceTime, doStripEmptyFields, stripUnsetFields]
);

const [isSubmitted, setIsSubmitted] = useState(false);
Expand Down Expand Up @@ -177,8 +183,16 @@ export function useForm<T extends FormData = FormData, I extends FormData = T>(

const fieldsToArray = useCallback<() => FieldHook[]>(() => Object.values(fieldsRefs.current), []);

const getFieldDefaultValue: FormHook<T, I>['getFieldDefaultValue'] = useCallback(
(fieldName) => get(defaultValueDeserialized.current ?? {}, fieldName),
[]
);

const getFieldsForOutput = useCallback(
(fields: FieldsMap, opts: { stripEmptyFields: boolean }): FieldsMap => {
(
fields: FieldsMap,
opts: { stripEmptyFields: boolean; stripUnsetFields: boolean }
): FieldsMap => {
return Object.entries(fields).reduce((acc, [key, field]) => {
if (!field.__isIncludedInOutput) {
return acc;
Expand All @@ -191,11 +205,17 @@ export function useForm<T extends FormData = FormData, I extends FormData = T>(
}
}

if (opts.stripUnsetFields) {
if (!field.isDirty && getFieldDefaultValue(field.path) === undefined) {
return acc;
}
}

acc[key] = field;
return acc;
}, {} as FieldsMap);
},
[]
[getFieldDefaultValue]
);

const updateFormDataAt: FormHook<T, I>['__updateFormDataAt'] = useCallback(
Expand Down Expand Up @@ -396,12 +416,13 @@ export function useForm<T extends FormData = FormData, I extends FormData = T>(
const getFormData: FormHook<T, I>['getFormData'] = useCallback(() => {
const fieldsToOutput = getFieldsForOutput(fieldsRefs.current, {
stripEmptyFields: formOptions.stripEmptyFields,
stripUnsetFields: formOptions.stripUnsetFields,
});
const fieldsValue = mapFormFields(fieldsToOutput, (field) => field.__serializeValue());
return serializer
? serializer(unflattenObject<I>(fieldsValue))
: unflattenObject<T>(fieldsValue);
}, [getFieldsForOutput, formOptions.stripEmptyFields, serializer]);
}, [getFieldsForOutput, formOptions.stripEmptyFields, formOptions.stripUnsetFields, serializer]);

const getErrors: FormHook<T, I>['getErrors'] = useCallback(() => {
if (isValid === true) {
Expand Down Expand Up @@ -455,11 +476,6 @@ export function useForm<T extends FormData = FormData, I extends FormData = T>(

const getFields: FormHook<T, I>['getFields'] = useCallback(() => fieldsRefs.current, []);

const getFieldDefaultValue: FormHook<T, I>['getFieldDefaultValue'] = useCallback(
(fieldName) => get(defaultValueDeserialized.current ?? {}, fieldName),
[]
);

const updateFieldValues: FormHook<T, I>['updateFieldValues'] = useCallback(
(updatedFormData, { runDeserializer = true } = {}) => {
if (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ export interface FormOptions {
* Remove empty string field ("") from form data
*/
stripEmptyFields?: boolean;
/**
* Remove fields from form data that don't have initial value and are not modified by the user.
*/
stripUnsetFields?: boolean;
}

export interface FieldHook<T = unknown, I = T> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,19 @@ interface Props {
}

const formSerializer = (formData: GenericObject, sourceFieldMode?: string) => {
const {
dynamicMapping: {
enabled: dynamicMappingsEnabled,
throwErrorsForUnmappedFields,
/* eslint-disable @typescript-eslint/naming-convention */
numeric_detection,
date_detection,
dynamic_date_formats,
/* eslint-enable @typescript-eslint/naming-convention */
},
sourceField,
metaField,
_routing,
_size,
subobjects,
} = formData;
const { dynamicMapping, sourceField, metaField, _routing, _size, subobjects } = formData;

const dynamic = dynamicMappingsEnabled ? true : throwErrorsForUnmappedFields ? 'strict' : false;
const dynamic = dynamicMapping?.enabled
? true
: dynamicMapping?.throwErrorsForUnmappedFields
? 'strict'
: dynamicMapping?.enabled;

const serialized = {
dynamic,
numeric_detection,
date_detection,
dynamic_date_formats,
numeric_detection: dynamicMapping?.numeric_detection,
date_detection: dynamicMapping?.date_detection,
dynamic_date_formats: dynamicMapping?.dynamic_date_formats,
_source: sourceFieldMode ? { mode: sourceFieldMode } : sourceField,
_meta: metaField,
_routing,
Expand Down Expand Up @@ -85,18 +74,18 @@ const formDeserializer = (formData: GenericObject) => {

return {
dynamicMapping: {
enabled: dynamic === true || dynamic === undefined,
throwErrorsForUnmappedFields: dynamic === 'strict',
enabled: dynamic === 'strict' ? false : dynamic,
throwErrorsForUnmappedFields: dynamic === 'strict' ? true : undefined,
numeric_detection,
date_detection,
dynamic_date_formats,
},
sourceField: {
enabled: enabled === true || enabled === undefined,
enabled,
includes,
excludes,
},
metaField: _meta ?? {},
metaField: _meta,
_routing,
_size,
subobjects,
Expand All @@ -121,6 +110,7 @@ export const ConfigurationForm = React.memo(({ value, esNodesPlugins }: Props) =
deserializer: formDeserializer,
defaultValue: value,
id: 'configurationForm',
options: { stripUnsetFields: true },
});
const dispatch = useDispatch();
const { subscribe, submit, reset, getFormData } = form;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
const comboBox = getService('comboBox');
const find = getService('find');
const browser = getService('browser');
const log = getService('log');

describe('Index template wizard', function () {
before(async () => {
Expand Down Expand Up @@ -162,6 +163,40 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
expect(await testSubjects.exists('fieldSubType')).to.be(true);
expect(await testSubjects.exists('nextButton')).to.be(true);
});

it("advanced options tab doesn't add default values to request by default", async () => {
await pageObjects.indexManagement.changeMappingsEditorTab('advancedOptions');
await testSubjects.click('previewIndexTemplate');
const templatePreview = await testSubjects.getVisibleText('simulateTemplatePreview');

await log.debug(`Template preview text: ${templatePreview}`);

// All advanced options should not be part of the request
expect(templatePreview).to.not.contain('"dynamic"');
expect(templatePreview).to.not.contain('"subobjects"');
expect(templatePreview).to.not.contain('"dynamic_date_formats"');
expect(templatePreview).to.not.contain('"date_detection"');
expect(templatePreview).to.not.contain('"numeric_detection"');
});

it('advanced options tab adds the set values to the request', async () => {
await pageObjects.indexManagement.changeMappingsEditorTab('advancedOptions');

// Toggle the subobjects field to false
await testSubjects.click('subobjectsToggle');

await testSubjects.click('previewIndexTemplate');
const templatePreview = await testSubjects.getVisibleText('simulateTemplatePreview');

await log.debug(`Template preview text: ${templatePreview}`);

// Only the subobjects option should be part of the request
expect(templatePreview).to.contain('"subobjects": false');
expect(templatePreview).to.not.contain('"dynamic"');
expect(templatePreview).to.not.contain('"dynamic_date_formats"');
expect(templatePreview).to.not.contain('"date_detection"');
expect(templatePreview).to.not.contain('"numeric_detection"');
});
});
});
};
14 changes: 14 additions & 0 deletions x-pack/test/functional/page_objects/index_management_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,20 @@ export function IndexManagementPageProvider({ getService }: FtrProviderContext)
await testSubjects.click(tab);
},

async changeMappingsEditorTab(
tab: 'mappedFields' | 'runtimeFields' | 'dynamicTemplates' | 'advancedOptions'
) {
const index = [
'mappedFields',
'runtimeFields',
'dynamicTemplates',
'advancedOptions',
].indexOf(tab);

const tabs = await testSubjects.findAll('formTab');
await tabs[index].click();
},

async clickNextButton() {
await testSubjects.click('nextButton');
},
Expand Down