Skip to content

Commit 84034cb

Browse files
committed
fix(connectors): save service account changes with source settings
1 parent b5d17aa commit 84034cb

17 files changed

Lines changed: 488 additions & 173 deletions

File tree

‎apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/access/route.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ export const PATCH = defineInternalJsonRoute({
2525
knowledgeBaseId: params.id,
2626
accessMode: body.accessMode,
2727
credentialId: body.credentialId,
28+
sourceConfig: body.sourceConfig,
29+
syncIntervalMinutes: body.syncIntervalMinutes,
2830
resolveBillingAttribution: (workspaceId: string) =>
2931
resolveInternalKnowledgeBillingAttribution(request, principal, workspaceId),
3032
source: 'ui' as const,

‎apps/sim/app/o/[organizationId]/settings/components/integrations/search-source-setup.test.tsx‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,13 @@ vi.mock('@/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-scope'
130130
}),
131131
}))
132132
vi.mock('@/hooks/queries/kb/connectors', () => ({
133+
isConnectorSyncingOrPending: (row: {
134+
status: string
135+
accessMode?: string
136+
memberSyncStatus?: string
137+
}) =>
138+
['pending', 'syncing'].includes(row.status) ||
139+
['pending', 'running'].includes(row.memberSyncStatus ?? ''),
133140
useSearchIndex: (
134141
scope: { workspaceId?: string; organizationId?: string },
135142
options: { enabled: boolean }
@@ -1367,10 +1374,10 @@ describe('administrator source prerequisites in real connector dialogs', () => {
13671374
(node) => node.textContent?.trim() === replacement.name
13681375
)!
13691376
await act(async () => option.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })))
1370-
expect(button('Save')).toBeDisabled()
1371-
expect(button('Change service account')).toBeEnabled()
1377+
expect(button('Save')).toBeEnabled()
1378+
expect(document.body.textContent).not.toContain('Change service account')
13721379

1373-
await click(button('Change service account'))
1380+
await click(button('Save'))
13741381

13751382
expect(mocks.applyAccess).toHaveBeenCalledExactlyOnceWith(
13761383
{

‎apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.test.tsx‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ vi.mock('@/hooks/use-oauth-return', () => ({ useOAuthReturnForKBConnectors: vi.f
3838
vi.mock('@/hooks/queries/kb/connectors', () => ({
3939
useSearchIndex: mocks.index,
4040
useConnectorDetail: mocks.detail,
41-
isConnectorSyncingOrPending: () => false,
41+
isConnectorSyncingOrPending: (row: ConnectorData) =>
42+
row.status === 'syncing' || row.status === 'pending',
4243
}))
4344
vi.mock('@/hooks/queries/search-integrations', () => ({
4445
useSearchIntegrations: mocks.integrations,
@@ -187,6 +188,19 @@ describe('organization source detail navigation', () => {
187188
expect(button, `Missing ${text}`).toBeTruthy()
188189
await act(async () => button!.click())
189190
}
191+
192+
it('passes live sync status to the form without replacing its settings baseline', async () => {
193+
await render('?view=settings')
194+
const baseline = mocks.form.mock.lastCall![0].connector
195+
mocks.dirty = true
196+
mocks.detail.mockReturnValue({ data: { ...connector, status: 'syncing' } })
197+
await render('?view=settings')
198+
expect(mocks.form.mock.lastCall![0]).toMatchObject({ connector: baseline, syncing: true })
199+
200+
mocks.detail.mockReturnValue({ data: { ...connector, status: 'active' } })
201+
await render('?view=settings')
202+
expect(mocks.form.mock.lastCall![0]).toMatchObject({ connector: baseline, syncing: false })
203+
})
190204
it.each(['documents', 'settings', 'history'])(
191205
'replaces the removed connection with Sources from the %s view',
192206
async (view) => {

‎apps/sim/app/o/[organizationId]/settings/integrations/sources/[connectorId]/source-detail.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,7 @@ function SourceSettingsForm({
425425
}: SourceSettingsFormProps) {
426426
const form = useConnectorSettingsForm({
427427
connector: baseline,
428+
syncing: isConnectorSyncingOrPending(connector),
428429
scope,
429430
knowledgeBaseId: connector.knowledgeBaseId,
430431
isSearchIndex: true,
@@ -444,6 +445,7 @@ function SourceSettingsForm({
444445
dirty: form.dirty,
445446
saving: form.saving,
446447
saveDisabled: !form.canSave,
448+
saveTooltip: form.saveBlockedReason,
447449
onSave: form.save,
448450
onDiscard,
449451
})}

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-access-field/connector-access-field.test.tsx‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ describe('connection method selection', () => {
9898
isAvailabilityReady: false,
9999
})
100100
expect(container.textContent).not.toContain('This connection method is not available')
101-
expect(container.querySelector('[aria-label="Sync using: Service account"]')).toBeDisabled()
101+
expect(container.textContent).toContain('Service account')
102+
expect(container.querySelector('[role="combobox"]')).toBeNull()
102103
})
103104

104105
it('shows a real unavailable method after availability finishes loading', async () => {
@@ -119,15 +120,10 @@ describe('connection method selection', () => {
119120
{ mode: 'admin', label: 'Service account' },
120121
] as const)('shows a locked $mode method without allowing changes', async ({ mode, label }) => {
121122
await render({ value: { accessMode: mode }, lockAccessMode: true })
122-
const dropdown = container.querySelector<HTMLButtonElement>(
123-
`[aria-label="Sync using: ${label}"]`
124-
)
125-
expect(dropdown).toBeDisabled()
126-
expect(dropdown).toHaveTextContent(label)
127-
expect(container.textContent).toContain('Add a new connection to change the sync method.')
123+
expect(container.textContent).toContain(label)
124+
expect(container.textContent).not.toContain('Add a new connection')
128125
expect(container.querySelector('[role="radiogroup"]')).toBeNull()
129-
await act(async () => dropdown!.click())
130-
expect(document.querySelector('[role="menu"]')).toBeNull()
126+
expect(container.querySelector('button')).toBeNull()
131127
expect(onChange).not.toHaveBeenCalled()
132128
})
133129

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-access-field/connector-access-field.tsx‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {
55
ChipButtonGroup,
66
ChipButtonGroupItem,
77
ChipCombobox,
8-
ChipDropdown,
98
ChipLink,
109
ChipModalField,
1110
type ComboboxOption,
@@ -159,23 +158,13 @@ export function ConnectorAccessField({
159158
hint={
160159
canAdmin && isAvailabilityReady && !currentMode?.allowed
161160
? `This connection method is not available in this ${scope.kind}.`
162-
: lockAccessMode
163-
? 'Add a new connection to change the sync method.'
164-
: value.accessMode === 'workspace'
165-
? 'Everyone in this workspace can search these documents.'
166-
: undefined
161+
: value.accessMode === 'workspace'
162+
? 'Everyone in this workspace can search these documents.'
163+
: undefined
167164
}
168165
>
169166
<div className='flex flex-col gap-2'>
170-
{slackSetupOnly ? null : lockAccessMode ? (
171-
<ChipDropdown
172-
aria-label={`Sync using: ${currentMode?.label ?? 'Unavailable'}`}
173-
value={value.accessMode}
174-
options={visibleModes.map(({ mode, label }) => ({ value: mode, label }))}
175-
disabled
176-
className='w-fit'
177-
/>
178-
) : showModeSelector ? (
167+
{slackSetupOnly ? null : !lockAccessMode && showModeSelector ? (
179168
<ChipButtonGroup
180169
value={value.accessMode}
181170
disabled={disabled}

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/connector-settings-fields.test.tsx‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ describe('connector settings service-account choices', () => {
338338
it.each([true, false])(
339339
'locks the sync method only for Search settings (%s)',
340340
async (isSearchIndex) => {
341-
await render(confluenceConnectorMeta, { isSearchIndex })
341+
await render(confluenceConnectorMeta, { isSearchIndex, needsWorkspaceCredential: false })
342342
expect(mocks.accessField).toHaveBeenLastCalledWith(
343343
expect.objectContaining({ lockAccessMode: isSearchIndex })
344344
)
@@ -501,6 +501,35 @@ describe('connector settings service-account choices', () => {
501501
)
502502
})
503503

504+
it('browses spaces with the draft replacement account without a separate save action', async () => {
505+
mocks.renderConfigFields = true
506+
mocks.credentials = [
507+
{
508+
id: 'replacement',
509+
name: 'Updated account',
510+
provider: 'confluence',
511+
type: 'service_account',
512+
},
513+
]
514+
await render(confluenceConnectorMeta, {
515+
credentialId: 'previous',
516+
workspaceCredentialId: 'replacement',
517+
accessModeChanged: false,
518+
sourceConfig: { domain: 'https://example.atlassian.net', spaceKey: ['ENG'] },
519+
isFieldVisible: (field) => field.id === 'spaceSelector',
520+
})
521+
522+
expect(mocks.selectorOptions).toHaveBeenLastCalledWith(
523+
'confluence.spaces',
524+
expect.objectContaining({
525+
context: expect.objectContaining({ oauthCredential: 'replacement' }),
526+
})
527+
)
528+
expect(mocks.accessField).not.toHaveBeenCalled()
529+
expect(container.textContent).not.toContain('Change service account')
530+
expect(container.textContent).not.toContain('Cancel')
531+
})
532+
504533
it.each([
505534
{
506535
meta: confluenceConnectorMeta,

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/connector-settings-fields.tsx‎

Lines changed: 70 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ export interface ConnectorSettingsFieldsProps {
9191
hasMaxAccess: boolean
9292
isSaving: boolean
9393
error: string | null
94+
saveBlockedReason?: string
9495
access: ConnectorAccessSelection
9596
onAccessChange: (access: ConnectorAccessSelection) => void
9697
canAdmin: boolean
@@ -133,6 +134,7 @@ export function ConnectorSettingsFields({
133134
hasMaxAccess,
134135
isSaving,
135136
error,
137+
saveBlockedReason,
136138
access,
137139
onAccessChange,
138140
canAdmin,
@@ -204,7 +206,9 @@ export function ConnectorSettingsFields({
204206
})
205207
useCredentialRefreshTriggers(refetchCredentials, providerId ?? '', scope)
206208
const [browseCredentialId, setBrowseCredentialId] = useState<string | null>(null)
207-
const selectorCredentialId = syncsPerMember ? browseCredentialId : credentialId
209+
const selectorCredentialId = syncsPerMember
210+
? browseCredentialId
211+
: (workspaceCredentialId ?? credentialId)
208212
const selectorCredential = rawCredentials.find((item) => item.id === selectorCredentialId)
209213
const installations = rawCredentials.filter(
210214
(credential) => credential.provider === GITHUB_INSTALLATION_PROVIDER_ID
@@ -228,6 +232,7 @@ export function ConnectorSettingsFields({
228232
[rawCredentials, connectorConfig, access.accessMode]
229233
)
230234

235+
const hideFixedAccessMode = isSearchIndex && needsWorkspaceCredential && canAdmin && allowAdmin
231236
const hiddenCapFieldIds = derivedAclCapFieldIds(connectorConfig, access.accessMode)
232237
const isOptionalSetupField = (field: ConnectorConfigField) =>
233238
Boolean(gitlabPermissions && connectorConfig) &&
@@ -330,70 +335,67 @@ export function ConnectorSettingsFields({
330335
disabled={isSaving || !canAdmin}
331336
/>
332337
)}
333-
{connectorConfig && showAccessField && !isGitHubInstallationSource && (
334-
<ConnectorAccessField
335-
scope={scope}
336-
connectorConfig={connectorConfig}
337-
value={access}
338-
onChange={onAccessChange}
339-
canAdmin={canAdmin}
340-
lockAccessMode={isSearchIndex}
341-
isAvailabilityReady={availability.isReady}
342-
allowMembers={allowMembers}
343-
allowAdmin={allowAdmin}
344-
allowWorkspace={allowWorkspace}
345-
disabled={isSaving}
346-
footer={
347-
canReenableMemberSync ? (
348-
<div className='flex flex-col gap-2'>
349-
<div>
350-
<Chip
351-
variant='primary'
352-
onClick={onApplyAccess}
353-
disabled={!accessComplete || isSaving}
354-
>
355-
{isSwitchingAccess ? 'Re-enabling…' : 'Re-enable per-member sync'}
356-
</Chip>
357-
</div>
358-
<p className='text-[var(--text-muted)] text-caption leading-snug'>
359-
Members and their documents are kept; the next sync restores their access.
360-
</p>
361-
</div>
362-
) : accessDirty ? (
363-
<div className='flex flex-col gap-2'>
364-
<div className='flex items-center gap-2'>
365-
<Chip
366-
variant='primary'
367-
onClick={onApplyAccess}
368-
disabled={!accessComplete || isSaving}
369-
>
370-
{isSwitchingAccess
371-
? 'Switching…'
372-
: isContentCredentialChange
373-
? isSearchIndex
374-
? requiresServiceAccount
375-
? 'Change service account'
376-
: 'Change account'
377-
: 'Change indexing account'
378-
: 'Apply connection method'}
379-
</Chip>
380-
<Chip onClick={onResetAccess} disabled={isSaving}>
381-
{accessSetupHint ? 'Edit settings' : 'Cancel'}
382-
</Chip>
338+
{connectorConfig &&
339+
showAccessField &&
340+
!isGitHubInstallationSource &&
341+
!hideFixedAccessMode && (
342+
<ConnectorAccessField
343+
scope={scope}
344+
connectorConfig={connectorConfig}
345+
value={access}
346+
onChange={onAccessChange}
347+
canAdmin={canAdmin}
348+
lockAccessMode={isSearchIndex}
349+
isAvailabilityReady={availability.isReady}
350+
allowMembers={allowMembers}
351+
allowAdmin={allowAdmin}
352+
allowWorkspace={allowWorkspace}
353+
disabled={isSaving}
354+
footer={
355+
canReenableMemberSync ? (
356+
<div className='flex flex-col gap-2'>
357+
<div>
358+
<Chip
359+
variant='primary'
360+
onClick={onApplyAccess}
361+
disabled={!accessComplete || isSaving}
362+
>
363+
{isSwitchingAccess ? 'Re-enabling…' : 'Re-enable per-member sync'}
364+
</Chip>
365+
</div>
366+
<p className='text-[var(--text-muted)] text-caption leading-snug'>
367+
Members and their documents are kept; the next sync restores their access.
368+
</p>
383369
</div>
384-
<p className='text-[var(--text-muted)] text-caption leading-snug'>
385-
{accessSetupHint ??
386-
(isContentCredentialChange
387-
? syncsPerMember
370+
) : accessDirty && (!isContentCredentialChange || syncsPerMember) ? (
371+
<div className='flex flex-col gap-2'>
372+
<div className='flex items-center gap-2'>
373+
<Chip
374+
variant='primary'
375+
onClick={onApplyAccess}
376+
disabled={!accessComplete || isSaving}
377+
>
378+
{isSwitchingAccess
379+
? 'Switching…'
380+
: isContentCredentialChange
381+
? 'Change indexing account'
382+
: 'Apply connection method'}
383+
</Chip>
384+
<Chip onClick={onResetAccess} disabled={isSaving}>
385+
{accessSetupHint ? 'Edit settings' : 'Cancel'}
386+
</Chip>
387+
</div>
388+
<p className='text-[var(--text-muted)] text-caption leading-snug'>
389+
{accessSetupHint ??
390+
(isContentCredentialChange
388391
? 'The next sync uses this account. Members keep their connected accounts and source permissions.'
389-
: 'The next sync uses this account and refreshes source permissions.'
390-
: SWITCH_NOTICE[access.accessMode])}
391-
</p>
392-
</div>
393-
) : undefined
394-
}
395-
/>
396-
)}
392+
: SWITCH_NOTICE[access.accessMode])}
393+
</p>
394+
</div>
395+
) : undefined
396+
}
397+
/>
398+
)}
397399

398400
{connectorConfig && needsWorkspaceCredential && canAdmin && (
399401
<ChipModalField
@@ -551,6 +553,11 @@ export function ConnectorSettingsFields({
551553
</ChipModalField>
552554
)}
553555

556+
{saveBlockedReason && (
557+
<p role='status' className='px-2 text-[var(--text-muted)] text-caption'>
558+
{saveBlockedReason}
559+
</p>
560+
)}
554561
<ChipModalError>{error}</ChipModalError>
555562
</>
556563
)

0 commit comments

Comments
 (0)