From d733e58e4d7d606116ce53b7604f52225687b09c Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Thu, 11 Nov 2021 16:40:25 -0500 Subject: [PATCH 1/9] add clobe to role mapping and update functionalit in role to match UX --- .../public/management/management_urls.ts | 5 + .../edit_role_mapping_page.tsx | 14 +- .../role_mappings_grid_page.tsx | 188 +++++++++------ .../role_mappings_management_app.tsx | 10 +- .../roles/roles_grid/roles_grid_page.tsx | 214 +++++++++++------- .../public/management/table_utils.tsx | 32 +++ 6 files changed, 308 insertions(+), 155 deletions(-) create mode 100644 x-pack/plugins/security/public/management/table_utils.tsx diff --git a/x-pack/plugins/security/public/management/management_urls.ts b/x-pack/plugins/security/public/management/management_urls.ts index 4a950b2ba1a16..371de4fb7f481 100644 --- a/x-pack/plugins/security/public/management/management_urls.ts +++ b/x-pack/plugins/security/public/management/management_urls.ts @@ -9,3 +9,8 @@ export const EDIT_ROLE_MAPPING_PATH = `/edit`; export const getEditRoleMappingHref = (roleMappingName: string) => `${EDIT_ROLE_MAPPING_PATH}/${encodeURIComponent(roleMappingName)}`; + +export const CLONE_ROLE_MAPPING_PATH = `/clone`; + +export const getCloneRoleMappingHref = (roleMappingName: string) => + `${CLONE_ROLE_MAPPING_PATH}/${encodeURIComponent(roleMappingName)}`; diff --git a/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.tsx b/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.tsx index 00f03b94436ea..123b0de6007f7 100644 --- a/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.tsx @@ -51,6 +51,7 @@ interface State { } interface Props { + action: 'edit' | 'clone'; name?: string; roleMappingsAPI: PublicMethodsOf; rolesAPIClient: PublicMethodsOf; @@ -295,13 +296,17 @@ export class EditRoleMappingPage extends Component { }); }; - private editingExistingRoleMapping = () => typeof this.props.name === 'string'; + private editingExistingRoleMapping = () => + typeof this.props.name === 'string' && this.props.action === 'edit'; + + private cloningExistingRoleMapping = () => + typeof this.props.name === 'string' && this.props.action === 'clone'; private async loadAppData() { try { const [features, roleMapping] = await Promise.all([ this.props.roleMappingsAPI.checkRoleMappingFeatures(), - this.editingExistingRoleMapping() + this.editingExistingRoleMapping() || this.cloningExistingRoleMapping() ? this.props.roleMappingsAPI.getRoleMapping(this.props.name!) : Promise.resolve({ name: '', @@ -327,7 +332,10 @@ export class EditRoleMappingPage extends Component { hasCompatibleRealms, canUseStoredScripts, canUseInlineScripts, - roleMapping, + roleMapping: { + ...roleMapping, + name: this.cloningExistingRoleMapping() ? '' : roleMapping.name, + }, }); } catch (e) { this.props.notifications.toasts.addDanger({ diff --git a/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.tsx b/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.tsx index ec386d75228e8..0a634b68e530d 100644 --- a/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.tsx @@ -6,7 +6,7 @@ */ import { EuiButton, - EuiButtonIcon, + EuiButtonEmpty, EuiCallOut, EuiFlexGroup, EuiFlexItem, @@ -32,18 +32,23 @@ import type { import { reactRouterNavigate } from '../../../../../../../src/plugins/kibana_react/public'; import type { Role, RoleMapping } from '../../../../common/model'; import { DisabledBadge, EnabledBadge } from '../../badges'; -import { EDIT_ROLE_MAPPING_PATH, getEditRoleMappingHref } from '../../management_urls'; +import { + EDIT_ROLE_MAPPING_PATH, + getCloneRoleMappingHref, + getEditRoleMappingHref, +} from '../../management_urls'; import { RoleTableDisplay } from '../../role_table_display'; import type { RolesAPIClient } from '../../roles'; +import { ActionsEuiTableFormatting } from '../../table_utils'; import { DeleteProvider, NoCompatibleRealms, PermissionDenied, SectionLoading, } from '../components'; +import type { DeleteRoleMappings } from '../components/delete_provider/delete_provider'; import type { RoleMappingsAPIClient } from '../role_mappings_api_client'; import { EmptyPrompt } from './empty_prompt'; - interface Props { rolesAPIClient: PublicMethodsOf; roleMappingsAPI: PublicMethodsOf; @@ -260,27 +265,39 @@ export class RoleMappingsGridPage extends Component { }; return ( - { - return { - 'data-test-subj': 'roleMappingRow', - }; + + {(deleteRoleMappingPrompt) => { + return ( + + { + return { + 'data-test-subj': 'roleMappingRow', + }; + }} + /> + + ); }} - /> + ); }; - private getColumnConfig = () => { + private getColumnConfig = (deleteRoleMappingPrompt: DeleteRoleMappings) => { const config = [ { field: 'name', @@ -357,72 +374,97 @@ export class RoleMappingsGridPage extends Component { }), actions: [ { + isPrimary: true, render: (record: RoleMapping) => { + const title = i18n.translate( + 'xpack.security.management.roleMappings.actionCloneTooltip', + { defaultMessage: 'Clone' } + ); + const label = i18n.translate( + 'xpack.security.management.roleMappings.actionCloneAriaLabel', + { + defaultMessage: `Clone '{name}'`, + values: { name: record.name }, + } + ); return ( - - + = 1} {...reactRouterNavigate( this.props.history, - getEditRoleMappingHref(record.name) + getCloneRoleMappingHref(record.name) )} - /> + > + {title} + + + ); + }, + }, + { + render: (record: RoleMapping) => { + const title = i18n.translate( + 'xpack.security.management.roleMappings.actionDeleteTooltip', + { defaultMessage: 'Delete' } + ); + const label = i18n.translate( + 'xpack.security.management.roleMappings.actionDeleteAriaLabel', + { + defaultMessage: `Delete '{name}'`, + values: { name: record.name }, + } + ); + return ( + + = 1} + onClick={() => deleteRoleMappingPrompt([record], this.onRoleMappingsDeleted)} + > + {title} + ); }, }, { + isPrimary: true, render: (record: RoleMapping) => { + const label = i18n.translate( + 'xpack.security.management.roleMappings.actionEditAriaLabel', + { + defaultMessage: `Edit '{name}'`, + values: { name: record.name }, + } + ); + const title = i18n.translate( + 'xpack.security.management.roleMappings.actionEditTooltip', + { defaultMessage: 'Edit' } + ); return ( - - - - {(deleteRoleMappingPrompt) => { - return ( - - - deleteRoleMappingPrompt([record], this.onRoleMappingsDeleted) - } - /> - - ); - }} - - - + + = 1} + {...reactRouterNavigate( + this.props.history, + getEditRoleMappingHref(record.name) + )} + > + {title} + + ); }, }, diff --git a/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.tsx b/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.tsx index 22d09e9e2a678..41e6a9562612d 100644 --- a/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.tsx @@ -56,7 +56,7 @@ export const roleMappingsManagementApp = Object.freeze({ const roleMappingsAPIClient = new RoleMappingsAPIClient(core.http); - const EditRoleMappingsPageWithBreadcrumbs = () => { + const EditRoleMappingsPageWithBreadcrumbs = ({ action }: { action: 'edit' | 'clone' }) => { const { name } = useParams<{ name?: string }>(); // Additional decoding is a workaround for a bug in react-router's version of the `history` module. @@ -64,7 +64,7 @@ export const roleMappingsManagementApp = Object.freeze({ const decodedName = name ? tryDecodeURIComponent(name) : undefined; const breadcrumbObj = - name && decodedName + action === 'edit' && name && decodedName ? { text: decodedName, href: `/edit/${encodeURIComponent(name)}` } : { text: i18n.translate('xpack.security.roleMappings.createBreadcrumb', { @@ -75,6 +75,7 @@ export const roleMappingsManagementApp = Object.freeze({ return ( - + + + + diff --git a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx index 909c5b1193cd9..0aa133164c1f7 100644 --- a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx +++ b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx @@ -8,7 +8,7 @@ import type { EuiBasicTableColumn, EuiSwitchEvent } from '@elastic/eui'; import { EuiButton, - EuiButtonIcon, + EuiButtonEmpty, EuiFlexGroup, EuiFlexItem, EuiInMemoryTable, @@ -17,6 +17,7 @@ import { EuiSpacer, EuiSwitch, EuiText, + EuiToolTip, } from '@elastic/eui'; import _ from 'lodash'; import React, { Component } from 'react'; @@ -36,6 +37,7 @@ import { isRoleReserved, } from '../../../../common/model'; import { DeprecatedBadge, DisabledBadge, ReservedBadge } from '../../badges'; +import { ActionsEuiTableFormatting } from '../../table_utils'; import type { RolesAPIClient } from '../roles_api_client'; import { ConfirmDelete } from './confirm_delete'; import { PermissionDenied } from './permission_denied'; @@ -129,53 +131,55 @@ export class RolesGridPage extends Component { /> ) : null} - !role.metadata || !role.metadata._reserved, - selectableMessage: (selectable: boolean) => (!selectable ? 'Role is reserved' : ''), - onSelectionChange: (selection: Role[]) => this.setState({ selection }), - }} - pagination={{ - initialPageSize: 20, - pageSizeOptions: [10, 20, 30, 50, 100], - }} - items={this.state.visibleRoles} - loading={roles.length === 0} - search={{ - toolsLeft: this.renderToolsLeft(), - toolsRight: this.renderToolsRight(), - box: { - incremental: true, - 'data-test-subj': 'searchRoles', - }, - onChange: (query: Record) => { - this.setState({ - filter: query.queryText, - visibleRoles: this.getVisibleRoles( - this.state.roles, - query.queryText, - this.state.includeReservedRoles - ), - }); - }, - }} - sorting={{ - sort: { - field: 'name', - direction: 'asc', - }, - }} - rowProps={() => { - return { - 'data-test-subj': 'roleRow', - }; - }} - isSelectable - /> + + !role.metadata || !role.metadata._reserved, + selectableMessage: (selectable: boolean) => (!selectable ? 'Role is reserved' : ''), + onSelectionChange: (selection: Role[]) => this.setState({ selection }), + }} + pagination={{ + initialPageSize: 20, + pageSizeOptions: [10, 20, 30, 50, 100], + }} + items={this.state.visibleRoles} + loading={roles.length === 0} + search={{ + toolsLeft: this.renderToolsLeft(), + toolsRight: this.renderToolsRight(), + box: { + incremental: true, + 'data-test-subj': 'searchRoles', + }, + onChange: (query: Record) => { + this.setState({ + filter: query.queryText, + visibleRoles: this.getVisibleRoles( + this.state.roles, + query.queryText, + this.state.includeReservedRoles + ), + }); + }, + }} + sorting={{ + sort: { + field: 'name', + direction: 'asc', + }, + }} + rowProps={() => { + return { + 'data-test-subj': 'roleRow', + }; + }} + isSelectable + /> + ); }; @@ -219,48 +223,98 @@ export class RolesGridPage extends Component { width: '150px', actions: [ { - available: (role: Role) => !isRoleReadOnly(role), + available: (role: Role) => !isRoleReserved(role), + isPrimary: true, render: (role: Role) => { - const title = i18n.translate('xpack.security.management.roles.editRoleActionName', { - defaultMessage: `Edit {roleName}`, + const title = i18n.translate('xpack.security.management.roles.cloneRoleActionName', { + defaultMessage: `Clone`, + }); + + const label = i18n.translate('xpack.security.management.roles.cloneRoleActionLabel', { + defaultMessage: `Clone {roleName}`, values: { roleName: role.name }, }); return ( - + + = 1} + iconType={'copy'} + {...reactRouterNavigate( + this.props.history, + getRoleManagementHref('clone', role.name) + )} + > + {title} + + ); }, }, { - available: (role: Role) => !isRoleReserved(role), + available: (role: Role) => !role.metadata || !role.metadata._reserved, render: (role: Role) => { - const title = i18n.translate('xpack.security.management.roles.cloneRoleActionName', { - defaultMessage: `Clone {roleName}`, + const title = i18n.translate('xpack.security.management.roles.deleteRoleActionName', { + defaultMessage: `Delete`, + }); + + const label = i18n.translate( + 'xpack.security.management.roles.deleteRoleActionLabel', + { + defaultMessage: `Delete {roleName}`, + values: { roleName: role.name }, + } + ); + + return ( + + = 1} + iconType={'trash'} + onClick={() => this.deleteOneRole(role)} + > + {title} + + + ); + }, + }, + { + available: (role: Role) => !isRoleReadOnly(role), + enable: () => this.state.selection.length === 0, + isPrimary: true, + render: (role: Role) => { + const title = i18n.translate('xpack.security.management.roles.editRoleActionName', { + defaultMessage: `Clone`, + }); + + const label = i18n.translate('xpack.security.management.roles.editRoleActionLabel', { + defaultMessage: `Edit {roleName}`, values: { roleName: role.name }, }); return ( - + + = 1} + iconType={'pencil'} + {...reactRouterNavigate( + this.props.history, + getRoleManagementHref('edit', role.name) + )} + > + {title} + + ); }, }, @@ -337,6 +391,13 @@ export class RolesGridPage extends Component { this.loadRoles(); }; + private deleteOneRole = (roleToDelete: Role) => { + this.setState({ + selection: [roleToDelete], + showDeleteConfirmation: true, + }); + }; + private async loadRoles() { try { const roles = await this.props.rolesAPIClient.getRoles(); @@ -385,6 +446,7 @@ export class RolesGridPage extends Component { ); } + private renderToolsRight() { return ( ( + ({ children }) => ( +
+ {children} +
+ ) +); From 916bc2a1c486aae2c4d7642702155512335dc142 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Mon, 15 Nov 2021 10:34:01 -0500 Subject: [PATCH 2/9] fix some I --- .../edit_role_mapping/edit_role_mapping_page.test.tsx | 1 + x-pack/test/functional/apps/security/role_mappings.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.test.tsx b/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.test.tsx index b624da2cd88b4..af7a2fb8d5240 100644 --- a/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.test.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/edit_role_mapping/edit_role_mapping_page.test.tsx @@ -39,6 +39,7 @@ describe('EditRoleMappingPage', () => { return mountWithIntl( { }); it('allows a role mapping to be deleted', async () => { - await testSubjects.click(`deleteRoleMappingButton-new_role_mapping`); + await testSubjects.click('euiCollapsedItemActionsButton'); + await testSubjects.click('deleteRoleMappingButton-new_role_mapping'); await testSubjects.click('confirmModalConfirmButton'); await testSubjects.existOrFail('deletedRoleMappingSuccessToast'); }); From 290322f68b626d3821055ba15ceb7ac823166433 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Mon, 15 Nov 2021 14:54:16 -0500 Subject: [PATCH 3/9] fix jest test + fix table selection when canceling deletion --- .../delete_provider/delete_provider.tsx | 18 +++++++++++++++--- .../role_mappings_grid_page.test.tsx | 1 + .../role_mappings_grid_page.tsx | 15 ++++++++++++++- .../role_mappings_management_app.test.tsx | 4 ++-- .../roles/roles_grid/roles_grid_page.test.tsx | 14 ++++++++------ .../roles/roles_grid/roles_grid_page.tsx | 10 +++++++--- 6 files changed, 47 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/security/public/management/role_mappings/components/delete_provider/delete_provider.tsx b/x-pack/plugins/security/public/management/role_mappings/components/delete_provider/delete_provider.tsx index f9194860ddded..9130a6ba1830c 100644 --- a/x-pack/plugins/security/public/management/role_mappings/components/delete_provider/delete_provider.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/components/delete_provider/delete_provider.tsx @@ -24,10 +24,12 @@ interface Props { export type DeleteRoleMappings = ( roleMappings: RoleMapping[], - onSuccess?: OnSuccessCallback + onSuccess?: OnSuccessCallback, + onCancel?: OnCancelCallback ) => void; type OnSuccessCallback = (deletedRoleMappings: string[]) => void; +type OnCancelCallback = () => void; export const DeleteProvider: React.FunctionComponent = ({ roleMappingsAPI, @@ -39,10 +41,12 @@ export const DeleteProvider: React.FunctionComponent = ({ const [isDeleteInProgress, setIsDeleteInProgress] = useState(false); const onSuccessCallback = useRef(null); + const onCancelCallback = useRef(null); const deleteRoleMappingsPrompt: DeleteRoleMappings = ( roleMappingsToDelete, - onSuccess = () => undefined + onSuccess = () => undefined, + onCancel = () => undefined ) => { if (!roleMappingsToDelete || !roleMappingsToDelete.length) { throw new Error('No Role Mappings specified for delete'); @@ -50,6 +54,7 @@ export const DeleteProvider: React.FunctionComponent = ({ setIsModalOpen(true); setRoleMappings(roleMappingsToDelete); onSuccessCallback.current = onSuccess; + onCancelCallback.current = onCancel; }; const closeModal = () => { @@ -57,6 +62,13 @@ export const DeleteProvider: React.FunctionComponent = ({ setRoleMappings([]); }; + const handleCancelModel = () => { + closeModal(); + if (onCancelCallback.current) { + onCancelCallback.current(); + } + }; + const deleteRoleMappings = async () => { let result; @@ -161,7 +173,7 @@ export const DeleteProvider: React.FunctionComponent = ({ } ) } - onCancel={closeModal} + onCancel={handleCancelModel} onConfirm={deleteRoleMappings} cancelButtonText={i18n.translate( 'xpack.security.management.roleMappings.deleteRoleMapping.confirmModal.cancelButtonLabel', diff --git a/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.test.tsx b/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.test.tsx index 5f237e6504d32..e16954f504f5c 100644 --- a/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.test.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.test.tsx @@ -188,6 +188,7 @@ describe('RoleMappingsGridPage', () => { expect(roleMappingsAPI.getRoleMappings).toHaveBeenCalledTimes(1); expect(roleMappingsAPI.deleteRoleMappings).not.toHaveBeenCalled(); + findTestSubject(wrapper, `euiCollapsedItemActionsButton`).simulate('click'); findTestSubject(wrapper, `deleteRoleMappingButton-some-realm`).simulate('click'); expect(findTestSubject(wrapper, 'deleteRoleMappingConfirmationModal')).toHaveLength(1); diff --git a/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.tsx b/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.tsx index 0a634b68e530d..373f3366f36c8 100644 --- a/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.tsx @@ -68,6 +68,7 @@ interface State { } export class RoleMappingsGridPage extends Component { + private tableRef: React.RefObject>; constructor(props: any) { super(props); this.state = { @@ -78,6 +79,7 @@ export class RoleMappingsGridPage extends Component { selectedItems: [], error: undefined, }; + this.tableRef = React.createRef(); } public componentDidMount() { @@ -229,7 +231,13 @@ export class RoleMappingsGridPage extends Component { {(deleteRoleMappingsPrompt) => { return ( deleteRoleMappingsPrompt(selectedItems, this.onRoleMappingsDeleted)} + onClick={() => + deleteRoleMappingsPrompt( + selectedItems, + this.onRoleMappingsDeleted, + this.onRoleMappingsDeleteCancel + ) + } color="danger" data-test-subj="bulkDeleteActionButton" > @@ -284,6 +292,7 @@ export class RoleMappingsGridPage extends Component { loading={loadState === 'loadingTable'} message={message} isSelectable={true} + ref={this.tableRef} rowProps={() => { return { 'data-test-subj': 'roleMappingRow', @@ -480,6 +489,10 @@ export class RoleMappingsGridPage extends Component { } }; + private onRoleMappingsDeleteCancel = () => { + this.tableRef.current?.setSelection([]); + }; + private async checkPrivileges() { try { const { canManageRoleMappings, hasCompatibleRealms } = diff --git a/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.test.tsx b/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.test.tsx index f6d17327b7118..892c7940675d3 100644 --- a/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.test.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/role_mappings_management_app.test.tsx @@ -100,7 +100,7 @@ describe('roleMappingsManagementApp', () => { expect(docTitle.reset).not.toHaveBeenCalled(); expect(container).toMatchInlineSnapshot(`
- Role Mapping Edit Page: {"roleMappingsAPI":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{},"externalUrl":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{},"externalUrl":{}}},"notifications":{"toasts":{}},"docLinks":{},"history":{"action":"PUSH","length":1,"location":{"pathname":"/edit","search":"","hash":""}}} + Role Mapping Edit Page: {"action":"edit","roleMappingsAPI":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{},"externalUrl":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{},"externalUrl":{}}},"notifications":{"toasts":{}},"docLinks":{},"history":{"action":"PUSH","length":1,"location":{"pathname":"/edit","search":"","hash":""}}}
`); @@ -128,7 +128,7 @@ describe('roleMappingsManagementApp', () => { expect(docTitle.reset).not.toHaveBeenCalled(); expect(container).toMatchInlineSnapshot(`
- Role Mapping Edit Page: {"name":"role@mapping","roleMappingsAPI":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{},"externalUrl":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{},"externalUrl":{}}},"notifications":{"toasts":{}},"docLinks":{},"history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/role@mapping","search":"","hash":""}}} + Role Mapping Edit Page: {"action":"edit","name":"role@mapping","roleMappingsAPI":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{},"externalUrl":{}}},"rolesAPIClient":{"http":{"basePath":{"basePath":"","serverBasePath":""},"anonymousPaths":{},"externalUrl":{}}},"notifications":{"toasts":{}},"docLinks":{},"history":{"action":"PUSH","length":1,"location":{"pathname":"/edit/role@mapping","search":"","hash":""}}}
`); diff --git a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.test.tsx b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.test.tsx index 9194fea271442..aa507cf823eff 100644 --- a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.test.tsx +++ b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.test.tsx @@ -144,31 +144,33 @@ describe('', () => { expect(wrapper.find(PermissionDenied)).toHaveLength(0); - let editButton = wrapper.find('EuiButtonIcon[data-test-subj="edit-role-action-test-role-1"]'); + let editButton = wrapper.find('EuiButtonEmpty[data-test-subj="edit-role-action-test-role-1"]'); expect(editButton).toHaveLength(1); expect(editButton.prop('href')).toBe('/edit/test-role-1'); editButton = wrapper.find( - 'EuiButtonIcon[data-test-subj="edit-role-action-special%chars%role"]' + 'EuiButtonEmpty[data-test-subj="edit-role-action-special%chars%role"]' ); expect(editButton).toHaveLength(1); expect(editButton.prop('href')).toBe('/edit/special%25chars%25role'); - let cloneButton = wrapper.find('EuiButtonIcon[data-test-subj="clone-role-action-test-role-1"]'); + let cloneButton = wrapper.find( + 'EuiButtonEmpty[data-test-subj="clone-role-action-test-role-1"]' + ); expect(cloneButton).toHaveLength(1); expect(cloneButton.prop('href')).toBe('/clone/test-role-1'); cloneButton = wrapper.find( - 'EuiButtonIcon[data-test-subj="clone-role-action-special%chars%role"]' + 'EuiButtonEmpty[data-test-subj="clone-role-action-special%chars%role"]' ); expect(cloneButton).toHaveLength(1); expect(cloneButton.prop('href')).toBe('/clone/special%25chars%25role'); expect( - wrapper.find('EuiButtonIcon[data-test-subj="edit-role-action-disabled-role"]') + wrapper.find('EuiButtonEmpty[data-test-subj="edit-role-action-disabled-role"]') ).toHaveLength(1); expect( - wrapper.find('EuiButtonIcon[data-test-subj="clone-role-action-disabled-role"]') + wrapper.find('EuiButtonEmpty[data-test-subj="clone-role-action-disabled-role"]') ).toHaveLength(1); }); diff --git a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx index 0aa133164c1f7..04985a9dae20d 100644 --- a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx +++ b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx @@ -63,6 +63,7 @@ const getRoleManagementHref = (action: 'edit' | 'clone', roleName?: string) => { }; export class RolesGridPage extends Component { + private tableRef: React.RefObject>; constructor(props: Props) { super(props); this.state = { @@ -74,6 +75,7 @@ export class RolesGridPage extends Component { permissionDenied: false, includeReservedRoles: true, }; + this.tableRef = React.createRef(); } public componentDidMount() { @@ -172,9 +174,10 @@ export class RolesGridPage extends Component { direction: 'asc', }, }} - rowProps={() => { + ref={this.tableRef} + rowProps={(role: Role) => { return { - 'data-test-subj': 'roleRow', + 'data-test-subj': `roleRow`, }; }} isSelectable @@ -463,6 +466,7 @@ export class RolesGridPage extends Component { ); } private onCancelDelete = () => { - this.setState({ showDeleteConfirmation: false }); + this.setState({ showDeleteConfirmation: false, selection: [] }); + this.tableRef.current?.setSelection([]); }; } From 5305afbcf0781fe1bc4c122cbc0f956ddbc5dced Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Mon, 15 Nov 2021 16:06:22 -0500 Subject: [PATCH 4/9] add tests around clone action in role mapping --- .../role_mappings_grid_page.test.tsx | 56 ++++++++++++++++++- .../functional/apps/security/role_mappings.ts | 9 +++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.test.tsx b/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.test.tsx index e16954f504f5c..d9009d49b592b 100644 --- a/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.test.tsx +++ b/x-pack/plugins/security/public/management/role_mappings/role_mappings_grid/role_mappings_grid_page.test.tsx @@ -10,7 +10,7 @@ import { act } from '@testing-library/react'; import React from 'react'; import { findTestSubject, mountWithIntl, nextTick } from '@kbn/test/jest'; -import type { CoreStart, ScopedHistory } from 'src/core/public'; +import type { CoreStart } from 'src/core/public'; import { coreMock, scopedHistoryMock } from 'src/core/public/mocks'; import { KibanaContextProvider } from 'src/plugins/kibana_react/public'; @@ -21,7 +21,7 @@ import { EmptyPrompt } from './empty_prompt'; import { RoleMappingsGridPage } from './role_mappings_grid_page'; describe('RoleMappingsGridPage', () => { - let history: ScopedHistory; + let history: ReturnType; let coreStart: CoreStart; const renderView = ( @@ -44,6 +44,7 @@ describe('RoleMappingsGridPage', () => { beforeEach(() => { history = scopedHistoryMock.create(); + history.createHref.mockImplementation((location) => location.pathname!); coreStart = coreMock.createStart(); }); @@ -247,4 +248,55 @@ describe('RoleMappingsGridPage', () => { `"The kibana_user role is deprecated. I don't like you."` ); }); + + it('renders role mapping actions as appropriate', async () => { + const roleMappingsAPI = roleMappingsAPIClientMock.create(); + roleMappingsAPI.getRoleMappings.mockResolvedValue([ + { + name: 'some-realm', + enabled: true, + roles: ['superuser'], + rules: { field: { username: '*' } }, + }, + ]); + roleMappingsAPI.checkRoleMappingFeatures.mockResolvedValue({ + canManageRoleMappings: true, + hasCompatibleRealms: true, + }); + roleMappingsAPI.deleteRoleMappings.mockResolvedValue([ + { + name: 'some-realm', + success: true, + }, + ]); + + const wrapper = renderView(roleMappingsAPI); + await nextTick(); + wrapper.update(); + + const editButton = wrapper.find( + 'EuiButtonEmpty[data-test-subj="editRoleMappingButton-some-realm"]' + ); + expect(editButton).toHaveLength(1); + expect(editButton.prop('href')).toBe('/edit/some-realm'); + + const cloneButton = wrapper.find( + 'EuiButtonEmpty[data-test-subj="cloneRoleMappingButton-some-realm"]' + ); + expect(cloneButton).toHaveLength(1); + expect(cloneButton.prop('href')).toBe('/clone/some-realm'); + + const actionMenuButton = wrapper.find( + 'EuiButtonIcon[data-test-subj="euiCollapsedItemActionsButton"]' + ); + expect(actionMenuButton).toHaveLength(1); + + actionMenuButton.simulate('click'); + wrapper.update(); + + const deleteButton = wrapper.find( + 'EuiButtonEmpty[data-test-subj="deleteRoleMappingButton-some-realm"]' + ); + expect(deleteButton).toHaveLength(1); + }); }); diff --git a/x-pack/test/functional/apps/security/role_mappings.ts b/x-pack/test/functional/apps/security/role_mappings.ts index 47a58139c81c9..54c92c4815b5d 100644 --- a/x-pack/test/functional/apps/security/role_mappings.ts +++ b/x-pack/test/functional/apps/security/role_mappings.ts @@ -163,6 +163,15 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { } }); + it('allows a role mapping to be cloned', async () => { + await testSubjects.click('cloneRoleMappingButton-a_enabled_role_mapping'); + await testSubjects.setValue('roleMappingFormNameInput', 'cloned_role_mapping'); + await testSubjects.click('saveRoleMappingButton'); + await testSubjects.existOrFail('savedRoleMappingSuccessToast'); + const rows = await testSubjects.findAll('roleMappingRow'); + expect(rows.length).to.eql(mappings.length + 1); + }); + it('allows a role mapping to be edited', async () => { await testSubjects.click('roleMappingName'); await testSubjects.click('saveRoleMappingButton'); From e27065f47f6f6a8ba335042e35581b3a71cc0727 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Mon, 15 Nov 2021 16:11:47 -0500 Subject: [PATCH 5/9] fix i18n --- x-pack/plugins/translations/translations/ja-JP.json | 4 ++-- x-pack/plugins/translations/translations/zh-CN.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index eee055e75eb8f..8fc27fe51e195 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -19893,7 +19893,7 @@ "xpack.security.management.roleMappings.rolesColumnName": "ロール", "xpack.security.management.roleMappingsTitle": "ロールマッピング", "xpack.security.management.roles.actionsColumnName": "アクション", - "xpack.security.management.roles.cloneRoleActionName": "{roleName} を複製", + "xpack.security.management.roles.cloneRoleActionName": "を複製", "xpack.security.management.roles.confirmDelete.cancelButtonLabel": "キャンセル", "xpack.security.management.roles.confirmDelete.deleteButtonLabel": "削除", "xpack.security.management.roles.confirmDelete.removingRolesDescription": "これらのロールを削除しようとしています:", @@ -19904,7 +19904,7 @@ "xpack.security.management.roles.deleteSelectedRolesButtonLabel": "ロール {numSelected} {numSelected, plural, one { } other {}} を削除しました", "xpack.security.management.roles.deletingRolesWarningMessage": "この操作は元に戻すことができません。", "xpack.security.management.roles.deniedPermissionTitle": "ロールを管理するにはパーミッションが必要です", - "xpack.security.management.roles.editRoleActionName": "{roleName} を編集", + "xpack.security.management.roles.editRoleActionName": "を編集", "xpack.security.management.roles.fetchingRolesErrorMessage": "ロールの取得中にエラーが発生:{message}", "xpack.security.management.roles.nameColumnName": "ロール", "xpack.security.management.roles.noIndexPatternsPermission": "利用可能なインデックスパターンのリストへのアクセス権が必要です。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 3a5d909e12049..a566914cfe652 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -20188,7 +20188,7 @@ "xpack.security.management.roleMappings.roleTemplates": "{templateCount, plural, other {# 个角色模板}}已定义", "xpack.security.management.roleMappingsTitle": "角色映射", "xpack.security.management.roles.actionsColumnName": "操作", - "xpack.security.management.roles.cloneRoleActionName": "克隆 {roleName}", + "xpack.security.management.roles.cloneRoleActionName": "克隆", "xpack.security.management.roles.confirmDelete.cancelButtonLabel": "取消", "xpack.security.management.roles.confirmDelete.deleteButtonLabel": "删除", "xpack.security.management.roles.confirmDelete.removingRolesDescription": "您即将删除以下角色:", @@ -20199,7 +20199,7 @@ "xpack.security.management.roles.deleteSelectedRolesButtonLabel": "删除 {numSelected} 个角色{numSelected, plural, other {}}", "xpack.security.management.roles.deletingRolesWarningMessage": "此操作无法撤消。", "xpack.security.management.roles.deniedPermissionTitle": "您需要用于管理角色的权限", - "xpack.security.management.roles.editRoleActionName": "编辑 {roleName}", + "xpack.security.management.roles.editRoleActionName": "编辑", "xpack.security.management.roles.fetchingRolesErrorMessage": "获取用户时出错:{message}", "xpack.security.management.roles.nameColumnName": "角色", "xpack.security.management.roles.noIndexPatternsPermission": "您需要访问可用索引模式列表的权限。", From 3ac3456dfff3677d327c0a920ac07ea538461dad Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Tue, 16 Nov 2021 12:14:11 -0500 Subject: [PATCH 6/9] remove i18n --- x-pack/plugins/translations/translations/ja-JP.json | 2 -- x-pack/plugins/translations/translations/zh-CN.json | 2 -- 2 files changed, 4 deletions(-) diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 8fc27fe51e195..493b718d3a6c5 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -19893,7 +19893,6 @@ "xpack.security.management.roleMappings.rolesColumnName": "ロール", "xpack.security.management.roleMappingsTitle": "ロールマッピング", "xpack.security.management.roles.actionsColumnName": "アクション", - "xpack.security.management.roles.cloneRoleActionName": "を複製", "xpack.security.management.roles.confirmDelete.cancelButtonLabel": "キャンセル", "xpack.security.management.roles.confirmDelete.deleteButtonLabel": "削除", "xpack.security.management.roles.confirmDelete.removingRolesDescription": "これらのロールを削除しようとしています:", @@ -19904,7 +19903,6 @@ "xpack.security.management.roles.deleteSelectedRolesButtonLabel": "ロール {numSelected} {numSelected, plural, one { } other {}} を削除しました", "xpack.security.management.roles.deletingRolesWarningMessage": "この操作は元に戻すことができません。", "xpack.security.management.roles.deniedPermissionTitle": "ロールを管理するにはパーミッションが必要です", - "xpack.security.management.roles.editRoleActionName": "を編集", "xpack.security.management.roles.fetchingRolesErrorMessage": "ロールの取得中にエラーが発生:{message}", "xpack.security.management.roles.nameColumnName": "ロール", "xpack.security.management.roles.noIndexPatternsPermission": "利用可能なインデックスパターンのリストへのアクセス権が必要です。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index a566914cfe652..6391f40fdba9e 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -20188,7 +20188,6 @@ "xpack.security.management.roleMappings.roleTemplates": "{templateCount, plural, other {# 个角色模板}}已定义", "xpack.security.management.roleMappingsTitle": "角色映射", "xpack.security.management.roles.actionsColumnName": "操作", - "xpack.security.management.roles.cloneRoleActionName": "克隆", "xpack.security.management.roles.confirmDelete.cancelButtonLabel": "取消", "xpack.security.management.roles.confirmDelete.deleteButtonLabel": "删除", "xpack.security.management.roles.confirmDelete.removingRolesDescription": "您即将删除以下角色:", @@ -20199,7 +20198,6 @@ "xpack.security.management.roles.deleteSelectedRolesButtonLabel": "删除 {numSelected} 个角色{numSelected, plural, other {}}", "xpack.security.management.roles.deletingRolesWarningMessage": "此操作无法撤消。", "xpack.security.management.roles.deniedPermissionTitle": "您需要用于管理角色的权限", - "xpack.security.management.roles.editRoleActionName": "编辑", "xpack.security.management.roles.fetchingRolesErrorMessage": "获取用户时出错:{message}", "xpack.security.management.roles.nameColumnName": "角色", "xpack.security.management.roles.noIndexPatternsPermission": "您需要访问可用索引模式列表的权限。", From f7f4dc902f5429ca136db557c074bf582c41e596 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Tue, 16 Nov 2021 12:17:34 -0500 Subject: [PATCH 7/9] review Greg I --- .../security/public/management/table_utils.tsx | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security/public/management/table_utils.tsx b/x-pack/plugins/security/public/management/table_utils.tsx index ce90c6e61d95c..102d15065a260 100644 --- a/x-pack/plugins/security/public/management/table_utils.tsx +++ b/x-pack/plugins/security/public/management/table_utils.tsx @@ -8,25 +8,17 @@ import { css } from '@emotion/react'; import type { ReactNode } from 'react'; import React from 'react'; -const RemoveButtonLabelInActionsCellCss = ` +const RemoveButtonLabelInActionsCellCss = css` .euiTableRowCell--hasActions .euiButtonEmpty .euiButtonContent { padding: 0px 0px; .euiButtonEmpty__text { - display: none; - } + display: none; + } } `; interface ActionsEuiTableFormattingProps { children: ReactNode; } export const ActionsEuiTableFormatting = React.memo( - ({ children }) => ( -
- {children} -
- ) + ({ children }) =>
{children}
); From ed2877cedcab29153b7867a72d97e0a1201a6c1b Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Wed, 17 Nov 2021 11:43:55 -0500 Subject: [PATCH 8/9] fix styling + name --- .../roles/roles_grid/roles_grid_page.tsx | 2 +- .../public/management/table_utils.tsx | 23 +++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx index 04985a9dae20d..d34a8bfea27bf 100644 --- a/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx +++ b/x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx @@ -294,7 +294,7 @@ export class RolesGridPage extends Component { isPrimary: true, render: (role: Role) => { const title = i18n.translate('xpack.security.management.roles.editRoleActionName', { - defaultMessage: `Clone`, + defaultMessage: `Edit`, }); const label = i18n.translate('xpack.security.management.roles.editRoleActionLabel', { diff --git a/x-pack/plugins/security/public/management/table_utils.tsx b/x-pack/plugins/security/public/management/table_utils.tsx index 102d15065a260..05b09270f994a 100644 --- a/x-pack/plugins/security/public/management/table_utils.tsx +++ b/x-pack/plugins/security/public/management/table_utils.tsx @@ -8,17 +8,22 @@ import { css } from '@emotion/react'; import type { ReactNode } from 'react'; import React from 'react'; -const RemoveButtonLabelInActionsCellCss = css` - .euiTableRowCell--hasActions .euiButtonEmpty .euiButtonContent { - padding: 0px 0px; - .euiButtonEmpty__text { - display: none; - } - } -`; interface ActionsEuiTableFormattingProps { children: ReactNode; } export const ActionsEuiTableFormatting = React.memo( - ({ children }) =>
{children}
+ ({ children }) => ( +
+ {children} +
+ ) ); From 820c1d7d4c7f3597099e65545f073aa74af2699f Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Wed, 17 Nov 2021 12:26:04 -0500 Subject: [PATCH 9/9] add explaination --- x-pack/plugins/security/public/management/table_utils.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/x-pack/plugins/security/public/management/table_utils.tsx b/x-pack/plugins/security/public/management/table_utils.tsx index 05b09270f994a..3f240daf3bd03 100644 --- a/x-pack/plugins/security/public/management/table_utils.tsx +++ b/x-pack/plugins/security/public/management/table_utils.tsx @@ -11,6 +11,14 @@ import React from 'react'; interface ActionsEuiTableFormattingProps { children: ReactNode; } + +/* + * Notes to future engineer: + * We created this component because as this time EUI actions table where not allowing to pass + * props href on an action. In our case, we want our actions to work with href + * and onClick. Then the problem is that the design did not match with EUI example, therefore + * we are doing some css magic to only have icon showing up when user is hovering a row + */ export const ActionsEuiTableFormatting = React.memo( ({ children }) => (