From 6765def84d25f2017d92a0addb719de208b98ff6 Mon Sep 17 00:00:00 2001 From: Poff Poffenberger Date: Mon, 30 Dec 2019 09:16:36 -0600 Subject: [PATCH 1/8] [Canvas] Migrate usage collector to NP plugin (#53303) * Move canvas usage collector to NP plugin * Removing old usage collector fom legacy Canvas plugin * Adding types placeholder Co-authored-by: Elastic Machine --- x-pack/legacy/plugins/canvas/server/plugin.ts | 2 -- x-pack/plugins/canvas/kibana.json | 4 ++-- .../canvas/server/collectors}/collector.ts | 16 +++++++++------- .../server/collectors}/collector_helpers.ts | 0 .../custom_element_collector.test.ts | 0 .../collectors}/custom_element_collector.ts | 8 ++++++-- .../canvas/server/collectors}/index.ts | 0 .../collectors}/workpad_collector.test.ts | 2 +- .../server/collectors}/workpad_collector.ts | 2 +- x-pack/plugins/canvas/server/plugin.ts | 17 +++++++++++++++-- x-pack/plugins/canvas/types/index.ts | 7 +++++++ 11 files changed, 41 insertions(+), 17 deletions(-) rename x-pack/{legacy/plugins/canvas/server/usage => plugins/canvas/server/collectors}/collector.ts (84%) rename x-pack/{legacy/plugins/canvas/server/usage => plugins/canvas/server/collectors}/collector_helpers.ts (100%) rename x-pack/{legacy/plugins/canvas/server/usage => plugins/canvas/server/collectors}/custom_element_collector.test.ts (100%) rename x-pack/{legacy/plugins/canvas/server/usage => plugins/canvas/server/collectors}/custom_element_collector.ts (95%) rename x-pack/{legacy/plugins/canvas/server/usage => plugins/canvas/server/collectors}/index.ts (100%) rename x-pack/{legacy/plugins/canvas/server/usage => plugins/canvas/server/collectors}/workpad_collector.test.ts (96%) rename x-pack/{legacy/plugins/canvas/server/usage => plugins/canvas/server/collectors}/workpad_collector.ts (98%) create mode 100644 x-pack/plugins/canvas/types/index.ts diff --git a/x-pack/legacy/plugins/canvas/server/plugin.ts b/x-pack/legacy/plugins/canvas/server/plugin.ts index 2dc87e4a61e0..07f4b7d9ac6d 100644 --- a/x-pack/legacy/plugins/canvas/server/plugin.ts +++ b/x-pack/legacy/plugins/canvas/server/plugin.ts @@ -7,7 +7,6 @@ import { CoreSetup, PluginsSetup } from './shim'; import { routes } from './routes'; import { functions } from '../canvas_plugin_src/functions/server'; -import { registerCanvasUsageCollector } from './usage'; import { loadSampleData } from './sample_data'; export class Plugin { @@ -61,7 +60,6 @@ export class Plugin { }, }); - registerCanvasUsageCollector(plugins.usageCollection, core); loadSampleData( plugins.home.sampleData.addSavedObjectsToSampleDataset, plugins.home.sampleData.addAppLinksToSampleDataset diff --git a/x-pack/plugins/canvas/kibana.json b/x-pack/plugins/canvas/kibana.json index 87214f028705..f18e7fe0590b 100644 --- a/x-pack/plugins/canvas/kibana.json +++ b/x-pack/plugins/canvas/kibana.json @@ -5,6 +5,6 @@ "configPath": ["xpack", "canvas"], "server": true, "ui": false, - "requiredPlugins": [] + "requiredPlugins": [], + "optionalPlugins": ["usageCollection"] } - \ No newline at end of file diff --git a/x-pack/legacy/plugins/canvas/server/usage/collector.ts b/x-pack/plugins/canvas/server/collectors/collector.ts similarity index 84% rename from x-pack/legacy/plugins/canvas/server/usage/collector.ts rename to x-pack/plugins/canvas/server/collectors/collector.ts index ae009f926572..8e9e5ede2e7f 100644 --- a/x-pack/legacy/plugins/canvas/server/usage/collector.ts +++ b/x-pack/plugins/canvas/server/collectors/collector.ts @@ -6,12 +6,11 @@ import { CallCluster } from 'src/legacy/core_plugins/elasticsearch'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; -import { CoreSetup } from '../shim'; -// @ts-ignore missing local declaration -import { CANVAS_USAGE_TYPE } from '../../common/lib/constants'; +import { CANVAS_USAGE_TYPE } from '../../../../legacy/plugins/canvas/common/lib/constants'; +import { TelemetryCollector } from '../../types'; + import { workpadCollector } from './workpad_collector'; import { customElementCollector } from './custom_element_collector'; -import { TelemetryCollector } from '../../types'; const collectors: TelemetryCollector[] = [workpadCollector, customElementCollector]; @@ -24,10 +23,13 @@ const collectors: TelemetryCollector[] = [workpadCollector, customElementCollect A usage collector function returns an object derived from current data in the ES Cluster. */ export function registerCanvasUsageCollector( - usageCollection: UsageCollectionSetup, - core: CoreSetup + usageCollection: UsageCollectionSetup | undefined, + kibanaIndex: string ) { - const kibanaIndex = core.getServerConfig().get('kibana.index'); + if (!usageCollection) { + return; + } + const canvasCollector = usageCollection.makeUsageCollector({ type: CANVAS_USAGE_TYPE, isReady: () => true, diff --git a/x-pack/legacy/plugins/canvas/server/usage/collector_helpers.ts b/x-pack/plugins/canvas/server/collectors/collector_helpers.ts similarity index 100% rename from x-pack/legacy/plugins/canvas/server/usage/collector_helpers.ts rename to x-pack/plugins/canvas/server/collectors/collector_helpers.ts diff --git a/x-pack/legacy/plugins/canvas/server/usage/custom_element_collector.test.ts b/x-pack/plugins/canvas/server/collectors/custom_element_collector.test.ts similarity index 100% rename from x-pack/legacy/plugins/canvas/server/usage/custom_element_collector.test.ts rename to x-pack/plugins/canvas/server/collectors/custom_element_collector.test.ts diff --git a/x-pack/legacy/plugins/canvas/server/usage/custom_element_collector.ts b/x-pack/plugins/canvas/server/collectors/custom_element_collector.ts similarity index 95% rename from x-pack/legacy/plugins/canvas/server/usage/custom_element_collector.ts rename to x-pack/plugins/canvas/server/collectors/custom_element_collector.ts index 218ac0fed08c..5f1944bea3ea 100644 --- a/x-pack/legacy/plugins/canvas/server/usage/custom_element_collector.ts +++ b/x-pack/plugins/canvas/server/collectors/custom_element_collector.ts @@ -8,8 +8,12 @@ import { SearchParams } from 'elasticsearch'; import { get } from 'lodash'; import { fromExpression } from '@kbn/interpreter/common'; import { collectFns } from './collector_helpers'; -import { TelemetryCollector } from '../../types'; -import { ExpressionAST, TelemetryCustomElement, TelemetryCustomElementDocument } from '../../types'; +import { + ExpressionAST, + TelemetryCollector, + TelemetryCustomElement, + TelemetryCustomElementDocument, +} from '../../types'; const CUSTOM_ELEMENT_TYPE = 'canvas-element'; interface CustomElementSearch { diff --git a/x-pack/legacy/plugins/canvas/server/usage/index.ts b/x-pack/plugins/canvas/server/collectors/index.ts similarity index 100% rename from x-pack/legacy/plugins/canvas/server/usage/index.ts rename to x-pack/plugins/canvas/server/collectors/index.ts diff --git a/x-pack/legacy/plugins/canvas/server/usage/workpad_collector.test.ts b/x-pack/plugins/canvas/server/collectors/workpad_collector.test.ts similarity index 96% rename from x-pack/legacy/plugins/canvas/server/usage/workpad_collector.test.ts rename to x-pack/plugins/canvas/server/collectors/workpad_collector.test.ts index 420b785771bf..70bc074ff3df 100644 --- a/x-pack/legacy/plugins/canvas/server/usage/workpad_collector.test.ts +++ b/x-pack/plugins/canvas/server/collectors/workpad_collector.test.ts @@ -6,7 +6,7 @@ import clonedeep from 'lodash.clonedeep'; import { summarizeWorkpads } from './workpad_collector'; -import { workpads } from '../../__tests__/fixtures/workpads'; +import { workpads } from '../../../../legacy/plugins/canvas/__tests__/fixtures/workpads'; describe('usage collector handle es response data', () => { it('should summarize workpads, pages, and elements', () => { diff --git a/x-pack/legacy/plugins/canvas/server/usage/workpad_collector.ts b/x-pack/plugins/canvas/server/collectors/workpad_collector.ts similarity index 98% rename from x-pack/legacy/plugins/canvas/server/usage/workpad_collector.ts rename to x-pack/plugins/canvas/server/collectors/workpad_collector.ts index 5e6e2fa6dbd6..6c86b8b2c746 100644 --- a/x-pack/legacy/plugins/canvas/server/usage/workpad_collector.ts +++ b/x-pack/plugins/canvas/server/collectors/workpad_collector.ts @@ -7,7 +7,7 @@ import { SearchParams } from 'elasticsearch'; import { sum as arraySum, min as arrayMin, max as arrayMax, get } from 'lodash'; import { fromExpression } from '@kbn/interpreter/common'; -import { CANVAS_TYPE } from '../../common/lib/constants'; +import { CANVAS_TYPE } from '../../../../legacy/plugins/canvas/common/lib/constants'; import { collectFns } from './collector_helpers'; import { ExpressionAST, TelemetryCollector, CanvasWorkpad } from '../../types'; diff --git a/x-pack/plugins/canvas/server/plugin.ts b/x-pack/plugins/canvas/server/plugin.ts index 76b86c2ac39b..0f27c68903b3 100644 --- a/x-pack/plugins/canvas/server/plugin.ts +++ b/x-pack/plugins/canvas/server/plugin.ts @@ -4,19 +4,32 @@ * you may not use this file except in compliance with the Elastic License. */ +import { first } from 'rxjs/operators'; import { CoreSetup, PluginInitializerContext, Plugin, Logger } from 'src/core/server'; +import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; import { initRoutes } from './routes'; +import { registerCanvasUsageCollector } from './collectors'; + +interface PluginsSetup { + usageCollection?: UsageCollectionSetup; +} export class CanvasPlugin implements Plugin { private readonly logger: Logger; - constructor(initializerContext: PluginInitializerContext) { + constructor(public readonly initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get(); } - public setup(coreSetup: CoreSetup): void { + public async setup(coreSetup: CoreSetup, plugins: PluginsSetup) { const canvasRouter = coreSetup.http.createRouter(); initRoutes({ router: canvasRouter, logger: this.logger }); + + // we need the kibana index provided by global config for the Canvas usage collector + const globalConfig = await this.initializerContext.config.legacy.globalConfig$ + .pipe(first()) + .toPromise(); + registerCanvasUsageCollector(plugins.usageCollection, globalConfig.kibana.index); } public start() {} diff --git a/x-pack/plugins/canvas/types/index.ts b/x-pack/plugins/canvas/types/index.ts new file mode 100644 index 000000000000..014b203754a2 --- /dev/null +++ b/x-pack/plugins/canvas/types/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export * from '../../../legacy/plugins/canvas/types'; From b8046c7964cbc21d23baeb21c690b70a6192100c Mon Sep 17 00:00:00 2001 From: Poff Poffenberger Date: Mon, 30 Dec 2019 09:19:51 -0600 Subject: [PATCH 2/8] [Canvas] Refactor Canvas to no longer use componentWillReceiveProps (#52129) * Removing componentWillReceiveProps from time filter * Changing expression form to componentDidUpdate * Updating expression to be key-driven updates and arg_types to use compomentDidUpdate * temporary * Revert "temporary" This reverts commit 255525d65f4ddd1aef28f1f6a141ecde12f76c51. * typo fix Co-authored-by: Elastic Machine --- .../components/datetime_input/index.ts | 17 +++++++++++------ .../components/time_picker/time_picker.tsx | 7 ++++--- .../public/components/expression/index.js | 8 -------- .../public/components/toolbar/toolbar.tsx | 2 +- .../arg_types/series_style/index.ts | 6 +++--- 5 files changed, 19 insertions(+), 21 deletions(-) diff --git a/x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/time_filter/components/datetime_input/index.ts b/x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/time_filter/components/datetime_input/index.ts index db4fd1aaf046..ff0e52580b42 100644 --- a/x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/time_filter/components/datetime_input/index.ts +++ b/x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/time_filter/components/datetime_input/index.ts @@ -21,15 +21,20 @@ export const DatetimeInput = compose( moment ? moment.format('YYYY-MM-DD HH:mm:ss') : '' ), lifecycle({ - // TODO: Refactor to no longer use componentWillReceiveProps since it is being deprecated - componentWillReceiveProps({ moment, setStrValue, setValid }) { - if (!moment) return; + componentDidUpdate(prevProps) { + const prevMoment = prevProps.moment; - if (this.props.moment && this.props.moment.isSame(moment)) { + // If we don't have a current moment, do nothing + if (!this.props.moment) return; + + // If we previously had a moment and it's the same as the current moment, do nothing + if (prevMoment && prevMoment.isSame(this.props.moment)) { return; } - setStrValue(moment.format('YYYY-MM-DD HH:mm:ss')); - setValid(true); + + // Set the string value of the current moment and mark as valid + this.props.setStrValue(this.props.moment.format('YYYY-MM-DD HH:mm:ss')); + this.props.setValid(true); }, }) )(Component); diff --git a/x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/time_filter/components/time_picker/time_picker.tsx b/x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/time_filter/components/time_picker/time_picker.tsx index fb0baa22c16f..a03e64ce3a02 100644 --- a/x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/time_filter/components/time_picker/time_picker.tsx +++ b/x-pack/legacy/plugins/canvas/canvas_plugin_src/renderers/time_filter/components/time_picker/time_picker.tsx @@ -47,9 +47,10 @@ export class TimePicker extends Component { isDirty: false, }; - // TODO: Refactor to no longer use componentWillReceiveProps since it is being deprecated - UNSAFE_componentWillReceiveProps({ from, to }: Props) { - if (from !== this.props.from || to !== this.props.to) { + componentDidUpdate(prevProps: Props) { + const { to, from } = this.props; + + if (prevProps.from !== from || prevProps.to !== to) { this.setState({ range: { from, to }, isDirty: false, diff --git a/x-pack/legacy/plugins/canvas/public/components/expression/index.js b/x-pack/legacy/plugins/canvas/public/components/expression/index.js index 4bcdb547186d..806ef388bc4f 100644 --- a/x-pack/legacy/plugins/canvas/public/components/expression/index.js +++ b/x-pack/legacy/plugins/canvas/public/components/expression/index.js @@ -55,14 +55,6 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => { }; const expressionLifecycle = lifecycle({ - componentWillReceiveProps({ formState, setFormState, expression }) { - if (this.props.expression !== expression && expression !== formState.expression) { - setFormState({ - expression, - dirty: false, - }); - } - }, componentDidMount() { const { functionDefinitionsPromise, setFunctionDefinitions } = this.props; functionDefinitionsPromise.then(defs => setFunctionDefinitions(defs)); diff --git a/x-pack/legacy/plugins/canvas/public/components/toolbar/toolbar.tsx b/x-pack/legacy/plugins/canvas/public/components/toolbar/toolbar.tsx index 6d0d7be1c63b..da3475eceb18 100644 --- a/x-pack/legacy/plugins/canvas/public/components/toolbar/toolbar.tsx +++ b/x-pack/legacy/plugins/canvas/public/components/toolbar/toolbar.tsx @@ -97,7 +97,7 @@ export const Toolbar = (props: Props) => { const trays = { pageManager: , - expression: !elementIsSelected ? null : , + expression: !elementIsSelected ? null : , }; return ( diff --git a/x-pack/legacy/plugins/canvas/public/expression_types/arg_types/series_style/index.ts b/x-pack/legacy/plugins/canvas/public/expression_types/arg_types/series_style/index.ts index 99730f7b8fe2..c3211c27eef7 100644 --- a/x-pack/legacy/plugins/canvas/public/expression_types/arg_types/series_style/index.ts +++ b/x-pack/legacy/plugins/canvas/public/expression_types/arg_types/series_style/index.ts @@ -37,9 +37,9 @@ const EnhancedExtendedTemplate = compose( this.props.setLabel(formatLabel(label, this.props)); } }, - componentWillReceiveProps(newProps) { - const newLabel = get(newProps.argValue, 'chain.0.arguments.label.0', ''); - if (newLabel && this.props.label !== formatLabel(newLabel, this.props)) { + componentDidUpdate(prevProps) { + const newLabel = get(this.props.argValue, 'chain.0.arguments.label.0', ''); + if (newLabel && prevProps.label !== formatLabel(newLabel, this.props)) { this.props.setLabel(formatLabel(newLabel, this.props)); } }, From 884fba2944c81d9fe51803fef100dcc7a4fb9b49 Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Mon, 30 Dec 2019 14:49:18 -0600 Subject: [PATCH 3/8] Do not remount applications between page navigations (#53851) --- .../integration_tests/router.test.tsx | 84 +++++++++++++++---- .../application/integration_tests/utils.tsx | 56 +++++++------ src/core/public/application/test_types.ts | 14 +++- .../public/application/ui/app_container.tsx | 2 +- src/core/server/rendering/views/styles.tsx | 2 +- 5 files changed, 113 insertions(+), 45 deletions(-) diff --git a/src/core/public/application/integration_tests/router.test.tsx b/src/core/public/application/integration_tests/router.test.tsx index ffc10820a9c3..10544c348afb 100644 --- a/src/core/public/application/integration_tests/router.test.tsx +++ b/src/core/public/application/integration_tests/router.test.tsx @@ -18,7 +18,7 @@ */ import React from 'react'; -import { createMemoryHistory, History } from 'history'; +import { createMemoryHistory, History, createHashHistory } from 'history'; import { AppRouter, AppNotFound } from '../ui'; import { EitherApp, MockedMounterMap, MockedMounterTuple } from '../test_types'; @@ -27,7 +27,15 @@ import { createRenderer, createAppMounter, createLegacyAppMounter } from './util describe('AppContainer', () => { let mounters: MockedMounterMap; let history: History; - let navigate: ReturnType; + let update: ReturnType; + + const navigate = (path: string) => { + history.push(path); + return update(); + }; + + const mockMountersToMounters = () => + new Map([...mounters].map(([appId, { mounter }]) => [appId, mounter])); beforeEach(() => { mounters = new Map([ @@ -38,14 +46,14 @@ describe('AppContainer', () => { createAppMounter('app3', '
App 3
', '/custom/path'), ] as Array>); history = createMemoryHistory(); - navigate = createRenderer(, history.push); + update = createRenderer(); }); it('calls mount handler and returned unmount function when navigating between apps', async () => { const dom1 = await navigate('/app/app1'); const app1 = mounters.get('app1')!; - expect(app1.mount).toHaveBeenCalled(); + expect(app1.mounter.mount).toHaveBeenCalled(); expect(dom1?.html()).toMatchInlineSnapshot(` "
basename: /app/app1 @@ -53,11 +61,11 @@ describe('AppContainer', () => {
" `); - const app1Unmount = await app1.mount.mock.results[0].value; + const app1Unmount = await app1.mounter.mount.mock.results[0].value; const dom2 = await navigate('/app/app2'); expect(app1Unmount).toHaveBeenCalled(); - expect(mounters.get('app2')!.mount).toHaveBeenCalled(); + expect(mounters.get('app2')!.mounter.mount).toHaveBeenCalled(); expect(dom2?.html()).toMatchInlineSnapshot(` "
basename: /app/app2 @@ -70,29 +78,77 @@ describe('AppContainer', () => { mounters.set(...createAppMounter('spaces', '
Custom Space
', '/spaces/fake-login')); mounters.set(...createAppMounter('login', '
Login Page
', '/fake-login')); history = createMemoryHistory(); - navigate = createRenderer(, history.push); + update = createRenderer(); await navigate('/fake-login'); - expect(mounters.get('spaces')!.mount).not.toHaveBeenCalled(); - expect(mounters.get('login')!.mount).toHaveBeenCalled(); + expect(mounters.get('spaces')!.mounter.mount).not.toHaveBeenCalled(); + expect(mounters.get('login')!.mounter.mount).toHaveBeenCalled(); }); it('should not mount when partial route path has higher specificity', async () => { mounters.set(...createAppMounter('login', '
Login Page
', '/fake-login')); mounters.set(...createAppMounter('spaces', '
Custom Space
', '/spaces/fake-login')); history = createMemoryHistory(); - navigate = createRenderer(, history.push); + update = createRenderer(); await navigate('/spaces/fake-login'); - expect(mounters.get('spaces')!.mount).toHaveBeenCalled(); - expect(mounters.get('login')!.mount).not.toHaveBeenCalled(); + expect(mounters.get('spaces')!.mounter.mount).toHaveBeenCalled(); + expect(mounters.get('login')!.mounter.mount).not.toHaveBeenCalled(); + }); + + it('should not remount when changing pages within app', async () => { + const { mounter, unmount } = mounters.get('app1')!; + await navigate('/app/app1/page1'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + + // Navigating to page within app does not trigger re-render + await navigate('/app/app1/page2'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + expect(unmount).not.toHaveBeenCalled(); + }); + + it('should not remount when going back within app', async () => { + const { mounter, unmount } = mounters.get('app1')!; + await navigate('/app/app1/page1'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + + // Hitting back button within app does not trigger re-render + await navigate('/app/app1/page2'); + history.goBack(); + await update(); + expect(mounter.mount).toHaveBeenCalledTimes(1); + expect(unmount).not.toHaveBeenCalled(); + }); + + it('should not remount when when changing pages within app using hash history', async () => { + history = createHashHistory(); + update = createRenderer(); + + const { mounter, unmount } = mounters.get('app1')!; + await navigate('/app/app1/page1'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + + // Changing hash history does not trigger re-render + await navigate('/app/app1/page2'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + expect(unmount).not.toHaveBeenCalled(); + }); + + it('should unmount when changing between apps', async () => { + const { mounter, unmount } = mounters.get('app1')!; + await navigate('/app/app1/page1'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + + // Navigating to other app triggers unmount + await navigate('/app/app2/page1'); + expect(unmount).toHaveBeenCalledTimes(1); }); it('calls legacy mount handler', async () => { await navigate('/app/legacyApp1'); - expect(mounters.get('legacyApp1')!.mount.mock.calls[0]).toMatchInlineSnapshot(` + expect(mounters.get('legacyApp1')!.mounter.mount.mock.calls[0]).toMatchInlineSnapshot(` Array [ Object { "appBasePath": "/app/legacyApp1", @@ -104,7 +160,7 @@ describe('AppContainer', () => { it('handles legacy apps with subapps', async () => { await navigate('/app/baseApp'); - expect(mounters.get('baseApp:legacyApp2')!.mount.mock.calls[0]).toMatchInlineSnapshot(` + expect(mounters.get('baseApp:legacyApp2')!.mounter.mount.mock.calls[0]).toMatchInlineSnapshot(` Array [ Object { "appBasePath": "/app/baseApp", diff --git a/src/core/public/application/integration_tests/utils.tsx b/src/core/public/application/integration_tests/utils.tsx index b8ade4d1d878..6367d1fa1269 100644 --- a/src/core/public/application/integration_tests/utils.tsx +++ b/src/core/public/application/integration_tests/utils.tsx @@ -26,19 +26,13 @@ import { App, LegacyApp, AppMountParameters } from '../types'; import { MockedMounter, MockedMounterTuple } from '../test_types'; type Dom = ReturnType | null; -type Renderer = (item: string) => Dom | Promise; +type Renderer = () => Dom | Promise; -export const createRenderer = ( - element: ReactElement | null, - callback?: (item: string) => void | Promise -): Renderer => { +export const createRenderer = (element: ReactElement | null): Renderer => { const dom: Dom = element && mount({element}); - return item => + return () => new Promise(async resolve => { - if (callback) { - await callback(item); - } if (dom) { dom.update(); } @@ -50,19 +44,26 @@ export const createAppMounter = ( appId: string, html: string, appRoute = `/app/${appId}` -): MockedMounterTuple => [ - appId, - { - appRoute, - appBasePath: appRoute, - mount: jest.fn(async ({ appBasePath: basename, element }: AppMountParameters) => { - Object.assign(element, { - innerHTML: `
\nbasename: ${basename}\nhtml: ${html}\n
`, - }); - return jest.fn(() => Object.assign(element, { innerHTML: '' })); - }), - }, -]; +): MockedMounterTuple => { + const unmount = jest.fn(); + return [ + appId, + { + mounter: { + appRoute, + appBasePath: appRoute, + mount: jest.fn(async ({ appBasePath: basename, element }: AppMountParameters) => { + Object.assign(element, { + innerHTML: `
\nbasename: ${basename}\nhtml: ${html}\n
`, + }); + unmount.mockImplementation(() => Object.assign(element, { innerHTML: '' })); + return unmount; + }), + }, + unmount, + }, + ]; +}; export const createLegacyAppMounter = ( appId: string, @@ -70,9 +71,12 @@ export const createLegacyAppMounter = ( ): MockedMounterTuple => [ appId, { - appRoute: `/app/${appId.split(':')[0]}`, - appBasePath: `/app/${appId.split(':')[0]}`, - unmountBeforeMounting: true, - mount: legacyMount, + mounter: { + appRoute: `/app/${appId.split(':')[0]}`, + appBasePath: `/app/${appId.split(':')[0]}`, + unmountBeforeMounting: true, + mount: legacyMount, + }, + unmount: jest.fn(), }, ]; diff --git a/src/core/public/application/test_types.ts b/src/core/public/application/test_types.ts index f5fb639eaa32..3d992cb950eb 100644 --- a/src/core/public/application/test_types.ts +++ b/src/core/public/application/test_types.ts @@ -17,7 +17,7 @@ * under the License. */ -import { App, LegacyApp, Mounter } from './types'; +import { App, LegacyApp, Mounter, AppUnmount } from './types'; import { ApplicationService } from './application_service'; /** @internal */ @@ -25,11 +25,19 @@ export type ApplicationServiceContract = PublicMethodsOf; /** @internal */ export type EitherApp = App | LegacyApp; /** @internal */ +export type MockedUnmount = jest.Mocked; +/** @internal */ export type MockedMounter = jest.Mocked>>; /** @internal */ -export type MockedMounterTuple = [string, MockedMounter]; +export type MockedMounterTuple = [ + string, + { mounter: MockedMounter; unmount: MockedUnmount } +]; /** @internal */ -export type MockedMounterMap = Map>; +export type MockedMounterMap = Map< + string, + { mounter: MockedMounter; unmount: MockedUnmount } +>; /** @internal */ export type MockLifecycle< T extends keyof ApplicationService, diff --git a/src/core/public/application/ui/app_container.tsx b/src/core/public/application/ui/app_container.tsx index 96ee91c7c21f..153582e805fa 100644 --- a/src/core/public/application/ui/app_container.tsx +++ b/src/core/public/application/ui/app_container.tsx @@ -65,7 +65,7 @@ export const AppContainer: FunctionComponent = ({ mounter, appId }: Props mount(); return unmount; - }); + }, [mounter]); return ( diff --git a/src/core/server/rendering/views/styles.tsx b/src/core/server/rendering/views/styles.tsx index 40261321dcff..f41627bcfe07 100644 --- a/src/core/server/rendering/views/styles.tsx +++ b/src/core/server/rendering/views/styles.tsx @@ -78,7 +78,7 @@ export const Styles: FunctionComponent = ({ darkMode }) => { background-repeat: no-repeat; background-size: contain; /* SVG optimized according to http://codepen.io/tigt/post/optimizing-svgs-in-data-uris */ - background-image: url'data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIzMCIgaGVpZ2h0PSIzOSIgdmlld0JveD0iMCAwIDMwIDM5Ij4gIDxnIGZpbGw9Im5vbmUiIGZpbGwtcnVsZT0iZXZlbm9kZCI+ICAgIDxwb2x5Z29uIGZpbGw9IiNGMDRFOTgiIHBvaW50cz0iMCAwIDAgMzQuNTQ3IDI5LjkyMiAuMDIiLz4gICAgPHBhdGggZmlsbD0iIzM0Mzc0MSIgZD0iTTAsMTQuNCBMMCwzNC41NDY4IEwxNC4yODcyLDE4LjA2MTIgQzEwLjA0MTYsMTUuNzM4IDUuMTgwNCwxNC40IDAsMTQuNCIvPiAgICA8cGF0aCBmaWxsPSIjMDBCRkIzIiBkPSJNMTcuMzc0MiwxOS45OTY4IEwyLjcyMSwzNi45MDQ4IEwxLjQzMzQsMzguMzg5MiBMMjkuMjYzOCwzOC4zODkyIEMyNy43NjE0LDMwLjgzODggMjMuNDA0MiwyNC4zMjY0IDE3LjM3NDIsMTkuOTk2OCIvPiAgPC9nPjwvc3ZnPg=='); + background-image: url('data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIzMCIgaGVpZ2h0PSIzOSIgdmlld0JveD0iMCAwIDMwIDM5Ij4gIDxnIGZpbGw9Im5vbmUiIGZpbGwtcnVsZT0iZXZlbm9kZCI+ICAgIDxwb2x5Z29uIGZpbGw9IiNGMDRFOTgiIHBvaW50cz0iMCAwIDAgMzQuNTQ3IDI5LjkyMiAuMDIiLz4gICAgPHBhdGggZmlsbD0iIzM0Mzc0MSIgZD0iTTAsMTQuNCBMMCwzNC41NDY4IEwxNC4yODcyLDE4LjA2MTIgQzEwLjA0MTYsMTUuNzM4IDUuMTgwNCwxNC40IDAsMTQuNCIvPiAgICA8cGF0aCBmaWxsPSIjMDBCRkIzIiBkPSJNMTcuMzc0MiwxOS45OTY4IEwyLjcyMSwzNi45MDQ4IEwxLjQzMzQsMzguMzg5MiBMMjkuMjYzOCwzOC4zODkyIEMyNy43NjE0LDMwLjgzODggMjMuNDA0MiwyNC4zMjY0IDE3LjM3NDIsMTkuOTk2OCIvPiAgPC9nPjwvc3ZnPg=='); } .kibanaWelcomeTitle { From e7ee646124fb1353d1b4ea2ac9a342d3fe5ce4c4 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Tue, 31 Dec 2019 00:48:48 +0300 Subject: [PATCH 4/8] [Vega] Sample [Flights] Airport Connections (Hover Over Airport) visualization not working (#53799) Closes: #53748 Co-authored-by: Elastic Machine --- src/legacy/core_plugins/tile_map/index.ts | 9 ++++++++- src/legacy/core_plugins/vis_type_vega/index.ts | 12 +++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/legacy/core_plugins/tile_map/index.ts b/src/legacy/core_plugins/tile_map/index.ts index 298675e75b0d..27f019318a82 100644 --- a/src/legacy/core_plugins/tile_map/index.ts +++ b/src/legacy/core_plugins/tile_map/index.ts @@ -30,7 +30,14 @@ const tileMapPluginInitializer: LegacyPluginInitializer = ({ Plugin }: LegacyPlu uiExports: { styleSheetPaths: resolve(__dirname, 'public/index.scss'), hacks: [resolve(__dirname, 'public/legacy')], - injectDefaultVars: server => ({}), + injectDefaultVars: server => { + const serverConfig = server.config(); + const mapConfig: Record = serverConfig.get('map'); + + return { + emsTileLayerId: mapConfig.emsTileLayerId, + }; + }, }, config(Joi: any) { return Joi.object({ diff --git a/src/legacy/core_plugins/vis_type_vega/index.ts b/src/legacy/core_plugins/vis_type_vega/index.ts index 153cd6afb3cc..52c253c6ac0b 100644 --- a/src/legacy/core_plugins/vis_type_vega/index.ts +++ b/src/legacy/core_plugins/vis_type_vega/index.ts @@ -33,9 +33,15 @@ const vegaPluginInitializer: LegacyPluginInitializer = ({ Plugin }: LegacyPlugin uiExports: { styleSheetPaths: resolve(__dirname, 'public/index.scss'), hacks: [resolve(__dirname, 'public/legacy')], - injectDefaultVars: server => ({ - enableExternalUrls: server.config().get('vega.enableExternalUrls'), - }), + injectDefaultVars: server => { + const serverConfig = server.config(); + const mapConfig: Record = serverConfig.get('map'); + + return { + emsTileLayerId: mapConfig.emsTileLayerId, + enableExternalUrls: serverConfig.get('vega.enableExternalUrls'), + }; + }, }, init: (server: Legacy.Server) => ({}), config(Joi: any) { From 9317f16c38c9fcf0aab7176396bf5af8ccf28643 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 30 Dec 2019 22:14:58 -0500 Subject: [PATCH 5/8] Skip failing test suite --- test/functional/apps/home/_newsfeed.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/apps/home/_newsfeed.ts b/test/functional/apps/home/_newsfeed.ts index 35d7ac8adefa..0019b817b72d 100644 --- a/test/functional/apps/home/_newsfeed.ts +++ b/test/functional/apps/home/_newsfeed.ts @@ -24,7 +24,8 @@ export default function({ getService, getPageObjects }: FtrProviderContext) { const globalNav = getService('globalNav'); const PageObjects = getPageObjects(['common', 'newsfeed']); - describe('Newsfeed', () => { + // Failing: https://github.com/elastic/kibana/issues/53860 + describe.skip('Newsfeed', () => { before(async () => { await PageObjects.newsfeed.resetPage(); }); From 98ac7a64ad76fa664a04a1d156d85a471bdaf5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20C=C3=B4t=C3=A9?= Date: Tue, 31 Dec 2019 13:36:39 -0500 Subject: [PATCH 6/8] Add tests to ensure AAD isn't broken after performing a change on an alert / action (#53333) --- .../legacy/plugins/alerting/server/plugin.ts | 2 +- .../alerting_api_integration/common/config.ts | 1 + .../common/fixtures/plugins/aad/index.ts | 58 +++++++++++++++++++ .../common/fixtures/plugins/aad/package.json | 7 +++ .../common/lib/check_aad.ts | 20 +++++++ .../common/lib/index.ts | 1 + .../tests/actions/create.ts | 11 +++- .../tests/actions/update.ts | 9 ++- .../tests/alerting/create.ts | 9 ++- .../tests/alerting/disable.ts | 15 ++++- .../tests/alerting/enable.ts | 15 ++++- .../tests/alerting/mute_all.ts | 15 ++++- .../tests/alerting/mute_instance.ts | 15 ++++- .../tests/alerting/unmute_all.ts | 15 ++++- .../tests/alerting/unmute_instance.ts | 15 ++++- .../tests/alerting/update.ts | 9 ++- .../tests/alerting/update_api_key.ts | 15 ++++- .../spaces_only/tests/actions/create.ts | 12 +++- .../spaces_only/tests/actions/update.ts | 10 +++- .../spaces_only/tests/alerting/create.ts | 9 ++- .../spaces_only/tests/alerting/disable.ts | 16 ++++- .../spaces_only/tests/alerting/enable.ts | 16 ++++- .../spaces_only/tests/alerting/mute_all.ts | 16 ++++- .../tests/alerting/mute_instance.ts | 16 ++++- .../spaces_only/tests/alerting/unmute_all.ts | 16 ++++- .../tests/alerting/unmute_instance.ts | 16 ++++- .../spaces_only/tests/alerting/update.ts | 10 +++- .../tests/alerting/update_api_key.ts | 16 ++++- 28 files changed, 360 insertions(+), 25 deletions(-) create mode 100644 x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/index.ts create mode 100644 x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/package.json create mode 100644 x-pack/test/alerting_api_integration/common/lib/check_aad.ts diff --git a/x-pack/legacy/plugins/alerting/server/plugin.ts b/x-pack/legacy/plugins/alerting/server/plugin.ts index 3b17fa066d55..935431c56ff3 100644 --- a/x-pack/legacy/plugins/alerting/server/plugin.ts +++ b/x-pack/legacy/plugins/alerting/server/plugin.ts @@ -71,7 +71,7 @@ export class Plugin { attributesToEncrypt: new Set(['apiKey']), attributesToExcludeFromAAD: new Set([ 'scheduledTaskId', - 'muted', + 'muteAll', 'mutedInstanceIds', 'updatedBy', ]), diff --git a/x-pack/test/alerting_api_integration/common/config.ts b/x-pack/test/alerting_api_integration/common/config.ts index 61e7ab3fe463..8356954073ec 100644 --- a/x-pack/test/alerting_api_integration/common/config.ts +++ b/x-pack/test/alerting_api_integration/common/config.ts @@ -78,6 +78,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions) `--plugin-path=${path.join(__dirname, 'fixtures', 'plugins', 'alerts')}`, `--plugin-path=${path.join(__dirname, 'fixtures', 'plugins', 'actions')}`, `--plugin-path=${path.join(__dirname, 'fixtures', 'plugins', 'task_manager')}`, + `--plugin-path=${path.join(__dirname, 'fixtures', 'plugins', 'aad')}`, `--server.xsrf.whitelist=${JSON.stringify(getAllExternalServiceSimulatorPaths())}`, ...(ssl ? [ diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/index.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/index.ts new file mode 100644 index 000000000000..d7bee93f5c94 --- /dev/null +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/index.ts @@ -0,0 +1,58 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import Joi from 'joi'; +import Hapi from 'hapi'; +import { Legacy } from 'kibana'; +import KbnServer from '../../../../../../../src/legacy/server/kbn_server'; +import { PluginStartContract } from '../../../../../../plugins/encrypted_saved_objects/server'; + +interface CheckAADRequest extends Hapi.Request { + payload: { + spaceId?: string; + type: string; + id: string; + }; +} + +// eslint-disable-next-line import/no-default-export +export default function(kibana: any) { + return new kibana.Plugin({ + require: ['actions', 'alerting', 'encryptedSavedObjects'], + name: 'aad-fixtures', + init(server: Legacy.Server) { + const newPlatform = ((server as unknown) as KbnServer).newPlatform; + const esoPlugin = newPlatform.start.plugins.encryptedSavedObjects as PluginStartContract; + + server.route({ + method: 'POST', + path: '/api/check_aad', + options: { + validate: { + payload: Joi.object() + .keys({ + spaceId: Joi.string().optional(), + type: Joi.string().required(), + id: Joi.string().required(), + }) + .required(), + }, + }, + async handler(request: CheckAADRequest) { + let namespace: string | undefined; + const spacesPlugin = server.plugins.spaces; + if (spacesPlugin && request.payload.spaceId) { + namespace = spacesPlugin.spaceIdToNamespace(request.payload.spaceId); + } + await esoPlugin.getDecryptedAsInternalUser(request.payload.type, request.payload.id, { + namespace, + }); + return { success: true }; + }, + }); + }, + }); +} diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/package.json b/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/package.json new file mode 100644 index 000000000000..f5d174c18a20 --- /dev/null +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/package.json @@ -0,0 +1,7 @@ +{ + "name": "aad-fixtures", + "version": "0.0.0", + "kibana": { + "version": "kibana" + } +} diff --git a/x-pack/test/alerting_api_integration/common/lib/check_aad.ts b/x-pack/test/alerting_api_integration/common/lib/check_aad.ts new file mode 100644 index 000000000000..0a75325aaed5 --- /dev/null +++ b/x-pack/test/alerting_api_integration/common/lib/check_aad.ts @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +interface Opts { + supertest: any; + spaceId?: string; + type: string; + id: string; +} + +export async function checkAAD({ supertest, spaceId, type, id }: Opts) { + await supertest + .post('/api/check_aad') + .set('kbn-xsrf', 'foo') + .send({ spaceId, type, id }) + .expect(200, { success: true }); +} diff --git a/x-pack/test/alerting_api_integration/common/lib/index.ts b/x-pack/test/alerting_api_integration/common/lib/index.ts index 31d257e3b25a..a2f21264634f 100644 --- a/x-pack/test/alerting_api_integration/common/lib/index.ts +++ b/x-pack/test/alerting_api_integration/common/lib/index.ts @@ -10,3 +10,4 @@ export { ES_TEST_INDEX_NAME, ESTestIndexTool } from './es_test_index_tool'; export { getTestAlertData } from './get_test_alert_data'; export { AlertUtils } from './alert_utils'; export { TaskManagerUtils } from './task_manager_utils'; +export { checkAAD } from './check_aad'; diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/create.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/create.ts index 57614aa816ff..32d419bb562b 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/create.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/create.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -52,6 +52,7 @@ export default function createActionTests({ getService }: FtrProviderContext) { case 'superuser at space1': case 'space_1_all at space1': expect(response.statusCode).to.eql(200); + objectRemover.add(space.id, response.body.id, 'action'); expect(response.body).to.eql({ id: response.body.id, name: 'My action', @@ -61,7 +62,13 @@ export default function createActionTests({ getService }: FtrProviderContext) { }, }); expect(typeof response.body.id).to.be('string'); - objectRemover.add(space.id, response.body.id, 'action'); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'action', + id: response.body.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/update.ts index d41a2a10b770..ed6a1bb3d14a 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/update.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -75,6 +75,13 @@ export default function updateActionTests({ getService }: FtrProviderContext) { unencrypted: `This value shouldn't get encrypted`, }, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'action', + id: createdAction.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts index a098a1fe02c1..772f85c4dac8 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { getTestAlertData, getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getTestAlertData, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -106,6 +106,13 @@ export default function createAlertTests({ getService }: FtrProviderContext) { alertId: response.body.id, spaceId: space.id, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: response.body.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/disable.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/disable.ts index d2076e0f92b3..bafca30abf28 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/disable.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/disable.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createDisableAlertTests({ getService }: FtrProviderContext) { @@ -65,6 +71,13 @@ export default function createDisableAlertTests({ getService }: FtrProviderConte } catch (e) { expect(e.status).to.eql(404); } + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/enable.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/enable.ts index 528db61dba21..9df1f955232b 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/enable.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/enable.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createEnableAlertTests({ getService }: FtrProviderContext) { @@ -70,6 +76,13 @@ export default function createEnableAlertTests({ getService }: FtrProviderContex alertId: createdAlert.id, spaceId: space.id, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_all.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_all.ts index 9e479064e66c..ce11fb8052b4 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_all.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_all.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteAlertTests({ getService }: FtrProviderContext) { @@ -55,6 +61,13 @@ export default function createMuteAlertTests({ getService }: FtrProviderContext) .auth(user.username, user.password) .expect(200); expect(updatedAlert.muteAll).to.eql(true); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_instance.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_instance.ts index 302b61074e87..f91b54514ae0 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_instance.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_instance.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteAlertInstanceTests({ getService }: FtrProviderContext) { @@ -55,6 +61,13 @@ export default function createMuteAlertInstanceTests({ getService }: FtrProvider .auth(user.username, user.password) .expect(200); expect(updatedAlert.mutedInstanceIds).to.eql(['1']); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_all.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_all.ts index 5e9ad66cf40f..f2598ff7c549 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_all.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_all.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createUnmuteAlertTests({ getService }: FtrProviderContext) { @@ -60,6 +66,13 @@ export default function createUnmuteAlertTests({ getService }: FtrProviderContex .auth(user.username, user.password) .expect(200); expect(updatedAlert.muteAll).to.eql(false); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_instance.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_instance.ts index b466575841d0..ca58b58e5e82 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_instance.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_instance.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteAlertInstanceTests({ getService }: FtrProviderContext) { @@ -60,6 +66,13 @@ export default function createMuteAlertInstanceTests({ getService }: FtrProvider .auth(user.username, user.password) .expect(200); expect(updatedAlert.mutedInstanceIds).to.eql([]); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts index 8cb01b546738..ec162a75ee0a 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts @@ -7,7 +7,7 @@ import expect from '@kbn/expect'; import { Response as SupertestResponse } from 'supertest'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -82,6 +82,13 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], scheduledTaskId: createdAlert.scheduledTaskId, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update_api_key.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update_api_key.ts index 4963d749c293..b54147348d9a 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update_api_key.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update_api_key.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createUpdateApiKeyTests({ getService }: FtrProviderContext) { @@ -55,6 +61,13 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .auth(user.username, user.password) .expect(200); expect(updatedAlert.apiKeyOwner).to.eql(user.username); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/create.ts index 982688cc45c0..74e8e0f83229 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/create.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -34,6 +34,7 @@ export default function createActionTests({ getService }: FtrProviderContext) { }); expect(response.statusCode).to.eql(200); + objectRemover.add(Spaces.space1.id, response.body.id, 'action'); expect(response.body).to.eql({ id: response.body.id, name: 'My action', @@ -43,7 +44,14 @@ export default function createActionTests({ getService }: FtrProviderContext) { }, }); expect(typeof response.body.id).to.be('string'); - objectRemover.add(Spaces.space1.id, response.body.id, 'action'); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'action', + id: response.body.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/update.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/update.ts index bb1ee31bddfe..fb0c5e13c072 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/update.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/update.ts @@ -5,7 +5,7 @@ */ import { Spaces } from '../../scenarios'; -import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -54,6 +54,14 @@ export default function updateActionTests({ getService }: FtrProviderContext) { unencrypted: `This value shouldn't get encrypted`, }, }); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'action', + id: createdAction.id, + }); }); it(`shouldn't update action from another space`, async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts index c61c94bd603f..d64065b59649 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -88,6 +88,13 @@ export default function createAlertTests({ getService }: FtrProviderContext) { alertId: response.body.id, spaceId: Spaces.space1.id, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'alert', + id: response.body.id, + }); }); it('should handle create alert request appropriately when an alert is disabled ', async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/disable.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/disable.ts index 750f94201216..9d5017530e07 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/disable.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/disable.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createDisableAlertTests({ getService }: FtrProviderContext) { @@ -43,6 +49,14 @@ export default function createDisableAlertTests({ getService }: FtrProviderConte } catch (e) { expect(e.status).to.eql(404); } + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); it(`shouldn't disable alert from another space`, async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/enable.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/enable.ts index 00cd40c0e80c..4459dd744fe4 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/enable.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/enable.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createEnableAlertTests({ getService }: FtrProviderContext) { @@ -49,6 +55,14 @@ export default function createEnableAlertTests({ getService }: FtrProviderContex alertId: createdAlert.id, spaceId: Spaces.space1.id, }); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); it(`shouldn't enable alert from another space`, async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_all.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_all.ts index dba3046aa4b7..4781aed640f5 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_all.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_all.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteTests({ getService }: FtrProviderContext) { @@ -34,6 +40,14 @@ export default function createMuteTests({ getService }: FtrProviderContext) { .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.muteAll).to.eql(true); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_instance.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_instance.ts index 09ca35971602..0ee4d076d7b3 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_instance.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_instance.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteInstanceTests({ getService }: FtrProviderContext) { @@ -34,6 +40,14 @@ export default function createMuteInstanceTests({ getService }: FtrProviderConte .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.mutedInstanceIds).to.eql(['1']); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_all.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_all.ts index 70ee32a9e6fb..a7bf065d5795 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_all.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_all.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createUnmuteTests({ getService }: FtrProviderContext) { @@ -35,6 +41,14 @@ export default function createUnmuteTests({ getService }: FtrProviderContext) { .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.muteAll).to.eql(false); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_instance.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_instance.ts index d0f1b2fdb3f9..868bbf66b1c3 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_instance.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_instance.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createUnmuteInstanceTests({ getService }: FtrProviderContext) { @@ -35,6 +41,14 @@ export default function createUnmuteInstanceTests({ getService }: FtrProviderCon .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.mutedInstanceIds).to.eql([]); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts index fd6d81e296ef..ecc614ad3807 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts @@ -5,7 +5,7 @@ */ import { Spaces } from '../../scenarios'; -import { getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -53,6 +53,14 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], scheduledTaskId: createdAlert.scheduledTaskId, }); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); it(`shouldn't update alert from another space`, async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update_api_key.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update_api_key.ts index 2cd363404374..4c4e641f9027 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update_api_key.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update_api_key.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; /** * Eventhough security is disabled, this test checks the API behavior. @@ -38,6 +44,14 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.apiKeyOwner).to.eql(null); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); it(`shouldn't update alert api key from another space`, async () => { From f8f777b5baf1b33ff73b4495ea50c219d678dd86 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Tue, 31 Dec 2019 15:14:55 -0500 Subject: [PATCH 7/8] Add kibanamachine support to Github PR comments (#53852) * Add kibanamachine support to Github PR comments * Temporary commit for quick successful pipeline * Only delete the last comment if it was made by kibanamachine * Revert "Temporary commit for quick successful pipeline" This reverts commit d31f579697682b93dfc1381070c98fa45491560c. --- vars/githubPr.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index 09a166192bf7..ce164ab98ab1 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -35,7 +35,7 @@ def withDefaultPrComments(closure) { def message = getNextCommentMessage(info) postComment(message) - if (lastComment) { + if (lastComment && lastComment.user.login == 'kibanamachine') { deleteComment(lastComment.id) } } @@ -49,7 +49,7 @@ def isPr() { def getLatestBuildComment() { return getComments() .reverse() - .find { it.user.login == 'elasticmachine' && it.body =~ /