Skip to content

Commit c24535f

Browse files
fix(slack): skip completed shared app verification setup
1 parent f45cab0 commit c24535f

2 files changed

Lines changed: 220 additions & 21 deletions

File tree

‎apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ const mocks = vi.hoisted(() => ({
1515
refetchApps: vi.fn(),
1616
manifest: vi.fn(),
1717
install: vi.fn(),
18+
accounts: vi.fn(),
19+
refetchAccounts: vi.fn(),
1820
}))
1921
vi.mock('@/hooks/queries/credential-groups', () => ({
2022
useStartSlackCredentialGroupConfiguration: () => ({
@@ -34,6 +36,11 @@ vi.mock('@/hooks/queries/slack-search', () => ({
3436
useStartSlackSearchOAuth: () => ({ mutate: mocks.install, isPending: false, reset: vi.fn() }),
3537
}))
3638

39+
vi.mock('@/hooks/queries/organization-accounts', () => ({
40+
organizationAccountsKeys: { detail: (id: string) => ['organization-accounts', id] },
41+
useOrganizationAccounts: mocks.accounts,
42+
}))
43+
3744
import type { WorkspaceCredential } from '@/lib/api/contracts/credentials'
3845
import {
3946
SLACK_MANAGED_USER_SCOPES,
@@ -69,6 +76,25 @@ describe('Slack member access selection', () => {
6976
vi.spyOn(toast, 'success').mockReturnValue('toast')
7077
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
7178
mocks.create.mockResolvedValue(undefined)
79+
mocks.accounts.mockReturnValue({
80+
isSuccess: true,
81+
isPending: false,
82+
isFetching: false,
83+
data: {
84+
credentialGroup: {
85+
id: 'group-1',
86+
options: [
87+
{
88+
provider: 'slack',
89+
status: 'active',
90+
configurationStatus: 'ready',
91+
},
92+
],
93+
},
94+
},
95+
error: null,
96+
refetch: mocks.refetchAccounts,
97+
})
7298
mocks.apps.mockReturnValue({
7399
isSuccess: true,
74100
isPending: false,
@@ -456,6 +482,7 @@ describe('Slack member access selection', () => {
456482
{
457483
id: 'installation-1',
458484
appId: 'A_APP',
485+
appKind: 'custom',
459486
teamId: 'T_TEAM',
460487
teamName: 'sim',
461488
credentialId: bot.id,
@@ -487,6 +514,137 @@ describe('Slack member access selection', () => {
487514
})
488515
})
489516

517+
it.each([false, true])(
518+
'skips completed shared app setup entirely (refreshed installation: %s)',
519+
async (refresh) => {
520+
const installation = {
521+
id: 'installation-1',
522+
appId: 'A_SHARED',
523+
appKind: 'shared',
524+
teamId: 'T_TEAM',
525+
teamName: 'sim',
526+
credentialId: bot.id,
527+
enabled: true,
528+
needsValidation: false,
529+
}
530+
if (refresh) {
531+
await render(undefined, [], 'org-1')
532+
expect(document.body.textContent).toContain('Install Sim Search first')
533+
}
534+
mocks.apps.mockReturnValue({
535+
isSuccess: true,
536+
isPending: false,
537+
data: { installations: [installation], bots: [bot], sharedAppAvailable: true },
538+
error: null,
539+
})
540+
await render(undefined, [], 'org-1')
541+
expect(document.querySelector('[role="dialog"]')).toBeNull()
542+
expect(mocks.onOpenChange).toHaveBeenCalledExactlyOnceWith(false)
543+
expect(mocks.start).not.toHaveBeenCalled()
544+
expect(mocks.install).not.toHaveBeenCalled()
545+
expect(window.open).not.toHaveBeenCalled()
546+
}
547+
)
548+
549+
it.each([
550+
{ enabled: false, needsValidation: false, sharedAppAvailable: true },
551+
{ enabled: true, needsValidation: true, sharedAppAvailable: true },
552+
{ enabled: true, needsValidation: false, sharedAppAvailable: false },
553+
])('keeps incomplete shared app setup actionable: %j', async (status) => {
554+
mocks.apps.mockReturnValue({
555+
isSuccess: true,
556+
isPending: false,
557+
data: {
558+
installations: [
559+
{
560+
id: 'installation-1',
561+
appId: 'A_SHARED',
562+
appKind: 'shared',
563+
teamId: 'T_TEAM',
564+
teamName: 'sim',
565+
credentialId: bot.id,
566+
enabled: status.enabled,
567+
needsValidation: status.needsValidation,
568+
},
569+
],
570+
bots: [bot],
571+
sharedAppAvailable: status.sharedAppAvailable,
572+
},
573+
error: null,
574+
})
575+
await render(undefined, [], 'org-1')
576+
expect(document.body.textContent).toContain('Manage Sim Search app')
577+
expect(document.body.textContent).not.toContain('Verify and add')
578+
expect(mocks.onOpenChange).not.toHaveBeenCalled()
579+
expect(mocks.start).not.toHaveBeenCalled()
580+
})
581+
582+
it.each(['removed', 'needs_update', 'pending', 'error', 'refreshing'])(
583+
'does not skip shared setup when member configuration is %s',
584+
async (state) => {
585+
mocks.apps.mockReturnValue({
586+
isSuccess: true,
587+
isPending: false,
588+
data: {
589+
installations: [
590+
{
591+
id: 'installation-1',
592+
appId: 'A_SHARED',
593+
appKind: 'shared',
594+
teamId: 'T_TEAM',
595+
teamName: 'sim',
596+
credentialId: bot.id,
597+
enabled: true,
598+
needsValidation: false,
599+
},
600+
],
601+
bots: [bot],
602+
sharedAppAvailable: true,
603+
},
604+
error: null,
605+
})
606+
const current = mocks.accounts()
607+
mocks.accounts.mockReturnValue({
608+
...current,
609+
isSuccess: !['pending', 'error'].includes(state),
610+
isPending: state === 'pending',
611+
isFetching: state === 'refreshing',
612+
error: state === 'error' ? new Error('Could not load member setup') : null,
613+
data:
614+
state === 'pending'
615+
? undefined
616+
: {
617+
credentialGroup: {
618+
id: 'group-1',
619+
options:
620+
state === 'removed'
621+
? []
622+
: [
623+
{
624+
provider: 'slack',
625+
status: 'active',
626+
configurationStatus:
627+
state === 'needs_update' ? 'needs_update' : 'ready',
628+
},
629+
],
630+
},
631+
},
632+
})
633+
await render(undefined, [], 'org-1')
634+
expect(mocks.onOpenChange).not.toHaveBeenCalled()
635+
expect(mocks.start).not.toHaveBeenCalled()
636+
if (state === 'error') {
637+
expect(document.body.textContent).toContain('Could not load member setup')
638+
await clickButton('Retry')
639+
expect(mocks.refetchAccounts).toHaveBeenCalledOnce()
640+
} else if (state === 'pending' || state === 'refreshing') {
641+
expect(document.body.textContent).toContain('Checking the installed Slack app')
642+
} else {
643+
expect(document.body.textContent).toContain('Manage Sim Search app')
644+
}
645+
}
646+
)
647+
490648
it('only changes existing workflow access after the user selects Search documents', async () => {
491649
await render(SLACK_MANAGED_USER_SCOPES)
492650
const access = Array.from(document.querySelectorAll('button')).find((node) =>

‎apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx‎

Lines changed: 62 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ import {
2626
} from '@/lib/credential-groups/slack-managed-user-scopes'
2727
import { ConnectSlackBotModal } from '@/app/workspace/[workspaceId]/integrations/components/connect-slack-bot-modal/connect-slack-bot-modal'
2828
import { useStartSlackCredentialGroupConfiguration } from '@/hooks/queries/credential-groups'
29-
import { organizationAccountsKeys } from '@/hooks/queries/organization-accounts'
29+
import {
30+
organizationAccountsKeys,
31+
useOrganizationAccounts,
32+
} from '@/hooks/queries/organization-accounts'
3033
import { useSlackSearchInstallations } from '@/hooks/queries/slack-search'
3134
import { credentialGroupKeys } from '@/hooks/queries/utils/credential-group-queries'
3235

@@ -101,6 +104,28 @@ export function SlackManagedUsersModal({
101104
const selectedApp =
102105
availableApps.find((app) => app.appId === appId) ??
103106
(availableApps.length === 1 && !appId ? availableApps[0] : undefined)
107+
const sharedAppInstalled = organizationSetup && selectedApp?.appKind === 'shared'
108+
const accounts = useOrganizationAccounts(open && sharedAppInstalled ? organizationId : undefined)
109+
const memberGroup = accounts.data?.credentialGroup
110+
const sharedAppReady = Boolean(
111+
sharedAppInstalled &&
112+
apps.isSuccess &&
113+
!apps.isFetching &&
114+
!apps.error &&
115+
apps.data?.sharedAppAvailable &&
116+
selectedApp.enabled &&
117+
!selectedApp.needsValidation &&
118+
accounts.isSuccess &&
119+
!accounts.isFetching &&
120+
!accounts.error &&
121+
memberGroup?.id === credentialGroupId &&
122+
memberGroup.options.some(
123+
(option) =>
124+
option.provider === 'slack' &&
125+
option.status === 'active' &&
126+
option.configurationStatus === 'ready'
127+
)
128+
)
104129
const [clientId, setClientId] = useState('')
105130
const [clientSecret, setClientSecret] = useState('')
106131
const [pending, setPending] = useState(false)
@@ -201,24 +226,27 @@ export function SlackManagedUsersModal({
201226
}
202227

203228
/**
204-
* The subscription's identity is `open` alone. Routing the handler through a
205-
* ref keeps a `bots` refetch from closing and reopening the channel mid-flow,
206-
* which would drop an already-queued authorization message from the popup.
229+
* Routing the handler through a ref keeps a bots refetch from reopening the
230+
* channel mid-flow and dropping an already-queued authorization message.
207231
*/
208232
const messageHandler = useRef(handleAuthorizationMessage)
209233
useEffect(() => {
210234
messageHandler.current = handleAuthorizationMessage
211235
})
212236

213237
useEffect(() => {
214-
if (!open) return
238+
if (!open || sharedAppReady) return
215239
const channel = new BroadcastChannel(CHANNEL_NAME)
216240
channel.onmessage = (event: MessageEvent<unknown>) => {
217241
if (!isSlackManagedUsersMessage(event.data)) return
218242
messageHandler.current(event.data)
219243
}
220244
return () => channel.close()
221-
}, [open])
245+
}, [open, sharedAppReady])
246+
247+
useEffect(() => {
248+
if (open && sharedAppReady && !appSetupOpen) onOpenChange(false)
249+
}, [open, sharedAppReady, appSetupOpen, onOpenChange])
222250

223251
useEffect(
224252
() => () => {
@@ -246,7 +274,7 @@ export function SlackManagedUsersModal({
246274
}
247275

248276
const handleSubmit = async () => {
249-
if (pending || (!organizationSetup && !selectedBot)) return
277+
if (pending || sharedAppInstalled || (!organizationSetup && !selectedBot)) return
250278
if (
251279
organizationSetup
252280
? !selectedApp || !requiredScopes.length
@@ -300,8 +328,14 @@ export function SlackManagedUsersModal({
300328
}
301329
}
302330

331+
if (sharedAppReady && !appSetupOpen) return null
332+
303333
const noBots = !organizationSetup && !isLoading && bots.length === 0
304334
const needsApp = organizationSetup && apps.isSuccess && availableApps.length === 0
335+
const checkingSetup =
336+
apps.isPending ||
337+
(sharedAppInstalled && (apps.isFetching || accounts.isPending || accounts.isFetching))
338+
const failedSetup = apps.error ? apps : sharedAppInstalled && accounts.error ? accounts : null
305339
const title = organizationSetup ? 'Set up Slack app' : 'Set up Slack'
306340
const primaryLabel = isLoading
307341
? 'Loading...'
@@ -330,15 +364,19 @@ export function SlackManagedUsersModal({
330364
</ChipModalHeader>
331365
<ChipModalBody>
332366
{organizationSetup ? (
333-
apps.isPending ? (
367+
checkingSetup ? (
334368
<ChipModalField type='custom' title='Sim Search app'>
335369
<p role='status' className='text-[var(--text-secondary)] text-sm'>
336370
Checking the installed Slack app…
337371
</p>
338372
</ChipModalField>
339-
) : apps.error ? (
340-
<ChipModalField type='custom' title='Sim Search app' error={apps.error.message}>
341-
<Chip onClick={() => void apps.refetch()} disabled={apps.isFetching}>
373+
) : failedSetup ? (
374+
<ChipModalField
375+
type='custom'
376+
title='Sim Search app'
377+
error={failedSetup.error?.message}
378+
>
379+
<Chip onClick={() => void failedSetup.refetch()} disabled={failedSetup.isFetching}>
342380
Retry
343381
</Chip>
344382
</ChipModalField>
@@ -375,8 +413,9 @@ export function SlackManagedUsersModal({
375413
)}
376414
<ChipModalField type='custom' title='Member accounts'>
377415
<p className='text-[var(--text-secondary)] text-sm'>
378-
Verify member authorization for the installed app. Each member can then connect
379-
their Slack account to index channels and DMs they can access.
416+
{sharedAppInstalled
417+
? 'The Sim Search installation needs attention. Manage the app to finish setup.'
418+
: 'Verify member authorization for the installed app. Each member can then connect their Slack account to index channels and DMs they can access.'}
380419
</p>
381420
{selectedApp && (
382421
<Chip onClick={() => setAppSetupOpen(true)} disabled={pending}>
@@ -473,15 +512,17 @@ export function SlackManagedUsersModal({
473512
onClick: () => setAppSetupOpen(true),
474513
},
475514
}
476-
: noBots
515+
: sharedAppInstalled
477516
? { defaultAction: 'dismiss' as const }
478-
: {
479-
primaryAction: {
480-
label: primaryLabel,
481-
onClick: () => void handleSubmit(),
482-
disabled: primaryDisabled,
483-
},
484-
})}
517+
: noBots
518+
? { defaultAction: 'dismiss' as const }
519+
: {
520+
primaryAction: {
521+
label: primaryLabel,
522+
onClick: () => void handleSubmit(),
523+
disabled: primaryDisabled,
524+
},
525+
})}
485526
/>
486527
</ChipModal>
487528
{open &&

0 commit comments

Comments
 (0)