Skip to content

Commit

Permalink
[8.x] [Roles] Fix bug with roles grid not sorting on clicking table h…
Browse files Browse the repository at this point in the history
…eader (#194196) (#194283)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Roles] Fix bug with roles grid not sorting on clicking table header
(#194196)](#194196)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Sid","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-27T10:59:28Z","message":"[Roles]
Fix bug with roles grid not sorting on clicking table header
(#194196)\n\nFixes
https://github.com/elastic/kibana/issues/193786\r\n\r\n##
Summary\r\nReverts a few changes made when the Roles grid page was moved
to a\r\nfunctional component. Fixes regression in table
sorting.\r\n\r\n### Notes\r\n\r\nWhen preparing for the Query Roles API,
we had moved the roles grid page\r\nto be a functional component. In
doing so, we also migrated away from\r\nthe In Memory table in favor of
the basic table. EUIBasicTable does not\r\nsupport sorting out of the
box and is meant to be used for server-side\r\nsorting, etc (unless we
implement custom sorting logic). I've made a few\r\nchanges:\r\n- Bring
back the InMemoryTable but keep the Search Bar.\r\n- Remove few (now)
unused functions which are to be brought back\r\nwhenever the Query
Roles API is ready.\r\n- Update tests\r\n\r\n### Screen
recording\r\n\r\n\r\nhttps://github.com/user-attachments/assets/4ac4f771-e7d1-4e17-807e-d6262767d100\r\n\r\n\r\n\r\n###
Release notes\r\nFixes UI regression in Roles listing page where users
could not sort\r\ntable by using the
headers.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"f4ca9e4730ee6b13d19808be9fcc633dce9087d5","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","regression","release_note:fix","Team:Security","Feature:Users/Roles/API
Keys","v9.0.0","backport:prev-major"],"title":"[Roles] Fix bug with
roles grid not sorting on clicking table
header","number":194196,"url":"https://github.com/elastic/kibana/pull/194196","mergeCommit":{"message":"[Roles]
Fix bug with roles grid not sorting on clicking table header
(#194196)\n\nFixes
https://github.com/elastic/kibana/issues/193786\r\n\r\n##
Summary\r\nReverts a few changes made when the Roles grid page was moved
to a\r\nfunctional component. Fixes regression in table
sorting.\r\n\r\n### Notes\r\n\r\nWhen preparing for the Query Roles API,
we had moved the roles grid page\r\nto be a functional component. In
doing so, we also migrated away from\r\nthe In Memory table in favor of
the basic table. EUIBasicTable does not\r\nsupport sorting out of the
box and is meant to be used for server-side\r\nsorting, etc (unless we
implement custom sorting logic). I've made a few\r\nchanges:\r\n- Bring
back the InMemoryTable but keep the Search Bar.\r\n- Remove few (now)
unused functions which are to be brought back\r\nwhenever the Query
Roles API is ready.\r\n- Update tests\r\n\r\n### Screen
recording\r\n\r\n\r\nhttps://github.com/user-attachments/assets/4ac4f771-e7d1-4e17-807e-d6262767d100\r\n\r\n\r\n\r\n###
Release notes\r\nFixes UI regression in Roles listing page where users
could not sort\r\ntable by using the
headers.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"f4ca9e4730ee6b13d19808be9fcc633dce9087d5"}},"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/194196","number":194196,"mergeCommit":{"message":"[Roles]
Fix bug with roles grid not sorting on clicking table header
(#194196)\n\nFixes
https://github.com/elastic/kibana/issues/193786\r\n\r\n##
Summary\r\nReverts a few changes made when the Roles grid page was moved
to a\r\nfunctional component. Fixes regression in table
sorting.\r\n\r\n### Notes\r\n\r\nWhen preparing for the Query Roles API,
we had moved the roles grid page\r\nto be a functional component. In
doing so, we also migrated away from\r\nthe In Memory table in favor of
the basic table. EUIBasicTable does not\r\nsupport sorting out of the
box and is meant to be used for server-side\r\nsorting, etc (unless we
implement custom sorting logic). I've made a few\r\nchanges:\r\n- Bring
back the InMemoryTable but keep the Search Bar.\r\n- Remove few (now)
unused functions which are to be brought back\r\nwhenever the Query
Roles API is ready.\r\n- Update tests\r\n\r\n### Screen
recording\r\n\r\n\r\nhttps://github.com/user-attachments/assets/4ac4f771-e7d1-4e17-807e-d6262767d100\r\n\r\n\r\n\r\n###
Release notes\r\nFixes UI regression in Roles listing page where users
could not sort\r\ntable by using the
headers.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"f4ca9e4730ee6b13d19808be9fcc633dce9087d5"}}]}]
BACKPORT-->

Co-authored-by: Sid <[email protected]>
  • Loading branch information
kibanamachine and SiddharthMantri committed Sep 27, 2024
1 parent 366679c commit 8c9060d
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 8c9060d

Please sign in to comment.