refactor(billing): adds default flag to payment methods#2314
refactor(billing): adds default flag to payment methods#2314ygrishajev merged 3 commits intomainfrom
Conversation
WalkthroughgetPaymentMethods now accepts an ACL/ability parameter and returns Stripe payment methods merged with local DB records; each returned method includes boolean flags Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant AuthService
participant StripeService
participant StripeAPI
participant LocalRepo
Client->>Controller: Request payment methods
Controller->>AuthService: get ability
AuthService-->>Controller: ability
Controller->>StripeService: getPaymentMethods(userId, customerId, ability)
par fetch remote and local
StripeService->>StripeAPI: fetch Stripe payment methods
StripeAPI-->>StripeService: remote methods
StripeService->>LocalRepo: query local payment methods (accessibleBy ability)
LocalRepo-->>StripeService: local methods (ACL filtered)
end
StripeService->>StripeService: merge remote + local, set validated & isDefault
alt out-of-sync detected
StripeService-->>StripeService: log STRIPE_PAYMENT_METHOD_OUT_OF_SYNC
end
StripeService-->>Controller: merged PaymentMethod[] (with isDefault, validated)
Controller-->>Client: return payment methods
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
🧠 Learnings (1)📚 Learning: 2025-11-12T16:36:02.543ZApplied to files:
🧬 Code graph analysis (1)apps/api/test/seeders/payment-method.seeder.ts (3)
⏰ 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). (8)
🔇 Additional comments (8)
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: 0
🧹 Nitpick comments (4)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
71-76: Pass ability intogetPaymentMethodsfor ACL-aware retrievalForwarding
this.authService.abilityhere is consistent with the updated StripeService API and ensures local payment-method metadata is scoped by ACL.If you want consistency with
markAsDefault/getDefaultPaymentMethod, you could destructure once (const { ability } = this.authService;) and passabilityinstead of reaching through the service directly.apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
1-1: Cover ACL-awaregetPaymentMethodsand new flags in testsUsing
createMongoAbilityand stubbingpaymentMethodRepository.accessibleByto return the repository mock makes the test align with the newgetPaymentMethods(userId, customerId, ability)contract, and the expectations onvalidated/isDefaultdefaulting tofalselook correct.It would be worthwhile to add an extra test where
findByUserIdreturns local rows (includingisValidated/isDefault) to exercise the merge path and the out-of-sync logging case.Also applies to: 774-855, 1471-1471
apps/api/src/billing/services/stripe/stripe.service.ts (2)
4-6: ACL-aware merge of remote/local payment methods with default flagThe new
PaymentMethodtype andgetPaymentMethods(userId, customerId, ability)implementation look solid: local records are keyed bypaymentMethodId, merged into Stripe data withvalidated/isDefaultbooleans, sorted bycreateddesc, and out-of-sync remote IDs are logged viadifference.One nuance: ability currently scopes only the local repository (
accessibleBy(ability, "read")), while all Stripe methods for the givencustomerIdare still returned (withvalidated/isDefaultdefaulting tofalsewhen there is no accessible local row). If ACL is intended to control which payment methods are visible at all (not just which ones have local metadata attached), consider additionally filteringremotes.databased on the set of IDs present in the accessible locals, or otherwise confirming that “show all Stripe cards for this customer but hide local flags” is the intended behavior.Also applies to: 35-36, 110-141
143-163: PropagateisDefaultthrough default-payment flows and align repository ACL usageExtending
getDefaultPaymentMethodandmarkPaymentMethodAsDefaultto return{ ...remote, validated, isDefault }keeps the service type consistent and ensures callers always see both flags. UsingaccessibleBy(ability, "read" | "update" | "create")on the repository also correctly routes these operations through ACL.It may be worth double-checking your CASL rules so that the endpoint guarded with
action: "update", subject: "PaymentMethod"also grants whatever"create"permission is required forcreateAsDefault, and to ensure that out-of-sync logging ingetDefaultPaymentMethoddoesn’t become noisy purely due to ACL filtering (i.e., when a default exists in Stripe but is intentionally hidden by ability).Also applies to: 182-209
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
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/http-schemas/stripe.schema.ts(1 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(3 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(6 hunks)apps/api/test/seeders/payment-method.seeder.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/billing/http-schemas/stripe.schema.tsapps/api/test/seeders/payment-method.seeder.tsapps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/src/billing/services/stripe/stripe.service.spec.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/api/src/billing/controllers/wallet/wallet.controller.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/api/src/billing/services/stripe/stripe.service.spec.ts
🧠 Learnings (2)
📚 Learning: 2025-09-25T14:31:44.914Z
Learnt from: baktun14
Repo: akash-network/console PR: 1969
File: apps/deploy-web/src/pages/payment.tsx:179-191
Timestamp: 2025-09-25T14:31:44.914Z
Learning: The payment confirmation endpoint in apps/api/src/billing/http-schemas/stripe.schema.ts uses zod schema validation with `amount: z.number().gte(20, "Amount must be greater or equal to $20")` to ensure all payment requests meet the minimum amount requirement, preventing zero-amount or invalid payments from reaching Stripe.
Applied to files:
apps/api/src/billing/http-schemas/stripe.schema.ts
📚 Learning: 2025-11-12T16:36:02.543Z
Learnt from: baktun14
Repo: akash-network/console PR: 2203
File: apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx:161-168
Timestamp: 2025-11-12T16:36:02.543Z
Learning: In apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx, the organization field captured during payment method setup is internal metadata. Errors from stripe.updateCustomerOrganization should be logged to Sentry but not surfaced to users, and the flow should continue even if the organization update fails, as it's non-critical and not something users can fix.
Applied to files:
apps/api/src/billing/controllers/stripe/stripe.controller.ts
🧬 Code graph analysis (3)
apps/api/test/seeders/payment-method.seeder.ts (3)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
PaymentMethod(242-242)apps/api/src/billing/services/stripe/stripe.service.ts (1)
PaymentMethod(35-35)packages/http-sdk/src/stripe/stripe.types.ts (1)
PaymentMethod(30-41)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
apps/api/src/auth/services/auth.service.ts (2)
currentUser(13-15)currentUser(17-24)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
apps/api/test/seeders/stripe-test-data.seeder.ts (1)
TEST_CONSTANTS(5-12)
⏰ 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). (3)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
16-20: ExposeisDefaultflag on PaymentMethod schemaAdding
isDefault?: booleanhere cleanly extends the API without breaking existing consumers and aligns with the new service-level default flag.apps/api/test/seeders/payment-method.seeder.ts (1)
58-61: Extend seeder to supportvalidatedandisDefaultflagsThe updated helper cleanly adds
isDefaultalongsidevalidated, coercing both to booleans so generated testPaymentMethodobjects match the service type.apps/api/src/billing/controllers/wallet/wallet.controller.ts (1)
40-46: Propagate ability intogetPaymentMethodsin trial creationPassing
this.authService.abilityintostripeService.getPaymentMethodskeeps the duplicate-trial and “must have payment method” logic consistent with the new ACL-aware merge in the service.
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (68.18%) 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 #2314 +/- ##
==========================================
+ Coverage 47.69% 48.03% +0.34%
==========================================
Files 1028 1038 +10
Lines 29318 29676 +358
Branches 7576 7652 +76
==========================================
+ Hits 13982 14254 +272
- Misses 14872 14936 +64
- Partials 464 486 +22
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
ygrishajev
force-pushed
the
feature/billing
branch
from
85faaba to
e5283b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts (1)
19-19: Consider usingWalletController.namein describe block.Per coding guidelines, use
<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references.-describe("WalletController", () => { +describe(WalletController.name, () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
apps/api/src/billing/controllers/stripe/stripe.controller.ts(1 hunks)apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts(1 hunks)apps/api/src/billing/controllers/wallet/wallet.controller.ts(1 hunks)apps/api/src/billing/http-schemas/stripe.schema.ts(1 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(3 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(6 hunks)apps/api/test/seeders/payment-method.seeder.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/test/seeders/payment-method.seeder.tsapps/api/src/billing/http-schemas/stripe.schema.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/api/src/billing/services/stripe/stripe.service.spec.tsapps/api/src/billing/controllers/wallet/wallet.controller.spec.tsapps/api/src/billing/controllers/wallet/wallet.controller.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/api/src/billing/services/stripe/stripe.service.spec.tsapps/api/src/billing/controllers/wallet/wallet.controller.spec.ts
🧠 Learnings (2)
📚 Learning: 2025-11-12T16:36:02.543Z
Learnt from: baktun14
Repo: akash-network/console PR: 2203
File: apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx:161-168
Timestamp: 2025-11-12T16:36:02.543Z
Learning: In apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx, the organization field captured during payment method setup is internal metadata. Errors from stripe.updateCustomerOrganization should be logged to Sentry but not surfaced to users, and the flow should continue even if the organization update fails, as it's non-critical and not something users can fix.
Applied to files:
apps/api/src/billing/controllers/stripe/stripe.controller.ts
📚 Learning: 2025-09-25T14:31:44.914Z
Learnt from: baktun14
Repo: akash-network/console PR: 1969
File: apps/deploy-web/src/pages/payment.tsx:179-191
Timestamp: 2025-09-25T14:31:44.914Z
Learning: The payment confirmation endpoint in apps/api/src/billing/http-schemas/stripe.schema.ts uses zod schema validation with `amount: z.number().gte(20, "Amount must be greater or equal to $20")` to ensure all payment requests meet the minimum amount requirement, preventing zero-amount or invalid payments from reaching Stripe.
Applied to files:
apps/api/src/billing/http-schemas/stripe.schema.ts
🧬 Code graph analysis (6)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
apps/api/src/auth/services/auth.service.ts (2)
currentUser(13-15)currentUser(17-24)
apps/api/test/seeders/payment-method.seeder.ts (3)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
PaymentMethod(242-242)apps/api/src/billing/services/stripe/stripe.service.ts (1)
PaymentMethod(35-35)packages/http-sdk/src/stripe/stripe.types.ts (1)
PaymentMethod(30-41)
apps/api/src/billing/services/stripe/stripe.service.ts (5)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
PaymentMethod(242-242)packages/http-sdk/src/stripe/stripe.types.ts (1)
PaymentMethod(30-41)apps/api/src/billing/services/wallet-balance-reload-check/wallet-balance-reload-check.handler.ts (2)
userId(92-135)user(137-156)apps/api/src/billing/services/wallet-settings/wallet-settings.service.ts (2)
userId(58-75)userId(77-105)apps/api/src/auth/services/auth.service.ts (2)
ability(44-46)ability(48-50)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
apps/api/test/seeders/stripe-test-data.seeder.ts (1)
TEST_CONSTANTS(5-12)
apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts (1)
apps/api/test/seeders/payment-method.seeder.ts (1)
generatePaymentMethod(11-56)
apps/api/src/billing/controllers/wallet/wallet.controller.ts (1)
apps/api/src/auth/services/auth.service.ts (2)
currentUser(13-15)currentUser(17-24)
⏰ 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). (3)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
apps/api/test/seeders/payment-method.seeder.ts (1)
58-60: LGTM!The
isDefaultflag is correctly added to the seeder, following the same pattern as the existingvalidatedfield. The double-negation (!!isDefault) ensures a proper boolean value.apps/api/src/billing/services/stripe/stripe.service.ts (4)
110-114: LGTM on ACL integration.The
abilityparameter is correctly integrated for ACL-based filtering of local payment method records usingaccessibleBy(ability, "read"). The parallel fetching of remote and local data is efficient.
35-35: LGTM!The
PaymentMethodtype extension withisDefaultis correctly added alongsidevalidated.
155-164: LGTM!The
getDefaultPaymentMethodcorrectly returns bothvalidatedandisDefaultflags from the local record, with appropriate error logging when data is out of sync.
191-208: LGTM!The
markPaymentMethodAsDefaultmethod correctly propagatesisDefaultandvalidatedflags from local records in both the update and create paths.apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts (1)
251-251: LGTM!The mock correctly includes the new
isDefault: falsefield, aligning with the updatedPaymentMethodtype.apps/api/src/billing/http-schemas/stripe.schema.ts (1)
16-19: LGTM!The
isDefaultfield is correctly added to the schema as optional, consistent with thevalidatedfield pattern.apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
74-74: LGTM!The
abilityparameter is correctly passed togetPaymentMethods, following the same pattern used inmarkAsDefault(line 52) andgetDefaultPaymentMethod(line 60).apps/api/src/billing/controllers/wallet/wallet.controller.ts (1)
44-44: LGTM! Ability parameter properly added for ACL filtering.The addition of
this.authService.abilityas the third parameter aligns with the updatedgetPaymentMethodssignature for ACL-aware payment method retrieval. The usage is consistent with line 103 whereabilityis also accessed fromauthService.apps/api/src/billing/services/stripe/stripe.service.spec.ts (3)
1-1: LGTM! Import added for ACL test utilities.The
createMongoAbilityimport is necessary for constructing ability objects in the updated test cases.
834-854: LGTM! Test properly updated for ACL-aware payment method retrieval.The test correctly:
- Passes an ability object as the third parameter using
createMongoAbility([{ action: "read", subject: "PaymentMethod" }])- Updates expectations to include the new
isDefault: falsefield on returned payment methods- Maintains the existing
validated: falsefieldThese changes align with the PR objective of adding a default flag to payment methods while introducing ACL-based filtering.
1470-1470: LGTM! Proper fluent API mock for CASL's accessibleBy.The mock correctly implements the fluent API pattern by returning the repository itself, which matches CASL's
accessibleBymethod behavior for chaining.
ygrishajev
force-pushed
the
feature/billing
branch
from
e5283b1 to
b086a89
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/api/src/billing/services/stripe/stripe.service.ts (2)
110-141: Merging logic looks solid with one consideration.The implementation correctly:
- Fetches remote and local payment methods in parallel
- Uses
accessibleBy(ability, "read")for ACL-based filtering- Merges remote methods with local state using
keyByfor O(1) lookups- Defaults
validatedandisDefaulttofalsewhen no local record exists- Sorts by creation timestamp (newest first)
Out-of-sync detection consideration:
The current logic detects remote payment methods without local DB records (
difference(remoteIds, Object.keys(localById))). This is useful for identifying webhook delivery delays or failures. However, consider also detecting the inverse case: local records referencing payment methods that no longer exist remotely. This could indicate deleted payment methods or data integrity issues.Optional enhancement to detect orphaned local records:
const outOfSyncIds = difference(remoteIds, Object.keys(localById)); +const orphanedLocalIds = difference(Object.keys(localById), remoteIds); if (outOfSyncIds.length) { this.loggerService.warn({ event: "STRIPE_PAYMENT_METHOD_OUT_OF_SYNC", userId, outOfSyncIds }); } + +if (orphanedLocalIds.length) { + this.loggerService.warn({ + event: "STRIPE_PAYMENT_METHOD_ORPHANED_LOCAL", + userId, + orphanedLocalIds + }); +}
143-165: Consider more specific out-of-sync logging.The method correctly merges remote and local state when both exist (line 156). However, the else block (lines 158-163) logs
STRIPE_PAYMENT_METHOD_OUT_OF_SYNCfor multiple distinct scenarios:
- Remote default exists but no local record
- Local default exists but no remote default
- Neither exists
For better observability, consider logging different event names or including flags to distinguish these cases, making it easier to diagnose the specific synchronization issue.
Example refinement:
- } else { + } else if (!remote && local) { this.loggerService.warn({ - event: "STRIPE_PAYMENT_METHOD_OUT_OF_SYNC", + event: "STRIPE_DEFAULT_PAYMENT_METHOD_LOCAL_ONLY", userId: user.id, - remoteId: typeof remote === "string" ? remote : remote?.id, localId: local?.paymentMethodId }); + } else if (remote && !local) { + this.loggerService.warn({ + event: "STRIPE_DEFAULT_PAYMENT_METHOD_REMOTE_ONLY", + userId: user.id, + remoteId: typeof remote === "string" ? remote : remote?.id + }); }apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
1470-1470: Mock pattern for accessibleBy correctly enables method chaining.The mock returns
paymentMethodRepositoryitself, which properly implements the AccessibleModel pattern whereaccessibleBy(ability, action)returns a filtered repository instance for method chaining. This matches the production implementation in PaymentMethodRepository whereaccessibleBy(...abilityParams)returnsthisafter callingwithAbility(). However, for consistency with other test files in the codebase, consider using.mockReturnThis()instead of.mockReturnValue(paymentMethodRepository).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/api/test/functional/__snapshots__/docs.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
apps/api/src/billing/controllers/stripe/stripe.controller.ts(1 hunks)apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts(1 hunks)apps/api/src/billing/controllers/wallet/wallet.controller.ts(1 hunks)apps/api/src/billing/http-schemas/stripe.schema.ts(1 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(3 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(6 hunks)apps/api/test/seeders/payment-method.seeder.ts(1 hunks)packages/http-sdk/src/stripe/stripe.types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/src/billing/http-schemas/stripe.schema.ts
- apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts
- apps/api/test/seeders/payment-method.seeder.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/api/src/billing/services/stripe/stripe.service.spec.tspackages/http-sdk/src/stripe/stripe.types.tsapps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/src/billing/services/stripe/stripe.service.tsapps/api/src/billing/controllers/wallet/wallet.controller.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/api/src/billing/services/stripe/stripe.service.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-11-12T16:36:02.543Z
Learnt from: baktun14
Repo: akash-network/console PR: 2203
File: apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx:161-168
Timestamp: 2025-11-12T16:36:02.543Z
Learning: In apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx, the organization field captured during payment method setup is internal metadata. Errors from stripe.updateCustomerOrganization should be logged to Sentry but not surfaced to users, and the flow should continue even if the organization update fails, as it's non-critical and not something users can fix.
Applied to files:
apps/api/src/billing/controllers/stripe/stripe.controller.tsapps/api/src/billing/services/stripe/stripe.service.ts
🧬 Code graph analysis (4)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
apps/api/test/seeders/stripe-test-data.seeder.ts (1)
TEST_CONSTANTS(5-12)
apps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
apps/api/src/auth/services/auth.service.ts (2)
currentUser(13-15)currentUser(17-24)
apps/api/src/billing/services/stripe/stripe.service.ts (3)
apps/api/src/billing/http-schemas/stripe.schema.ts (1)
PaymentMethod(242-242)packages/http-sdk/src/stripe/stripe.types.ts (1)
PaymentMethod(30-42)apps/api/src/auth/services/auth.service.ts (2)
ability(44-46)ability(48-50)
apps/api/src/billing/controllers/wallet/wallet.controller.ts (1)
apps/api/src/auth/services/auth.service.ts (2)
currentUser(13-15)currentUser(17-24)
⏰ 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). (11)
- GitHub Check: validate (packages) / validate-unsafe
- GitHub Check: validate (apps/deploy-web) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
packages/http-sdk/src/stripe/stripe.types.ts (1)
35-35: LGTM! Clean type extension.The addition of
isDefault: booleanto thePaymentMethodinterface is consistent with the existingvalidatedfield and aligns with the broader changes in this PR.apps/api/src/billing/controllers/wallet/wallet.controller.ts (1)
44-44: LGTM! Consistent with StripeController changes.The addition of
this.authService.abilityas the third argument aligns with the updatedStripeService.getPaymentMethodssignature and follows the same pattern as inStripeController.apps/api/src/billing/services/stripe/stripe.service.spec.ts (2)
1-1: LGTM! Necessary import for ability creation in tests.
834-855: Test properly updated for new signature.The test correctly:
- Passes
createMongoAbility([{ action: "read", subject: "PaymentMethod" }])as the third argument- Expects
validated: falseandisDefault: falseon returned payment methods- Mocks
paymentMethodRepository.findByUserIdto return empty array (no local records)This validates the merging logic where remote methods without local DB records have both flags set to false.
apps/api/src/billing/services/stripe/stripe.service.ts (3)
4-5: LGTM! Appropriate lodash utilities for merging logic.
35-35: LGTM! Type properly extends Stripe.PaymentMethod.
183-209: LGTM! Proper handling of local record creation and ACL filtering.The method correctly:
- Uses
accessibleBy(ability, "update")for the default update- Uses
accessibleBy(ability, "create")when creating a new local record- Handles both cases: existing local record and missing local record
- Returns the merged
PaymentMethodwithvalidatedandisDefaultflags in both pathsapps/api/src/billing/controllers/stripe/stripe.controller.ts (1)
69-79: Remove ability parameter from repository-level authorization filtering.The
@Protecteddecorator ensures ability is properly initialized before the controller method executes, so there are no runtime concerns about availability. However, passingabilitytostripe.getPaymentMethods()which then applies it at the repository level viaaccessibleBy()violates the architectural principle that ability checks should be enforced at the controller level, not the repository level. Repository methods shouldn't enforce ability checks as they don't have context about where they're being called from and may be reused in different scenarios.Likely an incorrect or invalid review comment.
ygrishajev
force-pushed
the
feature/billing
branch
from
b086a89 to
ea1f78c
Compare
3 participants
refs #1779
Summary by CodeRabbit
New Features
Improvements
Reliability
Tests
✏️ Tip: You can customize this high-level summary in your review settings.