Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactored Stripe subscription synchronization logic to replace separate update/create paths with Prisma upsert operations. Consolidated subscription-related data fields into a unified object and added defensive checks for missing data with error logging. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fix P2025 by upserting Premium records during Stripe sync in
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/ee/billing/stripe/sync-stripe.ts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)
**/*.{ts,tsx}: For API GET requests to server, use theswrpackage
Useresult?.serverErrorwithtoastErrorfrom@/components/Toastfor error handling in async operations
**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls
**/*.{ts,tsx}: For early access feature flags, create hooks using the naming conventionuse[FeatureName]Enabledthat return a boolean fromuseFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming conventionuse[FeatureName]Variantthat define variant types, useuseFeatureFlagVariantKey()with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g.,inbox-cleaner,pricing-options-2)
Always define types for A/B test variant flags (e.g.,type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the!postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Useas constinstead of literal types and type annotations
Use eitherT[]orArray<T>consistently
Initialize each enum member value explicitly
Useexport typefor types
Use `impo...
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)
Always import Prisma enums from
@/generated/prisma/enumsinstead of@/generated/prisma/clientto avoid Next.js bundling errors in client componentsImport Prisma using the project's centralized utility:
import prisma from '@/utils/prisma'
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g.,
import groupBy from 'lodash/groupBy')
apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Do not export types/interfaces that are only used within the same file. Export later if needed
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/security.mdc)
**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma'sselectoption. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. AllfindUnique/findFirstcalls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
AllfindManyqueries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g.,emailAccount: { id: emailAccountId }) to validate ownership
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Usenext/imagepackage for images
For API GET requests to server, use theswrpackage with hooks likeuseSWRto fetch data
For text inputs, use theInputcomponent withregisterPropsfor form integration and error handling
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
**/*.{tsx,ts,css}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Implement responsive design with Tailwind CSS using a mobile-first approach
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't useaccessKeyattribute on any HTML element
Don't setaria-hidden="true"on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like<marquee>or<blink>
Only use thescopeprop on<th>elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assigntabIndexto non-interactive HTML elements
Don't use positive integers fortabIndexproperty
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include atitleelement for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
AssigntabIndexto non-interactive HTML elements witharia-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include atypeattribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden)
Always include alangattribute on the html element
Always include atitleattribute for iframe elements
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
!(pages/_document).{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't use the next/head module in pages/_document.js on Next.js projects
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)
**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g.,import groupBy from 'lodash/groupBy')
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
apps/web/**/*.{ts,tsx,js,jsx}: Use@/path aliases for imports from project root
Prefer self-documenting code over comments; use descriptive variable and function names instead of explaining intent with comments
Add helper functions to the bottom of files, not the top
All imports go at the top of files, no mid-file dynamic imports
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
apps/web/**/*.{ts,tsx,js,jsx,json,css}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Format code with Prettier
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
apps/web/**/*.{example,ts,json}
📄 CodeRabbit inference engine (apps/web/CLAUDE.md)
Add environment variables to
.env.example,env.ts, andturbo.json
Files:
apps/web/ee/billing/stripe/sync-stripe.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Baz Reviewer
- GitHub Check: Macroscope - Correctness Check
- GitHub Check: test
🔇 Additional comments (3)
apps/web/ee/billing/stripe/sync-stripe.ts (3)
32-38: LGTM! Good defensive error handling.The error logging and self-healing approach via upsert is appropriate. The comment clearly explains the expected system behavior (customer IDs pre-created).
113-131: LGTM! Excellent refactoring with enhanced lifecycle tracking.The consolidation into a
subscriptionDataobject improves maintainability, and the addition ofstripeCanceledAtandstripeEndedAtfields provides better subscription lifecycle visibility. The type handling forproduct(string vs object) is appropriate.
133-144: LGTM! Successfully addresses the P2025 error.The replacement of
updatewithupsertcorrectly handles cases where the Premium record doesn't exist, preventing the P2025 error. The use of the consolidatedsubscriptionDataobject ensures consistency between update and create operations.
| const subscriptionData = { | ||
| stripeSubscriptionId: null, | ||
| stripeSubscriptionItemId: null, | ||
| stripePriceId: null, | ||
| stripeProductId: null, | ||
| stripeSubscriptionStatus: null, | ||
| stripeCancelAtPeriodEnd: null, | ||
| stripeRenewsAt: null, | ||
| stripeTrialEnd: null, | ||
| }; | ||
|
|
||
| await prisma.premium.upsert({ | ||
| where: { stripeCustomerId: customerId }, | ||
| data: { | ||
| stripeSubscriptionId: null, | ||
| stripeSubscriptionItemId: null, | ||
| stripePriceId: null, | ||
| stripeProductId: null, | ||
| stripeSubscriptionStatus: null, // Or 'none', 'canceled' depending on desired state | ||
| stripeCancelAtPeriodEnd: null, | ||
| stripeRenewsAt: null, | ||
| stripeTrialEnd: null, | ||
| // Keep stripeCanceledAt and stripeEndedAt as they might be relevant if it *was* canceled/ended previously | ||
| update: subscriptionData, | ||
| create: { | ||
| ...subscriptionData, | ||
| stripeCustomerId: customerId, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Consider including tier in the no-subscription data.
The subscriptionData object for the no-subscription case doesn't include the tier field, but the active subscription case (lines 113-131) does. When a customer has no subscription, should the tier be reset to null to prevent users from retaining premium access?
🔎 Proposed fix to include tier in no-subscription data
const subscriptionData = {
+ tier: null,
stripeSubscriptionId: null,
stripeSubscriptionItemId: null,
stripePriceId: null,
stripeProductId: null,
stripeSubscriptionStatus: null,
stripeCancelAtPeriodEnd: null,
stripeRenewsAt: null,
stripeTrialEnd: null,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const subscriptionData = { | |
| stripeSubscriptionId: null, | |
| stripeSubscriptionItemId: null, | |
| stripePriceId: null, | |
| stripeProductId: null, | |
| stripeSubscriptionStatus: null, | |
| stripeCancelAtPeriodEnd: null, | |
| stripeRenewsAt: null, | |
| stripeTrialEnd: null, | |
| }; | |
| await prisma.premium.upsert({ | |
| where: { stripeCustomerId: customerId }, | |
| data: { | |
| stripeSubscriptionId: null, | |
| stripeSubscriptionItemId: null, | |
| stripePriceId: null, | |
| stripeProductId: null, | |
| stripeSubscriptionStatus: null, // Or 'none', 'canceled' depending on desired state | |
| stripeCancelAtPeriodEnd: null, | |
| stripeRenewsAt: null, | |
| stripeTrialEnd: null, | |
| // Keep stripeCanceledAt and stripeEndedAt as they might be relevant if it *was* canceled/ended previously | |
| update: subscriptionData, | |
| create: { | |
| ...subscriptionData, | |
| stripeCustomerId: customerId, | |
| }, | |
| }); | |
| const subscriptionData = { | |
| tier: null, | |
| stripeSubscriptionId: null, | |
| stripeSubscriptionItemId: null, | |
| stripePriceId: null, | |
| stripeProductId: null, | |
| stripeSubscriptionStatus: null, | |
| stripeCancelAtPeriodEnd: null, | |
| stripeRenewsAt: null, | |
| stripeTrialEnd: null, | |
| }; | |
| await prisma.premium.upsert({ | |
| where: { stripeCustomerId: customerId }, | |
| update: subscriptionData, | |
| create: { | |
| ...subscriptionData, | |
| stripeCustomerId: customerId, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In apps/web/ee/billing/stripe/sync-stripe.ts around lines 55 to 73, the
subscriptionData object for the no-subscription case omits the tier field so
upsert won't clear a previous premium tier; add tier: null to subscriptionData
and ensure the create branch also includes tier: null (alongside
stripeCustomerId) so customers without a Stripe subscription have their tier
reset to null.
User description
Replaces prisma.premium.update() with upsert() in syncStripeDataToDb to handle cases where the Premium record doesn't exist yet.
Generated description
Below is a concise technical summary of the changes proposed in this PR:
graph LR syncStripeDataToDb_("syncStripeDataToDb"):::modified STRIPE_("STRIPE"):::modified PRISMA_("PRISMA"):::modified LOOPS_SERVICE_("LOOPS_SERVICE"):::modified syncStripeDataToDb_ -- "Now fetches subscriptions and handles absent Premium gracefully" --> STRIPE_ STRIPE_ -- "Response now used to build subscriptionData for upsert" --> syncStripeDataToDb_ syncStripeDataToDb_ -- "Replaces update with upsert; persists subscriptionData or nulls fields" --> PRISMA_ syncStripeDataToDb_ -- "Now upserts and returns premium/users before triggering Loops" --> LOOPS_SERVICE_ classDef added stroke:#15AA7A classDef removed stroke:#CD5270 classDef modified stroke:#EDAC4C linkStyle default stroke:#CBD5E1,font-size:13pxReplace
prisma.premium.update()withprisma.premium.upsert()in thesyncStripeDataToDbfunction to robustly handle cases where aPremiumrecord might not exist, preventing P2025 errors, and log an error if a record is unexpectedly missing.subscriptionDataobject across update and create operations within theupsertcalls.Modified files (1)
Latest Contributors(1)
upsertoperations to prevent P2025 errors whenPremiumrecords are missing and add logging for unexpected missing records.Modified files (1)
Latest Contributors(1)
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.