Skip to content
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 @@ -504,18 +504,21 @@ export const useField = <T, FormType = FormData, I = T>(
const { resetValue = true, defaultValue: updatedDefaultValue } = resetOptions;

setPristine(true);
setIsModified(false);
setValidating(false);
setIsChangingValue(false);
setIsValidated(false);
setStateErrors([]);

if (resetValue) {
hasBeenReset.current = true;
const newValue = deserializeValue(updatedDefaultValue ?? defaultValue);
// updateStateIfMounted('value', newValue);
setValue(newValue);
return newValue;

if (isMounted.current) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to add this if statement as there was a strange race condition in one of the tests.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add any tests for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is test coverage for form.reset() in the use_form.test.tsx which test this part of the code. Aside this is a very low risk change as we are simply protecting ourselves from updating the state on an unmounted component.

setIsModified(false);
setValidating(false);
setIsChangingValue(false);
setIsValidated(false);
setStateErrors([]);

if (resetValue) {
hasBeenReset.current = true;
const newValue = deserializeValue(updatedDefaultValue ?? defaultValue);
// updateStateIfMounted('value', newValue);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove this comment?

setValue(newValue);
return newValue;
}
}
},
[deserializeValue, defaultValue, setValue, setStateErrors]
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ import { SemVer } from 'semver';
import {
notificationServiceMock,
docLinksServiceMock,
uiSettingsServiceMock,
} from '../../../../../../src/core/public/mocks';
import { GlobalFlyout } from '../../../../../../src/plugins/es_ui_shared/public';
import { createKibanaReactContext } from '../../../../../../src/plugins/kibana_react/public';

import { MAJOR_VERSION } from '../../../common';
import { AppContextProvider } from '../../../public/application/app_context';
import { httpService } from '../../../public/application/services/http';
import { breadcrumbService } from '../../../public/application/services/breadcrumbs';
Expand All @@ -32,13 +35,10 @@ import {
} from '../../../public/application/components';
import { componentTemplatesMockDependencies } from '../../../public/application/components/component_templates/__jest__';
import { init as initHttpRequests } from './http_requests';
import { MAJOR_VERSION } from './constants';

const mockHttpClient = axios.create({ adapter: axiosXhrAdapter });
const { GlobalFlyoutProvider } = GlobalFlyout;

export const kibanaVersion = new SemVer(MAJOR_VERSION);

export const services = {
extensionsService: new ExtensionsService(),
uiMetricService: new UiMetricService('index_management'),
Expand All @@ -54,6 +54,15 @@ const appDependencies = {
plugins: {},
} as any;

export const kibanaVersion = new SemVer(MAJOR_VERSION);

const { Provider: KibanaReactContextProvider } = createKibanaReactContext({
uiSettings: uiSettingsServiceMock.createSetupContract(),
kibanaVersion: {
get: () => kibanaVersion,
},
});

export const setupEnvironment = () => {
// Mock initialization of services
// @ts-ignore
Expand All @@ -75,14 +84,16 @@ export const WithAppDependencies =
(props: any) => {
const mergedDependencies = merge({}, appDependencies, overridingDependencies);
return (
<AppContextProvider value={mergedDependencies}>
<MappingsEditorProvider>
<ComponentTemplatesProvider value={componentTemplatesMockDependencies}>
<GlobalFlyoutProvider>
<Comp {...props} />
</GlobalFlyoutProvider>
</ComponentTemplatesProvider>
</MappingsEditorProvider>
</AppContextProvider>
<KibanaReactContextProvider>
<AppContextProvider value={mergedDependencies}>
<MappingsEditorProvider>
<ComponentTemplatesProvider value={componentTemplatesMockDependencies}>
<GlobalFlyoutProvider>
<Comp {...props} />
</GlobalFlyoutProvider>
</ComponentTemplatesProvider>
</MappingsEditorProvider>
</AppContextProvider>
</KibanaReactContextProvider>
);
};
2 changes: 2 additions & 0 deletions x-pack/plugins/index_management/common/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,5 @@ export {
UIM_TEMPLATE_CLONE,
UIM_TEMPLATE_SIMULATE,
} from './ui_metric';

export { MAJOR_VERSION } from './plugin';
6 changes: 6 additions & 0 deletions x-pack/plugins/index_management/common/constants/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,9 @@ export const PLUGIN = {
defaultMessage: 'Index Management',
}),
};

// Ideally we want to access the Kibana major version from core
// "PluginInitializerContext.env.packageInfo.version". In some cases it is not possible
// to dynamically inject that version without a huge refactor on the code base.
// We will then keep this single constant to declare on which major branch we are.
export const MAJOR_VERSION = '8.0.0';
Copy link
Copy Markdown
Contributor Author

@sebelga sebelga Sep 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be changed to 7.16.0 when backported

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we create a ticket so we don't forget to do so?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will immediately do it after merging 😊

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done on the PR itself: d45518a

2 changes: 1 addition & 1 deletion x-pack/plugins/index_management/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// TODO: https://github.com/elastic/kibana/issues/110892
/* eslint-disable @kbn/eslint/no_export_all */

export { API_BASE_PATH, BASE_PATH } from './constants';
export { API_BASE_PATH, BASE_PATH, MAJOR_VERSION } from './constants';

export { getTemplateParameter } from './lib';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import React, { createContext, useContext } from 'react';
import { ScopedHistory } from 'kibana/public';
import { SemVer } from 'semver';
import { ManagementAppMountParams } from 'src/plugins/management/public';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/public';

Expand Down Expand Up @@ -37,6 +38,7 @@ export interface AppDependencies {
uiSettings: CoreSetup['uiSettings'];
url: SharePluginStart['url'];
docLinks: CoreStart['docLinks'];
kibanaVersion: SemVer;
}

export const AppContextProvider = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { act } from 'react-dom/test-utils';

import { componentHelpers, MappingsEditorTestBed } from '../helpers';
import { componentHelpers, MappingsEditorTestBed, kibanaVersion } from '../helpers';

const { setup, getMappingsEditorDataFactory } = componentHelpers.mappingsEditor;

Expand Down Expand Up @@ -92,6 +92,13 @@ describe('Mappings editor: scaled float datatype', () => {
await updateFieldAndCloseFlyout();
expect(exists('mappingsEditorFieldEdit')).toBe(false);

if (kibanaVersion.major < 7) {
expect(exists('boostParameterToggle')).toBe(true);
} else {
// Since 8.x the boost parameter is deprecated
expect(exists('boostParameterToggle')).toBe(false);
}

// It should have the default parameters values added, plus the scaling factor
updatedMappings.properties.myField = {
...defaultScaledFloatParameters,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { act } from 'react-dom/test-utils';

import { componentHelpers, MappingsEditorTestBed } from '../helpers';
import { componentHelpers, MappingsEditorTestBed, kibanaVersion } from '../helpers';
import { getFieldConfig } from '../../../lib';

const { setup, getMappingsEditorDataFactory } = componentHelpers.mappingsEditor;
Expand Down Expand Up @@ -64,6 +64,7 @@ describe('Mappings editor: text datatype', () => {

const {
component,
exists,
actions: { startEditField, getToggleValue, updateFieldAndCloseFlyout },
} = testBed;

Expand All @@ -74,6 +75,13 @@ describe('Mappings editor: text datatype', () => {
const indexFieldConfig = getFieldConfig('index');
expect(getToggleValue('indexParameter.formRowToggle')).toBe(indexFieldConfig.defaultValue);

if (kibanaVersion.major < 7) {
expect(exists('boostParameterToggle')).toBe(true);
} else {
// Since 8.x the boost parameter is deprecated
expect(exists('boostParameterToggle')).toBe(false);
}

// Save the field and close the flyout
await updateFieldAndCloseFlyout();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from './mappings_editor.helpers';

export { nextTick, getRandomString, findTestSubject, TestBed } from '@kbn/test/jest';
export { kibanaVersion } from './setup_environment';

export const componentHelpers = {
mappingsEditor: { setup: mappingsEditorSetup, getMappingsEditorDataFactory },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,10 @@ const createActions = (testBed: TestBed<TestSubjects>) => {

component.update();

if (subType !== undefined) {
if (subType !== undefined && type === 'other') {
await act(async () => {
if (type === 'other') {
// subType is a text input
form.setInputValue('createFieldForm.fieldSubType', subType);
}
// subType is a text input
form.setInputValue('createFieldForm.fieldSubType', subType);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,21 @@
*/

import React from 'react';
import { SemVer } from 'semver';

/* eslint-disable-next-line @kbn/eslint/no-restricted-paths */
import '../../../../../../../../../../src/plugins/es_ui_shared/public/components/code_editor/jest_mock';
import { GlobalFlyout } from '../../../../../../../../../../src/plugins/es_ui_shared/public';
import {
docLinksServiceMock,
uiSettingsServiceMock,
} from '../../../../../../../../../../src/core/public/mocks';
import { MAJOR_VERSION } from '../../../../../../../common';
import { MappingsEditorProvider } from '../../../mappings_editor_context';
import { createKibanaReactContext } from '../../../shared_imports';

export const kibanaVersion = new SemVer(MAJOR_VERSION);

jest.mock('@elastic/eui', () => {
const original = jest.requireActual('@elastic/eui');

Expand Down Expand Up @@ -72,6 +77,9 @@ const { GlobalFlyoutProvider } = GlobalFlyout;

const { Provider: KibanaReactContextProvider } = createKibanaReactContext({
uiSettings: uiSettingsServiceMock.createSetupContract(),
kibanaVersion: {
get: () => kibanaVersion,
},
});

const defaultProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const BoostParameter = ({ defaultToggleValue }: Props) => (
href: documentationService.getBoostLink(),
}}
defaultToggleValue={defaultToggleValue}
data-test-subj="boostParameter"
>
{/* Boost level */}
<UseField
Expand Down
Loading