Skip to content

Commit d7fb4f7

Browse files
fix(slack): skip completed shared app verification setup (#7756)
* fix(slack): skip completed shared app verification setup * fix(slack): preserve outdated member access repair
1 parent d7d66cf commit d7fb4f7

2 files changed

Lines changed: 259 additions & 22 deletions

File tree

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

Lines changed: 187 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,166 @@ 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+
const accounts = mocks.accounts()
555+
accounts.data.credentialGroup.options[0].configurationStatus = 'needs_update'
556+
mocks.apps.mockReturnValue({
557+
isSuccess: true,
558+
isPending: false,
559+
data: {
560+
installations: [
561+
{
562+
id: 'installation-1',
563+
appId: 'A_SHARED',
564+
appKind: 'shared',
565+
teamId: 'T_TEAM',
566+
teamName: 'sim',
567+
credentialId: bot.id,
568+
enabled: status.enabled,
569+
needsValidation: status.needsValidation,
570+
},
571+
],
572+
bots: [bot],
573+
sharedAppAvailable: status.sharedAppAvailable,
574+
},
575+
error: null,
576+
})
577+
await render(undefined, [], 'org-1')
578+
expect(document.body.textContent).toContain('Manage Sim Search app')
579+
expect(document.body.textContent).not.toContain('Verify and add')
580+
expect(document.body.textContent).not.toContain('Update member access')
581+
expect(mocks.onOpenChange).not.toHaveBeenCalled()
582+
expect(mocks.start).not.toHaveBeenCalled()
583+
})
584+
585+
it.each(['removed', 'needs_update', 'needs_update_failed', 'pending', 'error', 'refreshing'])(
586+
'does not skip shared setup when member configuration is %s',
587+
async (state) => {
588+
mocks.apps.mockReturnValue({
589+
isSuccess: true,
590+
isPending: false,
591+
data: {
592+
installations: [
593+
{
594+
id: 'installation-1',
595+
appId: 'A_SHARED',
596+
appKind: 'shared',
597+
teamId: 'T_TEAM',
598+
teamName: 'sim',
599+
credentialId: bot.id,
600+
enabled: true,
601+
needsValidation: false,
602+
},
603+
],
604+
bots: [bot],
605+
sharedAppAvailable: true,
606+
},
607+
error: null,
608+
})
609+
const current = mocks.accounts()
610+
mocks.accounts.mockReturnValue({
611+
...current,
612+
isSuccess: !['pending', 'error'].includes(state),
613+
isPending: state === 'pending',
614+
isFetching: state === 'refreshing',
615+
error: state === 'error' ? new Error('Could not load member setup') : null,
616+
data:
617+
state === 'pending'
618+
? undefined
619+
: {
620+
credentialGroup: {
621+
id: 'group-1',
622+
options:
623+
state === 'removed'
624+
? []
625+
: [
626+
{
627+
provider: 'slack',
628+
status: 'active',
629+
configurationStatus: 'needs_update',
630+
},
631+
],
632+
},
633+
},
634+
})
635+
await render(undefined, [], 'org-1')
636+
expect(mocks.onOpenChange).not.toHaveBeenCalled()
637+
expect(mocks.start).not.toHaveBeenCalled()
638+
if (state === 'error') {
639+
expect(document.body.textContent).toContain('Could not load member setup')
640+
expect(document.body.textContent).not.toContain('Update member access')
641+
await clickButton('Retry')
642+
expect(mocks.refetchAccounts).toHaveBeenCalledOnce()
643+
} else if (state === 'pending' || state === 'refreshing') {
644+
expect(document.body.textContent).toContain('Checking the installed Slack app')
645+
expect(document.body.textContent).not.toContain('Update member access')
646+
} else if (state === 'needs_update' || state === 'needs_update_failed') {
647+
expect(document.body.textContent).toContain('Member access is outdated')
648+
if (state === 'needs_update_failed')
649+
mocks.start.mockRejectedValueOnce(new Error('Try again'))
650+
await clickButton('Update member access')
651+
expect(mocks.start).toHaveBeenCalledExactlyOnceWith({
652+
organizationId: 'org-1',
653+
credentialGroupId: 'group-1',
654+
body: {
655+
appId: 'A_SHARED',
656+
teamId: 'T_TEAM',
657+
requiredScopes: [...SLACK_SEARCH_USER_SCOPES],
658+
},
659+
})
660+
expect(mocks.install).not.toHaveBeenCalled()
661+
if (state === 'needs_update_failed') {
662+
expect(toast.error).toHaveBeenCalledWith('Try again')
663+
expect(popup.close).toHaveBeenCalledOnce()
664+
expect(mocks.onOpenChange).not.toHaveBeenCalled()
665+
await clickButton('Update member access')
666+
}
667+
await completeAuthorization()
668+
expect(toast.success).toHaveBeenCalledWith('Slack configured')
669+
expect(mocks.onOpenChange).toHaveBeenCalledWith(false)
670+
} else {
671+
expect(document.body.textContent).toContain('Manage Sim Search app')
672+
expect(document.body.textContent).not.toContain('Update member access')
673+
}
674+
}
675+
)
676+
490677
it('only changes existing workflow access after the user selects Search documents', async () => {
491678
await render(SLACK_MANAGED_USER_SCOPES)
492679
const access = Array.from(document.querySelectorAll('button')).find((node) =>

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

Lines changed: 72 additions & 22 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 memberOption = memberGroup?.options.find(
111+
(option) => option.provider === 'slack' && option.status === 'active'
112+
)
113+
const sharedAppCanAuthorize = Boolean(
114+
sharedAppInstalled &&
115+
apps.isSuccess &&
116+
!apps.isFetching &&
117+
!apps.error &&
118+
apps.data?.sharedAppAvailable &&
119+
selectedApp.enabled &&
120+
!selectedApp.needsValidation &&
121+
accounts.isSuccess &&
122+
!accounts.isFetching &&
123+
!accounts.error &&
124+
memberGroup?.id === credentialGroupId
125+
)
126+
const sharedAppReady = sharedAppCanAuthorize && memberOption?.configurationStatus === 'ready'
127+
const sharedAppNeedsUpdate =
128+
sharedAppCanAuthorize && memberOption?.configurationStatus === 'needs_update'
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,12 @@ export function SlackManagedUsersModal({
246274
}
247275

248276
const handleSubmit = async () => {
249-
if (pending || (!organizationSetup && !selectedBot)) return
277+
if (
278+
pending ||
279+
(sharedAppInstalled && !sharedAppNeedsUpdate) ||
280+
(!organizationSetup && !selectedBot)
281+
)
282+
return
250283
if (
251284
organizationSetup
252285
? !selectedApp || !requiredScopes.length
@@ -300,14 +333,22 @@ export function SlackManagedUsersModal({
300333
}
301334
}
302335

336+
if (sharedAppReady && !appSetupOpen) return null
337+
303338
const noBots = !organizationSetup && !isLoading && bots.length === 0
304339
const needsApp = organizationSetup && apps.isSuccess && availableApps.length === 0
340+
const checkingSetup =
341+
apps.isPending ||
342+
(sharedAppInstalled && (apps.isFetching || accounts.isPending || accounts.isFetching))
343+
const failedSetup = apps.error ? apps : sharedAppInstalled && accounts.error ? accounts : null
305344
const title = organizationSetup ? 'Set up Slack app' : 'Set up Slack'
306345
const primaryLabel = isLoading
307346
? 'Loading...'
308347
: pending
309348
? 'Waiting for Slack...'
310-
: 'Verify and add'
349+
: sharedAppNeedsUpdate
350+
? 'Update member access'
351+
: 'Verify and add'
311352
const primaryDisabled =
312353
isLoading ||
313354
noBots ||
@@ -330,15 +371,19 @@ export function SlackManagedUsersModal({
330371
</ChipModalHeader>
331372
<ChipModalBody>
332373
{organizationSetup ? (
333-
apps.isPending ? (
374+
checkingSetup ? (
334375
<ChipModalField type='custom' title='Sim Search app'>
335376
<p role='status' className='text-[var(--text-secondary)] text-sm'>
336377
Checking the installed Slack app…
337378
</p>
338379
</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}>
380+
) : failedSetup ? (
381+
<ChipModalField
382+
type='custom'
383+
title='Sim Search app'
384+
error={failedSetup.error?.message}
385+
>
386+
<Chip onClick={() => void failedSetup.refetch()} disabled={failedSetup.isFetching}>
342387
Retry
343388
</Chip>
344389
</ChipModalField>
@@ -375,8 +420,11 @@ export function SlackManagedUsersModal({
375420
)}
376421
<ChipModalField type='custom' title='Member accounts'>
377422
<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.
423+
{sharedAppInstalled
424+
? sharedAppNeedsUpdate
425+
? 'Member access is outdated. Update it so members can reconnect their Slack accounts.'
426+
: 'The Sim Search installation needs attention. Manage the app to finish setup.'
427+
: 'Verify member authorization for the installed app. Each member can then connect their Slack account to index channels and DMs they can access.'}
380428
</p>
381429
{selectedApp && (
382430
<Chip onClick={() => setAppSetupOpen(true)} disabled={pending}>
@@ -473,15 +521,17 @@ export function SlackManagedUsersModal({
473521
onClick: () => setAppSetupOpen(true),
474522
},
475523
}
476-
: noBots
524+
: sharedAppInstalled && !sharedAppNeedsUpdate
477525
? { defaultAction: 'dismiss' as const }
478-
: {
479-
primaryAction: {
480-
label: primaryLabel,
481-
onClick: () => void handleSubmit(),
482-
disabled: primaryDisabled,
483-
},
484-
})}
526+
: noBots
527+
? { defaultAction: 'dismiss' as const }
528+
: {
529+
primaryAction: {
530+
label: primaryLabel,
531+
onClick: () => void handleSubmit(),
532+
disabled: primaryDisabled,
533+
},
534+
})}
485535
/>
486536
</ChipModal>
487537
{open &&

0 commit comments

Comments
 (0)