optimize: channel affinity tips - #2898
Conversation
WalkthroughThe change refactors channel column rendering in usage logs to display channel affinity via a Sparkles badge indicator, moves affinity handling from the retry column, and improves use_channel array validation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
152cf50 to
8f831fc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx`:
- Around line 620-628: The retry-column logic uses other.admin_info.use_channel
without ensuring it's an array, so calling .join('->') on non-arrays will throw;
update the block that constructs useChannelStr (in UsageLogsColumnDefs.jsx where
other.admin_info/use_channel is referenced) to first check
Array.isArray(other.admin_info.use_channel) &&
other.admin_info.use_channel.length > 0 before calling .join, and fall back to
treating a string value as-is (or skip/empty) when use_channel is a non-array
truthy value; ensure the variable names referenced (useChannel, useChannelStr,
content) are adapted accordingly.
🧹 Nitpick comments (1)
web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx (1)
614-618: UnsafeJSON.parsewithout try-catch — consider usinggetLogOtherfor consistency.The channel column uses
getLogOther(record.other)(which handles parse errors gracefully), but this retry column callsJSON.parse(record.other)directly. Malformed JSON will crash the render.Proposed fix
- let content = t('渠道') + `:${record.channel}`; - if (record.other !== '') { - let other = JSON.parse(record.other); - if (other === null) { - return <></>; - } + let content = t('渠道') + `:${record.channel}`; + let other = getLogOther(record.other); + if (other) {
| if (other.admin_info !== undefined) { | ||
| if ( | ||
| other.admin_info.use_channel !== null && | ||
| other.admin_info.use_channel !== undefined && | ||
| other.admin_info.use_channel !== '' | ||
| other.admin_info.use_channel !== null && | ||
| other.admin_info.use_channel !== undefined && | ||
| other.admin_info.use_channel !== '' | ||
| ) { | ||
| let useChannel = other.admin_info.use_channel; | ||
| let useChannelStr = useChannel.join('->'); | ||
| content = t('渠道') + `:${useChannelStr}`; |
There was a problem hiding this comment.
Inconsistent use_channel validation — .join() will throw on non-array values.
The channel column (line 321) now properly guards with Array.isArray(adminInfo.use_channel) && adminInfo.use_channel.length > 0, but this retry column still uses loose truthy checks. If use_channel is a non-array truthy value (e.g., a string), calling .join('->') on line 627 will throw a TypeError at runtime.
Proposed fix: align with channel column's validation
if (
- other.admin_info.use_channel !== null &&
- other.admin_info.use_channel !== undefined &&
- other.admin_info.use_channel !== ''
+ Array.isArray(other.admin_info.use_channel) &&
+ other.admin_info.use_channel.length > 0
) {📝 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 (other.admin_info !== undefined) { | |
| if ( | |
| other.admin_info.use_channel !== null && | |
| other.admin_info.use_channel !== undefined && | |
| other.admin_info.use_channel !== '' | |
| other.admin_info.use_channel !== null && | |
| other.admin_info.use_channel !== undefined && | |
| other.admin_info.use_channel !== '' | |
| ) { | |
| let useChannel = other.admin_info.use_channel; | |
| let useChannelStr = useChannel.join('->'); | |
| content = t('渠道') + `:${useChannelStr}`; | |
| if (other.admin_info !== undefined) { | |
| if ( | |
| Array.isArray(other.admin_info.use_channel) && | |
| other.admin_info.use_channel.length > 0 | |
| ) { | |
| let useChannel = other.admin_info.use_channel; | |
| let useChannelStr = useChannel.join('->'); | |
| content = t('渠道') + `:${useChannelStr}`; |
🤖 Prompt for AI Agents
In `@web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx` around lines 620
- 628, The retry-column logic uses other.admin_info.use_channel without ensuring
it's an array, so calling .join('->') on non-arrays will throw; update the block
that constructs useChannelStr (in UsageLogsColumnDefs.jsx where
other.admin_info/use_channel is referenced) to first check
Array.isArray(other.admin_info.use_channel) &&
other.admin_info.use_channel.length > 0 before calling .join, and fall back to
treating a string value as-is (or skip/empty) when use_channel is a non-array
truthy value; ensure the variable names referenced (useChannel, useChannelStr,
content) are adapted accordingly.
…inity-tips optimize: channel affinity tips
Summary by CodeRabbit
New Features
Improvements