feat(channel): 多密钥渠道: 密钥管理追加单个密码测试按钮.定期渠道测试时候,测试所有key,如果成功将自动禁用的key恢复 - #6015
Open
fengye110 wants to merge 3 commits into
Open
feat(channel): 多密钥渠道: 密钥管理追加单个密码测试按钮.定期渠道测试时候,测试所有key,如果成功将自动禁用的key恢复#6015fengye110 wants to merge 3 commits into
fengye110 wants to merge 3 commits into
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds multi-key channel testing across backend execution, routing, audit logging, frontend per-key testing, and localized UI strings. It also threads an optional key index through the existing channel test path and updates automatic multi-key test handling. ChangesBackend test-all-keys implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Frontend test-all-keys UI and API
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/default/src/features/channels/components/dialogs/multi-key-manage-dialog.tsx (1)
169-224: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
performActionhas no handler fortest-all-keys.The
MultiKeyConfirmActiontype includes'test-all-keys'andgetMultiKeyConfirmMessagereturns a confirm prompt for it, butperformActionhas notest-all-keysbranch. If this action is ever triggered,responsestaysundefinedand the user sees "Operation failed". If a "Test All Keys" button belongs in this PR, add a case that callstestAllMultiKeysand processes theTestAllKeysResponse:Proposed handler addition
} else if (type === 'delete-disabled') { response = await deleteDisabledMultiKeys(currentRow.id) + } else if (type === 'test-all-keys') { + const res = await testAllMultiKeys(currentRow.id) + response = { success: res.success, message: res.message } }If the button is planned for a follow-up, disregard.
🤖 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/components/dialogs/multi-key-manage-dialog.tsx` around lines 169 - 224, `performAction` is missing a branch for the `test-all-keys` action, so that confirm flow falls through to a failed response. Update `performAction` in `multi-key-manage-dialog` to handle `confirmAction.type === 'test-all-keys'` by calling the matching `testAllMultiKeys` helper, then treat its `TestAllKeysResponse` the same way as the other actions for success/error toast handling and refresh behavior. Keep the change aligned with the existing `enable-all`, `disable-all`, and `delete-disabled` branches, and make sure the new action is wired through the same `MultiKeyConfirmAction` flow used by `getMultiKeyConfirmMessage`.
🧹 Nitpick comments (2)
controller/channel-test.go (2)
386-442: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse stable action constants instead of display labels for control flow.
Comparing
result.Actionagainst"Auto Enabled","Auto Disabled", and"Kept Manual Disabled"ties control flow to display labels returned bychannelMultiKeyActionLabel. If those labels change, the comparisons break silently with no compiler error. Introduce stable action constants (e.g.,actionAutoEnabled,actionAutoDisabled,actionKeptManualDisabled) for comparisons, and map them to labels only for the API response.♻️ Proposed refactor
+const ( + actionKept = "kept" + actionAutoEnabled = "auto_enabled" + actionAutoDisabled = "auto_disabled" + actionKeptManualDisabled = "kept_manual_disabled" + actionSkipped = "skipped" +) + // ... in workers, set result.Action = actionAutoEnabled etc. // ... in response, convert to labels via channelMultiKeyActionLabel // Then in the aggregation loop: - if result.Action == "Auto Enabled" { + if result.Action == actionAutoEnabled { // ... - } else if result.Action == "Auto Disabled" { + } else if result.Action == actionAutoDisabled { // ... - } else if result.Action == "Kept Manual Disabled" { + } else if result.Action == actionKeptManualDisabled {🤖 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 386 - 442, The control flow in channel test result handling is comparing result.Action against display strings, which should be replaced with stable action constants. Update the logic in the result-processing loop to use dedicated constants for the Auto Enabled, Auto Disabled, and Kept Manual Disabled cases, and keep channelMultiKeyActionLabel only for formatting API-facing labels. This refactor should be applied where result.Action is checked and where summary/status updates occur, so behavior is not tied to user-visible wording.
386-468: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftEliminate duplicated status-mutation logic between
performTestAllKeysandapplySingleKeyTestOutcome.Lines 386-464 re-implement the same per-key status map updates and channel-level enable/disable logic already in
applySingleKeyTestOutcome(lines 197-283). These two implementations can diverge over time. Consider extracting the shared logic into a helper, or havingperformTestAllKeyscallapplySingleKeyTestOutcomefor each key after the concurrent test phase.🤖 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 386 - 468, The per-key status update and channel status transition logic in performTestAllKeys is duplicated from applySingleKeyTestOutcome and should be unified. Extract the shared mutation flow into a helper or reuse applySingleKeyTestOutcome from performTestAllKeys after the test results are collected, so status map updates, disabled reasons/timestamps, and channel-level enable/disable changes stay consistent. Make sure the shared logic still handles MultiKeyStatusList, MultiKeyDisabledReason, MultiKeyDisabledTime, and the status_reason/status_time updates in one place.
🤖 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 162-167: The maskChannelKey helper still exposes the full value
for short keys because the prefix and suffix slices overlap for lengths between
9 and 11. Update maskChannelKey to use a safer masking rule with a higher
minimum length threshold or a shorter suffix so no input length can reconstruct
the original key; keep the fallback "***" behavior for short values and adjust
the slicing logic accordingly.
- Around line 1402-1408: The JSON responses in the channel validation branch
still return hardcoded Chinese messages, which breaks API language consistency.
Update the nil-channel and non-multi-key error paths in the channel test handler
to use English response text, matching the audit template and the rest of the
API; use the existing handler logic around the channel check and IsMultiKey
guard to locate the affected responses.
- Around line 1489-1551: The multi-key path in the channel test flow ignores the
same auto-disable gates used by the single-key path. Update the
`performTestAllKeys` call inside the multi-key branch so `autoDisableFailed` is
only enabled when both `allowDisable` and `channel.GetAutoBan()` permit it,
matching the logic used elsewhere in this function; if the per-key behavior is
intentional, document that clearly near `performTestAllKeys` to explain why
those checks are bypassed.
---
Outside diff comments:
In
`@web/default/src/features/channels/components/dialogs/multi-key-manage-dialog.tsx`:
- Around line 169-224: `performAction` is missing a branch for the
`test-all-keys` action, so that confirm flow falls through to a failed response.
Update `performAction` in `multi-key-manage-dialog` to handle
`confirmAction.type === 'test-all-keys'` by calling the matching
`testAllMultiKeys` helper, then treat its `TestAllKeysResponse` the same way as
the other actions for success/error toast handling and refresh behavior. Keep
the change aligned with the existing `enable-all`, `disable-all`, and
`delete-disabled` branches, and make sure the new action is wired through the
same `MultiKeyConfirmAction` flow used by `getMultiKeyConfirmMessage`.
---
Nitpick comments:
In `@controller/channel-test.go`:
- Around line 386-442: The control flow in channel test result handling is
comparing result.Action against display strings, which should be replaced with
stable action constants. Update the logic in the result-processing loop to use
dedicated constants for the Auto Enabled, Auto Disabled, and Kept Manual
Disabled cases, and keep channelMultiKeyActionLabel only for formatting
API-facing labels. This refactor should be applied where result.Action is
checked and where summary/status updates occur, so behavior is not tied to
user-visible wording.
- Around line 386-468: The per-key status update and channel status transition
logic in performTestAllKeys is duplicated from applySingleKeyTestOutcome and
should be unified. Extract the shared mutation flow into a helper or reuse
applySingleKeyTestOutcome from performTestAllKeys after the test results are
collected, so status map updates, disabled reasons/timestamps, and channel-level
enable/disable changes stay consistent. Make sure the shared logic still handles
MultiKeyStatusList, MultiKeyDisabledReason, MultiKeyDisabledTime, and the
status_reason/status_time updates in one place.
🪄 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: 6bbb9667-6e50-443f-8830-e3a2cabebcec
📒 Files selected for processing (19)
controller/audit.gocontroller/channel-test.gocontroller/channel_test_internal_test.gorouter/channel-router.gorouter/channel_router_test.goweb/default/src/features/channels/api.tsweb/default/src/features/channels/components/dialogs/multi-key-manage-dialog.tsxweb/default/src/features/channels/components/dialogs/multi-key-table-row-actions.tsxweb/default/src/features/channels/constants.tsweb/default/src/features/channels/lib/multi-key-utils.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-TW.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/i18n/static-keys.ts
7 tasks
5 tasks
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.
Important
📝 变更描述 / Description
多密钥渠道时候追加两个功能:
自动禁用的key恢复🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
key_index.