diff --git a/.changeset/quiet-kangaroos-heal.md b/.changeset/quiet-kangaroos-heal.md new file mode 100644 index 00000000000..bc526d86bf0 --- /dev/null +++ b/.changeset/quiet-kangaroos-heal.md @@ -0,0 +1,5 @@ +--- +"@clerk/ui": patch +--- + +Use `user.organizationMemberships` from the already-loaded user object to populate the org select in the OAuth consent screen, avoiding a redundant memberships fetch. diff --git a/packages/ui/src/components/OAuthConsent/OAuthConsent.tsx b/packages/ui/src/components/OAuthConsent/OAuthConsent.tsx index 7ecc740f1b4..394fd953833 100644 --- a/packages/ui/src/components/OAuthConsent/OAuthConsent.tsx +++ b/packages/ui/src/components/OAuthConsent/OAuthConsent.tsx @@ -1,4 +1,4 @@ -import { useClerk, useOAuthConsent, useOrganizationList, useUser } from '@clerk/shared/react'; +import { useClerk, useOAuthConsent, useUser } from '@clerk/shared/react'; import { useState } from 'react'; import { useEnvironment, useOAuthConsentContext, withCoreUserGuard } from '@/ui/contexts'; @@ -22,7 +22,6 @@ import { ListGroupItemLabel, } from './ListGroup'; import { LogoGroup, LogoGroupIcon, LogoGroupItem, LogoGroupSeparator } from './LogoGroup'; -import type { OrgOption } from './OrgSelect'; import { OrgSelect } from './OrgSelect'; import { getForwardedParams, getOAuthConsentFromSearch, getRedirectDisplay, getRedirectUriFromSearch } from './utils'; @@ -32,16 +31,20 @@ function _OAuthConsent() { const ctx = useOAuthConsentContext(); const clerk = useClerk(); const { user } = useUser(); - const { applicationName, logoImageUrl } = useEnvironment().displayConfig; + const { + displayConfig: { applicationName, logoImageUrl }, + organizationSettings, + } = useEnvironment(); const [isUriModalOpen, setIsUriModalOpen] = useState(false); - const { isLoaded: isMembershipsLoaded, userMemberships } = useOrganizationList({ - userMemberships: ctx.enableOrgSelection ? { infinite: true } : undefined, - }); - const orgOptions: OrgOption[] = (userMemberships.data ?? []).map(m => ({ - value: m.organization.id, - label: m.organization.name, - logoUrl: m.organization.imageUrl, - })); + + const orgSelectionEnabled = !!(ctx.enableOrgSelection && organizationSettings.enabled); + const orgOptions = orgSelectionEnabled + ? (user?.organizationMemberships ?? []).map(m => ({ + value: m.organization.id, + label: m.organization.name, + logoUrl: m.organization.imageUrl, + })) + : []; const [selectedOrg, setSelectedOrg] = useState(null); const effectiveOrg = selectedOrg ?? orgOptions[0]?.value ?? null; @@ -114,17 +117,6 @@ function _OAuthConsent() { } } - if (ctx.enableOrgSelection && (!isMembershipsLoaded || userMemberships.isLoading)) { - return ( - - - - - - - ); - } - const actionUrl = clerk.oauthApplication.buildConsentActionUrl({ clientId: oauthClientId }); const forwardedParams = getForwardedParams(); @@ -221,13 +213,11 @@ function _OAuthConsent() { })} /> - {ctx.enableOrgSelection && orgOptions.length > 0 && effectiveOrg && ( + {orgSelectionEnabled && orgOptions.length > 0 && effectiveOrg && ( )} @@ -308,7 +298,7 @@ function _OAuthConsent() { value={value} /> ))} - {!hasContextCallbacks && ctx.enableOrgSelection && effectiveOrg && ( + {!hasContextCallbacks && orgSelectionEnabled && effectiveOrg && ( void; - hasMore?: boolean; - onLoadMore?: () => void; }; -export function OrgSelect({ options, value, onChange, hasMore, onLoadMore }: OrgSelectProps) { +export function OrgSelect({ options, value, onChange }: OrgSelectProps) { const buttonRef = useRef(null); const selected = options.find(option => option.value === value); - const { ref: loadMoreRef } = useInView({ - threshold: 0, - onChange: inView => { - if (inView && hasMore) { - onLoadMore?.(); - } - }, - }); return ( ); } diff --git a/packages/ui/src/components/OAuthConsent/__tests__/OAuthConsent.test.tsx b/packages/ui/src/components/OAuthConsent/__tests__/OAuthConsent.test.tsx index e871fd41fb1..c641e0c69ba 100644 --- a/packages/ui/src/components/OAuthConsent/__tests__/OAuthConsent.test.tsx +++ b/packages/ui/src/components/OAuthConsent/__tests__/OAuthConsent.test.tsx @@ -1,4 +1,3 @@ -import { useOrganizationList } from '@clerk/shared/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { bindCreateFixtures } from '@/test/create-fixtures'; @@ -6,30 +5,6 @@ import { render, waitFor } from '@/test/utils'; import { OAuthConsent } from '../OAuthConsent'; -// Captures the onChange injected into SelectOptionList's useInView so tests -// can simulate "user scrolled to the bottom of the org dropdown". -let capturedLoadMoreOnChange: ((inView: boolean) => void) | undefined; - -// Default: useOrganizationList returns no memberships and is not loaded. -// Individual tests override this mock to inject org data. -vi.mock('@clerk/shared/react', async importOriginal => { - const actual = await (importOriginal as () => Promise>)(); - return { - ...actual, - useOrganizationList: vi.fn().mockReturnValue({ - isLoaded: false, - userMemberships: { data: [], hasNextPage: false, fetchNext: vi.fn(), isLoading: false }, - }), - }; -}); - -vi.mock('@/ui/hooks/useInView', () => ({ - useInView: vi.fn().mockImplementation(({ onChange }: { onChange?: (inView: boolean) => void }) => { - capturedLoadMoreOnChange = onChange; - return { ref: vi.fn(), inView: false }; - }), -})); - const { createFixtures } = bindCreateFixtures('OAuthConsent'); const fakeConsentInfo = { @@ -66,7 +41,6 @@ describe('OAuthConsent', () => { const originalLocation = window.location; beforeEach(() => { - capturedLoadMoreOnChange = undefined; Object.defineProperty(window, 'location', { configurable: true, writable: true, @@ -347,23 +321,16 @@ describe('OAuthConsent', () => { describe('org selection', () => { it('does not render the org selector when __internal_enableOrgSelection is not set', async () => { const { wrapper, fixtures, props } = await createFixtures(f => { - f.withUser({ email_addresses: ['jane@example.com'] }); + f.withUser({ + email_addresses: ['jane@example.com'], + organization_memberships: [{ id: 'org_1', name: 'Acme Corp' }], + }); + f.withOrganizations(); }); props.setProps({ componentName: 'OAuthConsent' } as any); mockOAuthApplication(fixtures.clerk, { getConsentInfo: vi.fn().mockResolvedValue(fakeConsentInfo) }); - vi.mocked(useOrganizationList).mockReturnValue({ - isLoaded: true, - userMemberships: { - data: [ - { - organization: { id: 'org_1', name: 'Acme Corp', imageUrl: 'https://img.clerk.com/static/clerk.png' }, - }, - ], - }, - } as any); - const { queryByRole } = render(, { wrapper }); await waitFor(() => { @@ -371,24 +338,38 @@ describe('OAuthConsent', () => { }); }); - it('renders the org selector when __internal_enableOrgSelection is true and orgs are loaded', async () => { + it('does not render the org selector when organizations feature is disabled in the dashboard', async () => { + // SDK-63: enableOrgSelection is set but organizationSettings.enabled is false, + // so no org select and no useOrganizationList call. const { wrapper, fixtures, props } = await createFixtures(f => { - f.withUser({ email_addresses: ['jane@example.com'] }); + f.withUser({ + email_addresses: ['jane@example.com'], + organization_memberships: [{ id: 'org_1', name: 'Acme Corp' }], + }); + // intentionally NOT calling f.withOrganizations() }); props.setProps({ componentName: 'OAuthConsent', __internal_enableOrgSelection: true } as any); mockOAuthApplication(fixtures.clerk, { getConsentInfo: vi.fn().mockResolvedValue(fakeConsentInfo) }); - vi.mocked(useOrganizationList).mockReturnValue({ - isLoaded: true, - userMemberships: { - data: [ - { - organization: { id: 'org_1', name: 'Acme Corp', imageUrl: 'https://img.clerk.com/static/clerk.png' }, - }, - ], - }, - } as any); + const { queryByRole } = render(, { wrapper }); + + await waitFor(() => { + expect(queryByRole('combobox')).toBeNull(); + }); + }); + + it('renders the org selector when __internal_enableOrgSelection is true and user has memberships', async () => { + const { wrapper, fixtures, props } = await createFixtures(f => { + f.withUser({ + email_addresses: ['jane@example.com'], + organization_memberships: [{ id: 'org_1', name: 'Acme Corp' }], + }); + f.withOrganizations(); + }); + + props.setProps({ componentName: 'OAuthConsent', __internal_enableOrgSelection: true } as any); + mockOAuthApplication(fixtures.clerk, { getConsentInfo: vi.fn().mockResolvedValue(fakeConsentInfo) }); const { getByText } = render(, { wrapper }); @@ -397,25 +378,18 @@ describe('OAuthConsent', () => { }); }); - it('includes a hidden organization_id input in the form when org selection is enabled and an org is selected', async () => { + it('includes a hidden organization_id input when org selection is enabled and user has memberships', async () => { const { wrapper, fixtures, props } = await createFixtures(f => { - f.withUser({ email_addresses: ['jane@example.com'] }); + f.withUser({ + email_addresses: ['jane@example.com'], + organization_memberships: [{ id: 'org_1', name: 'Acme Corp' }], + }); + f.withOrganizations(); }); props.setProps({ componentName: 'OAuthConsent', __internal_enableOrgSelection: true } as any); mockOAuthApplication(fixtures.clerk, { getConsentInfo: vi.fn().mockResolvedValue(fakeConsentInfo) }); - vi.mocked(useOrganizationList).mockReturnValue({ - isLoaded: true, - userMemberships: { - data: [ - { - organization: { id: 'org_1', name: 'Acme Corp', imageUrl: 'https://img.clerk.com/static/clerk.png' }, - }, - ], - }, - } as any); - const { baseElement } = render(, { wrapper }); await waitFor(() => { @@ -442,54 +416,4 @@ describe('OAuthConsent', () => { }); }); }); - - describe('org selection — infinite scroll', () => { - const twoOrgs = [ - { organization: { id: 'org_1', name: 'Acme Corp', imageUrl: 'https://img.clerk.com/static/clerk.png' } }, - { organization: { id: 'org_2', name: 'Beta Inc', imageUrl: 'https://img.clerk.com/static/beta.png' } }, - ]; - - it('calls fetchNext when the load-more sentinel enters view and more pages are available', async () => { - const fetchNext = vi.fn(); - const { wrapper, fixtures, props } = await createFixtures(f => { - f.withUser({ email_addresses: ['jane@example.com'] }); - }); - - props.setProps({ componentName: 'OAuthConsent', __internal_enableOrgSelection: true } as any); - mockOAuthApplication(fixtures.clerk, { getConsentInfo: vi.fn().mockResolvedValue(fakeConsentInfo) }); - - vi.mocked(useOrganizationList).mockReturnValue({ - isLoaded: true, - userMemberships: { data: twoOrgs, hasNextPage: true, fetchNext, isLoading: false }, - } as any); - - render(, { wrapper }); - - await waitFor(() => expect(capturedLoadMoreOnChange).toBeDefined()); - - capturedLoadMoreOnChange!(true); - expect(fetchNext).toHaveBeenCalledTimes(1); - }); - - it('does not call fetchNext when hasNextPage is false', async () => { - const fetchNext = vi.fn(); - const { wrapper, fixtures, props } = await createFixtures(f => { - f.withUser({ email_addresses: ['jane@example.com'] }); - }); - - props.setProps({ componentName: 'OAuthConsent', __internal_enableOrgSelection: true } as any); - mockOAuthApplication(fixtures.clerk, { getConsentInfo: vi.fn().mockResolvedValue(fakeConsentInfo) }); - - vi.mocked(useOrganizationList).mockReturnValue({ - isLoaded: true, - userMemberships: { data: twoOrgs, hasNextPage: false, fetchNext, isLoading: false }, - } as any); - - render(, { wrapper }); - - await waitFor(() => expect(capturedLoadMoreOnChange).toBeDefined()); - capturedLoadMoreOnChange!(true); - expect(fetchNext).not.toHaveBeenCalled(); - }); - }); });