Skip to content

Commit 2549863

Browse files
committed
ref(integrations): Convert AddIntegration from class render prop to useAddIntegration hook
Replace the class-based AddIntegration render prop component with a useAddIntegration hook. This modernizes the integration setup flow to use hooks and prepares for the API-driven pipeline modal by adding feature-flag-gated support for opening a React pipeline modal instead of the legacy Django popup window. - Convert AddIntegration class component to useAddIntegration hook - Refactor AddIntegrationButton to use the hook directly - Add API pipeline feature flag support for github and gitlab providers - Rewrite tests to use renderHookWithProviders with comprehensive coverage for both legacy and API pipeline flows
1 parent 65bd267 commit 2549863

File tree

3 files changed

+506
-185
lines changed

3 files changed

+506
-185
lines changed

static/app/views/settings/organizationIntegrations/addIntegration.spec.tsx

Lines changed: 287 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2,86 +2,315 @@ import {GitHubIntegrationFixture} from 'sentry-fixture/githubIntegration';
22
import {GitHubIntegrationProviderFixture} from 'sentry-fixture/githubIntegrationProvider';
33
import {OrganizationFixture} from 'sentry-fixture/organization';
44

5-
import {render, waitFor} from 'sentry-test/reactTestingLibrary';
6-
import {setWindowLocation} from 'sentry-test/utils';
5+
import {act, renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary';
76

7+
import * as indicators from 'sentry/actionCreators/indicator';
8+
import * as pipelineModal from 'sentry/components/pipeline/modal';
89
import {ConfigStore} from 'sentry/stores/configStore';
910
import type {Config} from 'sentry/types/system';
10-
import {AddIntegration} from 'sentry/views/settings/organizationIntegrations/addIntegration';
11+
import {useAddIntegration} from 'sentry/views/settings/organizationIntegrations/addIntegration';
1112

12-
describe('AddIntegration', () => {
13+
describe('useAddIntegration', () => {
1314
const provider = GitHubIntegrationProviderFixture();
1415
const integration = GitHubIntegrationFixture();
1516
let configState: Config;
1617

17-
function interceptMessageEvent(event: MessageEvent) {
18-
if (event.origin === '') {
19-
event.stopImmediatePropagation();
20-
const eventWithOrigin = new MessageEvent('message', {
21-
data: event.data,
22-
origin: 'https://foobar.sentry.io',
23-
});
24-
window.dispatchEvent(eventWithOrigin);
25-
}
26-
}
27-
2818
beforeEach(() => {
2919
configState = ConfigStore.getState();
3020
ConfigStore.loadInitialData({
3121
...configState,
32-
customerDomain: {
33-
subdomain: 'foobar',
34-
organizationUrl: 'https://foobar.sentry.io',
35-
sentryUrl: 'https://sentry.io',
36-
},
3722
links: {
38-
organizationUrl: 'https://foobar.sentry.io',
23+
organizationUrl: document.location.origin,
3924
regionUrl: 'https://us.sentry.io',
4025
sentryUrl: 'https://sentry.io',
4126
},
4227
});
43-
44-
setWindowLocation('https://foobar.sentry.io');
45-
window.addEventListener('message', interceptMessageEvent);
4628
});
4729

4830
afterEach(() => {
49-
window.removeEventListener('message', interceptMessageEvent);
5031
ConfigStore.loadInitialData(configState);
32+
jest.restoreAllMocks();
5133
});
5234

53-
it('Adds an integration on dialog completion', async () => {
54-
const onAdd = jest.fn();
55-
56-
const focus = jest.fn();
57-
const open = jest.fn().mockReturnValue({focus});
58-
global.open = open;
59-
60-
render(
61-
<AddIntegration
62-
organization={OrganizationFixture()}
63-
provider={provider}
64-
onInstall={onAdd}
65-
>
66-
{openDialog => (
67-
<a href="#" onClick={() => openDialog()}>
68-
Click
69-
</a>
70-
)}
71-
</AddIntegration>
72-
);
73-
74-
const newIntegration = {
75-
success: true,
76-
data: Object.assign({}, integration, {
77-
id: '2',
78-
domain_name: 'new-integration.github.com',
79-
icon: 'http://example.com/new-integration-icon.png',
80-
name: 'New Integration',
81-
}),
82-
};
83-
84-
window.postMessage(newIntegration, '*');
85-
await waitFor(() => expect(onAdd).toHaveBeenCalledWith(newIntegration.data));
35+
/**
36+
* Dispatches a MessageEvent that appears to come from the mock popup window,
37+
* matching the origin and source checks in the hook's message handler.
38+
*/
39+
function postMessageFromPopup(popup: Window, data: unknown) {
40+
const event = new MessageEvent('message', {
41+
data,
42+
origin: document.location.origin,
43+
});
44+
Object.defineProperty(event, 'source', {value: popup});
45+
window.dispatchEvent(event);
46+
}
47+
48+
describe('legacy flow', () => {
49+
let popup: Window;
50+
51+
beforeEach(() => {
52+
popup = {focus: jest.fn(), close: jest.fn()} as unknown as Window;
53+
jest.spyOn(window, 'open').mockReturnValue(popup);
54+
});
55+
56+
it('opens a popup window when startFlow is called', () => {
57+
const {result} = renderHookWithProviders(() =>
58+
useAddIntegration({
59+
provider,
60+
organization: OrganizationFixture(),
61+
onInstall: jest.fn(),
62+
})
63+
);
64+
65+
act(() => result.current.startFlow());
66+
67+
expect(window.open).toHaveBeenCalledTimes(1);
68+
expect(jest.mocked(window.open).mock.calls[0][0]).toBe(
69+
'/github-integration-setup-uri/?'
70+
);
71+
expect(popup.focus).toHaveBeenCalledTimes(1);
72+
});
73+
74+
it('includes account and modalParams in the popup URL', () => {
75+
const {result} = renderHookWithProviders(() =>
76+
useAddIntegration({
77+
provider,
78+
organization: OrganizationFixture(),
79+
onInstall: jest.fn(),
80+
account: 'my-account',
81+
modalParams: {use_staging: '1'},
82+
})
83+
);
84+
85+
act(() => result.current.startFlow());
86+
87+
const calls = jest.mocked(window.open).mock.calls[0];
88+
const url = calls[0] as string;
89+
expect(url).toContain('account=my-account');
90+
expect(url).toContain('use_staging=1');
91+
expect(calls[1]).toBe('sentryAddStagingIntegration');
92+
});
93+
94+
it('includes urlParams passed to startFlow', () => {
95+
const {result} = renderHookWithProviders(() =>
96+
useAddIntegration({
97+
provider,
98+
organization: OrganizationFixture(),
99+
onInstall: jest.fn(),
100+
})
101+
);
102+
103+
act(() => result.current.startFlow({custom_param: 'value'}));
104+
105+
const url = jest.mocked(window.open).mock.calls[0][0] as string;
106+
expect(url).toContain('custom_param=value');
107+
});
108+
109+
it('calls onInstall when a success message is received', async () => {
110+
const onInstall = jest.fn();
111+
112+
const {result} = renderHookWithProviders(() =>
113+
useAddIntegration({
114+
provider,
115+
organization: OrganizationFixture(),
116+
onInstall,
117+
})
118+
);
119+
120+
act(() => result.current.startFlow());
121+
122+
const newIntegration = {
123+
success: true,
124+
data: {
125+
...integration,
126+
id: '2',
127+
domain_name: 'new-integration.github.com',
128+
icon: 'http://example.com/new-integration-icon.png',
129+
name: 'New Integration',
130+
},
131+
};
132+
133+
postMessageFromPopup(popup, newIntegration);
134+
await waitFor(() => expect(onInstall).toHaveBeenCalledWith(newIntegration.data));
135+
});
136+
137+
it('shows a success indicator on successful installation', async () => {
138+
const successSpy = jest.spyOn(indicators, 'addSuccessMessage');
139+
140+
const {result} = renderHookWithProviders(() =>
141+
useAddIntegration({
142+
provider,
143+
organization: OrganizationFixture(),
144+
onInstall: jest.fn(),
145+
})
146+
);
147+
148+
act(() => result.current.startFlow());
149+
150+
postMessageFromPopup(popup, {success: true, data: integration});
151+
await waitFor(() => expect(successSpy).toHaveBeenCalledWith('GitHub added'));
152+
});
153+
154+
it('shows an error indicator when the message has success: false', async () => {
155+
const errorSpy = jest.spyOn(indicators, 'addErrorMessage');
156+
157+
const {result} = renderHookWithProviders(() =>
158+
useAddIntegration({
159+
provider,
160+
organization: OrganizationFixture(),
161+
onInstall: jest.fn(),
162+
})
163+
);
164+
165+
act(() => result.current.startFlow());
166+
167+
postMessageFromPopup(popup, {success: false, data: {error: 'OAuth failed'}});
168+
await waitFor(() => expect(errorSpy).toHaveBeenCalledWith('OAuth failed'));
169+
});
170+
171+
it('shows a generic error when no error message is provided', async () => {
172+
const errorSpy = jest.spyOn(indicators, 'addErrorMessage');
173+
174+
const {result} = renderHookWithProviders(() =>
175+
useAddIntegration({
176+
provider,
177+
organization: OrganizationFixture(),
178+
onInstall: jest.fn(),
179+
})
180+
);
181+
182+
act(() => result.current.startFlow());
183+
184+
postMessageFromPopup(popup, {success: false, data: {}});
185+
await waitFor(() =>
186+
expect(errorSpy).toHaveBeenCalledWith('An unknown error occurred')
187+
);
188+
});
189+
190+
it('ignores messages from invalid origins', async () => {
191+
const onInstall = jest.fn();
192+
193+
renderHookWithProviders(() =>
194+
useAddIntegration({
195+
provider,
196+
organization: OrganizationFixture(),
197+
onInstall,
198+
})
199+
);
200+
201+
// jsdom's postMessage uses origin '' which won't match any valid origin
202+
window.postMessage({success: true, data: integration}, '*');
203+
204+
await act(async () => {
205+
await new Promise(resolve => setTimeout(resolve, 50));
206+
});
207+
expect(onInstall).not.toHaveBeenCalled();
208+
});
209+
210+
it('does not call onInstall when data is empty on success', async () => {
211+
const onInstall = jest.fn();
212+
213+
const {result} = renderHookWithProviders(() =>
214+
useAddIntegration({
215+
provider,
216+
organization: OrganizationFixture(),
217+
onInstall,
218+
})
219+
);
220+
221+
act(() => result.current.startFlow());
222+
223+
postMessageFromPopup(popup, {success: true, data: null});
224+
225+
await act(async () => {
226+
await new Promise(resolve => setTimeout(resolve, 50));
227+
});
228+
expect(onInstall).not.toHaveBeenCalled();
229+
});
230+
231+
it('closes the dialog on unmount', () => {
232+
const {result, unmount} = renderHookWithProviders(() =>
233+
useAddIntegration({
234+
provider,
235+
organization: OrganizationFixture(),
236+
onInstall: jest.fn(),
237+
})
238+
);
239+
240+
act(() => result.current.startFlow());
241+
unmount();
242+
243+
expect(popup.close).toHaveBeenCalledTimes(1);
244+
});
245+
});
246+
247+
describe('API pipeline flow', () => {
248+
it('opens the pipeline modal when feature flag is enabled', () => {
249+
const openPipelineModalSpy = jest.spyOn(pipelineModal, 'openPipelineModal');
250+
const onInstall = jest.fn();
251+
252+
const organization = OrganizationFixture({
253+
features: ['integration-api-pipeline-github'],
254+
});
255+
256+
const {result} = renderHookWithProviders(() =>
257+
useAddIntegration({
258+
provider,
259+
organization,
260+
onInstall,
261+
})
262+
);
263+
264+
act(() => result.current.startFlow());
265+
266+
expect(openPipelineModalSpy).toHaveBeenCalledWith({
267+
type: 'integration',
268+
provider: 'github',
269+
onComplete: expect.any(Function),
270+
});
271+
});
272+
273+
it('does not open a popup window when the pipeline modal is used', () => {
274+
jest.spyOn(pipelineModal, 'openPipelineModal');
275+
jest.spyOn(window, 'open');
276+
277+
const organization = OrganizationFixture({
278+
features: ['integration-api-pipeline-github'],
279+
});
280+
281+
const {result} = renderHookWithProviders(() =>
282+
useAddIntegration({
283+
provider,
284+
organization,
285+
onInstall: jest.fn(),
286+
})
287+
);
288+
289+
act(() => result.current.startFlow());
290+
291+
expect(window.open).not.toHaveBeenCalled();
292+
});
293+
294+
it('falls back to legacy flow when feature flag is not enabled', () => {
295+
const openPipelineModalSpy = jest.spyOn(pipelineModal, 'openPipelineModal');
296+
jest
297+
.spyOn(window, 'open')
298+
.mockReturnValue({focus: jest.fn(), close: jest.fn()} as unknown as Window);
299+
300+
const organization = OrganizationFixture({features: []});
301+
302+
const {result} = renderHookWithProviders(() =>
303+
useAddIntegration({
304+
provider,
305+
organization,
306+
onInstall: jest.fn(),
307+
})
308+
);
309+
310+
act(() => result.current.startFlow());
311+
312+
expect(openPipelineModalSpy).not.toHaveBeenCalled();
313+
expect(window.open).toHaveBeenCalledTimes(1);
314+
});
86315
});
87316
});

0 commit comments

Comments
 (0)