From c24535fccd97572d362d99d8f6bc427f492df6be Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Thu, 10 Sep 2026 19:12:36 -0700 Subject: [PATCH 1/2] fix(slack): skip completed shared app verification setup --- .../slack-managed-users-access.test.tsx | 158 ++++++++++++++++++ .../components/slack-managed-users-modal.tsx | 83 ++++++--- 2 files changed, 220 insertions(+), 21 deletions(-) diff --git a/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx b/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx index 2314c7d445c..164006f9af5 100644 --- a/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx +++ b/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx @@ -15,6 +15,8 @@ const mocks = vi.hoisted(() => ({ refetchApps: vi.fn(), manifest: vi.fn(), install: vi.fn(), + accounts: vi.fn(), + refetchAccounts: vi.fn(), })) vi.mock('@/hooks/queries/credential-groups', () => ({ useStartSlackCredentialGroupConfiguration: () => ({ @@ -34,6 +36,11 @@ vi.mock('@/hooks/queries/slack-search', () => ({ useStartSlackSearchOAuth: () => ({ mutate: mocks.install, isPending: false, reset: vi.fn() }), })) +vi.mock('@/hooks/queries/organization-accounts', () => ({ + organizationAccountsKeys: { detail: (id: string) => ['organization-accounts', id] }, + useOrganizationAccounts: mocks.accounts, +})) + import type { WorkspaceCredential } from '@/lib/api/contracts/credentials' import { SLACK_MANAGED_USER_SCOPES, @@ -69,6 +76,25 @@ describe('Slack member access selection', () => { vi.spyOn(toast, 'success').mockReturnValue('toast') vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true) mocks.create.mockResolvedValue(undefined) + mocks.accounts.mockReturnValue({ + isSuccess: true, + isPending: false, + isFetching: false, + data: { + credentialGroup: { + id: 'group-1', + options: [ + { + provider: 'slack', + status: 'active', + configurationStatus: 'ready', + }, + ], + }, + }, + error: null, + refetch: mocks.refetchAccounts, + }) mocks.apps.mockReturnValue({ isSuccess: true, isPending: false, @@ -456,6 +482,7 @@ describe('Slack member access selection', () => { { id: 'installation-1', appId: 'A_APP', + appKind: 'custom', teamId: 'T_TEAM', teamName: 'sim', credentialId: bot.id, @@ -487,6 +514,137 @@ describe('Slack member access selection', () => { }) }) + it.each([false, true])( + 'skips completed shared app setup entirely (refreshed installation: %s)', + async (refresh) => { + const installation = { + id: 'installation-1', + appId: 'A_SHARED', + appKind: 'shared', + teamId: 'T_TEAM', + teamName: 'sim', + credentialId: bot.id, + enabled: true, + needsValidation: false, + } + if (refresh) { + await render(undefined, [], 'org-1') + expect(document.body.textContent).toContain('Install Sim Search first') + } + mocks.apps.mockReturnValue({ + isSuccess: true, + isPending: false, + data: { installations: [installation], bots: [bot], sharedAppAvailable: true }, + error: null, + }) + await render(undefined, [], 'org-1') + expect(document.querySelector('[role="dialog"]')).toBeNull() + expect(mocks.onOpenChange).toHaveBeenCalledExactlyOnceWith(false) + expect(mocks.start).not.toHaveBeenCalled() + expect(mocks.install).not.toHaveBeenCalled() + expect(window.open).not.toHaveBeenCalled() + } + ) + + it.each([ + { enabled: false, needsValidation: false, sharedAppAvailable: true }, + { enabled: true, needsValidation: true, sharedAppAvailable: true }, + { enabled: true, needsValidation: false, sharedAppAvailable: false }, + ])('keeps incomplete shared app setup actionable: %j', async (status) => { + mocks.apps.mockReturnValue({ + isSuccess: true, + isPending: false, + data: { + installations: [ + { + id: 'installation-1', + appId: 'A_SHARED', + appKind: 'shared', + teamId: 'T_TEAM', + teamName: 'sim', + credentialId: bot.id, + enabled: status.enabled, + needsValidation: status.needsValidation, + }, + ], + bots: [bot], + sharedAppAvailable: status.sharedAppAvailable, + }, + error: null, + }) + await render(undefined, [], 'org-1') + expect(document.body.textContent).toContain('Manage Sim Search app') + expect(document.body.textContent).not.toContain('Verify and add') + expect(mocks.onOpenChange).not.toHaveBeenCalled() + expect(mocks.start).not.toHaveBeenCalled() + }) + + it.each(['removed', 'needs_update', 'pending', 'error', 'refreshing'])( + 'does not skip shared setup when member configuration is %s', + async (state) => { + mocks.apps.mockReturnValue({ + isSuccess: true, + isPending: false, + data: { + installations: [ + { + id: 'installation-1', + appId: 'A_SHARED', + appKind: 'shared', + teamId: 'T_TEAM', + teamName: 'sim', + credentialId: bot.id, + enabled: true, + needsValidation: false, + }, + ], + bots: [bot], + sharedAppAvailable: true, + }, + error: null, + }) + const current = mocks.accounts() + mocks.accounts.mockReturnValue({ + ...current, + isSuccess: !['pending', 'error'].includes(state), + isPending: state === 'pending', + isFetching: state === 'refreshing', + error: state === 'error' ? new Error('Could not load member setup') : null, + data: + state === 'pending' + ? undefined + : { + credentialGroup: { + id: 'group-1', + options: + state === 'removed' + ? [] + : [ + { + provider: 'slack', + status: 'active', + configurationStatus: + state === 'needs_update' ? 'needs_update' : 'ready', + }, + ], + }, + }, + }) + await render(undefined, [], 'org-1') + expect(mocks.onOpenChange).not.toHaveBeenCalled() + expect(mocks.start).not.toHaveBeenCalled() + if (state === 'error') { + expect(document.body.textContent).toContain('Could not load member setup') + await clickButton('Retry') + expect(mocks.refetchAccounts).toHaveBeenCalledOnce() + } else if (state === 'pending' || state === 'refreshing') { + expect(document.body.textContent).toContain('Checking the installed Slack app') + } else { + expect(document.body.textContent).toContain('Manage Sim Search app') + } + } + ) + it('only changes existing workflow access after the user selects Search documents', async () => { await render(SLACK_MANAGED_USER_SCOPES) const access = Array.from(document.querySelectorAll('button')).find((node) => diff --git a/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx b/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx index cd2fd9c6c0b..dea7da5248b 100644 --- a/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx +++ b/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx @@ -26,7 +26,10 @@ import { } from '@/lib/credential-groups/slack-managed-user-scopes' import { ConnectSlackBotModal } from '@/app/workspace/[workspaceId]/integrations/components/connect-slack-bot-modal/connect-slack-bot-modal' import { useStartSlackCredentialGroupConfiguration } from '@/hooks/queries/credential-groups' -import { organizationAccountsKeys } from '@/hooks/queries/organization-accounts' +import { + organizationAccountsKeys, + useOrganizationAccounts, +} from '@/hooks/queries/organization-accounts' import { useSlackSearchInstallations } from '@/hooks/queries/slack-search' import { credentialGroupKeys } from '@/hooks/queries/utils/credential-group-queries' @@ -101,6 +104,28 @@ export function SlackManagedUsersModal({ const selectedApp = availableApps.find((app) => app.appId === appId) ?? (availableApps.length === 1 && !appId ? availableApps[0] : undefined) + const sharedAppInstalled = organizationSetup && selectedApp?.appKind === 'shared' + const accounts = useOrganizationAccounts(open && sharedAppInstalled ? organizationId : undefined) + const memberGroup = accounts.data?.credentialGroup + const sharedAppReady = Boolean( + sharedAppInstalled && + apps.isSuccess && + !apps.isFetching && + !apps.error && + apps.data?.sharedAppAvailable && + selectedApp.enabled && + !selectedApp.needsValidation && + accounts.isSuccess && + !accounts.isFetching && + !accounts.error && + memberGroup?.id === credentialGroupId && + memberGroup.options.some( + (option) => + option.provider === 'slack' && + option.status === 'active' && + option.configurationStatus === 'ready' + ) + ) const [clientId, setClientId] = useState('') const [clientSecret, setClientSecret] = useState('') const [pending, setPending] = useState(false) @@ -201,9 +226,8 @@ export function SlackManagedUsersModal({ } /** - * The subscription's identity is `open` alone. Routing the handler through a - * ref keeps a `bots` refetch from closing and reopening the channel mid-flow, - * which would drop an already-queued authorization message from the popup. + * Routing the handler through a ref keeps a bots refetch from reopening the + * channel mid-flow and dropping an already-queued authorization message. */ const messageHandler = useRef(handleAuthorizationMessage) useEffect(() => { @@ -211,14 +235,18 @@ export function SlackManagedUsersModal({ }) useEffect(() => { - if (!open) return + if (!open || sharedAppReady) return const channel = new BroadcastChannel(CHANNEL_NAME) channel.onmessage = (event: MessageEvent) => { if (!isSlackManagedUsersMessage(event.data)) return messageHandler.current(event.data) } return () => channel.close() - }, [open]) + }, [open, sharedAppReady]) + + useEffect(() => { + if (open && sharedAppReady && !appSetupOpen) onOpenChange(false) + }, [open, sharedAppReady, appSetupOpen, onOpenChange]) useEffect( () => () => { @@ -246,7 +274,7 @@ export function SlackManagedUsersModal({ } const handleSubmit = async () => { - if (pending || (!organizationSetup && !selectedBot)) return + if (pending || sharedAppInstalled || (!organizationSetup && !selectedBot)) return if ( organizationSetup ? !selectedApp || !requiredScopes.length @@ -300,8 +328,14 @@ export function SlackManagedUsersModal({ } } + if (sharedAppReady && !appSetupOpen) return null + const noBots = !organizationSetup && !isLoading && bots.length === 0 const needsApp = organizationSetup && apps.isSuccess && availableApps.length === 0 + const checkingSetup = + apps.isPending || + (sharedAppInstalled && (apps.isFetching || accounts.isPending || accounts.isFetching)) + const failedSetup = apps.error ? apps : sharedAppInstalled && accounts.error ? accounts : null const title = organizationSetup ? 'Set up Slack app' : 'Set up Slack' const primaryLabel = isLoading ? 'Loading...' @@ -330,15 +364,19 @@ export function SlackManagedUsersModal({ {organizationSetup ? ( - apps.isPending ? ( + checkingSetup ? (

Checking the installed Slack app…

- ) : apps.error ? ( - - void apps.refetch()} disabled={apps.isFetching}> + ) : failedSetup ? ( + + void failedSetup.refetch()} disabled={failedSetup.isFetching}> Retry @@ -375,8 +413,9 @@ export function SlackManagedUsersModal({ )}

- Verify member authorization for the installed app. Each member can then connect - their Slack account to index channels and DMs they can access. + {sharedAppInstalled + ? 'The Sim Search installation needs attention. Manage the app to finish setup.' + : 'Verify member authorization for the installed app. Each member can then connect their Slack account to index channels and DMs they can access.'}

{selectedApp && ( setAppSetupOpen(true)} disabled={pending}> @@ -473,15 +512,17 @@ export function SlackManagedUsersModal({ onClick: () => setAppSetupOpen(true), }, } - : noBots + : sharedAppInstalled ? { defaultAction: 'dismiss' as const } - : { - primaryAction: { - label: primaryLabel, - onClick: () => void handleSubmit(), - disabled: primaryDisabled, - }, - })} + : noBots + ? { defaultAction: 'dismiss' as const } + : { + primaryAction: { + label: primaryLabel, + onClick: () => void handleSubmit(), + disabled: primaryDisabled, + }, + })} /> {open && From e8d346d537c836b9400170fce9b31ae2f12d9357 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Thu, 10 Sep 2026 19:28:53 -0700 Subject: [PATCH 2/2] fix(slack): preserve outdated member access repair --- .../slack-managed-users-access.test.tsx | 35 +++++++++++++++++-- .../components/slack-managed-users-modal.tsx | 33 ++++++++++------- 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx b/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx index 164006f9af5..a213b3fcb71 100644 --- a/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx +++ b/apps/sim/ee/credential-groups/components/slack-managed-users-access.test.tsx @@ -551,6 +551,8 @@ describe('Slack member access selection', () => { { enabled: true, needsValidation: true, sharedAppAvailable: true }, { enabled: true, needsValidation: false, sharedAppAvailable: false }, ])('keeps incomplete shared app setup actionable: %j', async (status) => { + const accounts = mocks.accounts() + accounts.data.credentialGroup.options[0].configurationStatus = 'needs_update' mocks.apps.mockReturnValue({ isSuccess: true, isPending: false, @@ -575,11 +577,12 @@ describe('Slack member access selection', () => { await render(undefined, [], 'org-1') expect(document.body.textContent).toContain('Manage Sim Search app') expect(document.body.textContent).not.toContain('Verify and add') + expect(document.body.textContent).not.toContain('Update member access') expect(mocks.onOpenChange).not.toHaveBeenCalled() expect(mocks.start).not.toHaveBeenCalled() }) - it.each(['removed', 'needs_update', 'pending', 'error', 'refreshing'])( + it.each(['removed', 'needs_update', 'needs_update_failed', 'pending', 'error', 'refreshing'])( 'does not skip shared setup when member configuration is %s', async (state) => { mocks.apps.mockReturnValue({ @@ -623,8 +626,7 @@ describe('Slack member access selection', () => { { provider: 'slack', status: 'active', - configurationStatus: - state === 'needs_update' ? 'needs_update' : 'ready', + configurationStatus: 'needs_update', }, ], }, @@ -635,12 +637,39 @@ describe('Slack member access selection', () => { expect(mocks.start).not.toHaveBeenCalled() if (state === 'error') { expect(document.body.textContent).toContain('Could not load member setup') + expect(document.body.textContent).not.toContain('Update member access') await clickButton('Retry') expect(mocks.refetchAccounts).toHaveBeenCalledOnce() } else if (state === 'pending' || state === 'refreshing') { expect(document.body.textContent).toContain('Checking the installed Slack app') + expect(document.body.textContent).not.toContain('Update member access') + } else if (state === 'needs_update' || state === 'needs_update_failed') { + expect(document.body.textContent).toContain('Member access is outdated') + if (state === 'needs_update_failed') + mocks.start.mockRejectedValueOnce(new Error('Try again')) + await clickButton('Update member access') + expect(mocks.start).toHaveBeenCalledExactlyOnceWith({ + organizationId: 'org-1', + credentialGroupId: 'group-1', + body: { + appId: 'A_SHARED', + teamId: 'T_TEAM', + requiredScopes: [...SLACK_SEARCH_USER_SCOPES], + }, + }) + expect(mocks.install).not.toHaveBeenCalled() + if (state === 'needs_update_failed') { + expect(toast.error).toHaveBeenCalledWith('Try again') + expect(popup.close).toHaveBeenCalledOnce() + expect(mocks.onOpenChange).not.toHaveBeenCalled() + await clickButton('Update member access') + } + await completeAuthorization() + expect(toast.success).toHaveBeenCalledWith('Slack configured') + expect(mocks.onOpenChange).toHaveBeenCalledWith(false) } else { expect(document.body.textContent).toContain('Manage Sim Search app') + expect(document.body.textContent).not.toContain('Update member access') } } ) diff --git a/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx b/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx index dea7da5248b..aee93dc4b36 100644 --- a/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx +++ b/apps/sim/ee/credential-groups/components/slack-managed-users-modal.tsx @@ -107,7 +107,10 @@ export function SlackManagedUsersModal({ const sharedAppInstalled = organizationSetup && selectedApp?.appKind === 'shared' const accounts = useOrganizationAccounts(open && sharedAppInstalled ? organizationId : undefined) const memberGroup = accounts.data?.credentialGroup - const sharedAppReady = Boolean( + const memberOption = memberGroup?.options.find( + (option) => option.provider === 'slack' && option.status === 'active' + ) + const sharedAppCanAuthorize = Boolean( sharedAppInstalled && apps.isSuccess && !apps.isFetching && @@ -118,14 +121,11 @@ export function SlackManagedUsersModal({ accounts.isSuccess && !accounts.isFetching && !accounts.error && - memberGroup?.id === credentialGroupId && - memberGroup.options.some( - (option) => - option.provider === 'slack' && - option.status === 'active' && - option.configurationStatus === 'ready' - ) + memberGroup?.id === credentialGroupId ) + const sharedAppReady = sharedAppCanAuthorize && memberOption?.configurationStatus === 'ready' + const sharedAppNeedsUpdate = + sharedAppCanAuthorize && memberOption?.configurationStatus === 'needs_update' const [clientId, setClientId] = useState('') const [clientSecret, setClientSecret] = useState('') const [pending, setPending] = useState(false) @@ -274,7 +274,12 @@ export function SlackManagedUsersModal({ } const handleSubmit = async () => { - if (pending || sharedAppInstalled || (!organizationSetup && !selectedBot)) return + if ( + pending || + (sharedAppInstalled && !sharedAppNeedsUpdate) || + (!organizationSetup && !selectedBot) + ) + return if ( organizationSetup ? !selectedApp || !requiredScopes.length @@ -341,7 +346,9 @@ export function SlackManagedUsersModal({ ? 'Loading...' : pending ? 'Waiting for Slack...' - : 'Verify and add' + : sharedAppNeedsUpdate + ? 'Update member access' + : 'Verify and add' const primaryDisabled = isLoading || noBots || @@ -414,7 +421,9 @@ export function SlackManagedUsersModal({

{sharedAppInstalled - ? 'The Sim Search installation needs attention. Manage the app to finish setup.' + ? sharedAppNeedsUpdate + ? 'Member access is outdated. Update it so members can reconnect their Slack accounts.' + : 'The Sim Search installation needs attention. Manage the app to finish setup.' : 'Verify member authorization for the installed app. Each member can then connect their Slack account to index channels and DMs they can access.'}

{selectedApp && ( @@ -512,7 +521,7 @@ export function SlackManagedUsersModal({ onClick: () => setAppSetupOpen(true), }, } - : sharedAppInstalled + : sharedAppInstalled && !sharedAppNeedsUpdate ? { defaultAction: 'dismiss' as const } : noBots ? { defaultAction: 'dismiss' as const }