From 506e41c116568d8288b2b80957b61756ee1953ec Mon Sep 17 00:00:00 2001 From: DawnMoon1542 <160485532+DawnMoon1542@users.noreply.github.com> Date: Fri, 17 Jul 2026 19:36:44 +0800 Subject: [PATCH] fix: add server-side sorting to user list, prevent client-side sort on paged data (#6194) * fix: add server-side sorting to user list, prevent client-side sort on paged data The user management table applied client-side sorting to the current page slice while pagination was handled server-side, causing ID-asc views to show pages out of order (e.g. 23-42, 3-22, 1-2). Backend: add sort_by/sort_order query params to GetAllUsers and SearchUsers with a column whitelist for safe ORDER BY generation. Frontend: pass sorting state to the API and reset to page 1 on sort change. Generic useDataTable hook now disables client-side sorted row model and sort UI for tables with manual pagination but no server-side sort handler. * fix: add id tie-breaker to non-unique sort columns, remove side effect from state updater Append secondary ORDER BY id DESC when sorting by non-unique columns (quota, created_at, etc.) to prevent row duplication/skipping across OFFSET pages. Move onPaginationChange out of setSorting updater to avoid side effects inside a pure function (React Strict Mode double-invocation safety). --- controller/user.go | 6 +- model/user.go | 66 +++++++++++++++++-- model/user_pagination_test.go | 66 +++++++++++++++++++ .../data-table/hooks/use-data-table.ts | 9 ++- .../models/components/models-table.tsx | 1 - web/default/src/features/users/api.ts | 15 ++++- .../features/users/components/users-table.tsx | 41 +++++++++++- web/default/src/features/users/types.ts | 14 ++++ 8 files changed, 207 insertions(+), 11 deletions(-) create mode 100644 model/user_pagination_test.go diff --git a/controller/user.go b/controller/user.go index 6316fd13..466353a4 100644 --- a/controller/user.go +++ b/controller/user.go @@ -308,7 +308,8 @@ func Register(c *gin.Context) { func GetAllUsers(c *gin.Context) { pageInfo := common.GetPageQuery(c) - users, total, err := model.GetAllUsers(pageInfo) + sortOptions := model.NewUserSortOptions(c.Query("sort_by"), c.Query("sort_order")) + users, total, err := model.GetAllUsers(pageInfo, sortOptions) if err != nil { common.ApiError(c, err) return @@ -337,7 +338,8 @@ func SearchUsers(c *gin.Context) { } } pageInfo := common.GetPageQuery(c) - users, total, err := model.SearchUsers(keyword, group, role, status, pageInfo.GetStartIdx(), pageInfo.GetPageSize()) + sortOptions := model.NewUserSortOptions(c.Query("sort_by"), c.Query("sort_order")) + users, total, err := model.SearchUsers(keyword, group, role, status, pageInfo.GetStartIdx(), pageInfo.GetPageSize(), sortOptions) if err != nil { common.ApiError(c, err) return diff --git a/model/user.go b/model/user.go index 03eb589e..3a33c82f 100644 --- a/model/user.go +++ b/model/user.go @@ -14,10 +14,66 @@ import ( "github.com/bytedance/gopkg/util/gopool" "gorm.io/gorm" + "gorm.io/gorm/clause" ) const UserNameMaxLength = 20 +var userSortColumns = map[string]string{ + "id": "id", + "username": "username", + "quota": "quota", + "group": "group", + "created_at": "created_at", + "last_login_at": "last_login_at", +} + +type UserSortOptions struct { + SortBy string + SortOrder string +} + +func NewUserSortOptions(sortBy string, sortOrder string) UserSortOptions { + normalizedSortBy := strings.ToLower(strings.TrimSpace(sortBy)) + normalizedSortOrder := strings.ToLower(strings.TrimSpace(sortOrder)) + if _, ok := userSortColumns[normalizedSortBy]; !ok { + normalizedSortBy = "id" + normalizedSortOrder = "desc" + } else if normalizedSortOrder != "asc" { + normalizedSortOrder = "desc" + } + + return UserSortOptions{ + SortBy: normalizedSortBy, + SortOrder: normalizedSortOrder, + } +} + +func (options UserSortOptions) Apply(query *gorm.DB) *gorm.DB { + columnName, ok := userSortColumns[options.SortBy] + if !ok { + columnName = "id" + } + q := query.Order(clause.OrderByColumn{ + Column: clause.Column{Name: columnName}, + Desc: options.SortOrder != "asc", + }) + if columnName != "id" { + q = q.Order(clause.OrderByColumn{ + Column: clause.Column{Name: "id"}, + Desc: true, + }) + } + return q +} + +func resolveUserSortOptions(sortOptions []UserSortOptions) UserSortOptions { + if len(sortOptions) == 0 { + return NewUserSortOptions("", "") + } + return sortOptions[0] +} + // User if you add sensitive fields, don't forget to clean them in setupLogin function. // Otherwise, the sensitive information will be saved on local storage in plain text! type User struct { @@ -286,7 +342,7 @@ func GetMaxUserId() int { return user.Id } -func GetAllUsers(pageInfo *common.PageInfo) (users []*User, total int64, err error) { +func GetAllUsers(pageInfo *common.PageInfo, sortOptions ...UserSortOptions) (users []*User, total int64, err error) { // Start transaction tx := DB.Begin() if tx.Error != nil { @@ -306,7 +362,8 @@ func GetAllUsers(pageInfo *common.PageInfo) (users []*User, total int64, err err } // Get paginated users within same transaction - err = tx.Unscoped().Order("id desc").Limit(pageInfo.GetPageSize()).Offset(pageInfo.GetStartIdx()).Omit("password", "access_token").Find(&users).Error + order := resolveUserSortOptions(sortOptions) + err = order.Apply(tx.Unscoped()).Limit(pageInfo.GetPageSize()).Offset(pageInfo.GetStartIdx()).Omit("password", "access_token").Find(&users).Error if err != nil { tx.Rollback() return nil, 0, err @@ -320,7 +377,7 @@ func GetAllUsers(pageInfo *common.PageInfo) (users []*User, total int64, err err return users, total, nil } -func SearchUsers(keyword string, group string, role *int, status *int, startIdx int, num int) ([]*User, int64, error) { +func SearchUsers(keyword string, group string, role *int, status *int, startIdx int, num int, sortOptions ...UserSortOptions) ([]*User, int64, error) { var users []*User var total int64 var err error @@ -374,7 +431,8 @@ func SearchUsers(keyword string, group string, role *int, status *int, startIdx } // 获取分页数据 - err = query.Omit("password", "access_token").Order("id desc").Limit(num).Offset(startIdx).Find(&users).Error + order := resolveUserSortOptions(sortOptions) + err = order.Apply(query.Omit("password", "access_token")).Limit(num).Offset(startIdx).Find(&users).Error if err != nil { tx.Rollback() return nil, 0, err diff --git a/model/user_pagination_test.go b/model/user_pagination_test.go new file mode 100644 index 00000000..1164019f --- /dev/null +++ b/model/user_pagination_test.go @@ -0,0 +1,66 @@ +package model + +import ( + "fmt" + "testing" + + "github.com/QuantumNous/new-api/common" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func insertUsersForPaginationTest(t *testing.T, total int) { + t.Helper() + for id := 1; id <= total; id++ { + user := &User{ + Id: id, + Username: fmt.Sprintf("user%02d", id), + Password: "password123", + DisplayName: fmt.Sprintf("User %02d", id), + Email: fmt.Sprintf("user%02d@example.com", id), + Role: common.RoleCommonUser, + Status: common.UserStatusEnabled, + Group: "default", + AffCode: fmt.Sprintf("aff%02d", id), + } + require.NoError(t, DB.Create(user).Error) + } +} + +func collectUserIDs(users []*User) []int { + ids := make([]int, 0, len(users)) + for _, user := range users { + ids = append(ids, user.Id) + } + return ids +} + +func TestGetAllUsersSortsBeforePagination(t *testing.T) { + truncateTables(t) + insertUsersForPaginationTest(t, 42) + + pageOne, total, err := GetAllUsers(&common.PageInfo{Page: 1, PageSize: 20}, NewUserSortOptions("id", "asc")) + require.NoError(t, err) + assert.Equal(t, int64(42), total) + assert.Equal(t, []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20}, collectUserIDs(pageOne)) + + pageTwo, total, err := GetAllUsers(&common.PageInfo{Page: 2, PageSize: 20}, NewUserSortOptions("id", "asc")) + require.NoError(t, err) + assert.Equal(t, int64(42), total) + assert.Equal(t, []int{21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40}, collectUserIDs(pageTwo)) + + pageThree, total, err := GetAllUsers(&common.PageInfo{Page: 3, PageSize: 20}, NewUserSortOptions("id", "asc")) + require.NoError(t, err) + assert.Equal(t, int64(42), total) + assert.Equal(t, []int{41, 42}, collectUserIDs(pageThree)) +} + +func TestSearchUsersSortsBeforePagination(t *testing.T) { + truncateTables(t) + insertUsersForPaginationTest(t, 42) + + users, total, err := SearchUsers("user", "", nil, nil, 20, 20, NewUserSortOptions("id", "asc")) + require.NoError(t, err) + assert.Equal(t, int64(42), total) + assert.Equal(t, []int{21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40}, collectUserIDs(users)) +} diff --git a/web/default/src/components/data-table/hooks/use-data-table.ts b/web/default/src/components/data-table/hooks/use-data-table.ts index 0b0da93a..3dd66af7 100644 --- a/web/default/src/components/data-table/hooks/use-data-table.ts +++ b/web/default/src/components/data-table/hooks/use-data-table.ts @@ -49,6 +49,7 @@ type DataTableFeatureOptions = Pick< | 'manualFiltering' | 'manualPagination' | 'manualSorting' + | 'enableSorting' | 'enableColumnResizing' > @@ -292,7 +293,7 @@ export function useDataTable(options: UseDataTableOptions) { initialPagination = { pageIndex: 0, pageSize: 20 }, withFilteredRowModel = !manualFiltering, withPaginationRowModel = !manualPagination, - withSortedRowModel = !manualSorting, + withSortedRowModel = !manualSorting && !manualPagination, withFacetedRowModel = !manualFiltering, withExpandedRowModel = false, } = options @@ -370,6 +371,11 @@ export function useDataTable(options: UseDataTableOptions) { (totalCount !== undefined ? Math.ceil(totalCount / pagination.pageSize) : undefined) + const resolvedEnableSorting = + options.enableSorting ?? + (!manualPagination || + Boolean(options.sorting) || + Boolean(options.onSortingChange)) const table = useReactTable({ data, @@ -387,6 +393,7 @@ export function useDataTable(options: UseDataTableOptions) { pagination, }, enableRowSelection: options.enableRowSelection, + enableSorting: resolvedEnableSorting, getRowId: options.getRowId, getSubRows: options.getSubRows, globalFilterFn: options.globalFilterFn, diff --git a/web/default/src/features/models/components/models-table.tsx b/web/default/src/features/models/components/models-table.tsx index 9a2a192b..dd0b73e8 100644 --- a/web/default/src/features/models/components/models-table.tsx +++ b/web/default/src/features/models/components/models-table.tsx @@ -172,7 +172,6 @@ export function ModelsTable() { onPaginationChange, onGlobalFilterChange, manualPagination: true, - manualSorting: true, manualFiltering: true, ensurePageInRange, }) diff --git a/web/default/src/features/users/api.ts b/web/default/src/features/users/api.ts index 325ea93f..f3f2ba91 100644 --- a/web/default/src/features/users/api.ts +++ b/web/default/src/features/users/api.ts @@ -40,8 +40,15 @@ import type { export async function getUsers( params: GetUsersParams = {} ): Promise { - const { p = 1, page_size = 10 } = params - const res = await api.get(`/api/user/?p=${p}&page_size=${page_size}`) + const { p = 1, page_size = 10, sort_by, sort_order } = params + const res = await api.get('/api/user/', { + params: { + p, + page_size, + sort_by, + sort_order, + }, + }) return res.data } @@ -58,6 +65,8 @@ export async function searchUsers( status = '', p = 1, page_size = 10, + sort_by, + sort_order, } = params const queryParams = new URLSearchParams() queryParams.set('keyword', keyword) @@ -66,6 +75,8 @@ export async function searchUsers( if (status) queryParams.set('status', status) queryParams.set('p', String(p)) queryParams.set('page_size', String(page_size)) + if (sort_by) queryParams.set('sort_by', sort_by) + if (sort_order) queryParams.set('sort_order', sort_order) const res = await api.get(`/api/user/search?${queryParams.toString()}`) return res.data } diff --git a/web/default/src/features/users/components/users-table.tsx b/web/default/src/features/users/components/users-table.tsx index 33838a7d..cbf4ef29 100644 --- a/web/default/src/features/users/components/users-table.tsx +++ b/web/default/src/features/users/components/users-table.tsx @@ -18,6 +18,8 @@ For commercial licensing, please contact support@quantumnous.com */ import { useQuery } from '@tanstack/react-query' import { getRouteApi } from '@tanstack/react-router' +import type { OnChangeFn, SortingState } from '@tanstack/react-table' +import { useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' import { toast } from 'sonner' @@ -37,13 +39,22 @@ import { getUserRoleOptions, isUserDeleted, } from '../constants' -import type { User } from '../types' +import type { User, UserSortBy } from '../types' import { DataTableBulkActions } from './data-table-bulk-actions' import { useUsersColumns } from './users-columns' import { useUsers } from './users-provider' const route = getRouteApi('/_authenticated/users/') +const USER_SORTABLE_COLUMNS = new Set([ + 'id', + 'username', + 'quota', + 'group', + 'created_at', + 'last_login_at', +]) + function isDisabledUserRow(user: User) { return isUserDeleted(user) || user.status === USER_STATUS.DISABLED } @@ -53,6 +64,7 @@ export function UsersTable() { const columns = useUsersColumns() const { refreshTrigger } = useUsers() const isMobile = useMediaQuery('(max-width: 640px)') + const [sorting, setSorting] = useState([]) const { globalFilter, @@ -85,6 +97,28 @@ export function UsersTable() { (columnFilters.find((filter) => filter.id === 'group')?.value as string) ?? '' + const sortParams = useMemo(() => { + const activeSort = sorting[0] + if ( + !activeSort || + !USER_SORTABLE_COLUMNS.has(activeSort.id as UserSortBy) + ) { + return {} + } + + return { + sort_by: activeSort.id as UserSortBy, + sort_order: activeSort.desc ? 'desc' : 'asc', + } as const + }, [sorting]) + + const handleSortingChange: OnChangeFn = (updater) => { + setSorting(updater) + if (pagination.pageIndex > 0) { + onPaginationChange({ ...pagination, pageIndex: 0 }) + } + } + // Fetch data with React Query const { data, isLoading, isFetching } = useQuery({ queryKey: [ @@ -95,6 +129,7 @@ export function UsersTable() { statusFilter, roleFilter, groupFilter, + sortParams, refreshTrigger, ], queryFn: async () => { @@ -104,6 +139,7 @@ export function UsersTable() { const params = { p: pagination.pageIndex + 1, page_size: pagination.pageSize, + ...sortParams, } const result = @@ -141,6 +177,7 @@ export function UsersTable() { columnFilters, globalFilter, pagination, + sorting, globalFilterFn: (row, _columnId, filterValue) => { const searchValue = String(filterValue).toLowerCase() const fields = [ @@ -157,8 +194,10 @@ export function UsersTable() { onPaginationChange, onGlobalFilterChange, onColumnFiltersChange, + onSortingChange: handleSortingChange, manualPagination: true, manualFiltering: true, + manualSorting: true, totalCount: data?.total || 0, ensurePageInRange, }) diff --git a/web/default/src/features/users/types.ts b/web/default/src/features/users/types.ts index a6f12514..ac73c975 100644 --- a/web/default/src/features/users/types.ts +++ b/web/default/src/features/users/types.ts @@ -78,9 +78,21 @@ export interface ApiResponse { data?: T } +export type UserSortBy = + | 'id' + | 'username' + | 'quota' + | 'group' + | 'created_at' + | 'last_login_at' + +export type UserSortOrder = 'asc' | 'desc' + export interface GetUsersParams { p?: number page_size?: number + sort_by?: UserSortBy + sort_order?: UserSortOrder } export interface GetUsersResponse { @@ -101,6 +113,8 @@ export interface SearchUsersParams { status?: string p?: number page_size?: number + sort_by?: UserSortBy + sort_order?: UserSortOrder } export interface UserFormData {