feat: support unlimited balance for users (#5827) - #5830
Closed
Gravirei wants to merge 2 commits into
Closed
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)
WalkthroughAdds a per-user unlimited-balance flag through backend user data, relay context, quota handling, and user management, plus frontend display/toggle support and locale strings. ChangesUnlimited Balance Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant UsersMutateDrawer
participant manageUserUnlimitedBalance
participant ManageUser
participant Cache
User->>UsersMutateDrawer: toggle unlimited balance
UsersMutateDrawer->>manageUserUnlimitedBalance: call with id, enabled
manageUserUnlimitedBalance->>ManageUser: POST /api/user/manage action unlimited_balance
ManageUser->>Cache: invalidate user cache
ManageUser-->>manageUserUnlimitedBalance: success response
manageUserUnlimitedBalance-->>UsersMutateDrawer: refresh user data
sequenceDiagram
participant Relay
participant RelayInfo
participant BillingSession
participant PreConsumeQuota
participant QuotaService
Relay->>RelayInfo: read ContextKeyUserUnlimitedBalance
RelayInfo->>BillingSession: pass UserUnlimitedBalance flag
BillingSession->>BillingSession: shouldTrust returns true for wallet
BillingSession->>PreConsumeQuota: continue with unlimited balance
PreConsumeQuota->>PreConsumeQuota: skip insufficient quota checks
PreConsumeQuota->>QuotaService: trust branch with unlimited balance
QuotaService->>QuotaService: skip user quota error
Possibly related PRs
Suggested reviewers: 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 |
Gravirei
force-pushed
the
feat/enhancement
branch
from
July 1, 2026 03:19
2319b35 to
fe814fb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
web/default/src/features/users/components/users-mutate-drawer.tsx (3)
405-418: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNested ternary exceeds one level.
The
valueexpression nests a second ternary (tokensOnly ? ... : ...) inside theunlimited_balanceternary, creating two levels of nesting.♻️ Suggested refactor: extract a small helper
+ const getQuotaInputValue = () => { + if (currentRow?.unlimited_balance) return t('Unlimited') + if (tokensOnly) return String(field.value || 0) + return (field.value || 0).toFixed(6) + } + <Input - value={ - currentRow?.unlimited_balance - ? t('Unlimited') - : tokensOnly - ? String(field.value || 0) - : (field.value || 0).toFixed(6) - } + value={getQuotaInputValue()} readOnly className='flex-1' />As per path instructions, "Avoid nested ternary expressions deeper than one level; prefer if/else, early returns, or extracted helper functions."
🤖 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/users/components/users-mutate-drawer.tsx` around lines 405 - 418, The Input value expression in users-mutate-drawer.tsx uses a nested ternary inside the currentRow?.unlimited_balance check, which violates the no-deep-nesting rule. Refactor the value logic by extracting it into a small helper or computing it with if/else/early returns near the affected JSX in the users-mutate-drawer component, so the Unlimited, tokensOnly, and fixed-precision cases are handled without nested ternaries.Source: Path instructions
438-477: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSwitch label/description aren't programmatically associated with the control.
Unlike the surrounding
FormField-wrapped controls (which gethtmlFor/aria-*wiring viauseFormField()), thisFormLabel/FormDescriptionpair sits outside anyFormItem/FormFieldcontext, so they render as plain text with nohtmlFor/idlink to theSwitch. Screen reader users won't get an accessible name for the toggle.♿ Suggested fix: wire up explicit ids
- <div className={sideDrawerSwitchItemClassName()}> + <div className={sideDrawerSwitchItemClassName()}> <div className='flex flex-col gap-0.5'> - <FormLabel className='text-sm'> + <FormLabel htmlFor='unlimited-balance-switch' className='text-sm'> {t('Unlimited Balance')} </FormLabel> - <FormDescription className='text-xs'> + <FormDescription id='unlimited-balance-desc' className='text-xs'> {t('Enable unlimited balance for this user')} </FormDescription> </div> <Switch + id='unlimited-balance-switch' + aria-describedby='unlimited-balance-desc' checked={currentRow.unlimited_balance ?? false}As per path instructions, "Use semantic HTML, proper form label associations, keyboard accessibility, appropriate ARIA attributes."
🤖 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/users/components/users-mutate-drawer.tsx` around lines 438 - 477, The Unlimited Balance toggle in users-mutate-drawer.tsx is missing a programmatic label association, so the FormLabel/FormDescription are not tied to the Switch. Update the toggle block around the currentRow.unlimited_balance Switch to give the control a stable id and connect the text with explicit htmlFor and/or aria-labelledby/aria-describedby wiring. Keep the existing FormLabel and FormDescription content, but make sure the Switch has an accessible name and description for screen readers.Source: Path instructions
449-475: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSwitch isn't disabled while the request is in flight.
Rapid toggling before
manageUserUnlimitedBalanceresolves can fire overlapping requests, and the UI gives no feedback that a change is pending (only relies oncurrentRowbeing refreshed afterward, which may lag a re-render).🔒 Suggested fix: track a pending flag
+ const [isTogglingBalance, setIsTogglingBalance] = useState(false) ... <Switch checked={currentRow.unlimited_balance ?? false} + disabled={isTogglingBalance} onCheckedChange={async (checked) => { + setIsTogglingBalance(true) try { const result = await manageUserUnlimitedBalance( currentRow.id, checked ) ... } catch { toast.error(t('Failed to update unlimited balance')) + } finally { + setIsTogglingBalance(false) } }} />🤖 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/users/components/users-mutate-drawer.tsx` around lines 449 - 475, The unlimited-balance Switch in users-mutate-drawer.tsx is not protected against overlapping updates, so rapid toggles can trigger multiple manageUserUnlimitedBalance calls before the first resolves. Add a pending/loading state around the Switch (for example in the currentRow update handler near manageUserUnlimitedBalance and refreshUserData) that is set before awaiting the request and cleared in finally, then pass it to the Switch as disabled and use it to block further onCheckedChange calls until the request finishes.
🤖 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/default/src/features/users/components/users-mutate-drawer.tsx`:
- Around line 405-418: The Input value expression in users-mutate-drawer.tsx
uses a nested ternary inside the currentRow?.unlimited_balance check, which
violates the no-deep-nesting rule. Refactor the value logic by extracting it
into a small helper or computing it with if/else/early returns near the affected
JSX in the users-mutate-drawer component, so the Unlimited, tokensOnly, and
fixed-precision cases are handled without nested ternaries.
- Around line 438-477: The Unlimited Balance toggle in users-mutate-drawer.tsx
is missing a programmatic label association, so the FormLabel/FormDescription
are not tied to the Switch. Update the toggle block around the
currentRow.unlimited_balance Switch to give the control a stable id and connect
the text with explicit htmlFor and/or aria-labelledby/aria-describedby wiring.
Keep the existing FormLabel and FormDescription content, but make sure the
Switch has an accessible name and description for screen readers.
- Around line 449-475: The unlimited-balance Switch in users-mutate-drawer.tsx
is not protected against overlapping updates, so rapid toggles can trigger
multiple manageUserUnlimitedBalance calls before the first resolves. Add a
pending/loading state around the Switch (for example in the currentRow update
handler near manageUserUnlimitedBalance and refreshUserData) that is set before
awaiting the request and cleared in finally, then pass it to the Switch as
disabled and use it to block further onCheckedChange calls until the request
finishes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4c69b95-77f6-47fb-ab1b-149aae282cbe
📒 Files selected for processing (18)
constant/context_key.gocontroller/user.gomodel/user.gomodel/user_cache.gorelay/common/relay_info.goservice/billing_session.goservice/pre_consume_quota.goservice/quota.goweb/default/src/features/users/api.tsweb/default/src/features/users/components/users-columns.tsxweb/default/src/features/users/components/users-mutate-drawer.tsxweb/default/src/features/users/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
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.
📝 变更描述 / Description
Adds an
unlimited_balanceboolean field to theUsermodel, mirroring the existingToken.UnlimitedQuotapattern. When enabled, all user balance checks during API relay are bypassed, allowing unlimited usage without manually setting a large balance.How it works
User.UnlimitedBalance(bool) — stored in DB, cached inUserBaseUserBase→ Gin context →RelayInfo.UserUnlimitedBalanceBillingSession.tryWallet()— skips quota ≤ 0 and pre-consume checksPreConsumeQuota()(legacy) — same bypassPreWssConsumeQuota()(realtime) — skips user quota checkManageUserendpoint with action"unlimited_balance"+ value0/1🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Summary by CodeRabbit