Skip to content

ref(onboarding): Persist project details form state in onboarding context #113128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jaydgoss wants to merge 1 commit into
base: master
Choose a base branch
from
+123 −6
Open

ref(onboarding): Persist project details form state in onboarding context #113128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions static/app/components/onboarding/onboardingContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,33 @@ import {createContext, useContext, useMemo} from 'react';
import type {ProductSolution} from 'sentry/components/onboarding/gettingStartedDoc/types';
import type {Integration, Repository} from 'sentry/types/integrations';
import type {OnboardingSelectedSDK} from 'sentry/types/onboarding';
import type {PlatformKey} from 'sentry/types/project';
import {useSessionStorage} from 'sentry/utils/useSessionStorage';
import type {AlertRuleOptions} from 'sentry/views/projectInstall/issueAlertOptions';

/**
* Persisted form state from the SCM project details step. Stored so the
* form can be restored when the user navigates back from setup-docs.
* `platform` records the platform at creation time so we can detect
* platform changes when the user navigates back through earlier steps.
*/
export interface ProjectDetailsFormState {
alertRuleConfig?: AlertRuleOptions;
platform?: PlatformKey;
projectName?: string;
teamSlug?: string;
}

type OnboardingContextProps = {
clearDerivedState: () => void;
setCreatedProjectSlug: (slug?: string) => void;
setProjectDetailsForm: (form?: ProjectDetailsFormState) => void;
setSelectedFeatures: (features?: ProductSolution[]) => void;
setSelectedIntegration: (integration?: Integration) => void;
setSelectedPlatform: (selectedSDK?: OnboardingSelectedSDK) => void;
setSelectedRepository: (repo?: Repository) => void;
createdProjectSlug?: string;
projectDetailsForm?: ProjectDetailsFormState;
selectedFeatures?: ProductSolution[];
selectedIntegration?: Integration;
selectedPlatform?: OnboardingSelectedSDK;
Expand All @@ -21,6 +38,7 @@ type OnboardingContextProps = {

export type OnboardingSessionState = {
createdProjectSlug?: string;
projectDetailsForm?: ProjectDetailsFormState;
selectedFeatures?: ProductSolution[];
selectedIntegration?: Integration;
selectedPlatform?: OnboardingSelectedSDK;
Expand All @@ -41,6 +59,8 @@ const OnboardingContext = createContext<OnboardingContextProps>({
setSelectedFeatures: () => {},
createdProjectSlug: undefined,
setCreatedProjectSlug: () => {},
projectDetailsForm: undefined,
setProjectDetailsForm: () => {},
clearDerivedState: () => {},
});

Expand Down Expand Up @@ -84,6 +104,10 @@ export function OnboardingContextProvider({children, initialValue}: ProviderProp
setCreatedProjectSlug: (createdProjectSlug?: string) => {
setOnboarding(prev => ({...prev, createdProjectSlug}));
},
projectDetailsForm: onboarding?.projectDetailsForm,
setProjectDetailsForm: (projectDetailsForm?: ProjectDetailsFormState) => {
setOnboarding(prev => ({...prev, projectDetailsForm}));
},
// Clear state derived from the selected repository (platform, features,
// created project) without wiping the entire session. Use this when the
// repo changes so downstream steps start fresh.
Expand All @@ -93,6 +117,7 @@ export function OnboardingContextProvider({children, initialValue}: ProviderProp
selectedPlatform: undefined,
selectedFeatures: undefined,
createdProjectSlug: undefined,
projectDetailsForm: undefined,
}));
},
}),
Expand Down
76 changes: 76 additions & 0 deletions static/app/views/onboarding/scmProjectDetails.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,82 @@ describe('ScmProjectDetails', () => {
expect(stored.selectedPlatform?.key).toBe('javascript-nextjs');
});

it('restores form inputs from persisted projectDetailsForm', async () => {
render(
<ScmProjectDetails
onComplete={jest.fn()}
stepIndex={3}
genSkipOnboardingLink={() => null}
/>,
{
organization,
additionalWrapper: makeOnboardingWrapper({
selectedPlatform: mockPlatform,
projectDetailsForm: {
projectName: 'my-saved-name',
teamSlug: teamWithAccess.slug,
platform: 'javascript-nextjs',
},
}),
}
);

const input = await screen.findByPlaceholderText('project-name');
expect(input).toHaveValue('my-saved-name');
});

it('persists form state to context on successful creation', async () => {
MockApiClient.addMockResponse({
url: `/teams/${organization.slug}/${teamWithAccess.slug}/projects/`,
method: 'POST',
body: ProjectFixture({slug: 'javascript-nextjs', name: 'javascript-nextjs'}),
});
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/`,
body: organization,
});
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/projects/`,
body: [],
});
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/teams/`,
body: [teamWithAccess],
});

const onComplete = jest.fn();

render(
<ScmProjectDetails
onComplete={onComplete}
stepIndex={3}
genSkipOnboardingLink={() => null}
/>,
{
organization,
additionalWrapper: makeOnboardingWrapper({
selectedPlatform: mockPlatform,
}),
}
);

await userEvent.click(await screen.findByRole('button', {name: 'Create project'}));

await waitFor(() => {
expect(onComplete).toHaveBeenCalled();
});

const stored = JSON.parse(sessionStorageWrapper.getItem('onboarding') ?? '{}');
expect(stored.projectDetailsForm).toEqual(
expect.objectContaining({
projectName: 'javascript-nextjs',
teamSlug: teamWithAccess.slug,
platform: 'javascript-nextjs',
})
);
expect(stored.projectDetailsForm.alertRuleConfig).toBeDefined();
});

it('shows error message on project creation failure', async () => {
const onComplete = jest.fn();

Expand Down
28 changes: 22 additions & 6 deletions static/app/views/onboarding/scmProjectDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,13 @@ const PROJECT_DETAILS_WIDTH = '285px';

export function ScmProjectDetails({onComplete}: StepProps) {
const organization = useOrganization();
const {selectedPlatform, selectedFeatures, setCreatedProjectSlug} =
useOnboardingContext();
const {
selectedPlatform,
selectedFeatures,
setCreatedProjectSlug,
projectDetailsForm,
setProjectDetailsForm,
} = useOnboardingContext();
const {teams} = useTeams();
const createProjectAndRules = useCreateProjectAndRules();
useEffect(() => {
Expand All @@ -44,15 +49,20 @@ export function ScmProjectDetails({onComplete}: StepProps) {
const firstAdminTeam = teams.find((team: Team) => team.access.includes('team:admin'));
const defaultName = slugify(selectedPlatform?.key ?? '');

// State tracks user edits; derived values fall back to defaults from context/teams
const [projectName, setProjectName] = useState<string | null>(null);
const [teamSlug, setTeamSlug] = useState<string | null>(null);
// State tracks user edits. When the user navigates back from setup-docs
// the persisted projectDetailsForm restores their previous inputs.
const [projectName, setProjectName] = useState<string | null>(
projectDetailsForm?.projectName ?? null
);
const [teamSlug, setTeamSlug] = useState<string | null>(
projectDetailsForm?.teamSlug ?? null
Comment on lines +54 to +58
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The project details form restores state from session storage without validating if the stored platform matches the current selection, leading to stale data if the user changes platforms mid-onboarding.
Severity: MEDIUM

Suggested Fix

When initializing the projectName state, add a check to ensure the platform stored in projectDetailsForm matches the currently selectedPlatform.key. If they do not match, the projectName should be initialized as null to clear the stale data. For example: useState<string | null>(projectDetailsForm?.platform === selectedPlatform?.key ? projectDetailsForm?.projectName : null).

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/views/onboarding/scmProjectDetails.tsx#L54-L58

Potential issue: The `scmProjectDetails.tsx` component restores form state from session
storage, including the project name and the platform it was created for. However, when
initializing the form, it fails to validate if the stored platform
(`projectDetailsForm.platform`) matches the currently selected platform
(`selectedPlatform.key`). If a user navigates back, changes their SCM platform, and then
returns to the project details step, the form will incorrectly display the project name
associated with the previous platform selection. This happens because the `platform`
field, which was added specifically to detect such changes, is never actually read or
used for validation.

Did we get this right? 👍 / 👎 to inform future reviews.

);

const projectNameResolved = projectName ?? defaultName;
const teamSlugResolved = teamSlug ?? firstAdminTeam?.slug ?? '';

const [alertRuleConfig, setAlertRuleConfig] = useState<AlertRuleOptions>(
DEFAULT_ISSUE_ALERT_OPTIONS_VALUES
projectDetailsForm?.alertRuleConfig ?? DEFAULT_ISSUE_ALERT_OPTIONS_VALUES
);

function handleAlertChange<K extends keyof AlertRuleOptions>(
Expand Down Expand Up @@ -116,6 +126,12 @@ export function ScmProjectDetails({onComplete}: StepProps) {
// the project via useRecentCreatedProject without corrupting
// selectedPlatform.key (which the platform features step needs).
setCreatedProjectSlug(project.slug);
setProjectDetailsForm({
projectName: projectNameResolved,
teamSlug: teamSlugResolved,
alertRuleConfig,
platform: selectedPlatform.key,
});

trackAnalytics('onboarding.scm_project_details_create_succeeded', {
organization,
Expand Down
Loading