Skip to content

Commit

Permalink
[Roles] Fix bug with roles grid not sorting on clicking table header (e…
Browse files Browse the repository at this point in the history
…lastic#194196)

Fixes elastic#193786

## Summary
Reverts a few changes made when the Roles grid page was moved to a
functional component. Fixes regression in table sorting.

### Notes

When preparing for the Query Roles API, we had moved the roles grid page
to be a functional component. In doing so, we also migrated away from
the In Memory table in favor of the basic table. EUIBasicTable does not
support sorting out of the box and is meant to be used for server-side
sorting, etc (unless we implement custom sorting logic). I've made a few
changes:
-  Bring back the InMemoryTable but keep the Search Bar.
- Remove few (now) unused functions which are to be brought back
whenever the Query Roles API is ready.
- Update tests

### Screen recording


https://github.com/user-attachments/assets/4ac4f771-e7d1-4e17-807e-d6262767d100



### Release notes
Fixes UI regression in Roles listing page where users could not sort
table by using the headers.

---------

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
SiddharthMantri and elasticmachine committed Sep 27, 2024
1 parent 07290bf commit f4ca9e4
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { EuiBasicTable, EuiIcon } from '@elastic/eui';
import { EuiIcon, EuiInMemoryTable } from '@elastic/eui';
import type { ReactWrapper } from 'enzyme';
import React from 'react';

Expand Down Expand Up @@ -209,7 +209,7 @@ describe('<RolesGridPage />', () => {
return updatedWrapper.find(EuiIcon).length > initialIconCount;
});

expect(wrapper.find(EuiBasicTable).props().items).toEqual([
expect(wrapper.find(EuiInMemoryTable).props().items).toEqual([
{
name: 'test-role-1',
elasticsearch: {
Expand Down Expand Up @@ -296,7 +296,7 @@ describe('<RolesGridPage />', () => {

findTestSubject(wrapper, 'showReservedRolesSwitch').simulate('click');

expect(wrapper.find(EuiBasicTable).props().items).toEqual([
expect(wrapper.find(EuiInMemoryTable).props().items).toEqual([
{
name: 'test-role-1',
elasticsearch: { cluster: [], indices: [], run_as: [] },
Expand All @@ -322,6 +322,115 @@ describe('<RolesGridPage />', () => {
]);
});

it('sorts columns on clicking the column header', async () => {
const wrapper = mountWithIntl(
<RolesGridPage
rolesAPIClient={apiClientMock}
history={history}
notifications={notifications}
i18n={i18n}
buildFlavor={'traditional'}
analytics={analytics}
theme={theme}
/>
);
const initialIconCount = wrapper.find(EuiIcon).length;

await waitForRender(wrapper, (updatedWrapper) => {
return updatedWrapper.find(EuiIcon).length > initialIconCount;
});

expect(wrapper.find(EuiInMemoryTable).props().items).toEqual([
{
name: 'test-role-1',
elasticsearch: {
cluster: [],
indices: [],
run_as: [],
},
kibana: [
{
base: [],
spaces: [],
feature: {},
},
],
},
{
name: 'test-role-with-description',
description: 'role-description',
elasticsearch: {
cluster: [],
indices: [],
run_as: [],
},
kibana: [
{
base: [],
spaces: [],
feature: {},
},
],
},
{
name: 'reserved-role',
elasticsearch: {
cluster: [],
indices: [],
run_as: [],
},
kibana: [
{
base: [],
spaces: [],
feature: {},
},
],
metadata: {
_reserved: true,
},
},
{
name: 'disabled-role',
elasticsearch: {
cluster: [],
indices: [],
run_as: [],
},
kibana: [
{
base: [],
spaces: [],
feature: {},
},
],
transient_metadata: {
enabled: false,
},
},
{
name: 'special%chars%role',
elasticsearch: {
cluster: [],
indices: [],
run_as: [],
},
kibana: [
{
base: [],
spaces: [],
feature: {},
},
],
},
]);

findTestSubject(wrapper, 'tableHeaderCell_name_0').simulate('click');

const firstRowElement = findTestSubject(wrapper, 'roleRowName').first();
expect(firstRowElement.text()).toBe('disabled-role');
});

it('hides controls when `readOnly` is enabled', async () => {
const wrapper = mountWithIntl(
<RolesGridPage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,13 @@
* 2.0.
*/

import type {
CriteriaWithPagination,
EuiBasicTableColumn,
EuiSwitchEvent,
Query,
} from '@elastic/eui';
import type { EuiBasicTableColumn, EuiSwitchEvent } from '@elastic/eui';
import {
EuiBasicTable,
EuiButton,
EuiButtonEmpty,
EuiFlexGroup,
EuiFlexItem,
EuiInMemoryTable,
EuiLink,
EuiSearchBar,
EuiSpacer,
Expand Down Expand Up @@ -59,12 +54,6 @@ export interface Props extends StartServices {
cloudOrgUrl?: string;
}

interface RolesTableState {
query: Query;
from: number;
size: number;
}

const getRoleManagementHref = (action: 'edit' | 'clone', roleName?: string) => {
return `/${action}${roleName ? `/${encodeURIComponent(roleName)}` : ''}`;
};
Expand All @@ -79,17 +68,6 @@ const getVisibleRoles = (roles: Role[], filter: string, includeReservedRoles: bo
});
};

const DEFAULT_TABLE_STATE = {
query: EuiSearchBar.Query.MATCH_ALL,
sort: {
field: 'creation' as const,
direction: 'desc' as const,
},
from: 0,
size: 25,
filters: {},
};

export const RolesGridPage: FC<Props> = ({
notifications,
rolesAPIClient,
Expand All @@ -109,7 +87,6 @@ export const RolesGridPage: FC<Props> = ({
const [permissionDenied, setPermissionDenied] = useState<boolean>(false);
const [includeReservedRoles, setIncludeReservedRoles] = useState<boolean>(true);
const [isLoading, setIsLoading] = useState<boolean>(false);
const [tableState, setTableState] = useState<RolesTableState>(DEFAULT_TABLE_STATE);

useEffect(() => {
loadRoles();
Expand Down Expand Up @@ -235,15 +212,6 @@ export const RolesGridPage: FC<Props> = ({
}
};

const onTableChange = ({ page, sort }: CriteriaWithPagination<Role>) => {
const newState = {
...tableState,
from: page?.index! * page?.size!,
size: page?.size!,
};
setTableState(newState);
};

const getColumnConfig = (): Array<EuiBasicTableColumn<Role>> => {
const config: Array<EuiBasicTableColumn<Role>> = [
{
Expand Down Expand Up @@ -365,12 +333,6 @@ export const RolesGridPage: FC<Props> = ({
setShowDeleteConfirmation(false);
};

const pagination = {
pageIndex: tableState.from / tableState.size,
pageSize: tableState.size,
totalItemCount: visibleRoles.length,
pageSizeOptions: [25, 50, 100],
};
return permissionDenied ? (
<PermissionDenied />
) : (
Expand Down Expand Up @@ -466,7 +428,7 @@ export const RolesGridPage: FC<Props> = ({
toolsRight={renderToolsRight()}
/>
<EuiSpacer size="s" />
<EuiBasicTable
<EuiInMemoryTable
data-test-subj="rolesTable"
itemId="name"
columns={getColumnConfig()}
Expand All @@ -481,9 +443,11 @@ export const RolesGridPage: FC<Props> = ({
selected: selection,
}
}
onChange={onTableChange}
pagination={pagination}
noItemsMessage={
pagination={{
initialPageSize: 20,
pageSizeOptions: [10, 20, 30, 50, 100],
}}
message={
buildFlavor === 'serverless' ? (
<FormattedMessage
id="xpack.security.management.roles.noCustomRolesFound"
Expand Down

0 comments on commit f4ca9e4

Please sign in to comment.