fix(billing): a user can add payment while upgrading their plan#2120
fix(billing): a user can add payment while upgrading their plan#2120
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
WalkthroughWalkthroughThe changes involve modifications to the billing and Stripe integration components within the dashboard application. Key updates include the introduction of a Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx (1)
88-92: Add error handling for PostHog event captureTo prevent uncaught exceptions during event capture, add error handling around
PostHogClient.capture.Apply this diff to add a try-catch block:
if (isChangingPlan) { + try { PostHogClient.capture({ distinctId: tenantId, event: "plan_changed", properties: { plan: new_plan, workspace: ws.id }, }); + } catch (error) { + console.error("PostHog capture failed:", error); + } }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (5)
- apps/dashboard/app/(app)/settings/billing/plans/button.tsx (2 hunks)
- apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (2 hunks)
- apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx (3 hunks)
- apps/dashboard/lib/posthog.ts (1 hunks)
- apps/dashboard/package.json (1 hunks)
Additional comments not posted (10)
apps/dashboard/lib/posthog.ts (1)
3-27: LGTM!The
PostHogClientWrapperclass is a well-designed singleton that provides a centralized way to initialize and access thePostHogclient instance. It handles the case when the required environment variables are missing by providing a mock client, ensuring that the application gracefully handles missing configuration. The class encapsulates the initialization logic and configuration of thePostHogclient, making it easier to manage and update.apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (3)
10-14: LGTM!The
Propstype definition is clear and accurately captures the structure of theStripeRedirectfunction's parameter. Thenew_planproperty is well-defined with the possible values it can take.
16-17: LGTM!The
StripeRedirectfunction signature is correctly updated to accept thepropsparameter of typeProps. Destructuringnew_planfromprops.searchParamsis a clean and readable approach.
63-68: LGTM!The modification to the
successUrlstring is a good addition. It allows the URL to carry the new plan information when redirecting after a successful Stripe checkout session. The condition ensures that thenew_planparameter is only appended when necessary, i.e., when it's defined and different from the current plan.apps/dashboard/app/(app)/settings/billing/plans/button.tsx (2)
48-58: LGTM!The
handleClickfunction correctly manages the logic for changing the billing plan. It checks for the presence of a payment method and redirects the user to add a payment method if necessary before proceeding with the plan change. The logic is well-structured and improves the user experience.
105-105: LGTM!Updating the button's
onClickevent handler to call thehandleClickfunction is a good change. It centralizes the plan change logic and improves code readability.apps/dashboard/package.json (1)
92-92: New dependency added for server-side analytics.The
posthog-nodepackage has been added as a new dependency, likely for implementing server-side analytics. This is a good addition to gain insights into user behavior and application performance.Please ensure that the package is being utilized effectively in the codebase to capture relevant events and metrics. Additionally, consider setting up a process to keep this dependency updated to benefit from bug fixes and enhancements while respecting the specified version constraint.
Can you provide more context on the specific use case and implementation of
posthog-nodein this project?apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx (3)
75-75: Check logical conditions forisChangingPlanEnsure that the logic correctly handles all cases, including when
new_planis undefined or the same asws.plan.Run the following script to verify the possible values of
new_planandws.plan:#!/bin/bash # Description: List all possible values of `ws.plan` in the codebase. rg --type javascript --type typescript 'ws\.plan' -A 2
88-92: Ensure compliance with data privacy when usingtenantIdUsing
tenantIdasdistinctIdmight raise privacy concerns if it can be linked to an individual. Verify that this usage complies with your data privacy policies.Run the following script to review all usages of
tenantIdwith PostHog:#!/bin/bash # Description: Find all instances where `tenantId` is used with PostHogClient. rg --type javascript --type typescript 'PostHogClient\.capture\({\s*distinctId:\s*tenantId' -A 5
6-6: Ensure PostHogClient is properly configuredThe import statement adds
PostHogClient, but please verify that it is correctly configured and initialized to avoid any runtime issues.Run the following script to check the initialization of
PostHogClient:Verification successful
PostHogClient is properly configured and used
The verification process has confirmed that the PostHogClient is correctly set up and utilized in the codebase:
- The PostHogClient is defined and configured in
apps/dashboard/lib/posthog.tsas a singleton instance, with proper error handling for missing environment variables.- The
PostHogProvidercomponent inapps/dashboard/providers/PostHogProvider.tsxensures client-side initialization and provides additional tracking utilities.- In the file under review,
PostHogClientis correctly imported and used to capture an event (line 88).No further action is required regarding the PostHogClient configuration.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find where PostHogClient is initialized. rg --type javascript --type typescript 'PostHogClient\s*=\s*' -A 5Length of output: 256
Script:
#!/bin/bash # Search for any occurrences of PostHogClient in the codebase echo "Searching for PostHogClient occurrences:" rg "PostHogClient" --type typescript --type javascript echo "\nSearching for PostHogClient imports:" rg "import.*PostHogClient" --type typescript --type javascript echo "\nSearching for PostHog configuration files:" fd "posthog" --type fLength of output: 696
Script:
#!/bin/bash echo "Content of apps/dashboard/lib/posthog.ts:" cat apps/dashboard/lib/posthog.ts echo "\nContent of apps/dashboard/providers/PostHogProvider.tsx:" cat apps/dashboard/providers/PostHogProvider.tsx echo "\nChecking usage of PostHogClient in apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx:" grep -n "PostHogClient" apps/dashboard/app/\(app\)/settings/billing/stripe/success/page.tsxLength of output: 3515
2 participants
What does this PR do?
Previously, when a free user without a payment method was attempting to upgrade their plan, it would error out and notify the user that they must add add a payment method before upgrading.
This change will now catch a missing payment method (stripe association) when the user is upgrading their plan and redirect them Stripe to configure a payment method, and then continue the processing the plan change upon stripe success.
Edge case: When a user has already their stripe associated with Unkey, but has deleted their payment method from stripe, the user will get an error PRECONDITION FAILED when attempting to process their plan change. This is due to technical limitation due to where the stripe check lives (client-side), but it's still checked and guarded on the trpc route. The likelihood of a user configuring stripe and then deleting a payment is very slim.
New Package: PostHog-node to capture the plan change event from the server-side.
New Env variables for PostHog-node:
NEXT_PUBLIC_POSTHOG_HOSTNEXT_PUBLIC_POSTHOG_KEYFixes #2080
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Manual testing
Upgrade flow when user has added a payment method first is unchanged.
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Bug Fixes
Chores
posthog-nodefor future use.