feat: 优化分页组件 - #1298
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Common
participant Model
Client->>Controller: GET /users?p=1&page_size=10
Controller->>Common: GetPageQuery(c)
Common-->>Controller: PageInfo or error
Controller->>Model: GetAllUsers(pageInfo)
Model-->>Controller: users, total, error
Controller->>PageInfo: SetTotal(total), SetItems(users)
Controller-->>Client: JSON response { data: PageInfo }
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
controller/user.go (1)
276-307: Maintain consistency with SearchUsers function.The
SearchUsersfunction still uses the old manual parameter parsing approach whileGetAllUsersnow uses the newPageInfoabstraction. This creates inconsistency in the codebase.Consider refactoring
SearchUsersto also usecommon.GetPageQuery()for consistency:func SearchUsers(c *gin.Context) { keyword := c.Query("keyword") group := c.Query("group") - p, _ := strconv.Atoi(c.Query("p")) - pageSize, _ := strconv.Atoi(c.Query("page_size")) - if p < 1 { - p = 1 - } - if pageSize < 0 { - pageSize = common.ItemsPerPage + pageInfo, err := common.GetPageQuery(c) + if err != nil { + c.JSON(http.StatusOK, gin.H{ + "success": false, + "message": "分页参数解析失败", + }) + return } - startIdx := (p - 1) * pageSize - users, total, err := model.SearchUsers(keyword, group, startIdx, pageSize) + users, total, err := model.SearchUsers(keyword, group, pageInfo.GetStartIdx(), pageInfo.GetPageSize())
🧹 Nitpick comments (3)
common/page_info.go (1)
11-12: Consider utilizing the timestamp fields.The
StartTimestampandEndTimestampfields are defined but not used inGetPageQuery(). Consider either using them for filtering or documenting their intended purpose.Do you want me to implement timestamp filtering logic or should these fields be removed if they're not needed?
model/user.go (1)
137-137: Consider adding timestamp filtering.The PageInfo struct includes
StartTimestampandEndTimestampfields, but they're not being used for filtering users. Consider adding timestamp-based filtering if these fields are intended to be functional.// Get paginated users within same transaction + query := tx.Unscoped().Order("id desc") + if pageInfo.StartTimestamp > 0 { + query = query.Where("created_at >= ?", time.Unix(pageInfo.StartTimestamp, 0)) + } + if pageInfo.EndTimestamp > 0 { + query = query.Where("created_at <= ?", time.Unix(pageInfo.EndTimestamp, 0)) + } + err = query.Limit(pageInfo.GetPageSize()).Offset(pageInfo.GetStartIdx()).Omit("password").Find(&users).Error - err = tx.Unscoped().Order("id desc").Limit(pageInfo.GetPageSize()).Offset(pageInfo.GetStartIdx()).Omit("password").Find(&users).Errorcontroller/user.go (1)
249-256: Use consistent language for error messages.The error message "parse page query failed" is in English while other error messages in this controller are in Chinese. This creates inconsistency in the API responses.
- "message": "parse page query failed", + "message": "分页参数解析失败",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
common/page_info.go(1 hunks)controller/user.go(1 hunks)model/user.go(2 hunks)
🔇 Additional comments (2)
model/user.go (1)
117-117: LGTM! Clean refactoring to use PageInfo.The function signature change properly utilizes the new pagination abstraction while maintaining the same functionality.
controller/user.go (1)
266-272: Verify the API response format change impact.The response format has changed from separate fields (
items,total,page,page_size) to embedding the entirePageInfoobject. This is a breaking change that could affect existing API consumers.#!/bin/bash # Description: Search for API consumers that might be affected by this response format change # Expected: Find references to the old response fields echo "Searching for API consumers that might be affected..." rg "\.data\.(items|total|page|page_size)" -A 2 -B 2 rg "response.*\.data\." -A 2 -B 2 rg "GetAllUsers.*response" -A 5 -B 5
| func (p *PageInfo) GetStartIdx() int { | ||
| return (p.Page - 1) * p.PageSize | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation to prevent negative indices.
The GetStartIdx() method could return negative values if Page is 0, which could cause issues with database queries.
func (p *PageInfo) GetStartIdx() int {
+ if p.Page < 1 {
+ return 0
+ }
return (p.Page - 1) * p.PageSize
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (p *PageInfo) GetStartIdx() int { | |
| return (p.Page - 1) * p.PageSize | |
| } | |
| func (p *PageInfo) GetStartIdx() int { | |
| if p.Page < 1 { | |
| return 0 | |
| } | |
| return (p.Page - 1) * p.PageSize | |
| } |
🤖 Prompt for AI Agents
In common/page_info.go around lines 18 to 20, the GetStartIdx() method can
return a negative index if Page is 0. Add validation to check if Page is less
than 1 and return 0 in that case to prevent negative indices, ensuring the
method always returns a non-negative start index.
| } | ||
|
|
||
| if pageInfo.PageSize == 0 { | ||
| pageInfo.PageSize = ItemsPerPage |
There was a problem hiding this comment.
❌ Incorrect review comment
Fix the undefined constant reference.
The ItemsPerPage constant is referenced but not defined in this file or imported from the common package.
🏁 Script executed:
#!/bin/bash
# Description: Search for ItemsPerPage definition in the codebase
# Expected: Find where ItemsPerPage is defined
rg "ItemsPerPage.*=" -A 2 -B 2Length of output: 215
Fix the undefined constant reference.
The ItemsPerPage constant is referenced but not defined in this file or imported from the common package.
#!/bin/bash
# Description: Search for ItemsPerPage definition in the codebase
# Expected: Find where ItemsPerPage is defined
rg "ItemsPerPage.*=" -A 2 -B 2🤖 Prompt for AI Agents
In common/page_info.go at line 59, the constant ItemsPerPage is used but not
defined or imported. Locate the definition of ItemsPerPage in the codebase, then
either define it in this file or import it properly from the package where it is
declared. Ensure the constant is accessible in this file to fix the undefined
reference error.
| if pageInfo.PageSize == 0 { | ||
| pageInfo.PageSize = ItemsPerPage | ||
| } | ||
| return pageInfo, nil |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for PageSize.
There's no validation to prevent negative or excessively large PageSize values, which could cause performance issues or security concerns.
if pageInfo.PageSize == 0 {
pageInfo.PageSize = ItemsPerPage
+ } else if pageInfo.PageSize < 0 || pageInfo.PageSize > 1000 {
+ return nil, errors.New("invalid page_size: must be between 1 and 1000")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if pageInfo.PageSize == 0 { | |
| pageInfo.PageSize = ItemsPerPage | |
| } | |
| return pageInfo, nil | |
| if pageInfo.PageSize == 0 { | |
| pageInfo.PageSize = ItemsPerPage | |
| } else if pageInfo.PageSize < 0 || pageInfo.PageSize > 1000 { | |
| return nil, errors.New("invalid page_size: must be between 1 and 1000") | |
| } | |
| return pageInfo, nil |
🤖 Prompt for AI Agents
In common/page_info.go around lines 58 to 61, add validation to ensure PageSize
is neither negative nor excessively large. Before setting the default PageSize,
check if PageSize is less than or equal to zero or exceeds a defined maximum
limit, and handle these cases by either setting it to a safe default or
returning an error. This prevents potential performance or security issues
caused by invalid PageSize values.
The hardcoded codex CLI version (0.104.0) causes upstream rejection when using gpt-5.5 with compact, as the server treats the request as an outdated client and returns 400/502. Update codexCLIVersion, codexCLIUserAgent, and openAICodexProbeVersion to 0.125.0 to match the current Codex CLI release. Fixes QuantumNous#1933, QuantumNous#1887, QuantumNous#1865 Related: QuantumNous#1609, QuantumNous#1298, QuantumNous#849
2 participants
Summary by CodeRabbit
New Features
Refactor