From 7faf18e9067d74dd0af35b6aaa94a5430d52fb44 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 11 Mar 2021 16:49:08 +0100 Subject: [PATCH 01/20] restore the banners ui settings --- x-pack/plugins/banners/public/plugin.tsx | 31 +++----- x-pack/plugins/banners/public/types.ts | 12 --- x-pack/plugins/banners/server/config.ts | 4 +- x-pack/plugins/banners/server/plugin.ts | 2 + x-pack/plugins/banners/server/routes/info.ts | 29 ++++++- .../banners/server/ui_settings.test.ts | 65 +++++++++++++++ x-pack/plugins/banners/server/ui_settings.ts | 79 +++++++++++++++++++ 7 files changed, 184 insertions(+), 38 deletions(-) delete mode 100644 x-pack/plugins/banners/public/types.ts create mode 100644 x-pack/plugins/banners/server/ui_settings.test.ts create mode 100644 x-pack/plugins/banners/server/ui_settings.ts diff --git a/x-pack/plugins/banners/public/plugin.tsx b/x-pack/plugins/banners/public/plugin.tsx index dca99a816a25b..290ae58e51537 100644 --- a/x-pack/plugins/banners/public/plugin.tsx +++ b/x-pack/plugins/banners/public/plugin.tsx @@ -9,35 +9,28 @@ import React from 'react'; import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'src/core/public'; import { toMountPoint } from '../../../../src/plugins/kibana_react/public'; import { Banner } from './components'; -import { BannerClientConfig } from './types'; import { getBannerInfo } from './get_banner_info'; export class BannersPlugin implements Plugin<{}, {}, {}, {}> { - private readonly config: BannerClientConfig; - - constructor(context: PluginInitializerContext) { - this.config = context.config.get(); - } + constructor(context: PluginInitializerContext) {} setup({}: CoreSetup<{}, {}>) { return {}; } start({ chrome, uiSettings, http }: CoreStart) { - if (this.config.placement !== 'disabled') { - getBannerInfo(http).then( - ({ allowed, banner }) => { - if (allowed) { - chrome.setHeaderBanner({ - content: toMountPoint(), - }); - } - }, - () => { - chrome.setHeaderBanner(undefined); + getBannerInfo(http).then( + ({ allowed, banner }) => { + if (allowed) { + chrome.setHeaderBanner({ + content: toMountPoint(), + }); } - ); - } + }, + () => { + chrome.setHeaderBanner(undefined); + } + ); return {}; } diff --git a/x-pack/plugins/banners/public/types.ts b/x-pack/plugins/banners/public/types.ts deleted file mode 100644 index 1f0ce524a785e..0000000000000 --- a/x-pack/plugins/banners/public/types.ts +++ /dev/null @@ -1,12 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { BannerPlacement } from '../common'; - -export interface BannerClientConfig { - placement: BannerPlacement; -} diff --git a/x-pack/plugins/banners/server/config.ts b/x-pack/plugins/banners/server/config.ts index 9a8cc9680c296..da59940c7a541 100644 --- a/x-pack/plugins/banners/server/config.ts +++ b/x-pack/plugins/banners/server/config.ts @@ -36,7 +36,5 @@ export type BannersConfigType = TypeOf; export const config: PluginConfigDescriptor = { schema: configSchema, - exposeToBrowser: { - placement: true, - }, + exposeToBrowser: {}, }; diff --git a/x-pack/plugins/banners/server/plugin.ts b/x-pack/plugins/banners/server/plugin.ts index 66cd083189975..2aae6f2ead53e 100644 --- a/x-pack/plugins/banners/server/plugin.ts +++ b/x-pack/plugins/banners/server/plugin.ts @@ -10,6 +10,7 @@ import { BannerConfiguration } from '../common'; import { BannersConfigType } from './config'; import { BannersRequestHandlerContext } from './types'; import { registerRoutes } from './routes'; +import { registerSettings } from './ui_settings'; export class BannersPlugin implements Plugin<{}, {}, {}, {}> { private readonly config: BannerConfiguration; @@ -21,6 +22,7 @@ export class BannersPlugin implements Plugin<{}, {}, {}, {}> { setup({ uiSettings, getStartServices, http }: CoreSetup<{}, {}>) { const router = http.createRouter(); registerRoutes(router, this.config); + registerSettings(uiSettings, this.config); return {}; } diff --git a/x-pack/plugins/banners/server/routes/info.ts b/x-pack/plugins/banners/server/routes/info.ts index e0db842028c37..6c59698860021 100644 --- a/x-pack/plugins/banners/server/routes/info.ts +++ b/x-pack/plugins/banners/server/routes/info.ts @@ -5,8 +5,9 @@ * 2.0. */ +import { IUiSettingsClient } from 'kibana/server'; import { ILicense } from '../../../licensing/server'; -import { BannerInfoResponse, BannerConfiguration } from '../../common'; +import { BannerInfoResponse, BannerConfiguration, BannerPlacement } from '../../common'; import { BannersRouter } from '../types'; export const registerInfoRoute = (router: BannersRouter, config: BannerConfiguration) => { @@ -15,16 +16,20 @@ export const registerInfoRoute = (router: BannersRouter, config: BannerConfigura path: '/api/banners/info', validate: false, options: { - authRequired: false, + authRequired: 'optional', }, }, - (ctx, req, res) => { + async (ctx, req, res) => { const allowed = isValidLicense(ctx.licensing.license); + const bannerConfig = req.auth.isAuthenticated + ? await getBannerConfig(ctx.core.uiSettings.client) + : config; + return res.ok({ body: { allowed, - banner: config, + banner: bannerConfig, } as BannerInfoResponse, }); } @@ -34,3 +39,19 @@ export const registerInfoRoute = (router: BannersRouter, config: BannerConfigura const isValidLicense = (license: ILicense): boolean => { return license.hasAtLeast('gold'); }; + +const getBannerConfig = async (client: IUiSettingsClient): Promise => { + const [placement, textContent, textColor, backgroundColor] = await Promise.all([ + client.get('banner:placement'), + client.get('banner:textContent'), + client.get('banner:textColor'), + client.get('banner:backgroundColor'), + ]); + + return { + placement, + textContent, + textColor, + backgroundColor, + }; +}; diff --git a/x-pack/plugins/banners/server/ui_settings.test.ts b/x-pack/plugins/banners/server/ui_settings.test.ts new file mode 100644 index 0000000000000..9947ff6189e56 --- /dev/null +++ b/x-pack/plugins/banners/server/ui_settings.test.ts @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { uiSettingsServiceMock } from '../../../../src/core/server/mocks'; +import type { BannerConfiguration } from '../common'; +import { registerSettings } from './ui_settings'; + +const createConfig = (parts: Partial = {}): BannerConfiguration => ({ + placement: 'disabled', + backgroundColor: '#0000', + textColor: '#FFFFFF', + textContent: 'Hello from the banner', + ...parts, +}); + +describe('registerSettings', () => { + let uiSettings: ReturnType; + + beforeEach(() => { + uiSettings = uiSettingsServiceMock.createSetupContract(); + }); + + it('registers the settings', () => { + registerSettings(uiSettings, createConfig()); + + expect(uiSettings.register).toHaveBeenCalledTimes(1); + expect(uiSettings.register).toHaveBeenCalledWith({ + 'banner:placement': expect.any(Object), + 'banner:textContent': expect.any(Object), + 'banner:textColor': expect.any(Object), + 'banner:backgroundColor': expect.any(Object), + }); + }); + + it('uses the configuration values as defaults', () => { + const config = createConfig({ + placement: 'header', + backgroundColor: '#FF00CC', + textColor: '#AAFFEE', + textContent: 'Some text', + }); + + registerSettings(uiSettings, config); + + expect(uiSettings.register).toHaveBeenCalledTimes(1); + expect(uiSettings.register).toHaveBeenCalledWith({ + 'banner:placement': expect.objectContaining({ + value: config.placement, + }), + 'banner:textContent': expect.objectContaining({ + value: config.textContent, + }), + 'banner:textColor': expect.objectContaining({ + value: config.textColor, + }), + 'banner:backgroundColor': expect.objectContaining({ + value: config.backgroundColor, + }), + }); + }); +}); diff --git a/x-pack/plugins/banners/server/ui_settings.ts b/x-pack/plugins/banners/server/ui_settings.ts new file mode 100644 index 0000000000000..d0c9c0bef00eb --- /dev/null +++ b/x-pack/plugins/banners/server/ui_settings.ts @@ -0,0 +1,79 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { schema } from '@kbn/config-schema'; +import { i18n } from '@kbn/i18n'; +import { UiSettingsServiceSetup } from 'src/core/server'; +import { BannerConfiguration } from '../common'; +import { isHexColor } from './utils'; + +export const registerSettings = ( + uiSettings: UiSettingsServiceSetup, + config: BannerConfiguration +) => { + uiSettings.register({ + 'banner:placement': { + name: i18n.translate('xpack.banners.settings.placement.title', { + defaultMessage: 'Banner placement', + }), + category: ['banner'], + order: 1, + type: 'select', + value: config.placement, + options: ['disabled', 'header'], + optionLabels: {}, + requiresPageReload: true, + schema: schema.oneOf([schema.literal('disabled'), schema.literal('header')]), + }, + 'banner:textContent': { + name: i18n.translate('xpack.banners.settings.textContent.title', { + defaultMessage: 'Banner text', + }), + sensitive: true, + category: ['banner'], + order: 2, + type: 'markdown', + value: config.textContent, + requiresPageReload: true, + schema: schema.string(), + }, + 'banner:textColor': { + name: i18n.translate('xpack.banners.settings.textColor.title', { + defaultMessage: 'Banner text color', + }), + category: ['banner'], + order: 3, + type: 'color', + value: config.textColor, + requiresPageReload: true, + schema: schema.string({ + validate: (color) => { + if (!isHexColor(color)) { + return `'banner:textColor' must be an hex color`; + } + }, + }), + }, + 'banner:backgroundColor': { + name: i18n.translate('xpack.banners.settings.backgroundColor.title', { + defaultMessage: 'Banner background color', + }), + category: ['banner'], + order: 4, + type: 'color', + value: config.backgroundColor, + requiresPageReload: true, + schema: schema.string({ + validate: (color) => { + if (!isHexColor(color)) { + return `'banner:backgroundColor' must be an hex color`; + } + }, + }), + }, + }); +}; From 14739dffcd18e1597181378b646c27834f02f63f Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 11 Mar 2021 17:45:37 +0100 Subject: [PATCH 02/20] fix banner init logic --- x-pack/plugins/banners/public/plugin.test.tsx | 25 ++++--------------- x-pack/plugins/banners/public/plugin.tsx | 2 +- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/x-pack/plugins/banners/public/plugin.test.tsx b/x-pack/plugins/banners/public/plugin.test.tsx index 036ad17e2598e..4f80ab80b3423 100644 --- a/x-pack/plugins/banners/public/plugin.test.tsx +++ b/x-pack/plugins/banners/public/plugin.test.tsx @@ -8,7 +8,6 @@ import { getBannerInfoMock } from './plugin.test.mocks'; import { coreMock } from '../../../../src/core/public/mocks'; import { BannersPlugin } from './plugin'; -import { BannerClientConfig } from './types'; const nextTick = async () => await new Promise((resolve) => resolve()); @@ -28,8 +27,8 @@ describe('BannersPlugin', () => { }); }); - const startPlugin = async (config: BannerClientConfig) => { - pluginInitContext = coreMock.createPluginInitializerContext(config); + const startPlugin = async () => { + pluginInitContext = coreMock.createPluginInitializerContext(); plugin = new BannersPlugin(pluginInitContext); plugin.setup(coreSetup); plugin.start(coreStart); @@ -42,29 +41,17 @@ describe('BannersPlugin', () => { }); it('calls `getBannerInfo` if `config.placement !== disabled`', async () => { - await startPlugin({ - placement: 'header', - }); + await startPlugin(); expect(getBannerInfoMock).toHaveBeenCalledTimes(1); }); - it('does not call `getBannerInfo` if `config.placement === disabled`', async () => { - await startPlugin({ - placement: 'disabled', - }); - - expect(getBannerInfoMock).not.toHaveBeenCalled(); - }); - it('registers the header banner if `getBannerInfo` return `allowed=true`', async () => { getBannerInfoMock.mockResolvedValue({ allowed: true, }); - await startPlugin({ - placement: 'header', - }); + await startPlugin(); expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledTimes(1); expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledWith({ @@ -77,9 +64,7 @@ describe('BannersPlugin', () => { allowed: false, }); - await startPlugin({ - placement: 'header', - }); + await startPlugin(); expect(coreStart.chrome.setHeaderBanner).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/banners/public/plugin.tsx b/x-pack/plugins/banners/public/plugin.tsx index 290ae58e51537..57853ece553d8 100644 --- a/x-pack/plugins/banners/public/plugin.tsx +++ b/x-pack/plugins/banners/public/plugin.tsx @@ -21,7 +21,7 @@ export class BannersPlugin implements Plugin<{}, {}, {}, {}> { start({ chrome, uiSettings, http }: CoreStart) { getBannerInfo(http).then( ({ allowed, banner }) => { - if (allowed) { + if (allowed && banner.placement === 'header') { chrome.setHeaderBanner({ content: toMountPoint(), }); From eedff07faa6d85b03829c5094c431d8dae428e15 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 16 Mar 2021 09:36:00 +0100 Subject: [PATCH 03/20] fix unit tests --- x-pack/plugins/banners/public/plugin.test.tsx | 72 ++++++++++++++----- 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/banners/public/plugin.test.tsx b/x-pack/plugins/banners/public/plugin.test.tsx index 4f80ab80b3423..232bd840ba353 100644 --- a/x-pack/plugins/banners/public/plugin.test.tsx +++ b/x-pack/plugins/banners/public/plugin.test.tsx @@ -7,10 +7,19 @@ import { getBannerInfoMock } from './plugin.test.mocks'; import { coreMock } from '../../../../src/core/public/mocks'; +import { BannerConfiguration } from '../common/types'; import { BannersPlugin } from './plugin'; const nextTick = async () => await new Promise((resolve) => resolve()); +const createBannerConfig = (parts: Partial = {}): BannerConfiguration => ({ + placement: 'disabled', + textContent: 'foo', + textColor: '#FFFFFF', + backgroundColor: '#000000', + ...parts, +}); + describe('BannersPlugin', () => { let plugin: BannersPlugin; let pluginInitContext: ReturnType; @@ -24,6 +33,7 @@ describe('BannersPlugin', () => { getBannerInfoMock.mockResolvedValue({ allowed: false, + banner: createBannerConfig(), }); }); @@ -40,32 +50,62 @@ describe('BannersPlugin', () => { getBannerInfoMock.mockReset(); }); - it('calls `getBannerInfo` if `config.placement !== disabled`', async () => { - await startPlugin(); + describe('when banner is allowed', () => { + it('registers the header banner if `banner.placement` is `header`', async () => { + getBannerInfoMock.mockResolvedValue({ + allowed: true, + banner: createBannerConfig({ + placement: 'header', + }), + }); - expect(getBannerInfoMock).toHaveBeenCalledTimes(1); - }); + await startPlugin(); - it('registers the header banner if `getBannerInfo` return `allowed=true`', async () => { - getBannerInfoMock.mockResolvedValue({ - allowed: true, + expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledTimes(1); + expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledWith({ + content: expect.any(Function), + }); }); - await startPlugin(); + it('does not register the header banner if `banner.placement` is `disabled`', async () => { + getBannerInfoMock.mockResolvedValue({ + allowed: true, + banner: createBannerConfig({ + placement: 'disabled', + }), + }); + + await startPlugin(); - expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledTimes(1); - expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledWith({ - content: expect.any(Function), + expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledTimes(0); }); }); - it('does not register the header banner if `getBannerInfo` return `allowed=false`', async () => { - getBannerInfoMock.mockResolvedValue({ - allowed: false, + describe('when banner is not allowed', () => { + it('does not register the header banner if `banner.placement` is `header`', async () => { + getBannerInfoMock.mockResolvedValue({ + allowed: false, + banner: createBannerConfig({ + placement: 'header', + }), + }); + + await startPlugin(); + + expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledTimes(0); }); - await startPlugin(); + it('does not register the header banner if `banner.placement` is `disabled`', async () => { + getBannerInfoMock.mockResolvedValue({ + allowed: false, + banner: createBannerConfig({ + placement: 'disabled', + }), + }); - expect(coreStart.chrome.setHeaderBanner).not.toHaveBeenCalled(); + await startPlugin(); + + expect(coreStart.chrome.setHeaderBanner).toHaveBeenCalledTimes(0); + }); }); }); From 4afec768b87b2c44077c61b9d9f6d3f39fa54253 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 16 Mar 2021 09:41:39 +0100 Subject: [PATCH 04/20] update telemetry schema --- .../server/collectors/management/schema.ts | 16 +++++++++++++ .../server/collectors/management/types.ts | 4 ++++ src/plugins/telemetry/schema/oss_plugins.json | 24 +++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts b/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts index f166e4fcebfa3..59516d1f11dda 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts @@ -43,6 +43,10 @@ export const stackManagementSchema: MakeSchemaFrom = { type: 'keyword', _meta: { description: 'Default value of the setting was changed.' }, }, + 'banner:textContent': { + type: 'text', + _meta: { description: 'Default value of the setting was changed.' }, + }, // non-sensitive 'visualize:enableLabs': { type: 'boolean', @@ -412,4 +416,16 @@ export const stackManagementSchema: MakeSchemaFrom = { type: 'boolean', _meta: { description: 'Non-default value of setting.' }, }, + 'banner:placement': { + type: 'keyword', + _meta: { description: 'Non-default value of setting.' }, + }, + 'banner:textColor': { + type: 'text', + _meta: { description: 'Non-default value of setting.' }, + }, + 'banner:backgroundColor': { + type: 'text', + _meta: { description: 'Non-default value of setting.' }, + }, }; diff --git a/src/plugins/kibana_usage_collection/server/collectors/management/types.ts b/src/plugins/kibana_usage_collection/server/collectors/management/types.ts index 8bbc14e0678d3..a3c74bf3c3f3e 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/management/types.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/management/types.ts @@ -18,6 +18,7 @@ export interface UsageStats { 'timelion:graphite.url': string; 'xpackDashboardMode:roles': string; 'securitySolution:ipReputationLinks': string; + 'banner:textContent': string; /** * non-sensitive settings */ @@ -113,4 +114,7 @@ export interface UsageStats { 'csv:quoteValues': boolean; 'dateFormat:dow': string; dateFormat: string; + 'banner:placement': string; + 'banner:textColor': string; + 'banner:backgroundColor': string; } diff --git a/src/plugins/telemetry/schema/oss_plugins.json b/src/plugins/telemetry/schema/oss_plugins.json index f7795dbf9b2f8..176e375f36f2b 100644 --- a/src/plugins/telemetry/schema/oss_plugins.json +++ b/src/plugins/telemetry/schema/oss_plugins.json @@ -7478,6 +7478,12 @@ "description": "Default value of the setting was changed." } }, + "banner:textContent": { + "type": "text", + "_meta": { + "description": "Default value of the setting was changed." + } + }, "visualize:enableLabs": { "type": "boolean", "_meta": { @@ -8032,6 +8038,24 @@ "_meta": { "description": "Non-default value of setting." } + }, + "banner:placement": { + "type": "keyword", + "_meta": { + "description": "Non-default value of setting." + } + }, + "banner:textColor": { + "type": "text", + "_meta": { + "description": "Non-default value of setting." + } + }, + "banner:backgroundColor": { + "type": "text", + "_meta": { + "description": "Non-default value of setting." + } } } }, From 28b08a517b610d428918b9d9a3e159a430b2c216 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 16 Mar 2021 10:10:53 +0100 Subject: [PATCH 05/20] add basic server-side plugin tests --- src/core/server/mocks.ts | 10 +++- .../banners/server/plugin.test.mocks.ts | 16 ++++++ x-pack/plugins/banners/server/plugin.test.ts | 53 +++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/banners/server/plugin.test.mocks.ts create mode 100644 x-pack/plugins/banners/server/plugin.test.ts diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index 19056ae1b9bc7..68304ce40b985 100644 --- a/src/core/server/mocks.ts +++ b/src/core/server/mocks.ts @@ -50,6 +50,8 @@ export { capabilitiesServiceMock } from './capabilities/capabilities_service.moc export { coreUsageDataServiceMock } from './core_usage_data/core_usage_data_service.mock'; export { i18nServiceMock } from './i18n/i18n_service.mock'; +type MockedPluginInitializerConfig = jest.Mocked['config']>; + export function pluginInitializerContextConfigMock(config: T) { const globalConfig: SharedGlobalConfig = { kibana: { @@ -68,7 +70,7 @@ export function pluginInitializerContextConfigMock(config: T) { }, }; - const mock: jest.Mocked['config']> = { + const mock: MockedPluginInitializerConfig = { legacy: { globalConfig$: of(globalConfig), get: () => globalConfig, @@ -80,8 +82,12 @@ export function pluginInitializerContextConfigMock(config: T) { return mock; } +type PluginInitializerContextMock = Omit, 'config'> & { + config: MockedPluginInitializerConfig; +}; + function pluginInitializerContextMock(config: T = {} as T) { - const mock: PluginInitializerContext = { + const mock: PluginInitializerContextMock = { opaqueId: Symbol(), logger: loggingSystemMock.create(), env: { diff --git a/x-pack/plugins/banners/server/plugin.test.mocks.ts b/x-pack/plugins/banners/server/plugin.test.mocks.ts new file mode 100644 index 0000000000000..316699c2c2020 --- /dev/null +++ b/x-pack/plugins/banners/server/plugin.test.mocks.ts @@ -0,0 +1,16 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export const registerRoutesMock = jest.fn(); +jest.doMock('./routes', () => ({ + registerRoutes: registerRoutesMock, +})); + +export const registerSettingsMock = jest.fn(); +jest.doMock('./ui_settings', () => ({ + registerSettings: registerSettingsMock, +})); diff --git a/x-pack/plugins/banners/server/plugin.test.ts b/x-pack/plugins/banners/server/plugin.test.ts new file mode 100644 index 0000000000000..3838447a576f3 --- /dev/null +++ b/x-pack/plugins/banners/server/plugin.test.ts @@ -0,0 +1,53 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { registerRoutesMock, registerSettingsMock } from './plugin.test.mocks'; + +import { coreMock } from '../../../../src/core/server/mocks'; +import { BannersPlugin } from './plugin'; +import { BannersConfigType } from './config'; + +describe('BannersPlugin', () => { + let plugin: BannersPlugin; + let pluginInitContext: ReturnType; + let coreSetup: ReturnType; + let bannerConfig: BannersConfigType; + + beforeEach(() => { + bannerConfig = { + placement: 'header', + textContent: 'foo', + backgroundColor: '#000000', + textColor: '#FFFFFF', + }; + pluginInitContext = coreMock.createPluginInitializerContext(); + pluginInitContext.config.get.mockReturnValue(bannerConfig); + coreSetup = coreMock.createSetup(); + + plugin = new BannersPlugin(pluginInitContext); + }); + + afterEach(() => { + registerRoutesMock.mockReset(); + registerSettingsMock.mockReset(); + }); + + describe('#setup', () => { + it('calls `registerRoutes` with the correct parameters', () => { + plugin.setup(coreSetup); + + expect(registerRoutesMock).toHaveBeenCalledTimes(1); + expect(registerRoutesMock).toHaveBeenCalledWith(expect.any(Object), bannerConfig); + }); + it('calls `registerSettings` with the correct parameters', () => { + plugin.setup(coreSetup); + + expect(registerSettingsMock).toHaveBeenCalledTimes(1); + expect(registerSettingsMock).toHaveBeenCalledWith(coreSetup.uiSettings, bannerConfig); + }); + }); +}); From 559509feb29655cc7291a32527464a4e650a2779 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 16 Mar 2021 14:01:44 +0100 Subject: [PATCH 06/20] add FTR tests for banners plugin --- .../management_app/components/field/field.tsx | 1 + test/functional/page_objects/settings_page.ts | 19 ++ x-pack/test/banners_functional/config.ts | 45 +++ .../ftr_provider_context.ts | 13 + .../test/banners_functional/tests/global.ts | 22 ++ x-pack/test/banners_functional/tests/index.ts | 17 ++ .../test/banners_functional/tests/spaces.ts | 67 ++++ .../es_archives/banners/multispace/data.json | 62 ++++ .../banners/multispace/mappings.json | 287 ++++++++++++++++++ .../functional/page_objects/banners_page.ts | 30 ++ x-pack/test/functional/page_objects/index.ts | 2 + .../global_search/global_search_bar.ts | 2 +- 12 files changed, 566 insertions(+), 1 deletion(-) create mode 100644 x-pack/test/banners_functional/config.ts create mode 100644 x-pack/test/banners_functional/ftr_provider_context.ts create mode 100644 x-pack/test/banners_functional/tests/global.ts create mode 100644 x-pack/test/banners_functional/tests/index.ts create mode 100644 x-pack/test/banners_functional/tests/spaces.ts create mode 100644 x-pack/test/functional/es_archives/banners/multispace/data.json create mode 100644 x-pack/test/functional/es_archives/banners/multispace/mappings.json create mode 100644 x-pack/test/functional/page_objects/banners_page.ts diff --git a/src/plugins/advanced_settings/public/management_app/components/field/field.tsx b/src/plugins/advanced_settings/public/management_app/components/field/field.tsx index f5db5c3e371b3..d4a5020bbbb82 100644 --- a/src/plugins/advanced_settings/public/management_app/components/field/field.tsx +++ b/src/plugins/advanced_settings/public/management_app/components/field/field.tsx @@ -326,6 +326,7 @@ export class Field extends PureComponent {
{ + return (window as any).ace.edit(editor).setValue(value); + }, + `advancedSetting-editField-${propertyName}-editor`, + propertyValue + ); + await testSubjects.click(`advancedSetting-saveButton`); + await PageObjects.header.waitUntilLoadingHasFinished(); + } + async toggleAdvancedSettingCheckbox(propertyName: string) { await testSubjects.click(`advancedSetting-editField-${propertyName}`); await PageObjects.header.waitUntilLoadingHasFinished(); @@ -162,6 +179,7 @@ export function SettingsPageProvider({ getService, getPageObjects }: FtrProvider async sortBy(columnName: string) { const chartTypes = await find.allByCssSelector('table.euiTable thead tr th button'); + async function getChartType(chart: Record) { const chartString = await chart.getVisibleText(); if (chartString === columnName) { @@ -169,6 +187,7 @@ export function SettingsPageProvider({ getService, getPageObjects }: FtrProvider await PageObjects.header.waitUntilLoadingHasFinished(); } } + const getChartTypesPromises = chartTypes.map(getChartType); return Promise.all(getChartTypesPromises); } diff --git a/x-pack/test/banners_functional/config.ts b/x-pack/test/banners_functional/config.ts new file mode 100644 index 0000000000000..21cce31ca5d85 --- /dev/null +++ b/x-pack/test/banners_functional/config.ts @@ -0,0 +1,45 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import path from 'path'; +import { FtrConfigProviderContext } from '@kbn/test/types/ftr'; +import { services, pageObjects } from './ftr_provider_context'; + +export default async function ({ readConfigFile }: FtrConfigProviderContext) { + const kibanaFunctionalConfig = await readConfigFile(require.resolve('../functional/config.js')); + + return { + testFiles: [require.resolve('./tests')], + servers: { + ...kibanaFunctionalConfig.get('servers'), + }, + services, + pageObjects, + + junit: { + reportName: 'X-Pack Banners Functional Tests', + }, + + esTestCluster: kibanaFunctionalConfig.get('esTestCluster'), + apps: { + ...kibanaFunctionalConfig.get('apps'), + }, + + esArchiver: { + directory: path.resolve(__dirname, '..', 'functional', 'es_archives'), + }, + + kbnTestServer: { + ...kibanaFunctionalConfig.get('kbnTestServer'), + serverArgs: [ + ...kibanaFunctionalConfig.get('kbnTestServer.serverArgs'), + '--xpack.banners.placement=header', + '--xpack.banners.textContent="global banner text"', + ], + }, + }; +} diff --git a/x-pack/test/banners_functional/ftr_provider_context.ts b/x-pack/test/banners_functional/ftr_provider_context.ts new file mode 100644 index 0000000000000..faac2954b00f6 --- /dev/null +++ b/x-pack/test/banners_functional/ftr_provider_context.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { GenericFtrProviderContext } from '@kbn/test/types/ftr'; +import { services } from '../functional/services'; +import { pageObjects } from '../functional/page_objects'; + +export type FtrProviderContext = GenericFtrProviderContext; +export { services, pageObjects }; diff --git a/x-pack/test/banners_functional/tests/global.ts b/x-pack/test/banners_functional/tests/global.ts new file mode 100644 index 0000000000000..cef404d7ed132 --- /dev/null +++ b/x-pack/test/banners_functional/tests/global.ts @@ -0,0 +1,22 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../ftr_provider_context'; + +export default function ({ getPageObjects }: FtrProviderContext) { + const PageObjects = getPageObjects(['common', 'security', 'banners']); + + describe('global pages', () => { + it('displays the global banner on the login page', async () => { + await PageObjects.common.navigateToApp('login'); + + expect(await PageObjects.banners.isTopBannerVisible()).to.eql(true); + expect(await PageObjects.banners.getTopBannerText()).to.eql('global banner text'); + }); + }); +} diff --git a/x-pack/test/banners_functional/tests/index.ts b/x-pack/test/banners_functional/tests/index.ts new file mode 100644 index 0000000000000..301c872c746e1 --- /dev/null +++ b/x-pack/test/banners_functional/tests/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrProviderContext } from '../ftr_provider_context'; + +export default function ({ loadTestFile }: FtrProviderContext) { + describe('banners - functional tests', function () { + this.tags('ciGroup2'); + + loadTestFile(require.resolve('./global')); + loadTestFile(require.resolve('./spaces')); + }); +} diff --git a/x-pack/test/banners_functional/tests/spaces.ts b/x-pack/test/banners_functional/tests/spaces.ts new file mode 100644 index 0000000000000..ea51461c94846 --- /dev/null +++ b/x-pack/test/banners_functional/tests/spaces.ts @@ -0,0 +1,67 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../ftr_provider_context'; + +export default function ({ getPageObjects, getService }: FtrProviderContext) { + const esArchiver = getService('esArchiver'); + const PageObjects = getPageObjects([ + 'common', + 'security', + 'banners', + 'settings', + 'spaceSelector', + ]); + + describe('per-spaces banners', () => { + before(async () => { + await esArchiver.load('banners/multispace'); + }); + + after(async () => { + await esArchiver.unload('banners/multispace'); + }); + + before(async () => { + await PageObjects.security.login(undefined, undefined, { + expectSpaceSelector: true, + }); + await PageObjects.spaceSelector.clickSpaceCard('default'); + + await PageObjects.settings.navigateTo(); + await PageObjects.settings.clickKibanaSettings(); + + await PageObjects.settings.setAdvancedSettingsTextArea( + 'banner:textContent', + 'default space banner text' + ); + }); + + it('displays the space-specific banner within the space', async () => { + await PageObjects.common.navigateToApp('home'); + + expect(await PageObjects.banners.isTopBannerVisible()).to.eql(true); + expect(await PageObjects.banners.getTopBannerText()).to.eql('default space banner text'); + }); + + it('displays the global banner within another space', async () => { + await PageObjects.common.navigateToApp('home', { basePath: '/s/another-space' }); + + expect(await PageObjects.banners.isTopBannerVisible()).to.eql(true); + expect(await PageObjects.banners.getTopBannerText()).to.eql('global banner text'); + }); + + it('displays the global banner on the login page', async () => { + await PageObjects.security.forceLogout(); + await PageObjects.common.navigateToApp('login'); + + expect(await PageObjects.banners.isTopBannerVisible()).to.eql(true); + expect(await PageObjects.banners.getTopBannerText()).to.eql('global banner text'); + }); + }); +} diff --git a/x-pack/test/functional/es_archives/banners/multispace/data.json b/x-pack/test/functional/es_archives/banners/multispace/data.json new file mode 100644 index 0000000000000..fc0e0dc7b7eee --- /dev/null +++ b/x-pack/test/functional/es_archives/banners/multispace/data.json @@ -0,0 +1,62 @@ +{ + "type": "doc", + "value": { + "id": "config:6.0.0", + "index": ".kibana", + "source": { + "config": { + "buildNum": 8467, + "dateFormat:tz": "UTC", + "defaultRoute": "http://example.com/evil" + }, + "type": "config" + } + } +} + +{ + "type": "doc", + "value": { + "id": "another-space:config:6.0.0", + "index": ".kibana", + "source": { + "namespace": "another-space", + "config": { + "buildNum": 8467, + "dateFormat:tz": "UTC", + "defaultRoute": "/app/canvas" + }, + "type": "config" + } + } +} + +{ + "type": "doc", + "value": { + "id": "space:default", + "index": ".kibana", + "source": { + "space": { + "description": "This is the default space!", + "name": "Default" + }, + "type": "space" + } + } +} + +{ + "type": "doc", + "value": { + "id": "space:another-space", + "index": ".kibana", + "source": { + "space": { + "description": "This is another space", + "name": "Another Space" + }, + "type": "space" + } + } +} diff --git a/x-pack/test/functional/es_archives/banners/multispace/mappings.json b/x-pack/test/functional/es_archives/banners/multispace/mappings.json new file mode 100644 index 0000000000000..f3793c7ca6780 --- /dev/null +++ b/x-pack/test/functional/es_archives/banners/multispace/mappings.json @@ -0,0 +1,287 @@ +{ + "type": "index", + "value": { + "index": ".kibana", + "mappings": { + "properties": { + "config": { + "dynamic": "true", + "properties": { + "buildNum": { + "type": "keyword" + }, + "dateFormat:tz": { + "fields": { + "keyword": { + "ignore_above": 256, + "type": "keyword" + } + }, + "type": "text" + }, + "defaultRoute": { + "type": "keyword" + } + } + }, + "dashboard": { + "dynamic": "strict", + "properties": { + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "optionsJSON": { + "type": "text" + }, + "panelsJSON": { + "type": "text" + }, + "refreshInterval": { + "properties": { + "display": { + "type": "keyword" + }, + "pause": { + "type": "boolean" + }, + "section": { + "type": "integer" + }, + "value": { + "type": "integer" + } + } + }, + "timeFrom": { + "type": "keyword" + }, + "timeRestore": { + "type": "boolean" + }, + "timeTo": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "uiStateJSON": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "index-pattern": { + "dynamic": "strict", + "properties": { + "fieldFormatMap": { + "type": "text" + }, + "fields": { + "type": "text" + }, + "intervalName": { + "type": "keyword" + }, + "notExpandable": { + "type": "boolean" + }, + "sourceFilters": { + "type": "text" + }, + "timeFieldName": { + "type": "keyword" + }, + "title": { + "type": "text" + } + } + }, + "search": { + "dynamic": "strict", + "properties": { + "columns": { + "type": "keyword" + }, + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "sort": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "server": { + "dynamic": "strict", + "properties": { + "uuid": { + "type": "keyword" + } + } + }, + "space": { + "properties": { + "_reserved": { + "type": "boolean" + }, + "color": { + "type": "keyword" + }, + "description": { + "type": "text" + }, + "disabledFeatures": { + "type": "keyword" + }, + "initials": { + "type": "keyword" + }, + "name": { + "fields": { + "keyword": { + "ignore_above": 2048, + "type": "keyword" + } + }, + "type": "text" + } + } + }, + "spaceId": { + "type": "keyword" + }, + "timelion-sheet": { + "dynamic": "strict", + "properties": { + "description": { + "type": "text" + }, + "hits": { + "type": "integer" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "timelion_chart_height": { + "type": "integer" + }, + "timelion_columns": { + "type": "integer" + }, + "timelion_interval": { + "type": "keyword" + }, + "timelion_other_interval": { + "type": "keyword" + }, + "timelion_rows": { + "type": "integer" + }, + "timelion_sheet": { + "type": "text" + }, + "title": { + "type": "text" + }, + "version": { + "type": "integer" + } + } + }, + "type": { + "type": "keyword" + }, + "url": { + "dynamic": "strict", + "properties": { + "accessCount": { + "type": "long" + }, + "accessDate": { + "type": "date" + }, + "createDate": { + "type": "date" + }, + "url": { + "fields": { + "keyword": { + "ignore_above": 2048, + "type": "keyword" + } + }, + "type": "text" + } + } + }, + "visualization": { + "dynamic": "strict", + "properties": { + "description": { + "type": "text" + }, + "kibanaSavedObjectMeta": { + "properties": { + "searchSourceJSON": { + "type": "text" + } + } + }, + "savedSearchId": { + "type": "keyword" + }, + "title": { + "type": "text" + }, + "uiStateJSON": { + "type": "text" + }, + "version": { + "type": "integer" + }, + "visState": { + "type": "text" + } + } + } + } + }, + "settings": { + "index": { + "number_of_replicas": "1", + "number_of_shards": "1" + } + } + } +} diff --git a/x-pack/test/functional/page_objects/banners_page.ts b/x-pack/test/functional/page_objects/banners_page.ts new file mode 100644 index 0000000000000..d2e4e43cec117 --- /dev/null +++ b/x-pack/test/functional/page_objects/banners_page.ts @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrProviderContext } from '../ftr_provider_context'; + +export function BannersPageProvider({ getService }: FtrProviderContext) { + const find = getService('find'); + + class BannersPage { + isTopBannerVisible() { + return find.existsByCssSelector('.header__topBanner .kbnUserBanner__container'); + } + + async getTopBannerText() { + if (!(await this.isTopBannerVisible())) { + return ''; + } + const bannerContainer = await find.byCssSelector( + '.header__topBanner .kbnUserBanner__container' + ); + return bannerContainer.getVisibleText(); + } + } + + return new BannersPage(); +} diff --git a/x-pack/test/functional/page_objects/index.ts b/x-pack/test/functional/page_objects/index.ts index 804f49e5ea075..d9aedd782dbca 100644 --- a/x-pack/test/functional/page_objects/index.ts +++ b/x-pack/test/functional/page_objects/index.ts @@ -40,6 +40,7 @@ import { IngestPipelinesPageProvider } from './ingest_pipelines_page'; import { TagManagementPageProvider } from './tag_management_page'; import { NavigationalSearchProvider } from './navigational_search'; import { SearchSessionsPageProvider } from './search_sessions_management_page'; +import { BannersPageProvider } from './banners_page'; // just like services, PageObjects are defined as a map of // names to Providers. Merge in Kibana's or pick specific ones @@ -77,4 +78,5 @@ export const pageObjects = { roleMappings: RoleMappingsPageProvider, ingestPipelines: IngestPipelinesPageProvider, navigationalSearch: NavigationalSearchProvider, + banners: BannersPageProvider, }; diff --git a/x-pack/test/plugin_functional/test_suites/global_search/global_search_bar.ts b/x-pack/test/plugin_functional/test_suites/global_search/global_search_bar.ts index 077044d29f7d9..a44ded43a0bfe 100644 --- a/x-pack/test/plugin_functional/test_suites/global_search/global_search_bar.ts +++ b/x-pack/test/plugin_functional/test_suites/global_search/global_search_bar.ts @@ -9,7 +9,7 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ getPageObjects, getService }: FtrProviderContext) { - describe('TOTO GlobalSearchBar', function () { + describe('GlobalSearchBar', function () { const { common, navigationalSearch } = getPageObjects(['common', 'navigationalSearch']); const esArchiver = getService('esArchiver'); const browser = getService('browser'); From 42ad72e9aaff03e5f324476b24ded119930eb6bd Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 16 Mar 2021 14:03:48 +0100 Subject: [PATCH 07/20] use keyword for sensitive setting --- .../server/collectors/management/schema.ts | 2 +- src/plugins/telemetry/schema/oss_plugins.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts b/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts index 59516d1f11dda..d042996ff6e45 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts @@ -44,7 +44,7 @@ export const stackManagementSchema: MakeSchemaFrom = { _meta: { description: 'Default value of the setting was changed.' }, }, 'banner:textContent': { - type: 'text', + type: 'keyword', _meta: { description: 'Default value of the setting was changed.' }, }, // non-sensitive diff --git a/src/plugins/telemetry/schema/oss_plugins.json b/src/plugins/telemetry/schema/oss_plugins.json index 176e375f36f2b..28257601e0069 100644 --- a/src/plugins/telemetry/schema/oss_plugins.json +++ b/src/plugins/telemetry/schema/oss_plugins.json @@ -7479,7 +7479,7 @@ } }, "banner:textContent": { - "type": "text", + "type": "keyword", "_meta": { "description": "Default value of the setting was changed." } From 574f8e4d70f06755d4c1bf4c74231b42e9429d65 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 16 Mar 2021 15:43:01 +0100 Subject: [PATCH 08/20] update snapshots --- .../field/__snapshots__/field.test.tsx.snap | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/plugins/advanced_settings/public/management_app/components/field/__snapshots__/field.test.tsx.snap b/src/plugins/advanced_settings/public/management_app/components/field/__snapshots__/field.test.tsx.snap index 517a6238c2519..be5163e89367c 100644 --- a/src/plugins/advanced_settings/public/management_app/components/field/__snapshots__/field.test.tsx.snap +++ b/src/plugins/advanced_settings/public/management_app/components/field/__snapshots__/field.test.tsx.snap @@ -1791,6 +1791,7 @@ exports[`Field for json setting should render as read only if saving is disabled maxLines={30} minLines={6} mode="json" + name="advancedSetting-editField-json:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -1897,6 +1898,7 @@ exports[`Field for json setting should render as read only with help text if ove maxLines={30} minLines={6} mode="json" + name="advancedSetting-editField-json:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -1979,6 +1981,7 @@ exports[`Field for json setting should render custom setting icon if it is custo maxLines={30} minLines={6} mode="json" + name="advancedSetting-editField-json:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -2092,6 +2095,7 @@ exports[`Field for json setting should render default value if there is no user maxLines={30} minLines={6} mode="json" + name="advancedSetting-editField-json:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -2181,6 +2185,7 @@ exports[`Field for json setting should render unsaved value if there are unsaved maxLines={30} minLines={6} mode="json" + name="advancedSetting-editField-json:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -2305,6 +2310,7 @@ exports[`Field for json setting should render user value if there is user value maxLines={30} minLines={6} mode="json" + name="advancedSetting-editField-json:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -2376,6 +2382,7 @@ exports[`Field for markdown setting should render as read only if saving is disa maxLines={30} minLines={6} mode="markdown" + name="advancedSetting-editField-markdown:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -2479,6 +2486,7 @@ exports[`Field for markdown setting should render as read only with help text if maxLines={30} minLines={6} mode="markdown" + name="advancedSetting-editField-markdown:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -2561,6 +2569,7 @@ exports[`Field for markdown setting should render custom setting icon if it is c maxLines={30} minLines={6} mode="markdown" + name="advancedSetting-editField-markdown:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -2632,6 +2641,7 @@ exports[`Field for markdown setting should render default value if there is no u maxLines={30} minLines={6} mode="markdown" + name="advancedSetting-editField-markdown:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -2721,6 +2731,7 @@ exports[`Field for markdown setting should render unsaved value if there are uns maxLines={30} minLines={6} mode="markdown" + name="advancedSetting-editField-markdown:test:setting-editor" onChange={[Function]} setOptions={ Object { @@ -2838,6 +2849,7 @@ exports[`Field for markdown setting should render user value if there is user va maxLines={30} minLines={6} mode="markdown" + name="advancedSetting-editField-markdown:test:setting-editor" onChange={[Function]} setOptions={ Object { From 9313585208ed7b8fb45bfd2e844f7a0bededbd8f Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 17 Mar 2021 08:22:38 +0100 Subject: [PATCH 09/20] setting name consistency with configuration properties --- x-pack/plugins/banners/server/routes/info.ts | 8 ++++---- .../plugins/banners/server/ui_settings.test.ts | 16 ++++++++-------- x-pack/plugins/banners/server/ui_settings.ts | 12 ++++++------ x-pack/test/banners_functional/tests/spaces.ts | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/banners/server/routes/info.ts b/x-pack/plugins/banners/server/routes/info.ts index 6c59698860021..ce7c804dce8b3 100644 --- a/x-pack/plugins/banners/server/routes/info.ts +++ b/x-pack/plugins/banners/server/routes/info.ts @@ -42,10 +42,10 @@ const isValidLicense = (license: ILicense): boolean => { const getBannerConfig = async (client: IUiSettingsClient): Promise => { const [placement, textContent, textColor, backgroundColor] = await Promise.all([ - client.get('banner:placement'), - client.get('banner:textContent'), - client.get('banner:textColor'), - client.get('banner:backgroundColor'), + client.get('banners:placement'), + client.get('banners:textContent'), + client.get('banners:textColor'), + client.get('banners:backgroundColor'), ]); return { diff --git a/x-pack/plugins/banners/server/ui_settings.test.ts b/x-pack/plugins/banners/server/ui_settings.test.ts index 9947ff6189e56..6939c84e354d1 100644 --- a/x-pack/plugins/banners/server/ui_settings.test.ts +++ b/x-pack/plugins/banners/server/ui_settings.test.ts @@ -29,10 +29,10 @@ describe('registerSettings', () => { expect(uiSettings.register).toHaveBeenCalledTimes(1); expect(uiSettings.register).toHaveBeenCalledWith({ - 'banner:placement': expect.any(Object), - 'banner:textContent': expect.any(Object), - 'banner:textColor': expect.any(Object), - 'banner:backgroundColor': expect.any(Object), + 'banners:placement': expect.any(Object), + 'banners:textContent': expect.any(Object), + 'banners:textColor': expect.any(Object), + 'banners:backgroundColor': expect.any(Object), }); }); @@ -48,16 +48,16 @@ describe('registerSettings', () => { expect(uiSettings.register).toHaveBeenCalledTimes(1); expect(uiSettings.register).toHaveBeenCalledWith({ - 'banner:placement': expect.objectContaining({ + 'banners:placement': expect.objectContaining({ value: config.placement, }), - 'banner:textContent': expect.objectContaining({ + 'banners:textContent': expect.objectContaining({ value: config.textContent, }), - 'banner:textColor': expect.objectContaining({ + 'banners:textColor': expect.objectContaining({ value: config.textColor, }), - 'banner:backgroundColor': expect.objectContaining({ + 'banners:backgroundColor': expect.objectContaining({ value: config.backgroundColor, }), }); diff --git a/x-pack/plugins/banners/server/ui_settings.ts b/x-pack/plugins/banners/server/ui_settings.ts index d0c9c0bef00eb..41d103dbe63b6 100644 --- a/x-pack/plugins/banners/server/ui_settings.ts +++ b/x-pack/plugins/banners/server/ui_settings.ts @@ -16,7 +16,7 @@ export const registerSettings = ( config: BannerConfiguration ) => { uiSettings.register({ - 'banner:placement': { + 'banners:placement': { name: i18n.translate('xpack.banners.settings.placement.title', { defaultMessage: 'Banner placement', }), @@ -29,7 +29,7 @@ export const registerSettings = ( requiresPageReload: true, schema: schema.oneOf([schema.literal('disabled'), schema.literal('header')]), }, - 'banner:textContent': { + 'banners:textContent': { name: i18n.translate('xpack.banners.settings.textContent.title', { defaultMessage: 'Banner text', }), @@ -41,7 +41,7 @@ export const registerSettings = ( requiresPageReload: true, schema: schema.string(), }, - 'banner:textColor': { + 'banners:textColor': { name: i18n.translate('xpack.banners.settings.textColor.title', { defaultMessage: 'Banner text color', }), @@ -53,12 +53,12 @@ export const registerSettings = ( schema: schema.string({ validate: (color) => { if (!isHexColor(color)) { - return `'banner:textColor' must be an hex color`; + return `'banners:textColor' must be an hex color`; } }, }), }, - 'banner:backgroundColor': { + 'banners:backgroundColor': { name: i18n.translate('xpack.banners.settings.backgroundColor.title', { defaultMessage: 'Banner background color', }), @@ -70,7 +70,7 @@ export const registerSettings = ( schema: schema.string({ validate: (color) => { if (!isHexColor(color)) { - return `'banner:backgroundColor' must be an hex color`; + return `'banners:backgroundColor' must be an hex color`; } }, }), diff --git a/x-pack/test/banners_functional/tests/spaces.ts b/x-pack/test/banners_functional/tests/spaces.ts index ea51461c94846..f8c412c0df0e3 100644 --- a/x-pack/test/banners_functional/tests/spaces.ts +++ b/x-pack/test/banners_functional/tests/spaces.ts @@ -37,7 +37,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await PageObjects.settings.clickKibanaSettings(); await PageObjects.settings.setAdvancedSettingsTextArea( - 'banner:textContent', + 'banners:textContent', 'default space banner text' ); }); From fda3ffc56984cd9549ad9de2d18034e789c8273b Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 17 Mar 2021 09:34:53 +0100 Subject: [PATCH 10/20] fix setting names in telemetry files --- .../server/collectors/management/schema.ts | 8 ++++---- .../server/collectors/management/types.ts | 8 ++++---- src/plugins/telemetry/schema/oss_plugins.json | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts b/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts index d042996ff6e45..ee64c8fcfbbbf 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts @@ -43,7 +43,7 @@ export const stackManagementSchema: MakeSchemaFrom = { type: 'keyword', _meta: { description: 'Default value of the setting was changed.' }, }, - 'banner:textContent': { + 'banners:textContent': { type: 'keyword', _meta: { description: 'Default value of the setting was changed.' }, }, @@ -416,15 +416,15 @@ export const stackManagementSchema: MakeSchemaFrom = { type: 'boolean', _meta: { description: 'Non-default value of setting.' }, }, - 'banner:placement': { + 'banners:placement': { type: 'keyword', _meta: { description: 'Non-default value of setting.' }, }, - 'banner:textColor': { + 'banners:textColor': { type: 'text', _meta: { description: 'Non-default value of setting.' }, }, - 'banner:backgroundColor': { + 'banners:backgroundColor': { type: 'text', _meta: { description: 'Non-default value of setting.' }, }, diff --git a/src/plugins/kibana_usage_collection/server/collectors/management/types.ts b/src/plugins/kibana_usage_collection/server/collectors/management/types.ts index a3c74bf3c3f3e..28ccd5041a41f 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/management/types.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/management/types.ts @@ -18,7 +18,7 @@ export interface UsageStats { 'timelion:graphite.url': string; 'xpackDashboardMode:roles': string; 'securitySolution:ipReputationLinks': string; - 'banner:textContent': string; + 'banners:textContent': string; /** * non-sensitive settings */ @@ -114,7 +114,7 @@ export interface UsageStats { 'csv:quoteValues': boolean; 'dateFormat:dow': string; dateFormat: string; - 'banner:placement': string; - 'banner:textColor': string; - 'banner:backgroundColor': string; + 'banners:placement': string; + 'banners:textColor': string; + 'banners:backgroundColor': string; } diff --git a/src/plugins/telemetry/schema/oss_plugins.json b/src/plugins/telemetry/schema/oss_plugins.json index 28257601e0069..3ff7f7b1152dc 100644 --- a/src/plugins/telemetry/schema/oss_plugins.json +++ b/src/plugins/telemetry/schema/oss_plugins.json @@ -7478,7 +7478,7 @@ "description": "Default value of the setting was changed." } }, - "banner:textContent": { + "banners:textContent": { "type": "keyword", "_meta": { "description": "Default value of the setting was changed." @@ -8039,19 +8039,19 @@ "description": "Non-default value of setting." } }, - "banner:placement": { + "banners:placement": { "type": "keyword", "_meta": { "description": "Non-default value of setting." } }, - "banner:textColor": { + "banners:textColor": { "type": "text", "_meta": { "description": "Non-default value of setting." } }, - "banner:backgroundColor": { + "banners:backgroundColor": { "type": "text", "_meta": { "description": "Non-default value of setting." From 847e0f253b43c778b6d061516dc7aee4c59d7975 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 22 Mar 2021 10:13:18 +0100 Subject: [PATCH 11/20] open banner links in new tab --- x-pack/plugins/banners/public/components/banner.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/banners/public/components/banner.tsx b/x-pack/plugins/banners/public/components/banner.tsx index ea30e46881d0c..ae28986297659 100644 --- a/x-pack/plugins/banners/public/components/banner.tsx +++ b/x-pack/plugins/banners/public/components/banner.tsx @@ -26,7 +26,7 @@ export const Banner: FC = ({ bannerConfig }) => { }} >
- +
); From 085d4014ad81a812c8e6e17a5685f0b3d90b1365 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 25 Mar 2021 14:31:20 +0100 Subject: [PATCH 12/20] add config.disableSpaceBanners property --- docs/settings/banners-settings.asciidoc | 3 +++ x-pack/plugins/banners/server/config.ts | 1 + x-pack/plugins/banners/server/plugin.ts | 7 ++----- x-pack/plugins/banners/server/routes/index.ts | 4 ++-- x-pack/plugins/banners/server/routes/info.ts | 10 ++++++---- x-pack/plugins/banners/server/ui_settings.test.ts | 11 +++++++++-- x-pack/plugins/banners/server/ui_settings.ts | 10 +++++----- 7 files changed, 28 insertions(+), 18 deletions(-) diff --git a/docs/settings/banners-settings.asciidoc b/docs/settings/banners-settings.asciidoc index 2a68cbe82f9f2..837b6352ff0df 100644 --- a/docs/settings/banners-settings.asciidoc +++ b/docs/settings/banners-settings.asciidoc @@ -27,6 +27,9 @@ You can configure the `xpack.banners` settings in your `kibana.yml` file. | `xpack.banners.backgroundColor` | The color of the banner background. Defaults to `#FFF9E8`. +| `xpack.banners.disableSpaceBanners` +| If true, per-space banner overrides will be disabled. Defaults to `false`. + |=== [NOTE] diff --git a/x-pack/plugins/banners/server/config.ts b/x-pack/plugins/banners/server/config.ts index da59940c7a541..3c9a7daa8df8f 100644 --- a/x-pack/plugins/banners/server/config.ts +++ b/x-pack/plugins/banners/server/config.ts @@ -30,6 +30,7 @@ const configSchema = schema.object({ }, defaultValue: '#FFF9E8', }), + disableSpaceBanners: schema.boolean({ defaultValue: false }), }); export type BannersConfigType = TypeOf; diff --git a/x-pack/plugins/banners/server/plugin.ts b/x-pack/plugins/banners/server/plugin.ts index 2aae6f2ead53e..852ba135c478b 100644 --- a/x-pack/plugins/banners/server/plugin.ts +++ b/x-pack/plugins/banners/server/plugin.ts @@ -6,17 +6,16 @@ */ import { CoreSetup, Plugin, PluginInitializerContext } from 'src/core/server'; -import { BannerConfiguration } from '../common'; import { BannersConfigType } from './config'; import { BannersRequestHandlerContext } from './types'; import { registerRoutes } from './routes'; import { registerSettings } from './ui_settings'; export class BannersPlugin implements Plugin<{}, {}, {}, {}> { - private readonly config: BannerConfiguration; + private readonly config: BannersConfigType; constructor(context: PluginInitializerContext) { - this.config = convertConfig(context.config.get()); + this.config = context.config.get(); } setup({ uiSettings, getStartServices, http }: CoreSetup<{}, {}>) { @@ -31,5 +30,3 @@ export class BannersPlugin implements Plugin<{}, {}, {}, {}> { return {}; } } - -const convertConfig = (raw: BannersConfigType): BannerConfiguration => raw; diff --git a/x-pack/plugins/banners/server/routes/index.ts b/x-pack/plugins/banners/server/routes/index.ts index a4eedc3234c86..347236b4df4e2 100644 --- a/x-pack/plugins/banners/server/routes/index.ts +++ b/x-pack/plugins/banners/server/routes/index.ts @@ -5,10 +5,10 @@ * 2.0. */ -import { BannerConfiguration } from '../../common'; +import { BannersConfigType } from '../config'; import { BannersRouter } from '../types'; import { registerInfoRoute } from './info'; -export const registerRoutes = (router: BannersRouter, config: BannerConfiguration) => { +export const registerRoutes = (router: BannersRouter, config: BannersConfigType) => { registerInfoRoute(router, config); }; diff --git a/x-pack/plugins/banners/server/routes/info.ts b/x-pack/plugins/banners/server/routes/info.ts index ce7c804dce8b3..806c4a7ae6b39 100644 --- a/x-pack/plugins/banners/server/routes/info.ts +++ b/x-pack/plugins/banners/server/routes/info.ts @@ -7,10 +7,11 @@ import { IUiSettingsClient } from 'kibana/server'; import { ILicense } from '../../../licensing/server'; +import { BannersConfigType } from '../config'; import { BannerInfoResponse, BannerConfiguration, BannerPlacement } from '../../common'; import { BannersRouter } from '../types'; -export const registerInfoRoute = (router: BannersRouter, config: BannerConfiguration) => { +export const registerInfoRoute = (router: BannersRouter, config: BannersConfigType) => { router.get( { path: '/api/banners/info', @@ -22,9 +23,10 @@ export const registerInfoRoute = (router: BannersRouter, config: BannerConfigura async (ctx, req, res) => { const allowed = isValidLicense(ctx.licensing.license); - const bannerConfig = req.auth.isAuthenticated - ? await getBannerConfig(ctx.core.uiSettings.client) - : config; + const bannerConfig = + req.auth.isAuthenticated && config.disableSpaceBanners === false + ? await getBannerConfig(ctx.core.uiSettings.client) + : config; return res.ok({ body: { diff --git a/x-pack/plugins/banners/server/ui_settings.test.ts b/x-pack/plugins/banners/server/ui_settings.test.ts index 6939c84e354d1..0cf447aed71a8 100644 --- a/x-pack/plugins/banners/server/ui_settings.test.ts +++ b/x-pack/plugins/banners/server/ui_settings.test.ts @@ -6,14 +6,15 @@ */ import { uiSettingsServiceMock } from '../../../../src/core/server/mocks'; -import type { BannerConfiguration } from '../common'; +import { BannersConfigType } from './config'; import { registerSettings } from './ui_settings'; -const createConfig = (parts: Partial = {}): BannerConfiguration => ({ +const createConfig = (parts: Partial = {}): BannersConfigType => ({ placement: 'disabled', backgroundColor: '#0000', textColor: '#FFFFFF', textContent: 'Hello from the banner', + disableSpaceBanners: false, ...parts, }); @@ -36,6 +37,12 @@ describe('registerSettings', () => { }); }); + it('does not register the settings if `config.disableSpaceBanners` is `true`', () => { + registerSettings(uiSettings, createConfig({ disableSpaceBanners: true })); + + expect(uiSettings.register).not.toHaveBeenCalled(); + }); + it('uses the configuration values as defaults', () => { const config = createConfig({ placement: 'header', diff --git a/x-pack/plugins/banners/server/ui_settings.ts b/x-pack/plugins/banners/server/ui_settings.ts index 41d103dbe63b6..bd97437213146 100644 --- a/x-pack/plugins/banners/server/ui_settings.ts +++ b/x-pack/plugins/banners/server/ui_settings.ts @@ -8,13 +8,13 @@ import { schema } from '@kbn/config-schema'; import { i18n } from '@kbn/i18n'; import { UiSettingsServiceSetup } from 'src/core/server'; -import { BannerConfiguration } from '../common'; +import { BannersConfigType } from './config'; import { isHexColor } from './utils'; -export const registerSettings = ( - uiSettings: UiSettingsServiceSetup, - config: BannerConfiguration -) => { +export const registerSettings = (uiSettings: UiSettingsServiceSetup, config: BannersConfigType) => { + if (config.disableSpaceBanners) { + return; + } uiSettings.register({ 'banners:placement': { name: i18n.translate('xpack.banners.settings.placement.title', { From 506a9e344f55dc0966cf149cb1d123fd62380691 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 25 Mar 2021 15:38:57 +0100 Subject: [PATCH 13/20] fix types --- x-pack/plugins/banners/server/plugin.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/banners/server/plugin.test.ts b/x-pack/plugins/banners/server/plugin.test.ts index 3838447a576f3..bf5dabc8fc564 100644 --- a/x-pack/plugins/banners/server/plugin.test.ts +++ b/x-pack/plugins/banners/server/plugin.test.ts @@ -23,6 +23,7 @@ describe('BannersPlugin', () => { textContent: 'foo', backgroundColor: '#000000', textColor: '#FFFFFF', + disableSpaceBanners: false, }; pluginInitContext = coreMock.createPluginInitializerContext(); pluginInitContext.config.get.mockReturnValue(bannerConfig); From 707ea3b269f0d3a61f500b73f3cfd79910abe4db Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 26 Mar 2021 09:24:33 +0100 Subject: [PATCH 14/20] add descriptions to banner settings --- x-pack/plugins/banners/server/ui_settings.ts | 33 ++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/x-pack/plugins/banners/server/ui_settings.ts b/x-pack/plugins/banners/server/ui_settings.ts index bd97437213146..8ca97134b96a4 100644 --- a/x-pack/plugins/banners/server/ui_settings.ts +++ b/x-pack/plugins/banners/server/ui_settings.ts @@ -15,11 +15,26 @@ export const registerSettings = (uiSettings: UiSettingsServiceSetup, config: Ban if (config.disableSpaceBanners) { return; } + + const subscriptionLink = ` + + ${i18n.translate('xpack.banners.settings.subscriptionRequiredLink.text', { + defaultMessage: 'Subscription required.', + })} + + `; + uiSettings.register({ 'banners:placement': { name: i18n.translate('xpack.banners.settings.placement.title', { defaultMessage: 'Banner placement', }), + description: i18n.translate('xpack.banners.settings.placement.description', { + defaultMessage: 'Set the placement of the banner. {subscriptionLink}', + values: { + subscriptionLink, + }, + }), category: ['banner'], order: 1, type: 'select', @@ -33,6 +48,12 @@ export const registerSettings = (uiSettings: UiSettingsServiceSetup, config: Ban name: i18n.translate('xpack.banners.settings.textContent.title', { defaultMessage: 'Banner text', }), + description: i18n.translate('xpack.banners.settings.text.description', { + defaultMessage: 'Add Markdown-formatted text to the banner. {subscriptionLink}', + values: { + subscriptionLink, + }, + }), sensitive: true, category: ['banner'], order: 2, @@ -45,6 +66,12 @@ export const registerSettings = (uiSettings: UiSettingsServiceSetup, config: Ban name: i18n.translate('xpack.banners.settings.textColor.title', { defaultMessage: 'Banner text color', }), + description: i18n.translate('xpack.banners.settings.textColor.description', { + defaultMessage: 'Set the color of the banner text. {subscriptionLink}', + values: { + subscriptionLink, + }, + }), category: ['banner'], order: 3, type: 'color', @@ -62,6 +89,12 @@ export const registerSettings = (uiSettings: UiSettingsServiceSetup, config: Ban name: i18n.translate('xpack.banners.settings.backgroundColor.title', { defaultMessage: 'Banner background color', }), + description: i18n.translate('xpack.banners.settings.backgroundColor.description', { + defaultMessage: 'Set the background color for the banner. {subscriptionLink}', + values: { + subscriptionLink, + }, + }), category: ['banner'], order: 4, type: 'color', From 0570ea1c647d78c7e2019f9613f9534f4525dafb Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 29 Mar 2021 09:39:01 +0200 Subject: [PATCH 15/20] change label and value header->top --- x-pack/plugins/banners/server/config.ts | 14 +++++++++++++- x-pack/plugins/banners/server/ui_settings.ts | 16 ++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/banners/server/config.ts b/x-pack/plugins/banners/server/config.ts index 3c9a7daa8df8f..37bf602daf360 100644 --- a/x-pack/plugins/banners/server/config.ts +++ b/x-pack/plugins/banners/server/config.ts @@ -5,12 +5,13 @@ * 2.0. */ +import { get } from 'lodash'; import { schema, TypeOf } from '@kbn/config-schema'; import { PluginConfigDescriptor } from 'kibana/server'; import { isHexColor } from './utils'; const configSchema = schema.object({ - placement: schema.oneOf([schema.literal('disabled'), schema.literal('header')], { + placement: schema.oneOf([schema.literal('disabled'), schema.literal('top')], { defaultValue: 'disabled', }), textContent: schema.string({ defaultValue: '' }), @@ -38,4 +39,15 @@ export type BannersConfigType = TypeOf; export const config: PluginConfigDescriptor = { schema: configSchema, exposeToBrowser: {}, + deprecations: () => [ + (rootConfig, fromPath, logger) => { + const pluginConfig = get(rootConfig, fromPath); + if (pluginConfig?.placement === 'header') { + logger('The `header` value for xpack.banners.placement has been replaced by `top`'); + pluginConfig.placement = 'top'; + } + + return rootConfig; + }, + ], }; diff --git a/x-pack/plugins/banners/server/ui_settings.ts b/x-pack/plugins/banners/server/ui_settings.ts index 8ca97134b96a4..d35ab76c41a58 100644 --- a/x-pack/plugins/banners/server/ui_settings.ts +++ b/x-pack/plugins/banners/server/ui_settings.ts @@ -30,7 +30,8 @@ export const registerSettings = (uiSettings: UiSettingsServiceSetup, config: Ban defaultMessage: 'Banner placement', }), description: i18n.translate('xpack.banners.settings.placement.description', { - defaultMessage: 'Set the placement of the banner. {subscriptionLink}', + defaultMessage: + 'Display a top banner for this space, above the Elastic header. {subscriptionLink}', values: { subscriptionLink, }, @@ -39,10 +40,17 @@ export const registerSettings = (uiSettings: UiSettingsServiceSetup, config: Ban order: 1, type: 'select', value: config.placement, - options: ['disabled', 'header'], - optionLabels: {}, + options: ['disabled', 'top'], + optionLabels: { + disabled: i18n.translate('xpack.banners.settings.placement.disabled', { + defaultMessage: 'Disabled', + }), + top: i18n.translate('xpack.banners.settings.placement.top', { + defaultMessage: 'Top', + }), + }, requiresPageReload: true, - schema: schema.oneOf([schema.literal('disabled'), schema.literal('header')]), + schema: schema.oneOf([schema.literal('disabled'), schema.literal('top')]), }, 'banners:textContent': { name: i18n.translate('xpack.banners.settings.textContent.title', { From 2ac2beaf96efe8e5cc20f28fe08138940a0d7670 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 29 Mar 2021 11:00:05 +0200 Subject: [PATCH 16/20] finishing header->top replacement --- docs/settings/banners-settings.asciidoc | 2 +- x-pack/plugins/banners/README.md | 4 ++-- x-pack/plugins/banners/common/types.ts | 2 +- x-pack/plugins/banners/public/plugin.test.tsx | 8 ++++---- x-pack/plugins/banners/public/plugin.tsx | 2 +- x-pack/plugins/banners/server/plugin.test.ts | 2 +- x-pack/plugins/banners/server/ui_settings.test.ts | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/settings/banners-settings.asciidoc b/docs/settings/banners-settings.asciidoc index 837b6352ff0df..d61da50018921 100644 --- a/docs/settings/banners-settings.asciidoc +++ b/docs/settings/banners-settings.asciidoc @@ -16,7 +16,7 @@ You can configure the `xpack.banners` settings in your `kibana.yml` file. |=== | `xpack.banners.placement` -| Set to `header` to enable the header banner. Defaults to `disabled`. +| Set to `top` to enable the top banner. Defaults to `disabled`. | `xpack.banners.textContent` | The text to display inside the banner, either plain text or Markdown. diff --git a/x-pack/plugins/banners/README.md b/x-pack/plugins/banners/README.md index 890c194e1bcb0..6ef2d9196f379 100644 --- a/x-pack/plugins/banners/README.md +++ b/x-pack/plugins/banners/README.md @@ -12,7 +12,7 @@ The options are The placement of the banner. The allowed values are: - `disabled` - The banner will be disabled - - `header` - The banner will be displayed in the header + - `top` - The banner will be displayed in the header - `textContent` @@ -31,7 +31,7 @@ The color for the banner's background. Must be a valid hex color `kibana.yml` ```yaml xpack.banners: - placement: 'header' + placement: 'top' textContent: 'Production environment - Proceed with **special levels** of caution' textColor: '#FF0000' backgroundColor: '#CC2211' diff --git a/x-pack/plugins/banners/common/types.ts b/x-pack/plugins/banners/common/types.ts index 0c785f516ddb3..6c16b4e8055bb 100644 --- a/x-pack/plugins/banners/common/types.ts +++ b/x-pack/plugins/banners/common/types.ts @@ -10,7 +10,7 @@ export interface BannerInfoResponse { banner: BannerConfiguration; } -export type BannerPlacement = 'disabled' | 'header'; +export type BannerPlacement = 'disabled' | 'top'; export interface BannerConfiguration { placement: BannerPlacement; diff --git a/x-pack/plugins/banners/public/plugin.test.tsx b/x-pack/plugins/banners/public/plugin.test.tsx index 232bd840ba353..8722d9516b9db 100644 --- a/x-pack/plugins/banners/public/plugin.test.tsx +++ b/x-pack/plugins/banners/public/plugin.test.tsx @@ -51,11 +51,11 @@ describe('BannersPlugin', () => { }); describe('when banner is allowed', () => { - it('registers the header banner if `banner.placement` is `header`', async () => { + it('registers the header banner if `banner.placement` is `top`', async () => { getBannerInfoMock.mockResolvedValue({ allowed: true, banner: createBannerConfig({ - placement: 'header', + placement: 'top', }), }); @@ -82,11 +82,11 @@ describe('BannersPlugin', () => { }); describe('when banner is not allowed', () => { - it('does not register the header banner if `banner.placement` is `header`', async () => { + it('does not register the header banner if `banner.placement` is `top`', async () => { getBannerInfoMock.mockResolvedValue({ allowed: false, banner: createBannerConfig({ - placement: 'header', + placement: 'top', }), }); diff --git a/x-pack/plugins/banners/public/plugin.tsx b/x-pack/plugins/banners/public/plugin.tsx index 57853ece553d8..014d2de58b9ea 100644 --- a/x-pack/plugins/banners/public/plugin.tsx +++ b/x-pack/plugins/banners/public/plugin.tsx @@ -21,7 +21,7 @@ export class BannersPlugin implements Plugin<{}, {}, {}, {}> { start({ chrome, uiSettings, http }: CoreStart) { getBannerInfo(http).then( ({ allowed, banner }) => { - if (allowed && banner.placement === 'header') { + if (allowed && banner.placement === 'top') { chrome.setHeaderBanner({ content: toMountPoint(), }); diff --git a/x-pack/plugins/banners/server/plugin.test.ts b/x-pack/plugins/banners/server/plugin.test.ts index bf5dabc8fc564..b3f8c7be696cd 100644 --- a/x-pack/plugins/banners/server/plugin.test.ts +++ b/x-pack/plugins/banners/server/plugin.test.ts @@ -19,7 +19,7 @@ describe('BannersPlugin', () => { beforeEach(() => { bannerConfig = { - placement: 'header', + placement: 'top', textContent: 'foo', backgroundColor: '#000000', textColor: '#FFFFFF', diff --git a/x-pack/plugins/banners/server/ui_settings.test.ts b/x-pack/plugins/banners/server/ui_settings.test.ts index 0cf447aed71a8..9fae019774336 100644 --- a/x-pack/plugins/banners/server/ui_settings.test.ts +++ b/x-pack/plugins/banners/server/ui_settings.test.ts @@ -45,7 +45,7 @@ describe('registerSettings', () => { it('uses the configuration values as defaults', () => { const config = createConfig({ - placement: 'header', + placement: 'top', backgroundColor: '#FF00CC', textColor: '#AAFFEE', textContent: 'Some text', From f43168b6507f36056b7be015a852b17ef23f7251 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 29 Mar 2021 20:00:34 +0200 Subject: [PATCH 17/20] doc nits --- docs/settings/banners-settings.asciidoc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/settings/banners-settings.asciidoc b/docs/settings/banners-settings.asciidoc index d61da50018921..ce56d4dbe7a4d 100644 --- a/docs/settings/banners-settings.asciidoc +++ b/docs/settings/banners-settings.asciidoc @@ -9,6 +9,11 @@ Banners are disabled by default. You need to manually configure them in order to You can configure the `xpack.banners` settings in your `kibana.yml` file. +[NOTE] +==== +Banners are a https://www.elastic.co/subscriptions[subscription feature]. +==== + [[general-banners-settings-kb]] ==== General banner settings @@ -16,7 +21,7 @@ You can configure the `xpack.banners` settings in your `kibana.yml` file. |=== | `xpack.banners.placement` -| Set to `top` to enable the top banner. Defaults to `disabled`. +| Set to `top` to display a banner above the Elastic header. Defaults to `disabled`. | `xpack.banners.textContent` | The text to display inside the banner, either plain text or Markdown. @@ -31,8 +36,3 @@ You can configure the `xpack.banners` settings in your `kibana.yml` file. | If true, per-space banner overrides will be disabled. Defaults to `false`. |=== - -[NOTE] -==== -The `banners` plugin is a https://www.elastic.co/subscriptions[subscription feature] -==== \ No newline at end of file From 14c33b6398ea918dfb62fdac9e7c185a3349db52 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 29 Mar 2021 20:14:13 +0200 Subject: [PATCH 18/20] add banners section to advanced options doc --- docs/management/advanced-options.asciidoc | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/management/advanced-options.asciidoc b/docs/management/advanced-options.asciidoc index 446b6a2cfd851..5411a5dc63f3b 100644 --- a/docs/management/advanced-options.asciidoc +++ b/docs/management/advanced-options.asciidoc @@ -209,6 +209,27 @@ from *{stack-monitor-app}*. Turns off all unnecessary animations in the {kib} UI. Refresh the page to apply the changes. +[float] +[[kibana-banners-settings]] +==== Banners + +[horizontal] +[[banners-placement]]`banners:placement`:: +Set to `Top` to display a banner above the Elastic header. Defaults to the value of +the `xpack.banners.placement` configuration property. + +[[banners-textcontent]]`banners:textContent`:: +The text to display inside the banner for this space, either plain text or Markdown. +Defaults to the value of the `xpack.banners.textContent` configuration property. + +[[banners-textcolor]]`banners:textColor`:: +The color for the banner text for this space. Defaults to the value of +the `xpack.banners.textColor` configuration property. + +[[banners-backgroundcolor]]`banners:backgroundColor`:: +The color of the banner background for this space. Defaults to the value of +the `xpack.banners.backgroundColor` configuration property. + [float] [[kibana-dashboard-settings]] ==== Dashboard From 0c8b881bd317540750c964c5ae276ebf1004036d Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 30 Mar 2021 09:06:31 +0200 Subject: [PATCH 19/20] feedback on advanced options doc --- docs/management/advanced-options.asciidoc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/management/advanced-options.asciidoc b/docs/management/advanced-options.asciidoc index 5411a5dc63f3b..a9de1888465f7 100644 --- a/docs/management/advanced-options.asciidoc +++ b/docs/management/advanced-options.asciidoc @@ -213,9 +213,14 @@ the changes. [[kibana-banners-settings]] ==== Banners +[NOTE] +==== +Banners are a https://www.elastic.co/subscriptions[subscription feature]. +==== + [horizontal] [[banners-placement]]`banners:placement`:: -Set to `Top` to display a banner above the Elastic header. Defaults to the value of +Set to `Top` to display a banner above the Elastic header for this space. Defaults to the value of the `xpack.banners.placement` configuration property. [[banners-textcontent]]`banners:textContent`:: From f39965ba77ce994d53d3db59c69a0ff5e9e60da6 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 31 Mar 2021 08:15:34 +0200 Subject: [PATCH 20/20] adapt deprecation to new format --- x-pack/plugins/banners/server/config.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/banners/server/config.ts b/x-pack/plugins/banners/server/config.ts index 37bf602daf360..ec1c7006a84ce 100644 --- a/x-pack/plugins/banners/server/config.ts +++ b/x-pack/plugins/banners/server/config.ts @@ -40,10 +40,12 @@ export const config: PluginConfigDescriptor = { schema: configSchema, exposeToBrowser: {}, deprecations: () => [ - (rootConfig, fromPath, logger) => { + (rootConfig, fromPath, addDeprecation) => { const pluginConfig = get(rootConfig, fromPath); if (pluginConfig?.placement === 'header') { - logger('The `header` value for xpack.banners.placement has been replaced by `top`'); + addDeprecation({ + message: 'The `header` value for xpack.banners.placement has been replaced by `top`', + }); pluginConfig.placement = 'top'; }