feat: task log show username - #2635
Conversation
WalkthroughA new Username field was added to the Task model and populated from user cache. Correspondingly, a USERNAME column was added to the task logs table UI with admin-only visibility controls and associated column management logic in the hooks layer. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 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
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
🤖 Fix all issues with AI agents
In @web/src/components/table/task-logs/TaskLogsColumnDefs.jsx:
- Around line 301-302: The code references a non-existent record.display_name in
TaskLogsColumnDefs.jsx; remove that line and stop using display_name as it will
always be undefined — update the label computation to use the populated
username/text fallback (e.g., const label = text || t('未知')) or, if you actually
need a display name, add display_name to the Task response in the backend
instead of referencing it here; ensure you edit the variables around displayName
and label so no unused display_name reference remains.
🧹 Nitpick comments (2)
model/task.go (1)
237-241: Consider batch-loading users to avoid potential N+1 queries.The current implementation calls
GetUserCacheonce per task, which could result in multiple cache lookups or DB queries. While the cache layer should mitigate this, consider batch-loading all required users in a single query for better performance on large result sets.♻️ Potential optimization approach
One approach is to collect all unique user IDs first, batch-fetch them, then populate the Username field:
// 获取数据 err = query.Order("id desc").Limit(num).Offset(startIdx).Find(&tasks).Error if err != nil { return nil } + // Collect unique user IDs + userIds := make(map[int]bool) + for _, task := range tasks { + userIds[task.UserId] = true + } + + // Batch load user cache (would require a new BatchGetUserCache function) + // userCacheMap, _ := BatchGetUserCache(userIds) + for _, task := range tasks { - if cache, err := GetUserCache(task.UserId); err == nil { - task.Username = cache.Username - } + // if cache, ok := userCacheMap[task.UserId]; ok { + // task.Username = cache.Username + // } + // Fallback to current implementation if batch function unavailable: + if cache, err := GetUserCache(task.UserId); err == nil { + task.Username = cache.Username + } } return tasksNote: This would require implementing a batch user cache retrieval function.
web/src/components/table/task-logs/TaskLogsColumnDefs.jsx (1)
303-308: Consider simplifying the avatar text logic.The nested ternary for extracting the avatar initial is functional but could be more readable.
♻️ Simplified approach
- const avatarText = - typeof displayName === 'string' && displayName.length > 0 - ? displayName[0] - : typeof text === 'string' && text.length > 0 - ? text[0] - : '?'; + const avatarText = (displayName || text || '?')[0];This is more concise and handles the same fallback logic.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
model/task.goweb/src/components/table/task-logs/TaskLogsColumnDefs.jsxweb/src/hooks/task-logs/useTaskLogsData.js
🧰 Additional context used
🧬 Code graph analysis (3)
model/task.go (1)
model/user_cache.go (1)
GetUserCache(74-113)
web/src/hooks/task-logs/useTaskLogsData.js (2)
web/src/hooks/common/useSidebar.js (1)
merged(61-61)web/src/hooks/mj-logs/useMjLogsData.js (2)
COLUMN_KEYS(38-51)isAdminUser(62-62)
web/src/components/table/task-logs/TaskLogsColumnDefs.jsx (2)
web/src/hooks/task-logs/useTaskLogsData.js (2)
COLUMN_KEYS(38-51)isAdminUser(61-61)web/src/helpers/render.jsx (1)
stringToColor(595-602)
🔇 Additional comments (6)
model/task.go (1)
60-60: LGTM! Username field declaration is appropriate.The field is correctly marked as non-persistent (
gorm:"-") since it's populated dynamically from user cache, and theomitemptyJSON tag ensures clean API responses.web/src/components/table/task-logs/TaskLogsColumnDefs.jsx (1)
45-46: LGTM! Imports are appropriate.The
stringToColorhelper and Semi-UI components are correctly imported for the USERNAME column implementation.web/src/hooks/task-logs/useTaskLogsData.js (4)
43-43: LGTM! USERNAME key added correctly.The column key follows the existing naming convention and matches the dataIndex used in the column definition.
106-109: LGTM! Admin-only column enforcement is secure.The logic correctly forces admin-only columns (CHANNEL and USERNAME) to
falsefor non-admin users, preventing localStorage manipulation from exposing restricted data.
127-127: LGTM! Default visibility correctly gated by user role.The USERNAME column visibility is properly initialized based on
isAdminUser, consistent with the CHANNEL column behavior.
157-165: LGTM! Select All logic properly handles admin-only columns.The implementation correctly prevents non-admin users from enabling admin-only columns (CHANNEL and USERNAME) via the "Select All" action, maintaining consistent access control.
| const displayName = record.display_name; | ||
| const label = displayName || text || t('未知'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for display_name in Go files to see if it's defined in Task model or set anywhere
echo "=== Searching for display_name in Go files ==="
rg -nP --type=go 'display_name|DisplayName' -C2
echo -e "\n=== Searching for display_name in JavaScript/JSX files ==="
rg -nP --type=js --type=jsx 'display_name|displayName' -C2
echo -e "\n=== Checking Task model structure in model/task.go ==="
find . -name "task.go" -path "*/model*" -type f | head -5Repository: QuantumNous/new-api
Length of output: 8984
🏁 Script executed:
#!/bin/bash
# Check the Task model structure
echo "=== Examining Task model structure ==="
cat -n model/task.go | head -100
echo -e "\n=== Search for Task struct definition ==="
rg -nP --type=go 'type Task struct' -A 50
echo -e "\n=== Search for task API responses or serialization ==="
rg -nP --type=go 'task.*display_name|Task.*DisplayName' -C3Repository: QuantumNous/new-api
Length of output: 7248
🏁 Script executed:
#!/bin/bash
# Search for task API endpoints in controllers
echo "=== Search for task-related API endpoints ==="
rg -nP --type=go 'func.*Task|GetTasks|ListTasks' -A 10 controller/
echo -e "\n=== Search for task response building ==="
rg -nP --type=go 'display_name.*task|task.*display_name' -B5 -A5
echo -e "\n=== Search for task-user enrichment ==="
rg -nP --type=go 'task.*user|User.*Task' controller/ | head -20Repository: QuantumNous/new-api
Length of output: 13400
🏁 Script executed:
#!/bin/bash
# Check the model functions that fetch tasks
echo "=== Search for TaskGetAllUserTask in model ==="
rg -nP --type=go 'TaskGetAllUserTask|TaskGetAllTask' -A 20
echo -e "\n=== Check if tasks are enriched with user data ==="
rg -nP --type=go 'func.*Task.*User|display_name.*Task|task.*User' model/ -B2 -A10 | head -50Repository: QuantumNous/new-api
Length of output: 5548
🏁 Script executed:
#!/bin/bash
# Check GetUserCache function to see what fields it returns
echo "=== Search for GetUserCache function ==="
rg -nP --type=go 'func.*GetUserCache' -A 15
echo -e "\n=== Check User cache struct ==="
rg -nP --type=go 'UserCache|type.*Cache' model/ -B2 -A 8 | head -60Repository: QuantumNous/new-api
Length of output: 3651
🏁 Script executed:
#!/bin/bash
# Check UserBase struct definition
echo "=== Search for UserBase struct ==="
rg -nP --type=go 'type UserBase struct' -A 20Repository: QuantumNous/new-api
Length of output: 1387
Remove the non-existent display_name field reference.
The Task model and user cache do not include a display_name field. Only Username is populated from the user cache (model/task.go lines 237-240). The line const displayName = record.display_name; will always be undefined, causing the label to fall back to text (username) or t('未知'). Either remove this reference or ensure display_name is added to the Task response if it's intended to display user display names.
🤖 Prompt for AI Agents
In @web/src/components/table/task-logs/TaskLogsColumnDefs.jsx around lines 301 -
302, The code references a non-existent record.display_name in
TaskLogsColumnDefs.jsx; remove that line and stop using display_name as it will
always be undefined — update the label computation to use the populated
username/text fallback (e.g., const label = text || t('未知')) or, if you actually
need a display name, add display_name to the Task response in the backend
instead of referencing it here; ensure you edit the variables around displayName
and label so no unused display_name reference remains.
…bfe886b93606003f6753fcb4e9d feat: task log show username
2 participants
任务日志显示用户名称
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.