From 4a692fc82eb1438d394fb9621b054c16599b147e Mon Sep 17 00:00:00 2001 From: Alex Khomenko Date: Fri, 29 Sep 2023 11:48:36 +0200 Subject: [PATCH] Admin: Use backend sort (#75228) * Admin: Use InteractiveTable * Admin: Fix pagination * Admin: Use CellWrapper * Admin: Split components * Admin: Separate OrgUsersTable * Admin: Remove UsersTable * Admin: Use the new table for TeamList * Admin: Cleanup TeamList page * Admin: Add edit team action * Admin: Use explicit edit action instead of a link wrapper * Admin: Fix responsive styles * Cleanup * Remove redundant sort * Add item key * Fix icon styles * Set loading by default * Use separate pagination component * Use default sorting functionality * Fix merge conflicts * Update betterer * Add controlled sort * Revert pagination changes * Move pagination inside OrgUsersTable.tsx * Add missing prop * Sort users table * Fix tests * Remove loader * Add sort to Teams * Cleanup * Update loadTeams action * Remove sort condition --- .betterer.results | 2 +- .../app/features/admin/UserListAdminPage.tsx | 26 ++++++------- .../features/admin/Users/OrgUsersTable.tsx | 37 +++++++++++-------- .../app/features/admin/Users/UsersTable.tsx | 30 +++++++++------ public/app/features/admin/state/actions.ts | 24 ++++++++++-- public/app/features/admin/state/reducers.ts | 7 +++- public/app/features/teams/TeamList.test.tsx | 1 + public/app/features/teams/TeamList.tsx | 20 ++++++---- public/app/features/teams/state/actions.ts | 25 +++++++++++-- public/app/features/teams/state/reducers.ts | 5 ++- .../app/features/users/UsersListPage.test.tsx | 3 +- public/app/features/users/UsersListPage.tsx | 5 ++- public/app/features/users/state/actions.ts | 16 ++++++-- public/app/features/users/state/reducers.ts | 15 +++++++- public/app/types/teams.ts | 1 + public/app/types/user.ts | 2 + 16 files changed, 151 insertions(+), 68 deletions(-) diff --git a/.betterer.results b/.betterer.results index ee1f2b81249..c77bf79a9ff 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1,5 +1,5 @@ // BETTERER RESULTS V2. -// +// // If this file contains merge conflicts, use `betterer merge` to automatically resolve them: // https://phenomnomnominal.github.io/betterer/docs/results-file/#merge // diff --git a/public/app/features/admin/UserListAdminPage.tsx b/public/app/features/admin/UserListAdminPage.tsx index f1fecda0671..a675cef2c2b 100644 --- a/public/app/features/admin/UserListAdminPage.tsx +++ b/public/app/features/admin/UserListAdminPage.tsx @@ -8,11 +8,10 @@ import { LinkButton, RadioButtonGroup, useStyles2, FilterInput } from '@grafana/ import { Page } from 'app/core/components/Page/Page'; import { contextSrv } from 'app/core/core'; -import PageLoader from '../../core/components/PageLoader/PageLoader'; import { AccessControlAction, StoreState, UserFilter } from '../../types'; import { UsersTable } from './Users/UsersTable'; -import { changeFilter, changePage, changeQuery, fetchUsers } from './state/actions'; +import { changeFilter, changePage, changeQuery, changeSort, fetchUsers } from './state/actions'; export interface FilterProps { filters: UserFilter[]; @@ -31,6 +30,7 @@ const mapDispatchToProps = { changeQuery, changePage, changeFilter, + changeSort, }; const mapStateToProps = (state: StoreState) => ({ @@ -40,7 +40,6 @@ const mapStateToProps = (state: StoreState) => ({ totalPages: state.userListAdmin.totalPages, page: state.userListAdmin.page, filters: state.userListAdmin.filters, - isLoading: state.userListAdmin.isLoading, }); const connector = connect(mapStateToProps, mapDispatchToProps); @@ -57,10 +56,10 @@ const UserListAdminPageUnConnected = ({ showPaging, changeFilter, filters, - isLoading, totalPages, page, changePage, + changeSort, }: Props) => { const styles = useStyles2(getStyles); @@ -97,17 +96,14 @@ const UserListAdminPageUnConnected = ({ )} - {isLoading ? ( - - ) : ( - - )} + ); }; diff --git a/public/app/features/admin/Users/OrgUsersTable.tsx b/public/app/features/admin/Users/OrgUsersTable.tsx index 48296a41b8b..8b6a8a548dd 100644 --- a/public/app/features/admin/Users/OrgUsersTable.tsx +++ b/public/app/features/admin/Users/OrgUsersTable.tsx @@ -13,6 +13,7 @@ import { Tag, InteractiveTable, Column, + FetchDataFunc, Pagination, HorizontalGroup, VerticalGroup, @@ -53,16 +54,24 @@ export interface Props { orgId?: number; onRoleChange: (role: OrgRole, user: OrgUser) => void; onRemoveUser: (user: OrgUser) => void; + fetchData?: FetchDataFunc; changePage: (page: number) => void; page: number; totalPages: number; } -export const OrgUsersTable = ({ users, orgId, onRoleChange, onRemoveUser, changePage, page, totalPages }: Props) => { +export const OrgUsersTable = ({ + users, + orgId, + onRoleChange, + onRemoveUser, + fetchData, + changePage, + page, + totalPages, +}: Props) => { const [userToRemove, setUserToRemove] = useState(null); const [roleOptions, setRoleOptions] = useState([]); - const enableSort = totalPages === 1; - const styles = useStyles2(getStyles); useEffect(() => { async function fetchOptions() { @@ -91,27 +100,25 @@ export const OrgUsersTable = ({ users, orgId, onRoleChange, onRemoveUser, change id: 'login', header: 'Login', cell: ({ cell: { value } }: Cell<'login'>) =>
{value}
, - sortType: enableSort ? 'string' : undefined, + sortType: 'string', }, { id: 'email', header: 'Email', cell: ({ cell: { value } }: Cell<'email'>) => value, - sortType: enableSort ? 'string' : undefined, + sortType: 'string', }, { id: 'name', header: 'Name', cell: ({ cell: { value } }: Cell<'name'>) => value, - sortType: enableSort ? 'string' : undefined, + sortType: 'string', }, { id: 'lastSeenAtAge', header: 'Last active', cell: ({ cell: { value } }: Cell<'lastSeenAtAge'>) => value, - sortType: enableSort - ? (a, b) => new Date(a.original.lastSeenAt).getTime() - new Date(b.original.lastSeenAt).getTime() - : undefined, + sortType: (a, b) => new Date(a.original.lastSeenAt).getTime() - new Date(b.original.lastSeenAt).getTime(), }, { id: 'role', @@ -175,17 +182,15 @@ export const OrgUsersTable = ({ users, orgId, onRoleChange, onRemoveUser, change }, }, ], - [orgId, roleOptions, onRoleChange, enableSort] + [orgId, roleOptions, onRoleChange] ); return ( -
- String(user.userId)} /> - - - -
+ String(user.userId)} fetchData={fetchData} /> + + + {Boolean(userToRemove) && ( void; currentPage: number; + fetchData?: FetchDataFunc; } -export const UsersTable = ({ users, showPaging, totalPages, onChangePage, currentPage }: UsersTableProps) => { +export const UsersTable = ({ + users, + showPaging, + totalPages, + onChangePage, + currentPage, + fetchData, +}: UsersTableProps) => { const showLicensedRole = useMemo(() => users.some((user) => user.licensedRole), [users]); - const enableSort = totalPages === 1; const columns: Array> = useMemo( () => [ { @@ -44,25 +52,25 @@ export const UsersTable = ({ users, showPaging, totalPages, onChangePage, curren id: 'login', header: 'Login', cell: ({ cell: { value } }: Cell<'login'>) => value, - sortType: enableSort ? 'string' : undefined, + sortType: 'string', }, { id: 'email', header: 'Email', cell: ({ cell: { value } }: Cell<'email'>) => value, - sortType: enableSort ? 'string' : undefined, + sortType: 'string', }, { id: 'name', header: 'Name', cell: ({ cell: { value } }: Cell<'name'>) => value, - sortType: enableSort ? 'string' : undefined, + sortType: 'string', }, { id: 'orgs', header: 'Belongs to', cell: OrgUnitsCell, - sortType: enableSort ? (a, b) => (a.original.orgs?.length || 0) - (b.original.orgs?.length || 0) : undefined, + sortType: (a, b) => (a.original.orgs?.length || 0) - (b.original.orgs?.length || 0), }, ...(showLicensedRole ? [ @@ -71,7 +79,7 @@ export const UsersTable = ({ users, showPaging, totalPages, onChangePage, curren header: 'Licensed role', cell: LicensedRoleCell, // Needs the assertion here, the types are not inferred correctly due to the conditional assignment - sortType: enableSort ? ('string' as const) : undefined, + sortType: 'string' as const, }, ] : []), @@ -83,9 +91,7 @@ export const UsersTable = ({ users, showPaging, totalPages, onChangePage, curren iconName: 'question-circle', }, cell: LastSeenAtCell, - sortType: enableSort - ? (a, b) => new Date(a.original.lastSeenAt!).getTime() - new Date(b.original.lastSeenAt!).getTime() - : undefined, + sortType: (a, b) => new Date(a.original.lastSeenAt!).getTime() - new Date(b.original.lastSeenAt!).getTime(), }, { id: 'authLabels', @@ -113,11 +119,11 @@ export const UsersTable = ({ users, showPaging, totalPages, onChangePage, curren }, }, ], - [showLicensedRole, enableSort] + [showLicensedRole] ); return ( - String(user.id)} /> + String(user.id)} fetchData={fetchData} /> {showPaging && ( diff --git a/public/app/features/admin/state/actions.ts b/public/app/features/admin/state/actions.ts index a51a0264836..f1daa23c723 100644 --- a/public/app/features/admin/state/actions.ts +++ b/public/app/features/admin/state/actions.ts @@ -2,6 +2,7 @@ import { debounce } from 'lodash'; import { dateTimeFormatTimeAgo } from '@grafana/data'; import { featureEnabled, getBackendSrv, isFetchError, locationService } from '@grafana/runtime'; +import { FetchDataArgs } from '@grafana/ui'; import config from 'app/core/config'; import { contextSrv } from 'app/core/core'; import { accessControlQueryParam } from 'app/core/utils/accessControl'; @@ -26,6 +27,7 @@ import { filterChanged, usersFetchBegin, usersFetchEnd, + sortChanged, } from './reducers'; // UserAdminPage @@ -281,10 +283,12 @@ const getFilters = (filters: UserFilter[]) => { export function fetchUsers(): ThunkResult { return async (dispatch, getState) => { try { - const { perPage, page, query, filters } = getState().userListAdmin; - const result = await getBackendSrv().get( - `/api/users/search?perpage=${perPage}&page=${page}&query=${query}&${getFilters(filters)}` - ); + const { perPage, page, query, filters, sort } = getState().userListAdmin; + let url = `/api/users/search?perpage=${perPage}&page=${page}&query=${query}&${getFilters(filters)}`; + if (sort) { + url += `&sort=${sort}`; + } + const result = await getBackendSrv().get(url); dispatch(usersFetched(result)); } catch (error) { usersFetchEnd(); @@ -318,3 +322,15 @@ export function changePage(page: number): ThunkResult { dispatch(fetchUsers()); }; } + +export function changeSort({ sortBy }: FetchDataArgs): ThunkResult { + const sort = sortBy.length ? `${sortBy[0].id}-${sortBy[0].desc ? 'desc' : 'asc'}` : undefined; + return async (dispatch, getState) => { + const currentSort = getState().userListAdmin.sort; + if (currentSort !== sort) { + dispatch(usersFetchBegin()); + dispatch(sortChanged(sort)); + dispatch(fetchUsers()); + } + }; +} diff --git a/public/app/features/admin/state/reducers.ts b/public/app/features/admin/state/reducers.ts index 0074bb67731..f525c7612ba 100644 --- a/public/app/features/admin/state/reducers.ts +++ b/public/app/features/admin/state/reducers.ts @@ -173,6 +173,11 @@ export const userListAdminSlice = createSlice({ ...state, page: action.payload, }), + sortChanged: (state, action: PayloadAction) => ({ + ...state, + page: 0, + sort: action.payload, + }), filterChanged: (state, action: PayloadAction) => { const { name, value } = action.payload; @@ -192,7 +197,7 @@ export const userListAdminSlice = createSlice({ }, }); -export const { usersFetched, usersFetchBegin, usersFetchEnd, queryChanged, pageChanged, filterChanged } = +export const { usersFetched, usersFetchBegin, usersFetchEnd, queryChanged, pageChanged, filterChanged, sortChanged } = userListAdminSlice.actions; export const userListAdminReducer = userListAdminSlice.reducer; diff --git a/public/app/features/teams/TeamList.test.tsx b/public/app/features/teams/TeamList.test.tsx index 628bbbd4210..4be97e0cf8f 100644 --- a/public/app/features/teams/TeamList.test.tsx +++ b/public/app/features/teams/TeamList.test.tsx @@ -25,6 +25,7 @@ const setup = (propOverrides?: object) => { deleteTeam: jest.fn(), changePage: jest.fn(), changeQuery: jest.fn(), + changeSort: jest.fn(), query: '', totalPages: 0, page: 0, diff --git a/public/app/features/teams/TeamList.tsx b/public/app/features/teams/TeamList.tsx index ce92020448f..8c2f8ce7b2b 100644 --- a/public/app/features/teams/TeamList.tsx +++ b/public/app/features/teams/TeamList.tsx @@ -25,7 +25,7 @@ import { AccessControlAction, Role, StoreState, Team } from 'app/types'; import { TeamRolePicker } from '../../core/components/RolePicker/TeamRolePicker'; import { Avatar } from '../admin/Users/Avatar'; -import { deleteTeam, loadTeams, changePage, changeQuery } from './state/actions'; +import { deleteTeam, loadTeams, changePage, changeQuery, changeSort } from './state/actions'; import { isPermissionTeamAdmin } from './state/selectors'; type Cell = CellProps; @@ -48,9 +48,9 @@ export const TeamList = ({ editorsCanAdmin, page, changePage, + changeSort, }: Props) => { const [roleOptions, setRoleOptions] = useState([]); - const enableSort = totalPages === 1; useEffect(() => { loadTeams(true); @@ -76,19 +76,19 @@ export const TeamList = ({ id: 'name', header: 'Name', cell: ({ cell: { value } }: Cell<'name'>) => value, - sortType: enableSort ? 'string' : undefined, + sortType: 'string', }, { id: 'email', header: 'Email', cell: ({ cell: { value } }: Cell<'email'>) => value, - sortType: enableSort ? 'string' : undefined, + sortType: 'string', }, { id: 'memberCount', header: 'Members', cell: ({ cell: { value } }: Cell<'memberCount'>) => value, - sortType: enableSort ? 'number' : undefined, + sortType: 'number', }, ...(displayRolePicker ? [ @@ -155,7 +155,7 @@ export const TeamList = ({ }, }, ], - [displayRolePicker, editorsCanAdmin, roleOptions, signedInUser, deleteTeam, enableSort] + [displayRolePicker, editorsCanAdmin, roleOptions, signedInUser, deleteTeam] ); return ( @@ -185,7 +185,12 @@ export const TeamList = ({ - String(team.id)} /> + String(team.id)} + fetchData={changeSort} + /> @@ -224,6 +229,7 @@ const mapDispatchToProps = { deleteTeam, changePage, changeQuery, + changeSort, }; const connector = connect(mapStateToProps, mapDispatchToProps); diff --git a/public/app/features/teams/state/actions.ts b/public/app/features/teams/state/actions.ts index b47b32ba9bd..3d28044b3eb 100644 --- a/public/app/features/teams/state/actions.ts +++ b/public/app/features/teams/state/actions.ts @@ -1,17 +1,26 @@ import { debounce } from 'lodash'; import { getBackendSrv } from '@grafana/runtime'; +import { FetchDataArgs } from '@grafana/ui'; import { updateNavIndex } from 'app/core/actions'; import { contextSrv } from 'app/core/core'; import { accessControlQueryParam } from 'app/core/utils/accessControl'; -import { AccessControlAction, TeamMember, ThunkResult } from 'app/types'; +import { AccessControlAction, Team, TeamMember, ThunkResult } from 'app/types'; import { buildNavModel } from './navModel'; -import { teamGroupsLoaded, queryChanged, pageChanged, teamLoaded, teamMembersLoaded, teamsLoaded } from './reducers'; +import { + teamGroupsLoaded, + queryChanged, + pageChanged, + teamLoaded, + teamMembersLoaded, + teamsLoaded, + sortChanged, +} from './reducers'; export function loadTeams(initial = false): ThunkResult { return async (dispatch, getState) => { - const { query, page, perPage } = getState().teams; + const { query, page, perPage, sort } = getState().teams; // Early return if the user cannot list teams if (!contextSrv.hasPermission(AccessControlAction.ActionTeamsRead)) { dispatch(teamsLoaded({ teams: [], totalCount: 0, page: 1, perPage, noTeams: true })); @@ -20,7 +29,7 @@ export function loadTeams(initial = false): ThunkResult { const response = await getBackendSrv().get( '/api/teams/search', - accessControlQueryParam({ query, page, perpage: perPage }) + accessControlQueryParam({ query, page, perpage: perPage, sort }) ); // We only want to check if there is no teams on the initial request. @@ -67,6 +76,14 @@ export function changePage(page: number): ThunkResult { }; } +export function changeSort({ sortBy }: FetchDataArgs): ThunkResult { + const sort = sortBy.length ? `${sortBy[0].id}-${sortBy[0].desc ? 'desc' : 'asc'}` : undefined; + return async (dispatch) => { + dispatch(sortChanged(sort)); + dispatch(loadTeams()); + }; +} + export function loadTeamMembers(): ThunkResult { return async (dispatch, getStore) => { const team = getStore().team.team; diff --git a/public/app/features/teams/state/reducers.ts b/public/app/features/teams/state/reducers.ts index 6cc15758e02..a854ddc1bd8 100644 --- a/public/app/features/teams/state/reducers.ts +++ b/public/app/features/teams/state/reducers.ts @@ -35,10 +35,13 @@ const teamsSlice = createSlice({ pageChanged: (state, action: PayloadAction): TeamsState => { return { ...state, page: action.payload }; }, + sortChanged: (state, action: PayloadAction): TeamsState => { + return { ...state, sort: action.payload, page: 1 }; + }, }, }); -export const { teamsLoaded, queryChanged, pageChanged } = teamsSlice.actions; +export const { teamsLoaded, queryChanged, pageChanged, sortChanged } = teamsSlice.actions; export const teamsReducer = teamsSlice.reducer; diff --git a/public/app/features/users/UsersListPage.test.tsx b/public/app/features/users/UsersListPage.test.tsx index 57a50a0ac0a..95e92ac482f 100644 --- a/public/app/features/users/UsersListPage.test.tsx +++ b/public/app/features/users/UsersListPage.test.tsx @@ -7,7 +7,7 @@ import { configureStore } from 'app/store/configureStore'; import { Invitee, OrgUser } from 'app/types'; import { Props, UsersListPageUnconnected } from './UsersListPage'; -import { pageChanged } from './state/reducers'; +import { pageChanged, sortChanged } from './state/reducers'; jest.mock('../../core/app_events', () => ({ emit: jest.fn(), @@ -36,6 +36,7 @@ const setup = (propOverrides?: object) => { updateUser: jest.fn(), removeUser: jest.fn(), changePage: mockToolkitActionCreator(pageChanged), + changeSort: mockToolkitActionCreator(sortChanged), isLoading: false, }; diff --git a/public/app/features/users/UsersListPage.tsx b/public/app/features/users/UsersListPage.tsx index 7e60721f6e2..9fbf57a50b8 100644 --- a/public/app/features/users/UsersListPage.tsx +++ b/public/app/features/users/UsersListPage.tsx @@ -12,7 +12,7 @@ import { fetchInvitees } from '../invites/state/actions'; import { selectInvitesMatchingQuery } from '../invites/state/selectors'; import { UsersActionBar } from './UsersActionBar'; -import { loadUsers, removeUser, updateUser, changePage } from './state/actions'; +import { loadUsers, removeUser, updateUser, changePage, changeSort } from './state/actions'; import { getUsers, getUsersSearchQuery } from './state/selectors'; function mapStateToProps(state: StoreState) { @@ -33,6 +33,7 @@ const mapDispatchToProps = { loadUsers, fetchInvitees, changePage, + changeSort, updateUser, removeUser, }; @@ -57,6 +58,7 @@ export const UsersListPageUnconnected = ({ changePage, updateUser, removeUser, + changeSort, }: Props) => { const [showInvites, setShowInvites] = useState(false); const externalUserMngInfoHtml = externalUserMngInfo ? renderMarkdown(externalUserMngInfo) : ''; @@ -86,6 +88,7 @@ export const UsersListPageUnconnected = ({ orgId={contextSrv.user.orgId} onRoleChange={onRoleChange} onRemoveUser={onRemoveUser} + fetchData={changeSort} changePage={changePage} page={page} totalPages={totalPages} diff --git a/public/app/features/users/state/actions.ts b/public/app/features/users/state/actions.ts index 3578144c863..3e7aacd93ff 100644 --- a/public/app/features/users/state/actions.ts +++ b/public/app/features/users/state/actions.ts @@ -1,20 +1,21 @@ import { debounce } from 'lodash'; import { getBackendSrv } from '@grafana/runtime'; +import { FetchDataArgs } from '@grafana/ui'; import { accessControlQueryParam } from 'app/core/utils/accessControl'; import { OrgUser } from 'app/types'; import { ThunkResult } from '../../../types'; -import { usersLoaded, pageChanged, usersFetchBegin, usersFetchEnd, searchQueryChanged } from './reducers'; +import { usersLoaded, pageChanged, usersFetchBegin, usersFetchEnd, searchQueryChanged, sortChanged } from './reducers'; export function loadUsers(): ThunkResult { return async (dispatch, getState) => { try { - const { perPage, page, searchQuery } = getState().users; + const { perPage, page, searchQuery, sort } = getState().users; const users = await getBackendSrv().get( `/api/org/users/search`, - accessControlQueryParam({ perpage: perPage, page, query: searchQuery }) + accessControlQueryParam({ perpage: perPage, page, query: searchQuery, sort }) ); dispatch(usersLoaded(users)); } catch (error) { @@ -47,6 +48,15 @@ export function changePage(page: number): ThunkResult { }; } +export function changeSort({ sortBy }: FetchDataArgs): ThunkResult { + const sort = sortBy.length ? `${sortBy[0].id}-${sortBy[0].desc ? 'desc' : 'asc'}` : undefined; + return async (dispatch) => { + dispatch(usersFetchBegin()); + dispatch(sortChanged(sort)); + dispatch(loadUsers()); + }; +} + export function changeSearchQuery(query: string): ThunkResult { return async (dispatch) => { dispatch(usersFetchBegin()); diff --git a/public/app/features/users/state/reducers.ts b/public/app/features/users/state/reducers.ts index 1dfeec92a91..ccc2269a439 100644 --- a/public/app/features/users/state/reducers.ts +++ b/public/app/features/users/state/reducers.ts @@ -51,6 +51,10 @@ const usersSlice = createSlice({ ...state, page: action.payload, }), + sortChanged: (state, action: PayloadAction) => ({ + ...state, + sort: action.payload, + }), usersFetchBegin: (state) => { return { ...state, isLoading: true }; }, @@ -60,8 +64,15 @@ const usersSlice = createSlice({ }, }); -export const { searchQueryChanged, setUsersSearchPage, usersLoaded, usersFetchBegin, usersFetchEnd, pageChanged } = - usersSlice.actions; +export const { + searchQueryChanged, + setUsersSearchPage, + usersLoaded, + usersFetchBegin, + usersFetchEnd, + pageChanged, + sortChanged, +} = usersSlice.actions; export const usersReducer = usersSlice.reducer; diff --git a/public/app/types/teams.ts b/public/app/types/teams.ts index 015f685f093..5e4c7bf16b2 100644 --- a/public/app/types/teams.ts +++ b/public/app/types/teams.ts @@ -63,6 +63,7 @@ export interface TeamsState { noTeams: boolean; totalPages: number; hasFetched: boolean; + sort?: string; } export interface TeamState { diff --git a/public/app/types/user.ts b/public/app/types/user.ts index 565c20b4832..6c31393b1bb 100644 --- a/public/app/types/user.ts +++ b/public/app/types/user.ts @@ -80,6 +80,7 @@ export interface UsersState { page: number; perPage: number; totalPages: number; + sort?: string; } export interface UserSession { @@ -124,4 +125,5 @@ export interface UserListAdminState { showPaging: boolean; filters: UserFilter[]; isLoading: boolean; + sort?: string; }