fix: refresh channel test dialog status - #5504
Open
lichengzhe wants to merge 2 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds ChangesChannel Test Response Time and Cache Invalidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/default/src/features/channels/lib/channel-actions.ts (1)
240-245: ⚡ Quick winAvoid nested ternary in response-time normalization.
Use explicit branching here; it keeps this API-shape fallback logic easier to read and maintain.
♻️ Proposed refactor
- const responseTime = - typeof response.data?.response_time === 'number' - ? response.data.response_time - : typeof response.time === 'number' - ? response.time * 1000 - : undefined + let responseTime: number | undefined + if (typeof response.data?.response_time === 'number') { + responseTime = response.data.response_time + } else if (typeof response.time === 'number') { + responseTime = response.time * 1000 + }As per coding guidelines,
web/default/**/*.{ts,tsx}prohibits nested ternary expressions with 2 or more levels and asks to useif-else, early returns, or extracted functions instead.🤖 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/default/src/features/channels/lib/channel-actions.ts` around lines 240 - 245, Replace the nested ternary operator in the responseTime variable assignment with explicit if-else branching. The current code has two levels of ternary operators checking response.data?.response_time, then response.time, and falling back to undefined. Convert this to use if-else statements or early returns to check each condition sequentially and assign the appropriate value, ensuring the logic is clearer and adheres to the coding guidelines that prohibit nested ternary expressions with 2 or more levels.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
`@web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx`:
- Around line 342-351: The success callback in the test completion handler uses
the closure-captured currentRow value, which becomes stale if the user switches
channels before the async test completes. Create a useRef to hold the current
row reference, update it whenever currentRow changes, and in the success
callback block (where success && typeof responseTime === 'number'), read the
latest row from the ref instead of using the closure variable. Additionally, add
a verification check comparing the response row's ID with the current ref row's
ID before calling setCurrentRow to ensure you only update results for the
channel the test was actually run against.
---
Nitpick comments:
In `@web/default/src/features/channels/lib/channel-actions.ts`:
- Around line 240-245: Replace the nested ternary operator in the responseTime
variable assignment with explicit if-else branching. The current code has two
levels of ternary operators checking response.data?.response_time, then
response.time, and falling back to undefined. Convert this to use if-else
statements or early returns to check each condition sequentially and assign the
appropriate value, ensuring the logic is clearer and adheres to the coding
guidelines that prohibit nested ternary expressions with 2 or more levels.
🪄 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: b2c3fc30-21c3-42f4-9c04-ec5aeb0fe4ad
📒 Files selected for processing (3)
web/default/src/features/channels/components/dialogs/channel-test-dialog.tsxweb/default/src/features/channels/lib/channel-actions.tsweb/default/src/features/channels/types.ts
Calcium-Ion
force-pushed
the
main
branch
2 times, most recently
from
August 30, 2026 15:03
51fdfc5 to
2b6f1df
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
【中文说明】Bug：新版渠道测试弹窗点击模型后仍显示“未测试”
在新版 UI 的「渠道管理 -> 测试连接」弹窗中，点击某个模型进行测试后，后端请求已经完成，但该模型所在行的状态可能仍停留在「未测试」，没有切换到「测试中 / 成功 / 失败」。
根因有两点：
/api/channel/test/:id当前返回的是顶层time字段，例如{ "success": true, "message": "", "time": 4.57 }。新版前端却按data.response_time读取耗时，导致结果数据没有被正确消费。model，没有把每个模型对应的testResult/isTesting绑定进 row data，测试状态更新后表格行展示容易继续显示旧状态。这个 PR 修复后，点击单个模型测试时，行状态会从「未测试」立即变为「测试中」，请求完成后再显示「成功」或「失败」；成功时也会同步刷新渠道响应时间和最后测试时间。
Summary
timeTest Plan
cd web/default && bun run typecheckNotes
The bug is visible in the new UI channel test dialog: clicking a model test can leave the row displayed as
Not testedeven after the backend request completes. The backend currently returns{ success, message, time }, while the new UI expecteddata.response_time.Summary by CodeRabbit