diff --git a/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts b/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts index 1b6e7391b6..fef3ee3939 100644 --- a/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts +++ b/apps/api/src/integration-platform/controllers/sync-available-providers.controller.spec.ts @@ -11,13 +11,13 @@ import { GenericDeviceSyncService } from '../services/generic-device-sync.servic import { DynamicIntegrationRepository } from '../repositories/dynamic-integration.repository'; import { CheckRunRepository } from '../repositories/check-run.repository'; -const mockConnectionFindFirst = jest.fn(); +const mockConnectionFindMany = jest.fn(); const mockGetActiveManifests = jest.fn(); jest.mock('@db', () => ({ db: { integrationConnection: { - findFirst: (...args: unknown[]) => mockConnectionFindFirst(...args), + findMany: (...args: unknown[]) => mockConnectionFindMany(...args), }, }, })); @@ -101,17 +101,15 @@ describe('SyncController - getAvailableSyncProviders connection status', () => { }); it('reports connected + connectionStatus active for an active connection', async () => { - mockConnectionFindFirst.mockImplementation( - (args: { where: { status: string } }) => - args.where.status === 'active' - ? { - id: 'icn_active', - status: 'active', - lastSyncAt: null, - nextSyncAt: null, - } - : null, - ); + mockConnectionFindMany.mockResolvedValue([ + { + id: 'icn_active', + status: 'active', + lastSyncAt: null, + nextSyncAt: null, + provider: { slug: 'intune' }, + }, + ]); const result = await controller.getAvailableSyncProviders(orgId, 'device'); @@ -123,15 +121,20 @@ describe('SyncController - getAvailableSyncProviders connection status', () => { connectionId: 'icn_active', }), ]); - // With an active connection there is no need to look up an errored one. - expect(mockConnectionFindFirst).toHaveBeenCalledTimes(1); + // ONE batched query for all providers — no per-provider point lookups. + expect(mockConnectionFindMany).toHaveBeenCalledTimes(1); + const args = mockConnectionFindMany.mock.calls[0][0] as { + where: { provider: { slug: { in: string[] } } }; + orderBy: { updatedAt: string }; + }; + expect(args.where.provider.slug.in).toEqual(['intune']); + expect(args.orderBy).toEqual({ updatedAt: 'desc' }); }); it('reports connectionStatus error (not connected) when the latest connection is broken', async () => { - mockConnectionFindFirst.mockImplementation( - (args: { where: { status?: string } }) => - args.where.status === 'active' ? null : { status: 'error' }, - ); + mockConnectionFindMany.mockResolvedValue([ + { id: 'icn_broken', status: 'error', lastSyncAt: null, nextSyncAt: null, provider: { slug: 'intune' } }, + ]); const result = await controller.getAvailableSyncProviders(orgId, 'device'); @@ -146,12 +149,12 @@ describe('SyncController - getAvailableSyncProviders connection status', () => { }); it('does not resurface a stale error when the latest connection is disconnected', async () => { - // Reconnects create new rows, so an old 'error' row can coexist with a - // newer 'disconnected' one. The latest row (disconnected) must win. - mockConnectionFindFirst.mockImplementation( - (args: { where: { status?: string } }) => - args.where.status === 'active' ? null : { status: 'disconnected' }, - ); + // Rows arrive newest-first (orderBy updatedAt desc): the user reconnected + // and later disconnected, leaving an older row stuck in 'error'. + mockConnectionFindMany.mockResolvedValue([ + { id: 'icn_new', status: 'disconnected', lastSyncAt: null, nextSyncAt: null, provider: { slug: 'intune' } }, + { id: 'icn_old', status: 'error', lastSyncAt: null, nextSyncAt: null, provider: { slug: 'intune' } }, + ]); const result = await controller.getAvailableSyncProviders(orgId, 'device'); @@ -163,18 +166,28 @@ describe('SyncController - getAvailableSyncProviders connection status', () => { connectionId: null, }), ]); - // The fallback must pick the LATEST row regardless of status (no status - // filter), ordered by recency — not hunt for any old 'error' row. - const fallbackArgs = mockConnectionFindFirst.mock.calls[1][0] as { - where: { status?: string }; - orderBy: { updatedAt: string }; - }; - expect(fallbackArgs.where.status).toBeUndefined(); - expect(fallbackArgs.orderBy).toEqual({ updatedAt: 'desc' }); + }); + + it('prefers an active connection even when a newer non-active row exists', async () => { + mockConnectionFindMany.mockResolvedValue([ + { id: 'icn_new_err', status: 'error', lastSyncAt: null, nextSyncAt: null, provider: { slug: 'intune' } }, + { id: 'icn_active', status: 'active', lastSyncAt: null, nextSyncAt: null, provider: { slug: 'intune' } }, + ]); + + const result = await controller.getAvailableSyncProviders(orgId, 'device'); + + expect(result.providers).toEqual([ + expect.objectContaining({ + slug: 'intune', + connected: true, + connectionStatus: 'active', + connectionId: 'icn_active', + }), + ]); }); it('reports connectionStatus null when the org has no connection for the provider', async () => { - mockConnectionFindFirst.mockResolvedValue(null); + mockConnectionFindMany.mockResolvedValue([]); const result = await controller.getAvailableSyncProviders(orgId, 'device'); diff --git a/apps/api/src/integration-platform/controllers/sync.controller.ts b/apps/api/src/integration-platform/controllers/sync.controller.ts index 9da0d3bdb0..e510db91ff 100644 --- a/apps/api/src/integration-platform/controllers/sync.controller.ts +++ b/apps/api/src/integration-platform/controllers/sync.controller.ts @@ -1777,53 +1777,55 @@ export class SyncController { m.capabilities?.includes(capability), ); - const results = await Promise.all( - syncProviders.map(async (m) => { - const connection = await db.integrationConnection.findFirst({ - where: { - organizationId, - status: 'active', - provider: { slug: m.id }, - }, - select: { - id: true, - status: true, - lastSyncAt: true, - nextSyncAt: true, - }, - }); - // No active connection: surface a broken (errored) one so the UI can - // prompt a reconnect instead of silently hiding sync for the provider. - // Multiple rows can exist per (org, provider) — reconnects create new - // rows — so the LATEST row decides: a newer 'disconnected' row means - // the user chose to disconnect, and a stale older 'error' row must - // not resurface a reconnect hint. - const latestConnection = connection - ? null - : await db.integrationConnection.findFirst({ - where: { - organizationId, - provider: { slug: m.id }, - }, - select: { status: true }, - orderBy: { updatedAt: 'desc' }, - }); - return { - slug: m.id, - name: m.name, - logoUrl: m.logoUrl, - connected: !!connection, - connectionStatus: connection - ? ('active' as const) - : latestConnection?.status === 'error' - ? ('error' as const) - : null, - connectionId: connection?.id ?? null, - lastSyncAt: connection?.lastSyncAt?.toISOString() ?? null, - nextSyncAt: connection?.nextSyncAt?.toISOString() ?? null, - }; - }), - ); + // One batched query for every provider's connections (newest first), then + // resolve per-provider status in memory — avoids N (to 2N) point lookups. + const orgConnections = await db.integrationConnection.findMany({ + where: { + organizationId, + provider: { slug: { in: syncProviders.map((m) => m.id) } }, + }, + select: { + id: true, + status: true, + lastSyncAt: true, + nextSyncAt: true, + provider: { select: { slug: true } }, + }, + orderBy: { updatedAt: 'desc' }, + }); + const connectionsBySlug = new Map(); + for (const conn of orgConnections) { + const slug = conn.provider.slug; + const list = connectionsBySlug.get(slug); + if (list) list.push(conn); + else connectionsBySlug.set(slug, [conn]); + } + + const results = syncProviders.map((m) => { + const conns = connectionsBySlug.get(m.id) ?? []; + const connection = conns.find((c) => c.status === 'active'); + // No active connection: surface a broken (errored) one so the UI can + // prompt a reconnect instead of silently hiding sync for the provider. + // Multiple rows can exist per (org, provider) — reconnects create new + // rows — so the LATEST row decides: a newer 'disconnected' row means + // the user chose to disconnect, and a stale older 'error' row must + // not resurface a reconnect hint. + const latestConnection = connection ? null : (conns[0] ?? null); + return { + slug: m.id, + name: m.name, + logoUrl: m.logoUrl, + connected: !!connection, + connectionStatus: connection + ? ('active' as const) + : latestConnection?.status === 'error' + ? ('error' as const) + : null, + connectionId: connection?.id ?? null, + lastSyncAt: connection?.lastSyncAt?.toISOString() ?? null, + nextSyncAt: connection?.nextSyncAt?.toISOString() ?? null, + }; + }); return { providers: results }; } diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx index 6fb4771fe7..ce099a1d58 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.test.tsx @@ -81,6 +81,22 @@ describe('DeviceSyncProviderSelector — RBAC gating', () => { ); }); + it('marks the saved provider as selected in the open dropdown (controlled select)', async () => { + const user = userEvent.setup(); + mockHasPermission.mockImplementation( + (resource: string, action: string) => + resource === 'integration' && action === 'update', + ); + + render(); + + await user.click( + screen.getByRole('combobox', { name: /Sync devices from/i }), + ); + const jamfOption = await screen.findByRole('option', { name: /Jamf/i }); + expect(jamfOption).toHaveAttribute('aria-selected', 'true'); + }); + it('renders nothing for a user without integration:update and disables the hook', () => { mockHasPermission.mockReturnValue(false); @@ -114,7 +130,7 @@ describe('DeviceSyncProviderSelector — RBAC gating', () => { expect(screen.getByText('Not syncing')).toBeInTheDocument(); await user.click(trigger); expect( - screen.getByRole('option', { name: /Kandji/i }), + await screen.findByRole('option', { name: /Kandji/i }), ).toBeInTheDocument(); // The saved provider is still the user's choice — "Don't auto-sync" must // NOT be marked Active just because the choice can't be resolved to a @@ -246,7 +262,9 @@ describe('DeviceSyncProviderSelector — connection states', () => { mockHook({ selectedProvider: null, availableProviders: [ - { ...provider, connected: false, connectionId: null }, + // Explicitly undefined — the base fixture carries 'active', which + // would not represent an older API response lacking the field. + { ...provider, connected: false, connectionStatus: undefined, connectionId: null }, ], hasAnyConnection: false, }); diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx index a5f2fd1531..23954bd40a 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceSyncProviderSelector.tsx @@ -104,8 +104,22 @@ export function DeviceSyncProviderSelector() { // the dropdown's info block.
- {/* Uncontrolled on purpose — mirrors the (working) people-sync select. */} - p.slug === selectedProvider) || + erroredProviders.some((p) => p.slug === selectedProvider) + ? selectedProvider + : null + } + onValueChange={handleValueChange} + disabled={isSyncing} + > {isSyncing ? (