Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Stateful sidenav] Add yml setting to force spaces solution view #191743

Merged
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions src/plugins/navigation/public/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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<Space, 'solution'>));

spaces.isSolutionViewEnabled = false;

const { isSolutionNavEnabled$ } = plugin.start(coreStart, {
unifiedSearch,
spaces,
});
await new Promise((resolve) => setTimeout(resolve));

Expand Down
4 changes: 1 addition & 3 deletions src/plugins/navigation/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,8 @@ export class NavigationPublicPlugin
const extensions = this.topNavMenuExtensionsRegistry.getAll();
const chrome = core.chrome as InternalChromeStart;
const activeSpace$: Observable<Space | undefined> = 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;

/*
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ export default function ({ getService }: PluginFunctionalProviderContext) {
'xpack.spaces.maxSpaces (number?)',
'xpack.spaces.allowFeatureVisibility (false|boolean?)',
'xpack.spaces.allowSolutionVisibility (false|boolean?)',
'xpack.spaces.experimental.forceSolutionVisibility (boolean?)',
'xpack.securitySolution.enableExperimental (array?)',
'xpack.securitySolution.prebuiltRulesPackageVersion (string?)',
'xpack.securitySolution.offeringSettings (record?)',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const mockSpacesApi: SpacesApi = {
useSpaces: jest.fn(),
},
hasOnlyDefaultSpace: false,
isSolutionViewEnabled: true,
};

describe('<LegacyUrlConflictCallOut />', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const mockSpacesApi: SpacesApi = {
useSpaces: jest.fn(),
},
hasOnlyDefaultSpace: false,
isSolutionViewEnabled: true,
};

describe('useLegacyUrlRedirect', () => {
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/spaces/public/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,7 @@ export interface ConfigType {
maxSpaces: number;
allowFeatureVisibility: boolean;
allowSolutionVisibility: boolean;
experimental: {
forceSolutionVisibility: boolean;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ describe('ManagementService', () => {
maxSpaces: 1000,
allowFeatureVisibility: true,
allowSolutionVisibility: true,
experimental: {
forceSolutionVisibility: false,
},
};

describe('#setup', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ const config: ConfigType = {
maxSpaces: 1000,
allowFeatureVisibility: true,
allowSolutionVisibility: true,
experimental: {
forceSolutionVisibility: false,
},
};

const eventTracker = new EventTracker({ reportEvent: jest.fn() });
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/spaces/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const createApiMock = (hasOnlyDefaultSpace: boolean): jest.Mocked<SpacesApi> =>
getActiveSpace: jest.fn(),
ui: createApiUiMock(),
hasOnlyDefaultSpace,
isSolutionViewEnabled: true,
});

type SpacesApiUiMock = Omit<jest.Mocked<SpacesApiUi>, 'components'> & {
Expand Down
163 changes: 146 additions & 17 deletions x-pack/plugins/spaces/public/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -13,7 +14,6 @@ import {
} from '@kbn/management-plugin/public/mocks';

import { SpacesPlugin } from './plugin';
// import { ConfigSchema } from './config';

describe('Spaces plugin', () => {
describe('#setup', () => {
Expand Down Expand Up @@ -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);
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: { ... } - Great way to scope and reuse const values in the same test - never thought of this before. Nice!

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);
}
});
});
});
23 changes: 15 additions & 8 deletions x-pack/plugins/spaces/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ export class SpacesPlugin implements Plugin<SpacesPluginSetup, SpacesPluginStart

public setup(core: CoreSetup<PluginsStart, SpacesPluginStart>, 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({
Expand All @@ -69,15 +77,14 @@ export class SpacesPlugin implements Plugin<SpacesPluginSetup, SpacesPluginStart
getActiveSpace$: () => 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);

Expand Down Expand Up @@ -133,7 +140,7 @@ export class SpacesPlugin implements Plugin<SpacesPluginSetup, SpacesPluginStart

registerAnalyticsContext(core.analytics, this.spacesManager.onActiveSpaceChange$);

return { hasOnlyDefaultSpace };
return { hasOnlyDefaultSpace, isSolutionViewEnabled: allowSolutionVisibility };
sebelga marked this conversation as resolved.
Show resolved Hide resolved
}

public start(core: CoreStart) {
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/spaces/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,9 @@ export interface SpacesApi {
* UI components and services to add spaces capabilities to an application.
*/
ui: SpacesApiUi;

/**
* Indicates whether the solution view is enabled.
*/
isSolutionViewEnabled: boolean;
}
9 changes: 9 additions & 0 deletions x-pack/plugins/spaces/server/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ describe('config schema', () => {
"allowFeatureVisibility": true,
"allowSolutionVisibility": true,
"enabled": true,
"experimental": Object {
"forceSolutionVisibility": false,
},
"maxSpaces": 1000,
}
`);
Expand All @@ -32,6 +35,9 @@ describe('config schema', () => {
"allowFeatureVisibility": true,
"allowSolutionVisibility": true,
"enabled": true,
"experimental": Object {
"forceSolutionVisibility": false,
},
"maxSpaces": 1000,
}
`);
Expand All @@ -41,6 +47,9 @@ describe('config schema', () => {
"allowFeatureVisibility": true,
"allowSolutionVisibility": true,
"enabled": true,
"experimental": Object {
"forceSolutionVisibility": false,
},
"maxSpaces": 1000,
}
`);
Expand Down
Loading