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).
This commit is contained in:
+4
-2
@@ -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
|
||||
|
||||
+62
-4
@@ -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
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
@@ -49,6 +49,7 @@ type DataTableFeatureOptions<TData> = Pick<
|
||||
| 'manualFiltering'
|
||||
| 'manualPagination'
|
||||
| 'manualSorting'
|
||||
| 'enableSorting'
|
||||
| 'enableColumnResizing'
|
||||
>
|
||||
|
||||
@@ -292,7 +293,7 @@ export function useDataTable<TData>(options: UseDataTableOptions<TData>) {
|
||||
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<TData>(options: UseDataTableOptions<TData>) {
|
||||
(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<TData>(options: UseDataTableOptions<TData>) {
|
||||
pagination,
|
||||
},
|
||||
enableRowSelection: options.enableRowSelection,
|
||||
enableSorting: resolvedEnableSorting,
|
||||
getRowId: options.getRowId,
|
||||
getSubRows: options.getSubRows,
|
||||
globalFilterFn: options.globalFilterFn,
|
||||
|
||||
@@ -172,7 +172,6 @@ export function ModelsTable() {
|
||||
onPaginationChange,
|
||||
onGlobalFilterChange,
|
||||
manualPagination: true,
|
||||
manualSorting: true,
|
||||
manualFiltering: true,
|
||||
ensurePageInRange,
|
||||
})
|
||||
|
||||
+13
-2
@@ -40,8 +40,15 @@ import type {
|
||||
export async function getUsers(
|
||||
params: GetUsersParams = {}
|
||||
): Promise<GetUsersResponse> {
|
||||
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
|
||||
}
|
||||
|
||||
+40
-1
@@ -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<UserSortBy>([
|
||||
'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<SortingState>([])
|
||||
|
||||
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<SortingState> = (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,
|
||||
})
|
||||
|
||||
+14
@@ -78,9 +78,21 @@ export interface ApiResponse<T = unknown> {
|
||||
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 {
|
||||
|
||||
Reference in New Issue
Block a user