Skip to content

Commit

Permalink
Spaces - Hiding management link (#38472) (#38903)
Browse files Browse the repository at this point in the history
* Changing the Spaces management section to behave like the other FC
controlled sections

* Adding those glorious tests and fixing a bug

* Fixing some test descriptions

* Making the mergeCapabilities operation emulate the old behavior

* Fixing privileges test with the addition of the new action

* Updating jest snapshot

* Adding tests, preventing additional clobbering

* Changing requireUICapability to use management.kibana.spaces
  • Loading branch information
kobelb authored Jun 13, 2019
1 parent e0b9c01 commit 3362066
Show file tree
Hide file tree
Showing 12 changed files with 312 additions and 19 deletions.
84 changes: 84 additions & 0 deletions src/legacy/server/capabilities/merge_capabilities.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { mergeCapabilities } from './merge_capabilities';

const defaultProps = {
catalogue: {},
management: {},
navLinks: {},
};

test(`"{ foo: {} }" doesn't clobber "{ foo: { bar: true } }"`, () => {
const output1 = mergeCapabilities({ foo: { bar: true } }, { foo: {} });
expect(output1).toEqual({ ...defaultProps, foo: { bar: true } });

const output2 = mergeCapabilities({ foo: { bar: true } }, { foo: {} });
expect(output2).toEqual({ ...defaultProps, foo: { bar: true } });
});

test(`"{ foo: { bar: true } }" doesn't clobber "{ baz: { quz: true } }"`, () => {
const output1 = mergeCapabilities({ foo: { bar: true } }, { baz: { quz: true } });
expect(output1).toEqual({ ...defaultProps, foo: { bar: true }, baz: { quz: true } });

const output2 = mergeCapabilities({ baz: { quz: true } }, { foo: { bar: true } });
expect(output2).toEqual({ ...defaultProps, foo: { bar: true }, baz: { quz: true } });
});

test(`"{ foo: { bar: { baz: true } } }" doesn't clobber "{ foo: { bar: { quz: true } } }"`, () => {
const output1 = mergeCapabilities(
{ foo: { bar: { baz: true } } },
{ foo: { bar: { quz: true } } }
);
expect(output1).toEqual({ ...defaultProps, foo: { bar: { baz: true, quz: true } } });

const output2 = mergeCapabilities(
{ foo: { bar: { quz: true } } },
{ foo: { bar: { baz: true } } }
);
expect(output2).toEqual({ ...defaultProps, foo: { bar: { baz: true, quz: true } } });
});

test(`error is thrown if boolean and object clash`, () => {
expect(() => {
mergeCapabilities({ foo: { bar: { baz: true } } }, { foo: { bar: true } });
}).toThrowErrorMatchingInlineSnapshot(`"a boolean and an object can't be merged"`);

expect(() => {
mergeCapabilities({ foo: { bar: true } }, { foo: { bar: { baz: true } } });
}).toThrowErrorMatchingInlineSnapshot(`"a boolean and an object can't be merged"`);
});

test(`supports duplicates as long as the booleans are the same`, () => {
const output1 = mergeCapabilities({ foo: { bar: true } }, { foo: { bar: true } });
expect(output1).toEqual({ ...defaultProps, foo: { bar: true } });

const output2 = mergeCapabilities({ foo: { bar: false } }, { foo: { bar: false } });
expect(output2).toEqual({ ...defaultProps, foo: { bar: false } });
});

test(`error is thrown if merging "true" and "false"`, () => {
expect(() => {
mergeCapabilities({ foo: { bar: false } }, { foo: { bar: true } });
}).toThrowErrorMatchingInlineSnapshot(`"\\"true\\" and \\"false\\" can't be merged"`);

expect(() => {
mergeCapabilities({ foo: { bar: true } }, { foo: { bar: false } });
}).toThrowErrorMatchingInlineSnapshot(`"\\"true\\" and \\"false\\" can't be merged"`);
});
29 changes: 17 additions & 12 deletions src/legacy/server/capabilities/merge_capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,28 @@
* under the License.
*/

import typeDetect from 'type-detect';
import { merge } from 'lodash';
import { Capabilities } from '../../../core/public';

export const mergeCapabilities = (...sources: Array<Partial<Capabilities>>): Capabilities =>
sources.reduce(
(capabilities: Capabilities, source) => {
Object.entries(source).forEach(([key, value = {}]) => {
capabilities[key] = {
...value,
...capabilities[key],
};
});

return capabilities;
},
merge(
{
navLinks: {},
management: {},
catalogue: {},
} as Capabilities
},
...sources,
(a: any, b: any) => {
if (
(typeDetect(a) === 'boolean' && typeDetect(b) === 'Object') ||
(typeDetect(b) === 'boolean' && typeDetect(a) === 'Object')
) {
throw new Error(`a boolean and an object can't be merged`);
}

if (typeDetect(a) === 'boolean' && typeDetect(b) === 'boolean' && a !== b) {
throw new Error(`"true" and "false" can't be merged`);
}
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,13 @@ describe('features', () => {
actions.login,
actions.version,
...(expectGetFeatures ? [actions.api.get('features')] : []),
...(expectManageSpaces ? [actions.space.manage, actions.ui.get('spaces', 'manage')] : []),
...(expectManageSpaces
? [
actions.space.manage,
actions.ui.get('spaces', 'manage'),
actions.ui.get('management', 'kibana', 'spaces'),
]
: []),
actions.app.get('app-1'),
actions.app.get('app-2'),
actions.ui.get('catalogue', 'catalogue-1'),
Expand Down Expand Up @@ -403,7 +409,13 @@ describe('features', () => {
actions.login,
actions.version,
...(expectGetFeatures ? [actions.api.get('features')] : []),
...(expectManageSpaces ? [actions.space.manage, actions.ui.get('spaces', 'manage')] : []),
...(expectManageSpaces
? [
actions.space.manage,
actions.ui.get('spaces', 'manage'),
actions.ui.get('management', 'kibana', 'spaces'),
]
: []),
actions.ui.get('catalogue', 'bar-catalogue-1'),
actions.ui.get('catalogue', 'bar-catalogue-2'),
actions.ui.get('management', 'bar-management', 'bar-management-1'),
Expand Down Expand Up @@ -614,7 +626,13 @@ describe('features', () => {
actions.login,
actions.version,
...(expectGetFeatures ? [actions.api.get('features')] : []),
...(expectManageSpaces ? [actions.space.manage, actions.ui.get('spaces', 'manage')] : []),
...(expectManageSpaces
? [
actions.space.manage,
actions.ui.get('spaces', 'manage'),
actions.ui.get('management', 'kibana', 'spaces'),
]
: []),
actions.allHack,
]);
expect(actual).toHaveProperty(`${group}.read`, [actions.login, actions.version]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export function privilegesFactory(actions: Actions, xpackMainPlugin: XPackMainPl
actions.api.get('features'),
actions.space.manage,
actions.ui.get('spaces', 'manage'),
actions.ui.get('management', 'kibana', 'spaces'),
...allActions,
actions.allHack,
],
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/spaces/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ export const spaces = (kibana: Record<string, any>) =>
spaces: {
manage: true,
},
management: {
kibana: {
spaces: true,
},
},
};
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class ManageSpacePageUI extends Component<Props, State> {
const { showAlteringActiveSpaceDialog } = this.state;

return (
<Fragment>
<div data-test-subj="spaces-edit-page">
{this.getFormHeading()}

<EuiSpacer size={'s'} />
Expand Down Expand Up @@ -188,7 +188,7 @@ class ManageSpacePageUI extends Component<Props, State> {
}}
/>
)}
</Fragment>
</div>
);
};

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/spaces/public/views/management/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import routes from 'ui/routes';
import { AdvancedSettingsSubtitle } from './components/advanced_settings_subtitle';
import { AdvancedSettingsTitle } from './components/advanced_settings_title';

const MANAGE_SPACES_KEY = 'manage_spaces';
const MANAGE_SPACES_KEY = 'spaces';

routes.defaults(/\/management/, {
resolve: {
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/spaces/public/views/management/page_routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const reactRootNodeId = 'manageSpacesReactRoot';
routes.when('/management/spaces/list', {
template,
k7Breadcrumbs: getListBreadcrumbs,
requireUICapability: 'management.kibana.spaces',
controller(
$scope: any,
$http: any,
Expand Down Expand Up @@ -53,6 +54,7 @@ routes.when('/management/spaces/list', {
routes.when('/management/spaces/create', {
template,
k7Breadcrumbs: getCreateBreadcrumbs,
requireUICapability: 'management.kibana.spaces',
controller(
$scope: any,
$http: any,
Expand Down Expand Up @@ -89,6 +91,7 @@ routes.when('/management/spaces/edit', {
routes.when('/management/spaces/edit/:spaceId', {
template,
k7Breadcrumbs: () => getEditBreadcrumbs(),
requireUICapability: 'management.kibana.spaces',
controller(
$scope: any,
$http: any,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class SpacesGridPageUI extends Component<Props, State> {

public render() {
return (
<div className="spcGridPage">
<div className="spcGridPage" data-test-subj="spaces-grid-page">
<EuiPageContent horizontalPosition="center">{this.getPageContent()}</EuiPageContent>
<SecureSpaceMessage />
{this.getConfirmDeleteModal()}
Expand Down
Loading

0 comments on commit 3362066

Please sign in to comment.