Feature/v1 governance#6236
Conversation
T1 部署（docker-compose.yml）： - 切到 MySQL 8（PRD 4.1），新增 mysql 服务 + 健康检查 - 加 LOG_CONTENT_ENABLED=false（不存请求/响应正文） - 加 INITIAL_POOL_BALANCE（初始预算池，元，待定来源） T2 数据模型： - User 加 RoleLevel(0/10/100) 与 Department（部门标签） - 新增 BudgetPool（单行 id=1）/ QuotaApplication / AuditLog 三张表 - 在 migrateDB 与 migrateDBFast 两处注册 AutoMigrate - SeedBudgetPoolIfEmpty 按 env 注入初始预算池 编译验证：go build ./... 通过（占位前端 dist 仅本地验证用）
T3 权限中间件: - model/role_level.go: 治理角色层级常量(0/10/100)，与原生 Role 共存 - middleware/governance.go: RequireRole / GovernanceAuth / 部门判断 helper - middleware/auth.go: authHelper 注入 role_level/department 到 context (session 优先，token 路径回退 user，零额外 DB 开销) - controller/user.go: 登录写入 role_level/department 到 session T4 模型白名单: - model/whitelist.go: GroupAllowsModel 包装原生分组可用模型 - 核实原生已按 abilities 表(group+model+enabled)实现分组白名单，v1 复用配置即可 T5 预算池与额度审批: - controller/quota.go: POST /api/quota/apply + /api/quota/approve - GORM 事务 + SELECT ... FOR UPDATE 行锁防并发超拨 - 自审批 handler+事务双校验；部门管理员仅本部门；货币单位=元(整元拨付) - model/audit_log.go: WriteAuditLog 手动埋点(不阻塞业务) - model/quota_application.go: GetQuotaApplicationById
- model/audit_log.go: WriteAuditLog 改为 gopool 异步 + 3 次重试 + 失败告警(RT3，不阻塞业务) - model/audit_log.go: 新增 SearchAuditLogs 分页检索(audit_log 表) - model/log.go: Log 扩展 hit_whitelist/department 字段(非破坏性，relay 埋点为后续) - middleware/governance.go: 新增 SuperAdminAuth(仅超管可见审计) - controller/audit_governance.go: GET /api/audit 检索 handler - router/api-router.go: 接入 GET /api/audit 路由
- deploy/backup.sh: 每日 mysqldump 容器化备份 + gzip + 保留期清理(默认30天) - deploy/restore-drill.md: 隔离库恢复演练步骤与校验清单(上线前至少一次)
WalkthroughAdds governance roles, quota application and approval workflows, audit-log storage and retrieval, an administrative audit page, MySQL deployment configuration, automated backups, and restore-drill documentation. ChangesGovernance workflows
MySQL backup and recovery
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Applicant
participant ApplyQuota
participant ApproveQuota
participant QuotaApplication
participant BudgetPool
Applicant->>ApplyQuota: submit quota amount and reason
ApplyQuota->>QuotaApplication: create pending application
ApproveQuota->>QuotaApplication: re-fetch pending application
ApproveQuota->>BudgetPool: lock pool and check balance
ApproveQuota->>QuotaApplication: update approval status
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (3)
docker-compose.yml (1)
51-55: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLeverage the MySQL healthcheck for startup ordering.
Since a healthcheck was added to the
mysqlservice, updating thedepends_onblock to use the object syntax ensuresnew-apiwaits for the database to be fully healthy before starting. This prevents database connection errors and crash loops during application startup.♻️ Proposed refactor for startup ordering
depends_on: - - redis - - mysql -# - postgres # v1 使用 MySQL，已弃用 postgres -# - clickhouse # Uncomment if using ClickHouse for LOG_SQL_DSN + redis: + condition: service_started + mysql: + condition: service_healthy +# clickhouse: +# condition: service_started🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose.yml` around lines 51 - 55, Update the new-api service’s depends_on configuration to object syntax and require the mysql dependency to reach the healthy condition before startup, while preserving the existing redis dependency and commented optional services.deploy/backup.sh (1)
52-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
findoverlsfor programmatic file counting.As highlighted by static analysis, using
findhandles filenames robustly and avoids potential quirks with non-alphanumeric characters.♻️ Proposed refactor for file counting
-echo "[$(date '+%F %T')] 完成。当前备份文件数：$(ls -1 "$BACKUP_DIR"/newapi-*.sql.gz 2>/dev/null | wc -l)" +echo "[$(date '+%F %T')] 完成。当前备份文件数：$(find "$BACKUP_DIR" -maxdepth 1 -name 'newapi-*.sql.gz' 2>/dev/null | wc -l)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/backup.sh` at line 52, Replace the ls-based count in the completion message with a find command scoped to BACKUP_DIR and matching newapi-*.sql.gz, then count its results while preserving the existing displayed count and timestamp.Source: Linters/SAST tools
controller/audit_governance.go (1)
17-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInline the single-caller timestamp parser.
parseInt64is mechanical logic used only byGetAuditLogs; keeping the parsing branches in the handler avoids adding another generic controller-package symbol.Proposed refactor
+ var from, to int64 + if value, err := strconv.ParseInt(c.Query("from"), 10, 64); err == nil { + from = value + } + if value, err := strconv.ParseInt(c.Query("to"), 10, 64); err == nil { + to = value + } + q := model.AuditLogQuery{ ActorName: c.Query("actor_name"), Action: c.Query("action"), TargetType: c.Query("target_type"), Keyword: c.Query("keyword"), - From: parseInt64(c.Query("from")), - To: parseInt64(c.Query("to")), + From: from, + To: to, StartIdx: pageInfo.GetStartIdx(), PageSize: pageInfo.GetPageSize(), }Remove Lines 46–56 afterward.
As per coding guidelines, avoid package-level helpers with only one caller unless they express a stable business concept.
Also applies to: 46-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/audit_governance.go` around lines 17 - 28, Inline the timestamp parsing logic from the single-caller parseInt64 helper directly into GetAuditLogs when constructing AuditLogQuery, preserving its existing parsing behavior for From and To. After inlining, remove the package-level parseInt64 function and any now-unused related code.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/quota.go`:
- Around line 42-45: Update the amount validation in the quota request handler
around the req.Amount check to reject fractional yuan values before any
allocation or rounding occurs. Accept only positive whole-yuan amounts,
preserving the existing bad-request response for invalid values, and apply the
same validation to the additional amount-processing path referenced by the
comment.
- Around line 197-200: Update the response in the quota decision handler to
return the application’s persisted status value (`approved` or `rejected`)
instead of `req.Decision` (`approve` or `reject`). Reuse the status assigned
when saving the application, while preserving the existing success response
structure and application_id.
- Around line 49-62: Update the department assignment in the quota application
creation flow to always derive the department from the authenticated session via
c.GetString("department"), ignoring req.Dept for ordinary applicants. Preserve
the existing ApplicantId, ApplicantName, and other application fields, and do
not add an override path here.
- Around line 129-171: The transaction’s initial quota-application lookup must
lock the application row before validating its pending status. Update the appTx
query in the transaction callback to use a FOR UPDATE row lock, ensuring both
approval and rejection paths serialize on the application row and prevent
duplicate processing.
In `@deploy/backup.sh`:
- Around line 36-38: Update the mysqldump invocation in the backup command to
remove the password from the -p command-line argument and provide it through the
MYSQL_PWD environment variable for the docker exec process. Preserve the
existing MYSQL_USER, MYSQL_DB, dump options, gzip pipeline, and output behavior.
- Line 27: Update the MYSQL_PASS extraction fallback to match Go MySQL DSNs such
as root:password@tcp(...), use sed -nE so unmatched input produces an empty
result, and return the password capture group rather than the username. Preserve
the existing MYSQL_PASS precedence and SQL_DSN fallback behavior.
In `@docker-compose.yml`:
- Line 99: Update the MySQL healthcheck to use CMD-SHELL with MYSQL_PWD
populated from the container’s current MYSQL_ROOT_PASSWORD, and invoke
mysqladmin without the hardcoded password so the check remains valid when the
configured root password changes.
In `@middleware/governance.go`:
- Around line 43-73: Move governance authorization into the authentication
chain: update GovernanceAuth and SuperAdminAuth to pass their required role
levels to authHelper, then extend authHelper to evaluate those roles immediately
before c.Next(). Preserve existing authentication, abort behavior, and
finishAdminAudit handling, and remove the post-authHelper RequireRole calls so
handlers cannot execute before authorization.
In `@model/audit_log.go`:
- Around line 50-57: Update the retry loop in the audit write flow around
DB.Create to wait before each retry after a failed attempt, using a short
escalating delay based on the attempt number; import and use the time package,
while preserving the existing maximum-attempt count, error logging, and
immediate return on success.
In `@model/budget_pool.go`:
- Around line 35-37: Update the BudgetPool seed creation to use GORM’s
clause.OnConflict with DB.Create, importing gorm.io/gorm/clause as needed.
Preserve the existing pool values and return the create operation’s error while
ignoring duplicate primary-key conflicts during concurrent startup.
In `@model/log.go`:
- Around line 81-84: Remove the database-specific gorm type tag from the
HitWhitelist field in model Log, leaving the *bool type and other JSON metadata
intact so GORM can map it per SQL dialect. Do not change the Department field or
existing write paths.
---
Nitpick comments:
In `@controller/audit_governance.go`:
- Around line 17-28: Inline the timestamp parsing logic from the single-caller
parseInt64 helper directly into GetAuditLogs when constructing AuditLogQuery,
preserving its existing parsing behavior for From and To. After inlining, remove
the package-level parseInt64 function and any now-unused related code.
In `@deploy/backup.sh`:
- Line 52: Replace the ls-based count in the completion message with a find
command scoped to BACKUP_DIR and matching newapi-*.sql.gz, then count its
results while preserving the existing displayed count and timestamp.
In `@docker-compose.yml`:
- Around line 51-55: Update the new-api service’s depends_on configuration to
object syntax and require the mysql dependency to reach the healthy condition
before startup, while preserving the existing redis dependency and commented
optional services.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f130cfe-c4b9-4b74-8842-c9e2da2ec852
📒 Files selected for processing (17)
controller/audit_governance.gocontroller/quota.gocontroller/user.godeploy/backup.shdeploy/restore-drill.mddocker-compose.ymlmiddleware/auth.gomiddleware/governance.gomodel/audit_log.gomodel/budget_pool.gomodel/log.gomodel/main.gomodel/quota_application.gomodel/role_level.gomodel/user.gomodel/whitelist.gorouter/api-router.go
| if req.Amount <= 0 { | ||
| c.JSON(http.StatusBadRequest, gin.H{"success": false, "message": "金额必须大于 0"}) | ||
| return | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject fractional amounts instead of silently rounding them.
An application for 0.49 yuan is approved for zero, while 1.50 yuan credits and deducts two yuan. This makes the recorded amount and actual allocation disagree.
Proposed fix for whole-yuan v1 accounting
- if req.Amount <= 0 {
- c.JSON(http.StatusBadRequest, gin.H{"success": false, "message": "金额必须大于 0"})
+ if req.Amount <= 0 || math.Trunc(req.Amount) != req.Amount {
+ c.JSON(http.StatusBadRequest, gin.H{"success": false, "message": "金额必须为正整数元"})
return
}Also applies to: 153-163
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/quota.go` around lines 42 - 45, Update the amount validation in
the quota request handler around the req.Amount check to reject fractional yuan
values before any allocation or rounding occurs. Accept only positive whole-yuan
amounts, preserving the existing bad-request response for invalid values, and
apply the same validation to the additional amount-processing path referenced by
the comment.
| dept := req.Dept | ||
| if dept == "" { | ||
| dept = c.GetString("department") // 未显式传 dept 时取当前用户部门 | ||
| } | ||
|
|
||
| app := model.QuotaApplication{ | ||
| ApplicantId: actorId, | ||
| ApplicantName: actorName, | ||
| Dept: dept, | ||
| Amount: req.Amount, | ||
| Reason: req.Reason, | ||
| Status: "pending", | ||
| CreatedAt: time.Now().Unix(), | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not let ordinary applicants choose their department.
req.Dept overrides the authenticated department, allowing users to place requests in another department’s approval scope. Derive it from the session; use a separately authorized on-behalf flow if overrides are required.
Proposed fix
- dept := req.Dept
- if dept == "" {
- dept = c.GetString("department")
- }
+ dept := c.GetString("department")📝 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.
| dept := req.Dept | |
| if dept == "" { | |
| dept = c.GetString("department") // 未显式传 dept 时取当前用户部门 | |
| } | |
| app := model.QuotaApplication{ | |
| ApplicantId: actorId, | |
| ApplicantName: actorName, | |
| Dept: dept, | |
| Amount: req.Amount, | |
| Reason: req.Reason, | |
| Status: "pending", | |
| CreatedAt: time.Now().Unix(), | |
| } | |
| dept := c.GetString("department") | |
| app := model.QuotaApplication{ | |
| ApplicantId: actorId, | |
| ApplicantName: actorName, | |
| Dept: dept, | |
| Amount: req.Amount, | |
| Reason: req.Reason, | |
| Status: "pending", | |
| CreatedAt: time.Now().Unix(), | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/quota.go` around lines 49 - 62, Update the department assignment
in the quota application creation flow to always derive the department from the
authenticated session via c.GetString("department"), ignoring req.Dept for
ordinary applicants. Preserve the existing ApplicantId, ApplicantName, and other
application fields, and do not add an override path here.
| txErr := model.DB.Transaction(func(tx *gorm.DB) error { | ||
| var appTx model.QuotaApplication | ||
| if e := tx.Where("id = ? AND status = ?", req.ApplicationId, "pending").First(&appTx).Error; e != nil { | ||
| return ErrAppNotFoundOrHandled | ||
| } | ||
| // 事务内二次自审批校验（并发安全） | ||
| if appTx.ApplicantId == actorId { | ||
| return ErrSelfApprove | ||
| } | ||
| now := time.Now().Unix() | ||
| if req.Decision == "reject" { | ||
| appTx.Status = "rejected" | ||
| appTx.ApproverId = actorId | ||
| appTx.ApproverName = actorName | ||
| appTx.DecidedAt = now | ||
| appTx.RejectReason = req.RejectReason | ||
| return tx.Save(&appTx).Error | ||
| } | ||
| // approve：行锁预算池（id=1）防止并发超拨 | ||
| var pool model.BudgetPool | ||
| if e := tx.Clauses(clause.Locking{Strength: "UPDATE"}). | ||
| Where("id = ?", 1).First(&pool).Error; e != nil { | ||
| return e | ||
| } | ||
| // 货币单位=元：预算池(decimal)与个人余额(Quota int)统一以「元」记账。 | ||
| // 为与 User.Quota(int) 对齐，拨付按整元四舍五入（角分在 v1 暂不保留，待生产决策）。 | ||
| deltaYuan := int64(math.Round(appTx.Amount)) | ||
| if pool.TotalBalance < float64(deltaYuan) { | ||
| return ErrBudgetInsufficient | ||
| } | ||
| if e := tx.Model(&model.User{}).Where("id = ?", appTx.ApplicantId). | ||
| UpdateColumn("quota", gorm.Expr("quota + ?", deltaYuan)).Error; e != nil { | ||
| return e | ||
| } | ||
| pool.TotalBalance -= float64(deltaYuan) | ||
| appTx.Status = "approved" | ||
| appTx.ApproverId = actorId | ||
| appTx.ApproverName = actorName | ||
| appTx.DecidedAt = now | ||
| if e := tx.Save(&pool).Error; e != nil { | ||
| return e | ||
| } | ||
| return tx.Save(&appTx).Error |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file first, then inspect the transaction block and related models.
ast-grep outline controller/quota.go --view expanded || true
echo
echo '--- controller/quota.go (around the cited lines) ---'
sed -n '100,220p' controller/quota.go
echo
echo '--- search for quota application / status transitions ---'
rg -n "QuotaApplication|pending|approved|rejected|RejectReason|BudgetPool|quota \+" controller -SRepository: QuantumNous/new-api
Length of output: 10705
Lock the quota application row before checking pending. Two concurrent approvals can both pass the initial read, then serialize only on the budget-pool lock. The second transaction resumes with a stale appTx and can apply the same request twice. Use FOR UPDATE on the application row here and in the reject path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/quota.go` around lines 129 - 171, The transaction’s initial
quota-application lookup must lock the application row before validating its
pending status. Update the appTx query in the transaction callback to use a FOR
UPDATE row lock, ensuring both approval and rejection paths serialize on the
application row and prevent duplicate processing.
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": true, | ||
| "data": gin.H{"application_id": app.Id, "status": req.Decision}, | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return the persisted status values.
The API currently returns approve or reject, although the application is stored as approved or rejected. This breaks clients expecting the model’s status contract.
Proposed fix
+ status := "approved"
+ if req.Decision == "reject" {
+ status = "rejected"
+ }
c.JSON(http.StatusOK, gin.H{
"success": true,
- "data": gin.H{"application_id": app.Id, "status": req.Decision},
+ "data": gin.H{"application_id": app.Id, "status": status},
})📝 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.
| c.JSON(http.StatusOK, gin.H{ | |
| "success": true, | |
| "data": gin.H{"application_id": app.Id, "status": req.Decision}, | |
| }) | |
| status := "approved" | |
| if req.Decision == "reject" { | |
| status = "rejected" | |
| } | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": true, | |
| "data": gin.H{"application_id": app.Id, "status": status}, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/quota.go` around lines 197 - 200, Update the response in the quota
decision handler to return the application’s persisted status value (`approved`
or `rejected`) instead of `req.Decision` (`approve` or `reject`). Reuse the
status assigned when saving the application, while preserving the existing
success response structure and application_id.
| fi | ||
| # 兜底默认值（与 docker-compose.yml 默认一致） | ||
| MYSQL_USER="${MYSQL_USER:-root}" | ||
| MYSQL_PASS="${MYSQL_PASS:-${SQL_DSN:+$(echo "$SQL_DSN" | sed -E 's#.*://([^:]+):([^@]+)@.*#\1#')}}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix password extraction from the MySQL DSN.
The regular expression .*://([^:]+):([^@]+)@.* expects a :// scheme, which Go's MySQL DSN format (e.g., root:123456@tcp(...)) does not contain. Because sed -E fails to match, it outputs the entire unchanged DSN string, setting $MYSQL_PASS to the full DSN and breaking authentication. Furthermore, even if it did match, \1 extracts the username, not the password.
Use -nE alongside \2 to accurately extract the password and safely return an empty string (triggering your fallback) if no match occurs.
🐛 Proposed fix for the regex
-MYSQL_PASS="${MYSQL_PASS:-${SQL_DSN:+$(echo "$SQL_DSN" | sed -E 's#.*://([^:]+):([^@]+)@.*#\1#')}}"
+MYSQL_PASS="${MYSQL_PASS:-${SQL_DSN:+$(echo "$SQL_DSN" | sed -nE 's#^([^:]+):([^@]+)@.*#\2#p')}}"📝 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.
| MYSQL_PASS="${MYSQL_PASS:-${SQL_DSN:+$(echo "$SQL_DSN" | sed -E 's#.*://([^:]+):([^@]+)@.*#\1#')}}" | |
| MYSQL_PASS="${MYSQL_PASS:-${SQL_DSN:+$(echo "$SQL_DSN" | sed -nE 's#^([^:]+):([^@]+)@.*#\2#p')}}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/backup.sh` at line 27, Update the MYSQL_PASS extraction fallback to
match Go MySQL DSNs such as root:password@tcp(...), use sed -nE so unmatched
input produces an empty result, and return the password capture group rather
than the username. Preserve the existing MYSQL_PASS precedence and SQL_DSN
fallback behavior.
| networks: | ||
| - new-api-network | ||
| healthcheck: | ||
| test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "-p123456"] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Prevent healthcheck failures due to a hardcoded password.
If the user changes MYSQL_ROOT_PASSWORD, the healthcheck will fail because the password here is hardcoded to 123456. Using CMD-SHELL allows you to pass the password securely via the MYSQL_PWD environment variable, ensuring the healthcheck evaluates the current password inside the container.
🐛 Proposed fix for the healthcheck
- test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "-p123456"]
+ test: ["CMD-SHELL", "MYSQL_PWD=$$MYSQL_ROOT_PASSWORD mysqladmin ping -h localhost -uroot"]📝 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.
| test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "-p123456"] | |
| test: ["CMD-SHELL", "MYSQL_PWD=$$MYSQL_ROOT_PASSWORD mysqladmin ping -h localhost -uroot"] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.yml` at line 99, Update the MySQL healthcheck to use CMD-SHELL
with MYSQL_PWD populated from the container’s current MYSQL_ROOT_PASSWORD, and
invoke mysqladmin without the hardcoded password so the check remains valid when
the configured root password changes.
| func GovernanceAuth() gin.HandlerFunc { | ||
| return func(c *gin.Context) { | ||
| authHelper(c, common.RoleCommonUser) // 至少需登录用户 | ||
| if c.IsAborted() { | ||
| return | ||
| } | ||
| RequireRole(model.RoleLevelDeptAdmin, model.RoleLevelSuperAdmin)(c) | ||
| } | ||
| } | ||
|
|
||
| // IsDeptAdmin 当前用户是否为部门管理员（RoleLevel=10）。 | ||
| func IsDeptAdmin(c *gin.Context) bool { | ||
| return c.GetInt("role_level") == model.RoleLevelDeptAdmin | ||
| } | ||
|
|
||
| // IsSuperAdmin 当前用户是否为超级管理员（RoleLevel=100）。 | ||
| func IsSuperAdmin(c *gin.Context) bool { | ||
| return c.GetInt("role_level") == model.RoleLevelSuperAdmin | ||
| } | ||
|
|
||
| // SuperAdminAuth 超管鉴权：先完成会话/令牌鉴权，再要求 RoleLevel=超级管理员。 | ||
| // 用于审计检索等仅超管可见的治理端点。 | ||
| func SuperAdminAuth() gin.HandlerFunc { | ||
| return func(c *gin.Context) { | ||
| authHelper(c, common.RoleCommonUser) | ||
| if c.IsAborted() { | ||
| return | ||
| } | ||
| RequireRole(model.RoleLevelSuperAdmin)(c) | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Authorization bypass: Role check runs after the route handler has executed.
GovernanceAuth and SuperAdminAuth call authHelper, which internally calls c.Next() to execute the remaining handler chain (including the actual route handler like ApproveQuota). Because RequireRole is evaluated after authHelper returns, the role check happens after the sensitive action has already been performed. This allows any authenticated user to bypass the role restriction and execute governance actions.
To fix this securely without duplicating authentication logic or breaking finishAdminAudit, the governance role check must occur inside authHelper before it calls c.Next().
🔒️ Proposed multi-file fix
Step 1: Update middleware/governance.go to pass the required roles to authHelper:
-func GovernanceAuth() gin.HandlerFunc {
- return func(c *gin.Context) {
- authHelper(c, common.RoleCommonUser) // 至少需登录用户
- if c.IsAborted() {
- return
- }
- RequireRole(model.RoleLevelDeptAdmin, model.RoleLevelSuperAdmin)(c)
- }
-}
+func GovernanceAuth() gin.HandlerFunc {
+ return func(c *gin.Context) {
+ authHelper(c, common.RoleCommonUser, model.RoleLevelDeptAdmin, model.RoleLevelSuperAdmin)
+ }
+}
// IsDeptAdmin 当前用户是否为部门管理员（RoleLevel=10）。
func IsDeptAdmin(c *gin.Context) bool {
return c.GetInt("role_level") == model.RoleLevelDeptAdmin
}
// IsSuperAdmin 当前用户是否为超级管理员（RoleLevel=100）。
func IsSuperAdmin(c *gin.Context) bool {
return c.GetInt("role_level") == model.RoleLevelSuperAdmin
}
-func SuperAdminAuth() gin.HandlerFunc {
- return func(c *gin.Context) {
- authHelper(c, common.RoleCommonUser)
- if c.IsAborted() {
- return
- }
- RequireRole(model.RoleLevelSuperAdmin)(c)
- }
-}
+func SuperAdminAuth() gin.HandlerFunc {
+ return func(c *gin.Context) {
+ authHelper(c, common.RoleCommonUser, model.RoleLevelSuperAdmin)
+ }
+}Step 2: Update middleware/auth.go to accept and evaluate the roles before c.Next():
Update the function signature (around line 37):
-func authHelper(c *gin.Context, minRole int) {
+func authHelper(c *gin.Context, minRole int, reqGovRoles ...int) {Insert the role check right before c.Next() is called (around line 176):
} else {
c.Set("department", "")
}
+
+ if len(reqGovRoles) > 0 {
+ level := c.GetInt("role_level")
+ ok := false
+ for _, l := range reqGovRoles {
+ if l == level {
+ ok = true
+ break
+ }
+ }
+ if !ok {
+ c.JSON(http.StatusForbidden, gin.H{
+ "success": false,
+ "message": common.TranslateMessage(c, i18n.MsgAuthInsufficientPrivilege),
+ })
+ c.Abort()
+ return
+ }
+ }
// 管理/root 写操作审计兜底：内聚在鉴权链路里，保证任何经过 AdminAuth/RootAuth📝 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 GovernanceAuth() gin.HandlerFunc { | |
| return func(c *gin.Context) { | |
| authHelper(c, common.RoleCommonUser) // 至少需登录用户 | |
| if c.IsAborted() { | |
| return | |
| } | |
| RequireRole(model.RoleLevelDeptAdmin, model.RoleLevelSuperAdmin)(c) | |
| } | |
| } | |
| // IsDeptAdmin 当前用户是否为部门管理员（RoleLevel=10）。 | |
| func IsDeptAdmin(c *gin.Context) bool { | |
| return c.GetInt("role_level") == model.RoleLevelDeptAdmin | |
| } | |
| // IsSuperAdmin 当前用户是否为超级管理员（RoleLevel=100）。 | |
| func IsSuperAdmin(c *gin.Context) bool { | |
| return c.GetInt("role_level") == model.RoleLevelSuperAdmin | |
| } | |
| // SuperAdminAuth 超管鉴权：先完成会话/令牌鉴权，再要求 RoleLevel=超级管理员。 | |
| // 用于审计检索等仅超管可见的治理端点。 | |
| func SuperAdminAuth() gin.HandlerFunc { | |
| return func(c *gin.Context) { | |
| authHelper(c, common.RoleCommonUser) | |
| if c.IsAborted() { | |
| return | |
| } | |
| RequireRole(model.RoleLevelSuperAdmin)(c) | |
| } | |
| } | |
| func GovernanceAuth() gin.HandlerFunc { | |
| return func(c *gin.Context) { | |
| authHelper(c, common.RoleCommonUser, model.RoleLevelDeptAdmin, model.RoleLevelSuperAdmin) | |
| } | |
| } | |
| // IsDeptAdmin 当前用户是否为部门管理员（RoleLevel=10）。 | |
| func IsDeptAdmin(c *gin.Context) bool { | |
| return c.GetInt("role_level") == model.RoleLevelDeptAdmin | |
| } | |
| // IsSuperAdmin 当前用户是否为超级管理员（RoleLevel=100）。 | |
| func IsSuperAdmin(c *gin.Context) bool { | |
| return c.GetInt("role_level") == model.RoleLevelSuperAdmin | |
| } | |
| // SuperAdminAuth 超管鉴权：先完成会话/令牌鉴权，再要求 RoleLevel=超级管理员。 | |
| // 用于审计检索等仅超管可见的治理端点。 | |
| func SuperAdminAuth() gin.HandlerFunc { | |
| return func(c *gin.Context) { | |
| authHelper(c, common.RoleCommonUser, model.RoleLevelSuperAdmin) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@middleware/governance.go` around lines 43 - 73, Move governance authorization
into the authentication chain: update GovernanceAuth and SuperAdminAuth to pass
their required role levels to authHelper, then extend authHelper to evaluate
those roles immediately before c.Next(). Preserve existing authentication, abort
behavior, and finishAdminAudit handling, and remove the post-authHelper
RequireRole calls so handlers cannot execute before authorization.
| for attempt := 0; attempt < auditWriteMaxRetry; attempt++ { | ||
| if err := DB.Create(&rec).Error; err != nil { | ||
| lastErr = err | ||
| common.SysLog(fmt.Sprintf("WriteAuditLog retry %d/%d failed: %v", attempt+1, auditWriteMaxRetry, err)) | ||
| continue | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a backoff delay to the database retry loop.
The current retry loop immediately retries DB.Create upon failure. If the database is experiencing a transient issue like a connection drop or temporary lock contention, back-to-back immediate retries will likely exhaust all 3 attempts in milliseconds before the database has time to recover. Adding a short delay (e.g., an escalating sleep) makes the retry mechanism far more resilient.
💤 Proposed fix to implement an escalating delay
for attempt := 0; attempt < auditWriteMaxRetry; attempt++ {
if err := DB.Create(&rec).Error; err != nil {
lastErr = err
common.SysLog(fmt.Sprintf("WriteAuditLog retry %d/%d failed: %v", attempt+1, auditWriteMaxRetry, err))
+ time.Sleep(time.Second * time.Duration(attempt+1))
continue
}
return
}Ensure time is imported at the top of your file:
import "time"📝 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.
| for attempt := 0; attempt < auditWriteMaxRetry; attempt++ { | |
| if err := DB.Create(&rec).Error; err != nil { | |
| lastErr = err | |
| common.SysLog(fmt.Sprintf("WriteAuditLog retry %d/%d failed: %v", attempt+1, auditWriteMaxRetry, err)) | |
| continue | |
| } | |
| return | |
| } | |
| for attempt := 0; attempt < auditWriteMaxRetry; attempt++ { | |
| if err := DB.Create(&rec).Error; err != nil { | |
| lastErr = err | |
| common.SysLog(fmt.Sprintf("WriteAuditLog retry %d/%d failed: %v", attempt+1, auditWriteMaxRetry, err)) | |
| time.Sleep(time.Second * time.Duration(attempt+1)) | |
| continue | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@model/audit_log.go` around lines 50 - 57, Update the retry loop in the audit
write flow around DB.Create to wait before each retry after a failed attempt,
using a short escalating delay based on the attempt number; import and use the
time package, while preserving the existing maximum-attempt count, error
logging, and immediate return on success.
| pool := BudgetPool{Id: 1, TotalBalance: balance, Currency: "CNY"} | ||
| return DB.Create(&pool).Error | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Prevent primary key conflict errors during concurrent startup.
In a multi-instance deployment (e.g., Docker Swarm, Kubernetes), multiple replicas might execute this seed function concurrently. They can both observe cnt == 0 and attempt to execute DB.Create(&pool). The slower replica will hit a primary key constraint violation (Duplicate entry '1') which will propagate as a startup error and crash the instance.
You can gracefully prevent this TOCTOU race condition by ignoring the conflict using GORM's built-in, cross-database clause.OnConflict.
🛡️ Proposed fix to gracefully handle concurrent seeds
- pool := BudgetPool{Id: 1, TotalBalance: balance, Currency: "CNY"}
- return DB.Create(&pool).Error
+ pool := BudgetPool{Id: 1, TotalBalance: balance, Currency: "CNY"}
+ return DB.Clauses(clause.OnConflict{DoNothing: true}).Create(&pool).ErrorAdd this import to the top of the file:
import "gorm.io/gorm/clause"📝 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.
| pool := BudgetPool{Id: 1, TotalBalance: balance, Currency: "CNY"} | |
| return DB.Create(&pool).Error | |
| } | |
| pool := BudgetPool{Id: 1, TotalBalance: balance, Currency: "CNY"} | |
| return DB.Clauses(clause.OnConflict{DoNothing: true}).Create(&pool).Error | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@model/budget_pool.go` around lines 35 - 37, Update the BudgetPool seed
creation to use GORM’s clause.OnConflict with DB.Create, importing
gorm.io/gorm/clause as needed. Preserve the existing pool values and return the
create operation’s error while ignoring duplicate primary-key conflicts during
concurrent startup.
| // v1 治理审计扩展（T6）：调用元数据补充字段，relay 埋点为后续工作（详见研发任务卡 T6）。 | ||
| // 仅新增列、不改既有写入路径，存量 Log 记录对应列为默认值（NULL / 空串）。 | ||
| HitWhitelist *bool `json:"hit_whitelist,omitempty" gorm:"type:tinyint(1)"` // 是否命中模型白名单（白名单拒绝路径=false） | ||
| Department string `json:"department,omitempty" gorm:"type:varchar(64);default:''"` // 调用者部门标签，P1 报表聚合预留 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the database-specific tinyint(1) type to prevent broken PostgreSQL migrations.
The type:tinyint(1) tag is explicitly MySQL-specific. Because PostgreSQL does not support the tinyint data type, retaining this tag will cause GORM to emit invalid SQL during schema migrations, crashing the startup sequence for any PostgreSQL deployment.
As per coding guidelines, migrations must support SQLite, MySQL, and PostgreSQL simultaneously without database-specific definitions. GORM natively maps *bool to the correct boolean/integer representation for each SQL dialect automatically, so removing the literal type definition safely restores cross-database compatibility.
🛠️ Proposed fix
// v1 治理审计扩展（T6）：调用元数据补充字段，relay 埋点为后续工作（详见研发任务卡 T6）。
// 仅新增列、不改既有写入路径，存量 Log 记录对应列为默认值（NULL / 空串）。
- HitWhitelist *bool `json:"hit_whitelist,omitempty" gorm:"type:tinyint(1)"` // 是否命中模型白名单（白名单拒绝路径=false）
+ HitWhitelist *bool `json:"hit_whitelist,omitempty"` // 是否命中模型白名单（白名单拒绝路径=false）
Department string `json:"department,omitempty" gorm:"type:varchar(64);default:''"` // 调用者部门标签，P1 报表聚合预留📝 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.
| // v1 治理审计扩展（T6）：调用元数据补充字段，relay 埋点为后续工作（详见研发任务卡 T6）。 | |
| // 仅新增列、不改既有写入路径，存量 Log 记录对应列为默认值（NULL / 空串）。 | |
| HitWhitelist *bool `json:"hit_whitelist,omitempty" gorm:"type:tinyint(1)"` // 是否命中模型白名单（白名单拒绝路径=false） | |
| Department string `json:"department,omitempty" gorm:"type:varchar(64);default:''"` // 调用者部门标签，P1 报表聚合预留 | |
| // v1 治理审计扩展（T6）：调用元数据补充字段，relay 埋点为后续工作（详见研发任务卡 T6）。 | |
| // 仅新增列、不改既有写入路径，存量 Log 记录对应列为默认值（NULL / 空串）。 | |
| HitWhitelist *bool `json:"hit_whitelist,omitempty"` // 是否命中模型白名单（白名单拒绝路径=false） | |
| Department string `json:"department,omitempty" gorm:"type:varchar(64);default:''"` // 调用者部门标签，P1 报表聚合预留 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@model/log.go` around lines 81 - 84, Remove the database-specific gorm type
tag from the HitWhitelist field in model Log, leaving the *bool type and other
JSON metadata intact so GORM can map it per SQL dialect. Do not change the
Department field or existing write paths.
Source: Coding guidelines
- 审计日志检索页：按 操作人/动作/关键词(详情)/时间区间 过滤，分页展示
- 路由 /console/audit 挂 AdminRoute；侧边栏新增「审计日志」入口
- 修复契约：响应按 {success,message,data:{page,page_size,total,items}} 解包
（原 res.data.items 应为 res.data.data.items），分页 p 传页码(1-based)
而非 offset，与 /api/log、/api/user 既有约定一致
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/classic/src/components/layout/SiderBar.jsx`:
- Around line 93-97: Restrict the `审计日志` menu entry in the sidebar to authorized
users by adding the established `isRoot()` or appropriate `isAdmin()` visibility
check, matching the protected route and API permissions. Consider moving the
entry from `workspaceItems` to `adminItems` so it appears only in the
administrator section.
In `@web/classic/src/pages/AuditLog/index.jsx`:
- Around line 101-108: Update the “查询” Button’s onClick handler to call load(1)
explicitly when resetting to the first page, while preserving the existing page
reset behavior and avoiding an additional unparameterized load call that could
cause duplicate API requests.
- Around line 35-65: Decouple filter edits from automatic loading in the
AuditLog component: remove the load useCallback dependency loop and make the
useEffect trigger only when page or pageSize changes. Preserve the existing load
request and filter values for explicit 查询 actions, while ensuring typing
actorName, action, keyword, from, or to does not immediately call /api/audit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2df3bd74-aa34-48ff-bc39-19835e4e4092
📒 Files selected for processing (3)
web/classic/src/App.jsxweb/classic/src/components/layout/SiderBar.jsxweb/classic/src/pages/AuditLog/index.jsx
| { | ||
| text: t('审计日志'), | ||
| itemKey: 'audit', | ||
| to: '/audit', | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Hide the Audit Log menu item from unauthorized users.
The 审计日志 (Audit Log) menu item is currently visible to all users because it lacks a role-based visibility check. Since its corresponding route is protected by AdminRoute and the /api/audit endpoint is restricted to super-admins, normal users will see this menu item but receive a "Forbidden" error when clicking it.
Please consider adding a role check to hide it, using isRoot() (or isAdmin() if appropriate). You may also want to move this entry from workspaceItems (控制台) into adminItems (管理员) to align with existing super-admin configurations.
🐛 Proposed fix (Visibility check)
{
text: t('审计日志'),
itemKey: 'audit',
to: '/audit',
+ className: isRoot() ? '' : 'tableHiddle',
},📝 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.
| { | |
| text: t('审计日志'), | |
| itemKey: 'audit', | |
| to: '/audit', | |
| }, | |
| { | |
| text: t('审计日志'), | |
| itemKey: 'audit', | |
| to: '/audit', | |
| className: isRoot() ? '' : 'tableHiddle', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/classic/src/components/layout/SiderBar.jsx` around lines 93 - 97,
Restrict the `审计日志` menu entry in the sidebar to authorized users by adding the
established `isRoot()` or appropriate `isAdmin()` visibility check, matching the
protected route and API permissions. Consider moving the entry from
`workspaceItems` to `adminItems` so it appears only in the administrator
section.
| const load = useCallback(async () => { | ||
| try { | ||
| const params = new URLSearchParams(); | ||
| // 关键：p 传「页码」(1-based)，由后端 common.GetPageQuery 内部换算 offset。 | ||
| // 与既有页面(/api/log、/api/user)保持一致，切勿传 (page-1)*pageSize 的 offset， | ||
| // 否则第 2 页起会二次偏移导致取错数据。 | ||
| params.set('p', String(page)); | ||
| params.set('page_size', String(pageSize)); | ||
| if (actorName) params.set('actor_name', actorName); | ||
| if (action) params.set('action', action); | ||
| if (keyword) params.set('keyword', keyword); | ||
| if (from) params.set('from', from); | ||
| if (to) params.set('to', to); | ||
| const res = await API.get(`/api/audit?${params.toString()}`); | ||
| // 后端 common.ApiSuccess 包裹为 { success, message, data:{ page, page_size, total, items } } | ||
| const { success, message, data } = res.data; | ||
| if (!success) { | ||
| showError(message || t('加载审计日志失败')); | ||
| return; | ||
| } | ||
| setItems(data.items || []); | ||
| setTotal(data.total || 0); | ||
| } catch (e) { | ||
| showError(e.message || t('加载审计日志失败')); | ||
| } | ||
| }, [page, pageSize, actorName, action, keyword, from, to, t]); | ||
|
|
||
| useEffect(() => { | ||
| load(); | ||
| }, [load]); | ||
|
|
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Prevent immediate API requests on every filter keystroke.
Because load is wrapped in a useCallback that depends on all filter states (actorName, action, etc.), typing a single character creates a new load function instance. The useEffect below depends on load, meaning an API call to /api/audit will be fired instantly for every keystroke. This causes heavy API spam and bypasses the intent of having a separate "查询" (Search) button.
To fix this, decouple the filter updates from the effect loop: remove useCallback and trigger the useEffect only on pagination changes (page, pageSize).
⚡ Proposed fix for the load and effect hooks
- const load = useCallback(async () => {
+ const load = async (currentPage = page) => {
try {
const params = new URLSearchParams();
// 关键：p 传「页码」(1-based)，由后端 common.GetPageQuery 内部换算 offset。
// 与既有页面(/api/log、/api/user)保持一致，切勿传 (page-1)*pageSize 的 offset，
// 否则第 2 页起会二次偏移导致取错数据。
- params.set('p', String(page));
+ params.set('p', String(currentPage));
params.set('page_size', String(pageSize));
if (actorName) params.set('actor_name', actorName);
if (action) params.set('action', action);
if (keyword) params.set('keyword', keyword);
if (from) params.set('from', from);
if (to) params.set('to', to);
const res = await API.get(`/api/audit?${params.toString()}`);
// 后端 common.ApiSuccess 包裹为 { success, message, data:{ page, page_size, total, items } }
const { success, message, data } = res.data;
if (!success) {
showError(message || t('加载审计日志失败'));
return;
}
setItems(data.items || []);
setTotal(data.total || 0);
} catch (e) {
showError(e.message || t('加载审计日志失败'));
}
- }, [page, pageSize, actorName, action, keyword, from, to, t]);
+ };
useEffect(() => {
load();
- }, [load]);
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [page, 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.
| const load = useCallback(async () => { | |
| try { | |
| const params = new URLSearchParams(); | |
| // 关键：p 传「页码」(1-based)，由后端 common.GetPageQuery 内部换算 offset。 | |
| // 与既有页面(/api/log、/api/user)保持一致，切勿传 (page-1)*pageSize 的 offset， | |
| // 否则第 2 页起会二次偏移导致取错数据。 | |
| params.set('p', String(page)); | |
| params.set('page_size', String(pageSize)); | |
| if (actorName) params.set('actor_name', actorName); | |
| if (action) params.set('action', action); | |
| if (keyword) params.set('keyword', keyword); | |
| if (from) params.set('from', from); | |
| if (to) params.set('to', to); | |
| const res = await API.get(`/api/audit?${params.toString()}`); | |
| // 后端 common.ApiSuccess 包裹为 { success, message, data:{ page, page_size, total, items } } | |
| const { success, message, data } = res.data; | |
| if (!success) { | |
| showError(message || t('加载审计日志失败')); | |
| return; | |
| } | |
| setItems(data.items || []); | |
| setTotal(data.total || 0); | |
| } catch (e) { | |
| showError(e.message || t('加载审计日志失败')); | |
| } | |
| }, [page, pageSize, actorName, action, keyword, from, to, t]); | |
| useEffect(() => { | |
| load(); | |
| }, [load]); | |
| const load = async (currentPage = page) => { | |
| try { | |
| const params = new URLSearchParams(); | |
| // 关键：p 传「页码」(1-based)，由后端 common.GetPageQuery 内部换算 offset。 | |
| // 与既有页面(/api/log、/api/user)保持一致，切勿传 (page-1)*pageSize 的 offset， | |
| // 否则第 2 页起会二次偏移导致取错数据。 | |
| params.set('p', String(currentPage)); | |
| params.set('page_size', String(pageSize)); | |
| if (actorName) params.set('actor_name', actorName); | |
| if (action) params.set('action', action); | |
| if (keyword) params.set('keyword', keyword); | |
| if (from) params.set('from', from); | |
| if (to) params.set('to', to); | |
| const res = await API.get(`/api/audit?${params.toString()}`); | |
| // 后端 common.ApiSuccess 包裹为 { success, message, data:{ page, page_size, total, items } } | |
| const { success, message, data } = res.data; | |
| if (!success) { | |
| showError(message || t('加载审计日志失败')); | |
| return; | |
| } | |
| setItems(data.items || []); | |
| setTotal(data.total || 0); | |
| } catch (e) { | |
| showError(e.message || t('加载审计日志失败')); | |
| } | |
| }; | |
| useEffect(() => { | |
| load(); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [page, pageSize]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/classic/src/pages/AuditLog/index.jsx` around lines 35 - 65, Decouple
filter edits from automatic loading in the AuditLog component: remove the load
useCallback dependency loop and make the useEffect trigger only when page or
pageSize changes. Preserve the existing load request and filter values for
explicit 查询 actions, while ensuring typing actorName, action, keyword, from, or
to does not immediately call /api/audit.
| <Button | ||
| onClick={() => { | ||
| setPage(1); | ||
| load(); | ||
| }} | ||
| > | ||
| {t('查询')} | ||
| </Button> |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Update the Search button to explicitly call load.
To complement the fix above, update the "查询" (Search) button so it properly invokes load(1) when the user is already on the first page, avoiding a double API fetch when setPage(1) does not trigger a re-render.
⚡ Proposed fix for the Search button
<Button
onClick={() => {
- setPage(1);
- load();
+ if (page === 1) {
+ load(1);
+ } else {
+ setPage(1);
+ }
}}
>
{t('查询')}
</Button>📝 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.
| <Button | |
| onClick={() => { | |
| setPage(1); | |
| load(); | |
| }} | |
| > | |
| {t('查询')} | |
| </Button> | |
| <Button | |
| onClick={() => { | |
| if (page === 1) { | |
| load(1); | |
| } else { | |
| setPage(1); | |
| } | |
| }} | |
| > | |
| {t('查询')} | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/classic/src/pages/AuditLog/index.jsx` around lines 101 - 108, Update the
“查询” Button’s onClick handler to call load(1) explicitly when resetting to the
first page, while preserving the existing page reset behavior and avoiding an
additional unparameterized load call that could cause duplicate API requests.
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit