Skip to content

Commit df6cc3a

Browse files
committed
fix(search): keep account views consistent and avoid false unsaved edits
1 parent 35f8d7d commit df6cc3a

6 files changed

Lines changed: 265 additions & 14 deletions

File tree

‎apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/provider-detail.test.tsx‎

Lines changed: 125 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,11 @@ describe('organization provider management', () => {
232232
expect(container.textContent).toContain('Loading integration')
233233
expect(container.textContent).not.toContain('Activate this integration')
234234
expect(mocks.people).not.toHaveBeenCalled()
235+
expect(
236+
container.querySelector(
237+
`input[placeholder="${params ? 'Search people...' : 'Search sources...'}"]`
238+
)
239+
).toBeEnabled()
235240
}
236241
)
237242

@@ -243,17 +248,21 @@ describe('organization provider management', () => {
243248
error: new ApiClientError({ status: 403, message: 'Access denied', body: null }),
244249
refetch,
245250
})
246-
await render('google_drive', '?view=accounts')
251+
await render('google_drive', '?view=accounts&credential-group-people=alex')
247252
expect(container.textContent).toContain('Access denied')
248253
expect(mocks.people).not.toHaveBeenCalled()
254+
expect(container.querySelector('input[placeholder="Search people..."]')).toHaveValue('alex')
255+
expect(container.querySelector('input[placeholder="Search people..."]')).toBeEnabled()
249256
await click('Try again')
250257
expect(refetch).toHaveBeenCalledOnce()
251258
})
252259

253260
it('offers activation from the accounts tab when an existing provider is deactivated', async () => {
254261
mocks.overview.mockReturnValue({ data: { providers: [{ ...provider, approved: false }] } })
255-
await render('google_drive', '?view=accounts')
262+
await render('google_drive', '?view=accounts&credential-group-people=alex')
256263
expect(mocks.people).not.toHaveBeenCalled()
264+
expect(container.querySelector('input[placeholder="Search people..."]')).toHaveValue('alex')
265+
expect(container.querySelector('input[placeholder="Search people..."]')).toBeEnabled()
257266
await click('Activate')
258267
expect(mocks.activate).toHaveBeenCalledWith({
259268
organizationId: 'org-one',
@@ -276,6 +285,46 @@ describe('organization provider management', () => {
276285
})
277286
})
278287

288+
it.each([
289+
['loading', 'Loading accounts…'],
290+
['error', 'Accounts unavailable'],
291+
['missing group', 'Add a source to set up account connections.'],
292+
['missing provider option', 'Add a source to set up account connections.'],
293+
])('preserves Accounts search while %s', async (state, message) => {
294+
const refetch = vi.fn()
295+
mocks.accounts.mockReturnValue(
296+
state === 'loading'
297+
? { isPending: true }
298+
: state === 'error'
299+
? { isError: true, error: new Error(message), refetch }
300+
: {
301+
data: {
302+
credentialGroup:
303+
state === 'missing group' ? null : { ...credentialGroup, options: [] },
304+
},
305+
isPending: false,
306+
}
307+
)
308+
await render('google_drive', '?view=accounts&credential-group-people=alex&search=handbook')
309+
310+
expect(container.textContent).toContain(message)
311+
expect(container.querySelector('input[placeholder="Search people..."]')).toHaveValue('alex')
312+
expect(container.querySelector('input[placeholder="Search people..."]')).toBeEnabled()
313+
expect(container.querySelector('input[placeholder="Search sources..."]')).toBeNull()
314+
expect(mocks.people).not.toHaveBeenCalled()
315+
if (state === 'error') {
316+
await click('Try again')
317+
expect(refetch).toHaveBeenCalledOnce()
318+
}
319+
320+
await click('Sources')
321+
expect(container.querySelector('input[placeholder="Search sources..."]')).toHaveValue(
322+
'handbook'
323+
)
324+
await click('Accounts')
325+
expect(container.querySelector('input[placeholder="Search people..."]')).toHaveValue('alex')
326+
})
327+
279328
it('preserves source navigation and retries connection availability failures', async () => {
280329
mocks.availabilityError = new Error('Connection availability could not be loaded')
281330
await render()
@@ -355,6 +404,80 @@ describe('organization provider management', () => {
355404
expect(query.has('addConnector')).toBe(false)
356405
})
357406

407+
it.each([
408+
{ name: 'missing bot', slackBotCredentialId: undefined, configurationStatus: 'ready' },
409+
{
410+
name: 'outdated app',
411+
slackBotCredentialId: 'slack-bot',
412+
configurationStatus: 'needs_update',
413+
},
414+
])('offers Slack Accounts recovery for an active option with $name', async (option) => {
415+
mocks.access = { admin: false, members: true }
416+
mocks.overview.mockReturnValue({
417+
data: { providers: [{ ...provider, connectorType: 'slack' }] },
418+
})
419+
mocks.accounts.mockReturnValue({
420+
data: {
421+
credentialGroup: {
422+
...credentialGroup,
423+
options: [
424+
{
425+
id: 'slack-option',
426+
provider: 'slack',
427+
status: 'active',
428+
slackBotCredentialId: option.slackBotCredentialId,
429+
configurationStatus: option.configurationStatus,
430+
},
431+
],
432+
},
433+
},
434+
isPending: false,
435+
})
436+
await render('slack', '?view=accounts&credential-group-people=alex')
437+
438+
expect(mocks.people).not.toHaveBeenCalled()
439+
expect(container.textContent).toContain('Set up the Slack app to connect accounts.')
440+
expect(container.querySelector('input[placeholder="Search people..."]')).toHaveValue('alex')
441+
await click('Set up Slack app')
442+
const query = new URLSearchParams(mocks.updateUrl.mock.calls.at(-1)![0].queryString)
443+
expect(query.get('connectedAccounts')).toBe('slack')
444+
expect(query.get('view')).toBe('accounts')
445+
expect(query.get('credential-group-people')).toBe('alex')
446+
expect(query.has('addConnector')).toBe(false)
447+
})
448+
449+
it('opens focused account management after the Slack app is ready', async () => {
450+
mocks.access = { admin: false, members: true }
451+
mocks.overview.mockReturnValue({
452+
data: { providers: [{ ...provider, connectorType: 'slack' }] },
453+
})
454+
mocks.accounts.mockReturnValue({
455+
data: {
456+
credentialGroup: {
457+
...credentialGroup,
458+
options: [
459+
{
460+
id: 'slack-option',
461+
provider: 'slack',
462+
status: 'active',
463+
slackBotCredentialId: 'slack-bot',
464+
configurationStatus: 'ready',
465+
},
466+
],
467+
},
468+
},
469+
isPending: false,
470+
})
471+
await render('slack', '?view=accounts')
472+
473+
expect(mocks.people).toHaveBeenCalledWith(
474+
expect.objectContaining({
475+
searchConnection: { optionId: 'slack-option', providerName: 'Slack' },
476+
})
477+
)
478+
expect(container.textContent).not.toContain('Set up the Slack app to connect accounts.')
479+
})
480+
358481
it('does not load admin queries or render setup controls for nonadmins', async () => {
359482
mocks.admin = false
360483
await render()

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

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,19 @@ describe('shared connector settings form', () => {
8080
let onSaved: ReturnType<typeof vi.fn>
8181
let baseline: ConnectorData
8282

83-
function Probe({ row }: { row: ConnectorData }) {
83+
function Probe({ row, isSearchIndex = true }: { row: ConnectorData; isSearchIndex?: boolean }) {
8484
form = useConnectorSettingsForm({
8585
scope: { kind: 'organization', organizationId: 'org-1' },
8686
knowledgeBaseId: 'kb-search',
87-
isSearchIndex: true,
87+
isSearchIndex,
8888
connector: row,
8989
onSaved,
9090
})
9191
return null
9292
}
9393

94-
function render(row = baseline, key = 'baseline') {
95-
act(() => root.render(<Probe key={key} row={row} />))
94+
function render(row = baseline, key = 'baseline', isSearchIndex = true) {
95+
act(() => root.render(<Probe key={key} row={row} isSearchIndex={isSearchIndex} />))
9696
}
9797

9898
beforeEach(() => {
@@ -134,6 +134,71 @@ describe('shared connector settings form', () => {
134134
expect(form.dirty).toBe(true)
135135
})
136136

137+
it('keeps a new member Gmail source clean and preserves its derived listing cap on save', () => {
138+
const sourceConfig = {
139+
label: ['INBOX'],
140+
dateRange: '7d',
141+
query: 'subject:SIM-SEARCH-QA',
142+
_canonicalModes: { label: 'advanced' },
143+
maxThreads: 0,
144+
}
145+
render(connector({ connectorType: 'gmail', sourceConfig }), 'gmail-members')
146+
147+
const capField = form.fieldsProps.connectorConfig?.configFields.find(
148+
(field) => field.id === 'maxThreads'
149+
)!
150+
expect(capField).toBeDefined()
151+
expect(form.fieldsProps.isFieldVisible(capField)).toBe(false)
152+
expect(form.dirty).toBe(false)
153+
expect(form.canSave).toBe(false)
154+
155+
act(() => form.fieldsProps.onFieldChange('query', 'subject:updated'))
156+
expect(form.dirty).toBe(true)
157+
expect(form.canSave).toBe(true)
158+
act(() => form.save())
159+
expect(mocks.update).toHaveBeenCalledWith(
160+
{
161+
knowledgeBaseId: 'kb-search',
162+
connectorId: baseline.id,
163+
updates: { sourceConfig: { ...sourceConfig, query: 'subject:updated' } },
164+
},
165+
expect.any(Object)
166+
)
167+
})
168+
169+
it('keeps general knowledge-base listing caps editable and includes their changes on save', () => {
170+
const sourceConfig = {
171+
label: ['INBOX'],
172+
_canonicalModes: { label: 'basic' },
173+
maxThreads: '100',
174+
}
175+
render(
176+
connector({ connectorType: 'gmail', accessMode: 'workspace', sourceConfig }),
177+
'gmail-workspace',
178+
false
179+
)
180+
181+
const capField = form.fieldsProps.connectorConfig?.configFields.find(
182+
(field) => field.id === 'maxThreads'
183+
)!
184+
expect(capField).toBeDefined()
185+
expect(form.fieldsProps.isFieldVisible(capField)).toBe(true)
186+
expect(form.dirty).toBe(false)
187+
188+
act(() => form.fieldsProps.onFieldChange('maxThreads', '200'))
189+
expect(form.dirty).toBe(true)
190+
expect(form.canSave).toBe(true)
191+
act(() => form.save())
192+
expect(mocks.update).toHaveBeenCalledWith(
193+
{
194+
knowledgeBaseId: 'kb-search',
195+
connectorId: baseline.id,
196+
updates: { sourceConfig: { ...sourceConfig, maxThreads: '200' } },
197+
},
198+
expect.any(Object)
199+
)
200+
})
201+
137202
it('guards access drafts separately from a settings save', () => {
138203
expect(form.dirty).toBe(false)
139204
expect(form.canSave).toBe(false)

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/use-connector-settings-form.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ export function useConnectorSettingsForm({
299299
if (didCanonicalModesChange(canonicalModes, persistedCanonicalModes)) return true
300300
const resolved = resolveSourceConfig()
301301
for (const [key, value] of Object.entries(resolved)) {
302+
if (hiddenCapFieldIds.has(key)) continue
302303
if (!valuesEqual(connector.sourceConfig[key], value)) return true
303304
}
304305
return false
@@ -309,6 +310,7 @@ export function useConnectorSettingsForm({
309310
connector.sourceConfig,
310311
canonicalModes,
311312
persistedCanonicalModes,
313+
hiddenCapFieldIds,
312314
])
313315

314316
const handleSave = useCallback(() => {
@@ -324,6 +326,7 @@ export function useConnectorSettingsForm({
324326
const resolved = resolveSourceConfig()
325327
const changedEntries: Record<string, unknown> = {}
326328
for (const [key, value] of Object.entries(resolved)) {
329+
if (hiddenCapFieldIds.has(key)) continue
327330
if (!valuesEqual(connector.sourceConfig[key], value)) changedEntries[key] = value
328331
}
329332

@@ -364,6 +367,7 @@ export function useConnectorSettingsForm({
364367
resolveSourceConfig,
365368
canonicalModes,
366369
persistedCanonicalModes,
370+
hiddenCapFieldIds,
367371
onSaved,
368372
updateConnector,
369373
knowledgeBaseId,

‎apps/sim/ee/credential-groups/components/organization-account-people.test.tsx‎

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,20 +108,24 @@ async function selectPersonAction(label: string) {
108108
)
109109
if (!action) throw new Error(`Missing ${label} action`)
110110
await act(async () => action.click())
111+
return action
111112
}
112113

113114
async function openConfirmation(label: string) {
114115
if (label === 'Revoke') await selectPersonAction(label)
115116
else await act(async () => button(container, label).click())
116117
}
117118

118-
async function renderPeople() {
119+
async function renderPeople(searchConnection?: { optionId: string; providerName: string }) {
119120
await act(async () =>
120121
root.render(
121122
<NuqsTestingAdapter hasMemory>
122123
<SettingsHeaderProvider>
123124
<SettingsHeaderShell>
124-
<OrganizationAccountPeople organizationId='organization-1' />
125+
<OrganizationAccountPeople
126+
organizationId='organization-1'
127+
searchConnection={searchConnection}
128+
/>
125129
</SettingsHeaderShell>
126130
</SettingsHeaderProvider>
127131
</NuqsTestingAdapter>
@@ -144,6 +148,7 @@ it('keeps the compact People rows and resends from the actions menu', async () =
144148
expect(container.textContent).toContain('2 accounts connected')
145149
expect(container.textContent).not.toContain('Copy new link')
146150
expect(container.textContent).not.toContain('gmail: active')
151+
expect(container.textContent).not.toContain('People (1)')
147152
await selectPersonAction('Resend')
148153
expect(mocks.resend).toHaveBeenCalledExactlyOnceWith(
149154
{ organizationId: 'organization-1', enrollmentId: 'enrollment-1' },
@@ -308,7 +313,64 @@ it('offers requests from an empty configured pool', async () => {
308313
await renderPeople()
309314

310315
expect(container.textContent).toContain('No people invited yet')
316+
expect(container.textContent).not.toContain('People (0)')
317+
expect(container.querySelector('input[placeholder="Search people..."]')).toBeEnabled()
318+
expect(button(container, 'Request connections')).toBeEnabled()
319+
})
320+
321+
it('loads more people below the ungrouped rows', async () => {
322+
const fetchNextPage = vi.fn()
323+
mocks.people.mockReturnValue({
324+
...mocks.people(),
325+
hasNextPage: true,
326+
isFetchingNextPage: false,
327+
fetchNextPage,
328+
})
329+
await renderPeople()
330+
331+
expect(container.textContent).toContain('person@example.com')
332+
expect(container.textContent).not.toContain('People (1+)')
333+
const personActions = container.querySelector('[aria-label="person@example.com actions"]')
334+
const loadMore = button(container, 'Load more')
335+
expect(personActions).not.toBeNull()
336+
expect(personActions!.compareDocumentPosition(loadMore) & Node.DOCUMENT_POSITION_FOLLOWING).toBe(
337+
Node.DOCUMENT_POSITION_FOLLOWING
338+
)
339+
expect(loadMore).toBeEnabled()
340+
await act(async () => loadMore.click())
341+
expect(fetchNextPage).toHaveBeenCalledOnce()
342+
})
343+
344+
it.each([
345+
{ view: 'all people', searchConnection: undefined },
346+
{
347+
view: 'provider accounts',
348+
searchConnection: { optionId: 'gmail-option', providerName: 'Gmail' },
349+
},
350+
])('disables resending revoked invitations in $view', async ({ searchConnection }) => {
351+
mocks.people.mockReturnValue({
352+
data: {
353+
pages: [
354+
{
355+
enrollments: [
356+
{
357+
id: 'enrollment-1',
358+
email: 'person@example.com',
359+
status: 'revoked',
360+
connections: [],
361+
mcpConnections: [],
362+
},
363+
],
364+
},
365+
],
366+
},
367+
})
368+
await renderPeople(searchConnection)
369+
311370
expect(button(container, 'Request connections')).toBeEnabled()
371+
const resend = await selectPersonAction('Resend')
372+
expect(resend).toHaveAttribute('aria-disabled', 'true')
373+
expect(mocks.resend).not.toHaveBeenCalled()
312374
})
313375

314376
it('hides cached people after a first-page authorization failure and offers a retry', async () => {
@@ -440,7 +502,6 @@ it.each([
440502
optionId: 'gmail-option',
441503
})
442504
expect(container.textContent).toContain('Gmail')
443-
expect(container.textContent).toContain('Organization account contributors')
444505
expect(container.textContent).toContain(label)
445506
expect(container.textContent).not.toContain('No people invited')
446507
}

0 commit comments

Comments
 (0)