Skip to content

Commit

Permalink
[8.x] [Mappings editor] Add code improvements for source field (#201188
Browse files Browse the repository at this point in the history
…) (#201577)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Mappings editor] Add code improvements for source field
(#201188)](#201188)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Elena
Stoeva","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-25T12:46:05Z","message":"[Mappings
editor] Add code improvements for source field (#201188)\n\nCloses
https://github.com/elastic/kibana/issues/200769\r\n\r\n##
Summary\r\n\r\nThis PR is a follow-up to
https://github.com/elastic/kibana/pull/199854\r\nand it adds the
following code improvements:\r\n\r\n- Replaces
Mappings-editor-context-level property `hasEnterpriceLicense`\r\nwith
plugin-context-level `canUseSyntheticSource` property\r\n- Adds jest
tests to check if the synthetic option is correctly\r\ndisplayed based
on license\r\n- Improves readability of serializer logic for the source
field\r\n\r\n\r\n**How to test:**\r\nThe same test instructions
from\r\nhttps://github.com//pull/199854 can be followed
with a\r\nfocus on checking that the synthetic option is only available
in\r\nEnterprise
license.","sha":"762bb7f59d1d980aa34358c62ae0ef53e81f726e","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Kibana
Management","release_note:skip","Feature:Mappings
Editor","v9.0.0","backport:prev-minor","v8.17.0","v8.18.0"],"title":"[Mappings
editor] Add code improvements for source
field","number":201188,"url":"https://github.com/elastic/kibana/pull/201188","mergeCommit":{"message":"[Mappings
editor] Add code improvements for source field (#201188)\n\nCloses
https://github.com/elastic/kibana/issues/200769\r\n\r\n##
Summary\r\n\r\nThis PR is a follow-up to
https://github.com/elastic/kibana/pull/199854\r\nand it adds the
following code improvements:\r\n\r\n- Replaces
Mappings-editor-context-level property `hasEnterpriceLicense`\r\nwith
plugin-context-level `canUseSyntheticSource` property\r\n- Adds jest
tests to check if the synthetic option is correctly\r\ndisplayed based
on license\r\n- Improves readability of serializer logic for the source
field\r\n\r\n\r\n**How to test:**\r\nThe same test instructions
from\r\nhttps://github.com//pull/199854 can be followed
with a\r\nfocus on checking that the synthetic option is only available
in\r\nEnterprise
license.","sha":"762bb7f59d1d980aa34358c62ae0ef53e81f726e"}},"sourceBranch":"main","suggestedTargetBranches":["8.17","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201188","number":201188,"mergeCommit":{"message":"[Mappings
editor] Add code improvements for source field (#201188)\n\nCloses
https://github.com/elastic/kibana/issues/200769\r\n\r\n##
Summary\r\n\r\nThis PR is a follow-up to
https://github.com/elastic/kibana/pull/199854\r\nand it adds the
following code improvements:\r\n\r\n- Replaces
Mappings-editor-context-level property `hasEnterpriceLicense`\r\nwith
plugin-context-level `canUseSyntheticSource` property\r\n- Adds jest
tests to check if the synthetic option is correctly\r\ndisplayed based
on license\r\n- Improves readability of serializer logic for the source
field\r\n\r\n\r\n**How to test:**\r\nThe same test instructions
from\r\nhttps://github.com//pull/199854 can be followed
with a\r\nfocus on checking that the synthetic option is only available
in\r\nEnterprise
license.","sha":"762bb7f59d1d980aa34358c62ae0ef53e81f726e"}},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Elena Stoeva <[email protected]>
  • Loading branch information
kibanamachine and ElenaStoeva authored Nov 25, 2024
1 parent f002825 commit fda12bc
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export interface AppDependencies {
docLinks: DocLinksStart;
kibanaVersion: SemVer;
overlays: OverlayStart;
canUseSyntheticSource: boolean;
}

export const AppContextProvider = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ const setup = (props: any = { onUpdate() {} }, appDependencies?: any) => {
return testBed;
};

const getContext = (sourceFieldEnabled: boolean = true, canUseSyntheticSource: boolean = true) =>
({
config: {
enableMappingsSourceFieldSection: sourceFieldEnabled,
},
canUseSyntheticSource,
} as unknown as AppDependencies);

describe('Mappings editor: configuration form', () => {
let testBed: TestBed<TestSubjects>;

Expand All @@ -49,14 +57,8 @@ describe('Mappings editor: configuration form', () => {

describe('_source field', () => {
it('renders the _source field when it is enabled', async () => {
const ctx = {
config: {
enableMappingsSourceFieldSection: true,
},
} as unknown as AppDependencies;

await act(async () => {
testBed = setup({ esNodesPlugins: [] }, ctx);
testBed = setup({ esNodesPlugins: [] }, getContext());
});
testBed.component.update();
const { exists } = testBed;
Expand All @@ -65,19 +67,37 @@ describe('Mappings editor: configuration form', () => {
});

it("doesn't render the _source field when it is disabled", async () => {
const ctx = {
config: {
enableMappingsSourceFieldSection: false,
},
} as unknown as AppDependencies;

await act(async () => {
testBed = setup({ esNodesPlugins: [] }, ctx);
testBed = setup({ esNodesPlugins: [] }, getContext(false));
});
testBed.component.update();
const { exists } = testBed;

expect(exists('sourceField')).toBe(false);
});

it('has synthetic option if `canUseSyntheticSource` is set to true', async () => {
await act(async () => {
testBed = setup({ esNodesPlugins: [] }, getContext(true, true));
});
testBed.component.update();
const { exists, find } = testBed;

// Clicking on the field to open the options dropdown
find('sourceValueField').simulate('click');
expect(exists('syntheticSourceFieldOption')).toBe(true);
});

it("doesn't have synthetic option if `canUseSyntheticSource` is set to false", async () => {
await act(async () => {
testBed = setup({ esNodesPlugins: [] }, getContext(true, false));
});
testBed.component.update();
const { exists, find } = testBed;

// Clicking on the field to open the options dropdown
find('sourceValueField').simulate('click');
expect(exists('syntheticSourceFieldOption')).toBe(false);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,9 @@ describe('Mappings editor: core', () => {
let onChangeHandler: jest.Mock = jest.fn();
let getMappingsEditorData = getMappingsEditorDataFactory(onChangeHandler);
let testBed: MappingsEditorTestBed;
let hasEnterpriseLicense = true;
const mockLicenseCheck = jest.fn((type: any) => hasEnterpriseLicense);
const appDependencies = {
plugins: {
ml: { mlApi: {} },
licensing: {
license$: {
subscribe: jest.fn((callback: any) => {
callback({
isActive: true,
hasAtLeast: mockLicenseCheck,
});
return { unsubscribe: jest.fn() };
}),
},
},
},
};

Expand Down Expand Up @@ -314,6 +301,7 @@ describe('Mappings editor: core', () => {
config: {
enableMappingsSourceFieldSection: true,
},
canUseSyntheticSource: true,
...appDependencies,
};

Expand Down Expand Up @@ -512,8 +500,7 @@ describe('Mappings editor: core', () => {
});

['logsdb', 'time_series'].forEach((indexMode) => {
it(`defaults to 'synthetic' with ${indexMode} index mode prop on enterprise license`, async () => {
hasEnterpriseLicense = true;
it(`defaults to 'synthetic' with ${indexMode} index mode prop when 'canUseSyntheticSource' is set to true`, async () => {
await act(async () => {
testBed = setup(
{
Expand All @@ -537,16 +524,15 @@ describe('Mappings editor: core', () => {
expect(find('sourceValueField').prop('value')).toBe('synthetic');
});

it(`defaults to 'standard' with ${indexMode} index mode prop on basic license`, async () => {
hasEnterpriseLicense = false;
it(`defaults to 'standard' with ${indexMode} index mode prop when 'canUseSyntheticSource' is set to true`, async () => {
await act(async () => {
testBed = setup(
{
value: { ...defaultMappings, _source: undefined },
onChange: onChangeHandler,
indexMode,
},
ctx
{ ...ctx, canUseSyntheticSource: false }
);
});
testBed.component.update();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,31 @@ interface SerializedSourceField {
excludes?: string[];
}

const serializeSourceField = (sourceField: any): SerializedSourceField | undefined => {
if (sourceField?.option === SYNTHETIC_SOURCE_OPTION) {
return { mode: SYNTHETIC_SOURCE_OPTION };
}
if (sourceField?.option === DISABLED_SOURCE_OPTION) {
return { enabled: false };
}
if (sourceField?.option === STORED_SOURCE_OPTION) {
return {
mode: 'stored',
includes: sourceField.includes,
excludes: sourceField.excludes,
};
}
if (sourceField?.includes || sourceField?.excludes) {
// If sourceField?.option is undefined, the user hasn't explicitly selected
// this option, so don't include the `mode` property
return {
includes: sourceField.includes,
excludes: sourceField.excludes,
};
}
return undefined;
};

export const formSerializer = (formData: GenericObject) => {
const { dynamicMapping, sourceField, metaField, _routing, _size, subobjects } = formData;

Expand All @@ -48,30 +73,12 @@ export const formSerializer = (formData: GenericObject) => {
? 'strict'
: dynamicMapping?.enabled;

const _source =
sourceField?.option === SYNTHETIC_SOURCE_OPTION
? { mode: SYNTHETIC_SOURCE_OPTION }
: sourceField?.option === DISABLED_SOURCE_OPTION
? { enabled: false }
: sourceField?.option === STORED_SOURCE_OPTION
? {
mode: 'stored',
includes: sourceField?.includes,
excludes: sourceField?.excludes,
}
: sourceField?.includes || sourceField?.excludes
? {
includes: sourceField?.includes,
excludes: sourceField?.excludes,
}
: undefined;

const serialized = {
dynamic,
numeric_detection: dynamicMapping?.numeric_detection,
date_detection: dynamicMapping?.date_detection,
dynamic_date_formats: dynamicMapping?.dynamic_date_formats,
_source: _source as SerializedSourceField,
_source: serializeSourceField(sourceField),
_meta: metaField,
_routing,
_size,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiLink, EuiSpacer, EuiComboBox, EuiFormRow, EuiCallOut, EuiText } from '@elastic/eui';

import { useMappingsState } from '../../../mappings_state_context';
import { useAppContext } from '../../../../../app_context';
import { documentationService } from '../../../../../services/documentation';
import { UseField, FormDataProvider, FormRow, SuperSelectField } from '../../../shared_imports';
import { ComboBoxOption } from '../../../types';
Expand All @@ -24,7 +24,7 @@ import {
} from './constants';

export const SourceFieldSection = () => {
const state = useMappingsState();
const { canUseSyntheticSource } = useAppContext();

const renderOptionDropdownDisplay = (option: SourceOptionKey) => (
<Fragment>
Expand All @@ -44,7 +44,7 @@ export const SourceFieldSection = () => {
},
];

if (state.hasEnterpriseLicense) {
if (canUseSyntheticSource) {
sourceValueOptions.push({
value: SYNTHETIC_SOURCE_OPTION,
inputDisplay: sourceOptionLabels[SYNTHETIC_SOURCE_OPTION],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,6 @@ describe('utils', () => {
selectedDataTypes: ['Boolean'],
},
inferenceToModelIdMap: {},
hasEnterpriseLicense: true,
mappingViewFields: { byId: {}, rootLevelFields: [], aliases: {}, maxNestedDepth: 0 },
};
test('returns list of matching fields with search term', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import React, { useMemo, useState, useEffect, useCallback } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiSpacer, EuiTabs, EuiTab } from '@elastic/eui';

import { ILicense } from '@kbn/licensing-plugin/common/types';
import { useAppContext } from '../../app_context';
import { IndexMode } from '../../../../common/types/data_streams';
import {
Expand Down Expand Up @@ -61,9 +60,7 @@ export interface Props {

export const MappingsEditor = React.memo(
({ onChange, value, docLinks, indexSettings, esNodesPlugins, indexMode }: Props) => {
const {
plugins: { licensing },
} = useAppContext();
const { canUseSyntheticSource } = useAppContext();
const { parsedDefaultValue, multipleMappingsDeclared } = useMemo<MappingsEditorParsedMetadata>(
() => parseMappings(value),
[value]
Expand Down Expand Up @@ -128,39 +125,22 @@ export const MappingsEditor = React.memo(
[dispatch]
);

const [isLicenseCheckComplete, setIsLicenseCheckComplete] = useState(false);
useEffect(() => {
const subscription = licensing?.license$.subscribe((license: ILicense) => {
dispatch({
type: 'hasEnterpriseLicense.update',
value: license.isActive && license.hasAtLeast('enterprise'),
});
setIsLicenseCheckComplete(true);
});

return () => subscription?.unsubscribe();
}, [dispatch, licensing]);

useEffect(() => {
if (
isLicenseCheckComplete &&
!state.configuration.defaultValue._source &&
(indexMode === LOGSDB_INDEX_MODE || indexMode === TIME_SERIES_MODE)
) {
if (state.hasEnterpriseLicense) {
if (canUseSyntheticSource) {
// If the source field is undefined (hasn't been set in the form)
// and if the user has selected a `logsdb` or `time_series` index mode in the Logistics step,
// update the form data with synthetic _source
dispatch({
type: 'configuration.save',
value: { ...state.configuration.defaultValue, _source: { mode: 'synthetic' } } as any,
});
}
}
}, [
indexMode,
dispatch,
state.configuration,
state.hasEnterpriseLicense,
isLicenseCheckComplete,
]);
}, [indexMode, dispatch, state.configuration, canUseSyntheticSource]);

const tabToContentMap = {
fields: (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ export const StateProvider: React.FC<{ children?: React.ReactNode }> = ({ childr
selectedDataTypes: [],
},
inferenceToModelIdMap: {},
hasEnterpriseLicense: false,
mappingViewFields: { byId: {}, rootLevelFields: [], aliases: {}, maxNestedDepth: 0 },
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -629,11 +629,5 @@ export const reducer = (state: State, action: Action): State => {
inferenceToModelIdMap: action.value.inferenceToModelIdMap,
};
}
case 'hasEnterpriseLicense.update': {
return {
...state,
hasEnterpriseLicense: action.value,
};
}
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ export interface State {
};
templates: TemplatesFormState;
inferenceToModelIdMap?: InferenceToModelIdMap;
hasEnterpriseLicense: boolean;
mappingViewFields: NormalizedFields; // state of the incoming index mappings, separate from the editor state above
}

Expand Down Expand Up @@ -141,7 +140,6 @@ export type Action =
| { type: 'fieldsJsonEditor.update'; value: { json: { [key: string]: any }; isValid: boolean } }
| { type: 'search:update'; value: string }
| { type: 'validity:update'; value: boolean }
| { type: 'filter:update'; value: { selectedOptions: EuiSelectableOption[] } }
| { type: 'hasEnterpriseLicense.update'; value: boolean };
| { type: 'filter:update'; value: { selectedOptions: EuiSelectableOption[] } };

export type Dispatch = (action: Action) => void;
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export function getIndexManagementDependencies({
cloud,
startDependencies,
uiMetricService,
canUseSyntheticSource,
}: {
core: CoreStart;
usageCollection: UsageCollectionSetup;
Expand All @@ -68,6 +69,7 @@ export function getIndexManagementDependencies({
cloud?: CloudSetup;
startDependencies: StartDependencies;
uiMetricService: UiMetricService;
canUseSyntheticSource: boolean;
}): AppDependencies {
const { docLinks, application, uiSettings, settings } = core;
const { url } = startDependencies.share;
Expand Down Expand Up @@ -100,6 +102,7 @@ export function getIndexManagementDependencies({
docLinks,
kibanaVersion,
overlays: core.overlays,
canUseSyntheticSource,
};
}

Expand All @@ -112,6 +115,7 @@ export async function mountManagementSection({
kibanaVersion,
config,
cloud,
canUseSyntheticSource,
}: {
coreSetup: CoreSetup<StartDependencies>;
usageCollection: UsageCollectionSetup;
Expand All @@ -121,6 +125,7 @@ export async function mountManagementSection({
kibanaVersion: SemVer;
config: AppDependencies['config'];
cloud?: CloudSetup;
canUseSyntheticSource: boolean;
}) {
const { element, setBreadcrumbs, history } = params;
const [core, startDependencies] = await coreSetup.getStartServices();
Expand Down Expand Up @@ -148,6 +153,7 @@ export async function mountManagementSection({
startDependencies,
uiMetricService,
usageCollection,
canUseSyntheticSource,
});

const unmountAppCallback = renderApp(element, { core, dependencies: appDependencies });
Expand Down
Loading

0 comments on commit fda12bc

Please sign in to comment.