From d80174277ee0cd84aaa84a71c0cb90472bc680cc Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 21 Jan 2020 11:16:49 +0100 Subject: [PATCH 1/2] reactification of management/section/settings, fix update config properly --- .../sections/settings/advanced_settings.tsx | 27 ++++---- .../settings/components/field/field.tsx | 3 - .../management/sections/settings/index.html | 2 +- .../management/sections/settings/index.js | 64 ++++++------------- 4 files changed, 37 insertions(+), 59 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/management/sections/settings/advanced_settings.tsx b/src/legacy/core_plugins/kibana/public/management/sections/settings/advanced_settings.tsx index 8d189743307b7..3ee2910f4850a 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/settings/advanced_settings.tsx +++ b/src/legacy/core_plugins/kibana/public/management/sections/settings/advanced_settings.tsx @@ -20,6 +20,7 @@ import React, { Component } from 'react'; import { Comparators, EuiFlexGroup, EuiFlexItem, EuiSpacer, Query } from '@elastic/eui'; +import { npStart } from 'ui/new_platform'; import { CallOuts } from './components/call_outs'; import { Search } from './components/search'; import { Form } from './components/form'; @@ -39,7 +40,6 @@ import { import { getSettingsComponent } from './components/component_registry'; interface AdvancedSettingsProps { - config: IUiSettingsClient; query: string; enableSaving: boolean; } @@ -53,6 +53,7 @@ interface AdvancedSettingsState { type GroupedSettings = Record; export class AdvancedSettings extends Component { + private config: IUiSettingsClient; private settings: FieldSetting[]; private groupedSettings: GroupedSettings; private categoryCounts: Record; @@ -60,9 +61,11 @@ export class AdvancedSettings extends Component { + const { query } = this.state; + this.init(this.config); + this.setState({ + filteredSettings: this.mapSettings(Query.execute(query, this.settings)), + }); }); } @@ -190,8 +193,8 @@ export class AdvancedSettings extends Component diff --git a/src/legacy/core_plugins/kibana/public/management/sections/settings/components/field/field.tsx b/src/legacy/core_plugins/kibana/public/management/sections/settings/components/field/field.tsx index 0d040ddc8fd94..39a4c7190faab 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/settings/components/field/field.tsx +++ b/src/legacy/core_plugins/kibana/public/management/sections/settings/components/field/field.tsx @@ -393,9 +393,6 @@ export class Field extends PureComponent { if (changeImage) { this.cancelChangeImage(); } - this.setState({ - savedValue: valueToSave, - }); } catch (e) { toastNotifications.addDanger( i18n.translate('kbn.management.settings.field.saveFieldErrorMessage', { diff --git a/src/legacy/core_plugins/kibana/public/management/sections/settings/index.html b/src/legacy/core_plugins/kibana/public/management/sections/settings/index.html index e1b901a966960..087e6054f141b 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/settings/index.html +++ b/src/legacy/core_plugins/kibana/public/management/sections/settings/index.html @@ -1,5 +1,5 @@ -
+ diff --git a/src/legacy/core_plugins/kibana/public/management/sections/settings/index.js b/src/legacy/core_plugins/kibana/public/management/sections/settings/index.js index 9d059279f54ff..92736b1d4dd88 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/settings/index.js +++ b/src/legacy/core_plugins/kibana/public/management/sections/settings/index.js @@ -29,38 +29,10 @@ import { } from 'ui/registry/feature_catalogue'; import React from 'react'; -import { render, unmountComponentAtNode } from 'react-dom'; import { AdvancedSettings } from './advanced_settings'; import { i18n } from '@kbn/i18n'; import { getBreadcrumbs } from './breadcrumbs'; -import { npStart } from 'ui/new_platform'; - -const REACT_ADVANCED_SETTINGS_DOM_ELEMENT_ID = 'reactAdvancedSettings'; - -function updateAdvancedSettings($scope, config, query) { - $scope.$$postDigest(() => { - const node = document.getElementById(REACT_ADVANCED_SETTINGS_DOM_ELEMENT_ID); - if (!node) { - return; - } - - render( - - - , - node - ); - }); -} - -function destroyAdvancedSettings() { - const node = document.getElementById(REACT_ADVANCED_SETTINGS_DOM_ELEMENT_ID); - node && unmountComponentAtNode(node); -} +import { useEffectOnce } from 'react-use'; uiRoutes.when('/management/kibana/settings/:setting?', { template: indexTemplate, @@ -87,24 +59,30 @@ uiModules.get('apps/management').directive('kbnManagementAdvanced', function($ro return { restrict: 'E', link: function($scope) { - updateAdvancedSettings($scope, npStart.core.uiSettings, $route.current.params.setting || ''); - npStart.core.uiSettings.getUpdate$().subscribe(() => { - updateAdvancedSettings( - $scope, - npStart.core.uiSettings, - $route.current.params.setting || '' - ); - }); - - $scope.$on('$destroy', () => { - destroyAdvancedSettings(); - }); - - $route.updateParams({ setting: null }); + $scope.route = $route; }, }; }); +const AdvancedSettingsApp = ({ route }) => { + useEffectOnce(() => { + route.updateParams({ setting: null }); + }); + + return ( + + + + ); +}; + +uiModules.get('apps/management').directive('kbnManagementAdvancedReact', function(reactDirective) { + return reactDirective(AdvancedSettingsApp, [['route', { watchDepth: 'reference' }]]); +}); + management.getSection('kibana').register('settings', { display: i18n.translate('kbn.management.settings.sectionLabel', { defaultMessage: 'Advanced Settings', From 73ec86c962ae8e7fad71fd66bdb1365e62feb88e Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 21 Jan 2020 14:07:46 +0100 Subject: [PATCH 2/2] fix: fix broken tests --- .../settings/advanced_settings.test.tsx | 320 +++++++++--------- 1 file changed, 164 insertions(+), 156 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/management/sections/settings/advanced_settings.test.tsx b/src/legacy/core_plugins/kibana/public/management/sections/settings/advanced_settings.test.tsx index ebc4ef2bb0f0c..4febfbd2c5eb9 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/settings/advanced_settings.test.tsx +++ b/src/legacy/core_plugins/kibana/public/management/sections/settings/advanced_settings.test.tsx @@ -27,9 +27,12 @@ import { UiSettingsType, } from '../../../../../../../core/server/'; import { FieldSetting } from './types'; - import { AdvancedSettings } from './advanced_settings'; +jest.mock('ui/new_platform', () => ({ + npStart: mockConfig(), +})); + jest.mock('./components/field', () => ({ Field: () => { return 'field'; @@ -48,178 +51,183 @@ jest.mock('./components/search', () => ({ }, })); -const defaultConfig: Partial = { - displayName: 'defaultName', - requiresPageReload: false, - isOverridden: false, - ariaName: 'ariaName', - readOnly: false, - isCustom: false, - defVal: 'defVal', - type: 'string' as UiSettingsType, - category: ['category'], -}; +function mockConfig() { + const defaultConfig: Partial = { + displayName: 'defaultName', + requiresPageReload: false, + isOverridden: false, + ariaName: 'ariaName', + readOnly: false, + isCustom: false, + defVal: 'defVal', + type: 'string' as UiSettingsType, + category: ['category'], + }; -const config = { - set: (key: string, value: any) => Promise.resolve(true), - remove: (key: string) => Promise.resolve(true), - isCustom: (key: string) => false, - isOverridden: (key: string) => Boolean(config.getAll()[key].isOverridden), - getRegistered: () => ({} as Readonly>), - overrideLocalDefault: (key: string, value: any) => {}, - getUpdate$: () => - new Observable<{ - key: string; - newValue: any; - oldValue: any; - }>(), - isDeclared: (key: string) => true, - isDefault: (key: string) => true, + const config = { + set: (key: string, value: any) => Promise.resolve(true), + remove: (key: string) => Promise.resolve(true), + isCustom: (key: string) => false, + isOverridden: (key: string) => Boolean(config.getAll()[key].isOverridden), + getRegistered: () => ({} as Readonly>), + overrideLocalDefault: (key: string, value: any) => {}, + getUpdate$: () => + new Observable<{ + key: string; + newValue: any; + oldValue: any; + }>(), + isDeclared: (key: string) => true, + isDefault: (key: string) => true, - getSaved$: () => - new Observable<{ - key: string; - newValue: any; - oldValue: any; - }>(), + getSaved$: () => + new Observable<{ + key: string; + newValue: any; + oldValue: any; + }>(), - getUpdateErrors$: () => new Observable(), - get: (key: string, defaultOverride?: any): any => config.getAll()[key] || defaultOverride, - get$: (key: string) => new Observable(config.get(key)), - getAll: (): Readonly> => { - return { - 'test:array:setting': { - ...defaultConfig, - value: ['default_value'], - name: 'Test array setting', - description: 'Description for Test array setting', - category: ['elasticsearch'], - }, - 'test:boolean:setting': { - ...defaultConfig, - value: true, - name: 'Test boolean setting', - description: 'Description for Test boolean setting', - category: ['elasticsearch'], - }, - 'test:image:setting': { - ...defaultConfig, - value: null, - name: 'Test image setting', - description: 'Description for Test image setting', - type: 'image', - }, - 'test:json:setting': { - ...defaultConfig, - value: '{"foo": "bar"}', - name: 'Test json setting', - description: 'Description for Test json setting', - type: 'json', - }, - 'test:markdown:setting': { - ...defaultConfig, - value: '', - name: 'Test markdown setting', - description: 'Description for Test markdown setting', - type: 'markdown', - }, - 'test:number:setting': { - ...defaultConfig, - value: 5, - name: 'Test number setting', - description: 'Description for Test number setting', - }, - 'test:select:setting': { - ...defaultConfig, - value: 'orange', - name: 'Test select setting', - description: 'Description for Test select setting', - type: 'select', - options: ['apple', 'orange', 'banana'], - }, - 'test:string:setting': { - ...defaultConfig, - ...{ - value: null, - name: 'Test string setting', - description: 'Description for Test string setting', - type: 'string', - isCustom: true, + getUpdateErrors$: () => new Observable(), + get: (key: string, defaultOverride?: any): any => config.getAll()[key] || defaultOverride, + get$: (key: string) => new Observable(config.get(key)), + getAll: (): Readonly> => { + return { + 'test:array:setting': { + ...defaultConfig, + value: ['default_value'], + name: 'Test array setting', + description: 'Description for Test array setting', + category: ['elasticsearch'], }, - }, - 'test:readonlystring:setting': { - ...defaultConfig, - ...{ - value: null, - name: 'Test readonly string setting', - description: 'Description for Test readonly string setting', - type: 'string', - readOnly: true, + 'test:boolean:setting': { + ...defaultConfig, + value: true, + name: 'Test boolean setting', + description: 'Description for Test boolean setting', + category: ['elasticsearch'], }, - }, - 'test:customstring:setting': { - ...defaultConfig, - ...{ + 'test:image:setting': { + ...defaultConfig, value: null, - name: 'Test custom string setting', - description: 'Description for Test custom string setting', + name: 'Test image setting', + description: 'Description for Test image setting', + type: 'image', + }, + 'test:json:setting': { + ...defaultConfig, + value: '{"foo": "bar"}', + name: 'Test json setting', + description: 'Description for Test json setting', + type: 'json', + }, + 'test:markdown:setting': { + ...defaultConfig, + value: '', + name: 'Test markdown setting', + description: 'Description for Test markdown setting', + type: 'markdown', + }, + 'test:number:setting': { + ...defaultConfig, + value: 5, + name: 'Test number setting', + description: 'Description for Test number setting', + }, + 'test:select:setting': { + ...defaultConfig, + value: 'orange', + name: 'Test select setting', + description: 'Description for Test select setting', + type: 'select', + options: ['apple', 'orange', 'banana'], + }, + 'test:string:setting': { + ...defaultConfig, + ...{ + value: null, + name: 'Test string setting', + description: 'Description for Test string setting', + type: 'string', + isCustom: true, + }, + }, + 'test:readonlystring:setting': { + ...defaultConfig, + ...{ + value: null, + name: 'Test readonly string setting', + description: 'Description for Test readonly string setting', + type: 'string', + readOnly: true, + }, + }, + 'test:customstring:setting': { + ...defaultConfig, + ...{ + value: null, + name: 'Test custom string setting', + description: 'Description for Test custom string setting', + type: 'string', + isCustom: true, + }, + }, + 'test:isOverridden:string': { + ...defaultConfig, + isOverridden: true, + value: 'foo', + name: 'An overridden string', + description: 'Description for overridden string', type: 'string', - isCustom: true, }, - }, - 'test:isOverridden:string': { - ...defaultConfig, - isOverridden: true, - value: 'foo', - name: 'An overridden string', - description: 'Description for overridden string', - type: 'string', - }, - 'test:isOverridden:number': { - ...defaultConfig, - isOverridden: true, - value: 1234, - name: 'An overridden number', - description: 'Description for overridden number', - type: 'number', - }, - 'test:isOverridden:json': { - ...defaultConfig, - isOverridden: true, - value: dedent` - { - "foo": "bar" - } - `, - name: 'An overridden json', - description: 'Description for overridden json', - type: 'json', - }, - 'test:isOverridden:select': { - ...defaultConfig, - isOverridden: true, - value: 'orange', - name: 'Test overridden select setting', - description: 'Description for overridden select setting', - type: 'select', - options: ['apple', 'orange', 'banana'], - }, - }; - }, -}; + 'test:isOverridden:number': { + ...defaultConfig, + isOverridden: true, + value: 1234, + name: 'An overridden number', + description: 'Description for overridden number', + type: 'number', + }, + 'test:isOverridden:json': { + ...defaultConfig, + isOverridden: true, + value: dedent` + { + "foo": "bar" + } + `, + name: 'An overridden json', + description: 'Description for overridden json', + type: 'json', + }, + 'test:isOverridden:select': { + ...defaultConfig, + isOverridden: true, + value: 'orange', + name: 'Test overridden select setting', + description: 'Description for overridden select setting', + type: 'select', + options: ['apple', 'orange', 'banana'], + }, + }; + }, + }; + return { + core: { + uiSettings: config, + }, + }; +} describe('AdvancedSettings', () => { it('should render specific setting if given setting key', async () => { - const component = shallow( - - ); + const component = shallow(); expect(component).toMatchSnapshot(); }); it('should render read-only when saving is disabled', async () => { const component = shallow( - + ); expect(component).toMatchSnapshot();