fix(billing): prevent 0 amount payments#1904
Conversation
WalkthroughUpdated test fixtures to use nested Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant W as Web Payment Page
participant A as API (ConfirmPayment)
participant S as Stripe
U->>W: Enter amount + payment method
W->>W: validateAmount(value, discounts, MINIMUM_PAYMENT_AMOUNT)
alt value <= 0
W-->>U: "Amount must be greater than $0"
else no discounts AND value < 20
W-->>U: "Minimum amount is $20"
else Valid
W->>A: POST /confirm-payment { amount, ... }
A->>A: Validate request against schema (amount >= 20)
alt amount < 20 (schema)
A-->>W: 400 Validation Error ("Amount must be greater or equal to $20")
else Proceed
A->>S: Create/confirm PaymentIntent
S-->>A: Client secret / success
A-->>W: Return client secret
W-->>U: Continue payment flow
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1904 +/- ##
==========================================
- Coverage 44.62% 44.59% -0.03%
==========================================
Files 979 979
Lines 27438 27454 +16
Branches 7099 7089 -10
==========================================
Hits 12243 12243
- Misses 14098 14846 +748
+ Partials 1097 365 -732
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
apps/api/src/billing/services/stripe-error/stripe-error.service.ts (2)
39-42: Add error-code mapping for the new message to avoid "unknown_payment_error".Right now, "Amount must be greater than $0" will surface with code "unknown_payment_error". Map it in getPaymentErrorCodeFromMessage so clients can branch reliably.
Apply this diff:
@@ private getPaymentErrorCodeFromMessage(message: string): string { const messageLower = message.toLowerCase(); if (messageLower.includes("insufficient funds")) { return "insufficient_funds"; @@ } else if (messageLower.includes("coupon id is required")) { return "coupon_id_required"; + } else if (messageLower.includes("must be greater than $0")) { + return "amount_must_be_positive"; } return "unknown_payment_error"; }
69-99: Consider stabilizing against strict message matching.The includes-based lookup on full English strings is brittle. Prefer internal codes (thrown or attached) and a single translation table.
If you want, I can sketch a minimal Error subclass + code plumbing to phase in without breaking existing messages.
Also applies to: 171-195
apps/deploy-web/src/pages/payment.tsx (2)
196-205: Handle NaN early to prevent false “valid” cases.parseFloat can yield NaN, which currently slips through and clears errors. Add a finite-number guard.
const validateAmount = (value: number) => { const finalAmount = getFinalAmount(value.toString()); + if (!Number.isFinite(value)) { + setAmountError("Enter a valid amount"); + return false; + } // Prevent $0 payments if (value <= 0) { setAmountError("Amount must be greater than $0"); return false; } // Only check for minimum amount if no coupon is applied if (!discounts.length && value < MINIMUM_PAYMENT_AMOUNT) { setAmountError(`Minimum amount is $${MINIMUM_PAYMENT_AMOUNT}`); return false; }
196-205: Optional: align UI/server wording for the min-amount error.Backend returns “Minimum payment amount is $20 (before any discounts)”; UI says “Minimum amount is $20”. Consider aligning to reduce confusion.
apps/api/src/billing/services/stripe-error/stripe-error.service.spec.ts (1)
80-90: Add an assertion for the frontend-facing error code.Once you map the code in the service, assert it here via getPaymentErrorCode for end-to-end safety.
describe("payment errors", () => { it("should handle 'Amount must be greater than $0'", () => { const { service } = setup(); const error = new Error("Amount must be greater than $0"); const result = service.toAppError(error, "payment"); expect(result).toHaveProperty("status", 400); expect(result).toHaveProperty("message", "Amount must be greater than $0"); }); + + it("should expose code 'amount_must_be_positive' for UI mapping", () => { + const { service } = setup(); + const code = service.getPaymentErrorCode(new Error("Amount must be greater than $0")); + expect(code).toEqual({ + message: "Amount must be greater than $0", + code: "amount_must_be_positive", + type: "payment_error" + }); + });Also applies to: 351-360
apps/api/src/billing/services/stripe/stripe.service.ts (2)
30-30: Consider sharing MINIMUM_PAYMENT_AMOUNT across API and UI to prevent drift.A small shared constants package (e.g., @src/shared/payments) would avoid divergence.
I can draft a minimal shared module if desired.
184-189: Remove redundant post-check for the same min condition.The later branch re-enforces the same “no-discount < $20” rule. Keeping one source reduces drift.
- } else if (!discounts.length && finalAmountDollars > 0 && finalAmountDollars < 20) { - throw new Error("Minimum payment amount is $20 (before any discounts)"); - } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/api/src/billing/services/stripe-error/stripe-error.service.spec.ts(1 hunks)apps/api/src/billing/services/stripe-error/stripe-error.service.ts(1 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(2 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(2 hunks)apps/deploy-web/src/pages/payment.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/billing/services/stripe-error/stripe-error.service.spec.tsapps/api/src/billing/services/stripe/stripe.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/services/stripe-error/stripe-error.service.spec.tsapps/api/src/billing/services/stripe-error/stripe-error.service.tsapps/api/src/billing/services/stripe/stripe.service.spec.tsapps/deploy-web/src/pages/payment.tsxapps/api/src/billing/services/stripe/stripe.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/services/stripe-error/stripe-error.service.spec.tsapps/api/src/billing/services/stripe-error/stripe-error.service.tsapps/api/src/billing/services/stripe/stripe.service.spec.tsapps/deploy-web/src/pages/payment.tsxapps/api/src/billing/services/stripe/stripe.service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: validate / validate-app
🔇 Additional comments (6)
apps/deploy-web/src/pages/payment.tsx (1)
18-18: Nice: single source for the UI minimum.Defining MINIMUM_PAYMENT_AMOUNT locally improves readability.
apps/api/src/billing/services/stripe-error/stripe-error.service.spec.ts (1)
81-88: Good coverage for the new mapping.Test correctly asserts 400 and message preservation.
apps/api/src/billing/services/stripe/stripe.service.spec.ts (3)
75-85: Tests for zero amount look good and match new contract.Covers the <= 0 guard.
86-96: Covers negative amounts too — nice.Complements the zero-amount case.
584-587: CSV tests updated to nested card shape — consistent with transformer.Matches transformTransactionForCsv’s accessors. Good.
apps/api/src/billing/services/stripe/stripe.service.ts (1)
148-156: Pre-discount guards are correct and user-friendly.Blocking non-positive amounts and enforcing the min when no discounts exist mirrors the UI behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
58-60: Currency-agnostic error textMessage hardcodes “$”. If multi-currency is possible, prefer a generic string.
- amount: z.number().gt(0, "Amount must be greater than $0"), + amount: z.number().gt(0, "Amount must be greater than 0"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/src/billing/http-schemas/stripe.schema.ts(1 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(1 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/billing/services/stripe/stripe.service.ts
- apps/api/src/billing/services/stripe/stripe.service.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/http-schemas/stripe.schema.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/http-schemas/stripe.schema.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
* fix(billing): prevent 0 amount payments * fix(billing): remove comments * fix(billing): move min amount of 0$ to zod * fix(billing): update amount validation to ensure it is greater than $0 * fix(billing): enhance amount validation to include exclusive minimum and minimum constraints * fix(billing): update minimum payment amount validation to $20
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests