Skip to content

Commit 2d65871

Browse files
committed
fix(project-creation): Preserve detected selection on back-to-recommended
handleBackToRecommended reset the selected features and project-details form whenever a detected platform existed, even when the platform did not change. Two problems followed: opening the manual picker and returning without a different choice wiped customized features and the project name, and returning while a non-top detected platform was selected switched it to the top detection, clearing a valid choice. Return early when the active platform is already one of the detected ones, mirroring handleSelectDetectedPlatform. Only a manual (non-detected) pick now falls back to the top detection, since the cards view cannot display it otherwise.
1 parent bc5633e commit 2d65871

2 files changed

Lines changed: 122 additions & 10 deletions

File tree

static/app/views/onboarding/components/scmPlatformFeaturesCore.spec.tsx

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import {useState} from 'react';
2+
import {DetectedPlatformFixture} from 'sentry-fixture/detectedPlatform';
13
import {OrganizationFixture} from 'sentry-fixture/organization';
4+
import {RepositoryFixture} from 'sentry-fixture/repository';
25

3-
import {render, screen} from 'sentry-test/reactTestingLibrary';
6+
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
47

58
import type {OnboardingSelectedSDK} from 'sentry/types/onboarding';
69
import * as analytics from 'sentry/utils/analytics';
@@ -96,4 +99,102 @@ describe('ScmPlatformFeaturesCore', () => {
9699
expect.anything()
97100
);
98101
});
102+
103+
it('keeps a non-default detected selection when returning to the recommended view', async () => {
104+
const onFeaturesChange = jest.fn();
105+
const onClearProjectDetailsForm = jest.fn();
106+
const repository = RepositoryFixture({
107+
id: '123',
108+
provider: {id: 'integrations:github', name: 'GitHub'},
109+
});
110+
// python is the recommendation (first detected); javascript is the second.
111+
MockApiClient.addMockResponse({
112+
url: `/organizations/${organization.slug}/repos/${repository.id}/platforms/`,
113+
body: {
114+
platforms: [
115+
DetectedPlatformFixture({platform: 'python'}),
116+
DetectedPlatformFixture({platform: 'javascript'}),
117+
],
118+
},
119+
});
120+
121+
// Controlled, so hold the selection in state for the manual-pick flow.
122+
function Host() {
123+
const [platform, setPlatform] = useState<OnboardingSelectedSDK | undefined>(
124+
pythonPlatform
125+
);
126+
return (
127+
<ScmPlatformFeaturesCore
128+
analyticsFlow="onboarding"
129+
selectedRepository={repository}
130+
selectedPlatform={platform}
131+
onPlatformChange={setPlatform}
132+
onFeaturesChange={onFeaturesChange}
133+
onClearProjectDetailsForm={onClearProjectDetailsForm}
134+
/>
135+
);
136+
}
137+
138+
render(<Host />, {organization});
139+
140+
// Select the second detected platform, then open the manual picker.
141+
await userEvent.click(
142+
await screen.findByRole('radio', {name: 'Browser JavaScript Language'})
143+
);
144+
await userEvent.click(
145+
screen.getByRole('button', {name: "Doesn't look right? Change platform"})
146+
);
147+
onFeaturesChange.mockClear();
148+
onClearProjectDetailsForm.mockClear();
149+
150+
// Returning keeps the chosen detected platform: it is already detected, so
151+
// nothing is reset and the card stays selected.
152+
await userEvent.click(
153+
screen.getByRole('button', {name: 'Back to recommended platforms'})
154+
);
155+
156+
expect(
157+
await screen.findByRole('radio', {name: 'Browser JavaScript Language'})
158+
).toBeChecked();
159+
expect(onFeaturesChange).not.toHaveBeenCalled();
160+
expect(onClearProjectDetailsForm).not.toHaveBeenCalled();
161+
});
162+
163+
it('does not reset state when returning without a change', async () => {
164+
const onFeaturesChange = jest.fn();
165+
const onClearProjectDetailsForm = jest.fn();
166+
const repository = RepositoryFixture({
167+
id: '123',
168+
provider: {id: 'integrations:github', name: 'GitHub'},
169+
});
170+
// python is both the recommendation and the already-selected platform.
171+
MockApiClient.addMockResponse({
172+
url: `/organizations/${organization.slug}/repos/${repository.id}/platforms/`,
173+
body: {platforms: [DetectedPlatformFixture({platform: 'python'})]},
174+
});
175+
176+
render(
177+
<ScmPlatformFeaturesCore
178+
{...defaultProps({
179+
selectedRepository: repository,
180+
selectedPlatform: pythonPlatform,
181+
onFeaturesChange,
182+
onClearProjectDetailsForm,
183+
})}
184+
/>,
185+
{organization}
186+
);
187+
188+
// Open the manual picker from the detected view, then return without
189+
// choosing a different platform.
190+
await userEvent.click(
191+
await screen.findByRole('button', {name: "Doesn't look right? Change platform"})
192+
);
193+
await userEvent.click(
194+
screen.getByRole('button', {name: 'Back to recommended platforms'})
195+
);
196+
197+
expect(onFeaturesChange).not.toHaveBeenCalled();
198+
expect(onClearProjectDetailsForm).not.toHaveBeenCalled();
199+
});
99200
});

