Skip to content

Commit 2a89eca

Browse files
jaydgossevanpurkhiser
authored andcommitted
ref(integrations): Refactor useAddIntegration to accept params at call time
Move useAddIntegration from views/settings/organizationIntegrations/ to utils/integrations/ and change the API so startFlow() accepts all params (provider, organization, onInstall, etc.) at call time instead of at hook init. The hook now manages its own message listener via refs, so a single instance can launch flows for any provider without a context provider. This enables multi-provider UIs like dropdowns to share one hook above the dropdown, passing the selected provider to startFlow on click. Extract computeCenteredWindow to utils/window/ and add urlParams to AddIntegrationParams for the installation_id case. Refs VDY-69
1 parent 5910df7 commit 2a89eca

File tree

6 files changed

+176
-185
lines changed

6 files changed

+176
-185
lines changed

static/app/views/settings/organizationIntegrations/addIntegration.spec.tsx renamed to static/app/utils/integrations/useAddIntegration.spec.tsx

Lines changed: 56 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import * as indicators from 'sentry/actionCreators/indicator';
88
import * as pipelineModal from 'sentry/components/pipeline/modal';
99
import {ConfigStore} from 'sentry/stores/configStore';
1010
import type {Config} from 'sentry/types/system';
11-
import {useAddIntegration} from 'sentry/views/settings/organizationIntegrations/addIntegration';
11+
import {useAddIntegration} from 'sentry/utils/integrations/useAddIntegration';
1212

