From f09dc210c93f2d26208f4c9528d484032a667745 Mon Sep 17 00:00:00 2001 From: Jay Goss Date: Fri, 31 Jul 2026 16:55:08 -0500 Subject: [PATCH] ref(onboarding): Make onboarding session exits explicit setSelectedPlatform(undefined) routed through removeOnboarding, so clearing one field wiped the entire onboarding session. That coupling was invisible at the call sites and made the field setter unsafe for any state that should outlive a platform change. Field setters are now local to their field, and the five full-flow exits that genuinely want a clean session call resetOnboarding explicitly: global Skip Onboarding, the SCM header Skip, back-from-select-platform, project deletion without state preservation, and the legacy welcome-step cleanup. The SCM welcome path uses clearDerivedState instead, which already existed for repository changes. User-visible effect: the platform picker's clear control in the SCM flow no longer discards the connected repository. --- .../onboarding/onboardingContext.spec.tsx | 69 ++++++++++++++++++- .../onboarding/onboardingContext.tsx | 13 ++-- .../components/onboardingSkipButton.tsx | 3 + static/app/views/onboarding/onboarding.tsx | 2 +- .../app/views/onboarding/useBackActions.tsx | 4 +- .../app/views/onboarding/useConfigureSdk.tsx | 2 +- .../onboarding/useWelcomeAnalyticsEffect.ts | 6 +- 7 files changed, 87 insertions(+), 12 deletions(-) diff --git a/static/app/components/onboarding/onboardingContext.spec.tsx b/static/app/components/onboarding/onboardingContext.spec.tsx index 42e1c7f4b2b9..37bcc79317a1 100644 --- a/static/app/components/onboarding/onboardingContext.spec.tsx +++ b/static/app/components/onboarding/onboardingContext.spec.tsx @@ -1,6 +1,6 @@ import {RepositoryFixture} from 'sentry-fixture/repository'; -import {render, screen} from 'sentry-test/reactTestingLibrary'; +import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; import {ProductSolution} from 'sentry/components/onboarding/gettingStartedDoc/types'; import { @@ -18,7 +18,13 @@ const platform = { }; function StateConsumer() { - const {selectedRepository, selectedPlatform, selectedFeatures} = useOnboardingContext(); + const { + selectedRepository, + selectedPlatform, + selectedFeatures, + setSelectedPlatform, + resetOnboarding, + } = useOnboardingContext(); return (
{selectedRepository ? `repo:${selectedRepository.id}` : 'no-repo'}
@@ -26,6 +32,8 @@ function StateConsumer() {
{selectedFeatures ? `features:${selectedFeatures.length}` : 'no-features'}
+ +
); } @@ -74,3 +82,60 @@ describe('OnboardingContextProvider', () => { expect(screen.getByText('features:1')).toBeInTheDocument(); }); }); + +describe('OnboardingContextProvider session semantics', () => { + afterEach(() => { + window.sessionStorage.clear(); + }); + + it('keeps the rest of the session when clearing the selected platform', async () => { + render( + + + + ); + + await userEvent.click(screen.getByRole('button', {name: 'Clear platform'})); + + // Clearing one field must stay local to that field. This previously routed + // through removeOnboarding and wiped the whole session, taking the + // connected repository with it. + expect(screen.getByText('no-platform')).toBeInTheDocument(); + expect(screen.getByText('repo:42')).toBeInTheDocument(); + expect(JSON.parse(sessionStorage.getItem('onboarding') ?? '{}')).toMatchObject({ + selectedRepository: {id: '42'}, + }); + }); + + it('clears persisted session state on resetOnboarding', async () => { + // Seeded through sessionStorage rather than the initialValue prop: + // useSessionStorage's removeItem resets in-memory state back to + // initialValue, so a seeded prop would be restored rather than cleared. + // The real provider passes no initialValue, so removal is total there. + sessionStorage.setItem( + 'onboarding', + JSON.stringify({ + selectedRepository: RepositoryFixture({id: '42'}), + selectedPlatform: platform, + }) + ); + + render( + + + + ); + expect(screen.getByText('platform:javascript-nextjs')).toBeInTheDocument(); + + await userEvent.click(screen.getByRole('button', {name: 'Reset onboarding'})); + + expect(screen.getByText('no-platform')).toBeInTheDocument(); + expect(screen.getByText('no-repo')).toBeInTheDocument(); + expect(sessionStorage.getItem('onboarding')).toBeNull(); + }); +}); diff --git a/static/app/components/onboarding/onboardingContext.tsx b/static/app/components/onboarding/onboardingContext.tsx index ec067e6c027c..3db34c4b6caf 100644 --- a/static/app/components/onboarding/onboardingContext.tsx +++ b/static/app/components/onboarding/onboardingContext.tsx @@ -7,6 +7,7 @@ import {useSessionStorage} from 'sentry/utils/useSessionStorage'; type OnboardingContextProps = { clearDerivedState: () => void; + resetOnboarding: () => void; setCreatedProjectSlug: (slug?: string) => void; setSelectedFeatures: (features?: ProductSolution[]) => void; setSelectedIntegration: (integration?: Integration) => void; @@ -42,6 +43,7 @@ const OnboardingContext = createContext({ createdProjectSlug: undefined, setCreatedProjectSlug: () => {}, clearDerivedState: () => {}, + resetOnboarding: () => {}, }); type ProviderProps = { @@ -85,11 +87,7 @@ export function OnboardingContextProvider({children, initialValue}: ProviderProp () => ({ selectedPlatform: onboarding?.selectedPlatform, setSelectedPlatform: (selectedPlatform?: OnboardingSelectedSDK) => { - if (selectedPlatform === undefined) { - removeOnboarding(); - } else { - setOnboarding(prev => ({...prev, selectedPlatform})); - } + setOnboarding(prev => ({...prev, selectedPlatform})); }, selectedIntegration: onboarding?.selectedIntegration, setSelectedIntegration: (selectedIntegration?: Integration) => { @@ -118,6 +116,11 @@ export function OnboardingContextProvider({children, initialValue}: ProviderProp createdProjectSlug: undefined, })); }, + // Full-flow exits should clear every staged choice explicitly. Do not + // reach for a selected-platform reset to do this: clearing one field must + // stay local to that field so organization-scoped state added later + // survives local repository and platform changes. + resetOnboarding: removeOnboarding, }), [onboarding, setOnboarding, removeOnboarding] ); diff --git a/static/app/views/onboarding/components/onboardingSkipButton.tsx b/static/app/views/onboarding/components/onboardingSkipButton.tsx index 9b13e59ae4cf..85680f380dec 100644 --- a/static/app/views/onboarding/components/onboardingSkipButton.tsx +++ b/static/app/views/onboarding/components/onboardingSkipButton.tsx @@ -1,5 +1,6 @@ import {LinkButton} from '@sentry/scraps/button'; +import {useOnboardingContext} from 'sentry/components/onboarding/onboardingContext'; import {useOnboardingSidebar} from 'sentry/components/onboarding/useOnboardingSidebar'; import {t} from 'sentry/locale'; import {trackAnalytics} from 'sentry/utils/analytics'; @@ -39,6 +40,7 @@ interface OnboardingSkipButtonProps { export function OnboardingSkipButton({stepId}: OnboardingSkipButtonProps) { const organization = useOrganization(); + const {resetOnboarding} = useOnboardingContext(); const {activateSidebar} = useOnboardingSidebar(); const config = SKIP_CONFIG_BY_STEP[stepId]; @@ -47,6 +49,7 @@ export function OnboardingSkipButton({stepId}: OnboardingSkipButtonProps) { } const handleClick = () => { + resetOnboarding(); trackAnalytics('onboarding.scm_header_skip_clicked', { organization, step: stepId, diff --git a/static/app/views/onboarding/onboarding.tsx b/static/app/views/onboarding/onboarding.tsx index 15b8cd7446cd..e795cf1662fd 100644 --- a/static/app/views/onboarding/onboarding.tsx +++ b/static/app/views/onboarding/onboarding.tsx @@ -370,7 +370,7 @@ export function OnboardingWithoutContext() { organization, source, }); - onboardingContext.setSelectedPlatform(undefined); + onboardingContext.resetOnboarding(); activateSidebar({ userClicked: false, source: 'targeted_onboarding_select_platform_skip', diff --git a/static/app/views/onboarding/useBackActions.tsx b/static/app/views/onboarding/useBackActions.tsx index 869bcfa27311..559f38257da8 100644 --- a/static/app/views/onboarding/useBackActions.tsx +++ b/static/app/views/onboarding/useBackActions.tsx @@ -49,7 +49,7 @@ export function useBackActions({ if (preserveOnboardingState) { onboardingContext.setCreatedProjectSlug(undefined); } else { - onboardingContext.setSelectedPlatform(undefined); + onboardingContext.resetOnboarding(); } try { @@ -106,7 +106,7 @@ export function useBackActions({ // from selected platform to welcome if (currentStep.id === 'select-platform') { - onboardingContext.setSelectedPlatform(undefined); + onboardingContext.resetOnboarding(); if (!browserBackButton) { goToStep(prevStep); diff --git a/static/app/views/onboarding/useConfigureSdk.tsx b/static/app/views/onboarding/useConfigureSdk.tsx index e96977a6c95d..9e9f8a38d239 100644 --- a/static/app/views/onboarding/useConfigureSdk.tsx +++ b/static/app/views/onboarding/useConfigureSdk.tsx @@ -146,7 +146,7 @@ export function useConfigureSdk({ platform: selectedPlatform.key, organization, }); - onboardingContext.setSelectedPlatform(undefined); + onboardingContext.resetOnboarding(); }, } ); diff --git a/static/app/views/onboarding/useWelcomeAnalyticsEffect.ts b/static/app/views/onboarding/useWelcomeAnalyticsEffect.ts index f6d772e09ae5..7d8a43d11251 100644 --- a/static/app/views/onboarding/useWelcomeAnalyticsEffect.ts +++ b/static/app/views/onboarding/useWelcomeAnalyticsEffect.ts @@ -26,7 +26,11 @@ export function useWelcomeAnalyticsEffect() { if (onboardingContext.selectedPlatform) { // At this point the selectedSDK shall be undefined but just in case, cleaning this up here too - onboardingContext.setSelectedPlatform(undefined); + if (hasScmOnboarding) { + onboardingContext.clearDerivedState(); + } else { + onboardingContext.resetOnboarding(); + } } }, [organization, onboardingContext, hasScmOnboarding]); }