feat: add stable group identifiers to prevent API keys from becoming invalid after groups are renamed or recreated - #6564
Open
zd-Ethan wants to merge 4 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds stable group identifiers and separate display names. Backend settings, validation, routing, and responses support the mapping. Frontend editors preserve identifiers, migrate references, submit grouped updates, and render localized group metadata. ChangesStable group identity and display-name management
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant GroupRatioVisualEditor
participant updateGroupOptions
participant UpdateGroupOptions
participant GroupDisplayNames
participant getUserGroups
Admin->>GroupRatioVisualEditor: edit group identifier or display name
GroupRatioVisualEditor->>GroupRatioVisualEditor: validate and migrate references
GroupRatioVisualEditor->>updateGroupOptions: submit changed settings
updateGroupOptions->>UpdateGroupOptions: PUT /api/option/group
UpdateGroupOptions->>GroupDisplayNames: validate and persist mapping
GroupDisplayNames-->>UpdateGroupOptions: updated configuration
UpdateGroupOptions-->>updateGroupOptions: update response
getUserGroups-->>Admin: stable identifier and display name
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 |
zd-Ethan
force-pushed
the
feat/stable-group-identifier
branch
from
July 31, 2026 07:54
eb1bc37 to
08f0ed6
Compare
![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: 5
🧹 Nitpick comments (2)
web/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsx (1)
50-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffUse React Testing Library for this component test.
This is a new test file under
web/**/__tests__/**/*.test.tsx. It rendersGroupRatioVisualEditorwithreact-dom/client createRootand queries the DOM directly withcontainer.querySelector, instead of@testing-library/react.Use
render,screen, andfireEvent/userEventfrom React Testing Library to query elements from the user's perspective and drive interactions, as required for component tests in this path.Based on learnings, this repo's
webworkspace should keep using Node's built-innode:testrunner for frontend unit tests rather than migrating to Vitest in isolation; this comment is about the missing use of React Testing Library within the existingnode:testsetup, not about switching test runners.
As per coding guidelines, "组件测试使用 React Testing Library，从用户视角查询元素并测试交互和行为，禁止断言内部 state、私有函数调用次数或无用户意义的 DOM 层级。"🤖 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/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsx` around lines 50 - 93, Update the GroupRatioVisualEditor test to use React Testing Library’s render, screen, and fireEvent or userEvent APIs instead of createRoot, direct container.querySelector calls, and the custom setInputValue helper. Preserve the existing node:test runner and test behavior while querying controls by accessible user-facing roles or labels and driving interactions through RTL utilities.Sources: Coding guidelines, Learnings
web/src/features/system-settings/models/ratio-settings-card.tsx (1)
407-433: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared server-error handler for consistency.
The catch block in
saveGroupRatiosbuilds its own error message instead of routing through the project's unifiedhandleServerError. The coding guideline forweb/**/*.{ts,tsx}files states server errors must go throughhandleServerError, withtoast.errorused for display.This mirrors the existing
resetMutation.onErrorpattern in this same file, so it is not a new regression, but the new grouped-save flow is a good place to start aligning with the documented convention.As per coding guidelines: "服务端错误统一使用
handleServerError;错误提示使用 i18n,统一通过toast.error等方式展示,路由错误由errorComponent承接。"🤖 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/src/features/system-settings/models/ratio-settings-card.tsx` around lines 407 - 433, Update the catch block in saveGroupRatios to route the caught error through the shared handleServerError utility, matching the existing resetMutation.onError pattern, and display the resulting localized message with toast.error instead of constructing an inline instanceof Error message.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 `@controller/group_option_test.go`:
- Around line 39-73: Update setupGroupOptionsUpdateTest to capture
setting.GroupDisplayNames2JSONString() before mutating test state and restore it
in t.Cleanup using the corresponding GroupDisplayNames update function,
alongside the existing GroupRatio and AutoGroups restoration. Keep the explicit
cache/state reset behavior unchanged.
In `@web/src/features/keys/components/api-keys-columns.tsx`:
- Around line 202-204: Update the auto branch’s GroupBadge rendering to pass the
configured display name through label={metadata?.label}, using the metadata
already derived near the ratio calculation. Leave other group rendering behavior
unchanged.
In `@web/src/i18n/locales/ru.json`:
- Line 2565: Update the Russian translation for the key “Manage group display
names, stable identifiers, ratios, and availability.” to explicitly translate
“display names” as “отображаемыми названиями,” while preserving the separate
wording for stable identifiers and the rest of the sentence.
- Line 2403: Update the Russian translation for "JSON map of stable group
identifiers to editable display names." so it explicitly describes each stable
group identifier mapping to an editable display name, preserving the source map
relationship rather than merely listing both concepts.
In `@web/src/i18n/locales/vi.json`:
- Line 2565: Update the Vietnamese translation for the key “Manage group display
names, stable identifiers, ratios, and availability.” to use “khả dụng” or
“trạng thái khả dụng” for availability, replacing “phạm vi sử dụng” while
preserving the rest of the translation.
---
Nitpick comments:
In
`@web/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsx`:
- Around line 50-93: Update the GroupRatioVisualEditor test to use React Testing
Library’s render, screen, and fireEvent or userEvent APIs instead of createRoot,
direct container.querySelector calls, and the custom setInputValue helper.
Preserve the existing node:test runner and test behavior while querying controls
by accessible user-facing roles or labels and driving interactions through RTL
utilities.
In `@web/src/features/system-settings/models/ratio-settings-card.tsx`:
- Around line 407-433: Update the catch block in saveGroupRatios to route the
caught error through the shared handleServerError utility, matching the existing
resetMutation.onError pattern, and display the resulting localized message with
toast.error instead of constructing an inline instanceof Error message.
🪄 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 Plus
Run ID: 20248e76-1f54-4f88-8116-81ca7df9b44a
📒 Files selected for processing (32)
controller/group.gocontroller/group_option_test.gocontroller/option.gomodel/option.gorouter/api-router.gosetting/group_display_name.gosetting/group_display_name_test.goweb/src/features/keys/components/api-keys-columns.tsxweb/src/features/keys/components/api-keys-mutate-drawer.tsxweb/src/features/models/components/drawers/model-mutate-drawer.tsxweb/src/features/playground/api.tsweb/src/features/system-settings/api.tsweb/src/features/system-settings/billing/index.tsxweb/src/features/system-settings/billing/section-registry.tsxweb/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsxweb/src/features/system-settings/models/__tests__/group-pricing.test.tsweb/src/features/system-settings/models/group-pricing.tsweb/src/features/system-settings/models/group-ratio-form.tsxweb/src/features/system-settings/models/group-ratio-visual-editor.tsxweb/src/features/system-settings/models/index.tsxweb/src/features/system-settings/models/ratio-settings-card.tsxweb/src/features/system-settings/types.tsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.jsonweb/src/lib/__tests__/group-options.test.tsweb/src/lib/api.tsweb/src/lib/group-options.ts
Contributor
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 `@web/src/i18n/locales/zh-TW.json`:
- Line 2565: Update the translation for the key “Manage group display names,
stable identifiers, ratios, and availability.” in the locale data so
“availability” is rendered as “可用性” instead of “可用範圍”, preserving the rest of
the translated text.
- Line 2403: Update the translation value for the JSON map description in zh-TW
so it explicitly states that stable group identifiers are the keys and editable
display names are the values, preserving the source mapping direction.
- Around line 2147-2149: Update the zh-TW translation for “Group identifier” to
use the established `標識符` terminology instead of `標識`, matching the existing
group-identifier messages while leaving the other translations unchanged.
🪄 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 Plus
Run ID: b7e91d6a-56be-4bf4-8b39-4c73e2a90a4f
📒 Files selected for processing (32)
controller/group.gocontroller/group_option_test.gocontroller/option.gomodel/option.gorouter/api-router.gosetting/group_display_name.gosetting/group_display_name_test.goweb/src/features/keys/components/api-keys-columns.tsxweb/src/features/keys/components/api-keys-mutate-drawer.tsxweb/src/features/models/components/drawers/model-mutate-drawer.tsxweb/src/features/playground/api.tsweb/src/features/system-settings/api.tsweb/src/features/system-settings/billing/index.tsxweb/src/features/system-settings/billing/section-registry.tsxweb/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsxweb/src/features/system-settings/models/__tests__/group-pricing.test.tsweb/src/features/system-settings/models/group-pricing.tsweb/src/features/system-settings/models/group-ratio-form.tsxweb/src/features/system-settings/models/group-ratio-visual-editor.tsxweb/src/features/system-settings/models/index.tsxweb/src/features/system-settings/models/ratio-settings-card.tsxweb/src/features/system-settings/types.tsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.jsonweb/src/lib/__tests__/group-options.test.tsweb/src/lib/api.tsweb/src/lib/group-options.ts
🚧 Files skipped from review as they are similar to previous changes (27)
- web/src/features/system-settings/models/index.tsx
- router/api-router.go
- web/src/features/keys/components/api-keys-mutate-drawer.tsx
- web/src/lib/tests/group-options.test.ts
- web/src/features/models/components/drawers/model-mutate-drawer.tsx
- web/src/features/system-settings/billing/index.tsx
- controller/group.go
- web/src/lib/group-options.ts
- web/src/features/system-settings/models/tests/group-pricing.test.ts
- setting/group_display_name_test.go
- web/src/features/system-settings/billing/section-registry.tsx
- web/src/features/system-settings/api.ts
- setting/group_display_name.go
- web/src/features/playground/api.ts
- web/src/i18n/locales/zh.json
- web/src/features/keys/components/api-keys-columns.tsx
- web/src/features/system-settings/models/group-pricing.ts
- web/src/features/system-settings/models/tests/group-identifier-presentation.test.tsx
- web/src/i18n/locales/en.json
- web/src/i18n/locales/fr.json
- web/src/lib/api.ts
- model/option.go
- web/src/features/system-settings/models/group-ratio-form.tsx
- controller/group_option_test.go
- web/src/i18n/locales/vi.json
- web/src/features/system-settings/models/ratio-settings-card.tsx
- web/src/features/system-settings/models/group-ratio-visual-editor.tsx
Contributor
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 (2)
web/src/i18n/locales/zh-TW.json (1)
1453-1453: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the established group-identifier terminology.
Line 1453 uses
標識, while this locale uses標識符for stable identifiers. Change the translation to分組標識符重複：{{identifiers}}.🤖 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/src/i18n/locales/zh-TW.json` at line 1453, Update the zh-TW translation for the “Duplicate group identifiers: {{identifiers}}” key to use the established “標識符” terminology, resulting in “分組標識符重複：{{identifiers}}”; preserve the placeholder unchanged.web/src/i18n/locales/zh.json (1)
2404-2404: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve the JSON mapping direction in the translation.
The English text states that stable group identifiers map to editable display names. The current translation only states an association, so users may miss that identifiers are keys and display names are values.
Proposed fix
- "JSON map of stable group identifiers to editable display names.": "稳定分组标识与可编辑显示名称的 JSON 映射。", + "JSON map of stable group identifiers to editable display names.": "从稳定分组标识到可编辑显示名称的 JSON 映射。",🤖 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/src/i18n/locales/zh.json` at line 2404, The zh.json translation for “JSON map of stable group identifiers to editable display names.” must explicitly preserve the mapping direction: stable group identifiers are keys that map to editable display names as values. Update only this translation string to convey that key-to-value relationship clearly.
🧹 Nitpick comments (1)
web/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsx (1)
234-288: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard cleanup with try/finally.
If an
assert.okcall fails inside this test,root.unmount()andcontainer.remove()at the end never run. The container stays attached todocument.bodyfor the rest of this test file's run.Wrap the render, assertions, and cleanup in try/finally so DOM state is always reset, regardless of test outcome.
As per path instructions: "每个测试独立初始化和清理全局状态、缓存、localStorage、mock 与定时器,不依赖执行顺序。"
♻️ Proposed fix to guarantee cleanup
- await act(async () => root.render(<Harness />)) - - const jsonModeButton = [...container.querySelectorAll('button')].find( - (button) => button.textContent?.includes('Switch to JSON') - ) - assert.ok(jsonModeButton) - await act(async () => jsonModeButton.click()) - - const editableFields = [ - 'GroupRatio', - 'GroupDisplayNames', - 'TopupGroupRatio', - 'UserUsableGroups', - 'GroupGroupRatio', - 'AutoGroups', - 'GroupSpecialUsableGroup', - ] - for (const fieldName of editableFields) { - assert.ok(container.querySelector(`textarea[name="${fieldName}"]`)) - } - - await act(async () => root.unmount()) - container.remove() + try { + await act(async () => root.render(<Harness />)) + + const jsonModeButton = [...container.querySelectorAll('button')].find( + (button) => button.textContent?.includes('Switch to JSON') + ) + assert.ok(jsonModeButton) + await act(async () => jsonModeButton.click()) + + const editableFields = [ + 'GroupRatio', + 'GroupDisplayNames', + 'TopupGroupRatio', + 'UserUsableGroups', + 'GroupGroupRatio', + 'AutoGroups', + 'GroupSpecialUsableGroup', + ] + for (const fieldName of editableFields) { + assert.ok(container.querySelector(`textarea[name="${fieldName}"]`)) + } + } finally { + await act(async () => root.unmount()) + container.remove() + }🤖 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/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsx` around lines 234 - 288, Wrap the test body after creating the DOM container and root in a try/finally block, placing rendering and all assertions in the try section and moving root.unmount() plus container.remove() into finally. Keep the existing test setup and assertions unchanged while ensuring cleanup runs even when an assertion fails.Source: Path instructions
🤖 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 `@web/src/i18n/locales/zh-TW.json`:
- Line 1453: Update the zh-TW translation for the “Duplicate group identifiers:
{{identifiers}}” key to use the established “標識符” terminology, resulting in
“分組標識符重複：{{identifiers}}”; preserve the placeholder unchanged.
In `@web/src/i18n/locales/zh.json`:
- Line 2404: The zh.json translation for “JSON map of stable group identifiers
to editable display names.” must explicitly preserve the mapping direction:
stable group identifiers are keys that map to editable display names as values.
Update only this translation string to convey that key-to-value relationship
clearly.
---
Nitpick comments:
In
`@web/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsx`:
- Around line 234-288: Wrap the test body after creating the DOM container and
root in a try/finally block, placing rendering and all assertions in the try
section and moving root.unmount() plus container.remove() into finally. Keep the
existing test setup and assertions unchanged while ensuring cleanup runs even
when an assertion fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e676757-5405-4fdf-9532-9a7fa9698132
📒 Files selected for processing (13)
controller/group_option_test.gosetting/group_display_name.goweb/src/features/keys/components/api-keys-columns.tsxweb/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsxweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.jsonweb/src/lib/__tests__/group-options.test.tsweb/src/lib/group-options.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- web/src/features/keys/components/api-keys-columns.tsx
- setting/group_display_name.go
- web/src/i18n/locales/fr.json
- web/src/lib/group-options.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsx (1)
103-113: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winType the test context and cleanup callbacks.
This file already uses
node:test, sotestContext.aftermatches the runner. Add explicit types for the untyped anonymous callbacks: type thetestContextparameter as the runner context and annotate each async hook as(callback: Promise<void>) => void.Also applies to: 151-161, 242-252
🤖 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/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsx` around lines 103 - 113, Update the affected tests in group-identifier-presentation.test.tsx to type the testContext parameter with the node:test runner context and annotate each async cleanup callback passed to testContext.after as (callback: Promise<void>) => void. Apply this consistently to the cleanup hooks around the tests at the shown locations, preserving their existing unmount and container-removal behavior.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.
Nitpick comments:
In
`@web/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsx`:
- Around line 103-113: Update the affected tests in
group-identifier-presentation.test.tsx to type the testContext parameter with
the node:test runner context and annotate each async cleanup callback passed to
testContext.after as (callback: Promise<void>) => void. Apply this consistently
to the cleanup hooks around the tests at the shown locations, preserving their
existing unmount and container-removal behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3df601c8-a981-4567-a566-63529c7db8b6
📒 Files selected for processing (3)
web/src/features/system-settings/models/__tests__/group-identifier-presentation.test.tsxweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/i18n/locales/zh-TW.json
- web/src/i18n/locales/zh.json
zd-Ethan
changed the title
zd-Ethan
changed the title
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
GroupDisplayNames配置；API Key、Playground 等界面显示分组名称，但提交和存储仍使用稳定标识。changedKeys仅持久化实际变更字段，并通过单个事务原子更新相关分组配置。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation