fix(pricing): add filtering for pricing based on usable groups - #4123
Conversation
WalkthroughA new filter function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/pricing.go`:
- Around line 16-18: The early return when len(usableGroup) == 0 hides models
whose model.EnableGroup includes "all"; remove that early return and instead
treat "all" as a global match before or during filtering: when iterating models
(the code that checks model.EnableGroup), first check if "all" is present and
include the model regardless of usableGroup, otherwise check membership against
usableGroup (returned by GetUserUsableGroups()). In short, delete the
len(usableGroup)==0 return, and ensure the EnableGroup check for the literal
"all" runs (or is evaluated) before refusing models due to an empty usableGroup.
🪄 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: 77cd3dee-71a2-4c89-b97e-e3fe05a37b75
📒 Files selected for processing (1)
controller/pricing.go
| if len(usableGroup) == 0 { | ||
| return []model.Pricing{} | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the pricing.go file and examine the full function
find . -name "pricing.go" -type f | head -20Repository: QuantumNous/new-api
Length of output: 122
🏁 Script executed:
# Look at the controller/pricing.go file around lines 12-34
cat -n controller/pricing.go | sed -n '1,50p'Repository: QuantumNous/new-api
Length of output: 1673
🏁 Script executed:
# Also search for GetUserUsableGroups to understand when it returns empty
rg "GetUserUsableGroups" --type go -A 5 -B 2Repository: QuantumNous/new-api
Length of output: 3627
🏁 Script executed:
# Search for tests related to filterPricingByUsableGroups
rg "filterPricingByUsableGroups|GetPricing" --type go -l | xargs grep -l "test\|Test" 2>/dev/null || echo "No direct tests found, searching more broadly"Repository: QuantumNous/new-api
Length of output: 91
🏁 Script executed:
# Look at GetUserUsableGroups full implementation
rg "func GetUserUsableGroups" --type go -A 30Repository: QuantumNous/new-api
Length of output: 3342
🏁 Script executed:
# Check for EnableGroup usage patterns and semantics
rg "EnableGroup" --type go -B 2 -A 2 | head -100Repository: QuantumNous/new-api
Length of output: 4045
🏁 Script executed:
# Check the model/pricing.go to understand Pricing struct
cat -n model/pricing.go | head -50Repository: QuantumNous/new-api
Length of output: 2446
🏁 Script executed:
# Look for any comments or context around the GetPricing function
cat -n controller/pricing.go | sed -n '36,100p'Repository: QuantumNous/new-api
Length of output: 2186
🏁 Script executed:
# Check if there are any related issues or comments in git history or code
rg "all.*EnableGroup|EnableGroup.*all" --type go -B 3 -A 3Repository: QuantumNous/new-api
Length of output: 409
🏁 Script executed:
# Look for any explicit tests or usage patterns that clarify intent
rg "filterPricingByUsableGroups|GetPricing" --type go -B 5 -A 10 | grep -A 10 -B 5 "test\|Test\|all"Repository: QuantumNous/new-api
Length of output: 724
🏁 Script executed:
# Check the context where filterPricingByUsableGroups is called and what happens when group is empty
cat -n controller/pricing.go | sed -n '36,75p'Repository: QuantumNous/new-api
Length of output: 1516
🏁 Script executed:
# Let's verify GetUserUsableGroups behavior more carefully when group is empty
cat -n service/group.go | sed -n '1,50p'Repository: QuantumNous/new-api
Length of output: 1852
🏁 Script executed:
# Check if there are any tests for GetPricing or filterPricingByUsableGroups
find . -name "*test*.go" -o -name "*_test.go" | xargs grep -l "GetPricing\|filterPricingByUsableGroups" 2>/dev/null | head -5Repository: QuantumNous/new-api
Length of output: 45
🏁 Script executed:
# Look at the complete GetUserUsableGroups function to understand when it returns empty
cat -n service/group.go | grep -A 30 "func GetUserUsableGroups"Repository: QuantumNous/new-api
Length of output: 1280
🏁 Script executed:
# Check setting/user_usable_group.go to understand when GetUserUsableGroupsCopy() would be empty
cat -n setting/user_usable_group.go | head -80Repository: QuantumNous/new-api
Length of output: 1669
🏁 Script executed:
# Search for documentation or comments about what "all" means in EnableGroup
rg "\"all\"" --type go -B 5 -A 5 | grep -i -B 5 -A 5 "enablegroup\|enable_group" | head -50Repository: QuantumNous/new-api
Length of output: 750
🏁 Script executed:
# Check if there are any integration tests or API examples
find . -name "*.md" -o -name "*.txt" | xargs grep -l "EnableGroup\|enable_group" 2>/dev/nullRepository: QuantumNous/new-api
Length of output: 47
🏁 Script executed:
# Final verification: check if there's any documentation or code that explicitly handles empty userUsableGroups as a valid scenario
rg "UpdateUserUsableGroupsByJSONString\|userUsableGroups\s*=" --type go -B 2 -A 2 | head -60Repository: QuantumNous/new-api
Length of output: 45
🏁 Script executed:
# Check if there are any admin settings or configuration that shows empty groups is a valid use case
rg "userUsableGroups|UserUsableGroups" --type go -l | xargs grep -h "empty\|nil\|check" 2>/dev/null | head -20Repository: QuantumNous/new-api
Length of output: 705
Models with EnableGroup containing "all" will be hidden when usableGroup is empty.
The early return on line 17 prevents the "all" check (line 22) from executing when usableGroup is empty. While GetUserUsableGroups() returns system-configured groups by default, it can be empty if the system's userUsableGroups setting is explicitly cleared. In this scenario, models intended to be visible to everyone (those with "all" in EnableGroup) would be incorrectly hidden.
If models with "all" should always be visible regardless of user groups, move the "all" check before the empty-map guard:
🐛 Proposed fix
func filterPricingByUsableGroups(pricing []model.Pricing, usableGroup map[string]string) []model.Pricing {
if len(pricing) == 0 {
return pricing
}
- if len(usableGroup) == 0 {
- return []model.Pricing{}
- }
filtered := make([]model.Pricing, 0, len(pricing))
for _, item := range pricing {
if common.StringsContains(item.EnableGroup, "all") {
filtered = append(filtered, item)
continue
}
+ if len(usableGroup) == 0 {
+ continue
+ }
for _, group := range item.EnableGroup {
if _, ok := usableGroup[group]; ok {
filtered = append(filtered, item)
break
}
}
}
return filtered
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/pricing.go` around lines 16 - 18, The early return when
len(usableGroup) == 0 hides models whose model.EnableGroup includes "all";
remove that early return and instead treat "all" as a global match before or
during filtering: when iterating models (the code that checks
model.EnableGroup), first check if "all" is present and include the model
regardless of usableGroup, otherwise check membership against usableGroup
(returned by GetUserUsableGroups()). In short, delete the len(usableGroup)==0
return, and ensure the EnableGroup check for the literal "all" runs (or is
evaluated) before refusing models due to an empty usableGroup.
Calcium-Ion
added
the
ready to merge
seefs001
removed
the
ready to merge
fix(pricing): add filtering for pricing based on usable groups
3 participants
修复模型广场模型泄露问题：
/api/pricing新增按用户可用分组过滤，避免仅在不可见分组启用的模型仍出现在模型广场。Summary by CodeRabbit