Skip to content

Commit c4d647b

Browse files
authored
chore(scm-project-creation-flow): Clearing alert channel on refetch (#119461)
Fixes: [LINEAR TICKET](https://linear.app/getsentry/issue/VDY-130/messaging-integration-refetch-resets-providerintegration-without) The notification picker's auto-select effect was keyed on `providersToIntegrations`, which gets a fresh reference on every refetch (stale time zero + refetch-on-focus), so refetches re-ran it and reset the user's provider/integration and could strand a chosen channel. This guards the init with a ref so it runs once after the query first succeeds and clears the channel there; later refetches no longer rewrite a user-selected provider, integration, or channel. Covered by new `issueAlertNotificationOptions.spec.tsx` tests simulating a refetch and asserting the user's selection persists, with existing specs updated.
1 parent ae1e155 commit c4d647b

4 files changed

Lines changed: 176 additions & 13 deletions

File tree

static/app/views/projectInstall/issueAlertNotificationOptions.spec.tsx

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,22 @@ import {GitHubIntegrationProviderFixture} from 'sentry-fixture/githubIntegration
22
import {OrganizationFixture} from 'sentry-fixture/organization';
33
import {OrganizationIntegrationsFixture} from 'sentry-fixture/organizationIntegrations';
44

5-
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
5+
import {
6+
act,
7+
render,
8+
renderHookWithProviders,
9+
screen,
10+
userEvent,
11+
waitFor,
12+
} from 'sentry-test/reactTestingLibrary';
613

14+
import {IssueAlertActionType} from 'sentry/types/alerts';
715
import type {OrganizationIntegration} from 'sentry/types/integrations';
816
import {
917
IssueAlertNotificationOptions,
1018
type IssueAlertNotificationProps,
19+
MultipleCheckboxOptions,
20+
useCreateNotificationAction,
1121
} from 'sentry/views/projectInstall/issueAlertNotificationOptions';
1222

1323
describe('MessagingIntegrationAlertRule', () => {
@@ -94,3 +104,116 @@ describe('MessagingIntegrationAlertRule', () => {
94104
expect(mockSetAction).toHaveBeenCalled();
95105
});
96106
});
107+
108+
describe('useCreateNotificationAction', () => {
109+
const organization = OrganizationFixture();
110+
111+
const slackIntegration = OrganizationIntegrationsFixture({
112+
id: '1',
113+
name: 'my-workspace',
114+
status: 'active',
115+
provider: {
116+
key: 'slack',
117+
slug: 'slack',
118+
name: 'Slack',
119+
canAdd: true,
120+
canDisable: false,
121+
features: [],
122+
aspects: {},
123+
},
124+
});
125+
126+
function addIntegrationsResponse(body: OrganizationIntegration[]) {
127+
return MockApiClient.addMockResponse({
128+
url: `/organizations/${organization.slug}/integrations/`,
129+
body,
130+
match: [MockApiClient.matchQuery({integrationType: 'messaging'})],
131+
});
132+
}
133+
134+
afterEach(() => {
135+
MockApiClient.clearMockResponses();
136+
});
137+
138+
it('defaults provider and integration from the first result on load', async () => {
139+
addIntegrationsResponse([slackIntegration]);
140+
141+
const {result} = renderHookWithProviders(() => useCreateNotificationAction(), {
142+
organization,
143+
});
144+
145+
// Initially unset while the query is pending.
146+
expect(result.current.notificationProps.provider).toBeUndefined();
147+
148+
// After the query resolves, defaults to the first provider/integration.
149+
await waitFor(() => expect(result.current.notificationProps.provider).toBe('slack'));
150+
expect(result.current.notificationProps.integration?.id).toBe(slackIntegration.id);
151+
expect(result.current.notificationProps.channel).toBeUndefined();
152+
});
153+
154+
it('does not clobber a user-selected channel when the integrations list refetches', async () => {
155+
const secondIntegration = OrganizationIntegrationsFixture({
156+
id: '2',
157+
name: 'another-workspace',
158+
status: 'active',
159+
provider: slackIntegration.provider,
160+
});
161+
162+
// Initial load returns one integration.
163+
addIntegrationsResponse([slackIntegration]);
164+
165+
const {result, rerender} = renderHookWithProviders(
166+
() => useCreateNotificationAction(),
167+
{organization}
168+
);
169+
170+
await waitFor(() => expect(result.current.notificationProps.provider).toBe('slack'));
171+
172+
// User picks a channel.
173+
act(() => {
174+
result.current.notificationProps.setChannel({label: '#alerts', value: '#alerts'});
175+
});
176+
expect(result.current.notificationProps.channel?.value).toBe('#alerts');
177+
178+
// A refetch comes in with an updated list. Simulate by providing a new mock response
179+
// with two integrations and re-rendering so the deps change.
180+
MockApiClient.clearMockResponses();
181+
addIntegrationsResponse([slackIntegration, secondIntegration]);
182+
act(() => {
183+
rerender();
184+
});
185+
186+
// The run-once guard holds: provider/integration/channel are not reset.
187+
expect(result.current.notificationProps.provider).toBe('slack');
188+
expect(result.current.notificationProps.integration?.id).toBe(slackIntegration.id);
189+
expect(result.current.notificationProps.channel?.value).toBe('#alerts');
190+
});
191+
192+
it('resolves provider, integration, and actions from defaultActions on mount', async () => {
193+
addIntegrationsResponse([slackIntegration]);
194+
195+
// Stable reference: the autofill effect depends on `defaultActions`, so an
196+
// inline array (new ref each render) would loop render -> setState -> render.
197+
const defaultActions = [
198+
{
199+
id: IssueAlertActionType.SLACK,
200+
workspace: slackIntegration.id,
201+
channel: '#eng',
202+
},
203+
];
204+
205+
const {result} = renderHookWithProviders(
206+
() => useCreateNotificationAction({actions: defaultActions}),
207+
{organization}
208+
);
209+
210+
await act(async () => {});
211+
212+
// Autofill effect from defaultActions sets provider, integration, and channel.
213+
expect(result.current.notificationProps.provider).toBe('slack');
214+
expect(result.current.notificationProps.actions).toContain(
215+
MultipleCheckboxOptions.INTEGRATION
216+
);
217+
expect(result.current.notificationProps.channel?.value).toBe('#eng');
218+
});
219+
});

static/app/views/projectInstall/issueAlertNotificationOptions.tsx

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import {Fragment, useCallback, useEffect, useMemo, useState, type ReactNode} from 'react';
1+
import {
2+
Fragment,
3+
useCallback,
4+
useEffect,
5+
useMemo,
6+
useRef,
7+
useState,
8+
type ReactNode,
9+
} from 'react';
210

311
import {Stack} from '@sentry/scraps/layout';
412

@@ -123,7 +131,16 @@ export function useCreateNotificationAction({
123131
undefined
124132
);
125133
const [channel, setChannel] = useState<IntegrationChannel | undefined>(undefined);
126-
const [shouldRenderSetupButton, setShouldRenderSetupButton] = useState(false);
134+
135+
const hasInitializedSelection = useRef(false);
136+
137+
// Derived rather than state so it stays in sync with the query instead of
138+
// freezing at its first-success value: if the first fetch has no
139+
// integrations, connecting one via SetupMessagingIntegrationButton
140+
// refetches this query and should reveal the integration checkbox.
141+
const shouldRenderSetupButton =
142+
messagingIntegrationsQuery.isSuccess &&
143+
Object.keys(providersToIntegrations).length === 0;
127144

128145
useEffect(() => {
129146
// Initializes form state based on the first default action and available integrations.
@@ -145,8 +162,6 @@ export function useCreateNotificationAction({
145162
setProvider(matchedProviderKey);
146163
setIntegration(matchedIntegration);
147164

148-
setShouldRenderSetupButton(!matchedIntegration);
149-
150165
const newActions =
151166
firstAction.id === IssueAlertActionType.NOTIFY_EMAIL
152167
? [MultipleCheckboxOptions.EMAIL]
@@ -164,14 +179,24 @@ export function useCreateNotificationAction({
164179
}, [defaultActions, providersToIntegrations]);
165180

166181
useEffect(() => {
167-
if (messagingIntegrationsQuery.isSuccess) {
168-
const providerKeys = Object.keys(providersToIntegrations);
169-
const firstProvider = providerKeys[0];
170-
const firstIntegration = providersToIntegrations[String(firstProvider)]?.[0];
171-
setProvider(firstProvider);
172-
setIntegration(firstIntegration);
173-
setShouldRenderSetupButton(!firstProvider);
182+
if (!messagingIntegrationsQuery.isSuccess || hasInitializedSelection.current) {
183+
return;
174184
}
185+
const providerKeys = Object.keys(providersToIntegrations);
186+
const firstProvider = providerKeys[0];
187+
188+
// If the first fetch returned no integrations, don't mark as initialized yet.
189+
// A subsequent refetch (e.g. after connecting via SetupMessagingIntegrationButton)
190+
// may deliver integrations and must be allowed to auto-select provider/integration.
191+
if (!firstProvider) {
192+
return;
193+
}
194+
195+
hasInitializedSelection.current = true;
196+
const firstIntegration = providersToIntegrations[String(firstProvider)]?.[0];
197+
setProvider(firstProvider);
198+
setIntegration(firstIntegration);
199+
setChannel(undefined);
175200
}, [messagingIntegrationsQuery.isSuccess, providersToIntegrations]);
176201

177202
const createNotificationAction = useCallback(

static/app/views/projectInstall/messagingIntegrationAlertRule.spec.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,21 @@ describe('MessagingIntegrationAlertRule', () => {
8888
expect(screen.getAllByRole('textbox')).toHaveLength(3);
8989
});
9090

91+
it('clears the channel select when channel prop becomes undefined', () => {
92+
const {rerender} = render(getComponent(), {organization});
93+
94+
// The initial channel value label is visible.
95+
expect(screen.getByText('channel')).toBeInTheDocument();
96+
97+
// Parent state clears channel (e.g. after provider or integration change).
98+
rerender(
99+
<MessagingIntegrationAlertRule {...notificationProps} channel={undefined} />
100+
);
101+
102+
// The stale channel label must no longer be shown; the select is empty.
103+
expect(screen.queryByText('channel')).not.toBeInTheDocument();
104+
});
105+
91106
it('calls setter when new integration is selected', async () => {
92107
render(getComponent());
93108
await selectEvent.select(

static/app/views/projectInstall/messagingIntegrationAlertRule.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ export function ChannelSelect({
171171
options={options}
172172
isLoading={isLoading}
173173
disabled={disabled}
174-
value={value ? {label: value.label, value: value.value} : undefined}
174+
value={value ? {label: value.label, value: value.value} : null}
175175
onChange={onChange}
176176
onCreateOption={onCreateOption}
177177
clearable

0 commit comments

Comments
 (0)