fix(people): settle header actions, narrow refusals, refresh people on disconnect

Author: waleedlatif1

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

@@ -91,10 +91,15 @@ export function usePeopleSyncActions(organizationId: string) {
       if (!body) throw new Error('Choose a directory connection')
       return requestJson(configurePeopleSyncContract, { params, body })
     },
-    onSuccess: async (result) => {
+    onSuccess: async (result, action) => {
       const queryKey = syncKeys.organization(organizationId)
       await client.cancelQueries({ queryKey })
-      const refreshed = refreshPublishedPeople(client, organizationId, result)
+      // A sync can publish while the disconnect is in flight, so its last-success time can't say
+      // whether the cached people still exist.
+      const refreshed =
+        action === 'disconnect'
+          ? client.invalidateQueries({ queryKey: peopleKeys.organization(organizationId) })
+          : refreshPublishedPeople(client, organizationId, result)
       client.setQueryData(queryKey, result)
       await refreshed
     },

apps/sim/platform/app/(shell)/people/_components/people-page/people-page.tsx

@@ -47,10 +47,13 @@ export function PeoplePage({ organizationId }: PeoplePageProps) {
   const [connecting, setConnecting] = useState<SyncSource | null>(null)
   const currentView = view ?? (search || department || departmentKey ? 'directory' : 'chart')
   const status = useDirectorySyncStatus(organizationId, isAdmin)
-  const { emptyOrganization } = useChartRoots(organizationId, isAdmin && currentView === 'chart')
+  const roots = useChartRoots(organizationId, isAdmin && currentView === 'chart')
   const openSync = () => void setView({ panel: 'sync' }, { history: 'push' })
   const openImport = () => setImporting(true)
-  const firstUse = isAdmin && emptyOrganization && status.loaded && !status.sync
+  const firstUse = isAdmin && roots.emptyOrganization && status.loaded && !status.sync
+  // First use has no header actions, so they wait for the reads that decide it rather than flash.
+  const showActions =
+    isAdmin && (currentView !== 'chart' || (roots.settled && status.settled && !firstUse))
   const lastSuccessAt = status.sync?.lastSuccessAt
   const modals = (
     <>
@@ -96,7 +99,7 @@ export function PeoplePage({ organizationId }: PeoplePageProps) {
       view={currentView}
       onViewChange={(next) => void setView({ view: next }, { history: 'push' })}
       actions={
-        isAdmin && !(firstUse && currentView === 'chart') ? (
+        showActions ? (
           <>
             {lastSuccessAt && (
               <span className={styles.updated} data-type='T050'>

apps/sim/platform/app/(shell)/people/_components/person/person-modal.tsx

@@ -40,6 +40,7 @@ export function PersonModal({ organizationId, person, onClose }: PersonModalProp
     startDate: person.startDate,
     managerMemberId: person.managerMemberId,
   })
+  const [saving, setSaving] = useState(false)
   const [errors, setErrors] = useState<Partial<Record<keyof PersonDraft, string>>>({})
   // The chosen manager's name, kept with the choice: the search results may no longer include them.
   // Only a stored manager who is suspended has an id and no name.
@@ -59,10 +60,15 @@ export function PersonModal({ organizationId, person, onClose }: PersonModalProp
       title={`Edit ${person.name}`}
       confirmLabel='Save'
       onConfirm={async () => {
-        const problems = await edit.save(draft)
-        if (problems) {
-          setErrors(problems)
-          throw new FieldProblems('Check the highlighted fields.')
+        setSaving(true)
+        try {
+          const problems = await edit.save(draft)
+          if (problems) {
+            setErrors(problems)
+            throw new FieldProblems('Check the highlighted fields.')
+          }
+        } finally {
+          setSaving(false)
         }
       }}
     >
@@ -75,6 +81,7 @@ export function PersonModal({ organizationId, person, onClose }: PersonModalProp
           value={draft[field] ?? ''}
           onChange={(value) => change(field, value || null)}
           error={errors[field]}
+          disabled={saving}
         />
       ))}
       <DatePicker
@@ -84,6 +91,7 @@ export function PersonModal({ organizationId, person, onClose }: PersonModalProp
         value={draft.startDate ?? null}
         onValueChange={(value) => change('startDate', value)}
         error={errors.startDate}
+        disabled={saving}
       />
       <SearchableSelect
         label='Manager'
@@ -94,6 +102,7 @@ export function PersonModal({ organizationId, person, onClose }: PersonModalProp
         search={picker.search}
         onOpenChange={picker.onOpenChange}
         error={errors.managerMemberId}
+        disabled={saving}
         onValueChange={(value) => {
           const manager = picker.people.find((candidate) => candidate.memberId === value)
           setManagerName(manager?.name ?? null)

apps/sim/platform/lib/data/people.ts

@@ -88,9 +88,11 @@ export function isApiConflict(error: unknown): boolean {
   return isApiClientError(error) && error.status === 409
 }
 
-/** A refusal replaces what is on screen; a transient failure keeps it. */
+const REFUSAL_STATUSES = new Set([400, 401, 403, 404])
+
+/** A refusal replaces what is on screen; a transient failure, including a 429, keeps it. */
 export function isRefusal(error: unknown): boolean {
-  return isApiClientError(error) && error.status < 500
+  return isApiClientError(error) && REFUSAL_STATUSES.has(error.status)
 }
 
 export function usePeopleUrl() {
@@ -290,7 +292,11 @@ export function useOrgChart(organizationId: string) {
 export function useChartRoots(organizationId: string, enabled = true) {
   const roots = usePeople(organizationId, '', '', { scope: 'roots', enabled })
   const firstTeam = roots.data?.pages[0]?.people[0]
-  return { firstTeam, emptyOrganization: Boolean(roots.data) && !firstTeam }
+  return {
+    firstTeam,
+    emptyOrganization: Boolean(roots.data) && !firstTeam,
+    settled: !roots.isPending,
+  }
 }
 
 export function usePersonProfile(organizationId: string, memberId: string) {
@@ -389,7 +395,7 @@ export type SyncFieldErrors = Partial<Record<keyof SyncDraft, string>>
 export function useDirectorySyncStatus(organizationId: string, enabled: boolean) {
   const query = usePeopleSyncStatus(organizationId, enabled)
   const status = isDirectorySyncAccessError(query.error) ? undefined : query.data
-  return { loaded: Boolean(status), sync: status?.sync ?? null }
+  return { loaded: Boolean(status), settled: !query.isPending, sync: status?.sync ?? null }
 }
 
 /** The status with the action state machine the hook keeps out of the cache. */

apps/sim/scripts/test-scim-e2e.ts

@@ -421,18 +421,16 @@ async function run() {
             waitUntil: 'domcontentloaded',
             timeout: 180_000,
           })
-          await page.getByRole('button', { name: 'Import people', exact: true }).click()
-          const picker = page.getByRole('dialog', { name: 'Import people data' })
-          await picker.getByRole('button', { name: 'SCIM provisioning' }).click()
-          await page
-            .getByRole('dialog', { name: 'Import from SCIM provisioning' })
-            .getByRole('button', { name: 'Preview import' })
-            .click()
-          const modal = page.getByRole('dialog', { name: 'Review import' })
-          await expect(modal.getByText(/1 profile will be updated/)).toBeVisible()
+          await page.getByRole('button', { name: /^Import( profiles)?$/ }).click()
+          const importDialog = page.getByRole('dialog', { name: 'Import profiles' })
+          await importDialog.getByRole('combobox', { name: 'Source' }).click()
+          await page.getByRole('option', { name: 'SCIM provisioning' }).click()
+          await importDialog.getByRole('button', { name: 'Preview import' }).click()
+          const modal = page.getByRole('dialog', { name: 'Review profile import' })
+          await expect(modal.getByText('To update', { exact: true })).toBeVisible()
           await mkdir(screenshotDir, { recursive: true })
           await page.screenshot({ path: join(screenshotDir, 'provisioned-import.png') })
-          await modal.getByRole('button', { name: 'Import', exact: true }).click()
+          await modal.getByRole('button', { name: 'Import 1 profile', exact: true }).click()
           await expect(modal).toHaveCount(0)
         } finally {
           await browser.close()
@@ -544,23 +542,26 @@ async function run() {
           await page.route(statusUrl, (route) =>
             route.fulfill({ status: 503, json: { error: 'Unavailable' } })
           )
-          await page.getByRole('button', { name: 'Directory sync', exact: true }).click()
-          const modal = page.getByRole('dialog', { name: 'Directory sync', exact: true })
-          await modal.getByRole('button', { name: 'Retry connection' }).waitFor()
+          const openSync = page.getByRole('button', { name: 'Directory sync', exact: true })
+          await openSync.click()
+          await page.getByRole('heading', { name: 'Directory sync', level: 1 }).waitFor()
+          await page.getByRole('button', { name: 'Retry', exact: true }).waitFor()
           await page.unroute(statusUrl)
-          await expect(modal.getByText('Sync failed', { exact: true })).toBeVisible({
+          await expect(page.getByText('Sync failed', { exact: true })).toBeVisible({
             timeout: 60_000,
           })
-          await expect(modal.getByText(/reporting loop/)).toBeVisible()
+          await expect(page.getByText(/reporting loop/)).toBeVisible()
           await page.screenshot({ path: join(screenshotDir, 'sync-failure.png') })
-          await modal.getByRole('button', { name: 'Retry sync' }).click()
-          await expect(modal.getByText('Connected', { exact: true })).toBeVisible()
-          await expect(modal.getByText('Last successful sync')).toBeVisible()
+          await page.getByRole('button', { name: 'Retry sync' }).click()
+          await expect(page.getByText('Connected', { exact: true })).toBeVisible()
+          await expect(page.getByText('Last successful sync')).toBeVisible()
           await page.setViewportSize({ width: 390, height: 844 })
           await page.screenshot({ path: join(screenshotDir, 'sync-recovered-mobile.png') })
-          const dialogBox = await modal.boundingBox()
-          assert(dialogBox && dialogBox.x >= 0 && dialogBox.x + dialogBox.width <= 391)
+          assert(
+            await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)
+          )
           assert.equal(record((await chart()).manager).email, bobEmail)
+          await page.getByRole('button', { name: 'Back to people' }).click()
           await expect(page.getByText('Principal Engineer', { exact: true })).toBeAttached()
           await patch(token, `/Users/${aliceId}`, [
             { op: 'replace', path: 'title', value: 'Staff Platform Engineer' },
@@ -569,20 +570,20 @@ async function run() {
           await expect(page.getByText('Staff Platform Engineer', { exact: true })).toBeAttached({
             timeout: 60_000,
           })
-          await modal.getByRole('button', { name: 'Disconnect', exact: true }).click()
+          await openSync.click()
+          await page.getByRole('button', { name: 'Disconnect SCIM provisioning' }).click()
           await page
             .getByRole('dialog', { name: 'Disconnect directory' })
             .getByRole('button', { name: 'Disconnect', exact: true })
             .click()
-          await expect(
-            modal.getByRole('button', { name: 'Connect SCIM provisioning' })
-          ).toBeVisible()
-          await modal.getByRole('button', { name: 'Connect SCIM provisioning' }).click()
+          const connectScim = page.getByRole('button', { name: 'Connect SCIM provisioning' })
+          await expect(connectScim).toBeVisible()
+          await connectScim.click()
           await page
             .getByRole('dialog', { name: 'Connect SCIM provisioning' })
             .getByRole('button', { name: 'Connect and sync' })
             .click()
-          await expect(modal.getByText('Connected', { exact: true })).toBeVisible()
+          await expect(page.getByText('Connected', { exact: true })).toBeVisible()
         } finally {
           await browser.close()
         }