From f4ca9e4730ee6b13d19808be9fcc633dce9087d5 Mon Sep 17 00:00:00 2001 From: Sid Date: Fri, 27 Sep 2024 12:59:28 +0200 Subject: [PATCH] [Roles] Fix bug with roles grid not sorting on clicking table header (#194196) Fixes https://github.com/elastic/kibana/issues/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 --- .../roles/roles_grid/roles_grid_page.test.tsx | 115 +++++++++++++++++- .../roles/roles_grid/roles_grid_page.tsx | 52 ++------ 2 files changed, 120 insertions(+), 47 deletions(-) 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 8951fd3e5c2023..57281f5ec754c9 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 @@ -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'; @@ -209,7 +209,7 @@ describe('', () => { 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: { @@ -296,7 +296,7 @@ describe('', () => { 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: [] }, @@ -322,6 +322,115 @@ describe('', () => { ]); }); + it('sorts columns on clicking the column header', async () => { + const wrapper = mountWithIntl( + + ); + 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( { return `/${action}${roleName ? `/${encodeURIComponent(roleName)}` : ''}`; }; @@ -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 = ({ notifications, rolesAPIClient, @@ -109,7 +87,6 @@ export const RolesGridPage: FC = ({ const [permissionDenied, setPermissionDenied] = useState(false); const [includeReservedRoles, setIncludeReservedRoles] = useState(true); const [isLoading, setIsLoading] = useState(false); - const [tableState, setTableState] = useState(DEFAULT_TABLE_STATE); useEffect(() => { loadRoles(); @@ -235,15 +212,6 @@ export const RolesGridPage: FC = ({ } }; - const onTableChange = ({ page, sort }: CriteriaWithPagination) => { - const newState = { - ...tableState, - from: page?.index! * page?.size!, - size: page?.size!, - }; - setTableState(newState); - }; - const getColumnConfig = (): Array> => { const config: Array> = [ { @@ -365,12 +333,6 @@ export const RolesGridPage: FC = ({ setShowDeleteConfirmation(false); }; - const pagination = { - pageIndex: tableState.from / tableState.size, - pageSize: tableState.size, - totalItemCount: visibleRoles.length, - pageSizeOptions: [25, 50, 100], - }; return permissionDenied ? ( ) : ( @@ -466,7 +428,7 @@ export const RolesGridPage: FC = ({ toolsRight={renderToolsRight()} /> - = ({ selected: selection, } } - onChange={onTableChange} - pagination={pagination} - noItemsMessage={ + pagination={{ + initialPageSize: 20, + pageSizeOptions: [10, 20, 30, 50, 100], + }} + message={ buildFlavor === 'serverless' ? (