From 5f02f1f6c68bc378fbe0139f2944237766f59de3 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Sat, 9 Nov 2024 11:07:52 +1100 Subject: [PATCH] [8.x] [Spaces] update privilege selection text and icon (#199498) (#199568) # Backport This will backport the following commits from `main` to `8.x`: - [[Spaces] update privilege selection text and icon (#199498)](https://github.com/elastic/kibana/pull/199498) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) \r\n- [x] Any UI touched in this PR is usable by keyboard only (learn more\r\nabout [keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n\r\n- [x] This renders correctly on smaller devices using a responsive\r\nlayout. (You can test this [in your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n","sha":"cb7e5f6a0a70a35a46ce577d31476eabe4f1555e","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","release_note:skip","v9.0.0","backport:prev-minor"],"title":"[Spaces] update privilege selection text and icon","number":199498,"url":"https://github.com/elastic/kibana/pull/199498","mergeCommit":{"message":"[Spaces] update privilege selection text and icon (#199498)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana-team/issues/1257\r\n\r\nThis PR modifies the spaces bulk edit context menu items to match the\r\nnew design icons and text\r\n\r\n#### Visuals\r\n\r\n##### After\r\n\"Screenshot\r\n\r\n\r\n##### Before\r\n\"Screenshot\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n\r\n- [x] Any UI touched in this PR is usable by keyboard only (learn more\r\nabout [keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n\r\n- [x] This renders correctly on smaller devices using a responsive\r\nlayout. (You can test this [in your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n","sha":"cb7e5f6a0a70a35a46ce577d31476eabe4f1555e"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199498","number":199498,"mergeCommit":{"message":"[Spaces] update privilege selection text and icon (#199498)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana-team/issues/1257\r\n\r\nThis PR modifies the spaces bulk edit context menu items to match the\r\nnew design icons and text\r\n\r\n#### Visuals\r\n\r\n##### After\r\n\"Screenshot\r\n\r\n\r\n##### Before\r\n\"Screenshot\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [x] Any text added follows [EUI's writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\r\nsentence case text and includes [i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n\r\n- [x] Any UI touched in this PR is usable by keyboard only (learn more\r\nabout [keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n\r\n- [x] This renders correctly on smaller devices using a responsive\r\nlayout. (You can test this [in your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n","sha":"cb7e5f6a0a70a35a46ce577d31476eabe4f1555e"}}]}] BACKPORT--> Co-authored-by: Eyo O. Eyo <7893459+eokoneyo@users.noreply.github.com> --- .../security/ui_components/src/constants.ts | 2 +- .../change_all_privileges.tsx | 48 +++- .../feature_table.test.tsx | 24 +- .../privilege_space_form.test.tsx | 248 +++++++++--------- .../space_assign_role_privilege_form.test.tsx | 79 +++--- 5 files changed, 217 insertions(+), 184 deletions(-) diff --git a/x-pack/packages/security/ui_components/src/constants.ts b/x-pack/packages/security/ui_components/src/constants.ts index a47a9bff9842d..d30c61bf02d6d 100644 --- a/x-pack/packages/security/ui_components/src/constants.ts +++ b/x-pack/packages/security/ui_components/src/constants.ts @@ -5,5 +5,5 @@ * 2.0. */ -export const NO_PRIVILEGE_VALUE: string = 'none'; +export const NO_PRIVILEGE_VALUE = 'none' as const; export const CUSTOM_PRIVILEGE_VALUE: string = 'custom'; diff --git a/x-pack/packages/security/ui_components/src/kibana_privilege_table/change_all_privileges.tsx b/x-pack/packages/security/ui_components/src/kibana_privilege_table/change_all_privileges.tsx index 4793f86a7a2a5..e475a5da7a106 100644 --- a/x-pack/packages/security/ui_components/src/kibana_privilege_table/change_all_privileges.tsx +++ b/x-pack/packages/security/ui_components/src/kibana_privilege_table/change_all_privileges.tsx @@ -15,9 +15,9 @@ import { EuiPopover, EuiText, } from '@elastic/eui'; -import _ from 'lodash'; import React, { Component } from 'react'; +import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; import type { KibanaPrivilege } from '@kbn/security-role-management-model'; @@ -38,6 +38,43 @@ export class ChangeAllPrivilegesControl extends Component { isPopoverOpen: false, }; + private getPrivilegeCopy = (privilege: string): { label?: string; icon?: string } => { + switch (privilege) { + case 'all': + return { + icon: 'documentEdit', + label: i18n.translate( + 'xpack.security.management.editRole.changeAllPrivileges.allSelectionLabel', + { + defaultMessage: 'Grant full access for all', + } + ), + }; + case 'read': + return { + icon: 'glasses', + label: i18n.translate( + 'xpack.security.management.editRole.changeAllPrivileges.readSelectionLabel', + { + defaultMessage: 'Grant read access for all', + } + ), + }; + case 'none': + return { + icon: 'trash', + label: i18n.translate( + 'xpack.security.management.editRole.changeAllPrivileges.noneSelectionLabel', + { + defaultMessage: 'Revoke access to all', + } + ), + }; + default: + return {}; + } + }; + public render() { const button = ( { ); const items = this.props.privileges.map((privilege) => { + const { icon, label } = this.getPrivilegeCopy(privilege.id); return ( { @@ -65,21 +104,24 @@ export class ChangeAllPrivilegesControl extends Component { }} disabled={this.props.disabled} > - {_.upperFirst(privilege.id)} + {label} ); }); items.push( { this.onSelectPrivilege(NO_PRIVILEGE_VALUE); }} disabled={this.props.disabled} + // @ts-expect-error leaving this here so that when https://github.com/elastic/eui/issues/8123 is fixed we remove this comment + css={({ euiTheme }) => ({ color: euiTheme.colors.danger })} > - {_.upperFirst(NO_PRIVILEGE_VALUE)} + {this.getPrivilegeCopy(NO_PRIVILEGE_VALUE).label} ); diff --git a/x-pack/packages/security/ui_components/src/kibana_privilege_table/feature_table.test.tsx b/x-pack/packages/security/ui_components/src/kibana_privilege_table/feature_table.test.tsx index 2c858e7bb6ff6..2ed172a49ad8b 100644 --- a/x-pack/packages/security/ui_components/src/kibana_privilege_table/feature_table.test.tsx +++ b/x-pack/packages/security/ui_components/src/kibana_privilege_table/feature_table.test.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import { EuiAccordion, EuiIconTip } from '@elastic/eui'; +import { EuiAccordion, EuiIconTip, EuiThemeProvider } from '@elastic/eui'; import React from 'react'; import type { KibanaFeature, SubFeatureConfig } from '@kbn/features-plugin/public'; @@ -47,16 +47,18 @@ const setup = (config: TestConfig) => { const onChange = jest.fn(); const onChangeAll = jest.fn(); const wrapper = mountWithIntl( - + + + ); const displayedPrivileges = config.calculateDisplayedPrivileges diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/space_aware_privilege_section/privilege_space_form.test.tsx b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/space_aware_privilege_section/privilege_space_form.test.tsx index 406e102b4529d..fb472191b561d 100644 --- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/space_aware_privilege_section/privilege_space_form.test.tsx +++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/space_aware_privilege_section/privilege_space_form.test.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import { EuiButtonGroup } from '@elastic/eui'; +import { EuiButtonGroup, EuiThemeProvider } from '@elastic/eui'; import React from 'react'; import { @@ -48,21 +48,28 @@ const displaySpaces: Space[] = [ }, ]; +const renderComponent = (props: React.ComponentProps) => { + return mountWithIntl( + + + + ); +}; + describe('PrivilegeSpaceForm', () => { it('renders an empty form when the role contains no Kibana privileges', () => { const role = createRole(); const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures); - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + privilegeIndex: -1, + canCustomizeSubFeaturePrivileges: true, + onChange: jest.fn(), + onCancel: jest.fn(), + }); expect( wrapper.find(EuiButtonGroup).filter('[name="basePrivilegeButtonGroup"]').props().idSelected @@ -110,17 +117,15 @@ describe('PrivilegeSpaceForm', () => { ]); const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures); - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: true, + privilegeIndex: 0, + onChange: jest.fn(), + onCancel: jest.fn(), + }); expect( wrapper.find(EuiButtonGroup).filter('[name="basePrivilegeButtonGroup"]').props().idSelected @@ -178,17 +183,15 @@ describe('PrivilegeSpaceForm', () => { ]); const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures); - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: true, + privilegeIndex: 0, + onChange: jest.fn(), + onCancel: jest.fn(), + }); expect( wrapper.find(EuiButtonGroup).filter('[name="basePrivilegeButtonGroup"]').props().idSelected @@ -249,16 +252,15 @@ describe('PrivilegeSpaceForm', () => { const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures); - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + privilegeIndex: -1, + canCustomizeSubFeaturePrivileges: true, + onChange: jest.fn(), + onCancel: jest.fn(), + }); wrapper.find(SpaceSelector).props().onChange(['*']); @@ -286,17 +288,15 @@ describe('PrivilegeSpaceForm', () => { ]); const kibanaPrivileges = createKibanaPrivileges(kibanaFeatures); - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: true, + privilegeIndex: 0, + onChange: jest.fn(), + onCancel: jest.fn(), + }); expect( wrapper.find(EuiButtonGroup).filter('[name="basePrivilegeButtonGroup"]').props().idSelected @@ -360,17 +360,15 @@ describe('PrivilegeSpaceForm', () => { const onChange = jest.fn(); - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: true, + privilegeIndex: 0, + onChange, + onCancel: jest.fn(), + }); findTestSubject(wrapper, 'changeAllPrivilegesButton').simulate('click'); findTestSubject(wrapper, 'changeAllPrivileges-read').simulate('click'); @@ -418,17 +416,15 @@ describe('PrivilegeSpaceForm', () => { const canCustomize = Symbol('can customize') as unknown as boolean; - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: canCustomize, + privilegeIndex: 0, + onChange, + onCancel: jest.fn(), + }); expect(wrapper.find(FeatureTable).props().canCustomizeSubFeaturePrivileges).toBe(canCustomize); }); @@ -464,17 +460,15 @@ describe('PrivilegeSpaceForm', () => { onChange.mockReset(); }); it('still allow other features privileges to be changed via "change read"', () => { - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: true, + privilegeIndex: 0, + onChange, + onCancel: jest.fn(), + }); findTestSubject(wrapper, 'changeAllPrivilegesButton').simulate('click'); findTestSubject(wrapper, 'changeAllPrivileges-read').simulate('click'); @@ -510,17 +504,15 @@ describe('PrivilegeSpaceForm', () => { }); it('still allow all privileges to be changed via "change all"', () => { - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: true, + privilegeIndex: 0, + onChange, + onCancel: jest.fn(), + }); findTestSubject(wrapper, 'changeAllPrivilegesButton').simulate('click'); findTestSubject(wrapper, 'changeAllPrivileges-all').simulate('click'); @@ -587,17 +579,15 @@ describe('PrivilegeSpaceForm', () => { }); it('still allow all features privileges to be changed via "change read" in foo space', () => { - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: true, + privilegeIndex: 0, + onChange, + onCancel: jest.fn(), + }); findTestSubject(wrapper, 'changeAllPrivilegesButton').simulate('click'); findTestSubject(wrapper, 'changeAllPrivileges-read').simulate('click'); @@ -631,17 +621,15 @@ describe('PrivilegeSpaceForm', () => { }); it('still allow other features privileges to be changed via "change all" in foo space', () => { - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: true, + privilegeIndex: 0, + onChange, + onCancel: jest.fn(), + }); findTestSubject(wrapper, 'changeAllPrivilegesButton').simulate('click'); findTestSubject(wrapper, 'changeAllPrivileges-all').simulate('click'); @@ -692,17 +680,15 @@ describe('PrivilegeSpaceForm', () => { spaces: ['bar'], }, ]); - const wrapper = mountWithIntl( - - ); + const wrapper = renderComponent({ + role: roleAllSpace, + spaces: displaySpaces, + kibanaPrivileges, + canCustomizeSubFeaturePrivileges: true, + privilegeIndex: 0, + onChange, + onCancel: jest.fn(), + }); findTestSubject(wrapper, 'changeAllPrivilegesButton').simulate('click'); findTestSubject(wrapper, 'changeAllPrivileges-all').simulate('click'); diff --git a/x-pack/plugins/spaces/public/management/edit_space/roles/component/space_assign_role_privilege_form.test.tsx b/x-pack/plugins/spaces/public/management/edit_space/roles/component/space_assign_role_privilege_form.test.tsx index 7f15a54e095a6..1c97b9c4d2bc6 100644 --- a/x-pack/plugins/spaces/public/management/edit_space/roles/component/space_assign_role_privilege_form.test.tsx +++ b/x-pack/plugins/spaces/public/management/edit_space/roles/component/space_assign_role_privilege_form.test.tsx @@ -5,6 +5,7 @@ * 2.0. */ +import { EuiThemeProvider } from '@elastic/eui'; import { render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import crypto from 'crypto'; @@ -81,46 +82,48 @@ const renderPrivilegeRolesForm = ({ preSelectedRoles?: Role[]; } = {}) => { return render( - - _), - navigateToUrl: jest.fn(), - license: licenseMock, - isRoleManagementEnabled: true, - capabilities: { - navLinks: {}, - management: {}, - catalogue: {}, - spaces: { manage: true }, - }, - dispatch: dispatchMock, - state: { - roles: new Map(), - fetchRolesError: false, - }, - invokeClient: spacesClientsInvocatorMock, - }} - > - + + _), + navigateToUrl: jest.fn(), + license: licenseMock, + isRoleManagementEnabled: true, + capabilities: { + navLinks: {}, + management: {}, + catalogue: {}, + spaces: { manage: true }, + }, + dispatch: dispatchMock, + state: { + roles: new Map(), + fetchRolesError: false, + }, + invokeClient: spacesClientsInvocatorMock, }} - /> - - + > + + + + ); };