[PULSE-35] Billing tab: cancel and change-plan UI and copy #19
Reference in New Issue
Block a user
No description provided.
Delete Branch "staging"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Work Item
PULSE-35
Summary
Changes
cancelSubscription,changePlan;SubscriptionDetailsincludescancel_at_period_end.lib/plans.ts—TRAFFIC_TIERS,PLAN_ID_SOLO, tier index/limit helpers for pricing and billing.Test Plan
Greptile Overview
Greptile Summary
This PR redesigns the Organization Settings → Billing tab UI and adds in-app subscription actions.
SubscriptionDetailswithcancel_at_period_endand introducescancelSubscriptionandchangePlanAPI helpers, plus reuse of checkout/portal flows.lib/plans.tsfor Solo plan traffic tiers and helper functions used by the change-plan modal.Key integration point is
components/settings/OrganizationSettings.tsx, which now orchestrates calls to the billing API helpers and redirects to Stripe when needed.Confidence Score: 3/5
invoices.length > 0. These should be corrected before merge.Important Files Changed
invoices.length > 0so loading/empty states never show.cancel_at_period_endto SubscriptionDetails and newcancelSubscription/changePlanAPI helpers; appears consistent with existing billingFetch usage.Sequence Diagram
3 files reviewed, 2 comments
Edit Code Review Agent Settings | Greptile
Incorrect subscription gating
hasActiveSubscriptionis currently defined ashas_payment_method && (active || trialing)(components/settings/OrganizationSettings.tsx:291), which will treat a trialing subscription with no payment method as not active. In that case the Change plan modal will incorrectly route users throughcreateCheckoutSession(components/settings/OrganizationSettings.tsx:303-307) instead of usingchangePlan, and the modal copy will say “You’ll start a new subscription.” This breaks the stated trial UX for orgs that are trialing before adding a card. Consider basing “existing subscription” onsubscription_statusalone (or on presence of a subscription id) rather thanhas_payment_method.Prompt To Fix With AI
@@ -797,2 +887,3 @@<span className={`px-2.5 py-0.5 rounded-full text-xs font-medium capitalize ${<div className="flex items-center gap-3"><span className={`px-2 py-0.5 rounded-full text-xs font-medium capitalize ${invoice.status === 'paid'Invoices never render while loading
The invoices section is gated by
invoices.length > 0(components/settings/OrganizationSettings.tsx:860), so when invoices are still loading (initially[]), the entire UI (including the spinner) is not rendered. This guarantees no loading state is shown and also hides the “No invoices found” state entirely. You likely want to render the section whenisLoadingInvoicesis true as well (or remove the outer length check).Prompt To Fix With AI
Issue: hasActiveSubscription required has_payment_method, so trialing orgs without a card were treated as having no subscription. They were sent to checkout instead of using the in-app change-plan flow.
Fix: hasActiveSubscription is now based only on subscription status:
subscription?.subscription_status === 'active' || subscription?.subscription_status === 'trialing'. No dependency on has_payment_method.
Why: Trials can exist before a payment method is added. Gating on status (or Stripe subscription id) matches that and keeps trialing users in the correct flow (change plan vs new checkout).
@@ -797,2 +887,3 @@<span className={`px-2.5 py-0.5 rounded-full text-xs font-medium capitalize ${<div className="flex items-center gap-3"><span className={`px-2 py-0.5 rounded-full text-xs font-medium capitalize ${invoice.status === 'paid'Issue: The invoices block was wrapped in invoices.length > 0, so while invoices was still [] (e.g. during load) the whole block (including spinner and empty state) was hidden.
Fix: The “Recent invoices” block always renders. The inner content is a three-way branch: show spinner when isLoadingInvoices, “No invoices found.” when loaded and invoices.length === 0, otherwise the list. The outer invoices.length > 0 guard was removed.
Why: Users need to see a loading state while invoices are fetched and an explicit empty state when there are none, not a missing section.
Greptile Overview
Greptile Summary
This PR redesigns the Organization Settings → Billing tab UI and adds in-app flows to cancel a subscription (at period end vs immediately) and change plan/tier (monthly vs yearly), backed by new billing client endpoints (
cancelSubscription,changePlan, andcreateCheckoutSession) plus shared tier definitions inlib/plans.ts.The main integration point is
components/settings/OrganizationSettings.tsx, which fetches subscription/invoice data and drives Stripe portal/checkout redirects or in-app API calls depending on subscription status. The API client changes are small and consistent with existingbillingFetchusage, andlib/plans.tscentralizes tier/limit mapping for the change-plan modal.Issues to fix before merge are localized to the billing UI: the invoices list currently contains invalid JSX around
invoices.map(...)and there is a loading-state edge case inloadMembersthat can leave the Members section stuck loading iforg_idis initially null and later hydrates.Confidence Score: 3/5
OrganizationSettings.tsxcurrently has invalid JSX in the invoices branch (invoices.map(...)not wrapped in{}) and aloadMembersearly-return that can preventisLoadingMembersfrom ever being cleared when org context hydrates after first render.Important Files Changed
Sequence Diagram
3 files reviewed, 2 comments
Edit Code Review Agent Settings | Greptile
@@ -714,2 +802,4 @@<div className="text-xs text-neutral-500 uppercase tracking-wider mb-1">Sites</div><div className="text-lg font-semibold text-neutral-900 dark:text-white">{typeof subscription.sites_count === 'number'? subscription.plan_id === 'solo'Invalid JSX mapping
The invoices branch renders
invoices.map(...)as a bare token inside JSX. This needs to be wrapped in curly braces (or wrapped in a fragment) to compile/render.Prompt To Fix With AI
Additional Comments (1)
components/settings/OrganizationSettings.tsxLoading state never clears
loadMembersreturns early whencurrentOrgIdis falsy, butisLoadingMembersonly gets set tofalsein thefinallyof the try/catch. IfcurrentOrgIdis temporarily null (e.g., auth context hydrates after first render),isLoadingMemberswill stay stucktruebecause the effect’selsebranch only runs whencurrentOrgIdchanges to falsy; it won’t run again when it becomes truthy. Consider ensuring the early-return path also clears the loading flag.Prompt To Fix With AI
@@ -714,2 +802,4 @@<div className="text-xs text-neutral-500 uppercase tracking-wider mb-1">Sites</div><div className="text-lg font-semibold text-neutral-900 dark:text-white">{typeof subscription.sites_count === 'number'? subscription.plan_id === 'solo'Issue: The third branch of the ternary was invoices.map(...) without being wrapped in a JS expression. In JSX, that can be treated as a bare token instead of an expression, so the map result may not render correctly.
Fix: Wrapped the map in curly braces so the third branch is { invoices.map((invoice) => ( ... )) }, and adjusted the closing to ))} so the block is valid JSX.
Why: In JSX, only expressions inside { } are evaluated; wrapping the map in braces makes it a proper expression and keeps behavior consistent across tools and React versions.
Greptile Overview
Greptile Summary
Billing UI redesign adds trial banner, cancel-at-period-end notice, modernized plan/usage card, and in-app Change plan and Cancel subscription flows. Copy clearly communicates trial auto-charge, access end dates, and 30-day data retention.
Key changes:
changePlanAPI (previous subscription gating issue fixed)cancelSubscription,changePlan,createCheckoutSessionlib/plans.tsfor tier definitions and limit helpersMinor issues found:
pageview_limitfallback logic treats 0 as falsy'en-US'localeConfidence Score: 4/5
Important Files Changed
Sequence Diagram
3 files reviewed, 3 comments
Edit Code Review Agent Settings | Greptile
@@ -257,6 +268,54 @@ export default function OrganizationSettings() {}Opening modal with stale tier selection if
pageview_limitis 0. Whensubscription?.pageview_limitis 0 or falsy (not just undefined), falls back to tier index 2 (100k). Consider checking for non-zero explicitly:subscription?.pageview_limit > 0.Prompt To Fix With AI
@@ -719,57 +809,61 @@ export default function OrganizationSettings() {</div>Hardcoded
'en-US'locale for currency formatting. This forces US-style formatting (e.g., $1,234.56) regardless of user's locale. Passundefinedor the user's locale to respect their regional formatting preferences.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
Loading state only shown for "Cancel at period end" button. When user clicks "Cancel immediately", both buttons should be disabled but only the first shows spinner. Consider passing which action is loading, or disable all buttons uniformly.
Prompt To Fix With AI
@@ -719,57 +809,61 @@ export default function OrganizationSettings() {</div>No change: keeping the hardcoded 'en-US' locale for currency formatting.
@@ -257,6 +268,54 @@ export default function OrganizationSettings() {}Issue: The condition subscription?.pageview_limit is falsy for 0, so when the limit was 0 we fell back to tier index 2 (100k) and opened the modal with the wrong tier.
Fix: Use an explicit check: subscription?.pageview_limit != null && subscription.pageview_limit > 0 before calling getTierIndexForLimit(subscription.pageview_limit); otherwise keep the default tier index 2.
Why: A real limit of 0 should not be treated as “unknown”; only when there’s no valid positive limit should we use the default.
Issue: Only the “Cancel at period end” button showed a loading state. For “Cancel immediately”, both actions were disabled but only the first showed a spinner, so it wasn’t clear which action was in progress.
Fix: Replaced the single isCanceling boolean with cancelLoadingAction: 'period_end' | 'immediate' | null. In handleCancelSubscription we set it to 'period_end' or 'immediate' at the start and clear it to null in finally. Both buttons are disabled when cancelLoadingAction != null. The first button uses isLoading={cancelLoadingAction === 'period_end'}, the second uses isLoading={cancelLoadingAction === 'immediate'}. The close and “Keep subscription” buttons use disabled={cancelLoadingAction != null}.
Why: The UI should show which action is in progress and keep all modal actions disabled until that request finishes.
Greptile Overview
Greptile Summary
This PR redesigns the billing tab with trial banners, cancel-at-period-end notices, and in-app change-plan and cancel-subscription flows. It adds API client functions for
cancelSubscriptionandchangePlan, and introduces a sharedlib/plans.tsmodule for traffic tier definitions.Key Changes:
changePlanAPI for existing onescancelSubscription,changePlan,createCheckoutSessionlib/plans.tswithTRAFFIC_TIERSand helper functionsCritical Issue:
Notes:
Confidence Score: 2/5
OrganizationSettings.tsx(mismatched braces in the invoices ternary) will cause compilation to fail. While the business logic appears sound and the developer has addressed previous issues, this syntax error is critical and prevents the code from running.components/settings/OrganizationSettings.tsxhas a JSX syntax error on line 874 that must be correctedImportant Files Changed
cancelSubscription,changePlan, andcreateCheckoutSessionAPI client functions with proper TypeScript typesSequence Diagram
3 files reviewed, 1 comment
Edit Code Review Agent Settings | Greptile
JSX syntax error: opening brace without matching close
Line 874 has
{beforeinvoices.map(...)but the closing is just)instead of)}. This will cause a compilation error.Prompt To Fix With AI
Issue: The third branch of the ternary was ( { invoices.map(...) } ). The { before invoices.map was closed by } on the same line as )), and the ) that closes the ternary branch was on the next line. That layout could be parsed as a missing )} and trigger a JSX syntax/compilation error.
Fix: Wrapped the third branch in a fragment so the structure is a single expression:
( <> { invoices.map((invoice) => ( ... )) } </> ).
The { ... } block is clearly closed by )}, and the ternary branch is closed by ) after </>.
Why: Using a fragment makes the ternary’s third branch one JSX node and keeps the expression’s {/} pairing obvious, which avoids the reported syntax/compilation issue.
Greptile Overview
Greptile Summary
This PR implements a comprehensive billing UI redesign with in-app subscription management. The changes include:
changePlanAPI, new subscriptions through checkoutcancelSubscriptionandchangePlanendpoints with proper TypeScript interfaceslib/plans.tswithTRAFFIC_TIERSconstants and tier index/limit conversion helpersThe implementation correctly handles the subscription status logic after addressing previous threading feedback. All previous issues around subscription gating, loading states, and invoice rendering have been resolved.
Confidence Score: 4/5
Important Files Changed
cancelSubscriptionandchangePlanendpoints with proper typingSequence Diagram