feat(wallet): add affiliate withdrawal feature - #5856
Open
Digital631 wants to merge 1 commit into
Open
Conversation
- add withdrawal UI in affiliate-rewards-card - add AffiliateWithdrawal API endpoints and routes - add withdrawal dialog integration - add AffiliateWithdrawal model and registration - add system settings for affiliate module - add i18n support (en, fr, ja) - update Go module dependencies (go.mod / go.sum)
Contributor
WalkthroughThis PR introduces an affiliate rebate and withdrawal system: new AffiliateUserRule, AffiliateRebate, and AffiliateWithdrawal models with rebate creation/release logic triggered by top-ups and redemptions, admin/global affiliate settings, per-user rule overrides, withdrawal API endpoints, and corresponding frontend settings, wallet UI, and translations across six locales. ChangesAffiliate Rebate & Withdrawal Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested labels: 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
web/default/src/features/wallet/components/dialogs/affiliate-withdrawal-dialog.tsx (1)
119-133: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueVerify amount/quota units vs
min/step.
min={1}whilestep={QUOTA_PER_DOLLAR}— ifQUOTA_PER_DOLLARrepresents a much larger unit (e.g. 500000 per dollar), the nativeminconstraint effectively allows sub-cent amounts inconsistent with the step granularity. This doesn't break the custominvalidcheck (which correctly gates onamount <= 0/> availableQuota), so it's cosmetic only.🤖 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/wallet/components/dialogs/affiliate-withdrawal-dialog.tsx` around lines 119 - 133, The withdrawal amount input in affiliate-withdrawal-dialog.tsx has a unit mismatch between the native min constraint and the QUOTA_PER_DOLLAR step granularity. Update the Input in the affiliate-withdrawal-dialog component so its min value matches the smallest valid quota increment (or remove the misleading native min if the custom validation already handles bounds), keeping it consistent with QUOTA_PER_DOLLAR, amount, and availableQuota.controller/user.go (2)
457-483: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider bounding
PaymentMethod/Account/Remarklength before persisting.Only presence is validated (
req.Amount <= 0 || req.PaymentMethod == "" || req.Account == ""); there's no upper-bound length check on these client-supplied strings before they reachmodel.CreateAffiliateWithdrawal. If the underlying columns have size constraints, an oversized value could fail the insert with an opaque DB error instead of a clean validation message.🛡️ Proposed length guard
req.PaymentMethod = strings.TrimSpace(req.PaymentMethod) req.Account = strings.TrimSpace(req.Account) req.Remark = strings.TrimSpace(req.Remark) - if req.Amount <= 0 || req.PaymentMethod == "" || req.Account == "" { + if req.Amount <= 0 || req.PaymentMethod == "" || req.Account == "" || + len(req.PaymentMethod) > 64 || len(req.Account) > 128 || len(req.Remark) > 500 { common.ApiErrorI18n(c, i18n.MsgInvalidParams) return }🤖 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 `@controller/user.go` around lines 457 - 483, Add upper-bound validation in CreateAffiliateWithdrawal for the client-supplied fields before calling model.CreateAffiliateWithdrawal: after trimming PaymentMethod, Account, and Remark, reject values that exceed the database-safe lengths with ApiErrorI18n and MsgInvalidParams. Keep the existing presence checks, and update the validation near the AffiliateWithdrawalRequest handling so oversized inputs are caught before persistence.
514-532: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider recording an audit trail for
ProcessAffiliateWithdrawal.This admin action mutates real quota balances (refunding on rejection) but, unlike
UpdateUser'srecordManageAuditFor(c, updatedUser.Id, "user.update", ...), it records no audit entry here. Given the compliance emphasis of this feature (payment-compliance gating elsewhere in the same file), logging who approved/rejected which withdrawal and why would aid dispute resolution and compliance review.🤖 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 `@controller/user.go` around lines 514 - 532, ProcessAffiliateWithdrawal updates withdrawal status and can affect real balances, but it currently does not create an audit record like UpdateUser does with recordManageAuditFor. Add an audit trail in ProcessAffiliateWithdrawal after a successful model.UpdateAffiliateWithdrawalStatus call, using the admin identity from c.GetInt("id") and the withdrawal/context details from req and id so approvals/rejections and remarks are recorded for compliance review. Keep the existing success/error handling intact and place the audit logging alongside the status update flow so it runs only when the mutation succeeds.
🤖 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/option.go`:
- Around line 151-160: The `affiliate_setting.reward_percent` validation in
`controller/option.go` accepts `NaN` because `strconv.ParseFloat` succeeds and
the existing range checks in the `case "affiliate_setting.reward_percent"` block
do not reject non-finite values. Update this branch to explicitly reject `NaN`
(and any other non-finite numeric input) before the 0–100 range check, while
keeping the existing compliance check and error handling intact. Use the
`option.Value` parsing path and the `common.ApiErrorMsg` / `common.ApiErrorI18n`
responses so invalid values are blocked consistently.
In `@controller/topup.go`:
- Around line 405-409: The top-up flow in IncreaseUserQuota currently applies
the user quota and then calls CreateAffiliateRebateForTopUp after the Epay
success response path, which can leave accounting partially applied if the
rebate fails. Move the topUp status update, quota credit, and affiliate rebate
creation into a single model transaction before returning success, or otherwise
enqueue a retryable affiliate-rebate job so the webhook is not acknowledged
until both the top-up and rebate steps are safely persisted.
In `@controller/user.go`:
- Around line 485-495: GetAffiliateWithdrawals is exposing admin-only fields
because it returns model.AffiliateWithdrawal objects directly. Update this
handler to map the results from model.GetUserAffiliateWithdrawals into a
user-facing DTO or filtered response before calling common.ApiSuccess, ensuring
admin_remark and processed_by are not serialized. Use GetAffiliateWithdrawals
and the model.AffiliateWithdrawal type as the key spots to adjust.
In `@main.go`:
- Around line 39-49: The embed directives in main.go are written as plain
comments instead of active directives, so the static assets will not be
included. Update the declarations for buildFS, indexPage, classicBuildFS, and
classicIndexPage to use the exact //go:embed syntax so the web entrypoint can
load its files correctly.
In `@model/affiliate.go`:
- Around line 336-343: The withdrawal logic in the affiliate debit flow is
non-atomic because the `AffQuota` check and the `Update` in the transaction can
race under concurrent withdrawals. Update the `affiliate` withdrawal path in
`model/affiliate.go` so the debit is done with a single conditional
`tx.Model(&User{})...Update(...)` guarded by `aff_quota >= amount`, then verify
`RowsAffected` and return `ErrAffiliateQuotaInsufficient` when no row was
updated. Keep the fix compatible with `tx`, `User`, and `gorm.Expr` so it works
across SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+.
- Around line 376-392: The withdrawal update in the pending-claim flow is
race-prone because `tx.Save(&withdrawal)` relies on an in-memory status check in
`AffiliateWithdrawal`, so two admins can both process the same record. In the
same transaction, change the `status` transition to an atomic conditional update
using `WHERE id = ? AND status = ?` before doing any refund work, and only
continue when that update affects one row. Keep the refund logic in the same
path around this transition so `AffiliateWithdrawalStatusPending`,
`AffiliateWithdrawalStatusRejected`, and the `tx`-based processing remain safe
across supported databases.
In `@model/user.go`:
- Around line 348-358: The invite reward update in inviteUser is vulnerable to
lost updates because it loads a User, mutates fields in memory, and calls
DB.Save on a stale struct. Update the inviter counters atomically using
gorm.Expr in the inviteUser path, and avoid relying on the in-memory User fields
for AffCount, AffQuota, and AffHistoryQuota so concurrent registrations cannot
overwrite each other.
In `@router/api-router.go`:
- Around line 96-97: The new POST /aff_withdrawal route is missing the same
abuse protection used by other financial mutation endpoints. Update the route
registration in api-router.go for CreateAffiliateWithdrawal so it is wrapped
with middleware.CriticalRateLimit(), matching the neighboring withdraw/pay/topup
handlers in the same route group. Keep the change local to the selfRoute setup
so the GetAffiliateWithdrawals route remains unchanged.
In `@web/default/src/features/users/components/users-mutate-drawer.tsx`:
- Around line 507-534: The `affiliate_rule.reward_percent` field in
`users-mutate-drawer.tsx` is storing `NaN` during partial decimal input, which
makes the controlled `Input` render blank while typing. Update the `FormField`
render handler so `field.onChange` never receives `NaN` from
`event.currentTarget.valueAsNumber`; preserve the typed value for incomplete
numeric states or normalize invalid values before storing them. Also adjust the
`value={field.value ?? 0}` logic in this input so it handles `NaN` explicitly,
not just `null`/`undefined`.
In `@web/default/src/i18n/locales/ja.json`:
- Around line 518-526: The withdrawal dialog is reusing a shared “Payment
Method” translation that is too top-up specific in Japanese. Update the locale
entries in ja.json by adding a withdrawal-specific label or changing the
existing key to a neutral term like 支払い方法, and make sure the withdrawal flow
uses the correct key so the label in the withdrawal UI is not rendered as
チャージ方法.
---
Nitpick comments:
In `@controller/user.go`:
- Around line 457-483: Add upper-bound validation in CreateAffiliateWithdrawal
for the client-supplied fields before calling model.CreateAffiliateWithdrawal:
after trimming PaymentMethod, Account, and Remark, reject values that exceed the
database-safe lengths with ApiErrorI18n and MsgInvalidParams. Keep the existing
presence checks, and update the validation near the AffiliateWithdrawalRequest
handling so oversized inputs are caught before persistence.
- Around line 514-532: ProcessAffiliateWithdrawal updates withdrawal status and
can affect real balances, but it currently does not create an audit record like
UpdateUser does with recordManageAuditFor. Add an audit trail in
ProcessAffiliateWithdrawal after a successful
model.UpdateAffiliateWithdrawalStatus call, using the admin identity from
c.GetInt("id") and the withdrawal/context details from req and id so
approvals/rejections and remarks are recorded for compliance review. Keep the
existing success/error handling intact and place the audit logging alongside the
status update flow so it runs only when the mutation succeeds.
In
`@web/default/src/features/wallet/components/dialogs/affiliate-withdrawal-dialog.tsx`:
- Around line 119-133: The withdrawal amount input in
affiliate-withdrawal-dialog.tsx has a unit mismatch between the native min
constraint and the QUOTA_PER_DOLLAR step granularity. Update the Input in the
affiliate-withdrawal-dialog component so its min value matches the smallest
valid quota increment (or remove the misleading native min if the custom
validation already handles bounds), keeping it consistent with QUOTA_PER_DOLLAR,
amount, and availableQuota.
🪄 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: 0046c723-2d85-4cb6-84a6-70a67c3b228b
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumweb/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
controller/option.gocontroller/topup.gocontroller/user.gomain.gomodel/affiliate.gomodel/affiliate_test.gomodel/main.gomodel/redemption.gomodel/task_cas_test.gomodel/topup.gomodel/user.gorouter/api-router.gosetting/operation_setting/affiliate_setting.goweb/default/src/features/system-settings/billing/index.tsxweb/default/src/features/system-settings/billing/section-registry.tsxweb/default/src/features/system-settings/general/quota-settings-section.tsxweb/default/src/features/system-settings/types.tsweb/default/src/features/users/components/users-mutate-drawer.tsxweb/default/src/features/users/lib/user-form.tsweb/default/src/features/users/types.tsweb/default/src/features/wallet/api.tsweb/default/src/features/wallet/components/affiliate-rewards-card.tsxweb/default/src/features/wallet/components/dialogs/affiliate-withdrawal-dialog.tsxweb/default/src/features/wallet/hooks/use-affiliate.tsweb/default/src/features/wallet/index.tsxweb/default/src/features/wallet/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
Comment on lines
+151
to
+160
| case "affiliate_setting.reward_percent": | ||
| percent, err := strconv.ParseFloat(strings.TrimSpace(option.Value.(string)), 64) | ||
| if err != nil || percent < 0 || percent > 100 { | ||
| common.ApiErrorMsg(c, "返利比例必须在 0 到 100 之间") | ||
| return | ||
| } | ||
| if percent > 0 && !operation_setting.IsPaymentComplianceConfirmed() { | ||
| common.ApiErrorI18n(c, i18n.MsgPaymentComplianceRequired) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
reward_percent validation lets "NaN" slip through.
strconv.ParseFloat parses "NaN" successfully (no error), and since NaN comparisons are always false, percent < 0 || percent > 100 never trips for it. An admin (or a raw API call bypassing the frontend Zod validation, which does reject NaN) could persist affiliate_setting.reward_percent = NaN, corrupting rebate percentage calculations that consume this setting downstream.
🐛 Proposed fix
case "affiliate_setting.reward_percent":
percent, err := strconv.ParseFloat(strings.TrimSpace(option.Value.(string)), 64)
- if err != nil || percent < 0 || percent > 100 {
+ if err != nil || math.IsNaN(percent) || percent < 0 || percent > 100 {
common.ApiErrorMsg(c, "返利比例必须在 0 到 100 之间")
return
}📝 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.
Suggested change
| case "affiliate_setting.reward_percent": | |
| percent, err := strconv.ParseFloat(strings.TrimSpace(option.Value.(string)), 64) | |
| if err != nil || percent < 0 || percent > 100 { | |
| common.ApiErrorMsg(c, "返利比例必须在 0 到 100 之间") | |
| return | |
| } | |
| if percent > 0 && !operation_setting.IsPaymentComplianceConfirmed() { | |
| common.ApiErrorI18n(c, i18n.MsgPaymentComplianceRequired) | |
| return | |
| } | |
| case "affiliate_setting.reward_percent": | |
| percent, err := strconv.ParseFloat(strings.TrimSpace(option.Value.(string)), 64) | |
| if err != nil || math.IsNaN(percent) || percent < 0 || percent > 100 { | |
| common.ApiErrorMsg(c, "返利比例必须在 0 到 100 之间") | |
| return | |
| } | |
| if percent > 0 && !operation_setting.IsPaymentComplianceConfirmed() { | |
| common.ApiErrorI18n(c, i18n.MsgPaymentComplianceRequired) | |
| return | |
| } |
🤖 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 `@controller/option.go` around lines 151 - 160, The
`affiliate_setting.reward_percent` validation in `controller/option.go` accepts
`NaN` because `strconv.ParseFloat` succeeds and the existing range checks in the
`case "affiliate_setting.reward_percent"` block do not reject non-finite values.
Update this branch to explicitly reject `NaN` (and any other non-finite numeric
input) before the 0–100 range check, while keeping the existing compliance check
and error handling intact. Use the `option.Value` parsing path and the
`common.ApiErrorMsg` / `common.ApiErrorI18n` responses so invalid values are
blocked consistently.
Comment on lines
+405
to
409
| if err == nil { | ||
| err = model.CreateAffiliateRebateForTopUp(topUp, quotaToAdd) | ||
| } | ||
| if err != nil { | ||
| logger.LogError(c.Request.Context(), fmt.Sprintf("易支付 更新用户额度失败 trade_no=%s user_id=%d client_ip=%s quota_to_add=%d error=%q topup=%q", topUp.TradeNo, topUp.UserId, c.ClientIP(), quotaToAdd, err.Error(), common.GetJsonString(topUp))) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Don’t let an affiliate-only failure leave Epay accounting partially applied.
IncreaseUserQuota commits before CreateAffiliateRebateForTopUp, and the webhook has already returned "success". If rebate creation fails, the user is credited but the inviter rebate is missing with no provider retry. Move top-up status, quota credit, and rebate creation into one model transaction before acknowledging, or persist a retryable affiliate-rebate job instead.
🤖 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 `@controller/topup.go` around lines 405 - 409, The top-up flow in
IncreaseUserQuota currently applies the user quota and then calls
CreateAffiliateRebateForTopUp after the Epay success response path, which can
leave accounting partially applied if the rebate fails. Move the topUp status
update, quota credit, and affiliate rebate creation into a single model
transaction before returning success, or otherwise enqueue a retryable
affiliate-rebate job so the webhook is not acknowledged until both the top-up
and rebate steps are safely persisted.
Comment on lines
+485
to
+495
| func GetAffiliateWithdrawals(c *gin.Context) { | ||
| pageInfo := common.GetPageQuery(c) | ||
| withdrawals, total, err := model.GetUserAffiliateWithdrawals(c.GetInt("id"), pageInfo) | ||
| if err != nil { | ||
| common.ApiError(c, err) | ||
| return | ||
| } | ||
| pageInfo.SetTotal(int(total)) | ||
| pageInfo.SetItems(withdrawals) | ||
| common.ApiSuccess(c, pageInfo) | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect AffiliateWithdrawal struct json tags for AdminRemark/ProcessedBy
rg -n -A20 'type AffiliateWithdrawal struct' model/affiliate.goRepository: QuantumNous/new-api
Length of output: 1073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== model.GetUserAffiliateWithdrawals =="
rg -n -A40 'func GetUserAffiliateWithdrawals' model/affiliate.go
echo
echo "== page info response shape =="
rg -n -A40 'type PageInfo struct|type Page' common model controller | head -n 120
echo
echo "== controller GetAffiliateWithdrawals =="
rg -n -A20 'func GetAffiliateWithdrawals' controller/user.goRepository: QuantumNous/new-api
Length of output: 4747
Hide admin-only withdrawal fields from this endpoint. GetAffiliateWithdrawals returns *model.AffiliateWithdrawal directly, so admin_remark and processed_by are serialized to the requester. Return a user-facing DTO or exclude those fields here.
🤖 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 `@controller/user.go` around lines 485 - 495, GetAffiliateWithdrawals is
exposing admin-only fields because it returns model.AffiliateWithdrawal objects
directly. Update this handler to map the results from
model.GetUserAffiliateWithdrawals into a user-facing DTO or filtered response
before calling common.ApiSuccess, ensuring admin_remark and processed_by are not
serialized. Use GetAffiliateWithdrawals and the model.AffiliateWithdrawal type
as the key spots to adjust.
Comment on lines
+39
to
49
| // go:embed web/default/dist | ||
| var buildFS embed.FS | ||
|
|
||
| //go:embed web/default/dist/index.html | ||
| // go:embed web/default/dist/index.html | ||
| var indexPage []byte | ||
|
|
||
| //go:embed web/classic/dist | ||
| // go:embed web/classic/dist | ||
| var classicBuildFS embed.FS | ||
|
|
||
| //go:embed web/classic/dist/index.html | ||
| // go:embed web/classic/dist/index.html | ||
| var classicIndexPage []byte |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the embed directives use the exact `//go:embed` form.
sed -n '39,49p' main.goRepository: QuantumNous/new-api
Length of output: 396
Restore the exact //go:embed syntax.
// go:embed is a plain comment, so these assets won’t be embedded and the web entrypoint will fail to load its static files.
🤖 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 `@main.go` around lines 39 - 49, The embed directives in main.go are written as
plain comments instead of active directives, so the static assets will not be
included. Update the declarations for buildFS, indexPage, classicBuildFS, and
classicIndexPage to use the exact //go:embed syntax so the web entrypoint can
load its files correctly.
Comment on lines
+336
to
+343
| if err := tx.Set("gorm:query_option", "FOR UPDATE").Where("id = ?", userId).First(&user).Error; err != nil { | ||
| return err | ||
| } | ||
| if user.AffQuota < amount { | ||
| return ErrAffiliateQuotaInsufficient | ||
| } | ||
| if err := tx.Model(&User{}).Where("id = ?", userId).Update("aff_quota", gorm.Expr("aff_quota - ?", amount)).Error; err != nil { | ||
| return err |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make the withdrawal debit atomic.
The balance check and debit are split, so concurrent withdrawals can both pass the AffQuota check and overdraw the affiliate balance. Use a conditional update and check RowsAffected.
Proposed fix
- var user User
- if err := tx.Set("gorm:query_option", "FOR UPDATE").Where("id = ?", userId).First(&user).Error; err != nil {
- return err
- }
- if user.AffQuota < amount {
- return ErrAffiliateQuotaInsufficient
- }
- if err := tx.Model(&User{}).Where("id = ?", userId).Update("aff_quota", gorm.Expr("aff_quota - ?", amount)).Error; err != nil {
- return err
- }
+ res := tx.Model(&User{}).
+ Where("id = ? AND aff_quota >= ?", userId, amount).
+ Update("aff_quota", gorm.Expr("aff_quota - ?", amount))
+ if res.Error != nil {
+ return res.Error
+ }
+ if res.RowsAffected == 0 {
+ return ErrAffiliateQuotaInsufficient
+ }
return tx.Create(withdrawal).ErrorAs per coding guidelines, all database code must support SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6 simultaneously.
📝 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.
Suggested change
| if err := tx.Set("gorm:query_option", "FOR UPDATE").Where("id = ?", userId).First(&user).Error; err != nil { | |
| return err | |
| } | |
| if user.AffQuota < amount { | |
| return ErrAffiliateQuotaInsufficient | |
| } | |
| if err := tx.Model(&User{}).Where("id = ?", userId).Update("aff_quota", gorm.Expr("aff_quota - ?", amount)).Error; err != nil { | |
| return err | |
| res := tx.Model(&User{}). | |
| Where("id = ? AND aff_quota >= ?", userId, amount). | |
| Update("aff_quota", gorm.Expr("aff_quota - ?", amount)) | |
| if res.Error != nil { | |
| return res.Error | |
| } | |
| if res.RowsAffected == 0 { | |
| return ErrAffiliateQuotaInsufficient | |
| } | |
| return tx.Create(withdrawal).Error |
🤖 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/affiliate.go` around lines 336 - 343, The withdrawal logic in the
affiliate debit flow is non-atomic because the `AffQuota` check and the `Update`
in the transaction can race under concurrent withdrawals. Update the `affiliate`
withdrawal path in `model/affiliate.go` so the debit is done with a single
conditional `tx.Model(&User{})...Update(...)` guarded by `aff_quota >= amount`,
then verify `RowsAffected` and return `ErrAffiliateQuotaInsufficient` when no
row was updated. Keep the fix compatible with `tx`, `User`, and `gorm.Expr` so
it works across SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+.
Source: Coding guidelines
Comment on lines
+376
to
+392
| var withdrawal AffiliateWithdrawal | ||
| if err := tx.Set("gorm:query_option", "FOR UPDATE").Where("id = ?", id).First(&withdrawal).Error; err != nil { | ||
| return err | ||
| } | ||
| if withdrawal.Status != AffiliateWithdrawalStatusPending { | ||
| return ErrAffiliateWithdrawalInvalid | ||
| } | ||
| if status == AffiliateWithdrawalStatusRejected { | ||
| if err := tx.Model(&User{}).Where("id = ?", withdrawal.UserId).Update("aff_quota", gorm.Expr("aff_quota + ?", withdrawal.Amount)).Error; err != nil { | ||
| return err | ||
| } | ||
| } | ||
| withdrawal.Status = status | ||
| withdrawal.AdminRemark = adminRemark | ||
| withdrawal.ProcessedAt = common.GetTimestamp() | ||
| withdrawal.ProcessedBy = operatorId | ||
| return tx.Save(&withdrawal).Error |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Claim the pending withdrawal with a conditional status transition.
Two admins can process the same pending withdrawal concurrently and both pass the in-memory status check, which can double-refund on rejection. Update status with WHERE id = ? AND status = 'pending' first, then refund only when that transition succeeds.
Proposed fix
var withdrawal AffiliateWithdrawal
- if err := tx.Set("gorm:query_option", "FOR UPDATE").Where("id = ?", id).First(&withdrawal).Error; err != nil {
+ if err := tx.Select("id", "user_id", "amount").Where("id = ?", id).First(&withdrawal).Error; err != nil {
return err
}
- if withdrawal.Status != AffiliateWithdrawalStatusPending {
+
+ res := tx.Model(&AffiliateWithdrawal{}).
+ Where("id = ? AND status = ?", id, AffiliateWithdrawalStatusPending).
+ Updates(map[string]interface{}{
+ "status": status,
+ "admin_remark": adminRemark,
+ "processed_at": common.GetTimestamp(),
+ "processed_by": operatorId,
+ })
+ if res.Error != nil {
+ return res.Error
+ }
+ if res.RowsAffected == 0 {
return ErrAffiliateWithdrawalInvalid
}
if status == AffiliateWithdrawalStatusRejected {
if err := tx.Model(&User{}).Where("id = ?", withdrawal.UserId).Update("aff_quota", gorm.Expr("aff_quota + ?", withdrawal.Amount)).Error; err != nil {
return err
}
}
- withdrawal.Status = status
- withdrawal.AdminRemark = adminRemark
- withdrawal.ProcessedAt = common.GetTimestamp()
- withdrawal.ProcessedBy = operatorId
- return tx.Save(&withdrawal).Error
+ return nilAs per coding guidelines, all database code must support SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6 simultaneously.
📝 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.
Suggested change
| var withdrawal AffiliateWithdrawal | |
| if err := tx.Set("gorm:query_option", "FOR UPDATE").Where("id = ?", id).First(&withdrawal).Error; err != nil { | |
| return err | |
| } | |
| if withdrawal.Status != AffiliateWithdrawalStatusPending { | |
| return ErrAffiliateWithdrawalInvalid | |
| } | |
| if status == AffiliateWithdrawalStatusRejected { | |
| if err := tx.Model(&User{}).Where("id = ?", withdrawal.UserId).Update("aff_quota", gorm.Expr("aff_quota + ?", withdrawal.Amount)).Error; err != nil { | |
| return err | |
| } | |
| } | |
| withdrawal.Status = status | |
| withdrawal.AdminRemark = adminRemark | |
| withdrawal.ProcessedAt = common.GetTimestamp() | |
| withdrawal.ProcessedBy = operatorId | |
| return tx.Save(&withdrawal).Error | |
| var withdrawal AffiliateWithdrawal | |
| if err := tx.Select("id", "user_id", "amount").Where("id = ?", id).First(&withdrawal).Error; err != nil { | |
| return err | |
| } | |
| res := tx.Model(&AffiliateWithdrawal{}). | |
| Where("id = ? AND status = ?", id, AffiliateWithdrawalStatusPending). | |
| Updates(map[string]interface{}{ | |
| "status": status, | |
| "admin_remark": adminRemark, | |
| "processed_at": common.GetTimestamp(), | |
| "processed_by": operatorId, | |
| }) | |
| if res.Error != nil { | |
| return res.Error | |
| } | |
| if res.RowsAffected == 0 { | |
| return ErrAffiliateWithdrawalInvalid | |
| } | |
| if status == AffiliateWithdrawalStatusRejected { | |
| if err := tx.Model(&User{}).Where("id = ?", withdrawal.UserId).Update("aff_quota", gorm.Expr("aff_quota + ?", withdrawal.Amount)).Error; err != nil { | |
| return err | |
| } | |
| } | |
| return nil |
🤖 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/affiliate.go` around lines 376 - 392, The withdrawal update in the
pending-claim flow is race-prone because `tx.Save(&withdrawal)` relies on an
in-memory status check in `AffiliateWithdrawal`, so two admins can both process
the same record. In the same transaction, change the `status` transition to an
atomic conditional update using `WHERE id = ? AND status = ?` before doing any
refund work, and only continue when that update affects one row. Keep the refund
logic in the same path around this transition so
`AffiliateWithdrawalStatusPending`, `AffiliateWithdrawalStatusRejected`, and the
`tx`-based processing remain safe across supported databases.
Source: Coding guidelines
Comment on lines
+348
to
358
| func inviteUser(inviterId int, rewardQuota int) (err error) { | ||
| user, err := GetUserById(inviterId, true) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| user.AffCount++ | ||
| user.AffQuota += common.QuotaForInviter | ||
| user.AffHistoryQuota += common.QuotaForInviter | ||
| if rewardQuota > 0 { | ||
| user.AffQuota += rewardQuota | ||
| user.AffHistoryQuota += rewardQuota | ||
| } | ||
| return DB.Save(user).Error |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use atomic increments for invite rewards.
Concurrent registrations for the same inviter can read the same row, increment in memory, and Save one update over the other. Update counters with gorm.Expr instead of saving a stale User struct.
Proposed fix
func inviteUser(inviterId int, rewardQuota int) (err error) {
- user, err := GetUserById(inviterId, true)
- if err != nil {
- return err
+ updates := map[string]interface{}{
+ "aff_count": gorm.Expr("aff_count + ?", 1),
}
- user.AffCount++
if rewardQuota > 0 {
- user.AffQuota += rewardQuota
- user.AffHistoryQuota += rewardQuota
+ updates["aff_quota"] = gorm.Expr("aff_quota + ?", rewardQuota)
+ updates["aff_history"] = gorm.Expr("aff_history + ?", rewardQuota)
}
- return DB.Save(user).Error
+ res := DB.Model(&User{}).Where("id = ?", inviterId).Updates(updates)
+ if res.Error != nil {
+ return res.Error
+ }
+ if res.RowsAffected == 0 {
+ return errors.New("inviter user not found")
+ }
+ return nil
}🤖 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/user.go` around lines 348 - 358, The invite reward update in inviteUser
is vulnerable to lost updates because it loads a User, mutates fields in memory,
and calls DB.Save on a stale struct. Update the inviter counters atomically
using gorm.Expr in the inviteUser path, and avoid relying on the in-memory User
fields for AffCount, AffQuota, and AffHistoryQuota so concurrent registrations
cannot overwrite each other.
Comment on lines
+96
to
+97
| selfRoute.GET("/aff_withdrawal/self", controller.GetAffiliateWithdrawals) | ||
| selfRoute.POST("/aff_withdrawal", controller.CreateAffiliateWithdrawal) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
POST /aff_withdrawal lacks rate limiting, unlike sibling financial write routes.
Every other user-initiated financial mutation on this route group (/topup, /pay, /stripe/pay, /creem/pay, /waffo/pay, etc.) is guarded with middleware.CriticalRateLimit(), but the new CreateAffiliateWithdrawal route is not. While the model layer serializes concurrent withdrawals via row locking, an unthrottled endpoint still allows abusive request volume (e.g. hammering the DB with repeated transactions).
🔒️ Proposed fix
- selfRoute.GET("/aff_withdrawal/self", controller.GetAffiliateWithdrawals)
- selfRoute.POST("/aff_withdrawal", controller.CreateAffiliateWithdrawal)
+ selfRoute.GET("/aff_withdrawal/self", controller.GetAffiliateWithdrawals)
+ selfRoute.POST("/aff_withdrawal", middleware.CriticalRateLimit(), controller.CreateAffiliateWithdrawal)📝 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.
Suggested change
| selfRoute.GET("/aff_withdrawal/self", controller.GetAffiliateWithdrawals) | |
| selfRoute.POST("/aff_withdrawal", controller.CreateAffiliateWithdrawal) | |
| selfRoute.GET("/aff_withdrawal/self", controller.GetAffiliateWithdrawals) | |
| selfRoute.POST("/aff_withdrawal", middleware.CriticalRateLimit(), controller.CreateAffiliateWithdrawal) |
🤖 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 `@router/api-router.go` around lines 96 - 97, The new POST /aff_withdrawal
route is missing the same abuse protection used by other financial mutation
endpoints. Update the route registration in api-router.go for
CreateAffiliateWithdrawal so it is wrapped with middleware.CriticalRateLimit(),
matching the neighboring withdraw/pay/topup handlers in the same route group.
Keep the change local to the selfRoute setup so the GetAffiliateWithdrawals
route remains unchanged.
Comment on lines
+507
to
+534
| <FormField | ||
| control={form.control} | ||
| name='affiliate_rule.reward_percent' | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel> | ||
| {t('Referral Rebate Percent')} | ||
| </FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| type='number' | ||
| min={0} | ||
| max={100} | ||
| step='0.01' | ||
| value={field.value ?? 0} | ||
| onChange={(event) => | ||
| field.onChange( | ||
| event.target.value === '' | ||
| ? 0 | ||
| : event.currentTarget.valueAsNumber | ||
| ) | ||
| } | ||
| /> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reward-percent input can render blank/NaN mid-typing.
onChange only special-cases an empty string; when the native input has an incomplete/invalid numeric string (e.g. typing "12.", ".", "-"), valueAsNumber is NaN. That NaN is then stored via field.onChange(NaN), and on re-render value={field.value ?? 0} doesn't catch it since ?? only substitutes for null/undefined, not NaN — so the controlled input receives an invalid value prop, effectively blanking the field while the admin is typing a decimal value.
🐛 Proposed fix
<Input
type='number'
min={0}
max={100}
step='0.01'
value={field.value ?? 0}
onChange={(event) =>
field.onChange(
- event.target.value === ''
- ? 0
- : event.currentTarget.valueAsNumber
+ event.target.value === '' ||
+ Number.isNaN(event.currentTarget.valueAsNumber)
+ ? 0
+ : event.currentTarget.valueAsNumber
)
}
/>📝 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.
Suggested change
| <FormField | |
| control={form.control} | |
| name='affiliate_rule.reward_percent' | |
| render={({ field }) => ( | |
| <FormItem> | |
| <FormLabel> | |
| {t('Referral Rebate Percent')} | |
| </FormLabel> | |
| <FormControl> | |
| <Input | |
| type='number' | |
| min={0} | |
| max={100} | |
| step='0.01' | |
| value={field.value ?? 0} | |
| onChange={(event) => | |
| field.onChange( | |
| event.target.value === '' | |
| ? 0 | |
| : event.currentTarget.valueAsNumber | |
| ) | |
| } | |
| /> | |
| </FormControl> | |
| <FormMessage /> | |
| </FormItem> | |
| )} | |
| /> | |
| <FormField | |
| control={form.control} | |
| name='affiliate_rule.reward_percent' | |
| render={({ field }) => ( | |
| <FormItem> | |
| <FormLabel> | |
| {t('Referral Rebate Percent')} | |
| </FormLabel> | |
| <FormControl> | |
| <Input | |
| type='number' | |
| min={0} | |
| max={100} | |
| step='0.01' | |
| value={field.value ?? 0} | |
| onChange={(event) => | |
| field.onChange( | |
| event.target.value === '' || | |
| Number.isNaN(event.currentTarget.valueAsNumber) | |
| ? 0 | |
| : event.currentTarget.valueAsNumber | |
| ) | |
| } | |
| /> | |
| </FormControl> | |
| <FormMessage /> | |
| </FormItem> | |
| )} | |
| /> |
🤖 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 507 - 534, The `affiliate_rule.reward_percent` field in
`users-mutate-drawer.tsx` is storing `NaN` during partial decimal input, which
makes the controlled `Input` render blank while typing. Update the `FormField`
render handler so `field.onChange` never receives `NaN` from
`event.currentTarget.valueAsNumber`; preserve the typed value for incomplete
numeric states or normalize invalid values before storing them. Also adjust the
`value={field.value ?? 0}` logic in this input so it handles `NaN` explicitly,
not just `null`/`undefined`.
Comment on lines
+518
to
+526
| "Withdraw": "出金", | ||
| "Withdraw Referral Rewards": "紹介報酬を出金", | ||
| "Withdrawal Amount": "出金額", | ||
| "Withdrawal request failed": "出金申請に失敗しました", | ||
| "Withdrawal request submitted": "出金申請を送信しました", | ||
| "Submit a withdrawal request for available referral rewards": "利用可能な紹介報酬の出金申請を送信します", | ||
| "Receiving Account": "受取口座", | ||
| "Bank transfer, PayPal, Alipay...": "銀行振込、PayPal、Alipay...", | ||
| "Account, email, or wallet address": "口座、メール、またはウォレットアドレス", |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify how many places reuse the "Payment Method" i18n key and whether
# a top-up-specific vs. withdrawal-specific label distinction already exists.
rg -n "t\('Payment Method'\)" web/default/src -C 3
rg -n '"Payment Method"' web/default/src/i18n/locales/ja.jsonRepository: QuantumNous/new-api
Length of output: 3788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether a withdrawal-specific payment-method label already exists
rg -n '"(Withdrawal|Withdraw).*Method"|Payment Method|Payout Method|Receiving Account|受取口座|チャージ方法|支払い方法' web/default/src/i18n/locales/ja.json web/default/src -g '!**/node_modules/**' -C 1Repository: QuantumNous/new-api
Length of output: 20283
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("web/default/src/i18n/locales/ja.json")
text = p.read_text(encoding="utf-8")
for key in ["Payment Method", "Withdrawal request failed", "Withdrawal request submitted", "Receiving Account"]:
idx = text.find(f'"{key}"')
if idx != -1:
start = max(0, idx - 120)
end = min(len(text), idx + 180)
print(f"\n--- {key} ---")
print(text[start:end])
PYRepository: QuantumNous/new-api
Length of output: 1485
Use a withdrawal-specific label for this field.
Payment Method is shared by recharge, billing, and withdrawal flows, but the Japanese translation is チャージ方法 (“top-up method”), which is wrong in the withdrawal dialog. Add a separate withdrawal key or switch this label to a neutral term like 支払い方法.
🤖 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/i18n/locales/ja.json` around lines 518 - 526, The withdrawal
dialog is reusing a shared “Payment Method” translation that is too top-up
specific in Japanese. Update the locale entries in ja.json by adding a
withdrawal-specific label or changing the existing key to a neutral term like
支払い方法, and make sure the withdrawal flow uses the correct key so the label in
the withdrawal UI is not rendered as チャージ方法.
JacksonsY
added a commit
to JacksonsY/new-api
that referenced
this pull request
Jul 4, 2026
上游本轮全部为前端与依赖更新：渠道测试弹窗交互/布局精简、模型测试行操作紧凑化、 ai-elements 嵌套 usage token 读取修复、web 依赖升级(移除 date-fns 等)。 无后端变更；PR QuantumNous#5856(充值返佣提现)仍未合入,无提现基建撞车。 与本地二开(jzlh)改动文件零交集,无冲突。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> # Conflicts: # web/bun.lock # web/default/package.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
This PR adds a configurable affiliate rebate module for referral rewards.
It introduces backend support for affiliate reward rules, withdrawal requests, and admin-controlled settings. Referral rewards can now be enabled globally, configured by percentage, optionally delayed until the invited user consumes the credited quota, and overridden per user. Redemption-code top-ups can also be included in affiliate rebates through a separate system setting.
On the frontend, the wallet referral card now shows available, pending, total earned rewards, and invite count, with withdrawal support when enabled. The affiliate withdrawal dialog and API integration are added, and all related UI labels are localized across the existing locale files.
Type of change
Related Issue
Checklist
Proof of Work
Summary by CodeRabbit
New Features
Bug Fixes