Skip to content

ref(integrations): Refactor useAddIntegration to accept params at call time #113059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
evanpurkhiser merged 1 commit into from
Apr 15, 2026
Merged

ref(integrations): Refactor useAddIntegration to accept params at call time #113059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 66 additions & 57 deletions ...ationIntegrations/addIntegration.spec.tsx → ...s/integrations/useAddIntegration.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as indicators from 'sentry/actionCreators/indicator';
import * as pipelineModal from 'sentry/components/pipeline/modal';
import {ConfigStore} from 'sentry/stores/configStore';
import type {Config} from 'sentry/types/system';
import {useAddIntegration} from 'sentry/views/settings/organizationIntegrations/addIntegration';
import {useAddIntegration} from 'sentry/utils/integrations/useAddIntegration';

describe('useAddIntegration', () => {
const provider = GitHubIntegrationProviderFixture();
Expand Down Expand Up @@ -54,16 +54,16 @@ describe('useAddIntegration', () => {
});

it('opens a popup window when startFlow is called', () => {
const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall: jest.fn(),
})
);

act(() => result.current.startFlow());

expect(window.open).toHaveBeenCalledTimes(1);
expect(jest.mocked(window.open).mock.calls[0]![0]).toBe(
'/github-integration-setup-uri/?'
Expand All @@ -72,8 +72,10 @@ describe('useAddIntegration', () => {
});

it('includes account and modalParams in the popup URL', () => {
const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall: jest.fn(),
Expand All @@ -82,8 +84,6 @@ describe('useAddIntegration', () => {
})
);

act(() => result.current.startFlow());

const calls = jest.mocked(window.open).mock.calls[0]!;
const url = calls[0] as string;
expect(url).toContain('account=my-account');
Expand All @@ -92,33 +92,34 @@ describe('useAddIntegration', () => {
});

it('includes urlParams passed to startFlow', () => {
const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall: jest.fn(),
urlParams: {custom_param: 'value'},
})
);

act(() => result.current.startFlow({custom_param: 'value'}));

const url = jest.mocked(window.open).mock.calls[0]![0] as string;
expect(url).toContain('custom_param=value');
});

it('calls onInstall when a success message is received', async () => {
const onInstall = jest.fn();

const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall,
})
);

act(() => result.current.startFlow());

const newIntegration = {
success: true,
data: {
Expand All @@ -137,50 +138,50 @@ describe('useAddIntegration', () => {
it('shows a success indicator on successful installation', async () => {
const successSpy = jest.spyOn(indicators, 'addSuccessMessage');

const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall: jest.fn(),
})
);

act(() => result.current.startFlow());

postMessageFromPopup(popup, {success: true, data: integration});
await waitFor(() => expect(successSpy).toHaveBeenCalledWith('GitHub added'));
});

it('shows an error indicator when the message has success: false', async () => {
const errorSpy = jest.spyOn(indicators, 'addErrorMessage');

const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall: jest.fn(),
})
);

act(() => result.current.startFlow());

postMessageFromPopup(popup, {success: false, data: {error: 'OAuth failed'}});
await waitFor(() => expect(errorSpy).toHaveBeenCalledWith('OAuth failed'));
});

it('shows a generic error when no error message is provided', async () => {
const errorSpy = jest.spyOn(indicators, 'addErrorMessage');

const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall: jest.fn(),
})
);

act(() => result.current.startFlow());

postMessageFromPopup(popup, {success: false, data: {}});
await waitFor(() =>
expect(errorSpy).toHaveBeenCalledWith('An unknown error occurred')
Expand All @@ -190,16 +191,23 @@ describe('useAddIntegration', () => {
it('ignores messages from invalid origins', async () => {
const onInstall = jest.fn();

renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall,
})
);

// jsdom's postMessage uses origin '' which won't match any valid origin
window.postMessage({success: true, data: integration}, '*');
const event = new MessageEvent('message', {
data: {success: true, data: integration},
origin: 'https://invalid.example.com',
});
Object.defineProperty(event, 'source', {value: popup});

window.dispatchEvent(event);

await act(async () => {
await new Promise(resolve => setTimeout(resolve, 50));
Expand All @@ -210,16 +218,16 @@ describe('useAddIntegration', () => {
it('does not call onInstall when data is empty on success', async () => {
const onInstall = jest.fn();

const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall,
})
);

act(() => result.current.startFlow());

postMessageFromPopup(popup, {success: true, data: null});

await act(async () => {
Expand All @@ -229,15 +237,15 @@ describe('useAddIntegration', () => {
});

it('closes the dialog on unmount', () => {
const {result, unmount} = renderHookWithProviders(() =>
useAddIntegration({
const {result, unmount} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization: OrganizationFixture(),
onInstall: jest.fn(),
})
);

act(() => result.current.startFlow());
unmount();

expect(popup.close).toHaveBeenCalledTimes(1);
Expand All @@ -253,16 +261,16 @@ describe('useAddIntegration', () => {
features: ['integration-api-pipeline-github'],
});

const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization,
onInstall,
})
);

act(() => result.current.startFlow());

expect(openPipelineModalSpy).toHaveBeenCalledWith({
type: 'integration',
provider: 'github',
Expand All @@ -278,16 +286,17 @@ describe('useAddIntegration', () => {
features: ['integration-api-pipeline-github'],
});

const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization,
onInstall: jest.fn(),
urlParams: {installation_id: '12345'},
})
);

act(() => result.current.startFlow({installation_id: '12345'}));

expect(openPipelineModalSpy).toHaveBeenCalledWith(
expect.objectContaining({
initialData: {installation_id: '12345'},
Expand All @@ -303,16 +312,16 @@ describe('useAddIntegration', () => {
features: ['integration-api-pipeline-github'],
});

const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization,
onInstall: jest.fn(),
})
);

act(() => result.current.startFlow());

expect(window.open).not.toHaveBeenCalled();
});

Expand All @@ -324,16 +333,16 @@ describe('useAddIntegration', () => {

const organization = OrganizationFixture({features: []});

const {result} = renderHookWithProviders(() =>
useAddIntegration({
const {result} = renderHookWithProviders(() => useAddIntegration());

act(() =>
result.current.startFlow({
provider,
organization,
onInstall: jest.fn(),
})
);

act(() => result.current.startFlow());

expect(openPipelineModalSpy).not.toHaveBeenCalled();
expect(window.open).toHaveBeenCalledTimes(1);
});
Expand Down
Loading
Loading