Skip to content

Commit 053b742

Browse files
committed
fix(people): preserve sync drafts and label paged search
1 parent cb0a74b commit 053b742

4 files changed

Lines changed: 143 additions & 22 deletions

File tree

‎apps/sim/app/o/[organizationId]/people/directory-sync-modal.tsx‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ import {
2020
RESOURCE_LIST_STACK,
2121
SettingsResourceRow,
2222
} from '@/app/workspace/[workspaceId]/settings/components/settings-resource-row'
23-
import { usePeopleSyncActions, usePeopleSyncStatus } from '@/hooks/queries/people-sync'
23+
import {
24+
isDirectorySyncAccessError,
25+
usePeopleSyncActions,
26+
usePeopleSyncStatus,
27+
} from '@/hooks/queries/people-sync'
2428

2529
const SOURCES = {
2630
okta: IMPORT_SOURCES.okta,
@@ -119,12 +123,14 @@ export function DirectorySyncModal({ organizationId, onClose }: DirectorySyncMod
119123
const query = usePeopleSyncStatus(organizationId)
120124
const actions = usePeopleSyncActions(organizationId)
121125
const [mode, setMode] = useState<PeopleSyncConfig['source'] | 'disconnect' | null>(null)
122-
const sync = query.isSuccess ? query.data.sync : null
126+
const status = isDirectorySyncAccessError(query.error) ? undefined : query.data
127+
const sync = status?.sync ?? null
128+
const activeMode = status ? mode : null
123129
const goBack = () => {
124130
setMode(null)
125131
actions.reset()
126132
}
127-
if (mode === 'disconnect')
133+
if (activeMode === 'disconnect')
128134
return (
129135
<ChipConfirmModal
130136
open
@@ -141,13 +147,13 @@ export function DirectorySyncModal({ organizationId, onClose }: DirectorySyncMod
141147
<ChipModalError>{actions.error?.message}</ChipModalError>
142148
</ChipConfirmModal>
143149
)
144-
if (mode && query.isSuccess)
150+
if (activeMode)
145151
return (
146152
<DirectorySyncForm
147-
key={mode}
153+
key={activeMode}
148154
organizationId={organizationId}
149-
source={mode}
150-
name={SOURCES[mode].name}
155+
source={activeMode}
156+
name={SOURCES[activeMode].name}
151157
current={sync}
152158
actions={actions}
153159
onBack={goBack}
@@ -163,7 +169,7 @@ export function DirectorySyncModal({ organizationId, onClose }: DirectorySyncMod
163169
<ChipModalBody>
164170
{query.isPending ? (
165171
<ChipModalDescription>Loading directory connection…</ChipModalDescription>
166-
) : query.isError ? (
172+
) : query.isError && !status ? (
167173
<>
168174
<ChipModalError>{query.error.message}</ChipModalError>
169175
<div className='px-2'>

‎apps/sim/hooks/queries/people-sync.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ const syncKeys = {
2323

2424
type SyncResult = ContractJsonResponse<typeof getPeopleSyncContract>
2525

26+
export function isDirectorySyncAccessError(error: unknown) {
27+
return isApiClientError(error) && [401, 403, 404].includes(error.status)
28+
}
29+
2630
function refreshPublishedPeople(client: QueryClient, organizationId: string, result: SyncResult) {
2731
const previous = client.getQueryData<SyncResult>(syncKeys.organization(organizationId))
2832
if ((previous?.sync?.lastSuccessAt ?? null) !== (result.sync?.lastSuccessAt ?? null)) {
@@ -46,10 +50,7 @@ export function usePeopleSyncStatus(organizationId: string) {
4650
refetchInterval: (query) => {
4751
const sync = query.state.data?.sync
4852
const error = query.state.error
49-
if (error)
50-
return isApiClientError(error) && [401, 403, 404].includes(error.status)
51-
? false
52-
: IDLE_SYNC_POLL_MS
53+
if (error) return isDirectorySyncAccessError(error) ? false : IDLE_SYNC_POLL_MS
5354
if (!sync) return false
5455
if (sync.retryAt) return IDLE_SYNC_POLL_MS
5556
return sync.state === 'syncing' || sync.state === 'queued'

‎apps/sim/scripts/test-people-e2e.ts‎

Lines changed: 120 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -608,10 +608,14 @@ async function run() {
608608
await page.goBack()
609609
await expect(focused('Maya Chen')).toBeVisible()
610610
const searched = new Set<string>()
611+
const pagedSearch = 'Directory colleague'
612+
const requestedPages: (string | null)[] = []
611613
page.on('request', (request) => {
612614
const url = new URL(request.url())
613615
const term = url.searchParams.get('search')
614-
if (url.pathname === `/api/organizations/${teamOrg}/people` && term) searched.add(term)
616+
if (url.pathname !== `/api/organizations/${teamOrg}/people` || !term) return
617+
searched.add(term)
618+
if (term === pagedSearch) requestedPages.push(url.searchParams.get('cursor'))
615619
})
616620
const boundedPaste = 'q'.repeat(200)
617621
await page.context().grantPermissions(['clipboard-read', 'clipboard-write'])
@@ -625,6 +629,44 @@ async function run() {
625629
await pickerSearch.press('ControlOrMeta+v')
626630
await expect(pickerSearch).toHaveValue(boundedPaste, { timeout: 5_000 })
627631
await expect.poll(() => searched.has(boundedPaste)).toBe(true)
632+
await sql.begin(async (tx) => {
633+
await tx`insert into people_directory_sync (organization_id, source, source_target, next_sync_at)
634+
values (${teamOrg}, 'okta', 'picker.okta.com', now() + interval '1 day')`
635+
await tx`insert into people_directory_entry (id, organization_id, source, external_id, email, name)
636+
select gen_random_uuid()::text, ${teamOrg}, 'okta', 'picker-' || n, 'picker-' || n || '@example.test',
637+
${pagedSearch}::text || ' ' || lpad(n::text, 3, '0') from generate_series(1, 101) n`
638+
})
639+
try {
640+
await pickerSearch.fill(pagedSearch)
641+
await expect(page.getByRole('option', { name: /^Directory colleague 001/ })).toBeVisible()
642+
const more = page.getByRole('button', { name: 'Load more', exact: true })
643+
await expect(more).toBeVisible({ timeout: 5_000 })
644+
await Promise.all([
645+
page.waitForResponse((response) => {
646+
const url = new URL(response.url())
647+
return (
648+
url.pathname === `/api/organizations/${teamOrg}/people` &&
649+
url.searchParams.get('search') === pagedSearch &&
650+
url.searchParams.has('cursor') &&
651+
response.ok()
652+
)
653+
}),
654+
more.click(),
655+
])
656+
await more.scrollIntoViewIfNeeded()
657+
await page.getByRole('option', { name: /^Directory colleague 100/ }).click()
658+
await expect(
659+
page.getByText('No reporting lines for Directory colleague 100', { exact: true })
660+
).toBeVisible()
661+
assert.equal(requestedPages.length, 2, 'One click loads exactly one additional page')
662+
assert.equal(requestedPages[0], null)
663+
assert(requestedPages[1], 'Page two resumes with a cursor')
664+
await page.goBack()
665+
await expect(focused('Maya Chen')).toBeVisible()
666+
} finally {
667+
await sql`delete from people_directory_sync where organization_id = ${teamOrg} and source_target = 'picker.okta.com'`
668+
}
669+
await page.getByLabel('Find a person').click()
628670
await page.getByPlaceholder('Search name, title or email…').fill(' Engineering ')
629671
await page.getByText(/Ben Ortega · Design Lead/).click()
630672
await focused('Ben Ortega').click()
@@ -634,7 +676,7 @@ async function run() {
634676
await page.getByText(/Maya Chen · VP Engineering/).click()
635677
await expect(focused('Maya Chen')).toBeVisible()
636678
await page.getByRole('radio', { name: 'Directory' }).click()
637-
const directorySearch = page.getByPlaceholder('Search people...')
679+
const directorySearch = page.getByRole('textbox', { name: 'Search people...', exact: true })
638680
await directorySearch.focus()
639681
await directorySearch.press('ControlOrMeta+v')
640682
await expect(directorySearch).toHaveValue(boundedPaste)
@@ -649,7 +691,7 @@ async function run() {
649691
[...searched].every((term) => term.length <= 200),
650692
'People sent an oversized search'
651693
)
652-
await page.getByPlaceholder('Search people...').fill('Amara')
694+
await directorySearch.fill('Amara')
653695
const profileLink = page.getByRole('link', { name: 'Amara Cole', exact: true })
654696
await profileLink.focus()
655697
await page.keyboard.press('Enter')
@@ -658,7 +700,7 @@ async function run() {
658700
.getByRole('link', { name: 'People', exact: true })
659701
.and(page.locator('a[href*="search=Amara"]'))
660702
await backToResults.click()
661-
await expect(page.getByPlaceholder('Search people...')).toHaveValue('Amara')
703+
await expect(directorySearch).toHaveValue('Amara')
662704
const chartOrigin = new URL(path, baseUrl)
663705
chartOrigin.searchParams.set('view', 'chart')
664706
chartOrigin.searchParams.set('focus', lead.memberId)
@@ -680,7 +722,7 @@ async function run() {
680722
)
681723
await expect(page.getByRole('button', { name: 'Explore Lena Ali' })).toBeVisible()
682724
await page.goto(new URL(`${path}?search=Amara`, baseUrl).href)
683-
await page.getByPlaceholder('Search people...').fill('')
725+
await directorySearch.fill('')
684726
await page.waitForURL(
685727
(url) => url.searchParams.get('view') === 'directory' && !url.searchParams.get('search')
686728
)
@@ -755,6 +797,79 @@ async function run() {
755797
await browser.close()
756798
}
757799
})
800+
801+
if (screenshotDir)
802+
await check(
803+
'a transient status poll preserves a credential draft until administrator access is revoked',
804+
async () => {
805+
const { chromium, expect } = await import('@playwright/test')
806+
const browser = await chromium.launch()
807+
try {
808+
await sql`insert into people_directory_sync (organization_id, source, source_target, next_sync_at)
809+
values (${teamOrg}, 'okta', 'draft.okta.com', now() + interval '1 hour')`
810+
const page = await browser.newPage()
811+
page.setDefaultTimeout(60_000)
812+
await page
813+
.context()
814+
.addCookies([
815+
{ name: 'better-auth.session_token', value: lead.cookie, url: baseUrl.origin },
816+
])
817+
const statusUrl = new URL(`/api/organizations/${teamOrg}/people/sync`, baseUrl).href
818+
const draft = `unsaved-e2e-${generateId()}`
819+
let transmitted = false
820+
page.on('request', (req) => {
821+
transmitted ||= [req.url(), req.postData() ?? '', ...Object.values(req.headers())].some(
822+
(value) => value.includes(draft)
823+
)
824+
})
825+
await page.clock.install()
826+
await page.goto(new URL(`/o/${teamOrg}/people?panel=sync`, baseUrl).href)
827+
await page.getByRole('button', { name: 'Update credentials' }).click()
828+
const token = page
829+
.getByRole('dialog', { name: 'Update Okta credentials' })
830+
.getByLabel('API token', { exact: true })
831+
await token.click()
832+
await token.fill(draft)
833+
834+
let failures = 0
835+
await page.route(statusUrl, (route) => {
836+
if (route.request().method() === 'GET' && failures < 2) {
837+
failures++
838+
return route.fulfill({
839+
status: 503,
840+
json: { error: 'Directory status temporarily unavailable' },
841+
})
842+
}
843+
return route.continue()
844+
})
845+
const response = (code: number) =>
846+
page.waitForResponse((event) => event.url() === statusUrl && event.status() === code)
847+
const failed = response(503)
848+
await page.clock.fastForward(30_001)
849+
await failed
850+
await expect.poll(() => failures).toBe(2)
851+
const recovered = response(200)
852+
await page.clock.fastForward(30_001)
853+
await recovered
854+
await expect(token).toHaveValue(draft)
855+
856+
await sql`update member set role='member' where id=${lead.memberId}`
857+
const denied = response(403)
858+
await page.clock.fastForward(30_001)
859+
await denied
860+
await expect(token).toHaveCount(0)
861+
assert.equal(
862+
transmitted,
863+
false,
864+
'Polling and revoking access must never transmit the unsaved credential'
865+
)
866+
} finally {
867+
await browser.close()
868+
await sql`update member set role='owner' where id=${lead.memberId}`
869+
await sql`delete from people_directory_sync where organization_id=${teamOrg}`
870+
}
871+
}
872+
)
758873
}
759874

760875
async function cleanup() {

‎packages/emcn/src/components/combobox/combobox.tsx‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -446,11 +446,10 @@ const Combobox = memo(
446446
overscan: 8,
447447
})
448448
const hasActiveSearch = searchActive || (searchable && searchQuery.trim().length > 0)
449-
const continuationAction = hasActiveSearch ? (onLoadAll ?? onLoadMore) : onLoadMore
450-
const continuationLabel = hasActiveSearch ? 'Search all options' : 'Load more'
451-
const continuationLoadingLabel = hasActiveSearch
452-
? 'Searching options...'
453-
: 'Loading options...'
449+
const searchAll = hasActiveSearch && Boolean(onLoadAll)
450+
const continuationAction = searchAll ? onLoadAll : onLoadMore
451+
const continuationLabel = searchAll ? 'Search all options' : 'Load more'
452+
const continuationLoadingLabel = searchAll ? 'Searching options...' : 'Loading options...'
454453

455454
/**
456455
* Handles selection of an option. In editable mode the input is blurred on

0 commit comments

Comments
 (0)