Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/relay_task.go`:
- Around line 370-372: The admin fallback currently runs when !exist regardless
of whether GetByTaskId returned an error; change the logic in the block that
calls model.GetByOnlyTaskId so it only executes when err == nil && !exist &&
model.IsAdmin(userId). In other words, after calling model.GetByTaskId(taskId)
check err first and return/handle the error if non-nil; only if err is nil and
exist is false and model.IsAdmin(userId) call model.GetByOnlyTaskId(taskId) to
populate originTask/exist/err so you don't overwrite or mask the original
database error.
🪄 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: 515f4995-15c7-47f4-b950-928ca9afcd1b
📒 Files selected for processing (1)
relay/relay_task.go
| if !exist && model.IsAdmin(userId) { | ||
| originTask, exist, err = model.GetByOnlyTaskId(taskId) | ||
| } |
There was a problem hiding this comment.
Check query errors before applying admin fallback.
At Line [370], fallback runs on !exist even when GetByTaskId returned err. That can mask the original DB failure for admins by overwriting err with the second query result.
Suggested fix
originTask, exist, err := model.GetByTaskId(userId, taskId)
-if !exist && model.IsAdmin(userId) {
+if err != nil {
+ taskResp = service.TaskErrorWrapper(err, "get_task_failed", http.StatusInternalServerError)
+ return
+}
+if !exist && model.IsAdmin(userId) {
originTask, exist, err = model.GetByOnlyTaskId(taskId)
-}
-if err != nil {
- taskResp = service.TaskErrorWrapper(err, "get_task_failed", http.StatusInternalServerError)
- return
+ if err != nil {
+ taskResp = service.TaskErrorWrapper(err, "get_task_failed", http.StatusInternalServerError)
+ return
+ }
}📝 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 !exist && model.IsAdmin(userId) { | |
| originTask, exist, err = model.GetByOnlyTaskId(taskId) | |
| } | |
| originTask, exist, err := model.GetByTaskId(userId, taskId) | |
| if err != nil { | |
| taskResp = service.TaskErrorWrapper(err, "get_task_failed", http.StatusInternalServerError) | |
| return | |
| } | |
| if !exist && model.IsAdmin(userId) { | |
| originTask, exist, err = model.GetByOnlyTaskId(taskId) | |
| if err != nil { | |
| taskResp = service.TaskErrorWrapper(err, "get_task_failed", http.StatusInternalServerError) | |
| return | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/relay_task.go` around lines 370 - 372, The admin fallback currently
runs when !exist regardless of whether GetByTaskId returned an error; change the
logic in the block that calls model.GetByOnlyTaskId so it only executes when err
== nil && !exist && model.IsAdmin(userId). In other words, after calling
model.GetByTaskId(taskId) check err first and return/handle the error if
non-nil; only if err is nil and exist is false and model.IsAdmin(userId) call
model.GetByOnlyTaskId(taskId) to populate originTask/exist/err so you don't
overwrite or mask the original database error.
1 participant
如果用户有管理员权限, 可以查询所有任务视频,方面排查问题
📸 运行证明 / Proof of Work
Summary by CodeRabbit