feat: enhance provider group selection in users page [#423] - #424
Conversation
Summary of ChangesHello @Hwwwww-dev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the user interface and functionality for managing provider groups within the application's users page. It introduces a more intuitive multi-selection experience for creating API keys, ensures smart handling of the 'default' provider group, and enhances the visual presentation of assigned groups across various user and key management views. These changes aim to streamline the process of assigning and viewing provider groups, improving overall usability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request enhances the provider group selection by introducing multi-select capabilities, automatically handling the 'default' group, and improving the UI for displaying groups. The changes are generally good, but I've found a recurring logic issue in how provider group strings are processed, which could lead to saving malformed data. I've also identified some dead code that can be cleaned up. My review includes suggestions to fix the bug and improve code quality.
| const handleUserProviderGroupChange = useCallback( | ||
| (newValue: string) => { | ||
| const groups = newValue | ||
| .split(",") | ||
| .map((g) => g.trim()) | ||
| .filter(Boolean); | ||
| // 如果有多个分组且包含 default,移除 default | ||
| if (groups.length > 1 && groups.includes(PROVIDER_GROUP.DEFAULT)) { | ||
| const withoutDefault = groups.filter((g) => g !== PROVIDER_GROUP.DEFAULT); | ||
| onChange("providerGroup", withoutDefault.join(",")); | ||
| } else { | ||
| onChange("providerGroup", newValue); | ||
| } | ||
| }, | ||
| [onChange] | ||
| ); |
There was a problem hiding this comment.
The handleUserProviderGroupChange function has a logic flaw. In the else block, it passes the raw newValue string to onChange. This string is not trimmed or cleaned of empty parts, unlike the logic in the if block. This can lead to inconsistent and malformed data being saved, for example, with leading/trailing spaces or extra commas.
A similar issue exists in src/app/[locale]/dashboard/_components/user/forms/provider-group-select.tsx. Consider extracting this logic into a shared utility function to avoid duplication and ensure consistency.
const handleUserProviderGroupChange = useCallback(
(newValue: string) => {
let groups = newValue
.split(",")
.map((g) => g.trim())
.filter(Boolean);
// 如果有多个分组且包含 default,移除 default
if (groups.length > 1 && groups.includes(PROVIDER_GROUP.DEFAULT)) {
groups = groups.filter((g) => g !== PROVIDER_GROUP.DEFAULT);
}
onChange("providerGroup", groups.join(","));
},
[onChange]
);
| const handleChange = useCallback( | ||
| (newValue: string) => { | ||
| const groupList = newValue | ||
| .split(",") | ||
| .map((g) => g.trim()) | ||
| .filter(Boolean); | ||
| // 如果有多个分组且包含 default,移除 default | ||
| if (groupList.length > 1 && groupList.includes(PROVIDER_GROUP.DEFAULT)) { | ||
| const withoutDefault = groupList.filter((g) => g !== PROVIDER_GROUP.DEFAULT); | ||
| onChange(withoutDefault.join(",")); | ||
| } else { | ||
| onChange(newValue); | ||
| } | ||
| }, | ||
| [onChange] | ||
| ); |
There was a problem hiding this comment.
This handleChange function has a logic flaw. The else branch passes the raw, unprocessed newValue to onChange, which can lead to saving malformed data (e.g. with extra spaces or commas).
This logic is also duplicated in src/app/[locale]/dashboard/_components/user/forms/key-edit-section.tsx. It would be best to extract this into a shared utility function to ensure consistency and maintainability.
const handleChange = useCallback(
(newValue: string) => {
let groupList = newValue
.split(",")
.map((g) => g.trim())
.filter(Boolean);
// 如果有多个分组且包含 default,移除 default
if (groupList.length > 1 && groupList.includes(PROVIDER_GROUP.DEFAULT)) {
groupList = groupList.filter((g) => g !== PROVIDER_GROUP.DEFAULT);
}
onChange(groupList.join(","));
},
[onChange]
);
| {keyGroupOptions.length > 0 ? ( | ||
| keyGroupOptions.map((group) => ( | ||
| <Badge key={group} variant="secondary" className="text-xs"> | ||
| {group} | ||
| </Badge> | ||
| )) | ||
| ) : ( | ||
| <Badge variant="outline" className="text-xs"> | ||
| {group} | ||
| {PROVIDER_GROUP.DEFAULT} | ||
| </Badge> | ||
| </SelectItem> | ||
| ))} | ||
| {userGroups.length > 1 && ( | ||
| <SelectItem value={normalizedUserProviderGroup}> | ||
| {translations.fields.providerGroup.allGroups || "全部分组"} | ||
| </SelectItem> | ||
| )} | ||
| </SelectContent> | ||
| </Select> | ||
| <p className="text-xs text-muted-foreground"> | ||
| {keyData.id > 0 | ||
| ? translations.fields.providerGroup.editHint || "已有密钥的分组不可修改" | ||
| : translations.fields.providerGroup.selectHint || "选择此 Key 可使用的供应商分组"} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
The else branch in this ternary operator appears to be unreachable. The keyGroupOptions array is derived from normalizedKeyProviderGroup, which is processed by normalizeGroupList. This utility function ensures it always returns a non-empty string (at least "default"), so keyGroupOptions will never be an empty array. This makes the else block dead code.
You can simplify the code by removing the ternary operator and the unreachable else block.
{keyGroupOptions.map((group) => (
<Badge key={group} variant="secondary" className="text-xs">
{group}
</Badge>
))}
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The implementation cleanly adds provider group selection functionality with proper auto-removal of the "default" group when users select other groups.
PR Size: S
- Lines changed: 475 (181 additions, 294 deletions)
- Files changed: 7
Review Highlights
New Functionality Reviewed:
handleUserProviderGroupChangecallback in key-edit-section.tsx correctly filters out "default" group when other groups are selectedhandleChangecallback in provider-group-select.tsx implements identical logic for admin context- TagInput
keepOpenparameter improves UX for multi-selection scenarios - Grid layout adjustments in key-row-item.tsx for better column balance
- Read-only badge display in user-edit-section.tsx for provider groups
Design Notes:
- The "default" group auto-removal logic is duplicated in two components, which is acceptable given they serve different user contexts (admin vs non-admin)
- The
maxTags={userGroups.length + 1}setting appropriately allows for the default group case
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - N/A (UI components)
- Code clarity - Good
Automated review by Claude AI
|
对于 Admin 侧,用户编辑部分的 ProviderGroup 也应该使用 Read-Only Display。 |
|
抱歉没注意,pr 到 main 了 🥲 |
base 分支我帮你改了,你可以考虑强推一次把 fork 这边的 rebase 为当前的 dev。非常感谢。 |
已经修改成了 RO,只不过使用了 Tag 显示 |
好的,我一会处理一下 |
- Add ProviderGroupSelect for admin user/key editing - Support multi-select for non-admin key creation with TagInputField - Auto-remove 'default' group when selecting other groups - Keep dropdown open during multi-selection (tag-input) - Display user provider group as read-only badges - Adjust key row grid layout for better group/expiry column balance - Show +N for remaining groups when >1 group exists 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
a509a6d to
9498358
Compare
|
@ding113 Rebase 已完成 |
Summary
Enhances provider group selection in the users page by adding multi-select capability for non-admin key creation and improving UX with auto-removal of the "default" group when specific groups are selected.
Problem
When non-admin users create API keys, they can only select a single provider group via dropdown. This limits flexibility since users may have access to multiple provider groups and want to assign multiple groups to a new key.
Fixes #423 - 用户模式创建 KEY 时供应商分组优化 (Optimize provider group selection when users create keys)
Solution
1. Multi-Select for Non-Admin Key Creation
Replaced the single-select dropdown with
TagInputFieldcomponent that supports multi-select:2. Auto-Remove "default" Group Logic
Added smart default group handling:
3. Read-Only Display for Existing Keys
Changed behavior when editing existing keys:
4. UI Polish
Changes
Core Changes
src/app/[locale]/dashboard/_components/user/forms/key-edit-section.tsx- Switch to TagInputField for multi-select, add auto-remove default logicsrc/app/[locale]/dashboard/_components/user/forms/provider-group-select.tsx- Add auto-remove "default" when selecting other groupssrc/components/ui/tag-input.tsx- Keep dropdown open during multi-selectionSupporting Changes
src/app/[locale]/dashboard/_components/user/forms/user-edit-section.tsx- Display provider group as read-only badgessrc/app/[locale]/dashboard/_components/user/key-row-item.tsx- Adjust grid layout, show +N for excess groupspublic/seed/litellm-prices.json- Updated model pricing data (unrelated maintenance)Related PRs
Testing
Manual Testing
Multi-Select Key Creation:
Auto-Remove Default:
Read-Only Edit Mode:
Checklist
Description enhanced by Claude AI