Skip to content

Commit e45dbe6

Browse files
committed
ref(onboarding): Drive the project-details form from host state
The hook kept name, team, and alert config in internal useState seeded once from the host's saved form, a relic of the step component it was extracted from, which remounts on every onboarding navigation. The single-view project-creation flow keeps the hook mounted, so the two copies of the form drift: clearing the host form on a platform or repo change never reached the live fields. Make the form fully controlled: the host owns the state and absent fields derive their defaults, so a cleared form re-derives (the name follows the platform) by construction. The unchanged-return reuse check compares against the form captured at mount, since the live prop can no longer be its own baseline. The wizard holds the live form in its in-memory state; the onboarding step holds it in local state seeded from the context, keeping the context to submitted values only so a remount cannot adopt unsubmitted edits as the reuse baseline.
1 parent 2a27c65 commit e45dbe6

3 files changed

Lines changed: 102 additions & 35 deletions

File tree

static/app/views/onboarding/components/useScmProjectDetails.ts

Lines changed: 54 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useCallback, useState} from 'react';
1+
import {useCallback, useRef} from 'react';
22
import * as Sentry from '@sentry/react';
33

44
import {addErrorMessage} from 'sentry/actionCreators/indicator';
@@ -69,6 +69,13 @@ interface UseScmProjectDetailsOptions {
6969
* both and advance in one place.
7070
*/
7171
onComplete: (completion: ScmProjectDetailsCompletion) => void;
72+
/**
73+
* Live form state, owned by the host. Fields absent from the form derive
74+
* their defaults (platform-based name, first admin team, default alert
75+
* config), so the host clearing the form makes the fields re-derive.
76+
*/
77+
onProjectDetailsFormChange: (form: ProjectDetailsFormState) => void;
78+
projectDetailsForm: ProjectDetailsFormState | undefined;
7279
selectedPlatform: OnboardingSelectedSDK | undefined;
7380
selectedRepository: Repository | undefined;
7481
/**
@@ -79,7 +86,6 @@ interface UseScmProjectDetailsOptions {
7986
allowMemberWithoutTeam?: boolean;
8087
/** Slug of an already-created project, used for the back-nav reuse check. */
8188
createdProjectSlug?: string;
82-
projectDetailsForm?: ProjectDetailsFormState;
8389
}
8490

8591
interface ScmProjectDetailsForm {
@@ -108,19 +114,22 @@ interface ScmProjectDetailsForm {
108114
}
109115

110116
/**
111-
* Owns the SCM project-details form state, the create-project + repo-link flow,
112-
* and the field/create analytics (routed by `analyticsFlow`). Returned to the
113-
* host so it can render the presentational `ScmProjectDetailsCore` form and
114-
* place its own Create button (onboarding's fixed footer, project creation's
115-
* page-level footer) independent of where the form fields render.
117+
* Drives the SCM project-details form (state owned by the host via
118+
* `projectDetailsForm`/`onProjectDetailsFormChange`), the create-project +
119+
* repo-link flow, and the field/create analytics (routed by `analyticsFlow`).
120+
* Returned to the host so it can render the presentational
121+
* `ScmProjectDetailsCore` form and place its own Create button (onboarding's
122+
* fixed footer, project creation's page-level footer) independent of where
123+
* the form fields render.
116124
*/
117125
export function useScmProjectDetails({
118126
analyticsFlow,
119127
onComplete,
128+
onProjectDetailsFormChange,
129+
projectDetailsForm,
120130
selectedPlatform,
121131
selectedRepository,
122132
createdProjectSlug,
123-
projectDetailsForm,
124133
allowMemberWithoutTeam,
125134
}: UseScmProjectDetailsOptions): ScmProjectDetailsForm {
126135
const organization = useOrganization();
@@ -139,41 +148,48 @@ export function useScmProjectDetails({
139148
!organization.access.includes('project:admin');
140149
const defaultName = slugify(selectedPlatform?.key ?? '');
141150

142-
// State tracks user edits. When the host restores a persisted
143-
// projectDetailsForm (e.g. back-navigation in onboarding) it seeds the inputs.
144-
const [projectName, setProjectName] = useState(projectDetailsForm?.projectName ?? null);
145-
const [teamSlug, setTeamSlug] = useState(projectDetailsForm?.teamSlug ?? null);
146-
const [alertRuleConfig, setAlertRuleConfig] = useState(
147-
projectDetailsForm?.alertRuleConfig ?? DEFAULT_ISSUE_ALERT_OPTIONS_VALUES
148-
);
151+
// Fields absent from the host-owned form fall back to derived defaults, so
152+
// a host clearing the form (e.g. on a platform change) re-derives them.
153+
const projectNameResolved = projectDetailsForm?.projectName ?? defaultName;
154+
const teamSlugResolved = projectDetailsForm?.teamSlug ?? firstAdminTeam?.slug ?? '';
155+
const alertRuleConfig =
156+
projectDetailsForm?.alertRuleConfig ?? DEFAULT_ISSUE_ALERT_OPTIONS_VALUES;
149157

150-
const projectNameResolved = projectName ?? defaultName;
151-
const teamSlugResolved = teamSlug ?? firstAdminTeam?.slug ?? '';
158+
// Baseline for the unchanged-return (reuse) check below: the form as it was
159+
// when this step mounted, i.e. a restored session's saved values. Live edits
160+
// flow through the controlled form, so the prop can't be its own baseline.
161+
const savedFormRef = useRef(projectDetailsForm);
152162

153-
const onProjectNameChange = useCallback((value: string) => {
154-
setProjectName(slugify(value));
155-
}, []);
163+
const onProjectNameChange = useCallback(
164+
(value: string) => {
165+
onProjectDetailsFormChange({...projectDetailsForm, projectName: slugify(value)});
166+
},
167+
[onProjectDetailsFormChange, projectDetailsForm]
168+
);
156169

157170
const onProjectNameBlur = useCallback(() => {
158-
if (projectName !== null) {
171+
if (projectDetailsForm?.projectName !== undefined) {
159172
trackAnalytics(NAME_EDITED_EVENT[analyticsFlow], {
160173
organization,
161-
custom: projectName !== defaultName,
174+
custom: projectDetailsForm.projectName !== defaultName,
162175
});
163176
}
164-
}, [projectName, defaultName, organization, analyticsFlow]);
177+
}, [projectDetailsForm?.projectName, defaultName, organization, analyticsFlow]);
165178

166179
const onTeamChange = useCallback(
167180
({value}: {value: string}) => {
168-
setTeamSlug(value);
181+
onProjectDetailsFormChange({...projectDetailsForm, teamSlug: value});
169182
trackAnalytics(TEAM_SELECTED_EVENT[analyticsFlow], {organization, team: value});
170183
},
171-
[organization, analyticsFlow]
184+
[onProjectDetailsFormChange, projectDetailsForm, organization, analyticsFlow]
172185
);
173186

174187
const onAlertChange = useCallback(
175188
<K extends keyof AlertRuleOptions>(key: K, value: AlertRuleOptions[K]) => {
176-
setAlertRuleConfig(prev => ({...prev, [key]: value}));
189+
onProjectDetailsFormChange({
190+
...projectDetailsForm,
191+
alertRuleConfig: {...alertRuleConfig, [key]: value},
192+
});
177193
if (key === 'alertSetting') {
178194
const optionMap: Record<number, string> = {
179195
[RuleAction.DEFAULT_ALERT]: 'high_priority',
@@ -186,7 +202,13 @@ export function useScmProjectDetails({
186202
});
187203
}
188204
},
189-
[organization, analyticsFlow]
205+
[
206+
onProjectDetailsFormChange,
207+
projectDetailsForm,
208+
alertRuleConfig,
209+
organization,
210+
analyticsFlow,
211+
]
190212
);
191213

192214
// Block submission until teams and the projects store have loaded so the
@@ -207,12 +229,13 @@ export function useScmProjectDetails({
207229
// snapshot because the Project model tracks it; alert fields are not on the
208230
// Project record so we compare those against the context snapshot.
209231
const samePlatform = existingProject?.platform === selectedPlatform?.key;
210-
const savedAlert = projectDetailsForm?.alertRuleConfig;
232+
const savedForm = savedFormRef.current;
233+
const savedAlert = savedForm?.alertRuleConfig;
211234
const nothingChanged =
212235
samePlatform &&
213-
!!projectDetailsForm &&
214-
projectNameResolved === projectDetailsForm.projectName &&
215-
teamSlugResolved === projectDetailsForm.teamSlug &&
236+
!!savedForm &&
237+
projectNameResolved === savedForm.projectName &&
238+
teamSlugResolved === savedForm.teamSlug &&
216239
alertRuleConfig.alertSetting === savedAlert?.alertSetting &&
217240
alertRuleConfig.interval === savedAlert?.interval &&
218241
alertRuleConfig.metric === savedAlert?.metric &&

static/app/views/onboarding/scmProjectDetails.spec.tsx

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@ import {ProjectFixture} from 'sentry-fixture/project';
33
import {RepositoryFixture} from 'sentry-fixture/repository';
44
import {TeamFixture} from 'sentry-fixture/team';
55

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

814
import type {ProductSolution} from 'sentry/components/onboarding/gettingStartedDoc/types';
915
import type {ProjectDetailsFormState} from 'sentry/components/onboarding/onboardingContext';
@@ -12,6 +18,7 @@ import {TeamStore} from 'sentry/stores/teamStore';
1218
import type {Repository} from 'sentry/types/integrations';
1319
import type {OnboardingSelectedSDK} from 'sentry/types/onboarding';
1420
import * as analytics from 'sentry/utils/analytics';
21+
import {useScmProjectDetails} from 'sentry/views/onboarding/components/useScmProjectDetails';
1522
import {MetricValues, RuleAction} from 'sentry/views/projectInstall/issueAlertOptions';
1623

1724
import {ScmProjectDetails} from './scmProjectDetails';
@@ -112,6 +119,32 @@ describe('ScmProjectDetails', () => {
112119
expect(screen.getByText("I'll create my own alerts later")).toBeInTheDocument();
113120
});
114121

122+
it('re-derives the fields when the host clears the form', () => {
123+
const hookProps: Parameters<typeof useScmProjectDetails>[0] = {
124+
analyticsFlow: 'onboarding',
125+
selectedPlatform: mockPlatform,
126+
selectedRepository: undefined,
127+
createdProjectSlug: undefined,
128+
projectDetailsForm: {
129+
projectName: 'restored-name',
130+
teamSlug: teamWithAccess.slug,
131+
},
132+
onProjectDetailsFormChange: jest.fn(),
133+
onComplete: jest.fn(),
134+
};
135+
const {result, rerender} = renderHookWithProviders(useScmProjectDetails, {
136+
organization,
137+
initialProps: hookProps,
138+
});
139+
140+
expect(result.current.projectName).toBe('restored-name');
141+
142+
// The host clears the form (platform or repo change in the single-view
143+
// flow); the name falls back to the platform default.
144+
rerender({...hookProps, projectDetailsForm: undefined});
145+
expect(result.current.projectName).toBe('javascript-nextjs');
146+
});
147+
115148
it('create project button is disabled without platform', async () => {
116149
render(<ScmProjectDetails {...defaultProps()} />, {organization});
117150

static/app/views/onboarding/scmProjectDetails.tsx

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import {useState} from 'react';
2+
13
import {Button} from '@sentry/scraps/button';
24
import {Flex} from '@sentry/scraps/layout';
35

@@ -41,20 +43,29 @@ export function ScmProjectDetails({
4143
selectedRepository,
4244
genBackButton,
4345
}: ScmProjectDetailsProps) {
46+
// Live form for this step, seeded from the saved form in the onboarding
47+
// context. The context only ever holds submitted values, which the hook's
48+
// unchanged-return reuse check relies on as its baseline, so live edits stay
49+
// here. This step remounts on every onboarding navigation, so each visit
50+
// re-seeds and abandoned edits are discarded.
51+
const [liveForm, setLiveForm] = useState(projectDetailsForm);
52+
4453
const form = useScmProjectDetails({
4554
analyticsFlow: 'onboarding',
4655
selectedPlatform,
4756
selectedRepository,
4857
createdProjectSlug,
49-
projectDetailsForm,
58+
projectDetailsForm: liveForm,
59+
onProjectDetailsFormChange: setLiveForm,
5060
onComplete: ({
5161
project,
5262
projectDetailsForm: submittedForm,
5363
}: ScmProjectDetailsCompletion) => {
5464
// Store the slug separately so onboarding.tsx can find the project via
5565
// useRecentCreatedProject without corrupting selectedPlatform.key (which
56-
// the platform features step needs), and persist the form so navigating
57-
// back from setup-docs restores it. Both land before the step advances.
66+
// the platform features step needs), and persist the submitted form so
67+
// navigating back from setup-docs restores it. Both land before the
68+
// step advances.
5869
onProjectCreated(project.slug);
5970
onProjectDetailsFormChange(submittedForm);
6071
onComplete(undefined, selectedFeatures ? {product: selectedFeatures} : undefined);

0 commit comments

Comments
 (0)