feat(integrations): connect slack in a new tab from every slack picker

frontend/src/lib/components/Comments/SlackDestinationPicker.tsx

@@ -39,7 +39,7 @@ export function SlackDestinationPicker({
     }
 
     if (!slackIntegrations?.length) {
-        return <SlackNotConfiguredBanner />
+        return <SlackNotConfiguredBanner onConnected={onIntegrationChange} />
     }
 
     return (

frontend/src/lib/integrations/SlackIntegrationHelpers.tsx

@@ -1,5 +1,5 @@
 import { useActions, useValues } from 'kea'
-import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
+import { useCallback, useEffect, useId, useMemo, useRef, useState } from 'react'
 
 import {
     LemonBanner,
@@ -16,44 +16,94 @@ import { RestrictionScope, useRestrictedArea } from 'lib/components/RestrictedAr
 import { OrganizationMembershipLevel } from 'lib/constants'
 import { usePeriodicRerender } from 'lib/hooks/usePeriodicRerender'
 import { IconSlackExternal } from 'lib/lemon-ui/icons'
+import { Spinner } from 'lib/lemon-ui/Spinner'
+import { urls } from 'scenes/urls'
 
 import { IntegrationType, SlackChannelType } from '~/types'
 
 import type { SlackUserApi } from 'products/integrations/frontend/generated/api.schemas'
 
+import { integrationsLogic } from './integrationsLogic'
 import { slackChannelId } from './slackChannel'
+import { slackConnectLogic } from './slackConnectLogic'
 import { slackIntegrationLogic } from './slackIntegrationLogic'
 
+export type SlackNotConfiguredBannerProps = Partial<Pick<LemonBannerProps, 'type' | 'className'>> & {
+    /** Replaces the lead sentence, so a caller can say what connecting Slack is for. */
+    description?: string
+    onConnected?: (integrationId: number) => void
+    onConnectClick?: () => void
+}
+
 export function SlackNotConfiguredBanner({
     type = 'info',
     className,
-}: Partial<Pick<LemonBannerProps, 'type' | 'className'>>): JSX.Element {
+    description,
+    onConnected,
+    onConnectClick,
+}: SlackNotConfiguredBannerProps): JSX.Element {
+    const connectKey = useId()
+    const logic = slackConnectLogic({ connectKey, onConnected })
+    const { waitingForSlack, slackIntegrations } = useValues(logic)
+    const { connectSlackClicked } = useActions(logic)
+    const { integrationsLoading } = useValues(integrationsLogic)
+    const { loadIntegrations } = useActions(integrationsLogic)
+
     return (
         <LemonBanner type={type} className={className}>
             <div className="flex flex-col gap-2">
-                <div className="flex justify-between gap-2 items-center">
+                <div className="flex flex-wrap justify-between gap-2 items-center">
                     <span>
-                        Slack is not yet configured for this project. Add PostHog to your Slack workspace to continue.
+                        {description ??
+                            'Slack is not yet configured for this project. Add PostHog to your Slack workspace to continue.'}
                     </span>
-                    <Link
-                        to={api.integrations.authorizeUrl({
-                            kind: 'slack',
-                            next: window.location.pathname + '?target_type=slack',
-                        })}
-                        disableClientSideRouting
-                    >
-                        <img
-                            alt="Add to Slack"
-                            height="40"
-                            width="139"
-                            src="https://platform.slack-edge.com/img/add_to_slack.png"
-                            srcSet="https://platform.slack-edge.com/img/add_to_slack.png 1x, https://platform.slack-edge.com/img/add_to_slack@2x.png 2x"
-                        />
-                    </Link>
+                    {/* A click before the first load has no workspace list to compare the new one against. */}
+                    {slackIntegrations === undefined ? (
+                        integrationsLoading ? (
+                            <Spinner className="text-lg" />
+                        ) : (
+                            <LemonButton
+                                type="secondary"
+                                size="small"
+                                tooltip="Couldn't load your Slack workspaces"
+                                onClick={() => loadIntegrations()}
+                            >
+                                Try again
+                            </LemonButton>
+                        )
+                    ) : (
+                        <Link
+                            // nosemgrep: prefer-codegen-api-namespaced-integrations - the generated authorize URL takes no query params
+                            to={api.integrations.authorizeUrl({
+                                kind: 'slack',
+                                next: urls.settings('project-integrations'),
+                            })}
+                            target="_blank"
+                            disableClientSideRouting
+                            onClick={() => {
+                                connectSlackClicked()
+                                onConnectClick?.()
+                            }}
+                        >
+                            <img
+                                alt="Add to Slack"
+                                height="40"
+                                width="139"
+                                src="https://platform.slack-edge.com/img/add_to_slack.png"
+                                srcSet="https://platform.slack-edge.com/img/add_to_slack.png 1x, https://platform.slack-edge.com/img/add_to_slack@2x.png 2x"
+                            />
+                        </Link>
+                    )}
                 </div>
+                {waitingForSlack ? (
+                    <span className="flex items-center gap-1 text-sm text-secondary">
+                        <Spinner className="text-sm" />
+                        Waiting for Slack to connect
+                    </span>
+                ) : null}
                 <span className="text-sm text-secondary">
-                    Adding PostHog creates a public #posthog-inbox channel in your Slack workspace, where PostHog posts
-                    what it finds.
+                    Slack opens in a new tab. Adding PostHog creates a public #posthog-inbox channel in your Slack
+                    workspace, where PostHog posts what it finds.
                 </span>
             </div>
         </LemonBanner>

frontend/src/lib/integrations/slackChannel.ts

@@ -31,6 +31,11 @@ export function slackChannelDisplayName(channelValue: string): string {
     return channelValue.slice(pipe + 1).trim() || channelValue
 }
 
+/** The bare channel name (no `#`) of a `${channelId}|#${channelName}` picker value, or nothing for a bare id. */
+export function slackChannelName(channelValue: string): string | undefined {
+    return channelValue.includes('|') ? slackChannelDisplayName(channelValue).replace(/^#/, '') : undefined
+}
+
 function storageKey(integrationId: number): string | null {
     const teamId = getCurrentTeamIdOrNone()
     if (teamId == null) {

frontend/src/lib/integrations/slackConnectLogic.test.ts

@@ -0,0 +1,113 @@
+import { MOCK_DEFAULT_BASIC_USER, MOCK_DEFAULT_USER } from 'lib/api.mock'
+
+import { expectLogic } from 'kea-test-utils'
+
+import { userLogic } from 'scenes/userLogic'
+
+import { useMocks } from '~/mocks/jest'
+import { initKeaTests } from '~/test/init'
+import { IntegrationType } from '~/types'
+
+import { integrationsLogic } from './integrationsLogic'
+import { SLACK_CONNECT_TIMEOUT_MS, slackConnectLogic } from './slackConnectLogic'
+
+const OTHER_MEMBER = { ...MOCK_DEFAULT_BASIC_USER, id: 999, uuid: 'other-member-uuid' }
+
+const integration = (
+    id: number,
+    kind: IntegrationType['kind'],
+    createdBy: IntegrationType['created_by'] = MOCK_DEFAULT_BASIC_USER
+): IntegrationType =>
+    ({
+        id,
+        kind,
+        display_name: `Workspace ${id}`,
+        icon_url: '',
+        config: {},
+        errors: '',
+        created_by: createdBy,
+    }) as IntegrationType
+
+describe('slackConnectLogic', () => {
+    let onConnected: jest.Mock
+    let logic: ReturnType<typeof slackConnectLogic.build>
+
+    beforeEach(() => {
+        useMocks({ get: { '/api/projects/:team_id/integrations/': () => [200, { results: [] }] } })
+        initKeaTests()
+        userLogic.actions.loadUserSuccess(MOCK_DEFAULT_USER)
+        integrationsLogic.mount()
+        onConnected = jest.fn()
+        logic = slackConnectLogic({ connectKey: 'test', onConnected })
+        logic.mount()
+    })
+
+    afterEach(() => {
+        logic.unmount()
+    })
+
+    it('selects the workspace that appears after the click and stops polling', async () => {
+        integrationsLogic.actions.loadIntegrationsSuccess([integration(1, 'slack')])
+        expect(onConnected).not.toHaveBeenCalled()
+
+        await expectLogic(logic, () => logic.actions.connectSlackClicked()).toMatchValues({ waitingForSlack: true })
+        expect(integrationsLogic.values.pollingSubscribers).toBe(1)
+
+        integrationsLogic.actions.loadIntegrationsSuccess([integration(1, 'slack'), integration(2, 'github')])
+        expect(onConnected).not.toHaveBeenCalled()
+
+        integrationsLogic.actions.loadIntegrationsSuccess([
+            integration(1, 'slack'),
+            integration(2, 'github'),
+            integration(3, 'slack'),
+        ])
+
+        expect(onConnected).toHaveBeenCalledWith(3)
+        expect(logic.values.waitingForSlack).toBe(false)
+        expect(integrationsLogic.values.pollingSubscribers).toBe(0)
+    })
+
+    it.each([
+        {
+            name: 'a workspace another member adds while this user connects',
+            before: [integration(1, 'slack')],
+            after: [integration(1, 'slack'), integration(2, 'slack', OTHER_MEMBER)],
+        },
+        {
+            name: 'an existing workspace when the list had not loaded at the click',
+            before: null,
+            after: [integration(1, 'slack')],
+        },
+    ])('does not select $name', ({ before, after }) => {
+        integrationsLogic.actions.loadIntegrationsSuccess(before as IntegrationType[])
+        logic.actions.connectSlackClicked()
+
+        integrationsLogic.actions.loadIntegrationsSuccess(after)
+
+        expect(onConnected).not.toHaveBeenCalled()
+        expect(logic.values.waitingForSlack).toBe(true)
+    })
+
+    it('stops waiting and polling when Slack never reports back', () => {
+        jest.useFakeTimers()
+        integrationsLogic.actions.loadIntegrationsSuccess([])
+        logic.actions.connectSlackClicked()
+
+        jest.advanceTimersByTime(SLACK_CONNECT_TIMEOUT_MS)
+
+        expect(logic.values.waitingForSlack).toBe(false)
+        expect(integrationsLogic.values.pollingSubscribers).toBe(0)
+        jest.useRealTimers()
+    })
+
+    it('stops polling when the banner unmounts before Slack connects', () => {
+        logic.actions.connectSlackClicked()
+        logic.actions.connectSlackClicked()
+        expect(integrationsLogic.values.pollingSubscribers).toBe(1)
+
+        logic.unmount()
+
+        expect(integrationsLogic.values.pollingSubscribers).toBe(0)
+        logic.mount()
+    })
+})

frontend/src/lib/integrations/slackConnectLogic.ts

@@ -0,0 +1,142 @@
+import { LogicWrapper, MakeLogicType, actions, connect, kea, key, listeners, path, props, reducers } from 'kea'
+
+import { integrationsLogic } from 'lib/integrations/integrationsLogic'
+import { userLogic } from 'scenes/userLogic'
+
+import { IntegrationType, UserType } from '~/types'
+
+export interface SlackConnectLogicProps {
+    connectKey: string
+    onConnected?: (integrationId: number) => void
+}
+
+// Generated by kea-typegen. Update if you're an agent, ignore if you're human.
+export interface slackConnectLogicValues {
+    slackIntegrations: IntegrationType[] | undefined // integrationsLogic
+    user: UserType | null // userLogic
+    waitingForSlack: boolean
+}
+
+// Generated by kea-typegen. Update if you're an agent, ignore if you're human.
+export interface slackConnectLogicActions {
+    loadIntegrationsSuccess: (
+        integrations: IntegrationType[],
+        payload?: any
+    ) => {
+        integrations: IntegrationType[]
+        payload?: any
+    } // integrationsLogic
+    startPolling: () => {
+        value: true
+    } // integrationsLogic
+    stopPolling: () => {
+        value: true
+    } // integrationsLogic
+    connectSlackClicked: () => {
+        value: true
+    }
+    slackConnectTimedOut: () => {
+        value: true
+    }
+    slackConnected: (integrationId: number) => {
+        integrationId: number
+    }
+}
+
+// Generated by kea-typegen. Update if you're an agent, ignore if you're human.
+export interface slackConnectLogicMeta {
+    key: string
+}
+
+export type slackConnectLogicType = MakeLogicType<
+    slackConnectLogicValues,
+    slackConnectLogicActions,
+    SlackConnectLogicProps,
+    slackConnectLogicMeta
+>
+
+/**
+ * Connecting Slack runs OAuth in another tab, so the page keeps what the user was filling in. The new
+ * workspace reaches this tab only through the integrations poller, which also refetches on focus.
+ */
+// An abandoned OAuth tab never reports back, so stop polling after this long and let the user retry.
+export const SLACK_CONNECT_TIMEOUT_MS = 10 * 60 * 1000
+
+export const slackConnectLogic: LogicWrapper<slackConnectLogicType> = kea<slackConnectLogicType>([
+    props({} as SlackConnectLogicProps),
+    key((props) => props.connectKey),
+    path((key) => ['lib', 'integrations', 'slackConnectLogic', key]),
+    connect(() => ({
+        values: [integrationsLogic, ['slackIntegrations'], userLogic, ['user']],
+        actions: [integrationsLogic, ['startPolling', 'stopPolling', 'loadIntegrationsSuccess']],
+    })),
+    actions({
+        connectSlackClicked: true,
+        slackConnected: (integrationId: number) => ({ integrationId }),
+        slackConnectTimedOut: true,
+    }),
+    reducers({
+        waitingForSlack: [
+            false,
+            {
+                connectSlackClicked: () => true,
+                slackConnected: () => false,
+                slackConnectTimedOut: () => false,
+            },
+        ],
+    }),
+    listeners(({ actions, values, props, cache }) => ({
+        connectSlackClicked: () => {
+            // Before the first successful load there is nothing to compare against, so the next load sets the baseline.
+            cache.knownSlackIntegrationIds = values.slackIntegrations
+                ? new Set(values.slackIntegrations.map((integration) => integration.id))
+                : null
+            // The poller counts subscribers, so the same key keeps a second click from adding one that
+            // nothing stops. It keeps running while this tab is hidden, because the user is in the
+            // Slack tab then and the focus refetch is what brings the workspace back.
+            cache.disposables.add(
+                () => {
+                    actions.startPolling()
+                    return () => actions.stopPolling()
+                },
+                'integrationsPolling',
+                { pauseOnPageHidden: false }
+            )
+            cache.disposables.add(
+                () => {
+                    const timeout = window.setTimeout(() => actions.slackConnectTimedOut(), SLACK_CONNECT_TIMEOUT_MS)
+                    return () => window.clearTimeout(timeout)
+                },
+                'slackConnectTimeout',
+                { pauseOnPageHidden: false }
+            )
+        },
+        loadIntegrationsSuccess: ({ integrations }) => {
+            if (!values.waitingForSlack) {
+                return
+            }
+            const slackIntegrations = integrations.filter((integration) => integration.kind === 'slack')
+            const known: Set<number> | null = cache.knownSlackIntegrationIds
+            if (!known) {
+                cache.knownSlackIntegrationIds = new Set(slackIntegrations.map((integration) => integration.id))
+                return
+            }
+            // Another member can add a workspace to the project meanwhile; only this user's OAuth counts.
+            const workspace = slackIntegrations.find(
+                (integration) =>
+                    !known.has(integration.id) && !!values.user && integration.created_by?.uuid === values.user.uuid
+            )
+            if (workspace) {
+                actions.slackConnected(workspace.id)
+            }
+        },
+        slackConnected: ({ integrationId }) => {
+            cache.disposables.dispose('integrationsPolling')
+            cache.disposables.dispose('slackConnectTimeout')
+            props.onConnected?.(integrationId)
+        },
+        slackConnectTimedOut: () => {
+            cache.disposables.dispose('integrationsPolling')
+        },
+    })),
+])