Skip to content

Commit

Permalink
[Spaces UI] Role Editor Flyout Should Match in Roles Mgmt (elastic#19…
Browse files Browse the repository at this point in the history
…8182)

## Summary

Part of elastic/kibana-team#1242

**Fixes for alignment of the Role editor flyout**
1. Remove the warning callout regarding global privileges that impact
other privileges
1. Unify the info callouts regarding combination of privileges
1. set "Customize" as the default selected option when assigning new
privileges
1. update placeholders for selector box when assigning privileges
1. Hide privileges controls if no spaces are selected
1. Update button group label text to "Define privileges" and align
helper texts below
1. Align headers for assign/edit states
1. Remove descriptions under headers
1. Update size of info callout above button group to small
1. Reduce text size for the "Manage roles" link
1. Remove the "Additional Stack Management permissions can be found
outside of this menu..." test for the Spaces Management context.

**Polish fixes**
1. Remove features visible column
1. ~~Remove identifier column from spaces grid~~
1. Fix vertical alignment of non-current space name in table
1. Ordered the listing of assigned roles during and after search
1. Removing a role from the space shows a confirmation modal
1. Update columns widths in the spaces grid
1. Remove the "By default your current view is Classic" callout

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
  • Loading branch information
tsullivan authored Nov 20, 2024
1 parent d435b8a commit 226924e
Show file tree
Hide file tree
Showing 18 changed files with 235 additions and 536 deletions.
2 changes: 1 addition & 1 deletion x-pack/packages/security/ui_components/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"type": "shared-common",
"type": "shared-browser",
"id": "@kbn/security-ui-components",
"owner": "@elastic/kibana-security"
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const setup = (config: TestConfig) => {
kibanaPrivileges={kibanaPrivileges}
onChange={onChange}
onChangeAll={onChangeAll}
showAdditionalPermissionsMessage={true}
canCustomizeSubFeaturePrivileges={config.canCustomizeSubFeaturePrivileges}
privilegeIndex={config.privilegeIndex}
allSpacesSelected={true}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,10 @@ interface Props {
privilegeIndex: number;
onChange: (featureId: string, privileges: string[]) => void;
onChangeAll: (privileges: string[]) => void;
showAdditionalPermissionsMessage: boolean;
canCustomizeSubFeaturePrivileges: boolean;
allSpacesSelected: boolean;
disabled?: boolean;
/**
* default is true, to remain backwards compatible
*/
showTitle?: boolean;
}

interface State {
Expand All @@ -62,7 +59,6 @@ export class FeatureTable extends Component<Props, State> {
public static defaultProps = {
privilegeIndex: -1,
showLocks: true,
showTitle: true,
};

private featureCategories: Map<string, SecuredFeature[]> = new Map();
Expand Down Expand Up @@ -189,20 +185,7 @@ export class FeatureTable extends Component<Props, State> {
return (
<div>
<EuiFlexGroup alignItems={'flexEnd'}>
<EuiFlexItem>
{this.props.showTitle && (
<EuiText size="xs">
<b>
{i18n.translate(
'xpack.security.management.editRole.featureTable.featureVisibilityTitle',
{
defaultMessage: 'Customize feature privileges',
}
)}
</b>
</EuiText>
)}
</EuiFlexItem>
<EuiFlexItem />
{!this.props.disabled && (
<EuiFlexItem grow={false}>
<ChangeAllPrivilegesControl
Expand Down Expand Up @@ -397,7 +380,7 @@ export class FeatureTable extends Component<Props, State> {
};

private getCategoryHelpText = (category: AppCategory) => {
if (category.id === 'management') {
if (category.id === 'management' && this.props.showAdditionalPermissionsMessage) {
return i18n.translate(
'xpack.security.management.editRole.featureTable.managementCategoryHelpText',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ export class SimplePrivilegeSection extends Component<Props, State> {
privilegeIndex={this.props.role.kibana.findIndex((k) =>
isGlobalPrivilegeDefinition(k)
)}
showAdditionalPermissionsMessage={true}
canCustomizeSubFeaturePrivileges={this.props.canCustomizeSubFeaturePrivileges}
allSpacesSelected
disabled={!this.props.editable}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import type { Space } from '@kbn/spaces-plugin/public';
import { findTestSubject, mountWithIntl } from '@kbn/test-jest-helpers';

import { PrivilegeSpaceForm } from './privilege_space_form';
import { SpaceSelector } from './space_selector';
import type { Role } from '../../../../../../../common';

const createRole = (kibana: Role['kibana'] = []): Role => {
Expand Down Expand Up @@ -57,7 +56,7 @@ const renderComponent = (props: React.ComponentProps<typeof PrivilegeSpaceForm>)
};

describe('PrivilegeSpaceForm', () => {
it('renders an empty form when the role contains no Kibana privileges', () => {
it('renders no form when no role is selected', () => {
const role = createRole();
const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures);

Expand All @@ -71,40 +70,9 @@ describe('PrivilegeSpaceForm', () => {
onCancel: jest.fn(),
});

expect(
wrapper.find(EuiButtonGroup).filter('[name="basePrivilegeButtonGroup"]').props().idSelected
).toEqual(`basePrivilege_custom`);
expect(wrapper.find(FeatureTable).props().disabled).toEqual(true);
expect(getDisplayedFeaturePrivileges(wrapper)).toMatchInlineSnapshot(`
Object {
"excluded_from_base": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"no_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"with_excluded_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"with_require_all_spaces_for_feature_and_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"with_require_all_spaces_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
"with_sub_features": Object {
"primaryFeaturePrivilege": "none",
"subFeaturePrivileges": Array [],
},
}
`);

expect(findTestSubject(wrapper, 'spaceFormGlobalPermissionsSupersedeWarning')).toHaveLength(0);
expect(wrapper.find(EuiButtonGroup).filter('[name="basePrivilegeButtonGroup"]')).toHaveLength(
0
);
});

it('renders when a base privilege is selected', () => {
Expand Down Expand Up @@ -232,43 +200,6 @@ describe('PrivilegeSpaceForm', () => {
expect(findTestSubject(wrapper, 'spaceFormGlobalPermissionsSupersedeWarning')).toHaveLength(0);
});

it('renders a warning when configuring a global privilege after space privileges are already defined', () => {
const role = createRole([
{
base: [],
feature: {
with_sub_features: ['read'],
},
spaces: ['foo'],
},
{
base: [],
feature: {
with_sub_features: ['all'],
},
spaces: ['*'],
},
]);

const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures);

const wrapper = renderComponent({
role,
spaces: displaySpaces,
kibanaPrivileges,
privilegeIndex: -1,
canCustomizeSubFeaturePrivileges: true,
onChange: jest.fn(),
onCancel: jest.fn(),
});

wrapper.find(SpaceSelector).props().onChange(['*']);

wrapper.update();

expect(findTestSubject(wrapper, 'globalPrivilegeWarning')).toHaveLength(1);
});

it('renders a warning when space privileges are less permissive than configured global privileges', () => {
const role = createRole([
{
Expand Down
Loading

0 comments on commit 226924e

Please sign in to comment.