Skip to content

Commit c63cc8e

Browse files
authored
feat(shared,clerk-js): Accept enabled option in Billing hooks (#7202)
1 parent 50e630a commit c63cc8e

File tree

10 files changed

+111
-42
lines changed

10 files changed

+111
-42
lines changed

.changeset/heavy-suns-divide.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
---
4+
5+
Fixes a bug where billing hooks would attempt to fetch billing information for an organization member with insufficient permissions, resulting in a 403 error.

.changeset/thin-swans-punch.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/shared': minor
3+
---
4+
5+
Billing hooks now accept a `{ enabled: boolean }` option, that controls the whether or not a request will fire.

packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationProfile.test.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ describe('OrganizationProfile', () => {
8585

8686
render(<OrganizationProfile />, { wrapper });
8787
await waitFor(() => expect(screen.queryByText('Billing')).toBeNull());
88-
expect(fixtures.clerk.billing.getSubscription).toHaveBeenCalled();
89-
expect(fixtures.clerk.billing.getStatements).toHaveBeenCalled();
88+
expect(fixtures.clerk.billing.getSubscription).not.toHaveBeenCalled();
89+
expect(fixtures.clerk.billing.getStatements).not.toHaveBeenCalled();
9090
});
9191

9292
it('does not include Billing when missing billing permission even with paid plans', async () => {
@@ -109,9 +109,8 @@ describe('OrganizationProfile', () => {
109109
render(<OrganizationProfile />, { wrapper });
110110
await waitFor(() => expect(screen.queryByText('Billing')).toBeNull());
111111

112-
// TODO(@RQ_MIGRATION): Offer a way to disable these, because they fire unnecessary requests.
113-
expect(fixtures.clerk.billing.getSubscription).toHaveBeenCalled();
114-
expect(fixtures.clerk.billing.getStatements).toHaveBeenCalled();
112+
expect(fixtures.clerk.billing.getSubscription).not.toHaveBeenCalled();
113+
expect(fixtures.clerk.billing.getStatements).not.toHaveBeenCalled();
115114
});
116115
it('does not include Billing when organization billing is disabled', async () => {
117116
const { wrapper, fixtures } = await createFixtures(f => {

packages/clerk-js/src/ui/components/PricingTable/__tests__/PricingTable.test.tsx

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,11 @@ describe('PricingTable - plans visibility', () => {
398398
// Should show plans when signed out
399399
expect(getByRole('heading', { name: 'Test Plan' })).toBeVisible();
400400
});
401+
402+
// Ensure API args reflect user context by default
403+
expect(fixtures.clerk.billing.getPlans).toHaveBeenCalledWith(expect.objectContaining({ for: 'user' }));
404+
// Signed-out users should not fetch subscriptions
405+
expect(fixtures.clerk.billing.getSubscription).not.toHaveBeenCalled();
401406
});
402407

403408
it('shows no plans when user is signed in but subscription is null', async () => {
@@ -517,7 +522,15 @@ describe('PricingTable - plans visibility', () => {
517522
const { wrapper, fixtures, props } = await createFixtures(f => {
518523
f.withBilling();
519524
f.withOrganizations();
520-
f.withUser({ email_addresses: ['[email protected]'], organization_memberships: ['Org1'] });
525+
f.withUser({
526+
email_addresses: ['[email protected]'],
527+
organization_memberships: [
528+
{
529+
name: 'Org1',
530+
permissions: ['org:sys_billing:manage'],
531+
},
532+
],
533+
});
521534
});
522535

523536
// Set legacy prop via context provider
@@ -572,7 +585,15 @@ describe('PricingTable - plans visibility', () => {
572585
const { wrapper, fixtures, props } = await createFixtures(f => {
573586
f.withBilling();
574587
f.withOrganizations();
575-
f.withUser({ email_addresses: ['[email protected]'], organization_memberships: ['Org1'] });
588+
f.withUser({
589+
email_addresses: ['[email protected]'],
590+
organization_memberships: [
591+
{
592+
name: 'Org1',
593+
permissions: ['org:sys_billing:read'],
594+
},
595+
],
596+
});
576597
});
577598

578599
// Set new prop via context provider

packages/clerk-js/src/ui/contexts/components/Plans.tsx

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
__experimental_useStatements,
66
__experimental_useSubscription,
77
useClerk,
8+
useOrganization,
89
useSession,
910
} from '@clerk/shared/react';
1011
import type {
@@ -15,6 +16,7 @@ import type {
1516
} from '@clerk/shared/types';
1617
import { useCallback, useMemo } from 'react';
1718

19+
import { useProtect } from '@/ui/common/Gate';
1820
import { getClosestProfileScrollBox } from '@/ui/utils/getClosestProfileScrollBox';
1921

2022
import type { LocalizationKey } from '../../localization';
@@ -28,44 +30,42 @@ export function normalizeFormatted(formatted: string) {
2830
return formatted.endsWith('.00') ? formatted.slice(0, -3) : formatted;
2931
}
3032

31-
// TODO(@COMMERCE): Rename payment sources to payment methods at the API level
32-
export const usePaymentMethods = () => {
33+
const useBillingHookParams = () => {
3334
const subscriberType = useSubscriberTypeContext();
34-
return __experimental_usePaymentMethods({
35+
const { organization } = useOrganization();
36+
const allowBillingRoutes = useProtect(
37+
has =>
38+
has({
39+
permission: 'org:sys_billing:read',
40+
}) || has({ permission: 'org:sys_billing:manage' }),
41+
);
42+
43+
return {
3544
for: subscriberType,
36-
initialPage: 1,
37-
pageSize: 10,
3845
keepPreviousData: true,
39-
});
46+
// If the user is in an organization, only fetch billing data if they have the necessary permissions
47+
enabled: subscriberType === 'organization' ? Boolean(organization) && allowBillingRoutes : true,
48+
};
49+
};
50+
51+
export const usePaymentMethods = () => {
52+
const params = useBillingHookParams();
53+
return __experimental_usePaymentMethods(params);
4054
};
4155

4256
export const usePaymentAttempts = () => {
43-
const subscriberType = useSubscriberTypeContext();
44-
return __experimental_usePaymentAttempts({
45-
for: subscriberType,
46-
initialPage: 1,
47-
pageSize: 10,
48-
keepPreviousData: true,
49-
});
57+
const params = useBillingHookParams();
58+
return __experimental_usePaymentAttempts(params);
5059
};
5160

52-
export const useStatements = (params?: { mode: 'cache' }) => {
53-
const subscriberType = useSubscriberTypeContext();
54-
return __experimental_useStatements({
55-
for: subscriberType,
56-
initialPage: 1,
57-
pageSize: 10,
58-
keepPreviousData: true,
59-
__experimental_mode: params?.mode,
60-
});
61+
export const useStatements = (externalParams?: { mode: 'cache' }) => {
62+
const params = useBillingHookParams();
63+
return __experimental_useStatements({ ...params, __experimental_mode: externalParams?.mode });
6164
};
6265

6366
export const useSubscription = () => {
64-
const subscriberType = useSubscriberTypeContext();
65-
const subscription = __experimental_useSubscription({
66-
for: subscriberType,
67-
keepPreviousData: true,
68-
});
67+
const params = useBillingHookParams();
68+
const subscription = __experimental_useSubscription(params);
6969
const subscriptionItems = useMemo(
7070
() => subscription.data?.subscriptionItems || [],
7171
[subscription.data?.subscriptionItems],
@@ -85,6 +85,7 @@ export const usePlans = (params?: { mode: 'cache' }) => {
8585
initialPage: 1,
8686
pageSize: 50,
8787
keepPreviousData: true,
88+
enabled: true,
8889
__experimental_mode: params?.mode,
8990
});
9091
};

packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,26 @@ describe('createBillingPaginatedHook', () => {
9898
expect(result.current.isFetching).toBe(false);
9999
});
100100

101+
it('does not fetch when enabled is false', () => {
102+
const { result } = renderHook(() => useDummyAuth({ initialPage: 1, pageSize: 4, enabled: false }), { wrapper });
103+
104+
expect(useFetcherMock).toHaveBeenCalledWith('user');
105+
106+
expect(fetcherMock).not.toHaveBeenCalled();
107+
expect(result.current.isLoading).toBe(false);
108+
expect(result.current.isFetching).toBe(false);
109+
expect(result.current.data).toEqual([]);
110+
});
111+
112+
it('fetches when enabled is true', async () => {
113+
const { result } = renderHook(() => useDummyAuth({ initialPage: 1, pageSize: 4, enabled: true }), { wrapper });
114+
115+
await waitFor(() => expect(result.current.isLoading).toBe(false));
116+
expect(useFetcherMock).toHaveBeenCalledWith('user');
117+
expect(fetcherMock).toHaveBeenCalled();
118+
expect(fetcherMock.mock.calls[0][0]).toStrictEqual({ initialPage: 1, pageSize: 4 });
119+
});
120+
101121
it('authenticated hook: does not fetch when user is null', () => {
102122
mockUser = null;
103123

packages/shared/src/react/hooks/createBillingPaginatedHook.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,23 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
4343
useFetcher,
4444
options,
4545
}: BillingHookConfig<TResource, TParams>) {
46-
type HookParams = PaginatedHookConfig<PagesOrInfiniteOptions> & {
46+
type HookParams = PaginatedHookConfig<
47+
PagesOrInfiniteOptions & {
48+
/**
49+
* If `true`, a request will be triggered when the hook is mounted.
50+
*
51+
* @default true
52+
*/
53+
enabled?: boolean;
54+
}
55+
> & {
4756
for?: ForPayerType;
4857
};
4958

5059
return function useBillingHook<T extends HookParams>(
5160
params?: T,
5261
): PaginatedResources<TResource, T extends { infinite: true } ? true : false> {
53-
const { for: _for, ...paginationParams } = params || ({} as Partial<T>);
62+
const { for: _for, enabled: externalEnabled, ...paginationParams } = params || ({} as Partial<T>);
5463

5564
const safeFor = _for || 'user';
5665

@@ -90,7 +99,7 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
9099
? environment?.commerceSettings.billing.organization.enabled
91100
: environment?.commerceSettings.billing.user.enabled;
92101

93-
const isEnabled = !!hookParams && clerk.loaded && !!billingEnabled;
102+
const isEnabled = !!hookParams && clerk.loaded && !!billingEnabled && (externalEnabled ?? true);
94103

95104
const result = usePagesOrInfinite<TParams, ClerkPaginatedResponse<TResource>>(
96105
(hookParams || {}) as TParams,

packages/shared/src/react/hooks/useSubscription.rq.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
} from '../contexts';
1313
import type { SubscriptionResult, UseSubscriptionParams } from './useSubscription.types';
1414

15-
const hookName = 'useSubscription';
15+
const HOOK_NAME = 'useSubscription';
1616

1717
/**
1818
* This is the new implementation of useSubscription using React Query.
@@ -21,7 +21,7 @@ const hookName = 'useSubscription';
2121
* @internal
2222
*/
2323
export function useSubscription(params?: UseSubscriptionParams): SubscriptionResult {
24-
useAssertWrappedByClerkProvider(hookName);
24+
useAssertWrappedByClerkProvider(HOOK_NAME);
2525

2626
const clerk = useClerkInstanceContext();
2727
const user = useUserContext();
@@ -30,7 +30,7 @@ export function useSubscription(params?: UseSubscriptionParams): SubscriptionRes
3030
// @ts-expect-error `__unstable__environment` is not typed
3131
const environment = clerk.__unstable__environment as unknown as EnvironmentResource | null | undefined;
3232

33-
clerk.telemetry?.record(eventMethodCalled(hookName));
33+
clerk.telemetry?.record(eventMethodCalled(HOOK_NAME));
3434

3535
const isOrganization = params?.for === 'organization';
3636
const billingEnabled = isOrganization
@@ -49,14 +49,16 @@ export function useSubscription(params?: UseSubscriptionParams): SubscriptionRes
4949
];
5050
}, [user?.id, isOrganization, organization?.id]);
5151

52+
const queriesEnabled = Boolean(user?.id && billingEnabled) && (params?.enabled ?? true);
53+
5254
const query = useClerkQuery({
5355
queryKey,
5456
queryFn: ({ queryKey }) => {
5557
const obj = queryKey[1] as { args: { orgId?: string } };
5658
return clerk.billing.getSubscription(obj.args);
5759
},
5860
staleTime: 1_000 * 60,
59-
enabled: Boolean(user?.id && billingEnabled) && ((params as any)?.enabled ?? true),
61+
enabled: queriesEnabled,
6062
// TODO(@RQ_MIGRATION): Add support for keepPreviousData
6163
});
6264

packages/shared/src/react/hooks/useSubscription.swr.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ export function useSubscription(params?: UseSubscriptionParams): SubscriptionRes
3535
const billingEnabled = isOrganization
3636
? environment?.commerceSettings.billing.organization.enabled
3737
: environment?.commerceSettings.billing.user.enabled;
38+
const isEnabled = (params?.enabled ?? true) && billingEnabled;
3839

3940
const swr = useSWR(
40-
billingEnabled
41+
isEnabled
4142
? {
4243
type: 'commerce-subscription',
4344
userId: user?.id,

packages/shared/src/react/hooks/useSubscription.types.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ export type UseSubscriptionParams = {
77
* Defaults to false.
88
*/
99
keepPreviousData?: boolean;
10+
/**
11+
* If `true`, a request will be triggered when the hook is mounted.
12+
*
13+
* @default true
14+
*/
15+
enabled?: boolean;
1016
};
1117

1218
export type SubscriptionResult = {

0 commit comments

Comments
 (0)