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.

Sign up for GitHub

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

Jump to bottom

[Enterprise Search] Refactor RoleMappingsTable to use EuiInMemoryTable #101918

Merged
merged 2 commits into from
Jun 10, 2021
Merged

[Enterprise Search] Refactor RoleMappingsTable to use EuiInMemoryTable #101918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1a0dac
Add shared actions component
scottybollinger Jun 10, 2021
78fb896
Refactor RoleMappingsTable to use EuiInMemoryTable
scottybollinger Jun 10, 2021
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
5 changes: 5 additions & 0 deletions x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,8 @@ export const ROLE_MAPPINGS_HEADING_BUTTON = i18n.translate(
'xpack.enterpriseSearch.roleMapping.roleMappingsHeadingButton',
{ defaultMessage: 'Create a new role mapping' }
);

export const ROLE_MAPPINGS_NO_RESULTS_MESSAGE = i18n.translate(
'xpack.enterpriseSearch.roleMapping.noResults.message',
{ defaultMessage: 'Create a new role mapping' }
);
1 change: 1 addition & 0 deletions x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export { RoleOptionLabel } from './role_option_label';
export { RoleSelector } from './role_selector';
export { RoleMappingFlyout } from './role_mapping_flyout';
export { RoleMappingsHeading } from './role_mappings_heading';
export { UsersAndRolesRowActions } from './users_and_roles_row_actions';
54 changes: 22 additions & 32 deletions ...ns/enterprise_search/public/applications/shared/role_mapping/role_mappings_table.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import { wsRoleMapping, asRoleMapping } from './__mocks__/roles';

import React from 'react';

import { shallow } from 'enzyme';
import { mount } from 'enzyme';

import { EuiFieldSearch, EuiTableRow } from '@elastic/eui';
import { EuiInMemoryTable, EuiTableHeaderCell } from '@elastic/eui';

import { ALL_LABEL, ANY_AUTH_PROVIDER_OPTION_LABEL } from './constants';

import { RoleMappingsTable } from './role_mappings_table';
import { UsersAndRolesRowActions } from './users_and_roles_row_actions';

