feat: add max top-up amount limit for Epay and Stripe#2933
feat: add max top-up amount limit for Epay and Stripe#2933worryzyy wants to merge 3 commits intoQuantumNous:mainfrom
Conversation
WalkthroughThis PR introduces a maximum top-up limit feature across the system. Backend controllers now support configurable max top-up amounts with localized validation messages. Frontend components receive and enforce these limits through UI validation and display. Configuration pages allow administrators to set maximum top-up values for standard and Stripe payments. The implementation includes new helper functions, validation logic, and extensive i18n translations across multiple locales. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
model/option.go (1)
343-356:⚠️ Potential issue | 🟡 MinorClamp negative MaxTopUp values to avoid blocking all top-ups.
Because0means “no limit,” a negative value (e.g., misconfigured-1) would effectively reject every top-up. Consider clamping to0(or rejecting) when parsing.✅ Proposed fix (clamp to 0)
case "MaxTopUp": - operation_setting.MaxTopUp, _ = strconv.Atoi(value) + maxVal, _ := strconv.Atoi(value) + if maxVal < 0 { + maxVal = 0 + } + operation_setting.MaxTopUp = maxVal @@ case "StripeMaxTopUp": - setting.StripeMaxTopUp, _ = strconv.Atoi(value) + stripeMax, _ := strconv.Atoi(value) + if stripeMax < 0 { + stripeMax = 0 + } + setting.StripeMaxTopUp = stripeMaxcontroller/topup.go (1)
55-65:⚠️ Potential issue | 🟠 MajorPotential mismatch between API response values and backend validation when
QuotaDisplayTypeis TOKENS.
max_topup(line 59) returns the rawoperation_setting.MaxTopUpvalue (in USD units), butgetMaxTopup()(used for server-side validation) converts it to token units when the display type is TOKENS. The frontend uses the raw API value for client-side validation against user input that's in display units, which would cause a mismatch when the display type is TOKENS.This appears to be a pre-existing pattern (same issue exists for
min_topupon line 58), so it's not a regression introduced by this PR, but worth noting for correctness.Consider returning the converted values in the API response:
Suggested fix
"min_topup": operation_setting.MinTopUp, - "max_topup": operation_setting.MaxTopUp, + "max_topup": getMaxTopup(), "stripe_min_topup": setting.StripeMinTopUp, - "stripe_max_topup": setting.StripeMaxTopUp, + "stripe_max_topup": getStripeMaxTopup(),
🤖 Fix all issues with AI agents
In `@setting/operation_setting/payment_setting_old.go`:
- Line 18: Remove the stray global variable MaxTopUp from payment_setting_old.go
and add it as a field on the PaymentSetting struct in payment_setting.go (e.g.,
MaxTopUp int with same semantics: 0 means no limit); update any constructors or
JSON/YAML tags and defaulting logic for PaymentSetting to initialize/serialize
this new field and update any code references to use paymentSetting.MaxTopUp
instead of the removed global variable.
In `@web/src/components/topup/index.jsx`:
- Around line 200-203: Add an upper-bound guard to prevent submissions when the
amount exceeds maxTopUp: in the block that currently checks topUpCount <
minTopUp (using variables topUpCount, minTopUp, maxTopUp and function
showError), also validate if topUpCount > maxTopUp, call showError with a
suitable message and return early to avoid opening the confirm modal or sending
requests; apply the same maxTopUp guard inside preTopUp so the pre-check and
confirm flow both reject values above maxTopUp.
In `@web/src/components/topup/RechargeCard.jsx`:
- Around line 347-354: The tooltip uses the raw maxTopUp while elsewhere (line
~292) the UI uses renderQuotaWithAmount(maxTopUp); update the Tooltip content to
use renderQuotaWithAmount(maxTopUp) instead of raw maxTopUp so the max limit is
consistently formatted; locate the Tooltip in RechargeCard.jsx and replace the
unformatted maxTopUp reference with a call to renderQuotaWithAmount(maxTopUp)
(keep the same i18n string concatenation logic).
In `@web/src/pages/Setting/Payment/SettingsPaymentGateway.jsx`:
- Around line 164-166: Before adding MaxTopUp to the options array, validate
that inputs.MaxTopUp is not less than inputs.MinTopUp; if it is, prevent
submission and surface a user-facing error (e.g., set a form validation error or
show a toast) so the invalid configuration cannot be saved. Update the logic
around the options.push for 'MaxTopUp' in SettingsPaymentGateway.jsx to check
inputs.MinTopUp and inputs.MaxTopUp, and only push the MaxTopUp option when
MaxTopUp >= MinTopUp; otherwise block the save flow and display the validation
message.
In `@web/src/pages/Setting/Payment/SettingsPaymentGatewayStripe.jsx`:
- Around line 127-135: Before pushing the StripeMaxTopUp option, validate that
inputs.StripeMaxTopUp is not a positive number smaller than
inputs.StripeMinTopUp; if inputs.StripeMaxTopUp > 0 && inputs.StripeMaxTopUp <
inputs.StripeMinTopUp, prevent the save (do not push into options) and surface a
clear validation error to the user using the component's existing error/display
mechanism (e.g., set form error or show toast). Update the block around
inputs.StripeMaxTopUp/options so the validation runs prior to adding the option
and returns early on failure with a user-facing message like "Max top-up must be
>= Min top-up."
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/Setting/Payment/SettingsPaymentGateway.jsx (1)
146-173:⚠️ Potential issue | 🟡 Minor
nullvalue from cleared InputNumber would crash on.toString().Semi UI's
InputNumbercan yieldnullwhen the user clears the field. The guard on line 171 checks!== ''and!== undefinedbut not!== null, sonull.toString()at line 172 would throw a runtime error.Proposed fix
- if (inputs.MaxTopUp !== '' && inputs.MaxTopUp !== undefined) { + if (inputs.MaxTopUp != null && inputs.MaxTopUp !== '') { options.push({ key: 'MaxTopUp', value: inputs.MaxTopUp.toString() }); }Same null guard should be applied to the validation block:
- if (inputs.MaxTopUp !== '' && inputs.MaxTopUp !== undefined) { + if (inputs.MaxTopUp != null && inputs.MaxTopUp !== '') { if (inputs.MaxTopUp > 0 && inputs.MaxTopUp < inputs.MinTopUp) {
🤖 Fix all issues with AI agents
In `@web/src/components/topup/index.jsx`:
- Around line 588-590: The check that returns early when value > maxTopUp leaves
the previous computed amount displayed; update the amount state to a cleared
value before returning so the UI doesn't show a stale payment amount (i.e., set
the component's amount/state to an empty/zero value whenever value > maxTopUp),
and apply the same change to the getStripeAmount function where it does the same
value > maxTopUp early return; ensure you reference and update the shared amount
state (the amount variable/state setter) in both places before the return.
🧹 Nitpick comments (3)
controller/topup.go (1)
151-158:getMinTopup()is called twice — consider caching in a local variable.Both
getMinTopup()(lines 151-152) andgetMaxTopup()(line 155) perform setting lookups and potential arithmetic. Storing the result avoids redundant computation and a theoretical TOCTOU inconsistency if the setting changes between calls.The same pattern appears in
RequestAmount(lines 343-348).♻️ Suggested refactor
func RequestEpay(c *gin.Context) { var req EpayRequest err := c.ShouldBindJSON(&req) if err != nil { c.JSON(200, gin.H{"message": "error", "data": "参数错误"}) return } - if req.Amount < getMinTopup() { - c.JSON(200, gin.H{"message": "error", "data": i18n.T(c, i18n.MsgTopupAmountTooLow, map[string]any{"Min": getMinTopup()})}) + minTopup := getMinTopup() + if req.Amount < minTopup { + c.JSON(200, gin.H{"message": "error", "data": i18n.T(c, i18n.MsgTopupAmountTooLow, map[string]any{"Min": minTopup})}) return }web/src/i18n/locales/ja.json (1)
438-442: New i18n keys break the file's existing sort order.The five new keys are grouped together near
"充值数量不能小于", but the rest of the file is sorted. For example,"0 表示不限制"should be near line 31–33 with other"0…"keys, and"最高充值数量"belongs near other"最…"entries (around line 512). Consider running the project's i18n sort tooling (bun run i18n:*) to re-sort the file and keep it consistent with existing conventions.web/src/components/topup/index.jsx (1)
204-211: Inconsistent formatting between min and max error messages.The min-topup error displays a raw number (
minTopUp), while the max-topup error usesrenderQuotaWithAmount(maxTopUp)which prepends a currency symbol (e.g.,$5). The same inconsistency exists inpreTopUp(lines 176 vs 180). Consider usingrenderQuotaWithAmountfor both or neither, so users see a uniform format.Proposed fix
if (topUpCount < minTopUp) { - showError(t('充值数量不能小于') + minTopUp); + showError(t('充值数量不能小于') + ' ' + renderQuotaWithAmount(minTopUp)); return; }Apply the same in
preTopUpat line 176.
| if (maxTopUp > 0 && value > maxTopUp) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Stale amount displayed when the entered value exceeds the max.
When value > maxTopUp, the function returns without updating amount. If the user previously entered a valid value (e.g., $50 → amount shows ¥365), then changes to an invalid value exceeding maxTopUp, the old payment amount remains displayed — potentially misleading.
Consider resetting the amount before returning:
Proposed fix
if (maxTopUp > 0 && value > maxTopUp) {
+ setAmount(0);
return;
}Apply the same in getStripeAmount (line 617–619).
📝 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.
| if (maxTopUp > 0 && value > maxTopUp) { | |
| return; | |
| } | |
| if (maxTopUp > 0 && value > maxTopUp) { | |
| setAmount(0); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In `@web/src/components/topup/index.jsx` around lines 588 - 590, The check that
returns early when value > maxTopUp leaves the previous computed amount
displayed; update the amount state to a cleared value before returning so the UI
doesn't show a stale payment amount (i.e., set the component's amount/state to
an empty/zero value whenever value > maxTopUp), and apply the same change to the
getStripeAmount function where it does the same value > maxTopUp early return;
ensure you reference and update the shared amount state (the amount
variable/state setter) in both places before the return.
1 participant
Summary
Summary by CodeRabbit
Release Notes
New Features
Internationalization