From 7a697e613ce09967fceb96b1d4f6d09666086f14 Mon Sep 17 00:00:00 2001 From: Jay Goss Date: Tue, 14 Apr 2026 12:28:50 -0500 Subject: [PATCH 1/2] ref(onboarding): Decouple SCM step components from OnboardingContext Extract useOnboardingContext() calls from ScmConnect, ScmPlatformFeatures, ScmProjectDetails, and ScmRepoSelector so they accept all shared state via props. Adapter wrappers in onboarding.tsx read from context and pass props to the decoupled components. This enables reusing the SCM step components in the upcoming project creation flow, which will provide state from local wizard state instead of OnboardingContext. Refs VDY-72 --- .../components/scmRepoSelector.spec.tsx | 79 ++--- .../onboarding/components/scmRepoSelector.tsx | 21 +- static/app/views/onboarding/onboarding.tsx | 74 ++++- static/app/views/onboarding/scmConnect.tsx | 40 ++- .../onboarding/scmPlatformFeatures.spec.tsx | 300 +++++------------- .../views/onboarding/scmPlatformFeatures.tsx | 54 ++-- .../onboarding/scmProjectDetails.spec.tsx | 213 ++----------- .../views/onboarding/scmProjectDetails.tsx | 25 +- 8 files changed, 293 insertions(+), 513 deletions(-) diff --git a/static/app/views/onboarding/components/scmRepoSelector.spec.tsx b/static/app/views/onboarding/components/scmRepoSelector.spec.tsx index b31fb60d67205c..27f7634b07b7dc 100644 --- a/static/app/views/onboarding/components/scmRepoSelector.spec.tsx +++ b/static/app/views/onboarding/components/scmRepoSelector.spec.tsx @@ -4,10 +4,7 @@ import {RepositoryFixture} from 'sentry-fixture/repository'; import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; -import { - OnboardingContextProvider, - type OnboardingSessionState, -} from 'sentry/components/onboarding/onboardingContext'; +import type {Repository} from 'sentry/types/integrations'; import {ScmRepoSelector} from './scmRepoSelector'; @@ -26,16 +23,6 @@ jest.mock('@tanstack/react-virtual', () => ({ })), })); -function makeOnboardingWrapper(initialState?: OnboardingSessionState) { - return function OnboardingWrapper({children}: {children?: React.ReactNode}) { - return ( - - {children} - - ); - }; -} - describe('ScmRepoSelector', () => { const organization = OrganizationFixture(); @@ -54,21 +41,34 @@ describe('ScmRepoSelector', () => { }, }); + let onRepositoryChange: jest.Mock; + + beforeEach(() => { + onRepositoryChange = jest.fn(); + }); + afterEach(() => { MockApiClient.clearMockResponses(); - sessionStorage.clear(); }); + function renderSelector(selectedRepository?: Repository) { + return render( + , + {organization} + ); + } + it('renders search placeholder', () => { MockApiClient.addMockResponse({ url: `/organizations/${organization.slug}/integrations/${mockIntegration.id}/repos/`, body: {repos: []}, }); - render(, { - organization, - wrapper: makeOnboardingWrapper(), - }); + renderSelector(); expect(screen.getByText('Search repositories')).toBeInTheDocument(); }); @@ -79,10 +79,7 @@ describe('ScmRepoSelector', () => { body: {repos: []}, }); - render(, { - organization, - wrapper: makeOnboardingWrapper(), - }); + renderSelector(); await userEvent.click(screen.getByRole('textbox')); @@ -100,10 +97,7 @@ describe('ScmRepoSelector', () => { body: {detail: 'Internal Error'}, }); - render(, { - organization, - wrapper: makeOnboardingWrapper(), - }); + renderSelector(); await userEvent.click(screen.getByRole('textbox')); @@ -123,10 +117,7 @@ describe('ScmRepoSelector', () => { }, }); - render(, { - organization, - wrapper: makeOnboardingWrapper(), - }); + renderSelector(); await userEvent.click(screen.getByRole('textbox')); @@ -136,7 +127,7 @@ describe('ScmRepoSelector', () => { expect(screen.getByRole('menuitemradio', {name: 'relay'})).toBeInTheDocument(); }); - it('shows selected repo value when one is in context', () => { + it('shows selected repo value when one is provided via props', () => { MockApiClient.addMockResponse({ url: `/organizations/${organization.slug}/integrations/${mockIntegration.id}/repos/`, body: {repos: []}, @@ -147,10 +138,7 @@ describe('ScmRepoSelector', () => { externalSlug: 'getsentry/old-repo', }); - render(, { - organization, - wrapper: makeOnboardingWrapper({selectedRepository: selectedRepo}), - }); + renderSelector(selectedRepo); expect(screen.getByText('getsentry/old-repo')).toBeInTheDocument(); }); @@ -173,10 +161,7 @@ describe('ScmRepoSelector', () => { ], }); - render(, { - organization, - wrapper: makeOnboardingWrapper(), - }); + renderSelector(); await userEvent.click(screen.getByRole('textbox')); await userEvent.click(await screen.findByRole('menuitemradio', {name: 'sentry'})); @@ -184,7 +169,7 @@ describe('ScmRepoSelector', () => { await waitFor(() => expect(reposLookup).toHaveBeenCalled()); }); - it('clears the selected repo', async () => { + it('calls onRepositoryChange with undefined when clearing', async () => { MockApiClient.addMockResponse({ url: `/organizations/${organization.slug}/integrations/${mockIntegration.id}/repos/`, body: {repos: []}, @@ -195,17 +180,14 @@ describe('ScmRepoSelector', () => { externalSlug: 'getsentry/old-repo', }); - render(, { - organization, - wrapper: makeOnboardingWrapper({selectedRepository: selectedRepo}), - }); + renderSelector(selectedRepo); expect(screen.getByText('getsentry/old-repo')).toBeInTheDocument(); await userEvent.click(await screen.findByTestId('icon-close')); await waitFor(() => { - expect(screen.queryByText('getsentry/old-repo')).not.toBeInTheDocument(); + expect(onRepositoryChange).toHaveBeenCalledWith(undefined); }); }); @@ -225,10 +207,7 @@ describe('ScmRepoSelector', () => { }, }); - render(, { - organization, - wrapper: makeOnboardingWrapper({selectedRepository: selectedRepo}), - }); + renderSelector(selectedRepo); await userEvent.click(screen.getByRole('textbox')); diff --git a/static/app/views/onboarding/components/scmRepoSelector.tsx b/static/app/views/onboarding/components/scmRepoSelector.tsx index 9d35f370dd5426..54c9b9839353d3 100644 --- a/static/app/views/onboarding/components/scmRepoSelector.tsx +++ b/static/app/views/onboarding/components/scmRepoSelector.tsx @@ -2,9 +2,8 @@ import {useMemo} from 'react'; import {Select} from '@sentry/scraps/select'; -import {useOnboardingContext} from 'sentry/components/onboarding/onboardingContext'; import {t} from 'sentry/locale'; -import type {Integration} from 'sentry/types/integrations'; +import type {Integration, Repository} from 'sentry/types/integrations'; import {trackAnalytics} from 'sentry/utils/analytics'; import {useOrganization} from 'sentry/utils/useOrganization'; @@ -13,14 +12,18 @@ import {ScmVirtualizedMenuList} from './scmVirtualizedMenuList'; import {useScmRepos} from './useScmRepos'; import {useScmRepoSelection} from './useScmRepoSelection'; -interface ScmRepoSelectorProps { +export interface ScmRepoSelectorProps { integration: Integration; + onRepositoryChange: (repo: Repository | undefined) => void; + selectedRepository: Repository | undefined; } -export function ScmRepoSelector({integration}: ScmRepoSelectorProps) { +export function ScmRepoSelector({ + integration, + onRepositoryChange, + selectedRepository, +}: ScmRepoSelectorProps) { const organization = useOrganization(); - const {selectedRepository, setSelectedRepository, clearDerivedState} = - useOnboardingContext(); const {reposByIdentifier, dropdownItems, isFetching, isError} = useScmRepos( integration.id, selectedRepository @@ -28,7 +31,7 @@ export function ScmRepoSelector({integration}: ScmRepoSelectorProps) { const {busy, handleSelect, handleRemove} = useScmRepoSelection({ integration, - onSelect: setSelectedRepository, + onSelect: onRepositoryChange, reposByIdentifier, }); @@ -50,10 +53,6 @@ export function ScmRepoSelector({integration}: ScmRepoSelectorProps) { }, [dropdownItems, selectedRepository]); function handleChange(option: {value: string} | null) { - // Changing or clearing the repo invalidates downstream state (platform, - // features, created project) which are all derived from the selected repo. - clearDerivedState(); - if (option === null) { handleRemove(); } else { diff --git a/static/app/views/onboarding/onboarding.tsx b/static/app/views/onboarding/onboarding.tsx index a535f6b3e3db46..9a9e2825c5a9bc 100644 --- a/static/app/views/onboarding/onboarding.tsx +++ b/static/app/views/onboarding/onboarding.tsx @@ -19,6 +19,7 @@ import {categoryList} from 'sentry/data/platformPickerCategories'; import {allPlatforms as platforms} from 'sentry/data/platforms'; import {IconArrow} from 'sentry/icons'; import {t} from 'sentry/locale'; +import type {Repository} from 'sentry/types/integrations'; import type {OnboardingSelectedSDK} from 'sentry/types/onboarding'; import type {PlatformKey} from 'sentry/types/project'; import {defined} from 'sentry/utils'; @@ -67,6 +68,73 @@ const legacyOnboardingSteps: StepDescriptor[] = [ }, ]; +// Adapter wrappers that read from OnboardingContext and pass props to the +// decoupled SCM step components. This keeps the step components reusable +// across onboarding and the future project creation flow. + +function ScmConnectAdapter({onComplete}: StepProps) { + const { + selectedIntegration, + setSelectedIntegration, + selectedRepository, + setSelectedRepository, + clearDerivedState, + } = useOnboardingContext(); + + const handleRepositoryChange = useCallback( + (repo: Repository | undefined) => { + clearDerivedState(); + setSelectedRepository(repo); + }, + [clearDerivedState, setSelectedRepository] + ); + + return ( + + ); +} + +function ScmPlatformFeaturesAdapter({onComplete}: StepProps) { + const { + selectedRepository, + selectedPlatform, + setSelectedPlatform, + selectedFeatures, + setSelectedFeatures, + } = useOnboardingContext(); + + return ( + + ); +} + +function ScmProjectDetailsAdapter({onComplete}: StepProps) { + const {selectedPlatform, selectedFeatures, setCreatedProjectSlug} = + useOnboardingContext(); + + return ( + + ); +} + const scmOnboardingSteps: StepDescriptor[] = [ { id: OnboardingStepId.WELCOME, @@ -77,19 +145,19 @@ const scmOnboardingSteps: StepDescriptor[] = [ { id: OnboardingStepId.SCM_CONNECT, title: t('Connect repository'), - Component: ScmConnect, + Component: ScmConnectAdapter, cornerVariant: 'top-left', }, { id: OnboardingStepId.SCM_PLATFORM_FEATURES, title: t('Platform & features'), - Component: ScmPlatformFeatures, + Component: ScmPlatformFeaturesAdapter, cornerVariant: 'top-left', }, { id: OnboardingStepId.SCM_PROJECT_DETAILS, title: t('Project details'), - Component: ScmProjectDetails, + Component: ScmProjectDetailsAdapter, cornerVariant: 'top-left', }, { diff --git a/static/app/views/onboarding/scmConnect.tsx b/static/app/views/onboarding/scmConnect.tsx index 07d30748c50812..2376d7cd47518a 100644 --- a/static/app/views/onboarding/scmConnect.tsx +++ b/static/app/views/onboarding/scmConnect.tsx @@ -7,10 +7,9 @@ import {Container, Flex, Stack} from '@sentry/scraps/layout'; import {Text} from '@sentry/scraps/text'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; -import {useOnboardingContext} from 'sentry/components/onboarding/onboardingContext'; import {IconCheckmark} from 'sentry/icons'; import {t} from 'sentry/locale'; -import type {Integration} from 'sentry/types/integrations'; +import type {Integration, Repository} from 'sentry/types/integrations'; import {trackAnalytics} from 'sentry/utils/analytics'; import {useOrganization} from 'sentry/utils/useOrganization'; @@ -22,16 +21,23 @@ import {ScmStepHeader} from './components/scmStepHeader'; import {useScmPlatformDetection} from './components/useScmPlatformDetection'; import {useScmProviders} from './components/useScmProviders'; import {SCM_STEP_CONTENT_WIDTH} from './consts'; -import type {StepProps} from './types'; -export function ScmConnect({onComplete}: StepProps) { +export interface ScmConnectProps { + onComplete: () => void; + onIntegrationChange: (integration: Integration | undefined) => void; + onRepositoryChange: (repo: Repository | undefined) => void; + selectedIntegration: Integration | undefined; + selectedRepository: Repository | undefined; +} + +export function ScmConnect({ + onComplete, + onIntegrationChange, + onRepositoryChange, + selectedIntegration, + selectedRepository, +}: ScmConnectProps) { const organization = useOrganization(); - const { - selectedIntegration, - setSelectedIntegration, - selectedRepository, - setSelectedRepository, - } = useOnboardingContext(); const { scmProviders, isPending, @@ -53,11 +59,11 @@ export function ScmConnect({onComplete}: StepProps) { const handleInstall = useCallback( (data: Integration) => { - setSelectedIntegration(data); - setSelectedRepository(undefined); + onIntegrationChange(data); + onRepositoryChange(undefined); refetchIntegrations(); }, - [setSelectedIntegration, setSelectedRepository, refetchIntegrations] + [onIntegrationChange, onRepositoryChange, refetchIntegrations] ); return ( @@ -93,7 +99,11 @@ export function ScmConnect({onComplete}: StepProps) { )} - + {selectedRepository ? ( { if (effectiveIntegration && !selectedIntegration) { - setSelectedIntegration(effectiveIntegration); + onIntegrationChange(effectiveIntegration); } onComplete(); }} diff --git a/static/app/views/onboarding/scmPlatformFeatures.spec.tsx b/static/app/views/onboarding/scmPlatformFeatures.spec.tsx index 2d11e246cdc512..1586fdb1a4eeeb 100644 --- a/static/app/views/onboarding/scmPlatformFeatures.spec.tsx +++ b/static/app/views/onboarding/scmPlatformFeatures.spec.tsx @@ -6,12 +6,9 @@ import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrar import {openConsoleModal, openModal} from 'sentry/actionCreators/modal'; import {ProductSolution} from 'sentry/components/onboarding/gettingStartedDoc/types'; -import { - OnboardingContextProvider, - type OnboardingSessionState, -} from 'sentry/components/onboarding/onboardingContext'; +import type {Repository} from 'sentry/types/integrations'; +import type {OnboardingSelectedSDK} from 'sentry/types/onboarding'; import * as analytics from 'sentry/utils/analytics'; -import {sessionStorageWrapper} from 'sentry/utils/sessionStorage'; import {ScmPlatformFeatures} from './scmPlatformFeatures'; @@ -49,29 +46,37 @@ jest.mock('sentry/data/platforms', () => { }; }); -function makeOnboardingWrapper(initialState?: OnboardingSessionState) { - return function OnboardingWrapper({children}: {children?: React.ReactNode}) { - return ( - - {children} - - ); - }; -} - const mockRepository = RepositoryFixture({ id: '42', provider: {id: 'integrations:github', name: 'GitHub'}, }); +const defaultProps = { + onComplete: jest.fn(), + onPlatformChange: jest.fn(), + onFeaturesChange: jest.fn(), + selectedPlatform: undefined as OnboardingSelectedSDK | undefined, + selectedFeatures: undefined as ProductSolution[] | undefined, + selectedRepository: undefined as Repository | undefined, +}; + describe('ScmPlatformFeatures', () => { const organization = OrganizationFixture({ features: ['performance-view', 'session-replay', 'profiling-view'], }); + function renderComponent( + overrides?: Partial, + orgOverride?: Parameters[1]['organization'] + ) { + const props = {...defaultProps, ...overrides}; + return render(, { + organization: orgOverride ?? organization, + }); + } + beforeEach(() => { jest.clearAllMocks(); - sessionStorageWrapper.clear(); }); afterEach(() => { @@ -93,19 +98,7 @@ describe('ScmPlatformFeatures', () => { }, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - }), - } - ); + renderComponent({selectedRepository: mockRepository}); expect(await screen.findByText('Next.js')).toBeInTheDocument(); expect(screen.getByText('Django')).toBeInTheDocument(); @@ -126,19 +119,7 @@ describe('ScmPlatformFeatures', () => { }, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - }), - } - ); + renderComponent({selectedRepository: mockRepository}); expect(await screen.findByText('What do you want to set up?')).toBeInTheDocument(); }); @@ -158,19 +139,7 @@ describe('ScmPlatformFeatures', () => { }, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - }), - } - ); + renderComponent({selectedRepository: mockRepository}); const changeButton = await screen.findByRole('button', { name: "Doesn't look right? Change platform", @@ -187,19 +156,7 @@ describe('ScmPlatformFeatures', () => { body: {detail: 'Internal Error'}, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - }), - } - ); + renderComponent({selectedRepository: mockRepository}); expect( await screen.findByRole('heading', {name: 'Select a platform'}) @@ -208,17 +165,7 @@ describe('ScmPlatformFeatures', () => { }); it('renders manual picker when no repository in context', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper(), - } - ); + renderComponent(); expect( await screen.findByRole('heading', {name: 'Select a platform'}) @@ -227,17 +174,7 @@ describe('ScmPlatformFeatures', () => { }); it('continue button is disabled when no platform selected', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper(), - } - ); + renderComponent(); // Wait for the component to fully settle (CompactSelect triggers async popper updates) await screen.findByRole('heading', {name: 'Select a platform'}); @@ -253,19 +190,7 @@ describe('ScmPlatformFeatures', () => { }, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - }), - } - ); + renderComponent({selectedRepository: mockRepository}); // Wait for auto-select of first detected platform await waitFor(() => { @@ -284,20 +209,13 @@ describe('ScmPlatformFeatures', () => { body: {platforms: [pythonPlatform]}, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - selectedFeatures: [ProductSolution.ERROR_MONITORING], - }), - } - ); + const onFeaturesChange = jest.fn(); + + renderComponent({ + selectedRepository: mockRepository, + selectedFeatures: [ProductSolution.ERROR_MONITORING], + onFeaturesChange, + }); // Wait for feature cards to appear await screen.findByText('What do you want to set up?'); @@ -309,24 +227,20 @@ describe('ScmPlatformFeatures', () => { // Enable profiling — tracing should auto-enable await userEvent.click(screen.getByRole('checkbox', {name: /Profiling/})); - expect(screen.getByRole('checkbox', {name: /Profiling/})).toBeChecked(); - expect(screen.getByRole('checkbox', {name: /Tracing/})).toBeChecked(); + // The component calls onFeaturesChange with both profiling and tracing enabled + expect(onFeaturesChange).toHaveBeenCalledWith( + expect.arrayContaining([ + ProductSolution.ERROR_MONITORING, + ProductSolution.PROFILING, + ProductSolution.PERFORMANCE_MONITORING, + ]) + ); }); it('shows framework suggestion modal when selecting a base language', async () => { const mockOpenModal = openModal as jest.Mock; - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper(), - } - ); + renderComponent(); await screen.findByRole('heading', {name: 'Select a platform'}); @@ -342,19 +256,12 @@ describe('ScmPlatformFeatures', () => { it('opens console modal when selecting a disabled gaming platform', async () => { const mockOpenConsoleModal = openConsoleModal as jest.Mock; - render( - null} - />, - { - // No enabledConsolePlatforms — all console platforms are blocked - organization: OrganizationFixture({ - features: ['performance-view', 'session-replay', 'profiling-view'], - }), - additionalWrapper: makeOnboardingWrapper(), - } + renderComponent( + {}, + // No enabledConsolePlatforms — all console platforms are blocked + OrganizationFixture({ + features: ['performance-view', 'session-replay', 'profiling-view'], + }) ); await screen.findByRole('heading', {name: 'Select a platform'}); @@ -379,32 +286,25 @@ describe('ScmPlatformFeatures', () => { body: {platforms: [pythonPlatform]}, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - selectedPlatform: { - key: 'python', - name: 'Python', - language: 'python', - type: 'language', - link: 'https://docs.sentry.io/platforms/python/', - category: 'popular', - }, - selectedFeatures: [ - ProductSolution.ERROR_MONITORING, - ProductSolution.PERFORMANCE_MONITORING, - ProductSolution.PROFILING, - ], - }), - } - ); + const onFeaturesChange = jest.fn(); + + renderComponent({ + selectedRepository: mockRepository, + selectedPlatform: { + key: 'python', + name: 'Python', + language: 'python', + type: 'language', + link: 'https://docs.sentry.io/platforms/python/', + category: 'popular', + }, + selectedFeatures: [ + ProductSolution.ERROR_MONITORING, + ProductSolution.PERFORMANCE_MONITORING, + ProductSolution.PROFILING, + ], + onFeaturesChange, + }); // Wait for feature cards to appear await screen.findByText('What do you want to set up?'); @@ -416,8 +316,8 @@ describe('ScmPlatformFeatures', () => { // Disable tracing — profiling should auto-disable await userEvent.click(screen.getByRole('checkbox', {name: /Tracing/})); - expect(screen.getByRole('checkbox', {name: /Tracing/})).not.toBeChecked(); - expect(screen.getByRole('checkbox', {name: /Profiling/})).not.toBeChecked(); + // The component calls onFeaturesChange with both tracing and profiling removed + expect(onFeaturesChange).toHaveBeenCalledWith([ProductSolution.ERROR_MONITORING]); }); describe('analytics', () => { @@ -428,17 +328,7 @@ describe('ScmPlatformFeatures', () => { }); it('fires step viewed event on mount', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper(), - } - ); + renderComponent(); await screen.findByRole('heading', {name: 'Select a platform'}); @@ -463,19 +353,7 @@ describe('ScmPlatformFeatures', () => { }, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - }), - } - ); + renderComponent({selectedRepository: mockRepository}); // Wait for detected platforms, then click the second one const djangoCard = await screen.findByRole('radio', {name: /Django/}); @@ -498,20 +376,10 @@ describe('ScmPlatformFeatures', () => { }, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - selectedFeatures: [ProductSolution.ERROR_MONITORING], - }), - } - ); + renderComponent({ + selectedRepository: mockRepository, + selectedFeatures: [ProductSolution.ERROR_MONITORING], + }); await screen.findByText('What do you want to set up?'); @@ -533,19 +401,7 @@ describe('ScmPlatformFeatures', () => { body: {platforms: [DetectedPlatformFixture()]}, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedRepository: mockRepository, - }), - } - ); + renderComponent({selectedRepository: mockRepository}); const changeButton = await screen.findByRole('button', { name: "Doesn't look right? Change platform", diff --git a/static/app/views/onboarding/scmPlatformFeatures.tsx b/static/app/views/onboarding/scmPlatformFeatures.tsx index 1ff89a1926c614..2992cc50e2f511 100644 --- a/static/app/views/onboarding/scmPlatformFeatures.tsx +++ b/static/app/views/onboarding/scmPlatformFeatures.tsx @@ -11,13 +11,13 @@ import {closeModal, openConsoleModal, openModal} from 'sentry/actionCreators/mod import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {SupportedLanguages} from 'sentry/components/onboarding/frameworkSuggestionModal'; import {ProductSolution} from 'sentry/components/onboarding/gettingStartedDoc/types'; -import {useOnboardingContext} from 'sentry/components/onboarding/onboardingContext'; import { getDisabledProducts, platformProductAvailability, } from 'sentry/components/onboarding/productSelection'; import {platforms} from 'sentry/data/platforms'; import {t} from 'sentry/locale'; +import type {Repository} from 'sentry/types/integrations'; import type {OnboardingSelectedSDK} from 'sentry/types/onboarding'; import type {PlatformIntegration, PlatformKey} from 'sentry/types/project'; import {trackAnalytics} from 'sentry/utils/analytics'; @@ -34,7 +34,6 @@ import { useScmPlatformDetection, type DetectedPlatform, } from './components/useScmPlatformDetection'; -import type {StepProps} from './types'; interface ResolvedPlatform extends DetectedPlatform { info: PlatformIntegration; @@ -76,15 +75,24 @@ function shouldSuggestFramework(platformKey: PlatformKey): boolean { // Wider than SCM_STEP_CONTENT_WIDTH (506px) used by the footer. const PLATFORM_CONTENT_WIDTH = '564px'; -export function ScmPlatformFeatures({onComplete}: StepProps) { +export interface ScmPlatformFeaturesProps { + onComplete: () => void; + onFeaturesChange: (features: ProductSolution[]) => void; + onPlatformChange: (platform: OnboardingSelectedSDK | undefined) => void; + selectedFeatures: ProductSolution[] | undefined; + selectedPlatform: OnboardingSelectedSDK | undefined; + selectedRepository: Repository | undefined; +} + +export function ScmPlatformFeatures({ + onComplete, + onFeaturesChange, + onPlatformChange, + selectedFeatures, + selectedPlatform, + selectedRepository, +}: ScmPlatformFeaturesProps) { const organization = useOrganization(); - const { - selectedRepository, - selectedPlatform, - setSelectedPlatform, - selectedFeatures, - setSelectedFeatures, - } = useOnboardingContext(); const [showManualPicker, setShowManualPicker] = useState(false); @@ -96,10 +104,10 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { (platformKey: PlatformKey) => { const info = getPlatformInfo(platformKey); if (info) { - setSelectedPlatform(toSelectedSdk(info)); + onPlatformChange(toSelectedSdk(info)); } }, - [setSelectedPlatform] + [onPlatformChange] ); const hasScmConnected = !!selectedRepository; @@ -178,7 +186,7 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { } } - setSelectedFeatures(Array.from(newFeatures)); + onFeaturesChange(Array.from(newFeatures)); trackAnalytics('onboarding.scm_platform_feature_toggled', { organization, @@ -189,7 +197,7 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { }, [ currentFeatures, - setSelectedFeatures, + onFeaturesChange, disabledProducts, availableFeatures, organization, @@ -199,10 +207,10 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { const applyPlatformSelection = useCallback( (sdk: OnboardingSelectedSDK) => { - setSelectedPlatform(sdk); - setSelectedFeatures([ProductSolution.ERROR_MONITORING]); + onPlatformChange(sdk); + onFeaturesChange([ProductSolution.ERROR_MONITORING]); }, - [setSelectedPlatform, setSelectedFeatures] + [onPlatformChange, onFeaturesChange] ); const handleManualPlatformSelect = useCallback( @@ -270,7 +278,7 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { } setPlatform(platformKey); - setSelectedFeatures([ProductSolution.ERROR_MONITORING]); + onFeaturesChange([ProductSolution.ERROR_MONITORING]); trackAnalytics('onboarding.scm_platform_selected', { organization, @@ -281,7 +289,7 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { [ selectedPlatform?.key, setPlatform, - setSelectedFeatures, + onFeaturesChange, applyPlatformSelection, organization, ] @@ -293,7 +301,7 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { return; } setPlatform(platformKey); - setSelectedFeatures([ProductSolution.ERROR_MONITORING]); + onFeaturesChange([ProductSolution.ERROR_MONITORING]); trackAnalytics('onboarding.scm_platform_selected', { organization, @@ -301,7 +309,7 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { source: 'detected', }); }, - [selectedPlatform?.key, setPlatform, setSelectedFeatures, organization] + [selectedPlatform?.key, setPlatform, onFeaturesChange, organization] ); function handleChangePlatformClick() { @@ -317,7 +325,7 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { setShowManualPicker(false); if (detectedPlatformKey) { setPlatform(detectedPlatformKey); - setSelectedFeatures([ProductSolution.ERROR_MONITORING]); + onFeaturesChange([ProductSolution.ERROR_MONITORING]); } } @@ -327,7 +335,7 @@ export function ScmPlatformFeatures({onComplete}: StepProps) { setPlatform(currentPlatformKey); } if (!selectedFeatures) { - setSelectedFeatures(currentFeatures); + onFeaturesChange(currentFeatures); } onComplete(); } diff --git a/static/app/views/onboarding/scmProjectDetails.spec.tsx b/static/app/views/onboarding/scmProjectDetails.spec.tsx index c0a38f35ea065a..e0ddc6945c42bf 100644 --- a/static/app/views/onboarding/scmProjectDetails.spec.tsx +++ b/static/app/views/onboarding/scmProjectDetails.spec.tsx @@ -1,31 +1,16 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; import {ProjectFixture} from 'sentry-fixture/project'; -import {RepositoryFixture} from 'sentry-fixture/repository'; import {TeamFixture} from 'sentry-fixture/team'; import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary'; -import { - OnboardingContextProvider, - type OnboardingSessionState, -} from 'sentry/components/onboarding/onboardingContext'; +import type {ProductSolution} from 'sentry/components/onboarding/gettingStartedDoc/types'; import {TeamStore} from 'sentry/stores/teamStore'; import type {OnboardingSelectedSDK} from 'sentry/types/onboarding'; import * as analytics from 'sentry/utils/analytics'; -import {sessionStorageWrapper} from 'sentry/utils/sessionStorage'; import {ScmProjectDetails} from './scmProjectDetails'; -function makeOnboardingWrapper(initialState?: OnboardingSessionState) { - return function OnboardingWrapper({children}: {children?: React.ReactNode}) { - return ( - - {children} - - ); - }; -} - const mockPlatform: OnboardingSelectedSDK = { key: 'javascript-nextjs', name: 'Next.js', @@ -35,14 +20,23 @@ const mockPlatform: OnboardingSelectedSDK = { type: 'framework', }; -const mockRepository = RepositoryFixture({id: '42', name: 'getsentry/sentry'}); - describe('ScmProjectDetails', () => { const organization = OrganizationFixture(); const teamWithAccess = TeamFixture({slug: 'my-team', access: ['team:admin']}); + const defaultProps = { + onComplete: jest.fn(), + onProjectCreated: jest.fn(), + selectedPlatform: mockPlatform as OnboardingSelectedSDK | undefined, + selectedFeatures: undefined as ProductSolution[] | undefined, + }; + + function renderComponent(overrides?: Partial) { + const props = {...defaultProps, ...overrides}; + return render(, {organization}); + } + beforeEach(() => { - sessionStorageWrapper.clear(); TeamStore.loadInitialData([teamWithAccess]); // useCreateNotificationAction queries messaging integrations on mount @@ -64,37 +58,13 @@ describe('ScmProjectDetails', () => { }); it('renders step header with heading', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent(); expect(await screen.findByText('Project details')).toBeInTheDocument(); }); it('renders section headers with icons', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent(); expect(await screen.findByText('Give your project a name')).toBeInTheDocument(); expect(screen.getByText('Assign a team')).toBeInTheDocument(); @@ -103,76 +73,29 @@ describe('ScmProjectDetails', () => { }); it('renders project name defaulted from platform key', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent(); const input = await screen.findByPlaceholderText('project-name'); expect(input).toHaveValue('javascript-nextjs'); }); - it('uses platform key as default name even when repository is in context', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - selectedRepository: mockRepository, - }), - } - ); + it('uses platform key as default name even when repository was connected', async () => { + renderComponent(); const input = await screen.findByPlaceholderText('project-name'); expect(input).toHaveValue('javascript-nextjs'); }); it('renders card-style alert frequency options', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent(); expect(await screen.findByText('High priority issues')).toBeInTheDocument(); expect(screen.getByText('Custom')).toBeInTheDocument(); expect(screen.getByText("I'll create my own alerts later")).toBeInTheDocument(); }); - it('create project button is disabled without platform in context', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper(), - } - ); + it('create project button is disabled without platform', async () => { + renderComponent({selectedPlatform: undefined}); expect(await screen.findByRole('button', {name: 'Create project'})).toBeDisabled(); }); @@ -200,19 +123,7 @@ describe('ScmProjectDetails', () => { body: [teamWithAccess], }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent({onComplete}); const createButton = await screen.findByRole('button', {name: 'Create project'}); await userEvent.click(createButton); @@ -227,25 +138,13 @@ describe('ScmProjectDetails', () => { }); it('defaults team selector to first admin team', async () => { - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent(); // TeamSelector renders the team slug as the selected value expect(await screen.findByText(`#${teamWithAccess.slug}`)).toBeInTheDocument(); }); - it('updates context with project slug after creation', async () => { + it('calls onProjectCreated with project slug after creation', async () => { const createdProject = ProjectFixture({ slug: 'my-custom-project', name: 'my-custom-project', @@ -270,20 +169,9 @@ describe('ScmProjectDetails', () => { }); const onComplete = jest.fn(); + const onProjectCreated = jest.fn(); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent({onComplete, onProjectCreated}); await userEvent.click(await screen.findByRole('button', {name: 'Create project'})); @@ -291,12 +179,7 @@ describe('ScmProjectDetails', () => { expect(onComplete).toHaveBeenCalled(); }); - // Verify the project slug was stored separately in context (not overwriting - // selectedPlatform.key) so onboarding.tsx can find the project via - // useRecentCreatedProject while preserving the original platform selection. - const stored = JSON.parse(sessionStorageWrapper.getItem('onboarding') ?? '{}'); - expect(stored.createdProjectSlug).toBe('my-custom-project'); - expect(stored.selectedPlatform?.key).toBe('javascript-nextjs'); + expect(onProjectCreated).toHaveBeenCalledWith('my-custom-project'); }); it('shows error message on project creation failure', async () => { @@ -309,19 +192,7 @@ describe('ScmProjectDetails', () => { body: {detail: 'Internal Error'}, }); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent({onComplete}); const createButton = await screen.findByRole('button', {name: 'Create project'}); await userEvent.click(createButton); @@ -334,19 +205,7 @@ describe('ScmProjectDetails', () => { it('fires step viewed analytics on mount', async () => { const trackAnalyticsSpy = jest.spyOn(analytics, 'trackAnalytics'); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent(); await screen.findByText('Project details'); @@ -379,19 +238,7 @@ describe('ScmProjectDetails', () => { const onComplete = jest.fn(); - render( - null} - />, - { - organization, - additionalWrapper: makeOnboardingWrapper({ - selectedPlatform: mockPlatform, - }), - } - ); + renderComponent({onComplete}); await userEvent.click(await screen.findByRole('button', {name: 'Create project'})); diff --git a/static/app/views/onboarding/scmProjectDetails.tsx b/static/app/views/onboarding/scmProjectDetails.tsx index 65741fb574b488..5dbc9e40765693 100644 --- a/static/app/views/onboarding/scmProjectDetails.tsx +++ b/static/app/views/onboarding/scmProjectDetails.tsx @@ -7,11 +7,12 @@ import {Container, Flex, Stack} from '@sentry/scraps/layout'; import {Text} from '@sentry/scraps/text'; import {addErrorMessage} from 'sentry/actionCreators/indicator'; -import {useOnboardingContext} from 'sentry/components/onboarding/onboardingContext'; +import type {ProductSolution} from 'sentry/components/onboarding/gettingStartedDoc/types'; import {useCreateProjectAndRules} from 'sentry/components/onboarding/useCreateProjectAndRules'; import {TeamSelector} from 'sentry/components/teamSelector'; import {IconGroup, IconProject, IconSiren} from 'sentry/icons'; import {t} from 'sentry/locale'; +import type {OnboardingSelectedSDK} from 'sentry/types/onboarding'; import type {Team} from 'sentry/types/organization'; import {trackAnalytics} from 'sentry/utils/analytics'; import {slugify} from 'sentry/utils/slugify'; @@ -27,14 +28,26 @@ import { import {ScmAlertFrequency} from './components/scmAlertFrequency'; import {ScmStepFooter} from './components/scmStepFooter'; import {ScmStepHeader} from './components/scmStepHeader'; -import type {StepProps} from './types'; const PROJECT_DETAILS_WIDTH = '285px'; -export function ScmProjectDetails({onComplete}: StepProps) { +export interface ScmProjectDetailsProps { + onComplete: ( + platform?: OnboardingSelectedSDK, + query?: Record + ) => void; + onProjectCreated: (slug: string) => void; + selectedFeatures: ProductSolution[] | undefined; + selectedPlatform: OnboardingSelectedSDK | undefined; +} + +export function ScmProjectDetails({ + onComplete, + onProjectCreated, + selectedFeatures, + selectedPlatform, +}: ScmProjectDetailsProps) { const organization = useOrganization(); - const {selectedPlatform, selectedFeatures, setCreatedProjectSlug} = - useOnboardingContext(); const {teams} = useTeams(); const createProjectAndRules = useCreateProjectAndRules(); useEffect(() => { @@ -115,7 +128,7 @@ export function ScmProjectDetails({onComplete}: StepProps) { // Store the project slug separately so onboarding.tsx can find // the project via useRecentCreatedProject without corrupting // selectedPlatform.key (which the platform features step needs). - setCreatedProjectSlug(project.slug); + onProjectCreated(project.slug); trackAnalytics('onboarding.scm_project_details_create_succeeded', { organization, From 39ea7d486a8557fbeb6acd591461ac697f1c656f Mon Sep 17 00:00:00 2001 From: Jay Goss Date: Tue, 14 Apr 2026 12:45:53 -0500 Subject: [PATCH 2/2] fix(onboarding): Fix knip and typecheck CI failures Un-export OnboardingSessionState (no longer imported externally). Add @public JSDoc tags to SCM prop interfaces so knip recognizes them as intentionally exported for the project creation flow. Fix renderComponent type in scmPlatformFeatures.spec.tsx. ref(onboarding): Un-export SCM prop interfaces until needed No external consumers yet. Re-export when the project creation flow imports them. --- static/app/components/onboarding/onboardingContext.tsx | 2 +- static/app/views/onboarding/components/scmRepoSelector.tsx | 2 +- static/app/views/onboarding/scmConnect.tsx | 2 +- static/app/views/onboarding/scmPlatformFeatures.spec.tsx | 2 +- static/app/views/onboarding/scmPlatformFeatures.tsx | 2 +- static/app/views/onboarding/scmProjectDetails.tsx | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/static/app/components/onboarding/onboardingContext.tsx b/static/app/components/onboarding/onboardingContext.tsx index 1db8257436d939..cd5019db2e7029 100644 --- a/static/app/components/onboarding/onboardingContext.tsx +++ b/static/app/components/onboarding/onboardingContext.tsx @@ -19,7 +19,7 @@ type OnboardingContextProps = { selectedRepository?: Repository; }; -export type OnboardingSessionState = { +type OnboardingSessionState = { createdProjectSlug?: string; selectedFeatures?: ProductSolution[]; selectedIntegration?: Integration; diff --git a/static/app/views/onboarding/components/scmRepoSelector.tsx b/static/app/views/onboarding/components/scmRepoSelector.tsx index 54c9b9839353d3..a32a5d3e488d12 100644 --- a/static/app/views/onboarding/components/scmRepoSelector.tsx +++ b/static/app/views/onboarding/components/scmRepoSelector.tsx @@ -12,7 +12,7 @@ import {ScmVirtualizedMenuList} from './scmVirtualizedMenuList'; import {useScmRepos} from './useScmRepos'; import {useScmRepoSelection} from './useScmRepoSelection'; -export interface ScmRepoSelectorProps { +interface ScmRepoSelectorProps { integration: Integration; onRepositoryChange: (repo: Repository | undefined) => void; selectedRepository: Repository | undefined; diff --git a/static/app/views/onboarding/scmConnect.tsx b/static/app/views/onboarding/scmConnect.tsx index 2376d7cd47518a..07b6a1afa929b0 100644 --- a/static/app/views/onboarding/scmConnect.tsx +++ b/static/app/views/onboarding/scmConnect.tsx @@ -22,7 +22,7 @@ import {useScmPlatformDetection} from './components/useScmPlatformDetection'; import {useScmProviders} from './components/useScmProviders'; import {SCM_STEP_CONTENT_WIDTH} from './consts'; -export interface ScmConnectProps { +interface ScmConnectProps { onComplete: () => void; onIntegrationChange: (integration: Integration | undefined) => void; onRepositoryChange: (repo: Repository | undefined) => void; diff --git a/static/app/views/onboarding/scmPlatformFeatures.spec.tsx b/static/app/views/onboarding/scmPlatformFeatures.spec.tsx index 1586fdb1a4eeeb..5e741dce95b9f1 100644 --- a/static/app/views/onboarding/scmPlatformFeatures.spec.tsx +++ b/static/app/views/onboarding/scmPlatformFeatures.spec.tsx @@ -67,7 +67,7 @@ describe('ScmPlatformFeatures', () => { function renderComponent( overrides?: Partial, - orgOverride?: Parameters[1]['organization'] + orgOverride?: typeof organization ) { const props = {...defaultProps, ...overrides}; return render(, { diff --git a/static/app/views/onboarding/scmPlatformFeatures.tsx b/static/app/views/onboarding/scmPlatformFeatures.tsx index 2992cc50e2f511..77e1ace0bc670c 100644 --- a/static/app/views/onboarding/scmPlatformFeatures.tsx +++ b/static/app/views/onboarding/scmPlatformFeatures.tsx @@ -75,7 +75,7 @@ function shouldSuggestFramework(platformKey: PlatformKey): boolean { // Wider than SCM_STEP_CONTENT_WIDTH (506px) used by the footer. const PLATFORM_CONTENT_WIDTH = '564px'; -export interface ScmPlatformFeaturesProps { +interface ScmPlatformFeaturesProps { onComplete: () => void; onFeaturesChange: (features: ProductSolution[]) => void; onPlatformChange: (platform: OnboardingSelectedSDK | undefined) => void; diff --git a/static/app/views/onboarding/scmProjectDetails.tsx b/static/app/views/onboarding/scmProjectDetails.tsx index 5dbc9e40765693..a49f3aa4b294f7 100644 --- a/static/app/views/onboarding/scmProjectDetails.tsx +++ b/static/app/views/onboarding/scmProjectDetails.tsx @@ -31,7 +31,7 @@ import {ScmStepHeader} from './components/scmStepHeader'; const PROJECT_DETAILS_WIDTH = '285px'; -export interface ScmProjectDetailsProps { +interface ScmProjectDetailsProps { onComplete: ( platform?: OnboardingSelectedSDK, query?: Record