describe('RoleMappingsTable', () => {
const initializeRoleMapping = jest.fn();
Expand All @@ -41,55 +42,44 @@ describe('RoleMappingsTable', () => {
handleDeleteMapping,
};

it('renders', () => {
const wrapper = shallow(<RoleMappingsTable {...props} />);
it('renders with "shouldShowAuthProvider" true', () => {
const wrapper = mount(<RoleMappingsTable {...props} />);

expect(wrapper.find(EuiFieldSearch)).toHaveLength(1);
expect(wrapper.find(EuiTableRow)).toHaveLength(1);
expect(wrapper.find(EuiInMemoryTable)).toHaveLength(1);
expect(wrapper.find(EuiTableHeaderCell)).toHaveLength(6);
});

it('renders auth provider display names', () => {
const wrapper = shallow(<RoleMappingsTable {...props} />);
it('renders with "shouldShowAuthProvider" false', () => {
const wrapper = mount(<RoleMappingsTable {...props} shouldShowAuthProvider={false} />);

expect(wrapper.find('[data-test-subj="AuthProviderDisplay"]').prop('children')).toEqual(
`${ANY_AUTH_PROVIDER_OPTION_LABEL}, other_auth`
);
expect(wrapper.find(EuiInMemoryTable)).toHaveLength(1);
expect(wrapper.find(EuiTableHeaderCell)).toHaveLength(5);
});

it('handles input change', () => {
const wrapper = shallow(<RoleMappingsTable {...props} />);
const input = wrapper.find(EuiFieldSearch);
const value = 'Query';
input.simulate('change', { target: { value } });
it('renders auth provider display names', () => {
const wrapper = mount(<RoleMappingsTable {...props} />);

expect(wrapper.find(EuiTableRow)).toHaveLength(0);
expect(wrapper.find('[data-test-subj="AuthProviderDisplayValue"]').prop('children')).toEqual(
`${ANY_AUTH_PROVIDER_OPTION_LABEL}, other_auth`
);
});

it('handles manage click', () => {
const wrapper = shallow(<RoleMappingsTable {...props} />);
wrapper.find('[data-test-subj="ManageButton"]').simulate('click');
const wrapper = mount(<RoleMappingsTable {...props} />);
wrapper.find(UsersAndRolesRowActions).prop('onManageClick')();

expect(initializeRoleMapping).toHaveBeenCalled();
});

it('handles delete click', () => {
const wrapper = shallow(<RoleMappingsTable {...props} />);
wrapper.find('[data-test-subj="DeleteButton"]').simulate('click');
const wrapper = mount(<RoleMappingsTable {...props} />);
wrapper.find(UsersAndRolesRowActions).prop('onDeleteClick')();

expect(handleDeleteMapping).toHaveBeenCalled();
});

it('handles input change with special chars', () => {
const wrapper = shallow(<RoleMappingsTable {...props} />);
const input = wrapper.find(EuiFieldSearch);
const value = '*//username';
input.simulate('change', { target: { value } });

expect(wrapper.find(EuiTableRow)).toHaveLength(1);
});

it('shows default message when "accessAllEngines" is true', () => {
const wrapper = shallow(
const wrapper = mount(
<RoleMappingsTable {...props} roleMappings={[asRoleMapping as any]} accessItemKey="engines" />
);

Expand All @@ -100,7 +90,7 @@ describe('RoleMappingsTable', () => {
const noItemsRoleMapping = { ...asRoleMapping, engines: [] };
noItemsRoleMapping.accessAllEngines = false;

const wrapper = shallow(
const wrapper = mount(
<RoleMappingsTable
{...props}
roleMappings={[noItemsRoleMapping as any]}
Expand Down
208 changes: 92 additions & 116 deletions ...plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,12 @@
* 2.0.
*/

import React, { Fragment, useState } from 'react';
import React, { Fragment } from 'react';

import {
EuiButtonIcon,
EuiFieldSearch,
EuiIconTip,
EuiSpacer,
EuiTable,
EuiTableBody,
EuiTableHeader,
EuiTableHeaderCell,
EuiTableRow,
EuiTableRowCell,
EuiTextColor,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { EuiTextColor, EuiInMemoryTable, EuiBasicTableColumn } from '@elastic/eui';

import { ASRoleMapping } from '../../app_search/types';
import { WSRoleMapping } from '../../workplace_search/types';
import { MANAGE_BUTTON_LABEL, DELETE_BUTTON_LABEL } from '../constants';
import { RoleRules } from '../types';

import './role_mappings_table.scss';
Expand All @@ -38,7 +24,9 @@ import {
EXTERNAL_ATTRIBUTE_LABEL,
ATTRIBUTE_VALUE_LABEL,
FILTER_ROLE_MAPPINGS_PLACEHOLDER,
ROLE_MAPPINGS_NO_RESULTS_MESSAGE,
} from './constants';
import { UsersAndRolesRowActions } from './users_and_roles_row_actions';

interface AccessItem {
name: string;
Expand All @@ -58,8 +46,6 @@ interface Props {
handleDeleteMapping(roleMappingId: string): void;
}

const MAX_CELL_WIDTH = 24;

const noItemsPlaceholder = <EuiTextColor color="subdued">&mdash;</EuiTextColor>;

const getAuthProviderDisplayValue = (authProvider: string) =>
Expand All @@ -73,114 +59,104 @@ export const RoleMappingsTable: React.FC<Props> = ({
initializeRoleMapping,
handleDeleteMapping,
}) => {
const [filterValue, updateValue] = useState('');
const getFirstAttributeName = (rules: RoleRules): string => Object.entries(rules)[0][0];
const getFirstAttributeValue = (rules: RoleRules): string => Object.entries(rules)[0][1];

// This is needed because App Search has `engines` and Workplace Search has `groups`.
const standardizeRoleMapping = (roleMappings as SharedRoleMapping[]).map((rm) => {
const standardizedRoleMappings = (roleMappings as SharedRoleMapping[]).map((rm) => {
const _rm = { ...rm } as SharedRoleMapping;
_rm.accessItems = rm[accessItemKey];
return _rm;
});

const filterResults = (result: SharedRoleMapping) => {
// Filter out non-alphanumeric characters, except for underscores, hyphens, and spaces
const sanitizedValue = filterValue.replace(/[^\w\s-]/g, '');
const values = Object.values(result);
const regexp = new RegExp(sanitizedValue, 'i');
return values.filter((x) => regexp.test(x)).length > 0;
}) as SharedRoleMapping[];

const attributeNameCol: EuiBasicTableColumn<SharedRoleMapping> = {
field: 'attribute',
name: EXTERNAL_ATTRIBUTE_LABEL,
render: (_, { rules }: SharedRoleMapping) => getFirstAttributeName(rules),
};

const filteredResults = standardizeRoleMapping.filter(filterResults);
const getFirstAttributeName = (rules: RoleRules): string => Object.entries(rules)[0][0];
const getFirstAttributeValue = (rules: RoleRules): string => Object.entries(rules)[0][1];
const attributeValueCol: EuiBasicTableColumn<SharedRoleMapping> = {
field: 'attributeValue',
name: ATTRIBUTE_VALUE_LABEL,
render: (_, { rules }: SharedRoleMapping) => getFirstAttributeValue(rules),
};

const rowActions = (id: string) => (
<>
<EuiButtonIcon
onClick={() => initializeRoleMapping(id)}
iconType="pencil"
aria-label={MANAGE_BUTTON_LABEL}
data-test-subj="ManageButton"
/>{' '}
<EuiButtonIcon
onClick={() => handleDeleteMapping(id)}
iconType="trash"
aria-label={DELETE_BUTTON_LABEL}
data-test-subj="DeleteButton"
const roleCol: EuiBasicTableColumn<SharedRoleMapping> = {
field: 'roleType',
name: ROLE_LABEL,
render: (_, { rules }: SharedRoleMapping) => getFirstAttributeValue(rules),
};

const accessItemsCol: EuiBasicTableColumn<SharedRoleMapping> = {
field: 'accessItems',
name: accessHeader,
render: (_, { accessAllEngines, accessItems }: SharedRoleMapping) => (
<span data-test-subj="AccessItemsList">
{accessAllEngines ? (
ALL_LABEL
) : (
<>
{accessItems.length === 0
? noItemsPlaceholder
: accessItems.map(({ name }) => (
<Fragment key={name}>
{name}
<br />
</Fragment>
))}
</>
)}
</span>
),
};

const authProviderCol: EuiBasicTableColumn<SharedRoleMapping> = {
field: 'authProvider',
name: AUTH_PROVIDER_LABEL,
render: (_, { authProvider }: SharedRoleMapping) => (
<span data-test-subj="AuthProviderDisplayValue">
{authProvider.map(getAuthProviderDisplayValue).join(', ')}
</span>
),
};

const actionsCol: EuiBasicTableColumn<SharedRoleMapping> = {
field: 'id',
name: '',
align: 'right',
render: (_, { id }: SharedRoleMapping) => (
<UsersAndRolesRowActions
onManageClick={() => initializeRoleMapping(id)}
onDeleteClick={() => handleDeleteMapping(id)}
/>
</>
);
),
};
Comment on lines +123 to +133
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this should probably be an action column https://elastic.github.io/eui/#/tabular-content/tables#adding-actions-to-table but its fine if this got polished later

Copy link
Contributor Author

@scottybollinger scottybollinger Jun 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I went back and forth with it. Ultimately, I decided against it because it limits the display to 2 icon buttons before collapsing the others with an ellipsis and the forthcoming invitations table for pending invitations has 3. I wasn't sure if there was any design difference between bespoke actions and that actions column but didn't want to have a mixture of them on this layout if there was.

Copy link
Member

@cee-chen cee-chen Jun 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for the lateness on this, but semi related to this, FYI we do need to add a column header to action columns - it's an a11y issue / axe failure for a table cell not to have an accompanying table heading. We can definitely add it later though, I intend to do an axe pass on our plugin next week


const columns = shouldShowAuthProvider
? [attributeNameCol, attributeValueCol, roleCol, accessItemsCol, authProviderCol, actionsCol]
: [attributeNameCol, attributeValueCol, roleCol, accessItemsCol, actionsCol];

const pagination = {
hidePerPageOptions: true,
};

const search = {
box: {
incremental: true,
fullWidth: false,
placeholder: FILTER_ROLE_MAPPINGS_PLACEHOLDER,
'data-test-subj': 'RoleMappingsTableSearchInput',
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you using this attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, but we hope to write some Cypress tests before 8.0 and I believe it could be used there.

},
};
return (
<>
<EuiFieldSearch
value={filterValue}
placeholder={FILTER_ROLE_MAPPINGS_PLACEHOLDER}
onChange={(e) => updateValue(e.target.value)}
/>
<EuiSpacer />
{filteredResults.length > 0 ? (
<EuiTable className="roleMappingsTable">
<EuiTableHeader>
<EuiTableHeaderCell>{EXTERNAL_ATTRIBUTE_LABEL}</EuiTableHeaderCell>
<EuiTableHeaderCell>{ATTRIBUTE_VALUE_LABEL}</EuiTableHeaderCell>
<EuiTableHeaderCell>{ROLE_LABEL}</EuiTableHeaderCell>
<EuiTableHeaderCell>{accessHeader}</EuiTableHeaderCell>
{shouldShowAuthProvider && (
<EuiTableHeaderCell>{AUTH_PROVIDER_LABEL}</EuiTableHeaderCell>
)}
<EuiTableHeaderCell />
</EuiTableHeader>
<EuiTableBody>
{filteredResults.map(
({ id, authProvider, rules, roleType, accessAllEngines, accessItems, toolTip }) => (
<EuiTableRow key={id}>
<EuiTableRowCell>{getFirstAttributeName(rules)}</EuiTableRowCell>
<EuiTableRowCell style={{ maxWidth: MAX_CELL_WIDTH }}>
{getFirstAttributeValue(rules)}
</EuiTableRowCell>
<EuiTableRowCell>{roleType}</EuiTableRowCell>
<EuiTableRowCell
data-test-subj="AccessItemsList"
style={{ maxWidth: MAX_CELL_WIDTH }}
>
{accessAllEngines ? (
ALL_LABEL
) : (
<>
{accessItems.length === 0
? noItemsPlaceholder
: accessItems.map(({ name }) => (
<Fragment key={name}>
{name}
<br />
</Fragment>
))}
</>
)}
</EuiTableRowCell>
{shouldShowAuthProvider && (
<EuiTableRowCell data-test-subj="AuthProviderDisplay">
{authProvider.map(getAuthProviderDisplayValue).join(', ')}
</EuiTableRowCell>
)}
<EuiTableRowCell align="right">
{id && rowActions(id)}
{toolTip && <EuiIconTip position="left" content={toolTip.content} />}
</EuiTableRowCell>
</EuiTableRow>
)
)}
</EuiTableBody>
</EuiTable>
) : (
<p>
{i18n.translate('xpack.enterpriseSearch.roleMapping.moResults.message', {
defaultMessage: "No results found for '{filterValue}'",
values: { filterValue },
})}
</p>
)}
</>
<EuiInMemoryTable
data-test-subj="RoleMappingsTable"
columns={columns}
items={standardizedRoleMappings}
search={search}
pagination={pagination}
message={ROLE_MAPPINGS_NO_RESULTS_MESSAGE}
responsive={false}
/>
);
};
Loading