From a6214c8cd936d1a0a5646ad14ddc1350d7fff117 Mon Sep 17 00:00:00 2001
From: Elena Stoeva <59341489+ElenaStoeva@users.noreply.github.com>
Date: Thu, 3 Oct 2024 10:37:56 +0100
Subject: [PATCH] [Mappings editor] Only add defined advanced options to
request (#194148)
Fixes https://github.com/elastic/kibana/issues/106006
Fixes https://github.com/elastic/kibana/issues/106151
Fixes https://github.com/elastic/kibana/issues/150395
## Summary
This PR makes the Advanced options (configuration) form add to the
request only the field values that have set values. This works by adding
the `stripUnsetFields` option to the `useForm` hook (similar to the
`stripEmptyFields` option) which determines if the unset values will be
returned by the form (unset means that the field hasn't been
set/modified by the user (is not dirty) and its initial value is
undefined).
https://github.com/user-attachments/assets/b46af90d-6886-4232-ae0f-66910902e238
---------
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 9f58ffd52755335aa4a8558cdad408c6b32526f4)
---
.../static/forms/docs/core/use_form_hook.mdx | 45 +++++++++++++++++++
.../forms/hook_form_lib/hooks/use_form.ts | 36 ++++++++++-----
.../static/forms/hook_form_lib/types.ts | 4 ++
.../configuration_form/configuration_form.tsx | 38 ++++++----------
.../index_management/index_template_wizard.ts | 35 +++++++++++++++
.../page_objects/index_management_page.ts | 14 ++++++
6 files changed, 138 insertions(+), 34 deletions(-)
diff --git a/src/plugins/es_ui_shared/static/forms/docs/core/use_form_hook.mdx b/src/plugins/es_ui_shared/static/forms/docs/core/use_form_hook.mdx
index 82cd0c88834a3..b8186aac3ac84 100644
--- a/src/plugins/es_ui_shared/static/forms/docs/core/use_form_hook.mdx
+++ b/src/plugins/es_ui_shared/static/forms/docs/core/use_form_hook.mdx
@@ -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 `` 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.
diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts
index ecf3a242f0f16..84b338fb95106 100644
--- a/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts
+++ b/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts
@@ -25,6 +25,7 @@ import { createArrayItem, getInternalArrayFieldPath } from '../components/use_ar
const DEFAULT_OPTIONS = {
valueChangeDebounceTime: 500,
stripEmptyFields: true,
+ stripUnsetFields: false,
};
export interface UseFormReturn {
@@ -66,13 +67,18 @@ export function useForm(
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);
@@ -177,8 +183,16 @@ export function useForm(
const fieldsToArray = useCallback<() => FieldHook[]>(() => Object.values(fieldsRefs.current), []);
+ const getFieldDefaultValue: FormHook['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;
@@ -191,11 +205,17 @@ export function useForm(
}
}
+ if (opts.stripUnsetFields) {
+ if (!field.isDirty && getFieldDefaultValue(field.path) === undefined) {
+ return acc;
+ }
+ }
+
acc[key] = field;
return acc;
}, {} as FieldsMap);
},
- []
+ [getFieldDefaultValue]
);
const updateFormDataAt: FormHook['__updateFormDataAt'] = useCallback(
@@ -396,12 +416,13 @@ export function useForm(
const getFormData: FormHook['getFormData'] = useCallback(() => {
const fieldsToOutput = getFieldsForOutput(fieldsRefs.current, {
stripEmptyFields: formOptions.stripEmptyFields,
+ stripUnsetFields: formOptions.stripUnsetFields,
});
const fieldsValue = mapFormFields(fieldsToOutput, (field) => field.__serializeValue());
return serializer
? serializer(unflattenObject(fieldsValue))
: unflattenObject(fieldsValue);
- }, [getFieldsForOutput, formOptions.stripEmptyFields, serializer]);
+ }, [getFieldsForOutput, formOptions.stripEmptyFields, formOptions.stripUnsetFields, serializer]);
const getErrors: FormHook['getErrors'] = useCallback(() => {
if (isValid === true) {
@@ -455,11 +476,6 @@ export function useForm(
const getFields: FormHook['getFields'] = useCallback(() => fieldsRefs.current, []);
- const getFieldDefaultValue: FormHook['getFieldDefaultValue'] = useCallback(
- (fieldName) => get(defaultValueDeserialized.current ?? {}, fieldName),
- []
- );
-
const updateFieldValues: FormHook['updateFieldValues'] = useCallback(
(updatedFormData, { runDeserializer = true } = {}) => {
if (
diff --git a/src/plugins/es_ui_shared/static/forms/hook_form_lib/types.ts b/src/plugins/es_ui_shared/static/forms/hook_form_lib/types.ts
index f700faa66e6fd..282b9de8d3ee4 100644
--- a/src/plugins/es_ui_shared/static/forms/hook_form_lib/types.ts
+++ b/src/plugins/es_ui_shared/static/forms/hook_form_lib/types.ts
@@ -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 {
diff --git a/x-pack/plugins/index_management/public/application/components/mappings_editor/components/configuration_form/configuration_form.tsx b/x-pack/plugins/index_management/public/application/components/mappings_editor/components/configuration_form/configuration_form.tsx
index b326779d8a76c..e3e32c55aada0 100644
--- a/x-pack/plugins/index_management/public/application/components/mappings_editor/components/configuration_form/configuration_form.tsx
+++ b/x-pack/plugins/index_management/public/application/components/mappings_editor/components/configuration_form/configuration_form.tsx
@@ -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,
@@ -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,
@@ -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;
diff --git a/x-pack/test/functional/apps/index_management/index_template_wizard.ts b/x-pack/test/functional/apps/index_management/index_template_wizard.ts
index e9eb68f749e78..5b49286f6182b 100644
--- a/x-pack/test/functional/apps/index_management/index_template_wizard.ts
+++ b/x-pack/test/functional/apps/index_management/index_template_wizard.ts
@@ -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 () => {
@@ -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"');
+ });
});
});
};
diff --git a/x-pack/test/functional/page_objects/index_management_page.ts b/x-pack/test/functional/page_objects/index_management_page.ts
index 8730d2807cd21..8077581bbbb48 100644
--- a/x-pack/test/functional/page_objects/index_management_page.ts
+++ b/x-pack/test/functional/page_objects/index_management_page.ts
@@ -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');
},