1313
describe('useAddIntegration', () => {
1414
const provider = GitHubIntegrationProviderFixture();
@@ -54,16 +54,16 @@ describe('useAddIntegration', () => {
5454
});
5555

5656
it('opens a popup window when startFlow is called', () => {
57-
const {result} = renderHookWithProviders(() =>
58-
useAddIntegration({
57+
const {result} = renderHookWithProviders(() => useAddIntegration());
58+
59+
act(() =>
60+
result.current.startFlow({
5961
provider,
6062
organization: OrganizationFixture(),
6163
onInstall: jest.fn(),
6264
})
6365
);
6466

65-
act(() => result.current.startFlow());
66-
6767
expect(window.open).toHaveBeenCalledTimes(1);
6868
expect(jest.mocked(window.open).mock.calls[0]![0]).toBe(
6969
'/github-integration-setup-uri/?'
@@ -72,8 +72,10 @@ describe('useAddIntegration', () => {
7272
});
7373

7474
it('includes account and modalParams in the popup URL', () => {
75-
const {result} = renderHookWithProviders(() =>
76-
useAddIntegration({
75+
const {result} = renderHookWithProviders(() => useAddIntegration());
76+
77+
act(() =>
78+
result.current.startFlow({
7779
provider,
7880
organization: OrganizationFixture(),
7981
onInstall: jest.fn(),
@@ -82,8 +84,6 @@ describe('useAddIntegration', () => {
8284
})
8385
);
8486

85-
act(() => result.current.startFlow());
86-
8787
const calls = jest.mocked(window.open).mock.calls[0]!;
8888
const url = calls[0] as string;
8989
expect(url).toContain('account=my-account');
@@ -92,33 +92,34 @@ describe('useAddIntegration', () => {
9292
});
9393

9494
it('includes urlParams passed to startFlow', () => {
95-
const {result} = renderHookWithProviders(() =>
96-
useAddIntegration({
95+
const {result} = renderHookWithProviders(() => useAddIntegration());
96+
97+
act(() =>
98+
result.current.startFlow({
9799
provider,
98100
organization: OrganizationFixture(),
99101
onInstall: jest.fn(),
102+
urlParams: {custom_param: 'value'},
100103
})
101104
);
102105

103-
act(() => result.current.startFlow({custom_param: 'value'}));
104-
105106
const url = jest.mocked(window.open).mock.calls[0]![0] as string;
106107
expect(url).toContain('custom_param=value');
107108
});
108109

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

112-
const {result} = renderHookWithProviders(() =>
113-
useAddIntegration({
113+
const {result} = renderHookWithProviders(() => useAddIntegration());
114+
115+
act(() =>
116+
result.current.startFlow({
114117
provider,
115118
organization: OrganizationFixture(),
116119
onInstall,
117120
})
118121
);
119122

120-
act(() => result.current.startFlow());
121-
122123
const newIntegration = {
123124
success: true,
124125
data: {
@@ -137,50 +138,50 @@ describe('useAddIntegration', () => {
137138
it('shows a success indicator on successful installation', async () => {
138139
const successSpy = jest.spyOn(indicators, 'addSuccessMessage');
139140

140-
const {result} = renderHookWithProviders(() =>
141-
useAddIntegration({
141+
const {result} = renderHookWithProviders(() => useAddIntegration());
142+
143+
act(() =>
144+
result.current.startFlow({
142145
provider,
143146
organization: OrganizationFixture(),
144147
onInstall: jest.fn(),
145148
})
146149
);
147150

148-
act(() => result.current.startFlow());
149-
150151
postMessageFromPopup(popup, {success: true, data: integration});
151152
await waitFor(() => expect(successSpy).toHaveBeenCalledWith('GitHub added'));
152153
});
153154

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

157-
const {result} = renderHookWithProviders(() =>
158-
useAddIntegration({
158+
const {result} = renderHookWithProviders(() => useAddIntegration());
159+
160+
act(() =>
161+
result.current.startFlow({
159162
provider,
160163
organization: OrganizationFixture(),
161164
onInstall: jest.fn(),
162165
})
163166
);
164167

165-
act(() => result.current.startFlow());
166-
167168
postMessageFromPopup(popup, {success: false, data: {error: 'OAuth failed'}});
168169
await waitFor(() => expect(errorSpy).toHaveBeenCalledWith('OAuth failed'));
169170
});
170171

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

174-
const {result} = renderHookWithProviders(() =>
175-
useAddIntegration({
175+
const {result} = renderHookWithProviders(() => useAddIntegration());
176+
177+
act(() =>
178+
result.current.startFlow({
176179
provider,
177180
organization: OrganizationFixture(),
178181
onInstall: jest.fn(),
179182
})
180183
);
181184

182-
act(() => result.current.startFlow());
183-
184185
postMessageFromPopup(popup, {success: false, data: {}});
185186
await waitFor(() =>
186187
expect(errorSpy).toHaveBeenCalledWith('An unknown error occurred')
@@ -190,13 +191,7 @@ describe('useAddIntegration', () => {
190191
it('ignores messages from invalid origins', async () => {
191192
const onInstall = jest.fn();
192193

193-
renderHookWithProviders(() =>
194-
useAddIntegration({
195-
provider,
196-
organization: OrganizationFixture(),
197-
onInstall,
198-
})
199-
);
194+
renderHookWithProviders(() => useAddIntegration());
200195

201196
// jsdom's postMessage uses origin '' which won't match any valid origin
202197
window.postMessage({success: true, data: integration}, '*');
@@ -210,16 +205,16 @@ describe('useAddIntegration', () => {
210205
it('does not call onInstall when data is empty on success', async () => {
211206
const onInstall = jest.fn();
212207

213-
const {result} = renderHookWithProviders(() =>
214-
useAddIntegration({
208+
const {result} = renderHookWithProviders(() => useAddIntegration());
209+
210+
act(() =>
211+
result.current.startFlow({
215212
provider,
216213
organization: OrganizationFixture(),
217214
onInstall,
218215
})
219216
);
220217

221-
act(() => result.current.startFlow());
222-
223218
postMessageFromPopup(popup, {success: true, data: null});
224219

225220
await act(async () => {
@@ -229,15 +224,15 @@ describe('useAddIntegration', () => {
229224
});
230225

231226
it('closes the dialog on unmount', () => {
232-
const {result, unmount} = renderHookWithProviders(() =>
233-
useAddIntegration({
227+
const {result, unmount} = renderHookWithProviders(() => useAddIntegration());
228+
229+
act(() =>
230+
result.current.startFlow({
234231
provider,
235232
organization: OrganizationFixture(),
236233
onInstall: jest.fn(),
237234
})
238235
);
239-
240-
act(() => result.current.startFlow());
241236
unmount();
242237

243238
expect(popup.close).toHaveBeenCalledTimes(1);
@@ -253,16 +248,16 @@ describe('useAddIntegration', () => {
253248
features: ['integration-api-pipeline-github'],
254249
});
255250

256-
const {result} = renderHookWithProviders(() =>
257-
useAddIntegration({
251+
const {result} = renderHookWithProviders(() => useAddIntegration());
252+
253+
act(() =>
254+
result.current.startFlow({
258255
provider,
259256
organization,
260257
onInstall,
261258
})
262259
);
263260

264-
act(() => result.current.startFlow());
265-
266261
expect(openPipelineModalSpy).toHaveBeenCalledWith({
267262
type: 'integration',
268263
provider: 'github',
@@ -278,16 +273,17 @@ describe('useAddIntegration', () => {
278273
features: ['integration-api-pipeline-github'],
279274
});
280275

281-
const {result} = renderHookWithProviders(() =>
282-
useAddIntegration({
276+
const {result} = renderHookWithProviders(() => useAddIntegration());
277+
278+
act(() =>
279+
result.current.startFlow({
283280
provider,
284281
organization,
285282
onInstall: jest.fn(),
283+
urlParams: {installation_id: '12345'},
286284
})
287285
);
288286

289-
act(() => result.current.startFlow({installation_id: '12345'}));
290-
291287
expect(openPipelineModalSpy).toHaveBeenCalledWith(
292288
expect.objectContaining({
293289
initialData: {installation_id: '12345'},
@@ -303,16 +299,16 @@ describe('useAddIntegration', () => {
303299
features: ['integration-api-pipeline-github'],
304300
});
305301

306-
const {result} = renderHookWithProviders(() =>
307-
useAddIntegration({
302+
const {result} = renderHookWithProviders(() => useAddIntegration());
303+
304+
act(() =>
305+
result.current.startFlow({
308306
provider,
309307
organization,
310308
onInstall: jest.fn(),
311309
})
312310
);
313311

314-
act(() => result.current.startFlow());
315-
316312
expect(window.open).not.toHaveBeenCalled();
317313
});
318314

@@ -324,16 +320,16 @@ describe('useAddIntegration', () => {
324320

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

327-
const {result} = renderHookWithProviders(() =>
328-
useAddIntegration({
323+
const {result} = renderHookWithProviders(() => useAddIntegration());
324+
325+
act(() =>
326+
result.current.startFlow({
329327
provider,
330328
organization,
331329
onInstall: jest.fn(),
332330
})
333331
);
334332

335-
act(() => result.current.startFlow());
336-
337333
expect(openPipelineModalSpy).not.toHaveBeenCalled();
338334
expect(window.open).toHaveBeenCalledTimes(1);
339335
});

0 commit comments

Comments
 (0)