From 9e91d11027a39297dd00ba70ba6585de8f3e5f00 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 23 Aug 2026 04:09:16 -0700 Subject: [PATCH] fix(settings): keep billing header stable --- .claude/rules/sim-settings-pages.md | 8 +- .../settings/[section]/settings.tsx | 1 - .../components/billing/billing.test.tsx | 124 +++++++++++++----- .../settings/components/billing/billing.tsx | 31 +++-- .../settings/settings-header-shell.test.tsx | 10 +- .../components/settings/settings-panel.tsx | 19 ++- 6 files changed, 137 insertions(+), 56 deletions(-) diff --git a/.claude/rules/sim-settings-pages.md b/.claude/rules/sim-settings-pages.md index 2cff0545957..b65deabafe3 100644 --- a/.claude/rules/sim-settings-pages.md +++ b/.claude/rules/sim-settings-pages.md @@ -13,8 +13,9 @@ The Next.js `settings/[section]/layout.tsx` owns all settings page chrome via `SettingsHeaderShell` — a fixed header bar (a left back chip + right-aligned action chips), a scroll region, and a centered `max-w-[48rem]` content column led by a **title + description from navigation metadata**. The chrome stays mounted -across section navigation (it never re-renders or re-lays-out). Each section -renders through the **`SettingsPanel`** registrar +across section navigation. Its routed title and description are available before +the section body resolves. Each section renders through the **`SettingsPanel`** +registrar (`@/app/workspace/[workspaceId]/settings/components/settings-panel`), which feeds the shell its header data and renders only the section body. Sections supply **data**, never chrome. @@ -82,6 +83,9 @@ return ( `children` instead and omit the prop. - `title?` / `description?` — overrides for the nav-driven defaults. **Only** for a detail sub-view that needs a different heading; normal pages never pass these. + A top-level page's header identity must remain stable while its data loads: + never replace navigation metadata with client-fetched copy after first paint. + Put data-dependent context in the page body instead. - `scrollContainerRef?: React.Ref` — forwards a ref to the scroll region (e.g. programmatic scroll-to-bottom). diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx index 2eebb5a5b80..edb01ca9214 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx @@ -177,7 +177,6 @@ export function SettingsPage({ section }: SettingsPageProps) { )} diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx index a270afc4070..4b75e1440d0 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx @@ -174,6 +174,14 @@ vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-panel', () = ), })) +vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-empty-state', () => ({ + SettingsEmptyState: ({ children, tone }: { children: ReactNode; tone?: 'muted' | 'error' }) => ( +
+ {children} +
+ ), +})) + vi.mock( '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section', () => ({ @@ -269,13 +277,7 @@ describe('Billing payer scope', () => { it('uses the target organization DTO for annual, canceled, credit, cap, and link state', async () => { await act(async () => { - root.render( - - ) + root.render() }) expect(mockUseSubscriptionData).toHaveBeenCalledWith( @@ -290,9 +292,7 @@ describe('Billing payer scope', () => { container.querySelector('a[href="/workspace/organization-workspace/upgrade"]')?.textContent ).toBe('Explore organization plans') expect(container.textContent).toContain('Organization Max for Teams plan') - expect(container.textContent).toContain( - 'Target organization’s subscription governs Production.' - ) + expect(container.querySelector('main > p')).toBeNull() expect(container.textContent).toContain('billed annually') expect(container.textContent).toContain('Access until') expect(container.textContent).toContain('Subscription canceled') @@ -316,19 +316,45 @@ describe('Billing payer scope', () => { it('uses a guaranteed personal payer workspace for account upgrades', async () => { await act(async () => { - root.render() + root.render() }) expect( container.querySelector('a[href="/workspace/personal-workspace/upgrade"]')?.textContent ).toBe('Explore personal plans') expect(container.textContent).toContain('Personal Pro plan') - expect(container.textContent).toContain( - 'Your personal subscription governs Personal workspace.' - ) }) - it('does not show a governing subscription description for a free personal workspace', async () => { + it('does not override the route-owned header while billing transitions from loading to success', async () => { + mockPersonalQuery.current = { + data: undefined, + error: null, + isLoading: true, + refetch: vi.fn(), + } + + await act(async () => { + root.render() + }) + + expect(container.innerHTML).toBe('') + + mockPersonalQuery.current = { + data: { success: true, context: 'user', data: PERSONAL_DATA }, + error: null, + isLoading: false, + refetch: vi.fn(), + } + + await act(async () => { + root.render() + }) + + expect(container.textContent).toContain('Personal Pro plan') + expect(container.querySelector('main > p')).toBeNull() + }) + + it('does not add a dynamic header description for a free personal workspace', async () => { mockPersonalQuery.current = { data: { success: true, @@ -340,7 +366,7 @@ describe('Billing payer scope', () => { } await act(async () => { - root.render() + root.render() }) expect(container.textContent).toContain('Personal Free plan') @@ -368,13 +394,7 @@ describe('Billing payer scope', () => { } await act(async () => { - root.render( - - ) + root.render() }) expect(container.textContent).toContain('Organization Free plan') @@ -398,13 +418,7 @@ describe('Billing payer scope', () => { } await act(async () => { - root.render( - - ) + root.render() }) expect(container.textContent).toContain('Organization Max for Teams plan ended') @@ -415,4 +429,54 @@ describe('Billing payer scope', () => { container.querySelector('a[href="/workspace/organization-workspace/upgrade"]')?.textContent ).toBe('Explore organization plans') }) + + it('renders the canonical error state when the active billing query fails', async () => { + mockPersonalQuery.current = { + data: undefined, + error: new Error('Billing temporarily unavailable'), + isLoading: false, + refetch: vi.fn(), + } + + await act(async () => { + root.render() + }) + + const errorState = container.querySelector('[data-testid="settings-empty-state"]') + expect(errorState).toHaveAttribute('data-tone', 'error') + expect(errorState?.textContent).toBe('Billing temporarily unavailable') + }) + + it('keeps cached billing content visible when a background refresh fails', async () => { + mockPersonalQuery.current = { + data: { success: true, context: 'user', data: PERSONAL_DATA }, + error: new Error('Background refresh failed'), + isLoading: false, + refetch: vi.fn(), + } + + await act(async () => { + root.render() + }) + + expect(container.textContent).toContain('Personal Pro plan') + expect(container.querySelector('[data-testid="settings-empty-state"]')).toBeNull() + }) + + it('renders the canonical fallback error when billing completes without data', async () => { + mockOrganizationQuery.current = { + data: undefined, + error: null, + isLoading: false, + refetch: vi.fn(), + } + + await act(async () => { + root.render() + }) + + const errorState = container.querySelector('[data-testid="settings-empty-state"]') + expect(errorState).toHaveAttribute('data-tone', 'error') + expect(errorState?.textContent).toBe('Failed to load billing information') + }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx index 31febdeaecc..0cc86a7be47 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx @@ -46,6 +46,7 @@ import { getBaseUrl } from '@/lib/core/utils/urls' import { CreditUsageSection } from '@/app/workspace/[workspaceId]/settings/components/billing/components/credit-usage-section/credit-usage-section' import { UsageLimitField } from '@/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field' import { getSubscriptionPermissions } from '@/app/workspace/[workspaceId]/settings/components/billing/subscription-permissions' +import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { RESOURCE_ROW_ARROW_CLASSES } from '@/app/workspace/[workspaceId]/settings/components/settings-resource-row' import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' @@ -103,20 +104,15 @@ interface BillingProps { scope: 'account' | 'organization' organizationId?: string creditUsageHref?: string - governingWorkspaceName?: string } -export function Billing({ - scope, - organizationId, - creditUsageHref, - governingWorkspaceName, -}: BillingProps) { +export function Billing({ scope, organizationId, creditUsageHref }: BillingProps) { const router = useRouter() const isOrganizationScope = scope === 'organization' const { data: subscriptionData, + error: subscriptionError, isLoading: isSubscriptionLoading, refetch: refetchSubscription, } = useSubscriptionData({ @@ -127,6 +123,7 @@ export function Billing({ const { data: organizationBillingData, + error: organizationBillingError, isLoading: isOrgBillingLoading, refetch: refetchOrganizationBilling, } = useOrganizationBilling(billingOrganizationId || '', { enabled: isOrganizationScope }) @@ -157,6 +154,7 @@ export function Billing({ ? (organizationBilling?.subscriptionStatus ?? 'inactive') : (subscriptionData?.data?.status ?? 'inactive') const isLoading = isOrganizationScope ? isOrgBillingLoading : isSubscriptionLoading + const billingError = isOrganizationScope ? organizationBillingError : subscriptionError const subscription = { isFree: isFree(plan), @@ -403,7 +401,15 @@ export function Billing({ } if (isLoading) return null - if (isOrganizationScope ? !organizationBilling : !subscriptionData?.data) return null + if (isOrganizationScope ? !organizationBilling : !subscriptionData?.data) { + return ( + + + {getErrorMessage(billingError, 'Failed to load billing information')} + + + ) + } const planName = getDisplayPlanName(subscription.plan) const billingInterval = isOrganizationScope @@ -458,16 +464,9 @@ export function Billing({ const explorePlansLabel = isOrganizationScope ? 'Explore organization plans' : 'Explore personal plans' - const subscriptionOwner = isOrganizationScope - ? `${organizationBilling?.organizationName ?? 'The organization'}’s subscription` - : 'Your personal subscription' - const settingsDescription = - governingWorkspaceName && subscription.isPaid - ? `${subscriptionOwner} governs ${governingWorkspaceName}.` - : undefined return ( - +
diff --git a/apps/sim/components/settings/settings-header-shell.test.tsx b/apps/sim/components/settings/settings-header-shell.test.tsx index 40cd2a33bde..e2d588159ff 100644 --- a/apps/sim/components/settings/settings-header-shell.test.tsx +++ b/apps/sim/components/settings/settings-header-shell.test.tsx @@ -41,7 +41,7 @@ function renderHeader(actions: SettingsAction[]) { root.render( - +
@@ -152,7 +152,11 @@ describe('SettingsHeaderShell static meta', () => { it('yields to a body that registers its own header', () => { renderWithMeta( - +
) @@ -192,7 +196,7 @@ describe('SettingsHeaderShell static meta', () => { it('falls back to the meta title when the body unmounts mid-navigation', () => { renderWithMeta( - +
) diff --git a/apps/sim/components/settings/settings-panel.tsx b/apps/sim/components/settings/settings-panel.tsx index 5a688d142a3..da919ad88be 100644 --- a/apps/sim/components/settings/settings-panel.tsx +++ b/apps/sim/components/settings/settings-panel.tsx @@ -38,17 +38,28 @@ export function SettingsSectionProvider({ ) } -interface SettingsPanelProps { +interface SettingsPanelBaseProps { children?: ReactNode actions?: SettingsAction[] - back?: SettingsBackAction search?: SettingsHeaderSearch - title?: string - description?: string docsLink?: string scrollContainerRef?: Ref } +type SettingsPanelProps = SettingsPanelBaseProps & + ( + | { + back: SettingsBackAction + title?: string + description?: string + } + | { + back?: undefined + title?: never + description?: never + } + ) + export function SettingsPanel({ children, actions,