Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/heavy-suns-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Fixes a bug where billing hooks would attempt to fetch billing information for an organization member with insufficient permissions, resulting in a 403 error.
5 changes: 5 additions & 0 deletions .changeset/thin-swans-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/shared': minor
---

Billing hooks now accept a `{ enabled: boolean }` option, that controls the whether or not a request will fire.
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ describe('OrganizationProfile', () => {

render(<OrganizationProfile />, { wrapper });
await waitFor(() => expect(screen.queryByText('Billing')).toBeNull());
expect(fixtures.clerk.billing.getSubscription).toHaveBeenCalled();
expect(fixtures.clerk.billing.getStatements).toHaveBeenCalled();
expect(fixtures.clerk.billing.getSubscription).not.toHaveBeenCalled();
expect(fixtures.clerk.billing.getStatements).not.toHaveBeenCalled();
});

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

// TODO(@RQ_MIGRATION): Offer a way to disable these, because they fire unnecessary requests.
expect(fixtures.clerk.billing.getSubscription).toHaveBeenCalled();
expect(fixtures.clerk.billing.getStatements).toHaveBeenCalled();
expect(fixtures.clerk.billing.getSubscription).not.toHaveBeenCalled();
expect(fixtures.clerk.billing.getStatements).not.toHaveBeenCalled();
});
it('does not include Billing when organization billing is disabled', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
Expand Down
54 changes: 27 additions & 27 deletions packages/clerk-js/src/ui/contexts/components/Plans.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
__experimental_useStatements,
__experimental_useSubscription,
useClerk,
useOrganization,
useSession,
} from '@clerk/shared/react';
import type {
Expand All @@ -15,6 +16,7 @@ import type {
} from '@clerk/shared/types';
import { useCallback, useMemo } from 'react';

import { useProtect } from '@/ui/common/Gate';
import { getClosestProfileScrollBox } from '@/ui/utils/getClosestProfileScrollBox';

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

// TODO(@COMMERCE): Rename payment sources to payment methods at the API level
export const usePaymentMethods = () => {
const useBillingHookParams = () => {
const subscriberType = useSubscriberTypeContext();
return __experimental_usePaymentMethods({
const { organization } = useOrganization();
const allowBillingRoutes = useProtect(
has =>
has({
permission: 'org:sys_billing:read',
}) || has({ permission: 'org:sys_billing:manage' }),
);

return {
for: subscriberType,
initialPage: 1,
pageSize: 10,
keepPreviousData: true,
});
// If the user is in an organization, only fetch billing data if they have the necessary permissions
enabled: organization ? allowBillingRoutes : true,
};
};

export const usePaymentMethods = () => {
const params = useBillingHookParams();
return __experimental_usePaymentMethods(params);
};

export const usePaymentAttempts = () => {
const subscriberType = useSubscriberTypeContext();
return __experimental_usePaymentAttempts({
for: subscriberType,
initialPage: 1,
pageSize: 10,
keepPreviousData: true,
});
const params = useBillingHookParams();
return __experimental_usePaymentAttempts(params);
};

export const useStatements = (params?: { mode: 'cache' }) => {
const subscriberType = useSubscriberTypeContext();
return __experimental_useStatements({
for: subscriberType,
initialPage: 1,
pageSize: 10,
keepPreviousData: true,
__experimental_mode: params?.mode,
});
export const useStatements = (externalParams?: { mode: 'cache' }) => {
const params = useBillingHookParams();
return __experimental_useStatements({ ...params, __experimental_mode: externalParams?.mode });
};

export const useSubscription = () => {
const subscriberType = useSubscriberTypeContext();
const subscription = __experimental_useSubscription({
for: subscriberType,
keepPreviousData: true,
});
const params = useBillingHookParams();
const subscription = __experimental_useSubscription(params);
const subscriptionItems = useMemo(
() => subscription.data?.subscriptionItems || [],
[subscription.data?.subscriptionItems],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,26 @@ describe('createBillingPaginatedHook', () => {
expect(result.current.isFetching).toBe(false);
});

it('does not fetch when enabled is false', () => {
const { result } = renderHook(() => useDummyAuth({ initialPage: 1, pageSize: 4, enabled: false }), { wrapper });

expect(useFetcherMock).toHaveBeenCalledWith('user');

expect(fetcherMock).not.toHaveBeenCalled();
expect(result.current.isLoading).toBe(false);
expect(result.current.isFetching).toBe(false);
expect(result.current.data).toEqual([]);
});

it('fetches when enabled is true', async () => {
const { result } = renderHook(() => useDummyAuth({ initialPage: 1, pageSize: 4, enabled: true }), { wrapper });

await waitFor(() => expect(result.current.isLoading).toBe(false));
expect(useFetcherMock).toHaveBeenCalledWith('user');
expect(fetcherMock).toHaveBeenCalled();
expect(fetcherMock.mock.calls[0][0]).toStrictEqual({ initialPage: 1, pageSize: 4 });
});

it('authenticated hook: does not fetch when user is null', () => {
mockUser = null;

Expand Down
15 changes: 12 additions & 3 deletions packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,23 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
useFetcher,
options,
}: BillingHookConfig<TResource, TParams>) {
type HookParams = PaginatedHookConfig<PagesOrInfiniteOptions> & {
type HookParams = PaginatedHookConfig<
PagesOrInfiniteOptions & {
/**
* If `true`, a request will be triggered when the hook is mounted.
*
* @default true
*/
enabled?: boolean;
}
> & {
for?: ForPayerType;
};

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

const safeFor = _for || 'user';

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

const isEnabled = !!hookParams && clerk.loaded && !!billingEnabled;
const isEnabled = !!hookParams && clerk.loaded && !!billingEnabled && (externalEnabled ?? true);

const result = usePagesOrInfinite<TParams, ClerkPaginatedResponse<TResource>>(
(hookParams || {}) as TParams,
Expand Down
10 changes: 6 additions & 4 deletions packages/shared/src/react/hooks/useSubscription.rq.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from '../contexts';
import type { SubscriptionResult, UseSubscriptionParams } from './useSubscription.types';

const hookName = 'useSubscription';
const HOOK_NAME = 'useSubscription';

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

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

clerk.telemetry?.record(eventMethodCalled(hookName));
clerk.telemetry?.record(eventMethodCalled(HOOK_NAME));

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

const queriesEnabled = Boolean(user?.id && billingEnabled) && (params?.enabled ?? true);

const query = useClerkQuery({
queryKey,
queryFn: ({ queryKey }) => {
const obj = queryKey[1] as { args: { orgId?: string } };
return clerk.billing.getSubscription(obj.args);
},
staleTime: 1_000 * 60,
enabled: Boolean(user?.id && billingEnabled) && ((params as any)?.enabled ?? true),
enabled: queriesEnabled,
// TODO(@RQ_MIGRATION): Add support for keepPreviousData
});

Expand Down
3 changes: 2 additions & 1 deletion packages/shared/src/react/hooks/useSubscription.swr.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ export function useSubscription(params?: UseSubscriptionParams): SubscriptionRes
const billingEnabled = isOrganization
? environment?.commerceSettings.billing.organization.enabled
: environment?.commerceSettings.billing.user.enabled;
const isEnabled = (params?.enabled ?? true) && billingEnabled;

const swr = useSWR(
billingEnabled
isEnabled
? {
type: 'commerce-subscription',
userId: user?.id,
Expand Down
6 changes: 6 additions & 0 deletions packages/shared/src/react/hooks/useSubscription.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ export type UseSubscriptionParams = {
* Defaults to false.
*/
keepPreviousData?: boolean;
/**
* If `true`, a request will be triggered when the hook is mounted.
*
* @default true
*/
enabled?: boolean;
};

export type SubscriptionResult = {
Expand Down
Loading