From 84c32d2626a3c7e0d97a764f16c8d8eb81a44d44 Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Wed, 15 Apr 2026 12:41:33 -0400 Subject: [PATCH] feat(integrations): GA Slack, Github, Gitlab, and Bitbucket API Pipelines --- .../integrations/useAddIntegration.spec.tsx | 67 +++++++++----- .../utils/integrations/useAddIntegration.tsx | 60 ++++++++----- .../rules/issue/addIntegrationRow.spec.tsx | 4 +- .../addIntegrationButton.spec.tsx | 4 +- .../integrationButton.spec.tsx | 4 +- tests/acceptance/test_scm_onboarding.py | 89 ++++++++++++++----- tests/js/fixtures/integrationProvider.ts | 35 ++++++++ 7 files changed, 190 insertions(+), 73 deletions(-) create mode 100644 tests/js/fixtures/integrationProvider.ts diff --git a/static/app/utils/integrations/useAddIntegration.spec.tsx b/static/app/utils/integrations/useAddIntegration.spec.tsx index cb78d1ce672ef6..9f934307d955d3 100644 --- a/static/app/utils/integrations/useAddIntegration.spec.tsx +++ b/static/app/utils/integrations/useAddIntegration.spec.tsx @@ -12,6 +12,10 @@ import {useAddIntegration} from 'sentry/utils/integrations/useAddIntegration'; describe('useAddIntegration', () => { const provider = GitHubIntegrationProviderFixture(); + const legacyProvider = GitHubIntegrationProviderFixture({ + key: 'custom_legacy', + slug: 'custom_legacy', + }); const integration = GitHubIntegrationFixture(); let configState: Config; @@ -58,7 +62,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall: jest.fn(), }) @@ -76,7 +80,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall: jest.fn(), account: 'my-account', @@ -96,7 +100,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall: jest.fn(), urlParams: {custom_param: 'value'}, @@ -114,7 +118,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall, }) @@ -142,7 +146,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall: jest.fn(), }) @@ -159,7 +163,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall: jest.fn(), }) @@ -176,7 +180,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall: jest.fn(), }) @@ -195,7 +199,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall, }) @@ -212,6 +216,7 @@ describe('useAddIntegration', () => { await act(async () => { await new Promise(resolve => setTimeout(resolve, 50)); }); + expect(onInstall).not.toHaveBeenCalled(); }); @@ -222,7 +227,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall, }) @@ -241,7 +246,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization: OrganizationFixture(), onInstall: jest.fn(), }) @@ -253,13 +258,11 @@ describe('useAddIntegration', () => { }); describe('API pipeline flow', () => { - it('opens the pipeline modal when feature flag is enabled', () => { + it('opens the pipeline modal for unconditionally API-driven providers', () => { const openPipelineModalSpy = jest.spyOn(pipelineModal, 'openPipelineModal'); const onInstall = jest.fn(); - const organization = OrganizationFixture({ - features: ['integration-api-pipeline-github'], - }); + const organization = OrganizationFixture({features: []}); const {result} = renderHookWithProviders(() => useAddIntegration()); @@ -282,9 +285,7 @@ describe('useAddIntegration', () => { it('passes urlParams as initialData to the pipeline modal', () => { const openPipelineModalSpy = jest.spyOn(pipelineModal, 'openPipelineModal'); - const organization = OrganizationFixture({ - features: ['integration-api-pipeline-github'], - }); + const organization = OrganizationFixture({features: []}); const {result} = renderHookWithProviders(() => useAddIntegration()); @@ -308,9 +309,7 @@ describe('useAddIntegration', () => { jest.spyOn(pipelineModal, 'openPipelineModal'); jest.spyOn(window, 'open'); - const organization = OrganizationFixture({ - features: ['integration-api-pipeline-github'], - }); + const organization = OrganizationFixture({features: []}); const {result} = renderHookWithProviders(() => useAddIntegration()); @@ -325,7 +324,31 @@ describe('useAddIntegration', () => { expect(window.open).not.toHaveBeenCalled(); }); - it('falls back to legacy flow when feature flag is not enabled', () => { + it('opens the pipeline modal for other unconditional providers without a flag', () => { + const openPipelineModalSpy = jest.spyOn(pipelineModal, 'openPipelineModal'); + const organization = OrganizationFixture({features: []}); + const gitlabProvider = GitHubIntegrationProviderFixture({ + key: 'gitlab', + slug: 'gitlab', + name: 'GitLab', + }); + + const {result} = renderHookWithProviders(() => useAddIntegration()); + + act(() => + result.current.startFlow({ + provider: gitlabProvider, + organization, + onInstall: jest.fn(), + }) + ); + + expect(openPipelineModalSpy).toHaveBeenCalledWith( + expect.objectContaining({provider: 'gitlab'}) + ); + }); + + it('falls back to legacy flow when the provider is not API driven', () => { const openPipelineModalSpy = jest.spyOn(pipelineModal, 'openPipelineModal'); jest .spyOn(window, 'open') @@ -337,7 +360,7 @@ describe('useAddIntegration', () => { act(() => result.current.startFlow({ - provider, + provider: legacyProvider, organization, onInstall: jest.fn(), }) diff --git a/static/app/utils/integrations/useAddIntegration.tsx b/static/app/utils/integrations/useAddIntegration.tsx index 3303edce04313f..236db567dee07e 100644 --- a/static/app/utils/integrations/useAddIntegration.tsx +++ b/static/app/utils/integrations/useAddIntegration.tsx @@ -36,44 +36,60 @@ export interface AddIntegrationParams { } /** - * Per-provider feature flags that gate the new API-driven pipeline setup flow. - * When enabled for a provider, the integration setup uses the React pipeline - * modal instead of the legacy Django view popup window. + * Providers that should always use the API-driven pipeline modal. + */ +const UNCONDITIONAL_API_PIPELINE_PROVIDERS = [ + 'aws_lambda', + 'bitbucket', + 'github', + 'gitlab', + 'slack', +] as const satisfies ReadonlyArray; + +type UnconditionalApiPipelineProvider = + (typeof UNCONDITIONAL_API_PIPELINE_PROVIDERS)[number]; + +/** + * Providers that support the API-driven pipeline modal but still require an + * organization feature flag during rollout. * - * Keys are provider identifiers (constrained to registered pipeline providers - * via `satisfies`), values are feature flag names without the `organizations:` - * prefix. + * Keys are provider identifiers, values are feature flag names without the + * `organizations:` prefix. */ -const API_PIPELINE_FEATURE_FLAGS = { - aws_lambda: 'integration-api-pipeline-aws-lambda', - bitbucket: 'integration-api-pipeline-bitbucket', - github: 'integration-api-pipeline-github', - gitlab: 'integration-api-pipeline-gitlab', - slack: 'integration-api-pipeline-slack', -} as const satisfies Partial>; +const API_PIPELINE_FEATURE_FLAGS = {} as const satisfies Partial< + Record +>; -type ApiPipelineProvider = keyof typeof API_PIPELINE_FEATURE_FLAGS; +type FlaggedApiPipelineProvider = keyof typeof API_PIPELINE_FEATURE_FLAGS; +type ApiPipelineProvider = UnconditionalApiPipelineProvider | FlaggedApiPipelineProvider; function getApiPipelineProvider( organization: Organization, providerKey: string ): ApiPipelineProvider | null { - if (!(providerKey in API_PIPELINE_FEATURE_FLAGS)) { - return null; + if ( + UNCONDITIONAL_API_PIPELINE_PROVIDERS.includes( + providerKey as UnconditionalApiPipelineProvider + ) + ) { + return providerKey as UnconditionalApiPipelineProvider; } - const key = providerKey as ApiPipelineProvider; - const flag = API_PIPELINE_FEATURE_FLAGS[key]; - if (!organization.features.includes(flag)) { - return null; + + if (providerKey in API_PIPELINE_FEATURE_FLAGS) { + const key = providerKey as keyof typeof API_PIPELINE_FEATURE_FLAGS; + if (organization.features.includes(API_PIPELINE_FEATURE_FLAGS[key])) { + return key; + } } - return key; + + return null; } /** * Opens the integration setup flow. Accepts all parameters at call time via * `startFlow(params)`, so a single hook instance can launch flows for any * provider. Automatically selects between the API-driven pipeline modal and - * the legacy popup-based flow depending on the organization's feature flags. + * the legacy popup-based flow based on the provider's rollout state. * * The hook manages its own `message` event listener for the legacy popup flow. * No context provider is needed. diff --git a/static/app/views/alerts/rules/issue/addIntegrationRow.spec.tsx b/static/app/views/alerts/rules/issue/addIntegrationRow.spec.tsx index 53a74f467b5908..85ddffa99e7514 100644 --- a/static/app/views/alerts/rules/issue/addIntegrationRow.spec.tsx +++ b/static/app/views/alerts/rules/issue/addIntegrationRow.spec.tsx @@ -1,4 +1,4 @@ -import {GitHubIntegrationProviderFixture} from 'sentry-fixture/githubIntegrationProvider'; +import {IntegrationProviderFixture} from 'sentry-fixture/integrationProvider'; import {OrganizationFixture} from 'sentry-fixture/organization'; import {ProjectFixture} from 'sentry-fixture/project'; @@ -12,7 +12,7 @@ jest.mock('sentry/actionCreators/modal'); describe('AddIntegrationRow', () => { let org: any; const project = ProjectFixture(); - const provider = GitHubIntegrationProviderFixture(); + const provider = IntegrationProviderFixture(); beforeEach(() => { org = OrganizationFixture(); diff --git a/static/app/views/settings/organizationIntegrations/addIntegrationButton.spec.tsx b/static/app/views/settings/organizationIntegrations/addIntegrationButton.spec.tsx index 5ea13693e5a1d3..b42e9a9d0b34df 100644 --- a/static/app/views/settings/organizationIntegrations/addIntegrationButton.spec.tsx +++ b/static/app/views/settings/organizationIntegrations/addIntegrationButton.spec.tsx @@ -1,5 +1,5 @@ /* global global */ -import {GitHubIntegrationProviderFixture} from 'sentry-fixture/githubIntegrationProvider'; +import {IntegrationProviderFixture} from 'sentry-fixture/integrationProvider'; import {OrganizationFixture} from 'sentry-fixture/organization'; import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; @@ -7,7 +7,7 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; import {AddIntegrationButton} from 'sentry/views/settings/organizationIntegrations/addIntegrationButton'; describe('AddIntegrationButton', () => { - const provider = GitHubIntegrationProviderFixture(); + const provider = IntegrationProviderFixture(); it('Opens the setup dialog on click', async () => { const focus = jest.fn(); diff --git a/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx b/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx index 892917cf7c5b8e..08f1828f7aceb6 100644 --- a/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationButton.spec.tsx @@ -1,4 +1,4 @@ -import {GitHubIntegrationProviderFixture} from 'sentry-fixture/githubIntegrationProvider'; +import {IntegrationProviderFixture} from 'sentry-fixture/integrationProvider'; import {OrganizationFixture} from 'sentry-fixture/organization'; import {ProjectFixture} from 'sentry-fixture/project'; @@ -17,7 +17,7 @@ describe('AddIntegrationButton', () => { const project = ProjectFixture(); beforeEach(() => { - provider = GitHubIntegrationProviderFixture(); + provider = IntegrationProviderFixture(); org = OrganizationFixture(); hasAccess = true; externalInstallText = undefined; diff --git a/tests/acceptance/test_scm_onboarding.py b/tests/acceptance/test_scm_onboarding.py index dc66ea88e883c2..4709a6f8e9ed53 100644 --- a/tests/acceptance/test_scm_onboarding.py +++ b/tests/acceptance/test_scm_onboarding.py @@ -2,15 +2,13 @@ import pytest -from sentry.api.serializers import serialize +from sentry.integrations.github.integration import GitHubOAuthLoginResult from sentry.integrations.models.integration import Integration -from sentry.integrations.models.organization_integration import OrganizationIntegration from sentry.models.project import Project from sentry.shared_integrations.exceptions import ApiError from sentry.testutils.asserts import assert_existing_projects_status from sentry.testutils.cases import AcceptanceTestCase from sentry.testutils.silo import no_silo_test -from sentry.utils import json pytestmark = pytest.mark.sentry_metrics @@ -157,7 +155,7 @@ def test_scm_onboarding_skip_integration(self) -> None: ) def test_scm_onboarding_with_integration_install(self) -> None: - """Install flow: welcome → install GitHub → repo search → detected platform → create project.""" + """Install flow: welcome → install GitHub via API pipeline → repo search → detected platform → create project.""" mock_repos = [ { "name": "sentry", @@ -177,6 +175,18 @@ def test_scm_onboarding_with_integration_install(self) -> None: } ] + mock_installation_response = { + "id": "12345", + "app_id": "1", + "account": { + "login": "getsentry", + "avatar_url": "https://example.com/avatar.png", + "html_url": "https://github.com/getsentry", + "type": "Organization", + "id": 67890, + }, + } + with ( self.feature( { @@ -196,44 +206,77 @@ def test_scm_onboarding_with_integration_install(self) -> None: "sentry.integrations.api.endpoints.organization_repository_platforms.detect_platforms", return_value=mock_platforms, ), + mock.patch( + "sentry.integrations.github.integration.exchange_github_oauth", + return_value=GitHubOAuthLoginResult( + authenticated_user="testuser", + installation_info=[], + ), + ), + mock.patch( + "sentry.integrations.github.integration.GitHubIntegrationProvider.get_installation_info", + return_value=mock_installation_response, + ), ): self.start_onboarding() # SCM Connect: no integration installed, provider pills are shown. - # Override window.open so that AddIntegration stores `window` as the - # dialog reference. When we later inject a postMessage from the same - # window, `message.source === this.dialog` passes. + # Override window.open so the pipeline popup steps (OAuth and app + # install) return `window` as the popup reference. This lets us + # inject postMessage from the same window and pass the + # `event.source === popupRef.current` check. self.browser.driver.execute_script( """ - window.__testOpenCalled = false; - window.open = function() { - window.__testOpenCalled = true; + window.__testOpenUrl = null; + window.open = function(url) { + window.__testOpenUrl = url; return window; }; """ ) # Wait for the providers to load, then click Install GitHub. + # This opens the API-driven pipeline modal (not a popup). self.browser.wait_until(xpath='//button[contains(., "GitHub")]') self.browser.click(xpath='//button[contains(., "GitHub")]') - assert self.browser.driver.execute_script("return window.__testOpenCalled") - - # Simulate the OAuth pipeline: create the integration in the DB, - # then serialize it with the same code path as IntegrationPipeline._dialog_response - # to avoid mock-drift between the test data and the real serializer. - integration = self.create_github_integration() - org_integration = OrganizationIntegration.objects.get( - integration=integration, organization_id=self.org.id + + # Step 1: OAuth Login — the modal shows "Authorize GitHub". + self.browser.wait_until(xpath='//button[contains(., "Authorize GitHub")]') + self.browser.click(xpath='//button[contains(., "Authorize GitHub")]') + + # The OAuth popup was intercepted. Extract the state parameter from + # the captured URL and send a postMessage callback. + oauth_url = self.browser.driver.execute_script("return window.__testOpenUrl") + assert oauth_url is not None + state = dict(pair.split("=") for pair in oauth_url.split("?")[1].split("&")).get( + "state", "" ) - # Resolve Django lazy objects (translations, datetimes) so - # Selenium can JSON-serialize the data for execute_script. - serialized = json.loads(json.dumps(serialize(org_integration, self.user))) self.browser.driver.execute_script( "window.postMessage(arguments[0], window.location.origin);", - {"success": True, "data": serialized}, + { + "_pipeline_source": "sentry-pipeline", + "code": "fake_oauth_code", + "state": state, + }, + ) + + # Step 2: Org Selection — fresh install, shows "Install GitHub App". + self.browser.wait_until(xpath='//button[contains(., "Install GitHub App")]') + self.browser.driver.execute_script("window.__testOpenUrl = null;") + self.browser.click(xpath='//button[contains(., "Install GitHub App")]') + + # The install popup was intercepted. Send a postMessage callback + # with the installation_id. The backend validates and completes + # the pipeline, creating the integration. + self.browser.driver.execute_script( + "window.postMessage(arguments[0], window.location.origin);", + { + "_pipeline_source": "sentry-pipeline", + "installation_id": "12345", + }, ) - # Wait for the component to process the message and show connected state. + # Wait for the pipeline modal to close and the connected state. self.browser.wait_until(xpath='//*[contains(text(), "Connected to")]') # Repo search (same flow as happy path from here on). diff --git a/tests/js/fixtures/integrationProvider.ts b/tests/js/fixtures/integrationProvider.ts new file mode 100644 index 00000000000000..3079af4dd708af --- /dev/null +++ b/tests/js/fixtures/integrationProvider.ts @@ -0,0 +1,35 @@ +import type {IntegrationProvider} from 'sentry/types/integrations'; + +export function IntegrationProviderFixture( + params: Partial = {} +): IntegrationProvider { + return { + key: 'generic-provider', + slug: 'generic-provider', + name: 'Generic Provider', + canAdd: true, + features: [], + setupDialog: { + url: '/generic-provider-setup-uri/', + width: 100, + height: 100, + }, + canDisable: true, + metadata: { + description: 'A generic integration provider for testing', + features: [ + { + description: 'Feature description', + featureGate: 'integrations-feature', + featureId: 1, + }, + ], + author: 'Test', + noun: 'Installation', + issue_url: 'http://example.com/issue_url', + source_url: 'http://example.com/source_url', + aspects: {}, + }, + ...params, + }; +}