feat(billing): add database schema and core types for payment method validation#1897
feat(billing): add database schema and core types for payment method validation#1897
Conversation
…validation - Add database migration for payment method changes - Add new payment method types and schemas - Update payment method repository with validation support - Update payment method seeder for testing
WalkthroughAdds an is_validated boolean column and composite index to payment_methods; updates Drizzle metadata and journal; extends the PaymentMethods schema and repository with validation-related methods; adapts seeders and tests to nested Stripe-style card data and to the renamed repository method used by StripeService. Changes
Sequence Diagram(s)sequenceDiagram
participant StripeService
participant PaymentMethodRepo
participant DB
Note over StripeService,PaymentMethodRepo #DDEEFF: Duplicate trial-account check (new)
StripeService->>PaymentMethodRepo: findOthersTrialingByFingerprint(fingerprints[], currentUserId)
PaymentMethodRepo->>DB: SELECT ... WHERE fingerprint IN (...) AND user_id != currentUserId
DB-->>PaymentMethodRepo: rows[]
PaymentMethodRepo-->>StripeService: PaymentMethod[] | undefined
alt any userWallets.isTrialing == true
StripeService->>StripeService: treat as duplicate
else
StripeService->>StripeService: no duplicate
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
🧹 Nitpick comments (4)
apps/api/src/billing/model-schemas/payment-method/payment-method.schema.ts (2)
18-18: Use camelCase field name and map to snake_case column.Keeps TS API consistent with existing fields (userId, paymentMethodId, createdAt).
Apply this diff:
- is_validated: boolean("is_validated").default(false).notNull(), + isValidated: boolean("is_validated").default(false).notNull(),And update downstream references to table.isValidated.
26-26: Update index definition to the camelCase property if you rename the field.Apply this diff after renaming:
- userIdIsValidatedIdx: index("payment_methods_user_id_is_validated_idx").on(table.userId, table.is_validated), + userIdIsValidatedIdx: index("payment_methods_user_id_is_validated_idx").on(table.userId, table.isValidated),apps/api/src/billing/types/payment-method.type.ts (1)
1-30: Prefer Stripe’s official types (or compose from them) to avoid drift.Reduces maintenance and mismatches with upstream API.
Apply this diff if Stripe is available:
+import type Stripe from "stripe"; -export interface PaymentMethod { - type: string; - id: string; - card?: { - brand: string | null; - last4: string | null; - exp_month: number; - exp_year: number; - funding?: string | null; - country?: string | null; - network?: string | null; - fingerprint?: string | null; - three_d_secure_usage?: { - supported?: boolean | null; - } | null; - } | null; - billing_details?: { - address?: { - city: string | null; - country: string | null; - line1: string | null; - line2: string | null; - postal_code: string | null; - state: string | null; - } | null; - email?: string | null; - name?: string | null; - phone?: string | null; - }; -} +export type PaymentMethod = Pick< + Stripe.PaymentMethod, + "id" | "type" | "card" | "billing_details" +>;If Stripe types aren’t desired, consider narrowing unions (brands/networks/funding) to known literals for stronger type safety.
apps/api/test/seeders/payment-method.seeder.ts (1)
11-24: Default type may not align with Stripe payment method types.faker.finance.transactionType() produces values that likely don’t match Stripe (e.g., “invoice”). Default to "card" or accept an explicit union.
Apply this diff:
- type: input.type || faker.finance.transactionType(), + type: input.type ?? "card",Optionally tighten the type: type?: "card" | "us_bank_account" | "sepa_debit" | string;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/api/drizzle/0019_dusty_polaris.sql(1 hunks)apps/api/drizzle/meta/0019_snapshot.json(1 hunks)apps/api/drizzle/meta/_journal.json(1 hunks)apps/api/src/billing/model-schemas/payment-method/payment-method.schema.ts(2 hunks)apps/api/src/billing/repositories/payment-method/payment-method.repository.ts(2 hunks)apps/api/src/billing/types/payment-method.type.ts(1 hunks)apps/api/test/seeders/payment-method.seeder.ts(1 hunks)
🧰 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/types/payment-method.type.tsapps/api/src/billing/model-schemas/payment-method/payment-method.schema.tsapps/api/test/seeders/payment-method.seeder.tsapps/api/src/billing/repositories/payment-method/payment-method.repository.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/types/payment-method.type.tsapps/api/src/billing/model-schemas/payment-method/payment-method.schema.tsapps/api/test/seeders/payment-method.seeder.tsapps/api/src/billing/repositories/payment-method/payment-method.repository.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). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (9)
apps/api/drizzle/0019_dusty_polaris.sql (2)
1-1: Column addition looks good.NOT NULL with DEFAULT false is safe for backfilling existing rows.
2-2: Create the index CONCURRENTLY and run it outside a transaction.On large tables plain CREATE INDEX blocks writes; use CONCURRENTLY and ensure the statement is executed outside a transactional migration (add a drizzle stmt-breakpoint before it).
File: apps/api/drizzle/0019_dusty_polaris.sql
Apply this diff:
-ALTER TABLE "payment_methods" ADD COLUMN "is_validated" boolean DEFAULT false NOT NULL;--> statement-breakpoint -CREATE INDEX IF NOT EXISTS "payment_methods_user_id_is_validated_idx" ON "payment_methods" USING btree ("user_id","is_validated"); +ALTER TABLE "payment_methods" ADD COLUMN "is_validated" boolean DEFAULT false NOT NULL;--> statement-breakpoint +-- drizzle:stmt-breakpoint +CREATE INDEX CONCURRENTLY IF NOT EXISTS "payment_methods_user_id_is_validated_idx" ON "payment_methods" USING btree ("user_id","is_validated");Also consider dropping the single-column user_id index later if it becomes redundant.
apps/api/drizzle/meta/_journal.json (1)
139-144: Journal entry matches migration tag and ordering.No issues.
apps/api/drizzle/meta/0019_snapshot.json (3)
193-199: Snapshot reflects the new is_validated column as boolean with default false.Matches the SQL migration.
267-287: Composite index captured correctly.Good alignment with the intended validated-by-user lookups.
43-50: Note for repository code: user_wallets column is named trial.Ensure repository checks use trial (boolean) and treat userWallets as an array, not an object.
apps/api/test/seeders/payment-method.seeder.ts (1)
3-3: Import path change looks fine.No issues.
apps/api/src/billing/repositories/payment-method/payment-method.repository.ts (2)
57-63: Validated lookup aligns with the new composite index.Good use of the is_validated filter.
65-67: ```shell
#!/bin/bashVerify payment-method repository and related artifacts
set -euo pipefail
echo "CWD: $(pwd)"
echo "----- locate target file -----"
rg -n --hidden "apps/api/src/billing/repositories/payment-method/payment-method.repository.ts" || trueecho "----- print file (if present) -----"
if [ -f "apps/api/src/billing/repositories/payment-method/payment-method.repository.ts" ]; then
sed -n '1,240p' apps/api/src/billing/repositories/payment-method/payment-method.repository.ts || true
else
echo "File not found at apps/api/src/billing/repositories/payment-method/payment-method.repository.ts"
fiecho "----- search for updateBy implementations/usages -----"
rg -n --hidden -C3 "updateBy\s*(" || true
rg -n --hidden -C3 "updateBy\s*[:=]\s*async" || trueecho "----- search for repository base classes -----"
rg -n --hidden -C3 "class\s+\w*Repository\b" || true
rg -n --hidden -C3 "BaseRepository|AbstractRepository|RepositoryBase" || trueecho "----- search for is_validated occurrences -----"
rg -n --hidden -C3 "is_validated" || trueecho "----- search for payment_methods/migrations/schema -----"
rg -n --hidden -C3 "payment_methods" || true
rg -n --hidden -C3 "payment-method|payment_method|PaymentMethod" || trueecho "----- search for updatedAt / updated_at conventions -----"
rg -n --hidden -C3 "updatedAt|updated_at" || trueecho "----- check for ORM or db tool indicators -----"
rg -n --hidden -C3 "prisma|TypeORM|knex|sequelize|objection|typeorm|pg" || trueecho "----- show package.json (root and apps/api) -----"
[ -f package.json ] && echo "---- root package.json ----" && sed -n '1,200p' package.json || echo "no root package.json"
[ -f apps/api/package.json ] && echo "---- apps/api/package.json ----" && sed -n '1,200p' apps/api/package.json || echo "no apps/api/package.json"</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
apps/api/src/billing/repositories/payment-method/payment-method.repository.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/src/billing/services/stripe/stripe.service.ts (1)
578-607: Avoid type name collision; use Stripe types or a clear alias.This local
PaymentMethodoverlaps conceptually with DB/domain payment method types introduced in this PR. PreferStripe.PaymentMethodin this service and remove the local interface to prevent confusion.Apply this diff:
- async getPaymentMethods(customerId: string): Promise<PaymentMethod[]> { + async getPaymentMethods(customerId: string): Promise<Stripe.PaymentMethod[]> { const paymentMethods = await this.paymentMethods.list({ customer: customerId }); return paymentMethods.data; }- async hasDuplicateTrialAccount(paymentMethods: PaymentMethod[], currentUserId: string): Promise<boolean> { + async hasDuplicateTrialAccount(paymentMethods: Stripe.PaymentMethod[], currentUserId: string): Promise<boolean> {-export interface PaymentMethod { - type: string; - id: string; - card?: { - brand: string | null; - last4: string | null; - exp_month: number; - exp_year: number; - funding?: string | null; - country?: string | null; - network?: string | null; - fingerprint?: string | null; - three_d_secure_usage?: { - supported?: boolean | null; - } | null; - } | null; - billing_details?: { - address?: { - city: string | null; - country: string | null; - line1: string | null; - line2: string | null; - postal_code: string | null; - state: string | null; - } | null; - email?: string | null; - name?: string | null; - phone?: string | null; - }; -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/billing/services/stripe/stripe.service.spec.ts(3 hunks)apps/api/src/billing/services/stripe/stripe.service.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/services/stripe/stripe.service.tsapps/api/src/billing/services/stripe/stripe.service.spec.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/stripe.service.tsapps/api/src/billing/services/stripe/stripe.service.spec.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
🧬 Code graph analysis (1)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
apps/api/src/billing/services/stripe/stripe.service.ts (1)
PaymentMethod(578-607)
⏰ 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). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (4)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (3)
899-909: LGTM! Test correctly updated for plural repository method.The test properly mocks
findOthersByFingerprintto return an array of PaymentMethod objects with the newisValidatedfield, which aligns with the repository method's updated signature.
914-914: LGTM! Method call updated correctly.The test correctly calls the updated plural method
findOthersByFingerprint.
928-928: LGTM! All test expectations updated consistently.All test cases have been correctly updated to use the plural method name
findOthersByFingerprintand handle the array return type appropriately.Also applies to: 933-933, 957-957, 962-962, 970-970, 975-975
apps/api/src/billing/services/stripe/stripe.service.ts (1)
572-572: Ensure duplicates are checked only against validated methods.Confirm that
findOthersByFingerprintfilters tois_validated = true. If not, either switch to the validated-only variant or filter in-memory to avoid false positives from unvalidated/seed data.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/src/billing/repositories/payment-method/payment-method.repository.ts (1)
26-47: Fix wallet relation shape/field; filter the result; add proper typing.
- userWallets is a collection; use
.some(...).- Field is
trial, notisTrialing.- Return only the trialing matches, not the whole set.
- Strongly type params and return type for consistency with other methods.
- async findOthersTrialingByFingerprint(fingerprints: string[], userId: string) { - const item = await this.cursor.query.PaymentMethods.findMany({ + async findOthersTrialingByFingerprint( + fingerprints: PaymentMethodOutput["fingerprint"][], + userId: PaymentMethodOutput["userId"] + ): Promise<PaymentMethodOutput[] | undefined> { + const items = await this.cursor.query.PaymentMethods.findMany({ where: this.whereAccessibleBy(and(inArray(this.table.fingerprint, fingerprints), ne(this.table.userId, userId))), with: { user: { with: { userWallets: { - columns: { - isTrialing: true - } + columns: { trial: true } } } } } }); - if (item && item.some(item => item.user?.userWallets?.isTrialing)) { - return this.toOutputList(item); + const filtered = items?.filter(pm => pm.user?.userWallets?.some(w => w.trial)) ?? []; + if (filtered.length) { + return this.toOutputList(filtered); } return undefined; }
🧹 Nitpick comments (2)
apps/api/src/billing/repositories/payment-method/payment-method.repository.ts (2)
27-39: Push the “trial” predicate into the DB to cut payload.If supported by your Drizzle relation query, add a nested where (and limit 1) on userWallets so you only hydrate trial wallets.
Example (adjust imports/types as needed):
with: { user: { with: { userWallets: { columns: { trial: true }, // where: eq(UserWallets.trial, true), // limit: 1, } } } }
65-67: Strongly type params for consistency.Align with other methods’ use of PaymentMethodOutput-derived types.
- async markAsValidated(paymentMethodId: string, userId: string) { + async markAsValidated( + paymentMethodId: PaymentMethodOutput["paymentMethodId"], + userId: PaymentMethodOutput["userId"] + ) { return await this.updateBy({ paymentMethodId, userId }, { isValidated: true }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/src/billing/repositories/payment-method/payment-method.repository.ts(2 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(3 hunks)apps/api/src/billing/services/stripe/stripe.service.ts(1 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/repositories/payment-method/payment-method.repository.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/payment-method/payment-method.repository.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). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (1)
apps/api/src/billing/repositories/payment-method/payment-method.repository.ts (1)
57-63: Validated query: LGTM.Matches the new (userId, is_validated) index intent and scopes via accessibleBy.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1897 +/- ##
==========================================
- Coverage 44.59% 44.19% -0.40%
==========================================
Files 979 969 -10
Lines 27439 27096 -343
Branches 7087 7034 -53
==========================================
- Hits 12236 11975 -261
+ Misses 14107 13949 -158
- Partials 1096 1172 +76
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
899-907: Good test data generation using the updated seeder.The test correctly uses the
generatePaymentMethodseeder with card overrides to create test payment methods with fingerprints.
🧹 Nitpick comments (2)
apps/api/test/seeders/payment-method.seeder.ts (2)
5-7: Consider making thePaymentMethodOverridestype more flexible for nested properties.The current type definition requires consumers to override the entire
cardobject even when they only want to change a single property. This could lead to unintended loss of default values.Consider using a deep partial type utility for better ergonomics:
-type PaymentMethodOverrides = Omit<Partial<Stripe.PaymentMethod>, "card"> & { - card?: Partial<Stripe.PaymentMethod.Card> | null; -}; +import type { DeepPartial } from "type-fest"; // or implement your own + +type PaymentMethodOverrides = DeepPartial<Stripe.PaymentMethod>;Alternatively, if you don't want to add a dependency, you could at least document that the merge behavior preserves nested defaults.
19-20: Potential issue with hardcoded card brand case mismatch.The hardcoded
brand: "Visa"uses title case, but test expectations use lowercase"visa". This inconsistency could cause test failures if the override doesn't specify the brand.card: { - brand: "Visa", + brand: "visa", last4: faker.finance.creditCardNumber().slice(-4),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/billing/services/stripe/stripe.service.spec.ts(5 hunks)apps/api/test/seeders/payment-method.seeder.ts(2 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/test/seeders/payment-method.seeder.tsapps/api/src/billing/services/stripe/stripe.service.spec.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/test/seeders/payment-method.seeder.tsapps/api/src/billing/services/stripe/stripe.service.spec.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
🧬 Code graph analysis (2)
apps/api/test/seeders/payment-method.seeder.ts (3)
apps/api/src/billing/services/stripe/stripe.service.ts (1)
PaymentMethod(578-607)apps/api/src/billing/http-schemas/stripe.schema.ts (1)
PaymentMethod(202-202)packages/http-sdk/src/stripe/stripe.types.ts (1)
PaymentMethod(34-44)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (3)
apps/api/test/seeders/payment-method.seeder.ts (1)
generatePaymentMethod(9-52)apps/api/src/billing/services/stripe/stripe.service.ts (1)
PaymentMethod(578-607)packages/http-sdk/src/stripe/stripe.types.ts (1)
PaymentMethod(34-44)
⏰ 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: validate / validate-app
🔇 Additional comments (4)
apps/api/test/seeders/payment-method.seeder.ts (1)
51-51: Good use of deep merge for applying overrides.The use of
lodash.mergecorrectly handles nested object overrides while preserving unspecified nested properties from the base object.apps/api/src/billing/services/stripe/stripe.service.spec.ts (3)
799-806: LGTM! Test correctly uses the updated seeder with nested card structure.The test properly leverages the new
generatePaymentMethodseeder to create payment methods with nested card data, aligning with the Stripe API structure.
909-919: LGTM! Test properly handles the new repository method and return type.The test correctly mocks
findOthersTrialingByFingerprintreturning an array ofPaymentMethodobjects with the newisValidatedfield, aligning with the schema changes in this PR.
949-961: Excellent edge case coverage for payment methods without fingerprints.The test thoroughly covers scenarios where payment methods have
nullcard orundefinedfingerprint, ensuring the filtering logic works correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (5)
28-42: Fix flakiness: don’t call StripeSeederCreate() inside expectations.Each call returns new random data, so these assertions will intermittently fail. Reuse the same stripeData from setup.
Apply this diff:
- const { service, userRepository } = setup(); + const { service, userRepository, stripeData } = setup(); @@ - expect(userRepository.updateBy).toHaveBeenCalledWith( + expect(userRepository.updateBy).toHaveBeenCalledWith( { id: user.id, stripeCustomerId: null }, - { stripeCustomerId: StripeSeederCreate().customer.id }, + { stripeCustomerId: stripeData.customer.id }, { returning: true } ); - expect(result).toEqual(StripeSeederCreate().customer.id); + expect(result).toEqual(stripeData.customer.id);And update setup() to return stripeData (see separate diff).
55-72: Same flakiness here: reuse seeded paymentIntent from setup.Apply this diff:
- const { service } = setup(); + const { service, stripeData } = setup(); @@ - expect(result).toEqual({ - success: true, - paymentIntentId: StripeSeederCreate().paymentIntent.id - }); + expect(result).toEqual({ success: true, paymentIntentId: stripeData.paymentIntent.id });
1035-1041: Expose stripeData from setup for deterministic assertions.Apply this diff:
return { service, userRepository, refillService, billingConfig, - paymentMethodRepository + paymentMethodRepository, + stripeData };
564-600: Removeas anycasts; use proper Stripe types or plain nulls.Violates repo guideline: never use
anyin TS files.Apply these diffs:
- percent_off: null as any, + percent_off: null, @@ - amount_off: null as any, + amount_off: null, @@ - jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(mockPromotionCode as any); + jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(mockPromotionCode as Stripe.PromotionCode); @@ - amount_off: null as any, + amount_off: null, @@ - jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(mockPromotionCode as any); + jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(mockPromotionCode as Stripe.PromotionCode); @@ - jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(null as any); + jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(null); @@ - jest.spyOn(service, "listCoupons").mockResolvedValue({ coupons: [mockCoupon] } as any); + jest.spyOn(service, "listCoupons").mockResolvedValue({ coupons: [mockCoupon] } as { coupons: Stripe.Coupon[] });Also applies to: 616-621, 635-637, 654-655
985-1043: Fix remainingas anycasts in spec filesrg output shows many lingering
as anyusages across tests — remove or narrow these casts (use proper types/test helpers) and re-run the same check. Examples:
- apps/notifications/src/modules/alert/services/chain-alert/chain-alert.service.spec.ts (multiple
options.callback(alerts as any)),- apps/notifications/src/modules/alert/services/deployment-balance-alerts/deployment-balance-alerts.service.spec.ts (
await options.callback(alerts as any)),- apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (
(result.props as any),config: {} as any),- apps/deploy-web/src/services/decodeInjectedConfig/decodeInjectedConfig.spec.ts (
(window as any),webcrypto as any),- apps/api/src/billing/services/stripe/stripe.service.spec.ts (several mocks cast
as any),- numerous functional tests under apps/api/test/functional/*.spec.ts (JSON responses cast
as any).
♻️ Duplicate comments (1)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
796-808: Nice: switched to seeders for PMs and added DB seeder usage.This addresses earlier review feedback to centralize fixtures.
Also applies to: 900-918
🧹 Nitpick comments (7)
apps/api/test/seeders/database-payment-method.seeder.ts (2)
13-22: Ensure updatedAt is >= createdAt for temporal consistency.Generate updatedAt from createdAt to avoid flaky asserts that compare timestamps.
Apply this diff:
export function generateDatabasePaymentMethod(overrides: Partial<DatabasePaymentMethod> = {}): DatabasePaymentMethod { - const basePaymentMethod: DatabasePaymentMethod = { + const createdAt = faker.date.recent(); + const basePaymentMethod: DatabasePaymentMethod = { id: faker.string.uuid(), userId: faker.string.uuid(), fingerprint: faker.string.uuid(), paymentMethodId: faker.string.uuid(), isValidated: false, - createdAt: faker.date.recent(), - updatedAt: faker.date.recent() + createdAt, + updatedAt: new Date(createdAt.getTime() + 1000) };
15-19: Consider realistic prefixes for IDs/fingerprint for readability.Using pm_/fp_ (still random) makes test intent clearer without changing behavior.
Example:
- fingerprint: faker.string.uuid(), - paymentMethodId: faker.string.uuid(), + fingerprint: `fp_${faker.string.uuid()}`, + paymentMethodId: `pm_${faker.string.uuid()}`,apps/api/src/billing/services/stripe/stripe.service.spec.ts (5)
401-405: Avoid Array.fromAsync for Node compatibility in CI.Use a for-await loop; safer across Node versions.
Apply this diff in all three places:
- const chunks = await Array.fromAsync(csvStream); - - const fullCsv = chunks.join(""); + const chunks: string[] = []; + for await (const chunk of csvStream) chunks.push(chunk); + const fullCsv = chunks.join("");Also applies to: 470-474, 510-513
985-1043: Test setup guideline gaps: location and signature.
- Setup is outside the root describe; guideline asks for it at the bottom of the root describe block.
- Signature should accept a single parameter with inline type (even if unused for now).
Proposed minimal change (can be applied later):
-function setup() { +function setup(_opts: { }: { } = {}) { // ... }And in a follow-up, move setup inside the outer describe as the last declaration.
186-193: Over-assertingcreated: undefinedcouples tests to implementation detail.Safer to assert subset of params without undefined fields.
Example:
- expect(service.charges.list).toHaveBeenCalledWith({ + expect(service.charges.list).toHaveBeenCalledWith({ customer: "cus_123", limit: 50, - created: undefined, - starting_after: undefined, + // created not set + // starting_after not set ending_before: "ch_before_id", expand: ["data.payment_intent"] });Also applies to: 225-236, 356-366
333-341: Optional: add Stripe types to local mocks for stronger compile-time checks.Annotate mock collections with
Stripe.ApiList<Stripe.Charge>or usesatisfiesto avoid accidental shape drift.Example:
- const mockCharges = { + const mockCharges = { data: [ { id: "ch_123", amount: 1000, currency: "usd", status: "succeeded", created: 1234567890, payment_method_details: { type: "card" }, receipt_url: "https://receipt.url", description: "Test charge", metadata: {} } ], has_more: false - }; + } satisfies Stripe.ApiList<Stripe.Charge>;Also applies to: 780-787, 120-136, 160-178, 196-214, 240-267, 305-323
565-566: You can dropas anyon nulls entirely.
nullsatisfies Stripe’s nullable fields; the extra cast is unnecessary.No diff added beyond the major fix above.
Also applies to: 597-598, 651-652
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/billing/services/stripe/stripe.service.spec.ts(6 hunks)apps/api/test/seeders/database-payment-method.seeder.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/test/seeders/database-payment-method.seeder.tsapps/api/src/billing/services/stripe/stripe.service.spec.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/test/seeders/database-payment-method.seeder.tsapps/api/src/billing/services/stripe/stripe.service.spec.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
🧬 Code graph analysis (1)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (4)
apps/api/test/seeders/payment-method.seeder.ts (1)
generatePaymentMethod(9-52)apps/api/test/seeders/database-payment-method.seeder.ts (1)
generateDatabasePaymentMethod(13-28)apps/api/src/billing/services/stripe/stripe.service.ts (1)
PaymentMethod(578-607)packages/http-sdk/src/stripe/stripe.types.ts (1)
PaymentMethod(34-44)
⏰ 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 (4)
apps/api/test/seeders/database-payment-method.seeder.ts (1)
3-11: Seeder type aligns with migration; good coverage.Interface shape is clear and matches the new isValidated column.
apps/api/src/billing/services/stripe/stripe.service.spec.ts (3)
895-924: Duplicate detection tests read well and cover edge-cases.Good coverage for multiple fingerprints, missing fingerprints, and empty inputs.
Also applies to: 926-943, 944-968, 970-981
1-16: Spec-only note: keep imports strictly typed.Good use of jest-mock-extended; no global
jest.mock()usage.
555-585: Good rollback test scenario.Verifies compensation when wallet top-up fails.
…validation (#1897) * feat(billing): add database schema and core types for payment method validation - Add database migration for payment method changes - Add new payment method types and schemas - Update payment method repository with validation support - Update payment method seeder for testing * refactor(billing): rename is_validated to isValidated * fix(billing): find others by fingerprint * fix(billing): rename function * fix(billing): fix types for seeder payments * fix(billing): payment method seeder
2 participants
#1886
Summary by CodeRabbit
New Features
Bug Fixes
Tests