feat(channel): add skip_auto_test to bypass auto test for quota-limited channels - #5206
feat(channel): add skip_auto_test to bypass auto test for quota-limited channels#5206YNight-FZQ wants to merge 3 commits into
Conversation
|
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 (13)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughAdds a configurable ChangesSkip Auto Test Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant runChannelTestTask
participant performChannelTests
participant Channel
Scheduler->>runChannelTestTask: invoke channel test task
runChannelTestTask->>runChannelTestTask: determine scheduled mode
runChannelTestTask->>performChannelTests: pass scheduled as skipAutoTest
performChannelTests->>Channel: read GetSkipAutoTest()
alt scheduled and skip enabled
performChannelTests->>performChannelTests: skip channel
else otherwise
performChannelTests->>Channel: run channel test
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/table/channels/modals/EditChannelModal.jsx`:
- Line 224: The skipAutoTest boolean in EditChannelModal.jsx is stored in
component state and isn't reset between modal sessions, causing stale values to
be submitted; update the component to reset skipAutoTest whenever the modal/form
is initialized or closed (e.g., in the modal open handler or a useEffect that
watches the modal open flag or the channel prop) or move skipAutoTest into the
form's values so it's reset with the form; ensure you call
setSkipAutoTest(false) (or set to the channel's initial value) during modal
init/reset and before handleSubmit/submit actions so each session uses the
correct value.
🪄 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: bf91b35b-16b3-4a94-b27a-6b4ed35838e3
📒 Files selected for processing (7)
controller/channel-test.gomodel/channel.goweb/classic/src/components/table/channels/modals/EditChannelModal.jsxweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/features/channels/constants.tsweb/default/src/features/channels/lib/channel-form.tsweb/default/src/features/channels/types.ts
39e809c to
b31758b
Compare
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 `@controller/channel-test.go`:
- Around line 929-931: The skip_auto_test flag is being applied to both
scheduled and manual runs; update the logic in the testAllChannels flow so
channel.GetSkipAutoTest() is honored only for scheduled runs (e.g., when the
caller indicates this is an automated/scheduled invocation). Constrain the early
continue to something like: if isScheduledRun && channel.GetSkipAutoTest() {
continue }, where isScheduledRun is the scheduled/manual indicator passed into
the testAllChannels handler or derived from the caller context; update any
callers or signature of testAllChannels if needed to pass that boolean so manual
invocations ignore GetSkipAutoTest().
In `@web/default/src/i18n/locales/ja.json`:
- Around line 3676-3677: Replace the two flat English source-text keys "Skip
Auto Test" and "Skip auto test to avoid consuming quota on limited channels"
with hierarchical keys such as channels.form.skipAutoTest.label and
channels.form.skipAutoTest.description in the ja.json locale; update any code
references that currently use the source-text keys (search for "Skip Auto Test"
and the full description) to the new keys so instrumentation and lookup use
channels.form.skipAutoTest.label and channels.form.skipAutoTest.description
consistently across the codebase.
In `@web/default/src/i18n/locales/vi.json`:
- Around line 3676-3677: Replace the flat sentence-style translation keys "Skip
Auto Test" and "Skip auto test to avoid consuming quota on limited channels"
with hierarchical keys (e.g. channel.skipAutoTest.label and
channel.skipAutoTest.description) in the vi.json locale and update every call
site that references the old literal keys to use the new keys (search for the
exact strings "Skip Auto Test" and "Skip auto test to avoid consuming quota on
limited channels"); place the new keys in the appropriate semantic section of
the vi.json file (under a "channel" or similar namespace) and ensure any code
that reads these keys (components, hooks, or i18n lookup functions) uses the new
dot-separated keys so translations load correctly.
🪄 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: 7a7ca7f1-4829-4fcd-b57a-f955ddbf53bc
📒 Files selected for processing (23)
controller/channel-test.gomodel/channel.gomodel/channel_skip_auto_test_test.gotest.txtweb/classic/src/components/table/channels/modals/EditChannelModal.jsxweb/classic/src/i18n/locales/en.jsonweb/classic/src/i18n/locales/fr.jsonweb/classic/src/i18n/locales/ja.jsonweb/classic/src/i18n/locales/ru.jsonweb/classic/src/i18n/locales/vi.jsonweb/classic/src/i18n/locales/zh-CN.jsonweb/classic/src/i18n/locales/zh-TW.jsonweb/classic/src/i18n/locales/zh.jsonweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/features/channels/constants.tsweb/default/src/features/channels/lib/channel-form.tsweb/default/src/features/channels/types.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.json
✅ Files skipped from review due to trivial changes (9)
- test.txt
- web/default/src/i18n/locales/en.json
- web/classic/src/i18n/locales/vi.json
- web/classic/src/i18n/locales/fr.json
- web/classic/src/i18n/locales/en.json
- web/classic/src/i18n/locales/zh-TW.json
- model/channel.go
- web/default/src/i18n/locales/zh.json
- web/default/src/i18n/locales/ru.json
🚧 Files skipped from review as they are similar to previous changes (6)
- model/channel_skip_auto_test_test.go
- web/default/src/features/channels/constants.ts
- web/default/src/features/channels/components/drawers/channel-mutate-drawer.tsx
- web/default/src/features/channels/types.ts
- web/default/src/features/channels/lib/channel-form.ts
- web/classic/src/components/table/channels/modals/EditChannelModal.jsx
b31758b to
0be51a1
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controller/channel-test.go (1)
909-912:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset
testAllChannelsRunningon pre-goroutine failure path.If
model.GetAllChannels(...)fails, the function returns whiletestAllChannelsRunningis stilltrue, which can block all future channel-test runs until restart.Proposed fix
channels, getChannelErr := model.GetAllChannels(0, 0, true, false) if getChannelErr != nil { + testAllChannelsLock.Lock() + testAllChannelsRunning = false + testAllChannelsLock.Unlock() return getChannelErr }🤖 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/channel-test.go` around lines 909 - 912, When model.GetAllChannels(0, 0, true, false) returns an error, testAllChannelsRunning remains true and can block future runs; modify the error path in the same function to set testAllChannelsRunning = false (and any associated cleanup or unlocking state) before returning the getChannelErr so the global/test flag is reset on failure. Locate the error branch that checks getChannelErr and add the assignment to testAllChannelsRunning (and return the original error) so subsequent channel-test invocations are not blocked.
🤖 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.
Outside diff comments:
In `@controller/channel-test.go`:
- Around line 909-912: When model.GetAllChannels(0, 0, true, false) returns an
error, testAllChannelsRunning remains true and can block future runs; modify the
error path in the same function to set testAllChannelsRunning = false (and any
associated cleanup or unlocking state) before returning the getChannelErr so the
global/test flag is reset on failure. Locate the error branch that checks
getChannelErr and add the assignment to testAllChannelsRunning (and return the
original error) so subsequent channel-test invocations are not blocked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f691f29-bebc-4268-a5bf-710b500c1d38
📒 Files selected for processing (1)
controller/channel-test.go
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
Hi @Calcium-Ion @t0ng7u, this PR is ready for review. It adds a |
|
Oh nice, I was just wondering if I can lower the check spam on my free tier endpoints. These checks burn quota for nothing when the channel's already rate-limited. |
|
@Calcium-Ion @t0ng7u friendly ping — all CodeRabbit feedback has been addressed, this PR is ready for review. Thanks! 🙏 |
b87c0d7 to
62596d4
Compare
|
已 rebase 到最新 main,并解决了与新引入的「渠道测试模式」重构的冲突(拆分出 testChannels + testAllChannels/testAutoDisabledChannels 包装函数 + selectChannelsForAutomaticTest 预筛选 + ChannelTestMode 模式)。 在适配新结构的同时保留了原 PR 语义——仅定时测试跳过开启 skip_auto_test 的渠道:
已通过编译和相关单元测试(controller / model 包)。无冲突残留(mergeable: MERGEABLE),等待 review,谢谢!🙏 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/channel-test.go`:
- Around line 1038-1040: The code is currently ignoring errors from
resolveChannelTestUserID and model.GetAllChannels by assigning them to
underscore blanks. Instead, capture these errors and check them immediately
after each function call. If either resolveChannelTestUserID or
model.GetAllChannels returns an error, log the error with appropriate context
information and use continue to skip to the next iteration of the scheduler
loop, allowing the scheduled run to fail fast and provide visibility into what
went wrong rather than silently proceeding with nil or empty data to the
subsequent testChannels call.
🪄 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: 48865b9f-f9a5-4e19-987c-1824a790febb
📒 Files selected for processing (16)
controller/channel-test.gomodel/channel.gomodel/channel_skip_auto_test_test.goweb/classic/src/components/table/channels/modals/EditChannelModal.jsxweb/classic/src/i18n/locales/en.jsonweb/classic/src/i18n/locales/fr.jsonweb/classic/src/i18n/locales/ja.jsonweb/classic/src/i18n/locales/ru.jsonweb/classic/src/i18n/locales/vi.jsonweb/classic/src/i18n/locales/zh-CN.jsonweb/classic/src/i18n/locales/zh-TW.jsonweb/classic/src/i18n/locales/zh.jsonweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/features/channels/constants.tsweb/default/src/features/channels/lib/channel-form.tsweb/default/src/features/channels/types.ts
💤 Files with no reviewable changes (4)
- web/default/src/features/channels/components/drawers/channel-mutate-drawer.tsx
- web/default/src/features/channels/types.ts
- web/default/src/features/channels/lib/channel-form.ts
- web/default/src/features/channels/constants.ts
✅ Files skipped from review due to trivial changes (7)
- web/classic/src/i18n/locales/zh.json
- web/classic/src/i18n/locales/fr.json
- web/classic/src/i18n/locales/en.json
- web/classic/src/i18n/locales/ja.json
- web/classic/src/i18n/locales/ru.json
- web/classic/src/i18n/locales/zh-CN.json
- web/classic/src/i18n/locales/vi.json
🚧 Files skipped from review as they are similar to previous changes (4)
- web/classic/src/i18n/locales/zh-TW.json
- model/channel_skip_auto_test_test.go
- model/channel.go
- web/classic/src/components/table/channels/modals/EditChannelModal.jsx
| testUserID, _ := resolveChannelTestUserID(nil) | ||
| channels, _ := model.GetAllChannels(0, 0, true, false) | ||
| _ = testChannels(selectChannelsForAutomaticTest(channels, operation_setting.ChannelTestModeScheduledAll), testUserID, false, true, true) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle scheduler fetch errors instead of silently discarding them
Line 1038 and Line 1039 ignore errors from resolveChannelTestUserID and model.GetAllChannels. In scheduled runs this can fail silently and skip/empty channel testing without any signal. Please fail fast for that iteration and log the reason.
Suggested fix
- testUserID, _ := resolveChannelTestUserID(nil)
- channels, _ := model.GetAllChannels(0, 0, true, false)
- _ = testChannels(selectChannelsForAutomaticTest(channels, operation_setting.ChannelTestModeScheduledAll), testUserID, false, true, true)
+ testUserID, err := resolveChannelTestUserID(nil)
+ if err != nil {
+ common.SysError(fmt.Sprintf("automatically test channels: resolve test user failed: %v", err))
+ continue
+ }
+ channels, err := model.GetAllChannels(0, 0, true, false)
+ if err != nil {
+ common.SysError(fmt.Sprintf("automatically test channels: load channels failed: %v", err))
+ continue
+ }
+ _ = testChannels(
+ selectChannelsForAutomaticTest(channels, operation_setting.ChannelTestModeScheduledAll),
+ testUserID,
+ false,
+ true,
+ true,
+ )🤖 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/channel-test.go` around lines 1038 - 1040, The code is currently
ignoring errors from resolveChannelTestUserID and model.GetAllChannels by
assigning them to underscore blanks. Instead, capture these errors and check
them immediately after each function call. If either resolveChannelTestUserID or
model.GetAllChannels returns an error, log the error with appropriate context
information and use continue to skip to the next iteration of the scheduler
loop, allowing the scheduled run to fail fast and provide visibility into what
went wrong rather than silently proceeding with nil or empty data to the
subsequent testChannels call.
62596d4 to
5cd234f
Compare
Summary
SkipAutoTest字段,允许渠道跳过定时自动测试以节省额度model/channel.go,controller/channel-test.gotypes.ts,constants.ts,channel-form.ts,channel-mutate-drawer.tsxEditChannelModal.jsxmodel/channel_skip_auto_test_test.goDescription
当渠道开启
Skip Auto Test选项后,在定时自动测试(CHANNEL_TEST_FREQUENCY)触发时该渠道将被跳过,不会消耗额度。同时不影响手动点击「测试」按钮和正常业务路由调用。实现参照
auto_ban字段的模式:number(0/1)0(不跳过)boolauto_ban并排放置Test Plan
go test ./model/确认单元测试通过go build ./...确认编译通过Checklist
GetSkipAutoTest()单元测试auto_ban实现模式)Summary by CodeRabbit
skip_auto_test) to channel creation/editing (including advanced routing/overrides).