Open
Conversation
Standardized upgrade/paywall CTAs across the app to route through a shared billing upgrade URL helper with plan-aware query params (plan, planTier). This fixes cases where upgrade flows defaulted to “next plan” instead of the feature-required plan (notably Partners), and centralizes plan recommendation logic for consistent behavior across all upgrade entry points.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR centralizes billing upgrade URL construction by introducing reusable helper functions that replace hardcoded upgrade paths across the codebase. A new utility module exports Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/ui/modals/add-edit-tag-modal.tsx (1)
211-243:⚠️ Potential issue | 🟡 MinorRemove the unreachable ternary —
getNextPlanalways returns a valueTwo issues here:
Dead code:
getNextPlanreturnsPRO_PLANwhen the input is undefined/null and always returns a plan object otherwise. ThenextPlan ? ... : undefinedguard is unreachable and can be simplified.Missing
planTier: The recommendation only passesplan: nextPlan.name.toLowerCase(). However,getNextPlan()returns aPlanDetailsobject which does not have atierproperty — it has an optional nestedtiersobject for multi-tier plans. The proposed fix that accessesnextPlan.tierwon't work. Since tier information is not available fromgetNextPlan()alone, the simplest fix is to remove the ternary and pass the base plan name.Proposed fix
href={getBillingUpgradePath({ slug, - recommendation: nextPlan - ? { - plan: nextPlan.name.toLowerCase(), - } - : undefined, + recommendation: { + plan: nextPlan.name.toLowerCase().split(" ")[0], + }, })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/modals/add-edit-tag-modal.tsx` around lines 211 - 243, The ternary guard is unnecessary because getNextPlan(...) always returns a PlanDetails object, so remove the unreachable conditional and pass the recommendation directly; update the call building the recommendation to always supply { plan: nextPlan.name.toLowerCase() } (i.e., drop the "nextPlan ? ... : undefined" and any attempt to access a non-existent nextPlan.tier), referencing getNextPlan, nextPlan and getBillingUpgradePath to locate the code to change.
🧹 Nitpick comments (3)
apps/web/ui/links/links-toolbar.tsx (1)
110-175:slugused insideuseMemobut missing from its dependency array
getBillingUpgradePathForFeatureis called withslug(lines 128 and 144) inside thebulkActionsmemo, butslugis not listed in the dependency array[plan, conversionsEnabled, selectedLinks]. Whileslugis effectively stable for the lifetime of a mounted component, the omission technically violates the exhaustive-deps rule and will trigger a lint warning ifreact-hooks/exhaustive-depsis enabled.♻️ Proposed fix
- [plan, conversionsEnabled, selectedLinks], + [plan, conversionsEnabled, selectedLinks, slug],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/links/links-toolbar.tsx` around lines 110 - 175, The bulkActions useMemo references slug when calling getBillingUpgradePathForFeature but slug is missing from its dependency array; update the dependency array for the useMemo that defines bulkActions to include slug (in addition to plan, conversionsEnabled, selectedLinks) so the memo properly invalidates when slug changes and satisfies exhaustive-deps; locate the useMemo definition of bulkActions in links-toolbar.tsx to apply this change.apps/web/ui/links/link-builder/link-preview.tsx (1)
124-128:target="_blank"will open the in-app upgrade page in a new tab whenslugis present.When
slugis defined,getBillingUpgradePathForFeaturereturns a relative path (/${slug}/settings/billing/upgrade?plan=pro). Combined withtarget="_blank"on line 128, this opens an in-app settings page in a new tab, which may not be the intended UX. If this was already the behavior before this PR, it could be addressed separately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/links/link-builder/link-preview.tsx` around lines 124 - 128, The link currently sets target="_blank" unconditionally which opens relative in-app paths returned by getBillingUpgradePathForFeature({ slug, feature: "pro" }) in a new tab; change LinkPreview so it computes href = getBillingUpgradePathForFeature(...) and only sets target="_blank" when the href is an external URL (e.g. startsWith('http')) or when slug is absent — for relative paths (startsWith('/')) leave target undefined (open in same tab) to preserve intended in-app navigation.apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/billing/plan-usage.tsx (1)
71-71: Remove redundantnextPlanrecomputation; use the value fromuseWorkspace()hook instead.Line 71 recomputes
nextPlanviagetNextPlan(plan), whileupgrade-banner.tsxreads it directly from theuseWorkspace()hook. Sinceplancomes from the same hook, this is redundant. Using the hook'snextPlankeeps the logic centralized and consistent across components.Change:
const nextPlan = getNextPlan(plan); // Remove this lineThen use
nextPlanfrom the hook destructuring (line 47-70). This aligns with howupgrade-banner.tsxhandles it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/settings/billing/plan-usage.tsx at line 71, Remove the redundant recomputation of nextPlan: eliminate the local call getNextPlan(plan) and instead use the nextPlan value provided by the useWorkspace() hook (the same destructured nextPlan used in upgrade-banner.tsx). Update any references in this file that currently read the locally computed nextPlan to reference the hook's nextPlan so logic remains centralized and consistent with upgrade-banner.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/ui/modals/add-edit-tag-modal.tsx`:
- Around line 211-243: The ternary guard is unnecessary because getNextPlan(...)
always returns a PlanDetails object, so remove the unreachable conditional and
pass the recommendation directly; update the call building the recommendation to
always supply { plan: nextPlan.name.toLowerCase() } (i.e., drop the "nextPlan ?
... : undefined" and any attempt to access a non-existent nextPlan.tier),
referencing getNextPlan, nextPlan and getBillingUpgradePath to locate the code
to change.
---
Nitpick comments:
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/settings/billing/plan-usage.tsx:
- Line 71: Remove the redundant recomputation of nextPlan: eliminate the local
call getNextPlan(plan) and instead use the nextPlan value provided by the
useWorkspace() hook (the same destructured nextPlan used in upgrade-banner.tsx).
Update any references in this file that currently read the locally computed
nextPlan to reference the hook's nextPlan so logic remains centralized and
consistent with upgrade-banner.tsx.
In `@apps/web/ui/links/link-builder/link-preview.tsx`:
- Around line 124-128: The link currently sets target="_blank" unconditionally
which opens relative in-app paths returned by getBillingUpgradePathForFeature({
slug, feature: "pro" }) in a new tab; change LinkPreview so it computes href =
getBillingUpgradePathForFeature(...) and only sets target="_blank" when the href
is an external URL (e.g. startsWith('http')) or when slug is absent — for
relative paths (startsWith('/')) leave target undefined (open in same tab) to
preserve intended in-app navigation.
In `@apps/web/ui/links/links-toolbar.tsx`:
- Around line 110-175: The bulkActions useMemo references slug when calling
getBillingUpgradePathForFeature but slug is missing from its dependency array;
update the dependency array for the useMemo that defines bulkActions to include
slug (in addition to plan, conversionsEnabled, selectedLinks) so the memo
properly invalidates when slug changes and satisfies exhaustive-deps; locate the
useMemo definition of bulkActions in links-toolbar.tsx to apply this change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/confirm-create-bounty-modal.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/partners-upgrade-cta.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/analytics/conversion-tracking-toggle.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/billing/plan-usage.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/domains/default/page-client.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/domains/email/page-client.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/integrations/[integrationSlug]/page-client.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/webhooks/page-client.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/links/page-client.tsxapps/web/lib/billing/upgrade-url.tsapps/web/lib/integrations/segment/ui/set-write-key.tsxapps/web/ui/analytics/analytics-loading-spinner.tsxapps/web/ui/analytics/chart-section.tsxapps/web/ui/analytics/events/events-export-button.tsxapps/web/ui/analytics/events/index.tsxapps/web/ui/analytics/toggle.tsxapps/web/ui/customers/customers-table/customers-table.tsxapps/web/ui/domains/register-domain-form.tsxapps/web/ui/folders/add-folder-form.tsxapps/web/ui/folders/edit-folder-sheet.tsxapps/web/ui/folders/folder-dropdown.tsxapps/web/ui/layout/sidebar/sidebar-usage.tsxapps/web/ui/layout/upgrade-banner.tsxapps/web/ui/links/link-builder/conversion-tracking-toggle.tsxapps/web/ui/links/link-builder/link-preview.tsxapps/web/ui/links/links-toolbar.tsxapps/web/ui/links/short-link-input.tsxapps/web/ui/modals/add-edit-domain-modal.tsxapps/web/ui/modals/add-edit-tag-modal.tsxapps/web/ui/modals/add-folder-modal.tsxapps/web/ui/modals/link-builder/index.tsxapps/web/ui/modals/link-qr-modal.tsxapps/web/ui/modals/manage-usage-modal.tsxapps/web/ui/partners/confirm-payouts-sheet.tsxapps/web/ui/partners/rewards/add-edit-reward-sheet.tsxapps/web/ui/shared/upgrade-required-toast.tsxapps/web/ui/workspaces/create-workspace-button.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Standardized upgrade/paywall CTAs across the app to route through a shared billing upgrade URL helper, fixing cases where Billing Upgrade defaulted to the next plan instead of the feature-required plan.
upgrade-url.tsis the central URL builder:getBillingUpgradePath(...)creates/${slug}/settings/billing/upgradeand appends optionalplan,planTier, and modal params.getBillingUpgradePathForFeature(...)maps gated features (e.g. partners/business/pro/advanced) to their recommended upgrade target so call sites can use a single feature key instead of hardcoded links/query strings.This makes upgrade routing consistent and easier to maintain as paywalls grow.
Eg.
dub.co/acme/settings/billing/upgrade?plan=proSummary by CodeRabbit
Release Notes