From c3041a3a78eb9486d039ca961984de9b9c80412f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 6 Dec 2024 14:44:58 +0100 Subject: [PATCH 1/7] Update space logic to set the isSolutionViewEnabled --- x-pack/plugins/spaces/public/config.ts | 3 - .../management/management_service.test.ts | 3 - .../management/spaces_management_app.test.tsx | 3 - x-pack/plugins/spaces/public/plugin.test.ts | 106 +----------------- x-pack/plugins/spaces/public/plugin.tsx | 16 +-- x-pack/plugins/spaces/server/index.ts | 3 - x-pack/plugins/spaces/server/plugin.ts | 21 +--- 7 files changed, 9 insertions(+), 146 deletions(-) diff --git a/x-pack/plugins/spaces/public/config.ts b/x-pack/plugins/spaces/public/config.ts index 3dd2d3bc89f92..dcd203eb696e3 100644 --- a/x-pack/plugins/spaces/public/config.ts +++ b/x-pack/plugins/spaces/public/config.ts @@ -9,7 +9,4 @@ export interface ConfigType { maxSpaces: number; allowFeatureVisibility: boolean; allowSolutionVisibility: boolean; - experimental: { - forceSolutionVisibility: boolean; - }; } diff --git a/x-pack/plugins/spaces/public/management/management_service.test.ts b/x-pack/plugins/spaces/public/management/management_service.test.ts index c89801953acdd..9aeddb8a66468 100644 --- a/x-pack/plugins/spaces/public/management/management_service.test.ts +++ b/x-pack/plugins/spaces/public/management/management_service.test.ts @@ -27,9 +27,6 @@ describe('ManagementService', () => { maxSpaces: 1000, allowFeatureVisibility: true, allowSolutionVisibility: true, - experimental: { - forceSolutionVisibility: false, - }, }; describe('#setup', () => { diff --git a/x-pack/plugins/spaces/public/management/spaces_management_app.test.tsx b/x-pack/plugins/spaces/public/management/spaces_management_app.test.tsx index d721ff79600c3..ff5a709765901 100644 --- a/x-pack/plugins/spaces/public/management/spaces_management_app.test.tsx +++ b/x-pack/plugins/spaces/public/management/spaces_management_app.test.tsx @@ -45,9 +45,6 @@ const config: ConfigType = { maxSpaces: 1000, allowFeatureVisibility: true, allowSolutionVisibility: true, - experimental: { - forceSolutionVisibility: false, - }, }; const eventTracker = new EventTracker({ reportEvent: jest.fn() }); diff --git a/x-pack/plugins/spaces/public/plugin.test.ts b/x-pack/plugins/spaces/public/plugin.test.ts index 33565748a99e3..9e0ce8530edc3 100644 --- a/x-pack/plugins/spaces/public/plugin.test.ts +++ b/x-pack/plugins/spaces/public/plugin.test.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { cloudMock } from '@kbn/cloud-plugin/public/mocks'; import { coreMock } from '@kbn/core/public/mocks'; import { homePluginMock } from '@kbn/home-plugin/public/mocks'; import { @@ -236,129 +235,34 @@ describe('Spaces plugin', () => { }); describe('isSolutionViewEnabled', () => { - it('when onCloud, not serverless and allowSolutionVisibility is "true"', () => { + it('when allowSolutionVisibility is "true"', () => { const coreSetup = coreMock.createSetup(); const coreStart = coreMock.createStart(); - const cloud = cloudMock.createSetup(); - cloud.isCloudEnabled = true; const plugin = new SpacesPlugin( - coreMock.createPluginInitializerContext( - { allowSolutionVisibility: true }, - { buildFlavor: 'traditional' } - ) + coreMock.createPluginInitializerContext({ allowSolutionVisibility: true }) ); - const spacesSetup = plugin.setup(coreSetup, { cloud }); + const spacesSetup = plugin.setup(coreSetup, {}); const spacesStart = plugin.start(coreStart); expect(spacesSetup.isSolutionViewEnabled).toBe(true); expect(spacesStart.isSolutionViewEnabled).toBe(true); }); - it('when not onCloud and allowSolutionVisibility is "true"', () => { + it('when allowSolutionVisibility is "false"', () => { const coreSetup = coreMock.createSetup(); const coreStart = coreMock.createStart(); { const plugin = new SpacesPlugin( - coreMock.createPluginInitializerContext( - { allowSolutionVisibility: true }, // it is true but we are not onCloud - { buildFlavor: 'traditional' } - ) + coreMock.createPluginInitializerContext({ allowSolutionVisibility: false }) ); const spacesSetup = plugin.setup(coreSetup, {}); const spacesStart = plugin.start(coreStart); - expect(spacesSetup.isSolutionViewEnabled).toBe(false); // so it should be false - expect(spacesStart.isSolutionViewEnabled).toBe(false); - } - - { - // unless the forceSolutionVisibility flag is set - const plugin = new SpacesPlugin( - coreMock.createPluginInitializerContext( - { allowSolutionVisibility: false, experimental: { forceSolutionVisibility: true } }, - { buildFlavor: 'traditional' } - ) - ); - const spacesSetup = plugin.setup(coreSetup, {}); // we are not onCloud but forceSolutionVisibility is true - const spacesStart = plugin.start(coreStart); - - expect(spacesSetup.isSolutionViewEnabled).toBe(true); - expect(spacesStart.isSolutionViewEnabled).toBe(true); - } - }); - - it('when onCloud, not serverless and allowSolutionVisibility is "false"', () => { - const coreSetup = coreMock.createSetup(); - const coreStart = coreMock.createStart(); - const cloud = cloudMock.createSetup(); - cloud.isCloudEnabled = true; - - { - const plugin = new SpacesPlugin( - coreMock.createPluginInitializerContext( - { allowSolutionVisibility: false }, - { buildFlavor: 'traditional' } - ) - ); - const spacesSetup = plugin.setup(coreSetup, { cloud }); - const spacesStart = plugin.start(coreStart); - expect(spacesSetup.isSolutionViewEnabled).toBe(false); expect(spacesStart.isSolutionViewEnabled).toBe(false); } - - { - // unless the forceSolutionVisibility flag is set - const plugin = new SpacesPlugin( - coreMock.createPluginInitializerContext( - { allowSolutionVisibility: false, experimental: { forceSolutionVisibility: true } }, - { buildFlavor: 'traditional' } - ) - ); - const spacesSetup = plugin.setup(coreSetup, { cloud }); - const spacesStart = plugin.start(coreStart); - - expect(spacesSetup.isSolutionViewEnabled).toBe(true); - expect(spacesStart.isSolutionViewEnabled).toBe(true); - } - }); - - it('when onCloud and serverless', () => { - const coreSetup = coreMock.createSetup(); - const coreStart = coreMock.createStart(); - const cloud = cloudMock.createSetup(); - cloud.isCloudEnabled = true; - - { - const plugin = new SpacesPlugin( - coreMock.createPluginInitializerContext( - { allowSolutionVisibility: true }, - { buildFlavor: 'serverless' } - ) - ); - const spacesSetup = plugin.setup(coreSetup, { cloud }); - const spacesStart = plugin.start(coreStart); - - expect(spacesSetup.isSolutionViewEnabled).toBe(false); - expect(spacesStart.isSolutionViewEnabled).toBe(false); - } - - { - // unless the forceSolutionVisibility flag is set - const plugin = new SpacesPlugin( - coreMock.createPluginInitializerContext( - { allowSolutionVisibility: true, experimental: { forceSolutionVisibility: true } }, - { buildFlavor: 'serverless' } - ) - ); - const spacesSetup = plugin.setup(coreSetup, { cloud }); - const spacesStart = plugin.start(coreStart); - - expect(spacesSetup.isSolutionViewEnabled).toBe(true); - expect(spacesStart.isSolutionViewEnabled).toBe(true); - } }); }); }); diff --git a/x-pack/plugins/spaces/public/plugin.tsx b/x-pack/plugins/spaces/public/plugin.tsx index 18083924200d9..ea0bf2576b32a 100644 --- a/x-pack/plugins/spaces/public/plugin.tsx +++ b/x-pack/plugins/spaces/public/plugin.tsx @@ -60,13 +60,6 @@ export class SpacesPlugin implements Plugin, plugins: PluginsSetup) { const hasOnlyDefaultSpace = this.config.maxSpaces === 1; - const onCloud = plugins.cloud !== undefined && plugins.cloud.isCloudEnabled; - - // We only allow "solution" to be set on cloud environments, not on prem - // unless the forceSolutionVisibility flag is set - const allowSolutionVisibility = - (onCloud && !this.isServerless && this.config.allowSolutionVisibility) || - Boolean(this.config.experimental?.forceSolutionVisibility); this.spacesManager = new SpacesManager(core.http); this.spacesApi = { @@ -77,12 +70,7 @@ export class SpacesPlugin implements Plugin this.spacesManager.onActiveSpaceChange$, getActiveSpace: () => this.spacesManager.getActiveSpace(), hasOnlyDefaultSpace, - isSolutionViewEnabled: allowSolutionVisibility, - }; - - this.config = { - ...this.config, - allowSolutionVisibility, + isSolutionViewEnabled: this.config.allowSolutionVisibility, }; registerSpacesEventTypes(core); @@ -167,7 +155,7 @@ export class SpacesPlugin implements Plugin(false); - constructor(private readonly initializerContext: PluginInitializerContext) { - this.config$ = combineLatest([ - initializerContext.config.create(), - this.onCloud$, - ]).pipe( - map( - ([config, onCloud]): ConfigType => ({ - ...config, - // We only allow "solution" to be set on cloud environments, not on prem - // unless the forceSolutionVisibility flag is set. - allowSolutionVisibility: - (onCloud && config.allowSolutionVisibility) || - Boolean(config.experimental?.forceSolutionVisibility), - }) - ) - ); + this.config$ = initializerContext.config.create(); this.hasOnlyDefaultSpace$ = this.config$.pipe(map(({ maxSpaces }) => maxSpaces === 1)); this.log = initializerContext.logger.get(); this.spacesService = new SpacesService(); @@ -148,7 +132,6 @@ export class SpacesPlugin } public setup(core: CoreSetup, plugins: PluginsSetup): SpacesPluginSetup { - this.onCloud$.next(plugins.cloud !== undefined && plugins.cloud.isCloudEnabled); const spacesClientSetup = this.spacesClientService.setup({ config$: this.config$ }); core.uiSettings.registerGlobal(getUiSettings()); From a6fcc731c54ba4b7a943e2c08beaf087f56101a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 6 Dec 2024 15:28:57 +0100 Subject: [PATCH 2/7] Update functional tests --- .../test/functional/apps/management/config.ts | 5 +- .../create_edit_space/create_edit_space.ts | 93 ++++++++++++- x-pack/test/functional/apps/spaces/index.ts | 1 + .../solution_tour.ts | 2 +- .../solution_view_flag_enabled/config.ts | 32 ----- .../create_edit_space.ts | 125 ------------------ .../solution_view_flag_enabled/index.ts | 15 --- x-pack/test/functional_search/config.ts | 7 +- .../functional_solution_sidenav/config.ts | 7 +- 9 files changed, 96 insertions(+), 191 deletions(-) rename x-pack/test/functional/apps/spaces/{solution_view_flag_enabled => }/solution_tour.ts (98%) delete mode 100644 x-pack/test/functional/apps/spaces/solution_view_flag_enabled/config.ts delete mode 100644 x-pack/test/functional/apps/spaces/solution_view_flag_enabled/create_edit_space.ts delete mode 100644 x-pack/test/functional/apps/spaces/solution_view_flag_enabled/index.ts diff --git a/x-pack/test/functional/apps/management/config.ts b/x-pack/test/functional/apps/management/config.ts index e4a06d30f260d..7ca5d972aa064 100644 --- a/x-pack/test/functional/apps/management/config.ts +++ b/x-pack/test/functional/apps/management/config.ts @@ -15,10 +15,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) { testFiles: [require.resolve('.')], kbnTestServer: { ...functionalConfig.get('kbnTestServer'), - serverArgs: [ - ...functionalConfig.get('kbnTestServer.serverArgs'), - '--xpack.spaces.experimental.forceSolutionVisibility=true', - ], + serverArgs: [...functionalConfig.get('kbnTestServer.serverArgs')], }, }; } diff --git a/x-pack/test/functional/apps/spaces/create_edit_space/create_edit_space.ts b/x-pack/test/functional/apps/spaces/create_edit_space/create_edit_space.ts index 4b100595a38c6..a2c00647adf92 100644 --- a/x-pack/test/functional/apps/spaces/create_edit_space/create_edit_space.ts +++ b/x-pack/test/functional/apps/spaces/create_edit_space/create_edit_space.ts @@ -15,6 +15,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const testSubjects = getService('testSubjects'); const spacesServices = getService('spaces'); const log = getService('log'); + const find = getService('find'); describe('Spaces Management: Create and Edit', () => { before(async () => { @@ -43,6 +44,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { it('create a space with a given name', async () => { await PageObjects.spaceSelector.addSpaceName(spaceName); + await PageObjects.spaceSelector.changeSolutionView('classic'); await PageObjects.spaceSelector.clickSaveSpaceCreation(); await testSubjects.existOrFail(`spacesListTableRow-${spaceId}`); }); @@ -60,6 +62,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { name: spaceName, disabledFeatures: [], color: '#AABBCC', + solution: 'classic', }); await PageObjects.common.navigateToApp('spacesManagement'); @@ -103,14 +106,100 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { }); describe('solution view', () => { - it('does not show solution view panel', async () => { + it('does show the solution view panel', async () => { await PageObjects.common.navigateToUrl('management', 'kibana/spaces/edit/default', { shouldUseHashForSubUrl: false, }); await testSubjects.existOrFail('spaces-view-page'); await testSubjects.existOrFail('spaces-view-page > generalPanel'); - await testSubjects.missingOrFail('spaces-view-page > navigationPanel'); // xpack.spaces.allowSolutionVisibility is not enabled, so the solution view picker should not appear + await testSubjects.existOrFail('spaces-view-page > navigationPanel'); + }); + + it('changes the space solution and updates the side navigation', async () => { + await PageObjects.common.navigateToUrl('management', 'kibana/spaces/edit/default', { + shouldUseHashForSubUrl: false, + }); + + // Make sure we are on the classic side nav + await testSubjects.existOrFail('mgtSideBarNav'); + await testSubjects.missingOrFail('searchSideNav'); + + // change to Enterprise Search + await PageObjects.spaceSelector.changeSolutionView('es'); + await PageObjects.spaceSelector.clickSaveSpaceCreation(); + await PageObjects.spaceSelector.confirmModal(); + + await find.waitForDeletedByCssSelector('.kibanaWelcomeLogo'); + + // Search side nav is loaded + await testSubjects.existOrFail('searchSideNav'); + await testSubjects.missingOrFail('mgtSideBarNav'); + + // change back to classic + await PageObjects.common.navigateToUrl('management', 'kibana/spaces/edit/default', { + shouldUseHashForSubUrl: false, + }); + + await testSubjects.missingOrFail('space-edit-page-user-impact-warning'); + await PageObjects.spaceSelector.changeSolutionView('classic'); + await testSubjects.existOrFail('space-edit-page-user-impact-warning'); // Warn that the change will impact other users + + await PageObjects.spaceSelector.clickSaveSpaceCreation(); + await PageObjects.spaceSelector.confirmModal(); + + await testSubjects.existOrFail('mgtSideBarNav'); + await testSubjects.missingOrFail('searchSideNav'); + }); + }); + + describe('API-created Space', () => { + before(async () => { + await spacesServices.create({ + id: 'foo-space', + name: 'Foo Space', + disabledFeatures: [], + color: '#AABBCC', + }); + }); + + after(async () => { + await spacesServices.delete('foo-space'); + }); + + it('enabled features can be changed while the solution view remains unselected', async () => { + const securityFeatureCheckboxId = 'featureCategoryCheckbox_securitySolution'; + + await PageObjects.common.navigateToUrl('management', 'kibana/spaces/edit/foo-space', { + shouldUseHashForSubUrl: false, + }); + + await testSubjects.existOrFail('spaces-view-page'); + + // ensure security feature is selected by default + expect(await testSubjects.isChecked(securityFeatureCheckboxId)).to.be(true); + + // Do not set a solution view first! + + await PageObjects.spaceSelector.toggleFeatureCategoryCheckbox('securitySolution'); + // + // ensure security feature now unselected + expect(await testSubjects.isChecked(securityFeatureCheckboxId)).to.be(false); + + await testSubjects.existOrFail('space-edit-page-user-impact-warning'); + + await PageObjects.spaceSelector.clickSaveSpaceCreation(); + + await testSubjects.click('confirmModalConfirmButton'); + + await testSubjects.existOrFail('spaces-view-page'); + + await testSubjects.click('foo-space-hyperlink'); + + await testSubjects.existOrFail('spaces-view-page'); + + // ensure security feature is still unselected + expect(await testSubjects.isChecked(securityFeatureCheckboxId)).to.be(false); }); }); }); diff --git a/x-pack/test/functional/apps/spaces/index.ts b/x-pack/test/functional/apps/spaces/index.ts index 3fe77a1a4528b..4a9334ab30486 100644 --- a/x-pack/test/functional/apps/spaces/index.ts +++ b/x-pack/test/functional/apps/spaces/index.ts @@ -15,5 +15,6 @@ export default function spacesApp({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./enter_space')); loadTestFile(require.resolve('./create_edit_space')); loadTestFile(require.resolve('./spaces_grid')); + loadTestFile(require.resolve('./solution_tour')); }); } diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts b/x-pack/test/functional/apps/spaces/solution_tour.ts similarity index 98% rename from x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts rename to x-pack/test/functional/apps/spaces/solution_tour.ts index ac8281f45a56e..862fe9146ee69 100644 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts +++ b/x-pack/test/functional/apps/spaces/solution_tour.ts @@ -7,7 +7,7 @@ import expect from '@kbn/expect'; import type { SolutionView, Space } from '@kbn/spaces-plugin/common'; -import { FtrProviderContext } from '../../../ftr_provider_context'; +import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ getPageObjects, getService }: FtrProviderContext) { const kibanaServer = getService('kibanaServer'); diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/config.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/config.ts deleted file mode 100644 index f99c2daf38960..0000000000000 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/config.ts +++ /dev/null @@ -1,32 +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 { FtrConfigProviderContext } from '@kbn/test'; - -/** - * NOTE: The solution view is currently only available in the cloud environment. - * This test suite fakes a cloud environement by setting the cloud.id and cloud.base_url - */ - -export default async function ({ readConfigFile }: FtrConfigProviderContext) { - const functionalConfig = await readConfigFile(require.resolve('../../../config.base.js')); - - return { - ...functionalConfig.getAll(), - testFiles: [require.resolve('.')], - kbnTestServer: { - ...functionalConfig.get('kbnTestServer'), - serverArgs: [ - ...functionalConfig.get('kbnTestServer.serverArgs'), - // Note: the base64 string in the cloud.id config contains the ES endpoint required in the functional tests - '--xpack.cloud.id=ftr_fake_cloud_id:aGVsbG8uY29tOjQ0MyRFUzEyM2FiYyRrYm4xMjNhYmM=', - '--xpack.cloud.base_url=https://cloud.elastic.co', - '--xpack.cloud.deployment_url=/deployments/deploymentId', - ], - }, - }; -} diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/create_edit_space.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/create_edit_space.ts deleted file mode 100644 index 8695077eae74e..0000000000000 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/create_edit_space.ts +++ /dev/null @@ -1,125 +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 expect from '@kbn/expect'; -import { FtrProviderContext } from '../../../ftr_provider_context'; - -export default function ({ getPageObjects, getService }: FtrProviderContext) { - const kibanaServer = getService('kibanaServer'); - const PageObjects = getPageObjects(['common', 'settings', 'security', 'spaceSelector']); - const testSubjects = getService('testSubjects'); - const spacesService = getService('spaces'); - const find = getService('find'); - - describe('edit space', () => { - before(async () => { - await kibanaServer.savedObjects.cleanStandardList(); - }); - - after(async () => { - await kibanaServer.savedObjects.cleanStandardList(); - }); - - describe('solution view', () => { - it('does show the solution view panel', async () => { - await PageObjects.common.navigateToUrl('management', 'kibana/spaces/edit/default', { - shouldUseHashForSubUrl: false, - }); - - await testSubjects.existOrFail('spaces-view-page'); - await testSubjects.existOrFail('spaces-view-page > generalPanel'); - await testSubjects.existOrFail('spaces-view-page > navigationPanel'); - }); - - it('changes the space solution and updates the side navigation', async () => { - await PageObjects.common.navigateToUrl('management', 'kibana/spaces/edit/default', { - shouldUseHashForSubUrl: false, - }); - - // Make sure we are on the classic side nav - await testSubjects.existOrFail('mgtSideBarNav'); - await testSubjects.missingOrFail('searchSideNav'); - - // change to Enterprise Search - await PageObjects.spaceSelector.changeSolutionView('es'); - await PageObjects.spaceSelector.clickSaveSpaceCreation(); - await PageObjects.spaceSelector.confirmModal(); - - await find.waitForDeletedByCssSelector('.kibanaWelcomeLogo'); - - // Search side nav is loaded - await testSubjects.existOrFail('searchSideNav'); - await testSubjects.missingOrFail('mgtSideBarNav'); - - // change back to classic - await PageObjects.common.navigateToUrl('management', 'kibana/spaces/edit/default', { - shouldUseHashForSubUrl: false, - }); - - await testSubjects.missingOrFail('space-edit-page-user-impact-warning'); - await PageObjects.spaceSelector.changeSolutionView('classic'); - await testSubjects.existOrFail('space-edit-page-user-impact-warning'); // Warn that the change will impact other users - - await PageObjects.spaceSelector.clickSaveSpaceCreation(); - await PageObjects.spaceSelector.confirmModal(); - - await testSubjects.existOrFail('mgtSideBarNav'); - await testSubjects.missingOrFail('searchSideNav'); - }); - }); - - describe('API-created Space', () => { - before(async () => { - await spacesService.create({ - id: 'foo-space', - name: 'Foo Space', - disabledFeatures: [], - color: '#AABBCC', - }); - }); - - after(async () => { - await spacesService.delete('foo-space'); - }); - - it('enabled features can be changed while the solution view remains unselected', async () => { - const securityFeatureCheckboxId = 'featureCategoryCheckbox_securitySolution'; - - await PageObjects.common.navigateToUrl('management', 'kibana/spaces/edit/foo-space', { - shouldUseHashForSubUrl: false, - }); - - await testSubjects.existOrFail('spaces-view-page'); - - // ensure security feature is selected by default - expect(await testSubjects.isChecked(securityFeatureCheckboxId)).to.be(true); - - // Do not set a solution view first! - - await PageObjects.spaceSelector.toggleFeatureCategoryCheckbox('securitySolution'); - // - // ensure security feature now unselected - expect(await testSubjects.isChecked(securityFeatureCheckboxId)).to.be(false); - - await testSubjects.existOrFail('space-edit-page-user-impact-warning'); - - await PageObjects.spaceSelector.clickSaveSpaceCreation(); - - await testSubjects.click('confirmModalConfirmButton'); - - await testSubjects.existOrFail('spaces-view-page'); - - await testSubjects.click('foo-space-hyperlink'); - - await testSubjects.existOrFail('spaces-view-page'); - - // ensure security feature is still unselected - expect(await testSubjects.isChecked(securityFeatureCheckboxId)).to.be(false); - }); - }); - }); -} diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/index.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/index.ts deleted file mode 100644 index 45a8f78387154..0000000000000 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/index.ts +++ /dev/null @@ -1,15 +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 { FtrProviderContext } from '../../../ftr_provider_context'; - -export default function spacesApp({ loadTestFile }: FtrProviderContext) { - describe('Spaces app (with solution view)', function spacesAppTestSuite() { - loadTestFile(require.resolve('./create_edit_space')); - loadTestFile(require.resolve('./solution_tour')); - }); -} diff --git a/x-pack/test/functional_search/config.ts b/x-pack/test/functional_search/config.ts index c7708363766b0..808edc73d97a6 100644 --- a/x-pack/test/functional_search/config.ts +++ b/x-pack/test/functional_search/config.ts @@ -30,12 +30,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) { }, kbnTestServer: { ...functionalConfig.get('kbnTestServer'), - serverArgs: [ - ...functionalConfig.get('kbnTestServer.serverArgs'), - // Note: the base64 string in the cloud.id config contains the ES endpoint required in the functional tests - '--xpack.cloud.id=ftr_fake_cloud_id:aGVsbG8uY29tOjQ0MyRFUzEyM2FiYyRrYm4xMjNhYmM=', - '--xpack.cloud.base_url=https://cloud.elastic.co', - ], + serverArgs: [...functionalConfig.get('kbnTestServer.serverArgs')], }, }; } diff --git a/x-pack/test/functional_solution_sidenav/config.ts b/x-pack/test/functional_solution_sidenav/config.ts index f997aaea7c5e2..031fb55d192b7 100644 --- a/x-pack/test/functional_solution_sidenav/config.ts +++ b/x-pack/test/functional_solution_sidenav/config.ts @@ -20,12 +20,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) { testFiles: [require.resolve('.')], kbnTestServer: { ...functionalConfig.get('kbnTestServer'), - serverArgs: [ - ...functionalConfig.get('kbnTestServer.serverArgs'), - // Note: the base64 string in the cloud.id config contains the ES endpoint required in the functional tests - '--xpack.cloud.id=ftr_fake_cloud_id:aGVsbG8uY29tOjQ0MyRFUzEyM2FiYyRrYm4xMjNhYmM=', - '--xpack.cloud.base_url=https://cloud.elastic.co', - ], + serverArgs: [...functionalConfig.get('kbnTestServer.serverArgs')], }, }; } From 3bc7b1d50c6068898a41e05eb2c8a898624e55be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 6 Dec 2024 15:29:16 +0100 Subject: [PATCH 3/7] Fix tour scroll issue --- .../public/nav_control/solution_view_tour/solution_view_tour.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx index eda87809c66b3..dc4f77a3b3e00 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx @@ -72,6 +72,7 @@ export const SolutionViewTour: FC = ({ children, solution, isTourOpen, on onFinish={onFinishTour} step={1} stepsTotal={1} + repositionOnScroll title={i18n.translate('xpack.spaces.navControl.tour.title', { defaultMessage: 'You chose the {solution} solution view', values: { solution: solutionLabel }, From c95865e679872cd00bcf548068dcfdc8cb3834fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Mon, 9 Dec 2024 17:00:34 +0000 Subject: [PATCH 4/7] Update jest test --- x-pack/plugins/spaces/server/plugin.test.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/spaces/server/plugin.test.ts b/x-pack/plugins/spaces/server/plugin.test.ts index 40aaf7044a4ea..cb2cee96a34e7 100644 --- a/x-pack/plugins/spaces/server/plugin.test.ts +++ b/x-pack/plugins/spaces/server/plugin.test.ts @@ -36,10 +36,7 @@ describe('Spaces plugin', () => { "hasOnlyDefaultSpace$": Observable { "operator": [Function], "source": Observable { - "operator": [Function], - "source": Observable { - "_subscribe": [Function], - }, + "_subscribe": [Function], }, }, "spacesClient": Object { @@ -123,10 +120,7 @@ describe('Spaces plugin', () => { "hasOnlyDefaultSpace$": Observable { "operator": [Function], "source": Observable { - "operator": [Function], - "source": Observable { - "_subscribe": [Function], - }, + "_subscribe": [Function], }, }, "spacesService": Object { From 3d80b56756b555c00a28a6a5c78dbd911354d98a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Mon, 9 Dec 2024 17:26:52 +0000 Subject: [PATCH 5/7] Fix functional test --- x-pack/test/accessibility/apps/group1/spaces.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/test/accessibility/apps/group1/spaces.ts b/x-pack/test/accessibility/apps/group1/spaces.ts index 324e3bcbcdccd..87cedf0f7d47c 100644 --- a/x-pack/test/accessibility/apps/group1/spaces.ts +++ b/x-pack/test/accessibility/apps/group1/spaces.ts @@ -54,6 +54,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.spaceSelector.clickCreateSpace(); await PageObjects.spaceSelector.clickEnterSpaceName(); await PageObjects.spaceSelector.addSpaceName('space_a'); + await PageObjects.spaceSelector.changeSolutionView('classic'); await a11y.testAppSnapshot(); }); @@ -90,6 +91,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.spaceSelector.clickCreateSpace(); await PageObjects.spaceSelector.clickEnterSpaceName(); await PageObjects.spaceSelector.addSpaceName('space_b'); + await PageObjects.spaceSelector.changeSolutionView('classic'); await PageObjects.spaceSelector.clickSaveSpaceCreation(); await PageObjects.common.navigateToApp('home'); await PageObjects.spaceSelector.openSpacesNav(); From 11c019cc8c9b222e24da478634278f2214d633fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Tue, 10 Dec 2024 14:23:55 +0000 Subject: [PATCH 6/7] Remove functional test config --- .buildkite/ftr_platform_stateful_configs.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.buildkite/ftr_platform_stateful_configs.yml b/.buildkite/ftr_platform_stateful_configs.yml index af04aef1e0911..d02d3d198a2ef 100644 --- a/.buildkite/ftr_platform_stateful_configs.yml +++ b/.buildkite/ftr_platform_stateful_configs.yml @@ -264,7 +264,6 @@ enabled: - x-pack/test/functional/apps/security/config.ts - x-pack/test/functional/apps/snapshot_restore/config.ts - x-pack/test/functional/apps/spaces/config.ts - - x-pack/test/functional/apps/spaces/solution_view_flag_enabled/config.ts - x-pack/test/functional/apps/status_page/config.ts - x-pack/test/functional/apps/transform/creation/index_pattern/config.ts - x-pack/test/functional/apps/transform/creation/runtime_mappings_saved_search/config.ts From 2fb0eec4b487c40f829cd655ed56572c60f19645 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Wed, 11 Dec 2024 11:10:32 +0000 Subject: [PATCH 7/7] Ensure allowSolutionVisibility is passed through the plugin config --- x-pack/plugins/spaces/public/plugin.test.ts | 39 +++++++++++++++------ x-pack/plugins/spaces/public/plugin.tsx | 4 +++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/spaces/public/plugin.test.ts b/x-pack/plugins/spaces/public/plugin.test.ts index 9e0ce8530edc3..40d6d7b39aa7c 100644 --- a/x-pack/plugins/spaces/public/plugin.test.ts +++ b/x-pack/plugins/spaces/public/plugin.test.ts @@ -19,7 +19,7 @@ describe('Spaces plugin', () => { it('should register the space selector app when buildFlavor is traditional', () => { const coreSetup = coreMock.createSetup(); const mockInitializerContext = coreMock.createPluginInitializerContext( - {}, + { allowSolutionVisibility: true }, { buildFlavor: 'traditional' } ); @@ -39,7 +39,7 @@ describe('Spaces plugin', () => { it('should not register the space selector app when buildFlavor is serverless and maxSpaces is 1', () => { const coreSetup = coreMock.createSetup(); const mockInitializerContext = coreMock.createPluginInitializerContext( - { maxSpaces: 1 }, + { maxSpaces: 1, allowSolutionVisibility: true }, { buildFlavor: 'serverless' } ); @@ -59,7 +59,7 @@ describe('Spaces plugin', () => { it('should register the space selector app when buildFlavor is serverless and and maxSpaces is >1', () => { const coreSetup = coreMock.createSetup(); const mockInitializerContext = coreMock.createPluginInitializerContext( - { maxSpaces: 2 }, + { maxSpaces: 2, allowSolutionVisibility: true }, { buildFlavor: 'serverless' } ); @@ -87,7 +87,7 @@ describe('Spaces plugin', () => { management.sections.section.kibana = mockSection; const mockInitializerContext = coreMock.createPluginInitializerContext( - {}, + { allowSolutionVisibility: true }, { buildFlavor: 'traditional' } ); @@ -121,7 +121,9 @@ describe('Spaces plugin', () => { management.sections.section.kibana = mockSection; - const plugin = new SpacesPlugin(coreMock.createPluginInitializerContext({ maxSpaces: 1 })); + const plugin = new SpacesPlugin( + coreMock.createPluginInitializerContext({ maxSpaces: 1, allowSolutionVisibility: true }) + ); plugin.setup(coreSetup, { management, home, @@ -151,7 +153,9 @@ describe('Spaces plugin', () => { management.sections.section.kibana = mockSection; - const plugin = new SpacesPlugin(coreMock.createPluginInitializerContext({ maxSpaces: 2 })); + const plugin = new SpacesPlugin( + coreMock.createPluginInitializerContext({ maxSpaces: 2, allowSolutionVisibility: true }) + ); plugin.setup(coreSetup, { management, home, @@ -178,7 +182,7 @@ describe('Spaces plugin', () => { const coreStart = coreMock.createStart(); const mockInitializerContext = coreMock.createPluginInitializerContext( - {}, + { allowSolutionVisibility: true }, { buildFlavor: 'traditional' } ); @@ -195,7 +199,7 @@ describe('Spaces plugin', () => { const coreStart = coreMock.createStart(); const mockInitializerContext = coreMock.createPluginInitializerContext( - { maxSpaces: 1 }, + { maxSpaces: 1, allowSolutionVisibility: true }, { buildFlavor: 'serverless' } ); @@ -213,7 +217,9 @@ describe('Spaces plugin', () => { const coreSetup = coreMock.createSetup(); const coreStart = coreMock.createStart(); - const plugin = new SpacesPlugin(coreMock.createPluginInitializerContext({ maxSpaces: 1 })); + const plugin = new SpacesPlugin( + coreMock.createPluginInitializerContext({ maxSpaces: 1, allowSolutionVisibility: true }) + ); const spacesSetup = plugin.setup(coreSetup, {}); const spacesStart = plugin.start(coreStart); @@ -225,7 +231,9 @@ describe('Spaces plugin', () => { const coreSetup = coreMock.createSetup(); const coreStart = coreMock.createStart(); - const plugin = new SpacesPlugin(coreMock.createPluginInitializerContext({ maxSpaces: 1000 })); + const plugin = new SpacesPlugin( + coreMock.createPluginInitializerContext({ maxSpaces: 1000, allowSolutionVisibility: true }) + ); const spacesSetup = plugin.setup(coreSetup, {}); const spacesStart = plugin.start(coreStart); @@ -264,5 +272,16 @@ describe('Spaces plugin', () => { expect(spacesStart.isSolutionViewEnabled).toBe(false); } }); + + it('when allowSolutionVisibility is "undefined"', () => { + const coreSetup = coreMock.createSetup(); + + const plugin = new SpacesPlugin( + coreMock.createPluginInitializerContext({ allowSolutionVisibility: undefined }) + ); + expect(() => plugin.setup(coreSetup, {})).toThrowErrorMatchingInlineSnapshot( + `"allowSolutionVisibility has not been set in the Spaces plugin config."` + ); + }); }); }); diff --git a/x-pack/plugins/spaces/public/plugin.tsx b/x-pack/plugins/spaces/public/plugin.tsx index ea0bf2576b32a..c3583204a7bb3 100644 --- a/x-pack/plugins/spaces/public/plugin.tsx +++ b/x-pack/plugins/spaces/public/plugin.tsx @@ -59,6 +59,10 @@ export class SpacesPlugin implements Plugin, plugins: PluginsSetup) { + if (this.config.allowSolutionVisibility === undefined) { + throw new Error('allowSolutionVisibility has not been set in the Spaces plugin config.'); + } + const hasOnlyDefaultSpace = this.config.maxSpaces === 1; this.spacesManager = new SpacesManager(core.http);