feat(billing): validate payment methods trial#1750
Conversation
WalkthroughThis change introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant API
participant Stripe
participant DB
User->>WebApp: Add Payment Method
WebApp->>Stripe: Request to attach payment method
Stripe-->>API: payment_method.attached webhook
API->>DB: Insert payment_methods record
API-->>Stripe: 200 OK
User->>WebApp: Start Trial
WebApp->>API: Request to create wallet
API->>Stripe: Fetch payment methods
API->>DB: Check for duplicate fingerprints
alt Duplicate found
API-->>WebApp: 403 error (duplicate trial)
else No duplicate
API->>DB: Create wallet
API-->>WebApp: Success
end
User->>WebApp: Remove Payment Method
alt User is trialing
WebApp-->>User: Show error (cannot remove during trial)
else Not trialing
WebApp->>API: Request to remove payment method
API->>Stripe: Detach payment method
Stripe-->>API: payment_method.detached webhook
API->>DB: Remove payment_methods record
API-->>WebApp: Success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (38.94%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1750 +/- ##
==========================================
- Coverage 72.59% 72.30% -0.30%
==========================================
Files 625 624 -1
Lines 14908 14937 +29
Branches 2606 2633 +27
==========================================
- Hits 10823 10800 -23
+ Misses 3862 3792 -70
- Partials 223 345 +122
*This pull request uses carry forward flags. Click here to find out more.
🚀 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: 4
🧹 Nitpick comments (4)
apps/deploy-web/src/utils/errorUtils.test.ts (1)
4-220: LGTM: Comprehensive test coverage for error utilitiesThe test suite provides excellent coverage for all error utility functions with well-structured test cases covering:
- All supported error formats (HTTP errors, Error objects, structured errors)
- Edge cases like null/undefined values and empty strings
- Type coercion scenarios for unexpected input types
The tests are clear, well-organized, and thoroughly validate the error handling logic.
Note: Consider using a
setupfunction as specified in the coding guidelines, even though the current test structure works well for these utility function tests.apps/api/drizzle/meta/0017_snapshot.json (2)
406-432: Redundant composite index includes primary key columnid
deployment_settings.id_auto_top_up_enabled_closed_idxstarts withid, which is already covered by the table’s primary-key index.
Because PostgreSQL can only use the left-most prefix of a b-tree, the presence ofidfirst renders the remaining columns (auto_top_up_enabled,closed) unusable for filter conditions that don’t also constrainid, defeating the purpose of the composite.Consider:
- "id_auto_top_up_enabled_closed_idx": { - "columns": ["id", "auto_top_up_enabled", "closed"], + "auto_top_up_enabled_closed_idx": { + "columns": ["auto_top_up_enabled", "closed"],or, more selectively,
("user_id", "auto_top_up_enabled", "closed")if queries filter by owner.
228-351: Table naming style is inconsistent (userSettingvs snake_case)Legacy camel-case table
userSettingco-exists with newly added snake_case tables (payment_methods,deployment_settings, …).
Mixed-case identifiers require quoting in every SQL statement and are easy to mis-type. Standardising on one naming convention (typically lowercase snake_case) will reduce friction and tooling surprises.apps/deploy-web/src/components/onboarding/steps/PaymentVerificationCard/PaymentVerificationCard.tsx (1)
33-37: Consider the semantic implications of changing from<p>to<div>elements.The change from paragraph elements to generic divs removes semantic meaning. While functionally equivalent, this could impact accessibility and SEO. If this change is for styling purposes, consider using CSS to achieve the desired layout while preserving semantic HTML.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
apps/api/drizzle/0017_sloppy_morg.sql(1 hunks)apps/api/drizzle/meta/0017_snapshot.json(1 hunks)apps/api/drizzle/meta/_journal.json(1 hunks)apps/api/src/billing/controllers/stripe/stripe.controller.ts(1 hunks)apps/api/src/billing/controllers/wallet/wallet.controller.ts(1 hunks)apps/api/src/billing/model-schemas/index.ts(1 hunks)apps/api/src/billing/model-schemas/payment-method/payment-method.schema.ts(1 hunks)apps/api/src/billing/repositories/index.ts(1 hunks)apps/api/src/billing/repositories/payment-method/payment-method.repository.ts(1 hunks)apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts(4 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(3 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(3 hunks)apps/deploy-web/src/components/onboarding/OnboardingPage.tsx(2 hunks)apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx(5 hunks)apps/deploy-web/src/components/onboarding/steps/PaymentMethodStep/PaymentMethodStep.tsx(4 hunks)apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.test.tsx(1 hunks)apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsx(5 hunks)apps/deploy-web/src/components/onboarding/steps/PaymentVerificationCard/PaymentVerificationCard.tsx(1 hunks)apps/deploy-web/src/components/user/payment/PaymentMethodsList.tsx(2 hunks)apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx(4 hunks)apps/deploy-web/src/hooks/useManagedWallet.ts(2 hunks)apps/deploy-web/src/pages/payment.tsx(3 hunks)apps/deploy-web/src/types/errors.ts(1 hunks)apps/deploy-web/src/types/index.ts(1 hunks)apps/deploy-web/src/utils/errorUtils.test.ts(1 hunks)apps/deploy-web/src/utils/errorUtils.ts(1 hunks)apps/deploy-web/src/utils/stripeErrorHandler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/repositories/index.tsapps/api/src/billing/controllers/stripe/stripe.controller.tsapps/deploy-web/src/components/onboarding/steps/PaymentMethodStep/PaymentMethodStep.tsxapps/deploy-web/src/components/onboarding/steps/PaymentVerificationCard/PaymentVerificationCard.tsxapps/api/src/billing/controllers/wallet/wallet.controller.tsapps/deploy-web/src/components/onboarding/OnboardingPage.tsxapps/api/src/billing/model-schemas/payment-method/payment-method.schema.tsapps/deploy-web/src/types/index.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/deploy-web/src/pages/payment.tsxapps/api/src/billing/repositories/payment-method/payment-method.repository.tsapps/api/src/billing/model-schemas/index.tsapps/api/src/billing/services/stripe-webhook/stripe-webhook.service.tsapps/deploy-web/src/utils/stripeErrorHandler.tsapps/deploy-web/src/types/errors.tsapps/deploy-web/src/hooks/useManagedWallet.tsapps/api/src/billing/services/stripe/stripe.service.spec.tsapps/deploy-web/src/components/user/payment/PaymentMethodsList.tsxapps/deploy-web/src/context/WalletProvider/WalletProvider.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsxapps/deploy-web/src/utils/errorUtils.test.tsapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.test.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsxapps/deploy-web/src/utils/errorUtils.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/repositories/index.tsapps/api/src/billing/controllers/stripe/stripe.controller.tsapps/deploy-web/src/components/onboarding/steps/PaymentMethodStep/PaymentMethodStep.tsxapps/deploy-web/src/components/onboarding/steps/PaymentVerificationCard/PaymentVerificationCard.tsxapps/api/src/billing/controllers/wallet/wallet.controller.tsapps/deploy-web/src/components/onboarding/OnboardingPage.tsxapps/api/src/billing/model-schemas/payment-method/payment-method.schema.tsapps/deploy-web/src/types/index.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/deploy-web/src/pages/payment.tsxapps/api/src/billing/repositories/payment-method/payment-method.repository.tsapps/api/src/billing/model-schemas/index.tsapps/api/src/billing/services/stripe-webhook/stripe-webhook.service.tsapps/deploy-web/src/utils/stripeErrorHandler.tsapps/deploy-web/src/types/errors.tsapps/deploy-web/src/hooks/useManagedWallet.tsapps/api/src/billing/services/stripe/stripe.service.spec.tsapps/deploy-web/src/components/user/payment/PaymentMethodsList.tsxapps/deploy-web/src/context/WalletProvider/WalletProvider.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsxapps/deploy-web/src/utils/errorUtils.test.tsapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.test.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsxapps/deploy-web/src/utils/errorUtils.ts
**/*.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/stripe.service.spec.ts
🧠 Learnings (10)
apps/deploy-web/src/types/index.ts (1)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use queryBy methods instead of getBy methods in test expectations in .spec.tsx files
apps/deploy-web/src/pages/payment.tsx (2)
Learnt from: baktun14
PR: #1432
File: apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentCloseAlert.tsx:38-38
Timestamp: 2025-06-05T21:07:51.985Z
Learning: The ContactPointSelect component in apps/deploy-web/src/components/alerts/ContactPointSelectForm/ContactPointSelect.tsx uses the useFormContext hook internally to connect to React Hook Form, so it doesn't need to be wrapped in a FormField component.
Learnt from: ygrishajev
PR: #1512
File: apps/deploy-web/src/components/deployments/DeploymentBalanceAlert/DeploymentBalanceAlert.tsx:47-68
Timestamp: 2025-06-19T16:00:05.428Z
Learning: In React Hook Form setups with zod validation, child components using useFormContext() can rely on parent form validation rather than implementing local input validation. Invalid inputs like NaN from parseFloat() are handled by the parent schema validation, eliminating the need for additional local validation in child components.
apps/api/src/billing/repositories/payment-method/payment-method.repository.ts (1)
Learnt from: stalniy
PR: #1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
apps/deploy-web/src/utils/stripeErrorHandler.ts (1)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-07-27T12:16:08.566Z
Learning: Applies to **/*.{js,ts,tsx} : Never use deprecated methods from libraries.
apps/deploy-web/src/types/errors.ts (1)
Learnt from: jzsfkzm
PR: #1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/api/src/billing/services/stripe/stripe.service.spec.ts (6)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use jest.mock() to mock dependencies in test files. Instead, use jest-mock-extended to create mocks and pass mocks as dependencies to the service under test.
Learnt from: stalniy
PR: #1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use setup function instead of beforeEach in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : setup function must be at the bottom of the root describe block in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : setup function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in setup function
apps/deploy-web/src/utils/errorUtils.test.ts (4)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use queryBy methods instead of getBy methods in test expectations in .spec.tsx files
Learnt from: stalniy
PR: #1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use getBy methods instead of queryBy methods when testing element presence with toBeInTheDocument() because getBy throws an error and shows DOM state when element is not found, providing better debugging information than queryBy which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use setup function instead of beforeEach in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : setup function creates an object under test and returns it
apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.test.tsx (9)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use queryBy methods instead of getBy methods in test expectations in .spec.tsx files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use setup function instead of beforeEach in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : setup function creates an object under test and returns it
Learnt from: stalniy
PR: #1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use getBy methods instead of queryBy methods when testing element presence with toBeInTheDocument() because getBy throws an error and shows DOM state when element is not found, providing better debugging information than queryBy which returns null.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : setup function must be at the bottom of the root describe block in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use jest.mock() to mock dependencies in test files. Instead, use jest-mock-extended to create mocks and pass mocks as dependencies to the service under test.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : setup function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in setup function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of setup function
apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsx (1)
Learnt from: baktun14
PR: #1432
File: apps/deploy-web/src/components/deployments/DeploymentAlerts/DeploymentCloseAlert.tsx:38-38
Timestamp: 2025-06-05T21:07:51.985Z
Learning: The ContactPointSelect component in apps/deploy-web/src/components/alerts/ContactPointSelectForm/ContactPointSelect.tsx uses the useFormContext hook internally to connect to React Hook Form, so it doesn't need to be wrapped in a FormField component.
apps/deploy-web/src/utils/errorUtils.ts (1)
Learnt from: jzsfkzm
PR: #1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
🧬 Code Graph Analysis (12)
apps/deploy-web/src/components/onboarding/steps/PaymentMethodStep/PaymentMethodStep.tsx (1)
apps/deploy-web/src/types/errors.ts (1)
AppError(26-26)
apps/api/src/billing/controllers/wallet/wallet.controller.ts (1)
apps/api/src/billing/services/stripe/stripe.service.ts (1)
hasDuplicateTrialAccount(442-453)
apps/deploy-web/src/components/onboarding/OnboardingPage.tsx (3)
apps/deploy-web/src/utils/urlUtils.ts (1)
UrlService(16-80)apps/provider-console/src/utils/styleUtils.ts (1)
cn(4-6)packages/ui/components/button.tsx (1)
buttonVariants(46-46)
apps/api/src/billing/model-schemas/payment-method/payment-method.schema.ts (1)
apps/api/src/user/model-schemas/user/user.schema.ts (1)
Users(8-28)
apps/api/src/billing/services/stripe/stripe.service.ts (1)
packages/http-sdk/src/stripe/stripe.types.ts (1)
PaymentMethod(34-44)
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts (2)
packages/logging/src/servicies/logger/logger.service.ts (1)
error(107-109)apps/api/src/core/services/tx/tx.service.ts (1)
WithTransaction(33-45)
apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (2)
apps/deploy-web/src/types/errors.ts (1)
AppError(26-26)apps/deploy-web/src/hooks/useManagedWallet.ts (1)
useManagedWallet(15-87)
apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx (1)
apps/deploy-web/src/types/errors.ts (1)
AppError(26-26)
apps/deploy-web/src/utils/errorUtils.test.ts (2)
apps/deploy-web/src/utils/errorUtils.ts (3)
extractErrorMessage(6-28)isHttpErrorResponse(33-35)extractErrorData(40-45)apps/deploy-web/src/types/errors.ts (1)
AppError(26-26)
apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.test.tsx (2)
apps/deploy-web/src/types/errors.ts (1)
AppError(26-26)apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsx (1)
PaymentMethodsDisplay(32-140)
apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsx (3)
apps/deploy-web/src/types/errors.ts (1)
AppError(26-26)apps/deploy-web/src/utils/urlUtils.ts (1)
UrlService(16-80)apps/deploy-web/src/utils/errorUtils.ts (1)
extractErrorMessage(6-28)
apps/deploy-web/src/utils/errorUtils.ts (1)
apps/deploy-web/src/types/errors.ts (2)
AppError(26-26)ErrorResponse(4-10)
⏰ 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
🔇 Additional comments (69)
apps/deploy-web/src/components/onboarding/OnboardingPage.tsx (2)
2-5: LGTM! Clean imports with proper TypeScript types.The new imports are well-organized and properly typed. The imports follow the established patterns in the codebase:
- UI components from the design system package
- Utility functions for styling
- Icon from the iconoir-react library
- Next.js Link component for navigation
No usage of
anytypes, which aligns with the coding guidelines.
18-24: Well-implemented navigation element with good UX patterns.The "Back to Console" link is properly implemented with:
- Semantic HTML structure using Next.js Link
- Appropriate styling with ghost button variant and proper spacing
- Good accessibility with descriptive text and visual icon
- Consistent use of utility classes and design system components
- Proper navigation to home page using UrlService.home()
The centered layout and spacing (py-8) provides good visual separation from the onboarding content above.
apps/api/drizzle/meta/_journal.json (1)
124-130: LGTM! Migration journal entry is properly formatted.The new migration entry follows the consistent structure of previous entries and correctly references the payment_methods table migration.
apps/deploy-web/src/pages/payment.tsx (3)
11-11: LGTM! Clean import addition.The import follows the established pattern and integrates well with existing imports.
42-42: LGTM! Proper hook usage.The destructuring of
isTrialingfromuseWalletfollows React best practices and is consistent with other hook usage in the component.
258-258: LGTM! Appropriate prop passing.The
isTrialingprop is correctly passed to thePaymentMethodsListcomponent, enabling trial-specific UI behavior as intended.apps/deploy-web/src/types/index.ts (1)
11-11: LGTM! Proper barrel export addition.The export statement follows the established pattern and correctly exposes the new error types for use throughout the application.
apps/api/src/billing/model-schemas/index.ts (1)
3-3: LGTM! Clean schema export addition.The export statement properly follows the established pattern and makes the payment method schema accessible from the billing schemas index.
apps/api/src/billing/repositories/index.ts (1)
3-3: LGTM! Proper repository export addition.The export statement correctly follows the established pattern and makes the PaymentMethodRepository accessible from the billing repositories index.
apps/deploy-web/src/hooks/useManagedWallet.ts (4)
21-21: LGTM!Properly destructures the error state from the mutation hook with a clear, descriptive name.
83-86: LGTM!Correctly exposes the error state in the return object and properly updates the dependency array to ensure the memoized result updates when the error state changes.
21-21: LGTM!Properly extracts the error state from the mutation hook for downstream consumption.
83-86: LGTM!Correctly exposes the createError and includes it in the dependency array for proper memoization.
apps/deploy-web/src/utils/stripeErrorHandler.ts (2)
1-2: LGTM!Good refactoring to centralize the
ErrorResponsetype definition. This reduces code duplication and provides a single source of truth for error types across the application.
1-2: LGTM!Good refactoring to use centralized type definitions. This reduces code duplication and improves maintainability.
apps/api/src/billing/controllers/stripe/stripe.controller.ts (2)
106-106: LGTM!Proper implementation of trial restriction for payment method removal. The assertion is correctly placed, uses the appropriate HTTP status code (403), and provides a clear, actionable error message for users.
106-106: LGTM!Proper implementation of trial restrictions for payment method removal. The assertion is well-placed, uses appropriate HTTP status code, and provides clear user guidance.
apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (8)
24-24: LGTM!Correct import of the
AppErrortype for error handling integration.
58-58: LGTM!Properly adds the optional managed wallet error property to the context type interface with correct TypeScript typing.
88-88: LGTM!Correctly destructures the error state from the managed wallet hook with descriptive renaming for clarity.
343-343: LGTM!Properly exposes the managed wallet error through the context value, enabling downstream components to access and handle wallet creation errors.
24-24: LGTM!Correctly imports the centralized AppError type for consistent error handling.
58-58: LGTM!Proper addition of optional managedWalletError property to the context interface. The optional nature maintains backward compatibility.
88-88: LGTM!Clean destructuring and renaming of the error state for better semantic clarity in the context.
343-343: LGTM!Properly exposes the managed wallet error state through the context for downstream error handling.
apps/deploy-web/src/components/onboarding/steps/PaymentMethodStep/PaymentMethodStep.tsx (3)
8-8: LGTM: Clean import of error type.The import of
AppErrortype is correctly added to support the new error handling functionality.
20-20: LGTM: Proper optional prop addition.The
managedWalletErrorprop is correctly defined as optional with the appropriateAppErrortype, maintaining backward compatibility.
37-37: LGTM: Consistent error prop handling.The error prop is properly destructured from the component parameters and correctly passed down to the
PaymentMethodsDisplaycomponent, maintaining the error propagation chain.Also applies to: 56-56
apps/deploy-web/src/components/user/payment/PaymentMethodsList.tsx (3)
11-11: LGTM: Clear boolean prop addition.The
isTrialingboolean prop is appropriately added to control payment method removal during trial periods.
19-21: LGTM: Proper prop destructuring.The
isTrialingprop is correctly added to the component's parameter destructuring.
55-67: LGTM: Effective trial restriction implementation.The conditional rendering of the Remove button based on
!isTrialingproperly prevents users from attempting payment method removal during trial periods, which aligns with the backend validation logic mentioned in the AI summary.apps/api/src/billing/controllers/wallet/wallet.controller.ts (1)
48-49: LGTM: Robust duplicate payment method validation.The addition of
hasDuplicateTrialAccountvalidation effectively prevents abuse by detecting when the same payment method fingerprint is already associated with another trial account. The error message is clear and the 403 status code is appropriate for this security restriction.apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx (4)
8-8: LGTM: Proper error type integration.The import and addition of
managedWalletErrorto the props interface correctly incorporates error handling into the component's API.Also applies to: 27-27
45-45: LGTM: Clean error state extraction.The
managedWalletErroris properly extracted from the wallet context alongside other wallet-related state.
64-68: LGTM: Effective error state management.This useEffect properly handles wallet connection errors by resetting the
isConnectingWalletstate when an error occurs, preventing the UI from being stuck in a loading state.
117-117: LGTM: Complete error propagation.The error is correctly passed to children components, maintaining the error handling chain throughout the component tree.
apps/api/src/billing/model-schemas/payment-method/payment-method.schema.ts (2)
6-18: LGTM: Well-structured table schema.The
PaymentMethodstable schema is properly defined with:
- UUID primary key with auto-generation
- Appropriate foreign key relationship to Users with cascade delete
- Required fields properly marked as
notNull()- Standard timestamp columns with sensible defaults
The cascade delete behavior is appropriate for payment methods that should be removed when a user is deleted.
20-25: LGTM: Proper ORM relations definition.The relations definition correctly establishes the one-to-many relationship between Users and PaymentMethods, enabling proper ORM functionality for queries and joins.
apps/api/src/billing/services/stripe/stripe.service.ts (1)
7-7: LGTM: Dependencies added correctlyThe new dependencies
PaymentMethodRepositoryandUserWalletRepositoryare properly imported and injected into the constructor to support the duplicate payment method detection feature.Also applies to: 33-34
apps/deploy-web/src/types/errors.ts (1)
1-34: LGTM: Well-structured error type definitionsThe error type definitions are comprehensive and well-documented. The union type
AppErroreffectively covers various error formats that can be encountered, and the interfaces provide good structure for consistent error handling across the application.apps/api/src/billing/services/stripe/stripe.service.spec.ts (3)
4-4: LGTM: Proper import of new repository typesThe new repository types are correctly imported to support the expanded test coverage.
702-794: LGTM: Comprehensive test coverage for hasDuplicateTrialAccountThe test suite provides excellent coverage for the new method with well-structured test cases:
- Tests both positive and negative duplicate detection scenarios
- Properly handles edge cases like missing fingerprints and empty arrays
- Uses proper mocking with jest-mock-extended as per coding guidelines
- Clear assertions and meaningful test data
801-802: LGTM: Setup function properly updatedThe setup function correctly mocks the new dependencies and returns them for use in tests, following the established pattern.
Also applies to: 804-804, 852-854
apps/api/drizzle/0017_sloppy_morg.sql (1)
1-14: LGTM: Well-structured database migrationThe migration creates a proper payment_methods table with:
- Appropriate column types and constraints
- UUID primary key with auto-generation
- Foreign key constraint with cascade delete for data integrity
- Proper exception handling to prevent duplicate constraint errors
- Timestamp fields for audit trail
The table structure supports the payment method tracking functionality effectively.
apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsx (8)
3-3: LGTM: Clean imports for error handling functionality.The new imports for WarningTriangle icon, AppError type, and extractErrorMessage utility are properly structured and support the enhanced error handling capabilities.
Also applies to: 6-7
26-29: LGTM: Well-designed props for error handling and dependency injection.The optional
managedWalletErrorprop is properly typed withAppError, and the dependency injection pattern forUrlServiceimproves testability while maintaining backward compatibility.
37-39: LGTM: Consistent parameter destructuring.The new props are properly destructured following the existing code patterns.
41-41: LGTM: Clean dependency injection pattern.The fallback pattern
dependencies?.UrlService || UrlServiceprovides excellent testability while maintaining default functionality.
50-53: LGTM: Well-designed error message helper.The
getErrorMessagefunction provides appropriate context-specific fallback messaging while properly delegating to the sharedextractErrorMessageutility.
58-58: LGTM: Good layout improvement.Adding
flex-shrink-0prevents the success alert icon from shrinking, maintaining visual consistency.
103-113: LGTM: Consistent error alert implementation.The error alert follows the same design patterns as the success alert, uses appropriate destructive styling with WarningTriangle icon, and properly extracts error messages using the helper function.
129-129: LGTM: Proper usage of injected URL service.The component correctly uses the injected
urlServiceinstead of the static import, maintaining consistency with the dependency injection pattern while preserving functionality.Also applies to: 133-133
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts (4)
5-5: LGTM: Proper dependency injection setup.The PaymentMethodRepository import and constructor injection follow established patterns and are properly typed.
Also applies to: 21-22
34-58: LGTM: Improved event routing with better error handling.The refactor from if-else to switch statement improves readability, and the try-catch wrapper provides comprehensive error logging while preserving error propagation. The new payment method event cases are properly integrated.
144-188: LGTM: Well-implemented webhook handler with comprehensive validation.The method properly handles the payment method attachment with:
- Transaction safety via
@WithTransaction()- Comprehensive validation of customer ID, user existence, and fingerprint
- Detailed error logging for debugging
- Clean database operation
The validation logic and error handling are thorough and appropriate.
190-236: LGTM: Robust payment method detachment handler.The method handles the complex detachment scenario effectively:
- Properly considers both current and previous customer ID from the event
- Uses appropriate
warnlevel logging for potentially expected missing data- Transaction safety with
@WithTransaction()- Clean deletion logic with result logging
The handling of
event.data.previous_attributes?.customeris particularly well thought out for detachment events.apps/deploy-web/src/utils/errorUtils.ts (3)
6-28: LGTM: Comprehensive error message extraction.The function handles all the expected error formats with appropriate fallbacks. The logic flow is clear and the fallback messages are user-friendly. Consider using the
isHttpErrorResponsetype guard for consistency, but the current implementation is functionally correct.
33-35: LGTM: Well-implemented TypeScript type guard.The type guard properly checks for HTTP error response structure and provides accurate type narrowing with a comprehensive return type.
40-45: LGTM: Clean error data extraction using type guard.The function properly leverages the
isHttpErrorResponsetype guard for type safety and provides a clean interface for extracting structured error data.apps/api/src/billing/repositories/payment-method/payment-method.repository.ts (4)
8-24: LGTM: Proper repository setup following established patterns.The type definitions, dependency injection, and BaseRepository extension follow the established patterns in the codebase correctly.
26-31: LGTM: Well-implemented fingerprint deduplication query.The method correctly uses
inArrayandneoperators to find payment methods with matching fingerprints from other users, with proper access control viawhereAccessibleBy.
33-39: LGTM: Clean user payment methods query.The method properly queries payment methods by user ID with appropriate access control and return type handling.
41-43: LGTM: Clean deletion wrapper method.The method provides a clear interface for payment method deletion by fingerprint, properly delegating to the inherited
deleteBymethod.apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.test.tsx (6)
1-41: LGTM: Well-structured test suite with proper organization.The test structure uses logical describe blocks and follows testing best practices. The use of
getBymethods for presence testing provides better debugging information as recommended.
43-84: LGTM: Comprehensive interaction testing.The tests properly verify button interactions, disabled states, and edge cases with appropriate mock usage and element selection.
86-211: LGTM: Exceptionally comprehensive error handling test coverage.The error handling tests cover all error types comprehensively:
- HTTP error responses with nested data
- JavaScript Error objects
- Structured error objects
- Edge cases (null, undefined, empty objects)
- Complex nested error structures
The use of
queryByfor non-existence testing follows established patterns correctly.
213-286: LGTM: Thorough edge case coverage.The edge case tests properly handle scenarios like missing card details, mixed payment method types, and date formatting variations that are likely to occur in production.
288-311: LGTM: Complete loading state testing.The tests properly verify loading button states and disabled interactions during both loading and removal operations.
313-380: LGTM: Excellent setup function following all established patterns.The setup function correctly implements all the established testing patterns:
- Positioned at bottom of root describe block
- Uses inline parameter type definition
- Avoids shared state
- Uses
jest.fn()for mocks instead ofjest.mock()- Provides proper test isolation and mock management
- Includes dependency injection testing
...ploy-web/src/components/onboarding/steps/PaymentVerificationCard/PaymentVerificationCard.tsx
Show resolved
Hide resolved
stalniy
left a comment
stalniy
left a comment
There was a problem hiding this comment.
I have few questions which I'd like to clarify
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts
Outdated
Show resolved
Hide resolved
apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsx
Outdated
Show resolved
Hide resolved
* fix(billing): p tag child warning * feat(billing): add back to console button in onboarding * feat(billing): add payment method tracking * feat(billing): show start trial error * feat: add tests * feat(billing): hide remove payment method when trial * fix(billing): prevent removing payment method when in trial * fix: pr fixes * fix(billing): add indexes and pr fixes * fix(billing): update error codes for wallet assertions to 400 * refactor(billing): integrate ServicesProvider for UrlService in PaymentMethodsDisplay
2 participants
#1732
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores