Skip to content

Commit b7738b8

Browse files
fix(slack-search): simplify setup and align source readiness
1 parent 65e7f6b commit b7738b8

18 files changed

Lines changed: 508 additions & 180 deletions

File tree

apps/sim/app/api/knowledge/slack/onboarding/route.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { OrchestrationError } from '@/lib/core/orchestration/types'
2525
import { POST } from '@/app/api/knowledge/slack/onboarding/retry/route'
2626
import { GET } from '@/app/api/knowledge/slack/onboarding/route'
2727

28-
const token = 'bf1ff774-505b-4f2f-946d-9c54ed75de47'
28+
const token = '11111111-1111-4111-8111-111111111111'
2929
const url = `http://localhost/api/knowledge/slack/onboarding?token=${token}`
3030

3131
beforeEach(() => {
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/** @vitest-environment node */
2+
import { authMockFns, createMockRequest } from '@sim/testing'
3+
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
5+
const mocks = vi.hoisted(() => ({ prepare: vi.fn(), start: vi.fn() }))
6+
vi.mock('@/lib/knowledge/application/slack-search/setup', async () => {
7+
const { knowledgeOperations } = await import('@/lib/knowledge/application/operations')
8+
return {
9+
prepareSlackSearchSetup: {
10+
operation: knowledgeOperations.prepareSlackInstallation,
11+
execute: mocks.prepare,
12+
},
13+
startSlackSearchSetup: {
14+
operation: knowledgeOperations.startSlackInstallation,
15+
execute: mocks.start,
16+
},
17+
}
18+
})
19+
20+
import { createSlackSearchManifest } from '@/lib/slack-search/manifest'
21+
import { POST as start } from '@/app/api/knowledge/slack/oauth/route'
22+
import { POST as prepare } from '@/app/api/knowledge/slack/setup/route'
23+
24+
const input = { organizationId: 'organization-1', name: 'Sim Search', description: 'Search' }
25+
26+
beforeEach(() => {
27+
vi.clearAllMocks()
28+
authMockFns.mockGetSession.mockResolvedValue({
29+
user: { id: 'admin' },
30+
session: { id: 'session' },
31+
})
32+
})
33+
34+
describe.each([
35+
['prepare', prepare, mocks.prepare],
36+
['OAuth', start, mocks.start],
37+
] as const)('Slack %s route errors', (_name, route, execute) => {
38+
it('returns an actionable 400 for a non-HTTPS app URL', async () => {
39+
execute.mockImplementation(() =>
40+
createSlackSearchManifest(input.name, input.description, 'http://localhost:3000')
41+
)
42+
const response = await route(createMockRequest('POST', input))
43+
expect(response.status).toBe(400)
44+
expect(await response.json()).toMatchObject({ error: expect.stringContaining('public HTTPS') })
45+
expect(execute).toHaveBeenCalledOnce()
46+
})
47+
48+
it('still conceals unexpected errors', async () => {
49+
execute.mockRejectedValue(new Error('private database configuration'))
50+
const response = await route(createMockRequest('POST', input))
51+
expect(response.status).toBe(500)
52+
expect(await response.json()).toMatchObject({ error: 'Internal server error' })
53+
})
54+
55+
it('authenticates before exposing setup configuration', async () => {
56+
authMockFns.mockGetSession.mockResolvedValue(null)
57+
const response = await route(createMockRequest('POST', {}))
58+
expect(response.status).toBe(401)
59+
expect(execute).not.toHaveBeenCalled()
60+
})
61+
})

apps/sim/app/api/knowledge/utils.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '@sim/testing'
1616
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
1717
import * as billingAttributionModule from '@/lib/billing/core/billing-attribution'
18+
import * as apiKeysModule from '@/lib/core/config/api-keys'
1819
import { env } from '@/lib/core/config/env'
1920
import * as documentsUtilsModule from '@/lib/knowledge/documents/utils'
2021
import * as workspacesUtilsModule from '@/lib/workspaces/utils'
@@ -322,21 +323,20 @@ describe('Knowledge Utils', () => {
322323
})
323324

324325
it('should throw error when no API configuration provided', async () => {
325-
const { env } = await import('@/lib/core/config/env')
326-
Object.keys(env).forEach((key) => delete (env as any)[key])
327-
// The env object lazily reads process.env, so a developer's local .env
328-
// keys survive the deletion above — stub the direct key empty and fail
329-
// the hosted rotation fallback for hermeticity on any machine.
326+
Object.keys(env).forEach((key) => delete (env as Record<string, unknown>)[key])
327+
/** Prevent local credentials from satisfying the missing-configuration scenario. */
330328
vi.stubEnv('OPENAI_API_KEY', '')
331-
const apiKeysModule = await import('@/lib/core/config/api-keys')
329+
const configurationError = new Error('No rotation keys configured')
332330
const rotationSpy = vi.spyOn(apiKeysModule, 'getRotatingApiKey').mockImplementation(() => {
333-
throw new Error('No rotation keys configured')
331+
throw configurationError
334332
})
335333

336334
try {
337-
await expect(generateEmbeddings(['test text'], DEFAULT_EMBEDDING_TARGET)).rejects.toThrow(
338-
'OPENAI_API_KEY is not configured'
335+
await expect(generateEmbeddings(['test text'], DEFAULT_EMBEDDING_TARGET)).rejects.toBe(
336+
configurationError
339337
)
338+
expect(rotationSpy).toHaveBeenCalledWith('openai')
339+
expect(fetch).not.toHaveBeenCalled()
340340
} finally {
341341
rotationSpy.mockRestore()
342342
vi.unstubAllEnvs()

apps/sim/app/o/[organizationId]/integrations/integrations.test.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,26 @@ describe('organization integrations role and source paths', () => {
319319
expect(buttons('Connect account')).toHaveLength(0)
320320
expect(document.body.textContent).toContain('An admin needs to finish source setup')
321321
})
322+
it('takes admins directly to unfinished Slack indexing setup', async () => {
323+
mocks.context.mockReturnValue({
324+
organization: { id: scope.organizationId },
325+
viewer: { isAdmin: true },
326+
searchAccess: { memberScoped: true, sourceMirrored: true },
327+
})
328+
mocks.sources.mockReturnValue({ data: [], isPending: false })
329+
mocks.overview.mockReturnValue({ data: { providers: [] }, isPending: false })
330+
mocks.integrations.mockReturnValue({
331+
data: [{ connectorType: 'slack', approved: true }],
332+
isPending: false,
333+
})
334+
await render()
335+
expect(
336+
document.querySelector('a[href="/o/organization-a/settings/integrations/providers/slack"]')
337+
).toHaveTextContent('Finish Slack setup')
338+
expect(document.body.textContent).not.toContain('An admin needs to finish source setup')
339+
expect(buttons('Connect account')).toHaveLength(0)
340+
})
341+
322342
it('keeps personal rows consistent for admins and directs management through Sources', async () => {
323343
mocks.context.mockReturnValue({
324344
organization: { id: scope.organizationId },

apps/sim/app/o/[organizationId]/integrations/integrations.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,9 @@ export function OrganizationIntegrations({ slackOnboarding }: OrganizationIntegr
214214
? 'Connect a different site or content scope'
215215
: canConnect
216216
? 'Connect your account to search this source'
217-
: 'An admin needs to finish source setup'
217+
: type === 'slack' && viewer.isAdmin
218+
? 'Finish setting up Slack indexing to connect accounts'
219+
: 'An admin needs to finish source setup'
218220
}
219221
trailing={
220222
canConnect ? (
@@ -225,6 +227,8 @@ export function OrganizationIntegrations({ slackOnboarding }: OrganizationIntegr
225227
>
226228
{hasSources ? 'Add source' : 'Connect account'}
227229
</Chip>
230+
) : type === 'slack' && viewer.isAdmin ? (
231+
<ChipLink href={routes.searchProvider('slack')}>Finish Slack setup</ChipLink>
228232
) : undefined
229233
}
230234
/>

apps/sim/app/o/[organizationId]/integrations/page.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ vi.mock('next/navigation', () => ({
1616

1717
import OrganizationIntegrationsPage from '@/app/o/[organizationId]/integrations/page'
1818

19-
const token = 'bf1ff774-505b-4f2f-946d-9c54ed75de47'
19+
const token = '11111111-1111-4111-8111-111111111111'
2020
const props = {
2121
params: Promise.resolve({ organizationId: 'organization-a' }),
2222
searchParams: Promise.resolve({ slack: token }),
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/** @vitest-environment jsdom */
2+
import { act, type ReactNode } from 'react'
3+
import { createRoot, type Root } from 'react-dom/client'
4+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
5+
import type { SlackSearchInstallationView } from '@/lib/api/contracts/knowledge/slack'
6+
7+
const mocks = vi.hoisted(() => ({
8+
context: vi.fn(),
9+
list: vi.fn(),
10+
manifest: vi.fn(),
11+
configure: vi.fn(),
12+
remove: vi.fn(),
13+
install: vi.fn(),
14+
refetch: vi.fn(),
15+
removeError: null as Error | null,
16+
}))
17+
vi.mock('nuqs', () => ({ useQueryState: () => [null, vi.fn()] }))
18+
vi.mock('@/components/settings/settings-panel', () => ({
19+
SettingsPanel: ({ children }: { children: ReactNode }) => children,
20+
}))
21+
vi.mock('@/app/o/[organizationId]/providers/organization-provider', () => ({
22+
useOrganizationContext: mocks.context,
23+
}))
24+
vi.mock('@/hooks/queries/slack-search', () => ({
25+
useSlackSearchInstallations: mocks.list,
26+
useSlackSearchManifest: mocks.manifest,
27+
useConfigureSlackSearch: () => ({ mutate: mocks.configure, isPending: false }),
28+
useRemoveSlackSearch: () => ({
29+
mutate: mocks.remove,
30+
isPending: false,
31+
error: mocks.removeError,
32+
reset: vi.fn(),
33+
}),
34+
useStartSlackSearchOAuth: () => ({ mutate: mocks.install, isPending: false, reset: vi.fn() }),
35+
}))
36+
37+
import { OrganizationSearchSlack } from '@/app/o/[organizationId]/settings/components/organization-search-slack'
38+
39+
const installation: SlackSearchInstallationView = {
40+
id: 'installation-1',
41+
credentialId: 'credential-1',
42+
appId: 'A1',
43+
teamId: 'T1',
44+
teamName: 'Test workspace',
45+
enabled: true,
46+
needsValidation: false,
47+
lastOutcome: null,
48+
lastEventAt: null,
49+
}
50+
51+
let root: Root
52+
let container: HTMLDivElement
53+
beforeEach(() => {
54+
vi.clearAllMocks()
55+
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
56+
mocks.context.mockReturnValue({ organization: { id: 'org-1' }, viewer: { isAdmin: true } })
57+
mocks.list.mockReturnValue({ data: { installations: [], bots: [] } })
58+
mocks.manifest.mockReturnValue({
59+
data: { manifest: '{}', existingApp: null, createAppUrl: 'https://api.slack.com/apps' },
60+
isPending: false,
61+
refetch: mocks.refetch,
62+
})
63+
mocks.removeError = null
64+
container = document.createElement('div')
65+
document.body.appendChild(container)
66+
root = createRoot(container)
67+
})
68+
afterEach(async () => {
69+
await act(async () => root.unmount())
70+
container.remove()
71+
vi.unstubAllGlobals()
72+
})
73+
async function render(installed = false) {
74+
if (installed) {
75+
mocks.list.mockReturnValue({
76+
data: {
77+
installations: [installation],
78+
bots: [{ id: 'credential-1', displayName: 'Sim Search' }],
79+
},
80+
})
81+
}
82+
await act(async () => root.render(<OrganizationSearchSlack />))
83+
}
84+
function button(label: string) {
85+
const element = Array.from(document.querySelectorAll<HTMLButtonElement>('button')).find(
86+
(element) => element.textContent?.trim() === label
87+
)
88+
expect(element, label).toBeDefined()
89+
return element!
90+
}
91+
async function click(label: string) {
92+
await act(async () => button(label).click())
93+
}
94+
async function action(label: string) {
95+
const trigger = container.querySelector<HTMLButtonElement>('[aria-label="Sim Search actions"]')!
96+
await act(async () => {
97+
trigger.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))
98+
})
99+
const item = Array.from(document.querySelectorAll<HTMLElement>('[role="menuitem"]')).find(
100+
(item) => item.textContent?.trim() === label
101+
)
102+
expect(item, label).toBeDefined()
103+
await act(async () => item!.click())
104+
}
105+
106+
describe('Slack Search settings and shared wizard', () => {
107+
it('starts with one setup action and the prefilled manifest, with no name or token form', async () => {
108+
await render()
109+
expect(container.querySelectorAll('button')).toHaveLength(1)
110+
await click('Set up')
111+
expect(document.querySelector('[role="dialog"]')).toHaveTextContent('App manifest')
112+
expect(document.querySelectorAll('input')).toHaveLength(0)
113+
expect(mocks.manifest).toHaveBeenCalledWith('org-1', 'Sim Search')
114+
expect(mocks.install).not.toHaveBeenCalled()
115+
})
116+
117+
it('shows setup errors and blocks progression until the manifest loads', async () => {
118+
mocks.manifest.mockReturnValue({
119+
error: new Error('Slack needs a public HTTPS URL to send messages to Sim.'),
120+
refetch: mocks.refetch,
121+
isPending: false,
122+
})
123+
await render()
124+
await click('Set up')
125+
expect(document.querySelector('[role="alert"]')).toHaveTextContent('public HTTPS')
126+
expect(button('Continue').disabled).toBe(true)
127+
await click('Retry')
128+
expect(mocks.refetch).toHaveBeenCalledOnce()
129+
expect(mocks.install).not.toHaveBeenCalled()
130+
})
131+
132+
it('reconnects the existing app and credential without offering duplicate setup', async () => {
133+
await render(true)
134+
expect(container.textContent).not.toContain('Set up')
135+
expect(container.textContent).toContain('Enabled')
136+
await action('Reconnect')
137+
expect(document.querySelector('[role="dialog"]')).toHaveTextContent('Reconnect Slack Search')
138+
expect(document.querySelector('a[href="https://api.slack.com/apps/A1"]')).not.toBeNull()
139+
await click('Continue')
140+
expect(document.querySelector('[role="dialog"]')).toHaveTextContent('Leave fields blank')
141+
await click('Continue')
142+
await click('Install in Slack')
143+
expect(mocks.install).toHaveBeenCalledWith(
144+
expect.objectContaining({
145+
installationId: 'installation-1',
146+
organizationId: 'org-1',
147+
name: 'Sim Search',
148+
}),
149+
expect.any(Object)
150+
)
151+
expect(mocks.install.mock.calls[0][0]).not.toHaveProperty('clientSecret')
152+
})
153+
154+
it('disables the selected connection from the actions menu', async () => {
155+
await render(true)
156+
await action('Disable')
157+
expect(mocks.configure).toHaveBeenCalledExactlyOnceWith({
158+
organizationId: 'org-1',
159+
credentialId: 'credential-1',
160+
enabled: false,
161+
})
162+
})
163+
164+
it('requires confirmation to remove and keeps a failed removal visible inside the modal', async () => {
165+
await render(true)
166+
await action('Remove from Search')
167+
expect(mocks.remove).not.toHaveBeenCalled()
168+
await click('Remove')
169+
expect(mocks.remove).toHaveBeenCalledWith(
170+
{ organizationId: 'org-1', installationId: 'installation-1' },
171+
expect.any(Object)
172+
)
173+
mocks.removeError = new Error('Connection could not be removed')
174+
await render(true)
175+
expect(document.querySelector('[role="dialog"] [role="alert"]')).toHaveTextContent(
176+
'Connection could not be removed'
177+
)
178+
})
179+
})

0 commit comments

Comments
 (0)