static/app/views/onboarding/components/scmPlatformFeaturesCore.tsx

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,13 @@ export function ScmPlatformFeaturesCore({
124124
isDetectionError,
125125
} = useScmResolvedPlatform({selectedPlatform, selectedRepository});
126126

127+
// Whether the active platform is one of the detected ones. Gates the
128+
// detected-cards view below, and lets "Back to recommended" keep a detected
129+
// selection (including a non-top one) rather than forcing the top detection.
130+
const currentPlatformIsDetected = resolvedPlatforms.some(
131+
p => p.platform === currentPlatformKey
132+
);
133+
127134
// Adopt the first detected platform once per repo when the user hasn't
128135
// explicitly chosen one: commit it to the host so flows without a Continue
129136
// boundary (single-view project creation) get a platform without an explicit
@@ -257,11 +264,18 @@ export function ScmPlatformFeaturesCore({
257264

258265
function handleBackToRecommended() {
259266
setShowManualPicker(false);
260-
if (detectedPlatformKey) {
261-
setPlatform(detectedPlatformKey);
262-
onFeaturesChange(DEFAULT_SCM_FEATURES);
263-
onClearProjectDetailsForm();
267+
// If the active platform is already a detected one, just reopen the cards
268+
// view with it still selected. The user may have picked a non-top detection
269+
// (or the auto-adopted default), so forcing the top detection here would
270+
// clear a valid selection and wipe the derived features/form for no reason.
271+
// Only a manual (non-detected) pick needs the fallback, since the cards view
272+
// can't display it otherwise.
273+
if (currentPlatformIsDetected || !detectedPlatformKey) {
274+
return;
264275
}
276+
setPlatform(detectedPlatformKey);
277+
onFeaturesChange(DEFAULT_SCM_FEATURES);
278+
onClearProjectDetailsForm();
265279
}
266280

267281
// Ensure the selected platform is always present in the dropdown options
@@ -287,11 +301,8 @@ export function ScmPlatformFeaturesCore({
287301
];
288302
}, [currentPlatformKey]);
289303

290-
// If the user previously selected a platform manually (not in the detected
291-
// list), show the manual picker so their selection is visible.
292-
const currentPlatformIsDetected = resolvedPlatforms.some(
293-
p => p.platform === currentPlatformKey
294-
);
304+
// When the active platform is a manual (non-detected) pick, show the manual
305+
// picker so the selection stays visible (see showDetectedPlatforms below).
295306
const hasDetectedPlatforms = resolvedPlatforms.length > 0 || isDetecting;
296307
// Fall through to manual picker on detection error
297308
const showDetectedPlatforms =

0 commit comments

Comments
 (0)