From 6f6883d7818a983e642a536d8892a0072a794148 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Wed, 18 Sep 2024 13:12:20 +0100 Subject: [PATCH] [Stateful sidenav] Add yml setting to force spaces solution view (#191743) --- src/plugins/navigation/public/plugin.test.ts | 28 ++- src/plugins/navigation/public/plugin.tsx | 4 +- .../legacy_url_conflict_callout.test.tsx | 1 + .../use_redirect_legacy_url.test.ts | 1 + 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/mocks.ts | 1 + x-pack/plugins/spaces/public/plugin.test.ts | 163 ++++++++++++++++-- x-pack/plugins/spaces/public/plugin.tsx | 23 ++- x-pack/plugins/spaces/public/types.ts | 5 + x-pack/plugins/spaces/server/config.test.ts | 9 + x-pack/plugins/spaces/server/config.ts | 7 + x-pack/plugins/spaces/server/index.ts | 3 + x-pack/plugins/spaces/server/plugin.ts | 5 +- 15 files changed, 224 insertions(+), 35 deletions(-) diff --git a/src/plugins/navigation/public/plugin.test.ts b/src/plugins/navigation/public/plugin.test.ts index 02f23d16de209..a88b51abba665 100644 --- a/src/plugins/navigation/public/plugin.test.ts +++ b/src/plugins/navigation/public/plugin.test.ts @@ -97,11 +97,11 @@ describe('Navigation Plugin', () => { describe('addSolutionNavigation()', () => { it('should update the solution navigation definitions', async () => { - const { plugin, coreStart, unifiedSearch, cloud } = setup(); + const { plugin, coreStart, unifiedSearch, spaces } = setup(); const { addSolutionNavigation } = plugin.start(coreStart, { unifiedSearch, - cloud, + spaces, }); await new Promise((resolve) => setTimeout(resolve)); @@ -180,13 +180,29 @@ describe('Navigation Plugin', () => { }); describe('isSolutionNavEnabled$', () => { - // This test will need to be changed when we remove the feature flag - it('should be off by default', async () => { - const { plugin, coreStart, unifiedSearch, cloud } = setup(); + it('should be off if spaces plugin not available', async () => { + const { plugin, coreStart, unifiedSearch } = setup(); const { isSolutionNavEnabled$ } = plugin.start(coreStart, { unifiedSearch, - cloud, + }); + await new Promise((resolve) => setTimeout(resolve)); + + const isEnabled = await firstValueFrom(isSolutionNavEnabled$); + expect(isEnabled).toBe(false); + }); + + it('should be off if spaces plugin `isSolutionViewEnabled` = false', async () => { + const { plugin, coreStart, unifiedSearch, spaces } = setup(); + spaces.getActiveSpace$ = jest + .fn() + .mockReturnValue(of({ solution: 'es' } as Pick)); + + spaces.isSolutionViewEnabled = false; + + const { isSolutionNavEnabled$ } = plugin.start(coreStart, { + unifiedSearch, + spaces, }); await new Promise((resolve) => setTimeout(resolve)); diff --git a/src/plugins/navigation/public/plugin.tsx b/src/plugins/navigation/public/plugin.tsx index d2aa1f4c7f557..f382b80221642 100644 --- a/src/plugins/navigation/public/plugin.tsx +++ b/src/plugins/navigation/public/plugin.tsx @@ -72,10 +72,8 @@ export class NavigationPublicPlugin const extensions = this.topNavMenuExtensionsRegistry.getAll(); const chrome = core.chrome as InternalChromeStart; const activeSpace$: Observable = spaces?.getActiveSpace$() ?? of(undefined); - const onCloud = cloud !== undefined; // The new side nav will initially only be available to cloud users const isServerless = this.initializerContext.env.packageInfo.buildFlavor === 'serverless'; - - this.isSolutionNavEnabled = onCloud && !isServerless; + this.isSolutionNavEnabled = spaces?.isSolutionViewEnabled ?? false; /* * diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/legacy_url_conflict_callout.test.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/legacy_url_conflict_callout.test.tsx index 0582cc13eb92b..5334143f27047 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/legacy_url_conflict_callout.test.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/legacy_url_conflict_callout.test.tsx @@ -31,6 +31,7 @@ const mockSpacesApi: SpacesApi = { useSpaces: jest.fn(), }, hasOnlyDefaultSpace: false, + isSolutionViewEnabled: true, }; describe('', () => { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_redirect_legacy_url.test.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_redirect_legacy_url.test.ts index d7971cce43f80..e706f5fda4b39 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_redirect_legacy_url.test.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_details_ui/pages/rule_details/use_redirect_legacy_url.test.ts @@ -32,6 +32,7 @@ const mockSpacesApi: SpacesApi = { useSpaces: jest.fn(), }, hasOnlyDefaultSpace: false, + isSolutionViewEnabled: true, }; describe('useLegacyUrlRedirect', () => { diff --git a/x-pack/plugins/spaces/public/config.ts b/x-pack/plugins/spaces/public/config.ts index dcd203eb696e3..3dd2d3bc89f92 100644 --- a/x-pack/plugins/spaces/public/config.ts +++ b/x-pack/plugins/spaces/public/config.ts @@ -9,4 +9,7 @@ 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 5f97515171518..e5438c0cf5e9c 100644 --- a/x-pack/plugins/spaces/public/management/management_service.test.ts +++ b/x-pack/plugins/spaces/public/management/management_service.test.ts @@ -24,6 +24,9 @@ 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 61619c499181c..d9852a82f8259 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 @@ -31,6 +31,9 @@ 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/mocks.ts b/x-pack/plugins/spaces/public/mocks.ts index 1499384f4ed72..8478ed010cd98 100644 --- a/x-pack/plugins/spaces/public/mocks.ts +++ b/x-pack/plugins/spaces/public/mocks.ts @@ -16,6 +16,7 @@ const createApiMock = (hasOnlyDefaultSpace: boolean): jest.Mocked => getActiveSpace: jest.fn(), ui: createApiUiMock(), hasOnlyDefaultSpace, + isSolutionViewEnabled: true, }); type SpacesApiUiMock = Omit, 'components'> & { diff --git a/x-pack/plugins/spaces/public/plugin.test.ts b/x-pack/plugins/spaces/public/plugin.test.ts index 9afe7d27e9a04..33565748a99e3 100644 --- a/x-pack/plugins/spaces/public/plugin.test.ts +++ b/x-pack/plugins/spaces/public/plugin.test.ts @@ -5,6 +5,7 @@ * 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 { @@ -13,7 +14,6 @@ import { } from '@kbn/management-plugin/public/mocks'; import { SpacesPlugin } from './plugin'; -// import { ConfigSchema } from './config'; describe('Spaces plugin', () => { describe('#setup', () => { @@ -209,27 +209,156 @@ describe('Spaces plugin', () => { }); }); - it('determines hasOnlyDefaultSpace correctly when maxSpaces=1', () => { - const coreSetup = coreMock.createSetup(); - const coreStart = coreMock.createStart(); + describe('hasOnlyDefaultSpace', () => { + it('determines hasOnlyDefaultSpace correctly when maxSpaces=1', () => { + const coreSetup = coreMock.createSetup(); + const coreStart = coreMock.createStart(); - const plugin = new SpacesPlugin(coreMock.createPluginInitializerContext({ maxSpaces: 1 })); - const spacesSetup = plugin.setup(coreSetup, {}); - const spacesStart = plugin.start(coreStart); + const plugin = new SpacesPlugin(coreMock.createPluginInitializerContext({ maxSpaces: 1 })); + const spacesSetup = plugin.setup(coreSetup, {}); + const spacesStart = plugin.start(coreStart); - expect(spacesSetup.hasOnlyDefaultSpace).toBe(true); - expect(spacesStart.hasOnlyDefaultSpace).toBe(true); + expect(spacesSetup.hasOnlyDefaultSpace).toBe(true); + expect(spacesStart.hasOnlyDefaultSpace).toBe(true); + }); + + it('determines hasOnlyDefaultSpace correctly when maxSpaces=1000', () => { + const coreSetup = coreMock.createSetup(); + const coreStart = coreMock.createStart(); + + const plugin = new SpacesPlugin(coreMock.createPluginInitializerContext({ maxSpaces: 1000 })); + const spacesSetup = plugin.setup(coreSetup, {}); + const spacesStart = plugin.start(coreStart); + + expect(spacesSetup.hasOnlyDefaultSpace).toBe(false); + expect(spacesStart.hasOnlyDefaultSpace).toBe(false); + }); }); - it('determines hasOnlyDefaultSpace correctly when maxSpaces=1000', () => { - const coreSetup = coreMock.createSetup(); - const coreStart = coreMock.createStart(); + describe('isSolutionViewEnabled', () => { + it('when onCloud, not serverless and 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' } + ) + ); + const spacesSetup = plugin.setup(coreSetup, { cloud }); + const spacesStart = plugin.start(coreStart); + + expect(spacesSetup.isSolutionViewEnabled).toBe(true); + expect(spacesStart.isSolutionViewEnabled).toBe(true); + }); - const plugin = new SpacesPlugin(coreMock.createPluginInitializerContext({ maxSpaces: 1000 })); - const spacesSetup = plugin.setup(coreSetup, {}); - const spacesStart = plugin.start(coreStart); + it('when not onCloud and allowSolutionVisibility is "true"', () => { + const coreSetup = coreMock.createSetup(); + const coreStart = coreMock.createStart(); - expect(spacesSetup.hasOnlyDefaultSpace).toBe(false); - expect(spacesStart.hasOnlyDefaultSpace).toBe(false); + { + const plugin = new SpacesPlugin( + coreMock.createPluginInitializerContext( + { allowSolutionVisibility: true }, // it is true but we are not onCloud + { buildFlavor: 'traditional' } + ) + ); + 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 31ba324b926f3..6d545cdb70e61 100644 --- a/x-pack/plugins/spaces/public/plugin.tsx +++ b/x-pack/plugins/spaces/public/plugin.tsx @@ -60,6 +60,14 @@ 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 = { ui: getUiApi({ @@ -69,15 +77,14 @@ export class SpacesPlugin implements Plugin this.spacesManager.onActiveSpaceChange$, getActiveSpace: () => this.spacesManager.getActiveSpace(), hasOnlyDefaultSpace, + isSolutionViewEnabled: allowSolutionVisibility, + }; + + this.config = { + ...this.config, + allowSolutionVisibility, }; - const onCloud = plugins.cloud !== undefined && plugins.cloud.isCloudEnabled; - if (!onCloud) { - this.config = { - ...this.config, - allowSolutionVisibility: false, - }; - } registerSpacesEventTypes(core); this.eventTracker = new EventTracker(core.analytics); @@ -133,7 +140,7 @@ export class SpacesPlugin implements Plugin { "allowFeatureVisibility": true, "allowSolutionVisibility": true, "enabled": true, + "experimental": Object { + "forceSolutionVisibility": false, + }, "maxSpaces": 1000, } `); @@ -32,6 +35,9 @@ describe('config schema', () => { "allowFeatureVisibility": true, "allowSolutionVisibility": true, "enabled": true, + "experimental": Object { + "forceSolutionVisibility": false, + }, "maxSpaces": 1000, } `); @@ -41,6 +47,9 @@ describe('config schema', () => { "allowFeatureVisibility": true, "allowSolutionVisibility": true, "enabled": true, + "experimental": Object { + "forceSolutionVisibility": false, + }, "maxSpaces": 1000, } `); diff --git a/x-pack/plugins/spaces/server/config.ts b/x-pack/plugins/spaces/server/config.ts index a7c9606e74543..ef6b300d43965 100644 --- a/x-pack/plugins/spaces/server/config.ts +++ b/x-pack/plugins/spaces/server/config.ts @@ -54,6 +54,13 @@ export const ConfigSchema = schema.object({ defaultValue: true, }), }), + experimental: schema.maybe( + offeringBasedSchema({ + traditional: schema.object({ + forceSolutionVisibility: schema.boolean({ defaultValue: false }), + }), + }) + ), }); export function createConfig$(context: PluginInitializerContext) { diff --git a/x-pack/plugins/spaces/server/index.ts b/x-pack/plugins/spaces/server/index.ts index a568f52c7c29a..297a99a525ec7 100644 --- a/x-pack/plugins/spaces/server/index.ts +++ b/x-pack/plugins/spaces/server/index.ts @@ -34,6 +34,9 @@ export const config: PluginConfigDescriptor = { maxSpaces: true, allowFeatureVisibility: true, allowSolutionVisibility: true, + experimental: { + forceSolutionVisibility: true, + }, }, }; diff --git a/x-pack/plugins/spaces/server/plugin.ts b/x-pack/plugins/spaces/server/plugin.ts index 8b91dcac35917..2f8fb2ec30842 100644 --- a/x-pack/plugins/spaces/server/plugin.ts +++ b/x-pack/plugins/spaces/server/plugin.ts @@ -130,7 +130,10 @@ export class SpacesPlugin ([config, onCloud]): ConfigType => ({ ...config, // We only allow "solution" to be set on cloud environments, not on prem - allowSolutionVisibility: onCloud ? config.allowSolutionVisibility : false, + // unless the forceSolutionVisibility flag is set. + allowSolutionVisibility: + (onCloud && config.allowSolutionVisibility) || + Boolean(config.experimental?.forceSolutionVisibility), }) ) );