feat(subscription): add admin quota reset actions - #5952
Conversation
WalkthroughAdds transactional admin subscription reset flows for plan-wide and per-user plan resets, with matching routes, audit entries, web actions/dialogs, and locale updates. ChangesAdmin Subscription Quota Reset
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant Router as api-router
participant Controller as controller.subscription
participant Model as model.subscription
participant Audit as controller.audit
Admin->>Router: POST /api/subscription/admin/plans/:id/subscriptions/reset
Router->>Controller: AdminResetPlanSubscriptions(c)
Controller->>Model: AdminResetPlanSubscriptions(planId, advanceResetTime)
Model-->>Controller: SubscriptionResetResult
Controller->>Audit: render subscription.plan_reset template
Controller-->>Admin: JSON response
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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
model/subscription.go (1)
1047-1063: 🚀 Performance & Scalability | 🔵 TrivialPlan-wide reset loads all matching rows and issues one
Saveper subscription inside the transaction.For a popular plan this materializes every active subscription into memory and performs N individual
UPDATEstatements while holdingFOR UPDATElocks, producing a long-lived transaction and elevated lock contention. WhenadvanceResetTimeis false the update is uniform (amount_used = 0) and could be a single bulkUPDATE; when true, consider batching. Not blocking, but worth planning before this runs against large plans.🤖 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 `@model/subscription.go` around lines 1047 - 1063, The plan-wide reset in adminResetPlanSubscriptionsTx currently loads every matching UserSubscription and then calls resetUserSubscriptionTx per row, which causes N updates and long lock holding. Refactor the reset path to avoid per-subscription Save in the uniform case: when advanceResetTime is false, use a single bulk update for all matching rows; when advanceResetTime is true, consider batching or a set-based approach instead of materializing all rows. Keep the existing result-building behavior in buildSubscriptionResetResult and preserve the locking/query criteria around plan_id, status, and end_time.
🤖 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 `@model/subscription.go`:
- Around line 1030-1033: Gate the locking behavior in the subscription reset
query so `FOR UPDATE` is only applied on dialects that support it. Update the
query built in `subscription.go` around the `tx.Set("gorm:query_option", "FOR
UPDATE")` usage to conditionally add the lock based on the current DB dialect,
and apply the same dialect-based fallback to the second reset query in the same
flow so SQLite/local `model.InitDB()` setups do not execute `SELECT ... FOR
UPDATE`.
In
`@web/default/src/features/subscriptions/components/dialogs/reset-subscriptions-dialog.tsx`:
- Around line 42-63: The reset flow in handleConfirm only shows an error in the
catch block, so a resolved response from resetPlanSubscriptions with success
false is ignored and the dialog appears to do nothing. Update handleConfirm in
reset-subscriptions-dialog.tsx to explicitly handle the non-throwing failure
path by checking res.success and showing a toast.error when it is false, while
keeping the existing success path (toast.success, triggerRefresh, setOpen(null))
unchanged.
In
`@web/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx`:
- Around line 131-147: The `activePlanOptions` memo in
`user-subscriptions-dialog` is filtering out never-expiring subscriptions
because `(sub.end_time || 0) <= now` treats `end_time === 0` as expired. Update
the filtering logic in `activePlanOptions` to match the existing
`isExpired`/`isActive` convention used elsewhere in this component, so `end_time
=== 0` is treated as active. Keep the dedupe-by-`plan_id` behavior intact and
ensure unlimited/lifetime plans appear in the reset dropdown.
---
Nitpick comments:
In `@model/subscription.go`:
- Around line 1047-1063: The plan-wide reset in adminResetPlanSubscriptionsTx
currently loads every matching UserSubscription and then calls
resetUserSubscriptionTx per row, which causes N updates and long lock holding.
Refactor the reset path to avoid per-subscription Save in the uniform case: when
advanceResetTime is false, use a single bulk update for all matching rows; when
advanceResetTime is true, consider batching or a set-based approach instead of
materializing all rows. Keep the existing result-building behavior in
buildSubscriptionResetResult and preserve the locking/query criteria around
plan_id, status, and end_time.
🪄 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: 1cf954e9-5317-4191-b492-259f56f5665c
📒 Files selected for processing (17)
controller/audit.gocontroller/subscription.gomodel/subscription.gomodel/subscription_reset_test.gorouter/api-router.goweb/default/src/features/subscriptions/api.tsweb/default/src/features/subscriptions/components/data-table-row-actions.tsxweb/default/src/features/subscriptions/components/dialogs/reset-subscriptions-dialog.tsxweb/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsxweb/default/src/features/subscriptions/components/subscriptions-dialogs.tsxweb/default/src/features/subscriptions/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
| if err := tx.Set("gorm:query_option", "FOR UPDATE"). | ||
| Where("user_id = ? AND plan_id = ? AND status = ? AND end_time > ?", userId, plan.Id, "active", now). | ||
| Order("end_time asc, id asc"). | ||
| Find(&subs).Error; err != nil { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# How is FOR UPDATE / locking used elsewhere, and what DB do model tests run on?
rg -nP 'gorm:query_option|FOR UPDATE|clause\.Locking' --type=go -C2 -g '!**/vendor/**'
echo '--- test DB backend setup ---'
rg -nP 'sqlite|SQLITE|InitDB|TestMain|truncateTables' --type=go -g '!**/vendor/**' -C2 model/ | head -100Repository: QuantumNous/new-api
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- model/subscription.go outline ---'
ast-grep outline model/subscription.go --view expanded | sed -n '1,220p'
echo '--- relevant lines around 980-1070 ---'
sed -n '980,1070p' model/subscription.go | cat -n
echo '--- search for SQLite / test DB setup ---'
rg -n --hidden -g '!**/vendor/**' -e 'sqlite|SQLITE|InitDB|TestMain|truncateTables|gorm.*sqlite|dsn' model . | sed -n '1,220p'
echo '--- search for locking APIs elsewhere ---'
rg -n --hidden -g '!**/vendor/**' -e 'gorm:query_option|FOR UPDATE|clause\.Locking|Locking\{' . | sed -n '1,220p'Repository: QuantumNous/new-api
Length of output: 29058
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- file list for model ---'
git ls-files model | sed -n '1,200p'
echo '--- size of model/subscription.go ---'
wc -l model/subscription.go
echo '--- snippets around the queried functions ---'
sed -n '1020,1068p' model/subscription.go | cat -nRepository: QuantumNous/new-api
Length of output: 3473
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- locate subscription.go ---'
fd -a 'subscription.go$' . | sed -n '1,40p'
echo '--- locate test main / sqlite config ---'
rg -n --hidden -g '!**/vendor/**' -e 'sqlite|gorm.*sqlite|TestMain|InitDB|dsn|database/sql' . | sed -n '1,240p'Repository: QuantumNous/new-api
Length of output: 6980
🌐 Web query:
SQLite FOR UPDATE support syntax error row-level locking documentation
💡 Result:
SQLite does not support the SELECT FOR UPDATE syntax [1][2]. Attempting to use this syntax in an SQLite database will result in a syntax error [1]. SQLite does not support row-level locking [3]. Instead, SQLite uses database-level locking [4][3], or in WAL (Write-Ahead Logging) mode, it allows for concurrent readers and a single writer, but still lacks the fine-grained row-level locking mechanisms found in client-server database systems [3][5]. To manage transaction behavior and concurrency in SQLite, consider the following approaches: 1. Transaction Modes: Use explicit transaction types to control locking behavior. - BEGIN IMMEDIATE: Starts a write transaction immediately, preventing other connections from starting a write transaction [6][7]. This can help avoid SQLITE_BUSY errors that might occur if a transaction is upgraded from a read to a write later [6][7]. - BEGIN EXCLUSIVE: Similar to IMMEDIATE but also prevents other connections from reading the database [7]. 2. WAL Mode: Enable Write-Ahead Logging to improve concurrency, allowing multiple readers to operate simultaneously with a writer, without blocking each other [5]. 3. Application-Level Logic: Since SQLite handles locking at the database level [3], race conditions must be managed by the application, often by ensuring write transactions are started early or by handling busy/retry logic via SQLite's busy_handler [3][8]. For authoritative details on SQLite's concurrency and locking, see the official documentation on File Locking and Concurrency [4] and Transaction behavior [6][7].
Citations:
- 1: https://www.cockroachlabs.com/blog/select-for-update/
- 2: https://groups.google.com/g/sqlalchemy/c/RIBdLP_s6hk
- 3: https://system.data.sqlite.org/home/doc/fd6a7e09b8/Doc/Extra/limitations.html
- 4: https://www.sqlite.org/lockingv3.html
- 5: https://sqlite.org/isolation.html
- 6: https://www.sqlite.org/lang%5Ftransaction.html
- 7: https://www.sqlite.org/lang_transaction.html
- 8: https://sqlite.org/src/doc/begin-concurrent/doc/begin_concurrent.md
Gate the FOR UPDATE lock by dialect. SQLite rejects SELECT ... FOR UPDATE, and model.InitDB() defaults to SQLite in local setups, so this path will fail there. Apply the same fallback to the second reset query below.
🤖 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 `@model/subscription.go` around lines 1030 - 1033, Gate the locking behavior in
the subscription reset query so `FOR UPDATE` is only applied on dialects that
support it. Update the query built in `subscription.go` around the
`tx.Set("gorm:query_option", "FOR UPDATE")` usage to conditionally add the lock
based on the current DB dialect, and apply the same dialect-based fallback to
the second reset query in the same flow so SQLite/local `model.InitDB()` setups
do not execute `SELECT ... FOR UPDATE`.
Source: Coding guidelines
| const handleConfirm = async () => { | ||
| if (!plan?.id) return | ||
| setResetting(true) | ||
| try { | ||
| const res = await resetPlanSubscriptions(plan.id, { | ||
| advance_reset_time: advanceResetTime, | ||
| }) | ||
| if (res.success) { | ||
| toast.success( | ||
| t('Reset {{count}} active subscriptions', { | ||
| count: res.data?.reset_count || 0, | ||
| }) | ||
| ) | ||
| triggerRefresh() | ||
| setOpen(null) | ||
| } | ||
| } catch { | ||
| toast.error(t('Operation failed')) | ||
| } finally { | ||
| setResetting(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle res.success === false case.
If resetPlanSubscriptions resolves with success: false (rather than throwing), no error is surfaced to the user — the dialog just does nothing since only the catch block calls toast.error.
🐛 Proposed fix
if (res.success) {
toast.success(
t('Reset {{count}} active subscriptions', {
count: res.data?.reset_count || 0,
})
)
triggerRefresh()
setOpen(null)
+ } else {
+ toast.error(res.message || t('Operation failed'))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleConfirm = async () => { | |
| if (!plan?.id) return | |
| setResetting(true) | |
| try { | |
| const res = await resetPlanSubscriptions(plan.id, { | |
| advance_reset_time: advanceResetTime, | |
| }) | |
| if (res.success) { | |
| toast.success( | |
| t('Reset {{count}} active subscriptions', { | |
| count: res.data?.reset_count || 0, | |
| }) | |
| ) | |
| triggerRefresh() | |
| setOpen(null) | |
| } | |
| } catch { | |
| toast.error(t('Operation failed')) | |
| } finally { | |
| setResetting(false) | |
| } | |
| } | |
| const handleConfirm = async () => { | |
| if (!plan?.id) return | |
| setResetting(true) | |
| try { | |
| const res = await resetPlanSubscriptions(plan.id, { | |
| advance_reset_time: advanceResetTime, | |
| }) | |
| if (res.success) { | |
| toast.success( | |
| t('Reset {{count}} active subscriptions', { | |
| count: res.data?.reset_count || 0, | |
| }) | |
| ) | |
| triggerRefresh() | |
| setOpen(null) | |
| } else { | |
| toast.error(res.message || t('Operation failed')) | |
| } | |
| } catch { | |
| toast.error(t('Operation failed')) | |
| } finally { | |
| setResetting(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/subscriptions/components/dialogs/reset-subscriptions-dialog.tsx`
around lines 42 - 63, The reset flow in handleConfirm only shows an error in the
catch block, so a resolved response from resetPlanSubscriptions with success
false is ignored and the dialog appears to do nothing. Update handleConfirm in
reset-subscriptions-dialog.tsx to explicitly handle the non-throwing failure
path by checking res.success and showing a toast.error when it is false, while
keeping the existing success path (toast.success, triggerRefresh, setOpen(null))
unchanged.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx (2)
208-232: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
handleServerErrorfor server error handling instead of a generic catch.
handleResetConfirmswallows all failures into a singlet('Operation failed')toast, losing any server-provided validation detail (e.g. why the reset was rejected).♻️ Proposed fix
} catch { - toast.error(t('Operation failed')) + handleServerError(err) } finally {As per coding guidelines, "Handle server errors with
handleServerError, surface user-facing errors throughtoast.error... and map form/server validation errors to fields."🤖 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/subscriptions/components/dialogs/user-subscriptions-dialog.tsx` around lines 208 - 232, The handleResetConfirm flow is catching every failure as a generic toast, which hides server-provided validation details. Update handleResetConfirm to use handleServerError for the resetUserSubscriptionsByPlan request failure path, and surface the returned user-facing message through toast.error instead of a fixed Operation failed string. Keep the success path and state cleanup intact, and reference the existing resetUserSubscriptionsByPlan and loadData flow when wiring the error handling.Source: Coding guidelines
110-467: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffComponent file exceeds the 200-line guideline.
The component now spans 400+ lines with three inline dialogs, a large
StaticDataTablecolumn config, and multiple handlers. Consider extracting the reset-confirmation dialog (443-464) and/or the row-action menu (363-410) into dedicated subcomponents, mirroring how the plan-wide reset flow already lives in its ownreset-subscriptions-dialog.tsx.As per coding guidelines, "Keep files reasonably small; when a single file grows beyond about 200 lines, consider extracting subcomponents or custom hooks."
🤖 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/subscriptions/components/dialogs/user-subscriptions-dialog.tsx` around lines 110 - 467, The UserSubscriptionsDialog component is too large and should be split into smaller pieces. Extract the reset-confirmation dialog and/or the per-row action menu from UserSubscriptionsDialog into dedicated subcomponents, and consider moving the table column/action logic into a helper or hook so the main component stays under the file size guideline. Use the existing UserSubscriptionsDialog, handleResetConfirm, handleConfirmAction, and the row actions rendered inside StaticDataTable as the main extraction points.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/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx`:
- Around line 208-232: The handleResetConfirm flow is catching every failure as
a generic toast, which hides server-provided validation details. Update
handleResetConfirm to use handleServerError for the resetUserSubscriptionsByPlan
request failure path, and surface the returned user-facing message through
toast.error instead of a fixed Operation failed string. Keep the success path
and state cleanup intact, and reference the existing
resetUserSubscriptionsByPlan and loadData flow when wiring the error handling.
- Around line 110-467: The UserSubscriptionsDialog component is too large and
should be split into smaller pieces. Extract the reset-confirmation dialog
and/or the per-row action menu from UserSubscriptionsDialog into dedicated
subcomponents, and consider moving the table column/action logic into a helper
or hook so the main component stays under the file size guideline. Use the
existing UserSubscriptionsDialog, handleResetConfirm, handleConfirmAction, and
the row actions rendered inside StaticDataTable as the main extraction points.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf316e5c-42e8-4e35-92b7-2f0d9edfc60e
📒 Files selected for processing (1)
web/default/src/features/subscriptions/components/dialogs/user-subscriptions-dialog.tsx
* feat(subscription): add admin quota reset actions * fix(subscription): keep quota reset in plan row actions * refactor(subscription): move user subscription actions into menu
SECURITY: pulls the critical quota-overflow billing fix (rc.18/rc.19). Root cause: user-controlled billing multipliers (image `n`, video/task `seconds`, NaN/Inf ratios) were unbounded and quota was cast float64->int with a raw conversion. When baseQuota * ratios * count/duration exceeded int32, the conversion wrapped to a NEGATIVE value, so consumption CREDITED the account instead of debiting it -> abnormal balance growth for any user who could call the relay API (unrelated to the payment/top-up gateway). Fix set adopted: common.QuotaFromFloat saturating conversion (clamp int32, NaN->0), MaxImageN=128, MaxTaskDurationSeconds=3600, AddOtherRatio NaN/Inf guard, and saturating conversions across all billing paths (relay_task, text_quota, task_billing, token_counter, tool_billing, kling, price, billingexpr). Plus admin quota-reset disposal tooling (QuantumNous#5952) and quota saturation audit logging. Conflict resolutions (fork features preserved per abandon-or-adapt policy): - Multi-site user isolation kept; upstream's global email-normalization + advisory-lock refactor (NormalizeEmail/EnsureEmailAvailable/BindEmailToUser/ prepareForInsert/GetUniqueUserByEmail) dropped for model/user.go, controller/user.go, oauth.go, misc.go (architecturally incompatible with per-site email uniqueness; not the security fix). - Redemption: combined upstream status filter + hardened atomic Redeem (row-lock + conditional update) with fork's site-scoped SearchRedemptions (keyword, status, startIdx, num, siteScope) and RedeemForSite. - SSRF: kept fork ValidateRelayTargetURL + upstream GetSSRFProtectedHTTPClient. - Grafted upstream password hardening (errUserPasswordUnset / errOriginalPasswordFail in checkUpdatePassword) onto fork user.go; passwordless users still set passwords via the site-scoped email reset flow. - audit templates merged; web deps took upstream bumps (ai 6->7 etc.) + kept fork altcha; bun.lock regenerated. Verified: go build ./... clean; common/relay/service/model/controller tests pass; web/default build passes; SQLite boot + migrations clean.
* feat(subscription): add admin quota reset actions * fix(subscription): keep quota reset in plan row actions * refactor(subscription): move user subscription actions into menu
* feat(subscription): add admin quota reset actions * fix(subscription): keep quota reset in plan row actions * refactor(subscription): move user subscription actions into menu
* feat(subscription): add admin quota reset actions * fix(subscription): keep quota reset in plan row actions * refactor(subscription): move user subscription actions into menu
* feat(subscription): add admin quota reset actions * fix(subscription): keep quota reset in plan row actions * refactor(subscription): move user subscription actions into menu
* feat(subscription): add admin quota reset actions * fix(subscription): keep quota reset in plan row actions * refactor(subscription): move user subscription actions into menu
* feat(subscription): add admin quota reset actions * fix(subscription): keep quota reset in plan row actions * refactor(subscription): move user subscription actions into menu
* feat(subscription): add admin quota reset actions * fix(subscription): keep quota reset in plan row actions * refactor(subscription): move user subscription actions into menu
2 participants
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
针对订阅，增加对用户的订阅重置和针对套餐的全量用户重置操作。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit