-
Notifications
You must be signed in to change notification settings - Fork 474
feat(shared): add directory sync credential and status hooks #9720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@clerk/shared': minor | ||
| --- | ||
|
|
||
| Add credential and sync mutations to `__internal_useOrganizationDirectorySync`, and a `__internal_useOrganizationDirectorySyncStatus` hook that reports a directory's last sync result with opt-in polling. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| import { renderHook, waitFor } from '@testing-library/react'; | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import type { DirectorySyncResource } from '@/types/directorySync'; | ||
|
|
||
| import { __internal_useOrganizationDirectorySyncStatus } from '../useOrganizationDirectorySyncStatus'; | ||
| import { createMockClerk, createMockQueryClient } from './mocks/clerk'; | ||
| import { wrapper } from './wrapper'; | ||
|
|
||
| const POLL_INTERVAL_MS = 20; | ||
|
|
||
| const getSyncStatusSpy = vi.fn(() => | ||
| Promise.resolve({ lastSyncedAt: new Date(1700000000000), lastSyncStatus: 'succeeded', lastSyncError: null }), | ||
| ); | ||
|
|
||
| const createDirectory = (id: string, organizationId = 'org_1') => | ||
| ({ | ||
| id, | ||
| organizationId, | ||
| enterpriseConnectionId: 'ent_1', | ||
| getSyncStatus: getSyncStatusSpy, | ||
| }) as unknown as DirectorySyncResource; | ||
|
|
||
| const defaultQueryClient = createMockQueryClient(); | ||
|
|
||
| const mockClerk = createMockClerk({ | ||
| queryClient: defaultQueryClient, | ||
| __internal_lastEmittedResources: { | ||
| user: null, | ||
| session: null, | ||
| organization: { id: 'org_1' }, | ||
| client: null, | ||
| }, | ||
| }); | ||
|
|
||
| vi.mock('../../contexts', () => ({ | ||
| useAssertWrappedByClerkProvider: () => {}, | ||
| useClerkInstanceContext: () => mockClerk, | ||
| useInitialStateContext: () => undefined, | ||
| })); | ||
|
|
||
| type RenderProps = { directory: DirectorySyncResource | null; poll?: boolean }; | ||
|
|
||
| const renderStatus = (initialProps: RenderProps) => | ||
| renderHook( | ||
| ({ directory, poll }: RenderProps) => | ||
| __internal_useOrganizationDirectorySyncStatus({ directory, poll, pollIntervalMs: POLL_INTERVAL_MS }), | ||
| { wrapper, initialProps }, | ||
| ); | ||
|
|
||
| describe('useOrganizationDirectorySyncStatus', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| defaultQueryClient.client.clear(); | ||
| mockClerk.loaded = true; | ||
| }); | ||
|
|
||
| it('stays dormant without a directory', () => { | ||
| const { result } = renderStatus({ directory: null, poll: true }); | ||
|
|
||
| expect(getSyncStatusSpy).not.toHaveBeenCalled(); | ||
| expect(result.current.data).toBeUndefined(); | ||
| expect(result.current.isPolling).toBe(false); | ||
| }); | ||
|
|
||
| it('reads the last sync result once a directory is present', async () => { | ||
| const { result } = renderStatus({ directory: createDirectory('dir_1') }); | ||
|
|
||
| await waitFor(() => expect(result.current.data).toBeDefined()); | ||
| expect(getSyncStatusSpy).toHaveBeenCalled(); | ||
| expect(result.current.data?.lastSyncStatus).toBe('succeeded'); | ||
| expect(result.current.isPolling).toBe(false); | ||
| }); | ||
|
|
||
| it('polls while armed', async () => { | ||
| const { result } = renderStatus({ directory: createDirectory('dir_1'), poll: true }); | ||
|
|
||
| await waitFor(() => expect(result.current.data).toBeDefined()); | ||
| expect(result.current.isPolling).toBe(true); | ||
|
|
||
| const callsAfterFirstLoad = getSyncStatusSpy.mock.calls.length; | ||
| await waitFor(() => expect(getSyncStatusSpy.mock.calls.length).toBeGreaterThan(callsAfterFirstLoad)); | ||
| }); | ||
|
|
||
| it('refuses a directory belonging to another organization', async () => { | ||
| const { result } = renderStatus({ directory: createDirectory('dir_other', 'org_2') }); | ||
|
|
||
| // The cache key is built from the context organization, so reading a | ||
| // foreign directory would file its result under this organization. | ||
| expect(getSyncStatusSpy).not.toHaveBeenCalled(); | ||
| expect(result.current.data).toBeUndefined(); | ||
| expect(result.current.isPolling).toBe(false); | ||
| }); | ||
|
|
||
| it('does not carry one directory status onto another', async () => { | ||
| const { result, rerender } = renderStatus({ directory: createDirectory('dir_1') }); | ||
|
|
||
| await waitFor(() => expect(result.current.data).toBeDefined()); | ||
|
|
||
| // A different directory must not momentarily report the previous one's run. | ||
| // "Never synced" and "synced an hour ago" drive different UI. | ||
| getSyncStatusSpy.mockImplementationOnce(() => new Promise<never>(() => {})); | ||
| rerender({ directory: createDirectory('dir_2') }); | ||
|
|
||
| expect(result.current.data).toBeUndefined(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| import { useCallback } from 'react'; | ||
|
|
||
| import type { DirectorySyncResource, DirectorySyncStatusResource } from '../../types/directorySync'; | ||
| import { useClerkInstanceContext } from '../contexts'; | ||
| import { useClerkQueryClient } from '../query/use-clerk-query-client'; | ||
| import { useClerkQuery } from '../query/useQuery'; | ||
| import { useOrganizationBase } from './base/useOrganizationBase'; | ||
| import { useClearQueriesOnSignOut } from './useClearQueriesOnSignOut'; | ||
| import { useOrganizationDirectorySyncStatusCacheKeys } from './useOrganizationDirectorySync.shared'; | ||
|
|
||
| const DEFAULT_POLL_INTERVAL_MS = 2_000; | ||
|
|
||
| export type UseOrganizationDirectorySyncStatusParams = { | ||
| /** The directory to read status for, e.g. `data` from `useOrganizationDirectorySync`. Nothing is fetched while `null` or `undefined`. */ | ||
| directory: DirectorySyncResource | null | undefined; | ||
| /** | ||
| * Poll for changes while `true`. Tie this to the view that needs the live | ||
| * status so polling stops when that view goes away. | ||
| * | ||
| * @default false | ||
| */ | ||
| poll?: boolean; | ||
| /** | ||
| * Polling interval (ms) used while `poll` is `true`. | ||
| * | ||
| * @default 2000 | ||
| */ | ||
| pollIntervalMs?: number; | ||
| /** | ||
| * If `false`, nothing is fetched and polling is paused. | ||
| * | ||
| * @default true | ||
| */ | ||
| enabled?: boolean; | ||
| }; | ||
|
|
||
| export type UseOrganizationDirectorySyncStatusReturn = { | ||
| /** `undefined` while loading and while the hook is disabled. Every field is `null` before the first sync completes. */ | ||
| data: DirectorySyncStatusResource | undefined; | ||
| error: Error | null; | ||
| isLoading: boolean; | ||
| isFetching: boolean; | ||
| /** `true` while the hook is polling. */ | ||
| isPolling: boolean; | ||
| revalidate: () => Promise<void>; | ||
| }; | ||
|
|
||
| /** | ||
| * The result of a Directory Sync directory's most recent sync. | ||
| * | ||
| * Only pull-based directories sync, so this stays dormant for push providers, | ||
| * which are driven by the identity provider and have no sync to report. | ||
| * | ||
| * @internal | ||
| */ | ||
| function useOrganizationDirectorySyncStatus( | ||
| params: UseOrganizationDirectorySyncStatusParams, | ||
| ): UseOrganizationDirectorySyncStatusReturn { | ||
| const { directory, poll = false, pollIntervalMs = DEFAULT_POLL_INTERVAL_MS, enabled = true } = params; | ||
|
|
||
| const clerk = useClerkInstanceContext(); | ||
| const organization = useOrganizationBase(); | ||
| const [queryClient] = useClerkQueryClient(); | ||
| const enterpriseConnectionId = directory?.enterpriseConnectionId ?? null; | ||
| const directoryId = directory?.id ?? null; | ||
|
|
||
| const { queryKey, invalidationKey, stableKey, authenticated } = useOrganizationDirectorySyncStatusCacheKeys({ | ||
| organizationId: organization?.id ?? null, | ||
| enterpriseConnectionId, | ||
| directoryId, | ||
| }); | ||
|
|
||
| useClearQueriesOnSignOut({ | ||
| isSignedOut: organization === null, | ||
| authenticated, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Security & Privacy | π Major | β‘ Quick win Keep sign-out cleanup enabled for the status query. When Omit π€ Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. β Leaving this one, though the mechanism you describe is real: The reason not to change it here is that this is the repo-wide convention, not something this hook invented. π‘ If the behaviour is wrong it is wrong everywhere, which makes it worth its own change across all of them rather than a one-off here. Flagged to the author.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
A repository-wide change should evaluate all βοΈ Learnings added
You are interacting with an AI system. |
||
| stableKeys: stableKey, | ||
| }); | ||
|
|
||
| // The directory comes from the caller while the organization comes from | ||
| // context, and the cache key is built from both. A directory belonging to | ||
| // another organization would therefore cache that organization's sync result | ||
| // under this one's key, so refuse to read it. | ||
| const belongsToActiveOrganization = Boolean(organization) && directory?.organizationId === organization?.id; | ||
| const queryEnabled = enabled && clerk.loaded && belongsToActiveOrganization && Boolean(directory); | ||
|
|
||
| const query = useClerkQuery({ | ||
| queryKey, | ||
| queryFn: () => { | ||
| if (!directory) { | ||
| throw new Error('directory is required to fetch sync status'); | ||
| } | ||
| return directory.getSyncStatus(); | ||
| }, | ||
| refetchInterval: () => (poll ? pollIntervalMs : false), | ||
| enabled: queryEnabled, | ||
| refetchIntervalInBackground: false, | ||
| // No placeholderData: a stale run result shown against a different directory | ||
| // would misreport whether that directory has ever synced. | ||
| }); | ||
|
|
||
| const revalidate = useCallback(async () => { | ||
| await queryClient.invalidateQueries({ queryKey: invalidationKey }); | ||
| }, [queryClient, invalidationKey]); | ||
|
|
||
| return { | ||
| // A disabled query still exposes whatever is cached under its key; report none until it can run. | ||
| data: queryEnabled ? query.data : undefined, | ||
| error: query.error ?? null, | ||
| isLoading: query.isLoading, | ||
| isFetching: query.isFetching, | ||
| isPolling: queryEnabled && poll, | ||
| revalidate, | ||
| }; | ||
| } | ||
|
|
||
| export { useOrganizationDirectorySyncStatus as __internal_useOrganizationDirectorySyncStatus }; | ||
Uh oh!
There was an error while loading. Please reload this page.