feat(onboarding): Wire ScmIntegrationConnect into single-view project creation#116582
Conversation
📊 Type Coverage Diff✅ no issues found |
7f96854 to
d4bb6f8
Compare
97a8c13 to
d1c556a
Compare
d4bb6f8 to
199e2a7
Compare
d1c556a to
e985d01
Compare
199e2a7 to
88b03b6
Compare
e985d01 to
75000e7
Compare
88b03b6 to
f649a6f
Compare
75000e7 to
8604be7
Compare
8604be7 to
6b66ee3
Compare
f649a6f to
4c9f55e
Compare
6b66ee3 to
2a70b62
Compare
6030e55 to
418bb41
Compare
e28c03f to
b9d1529
Compare
418bb41 to
a02c19d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a02c19db2dd9c73d4975655b6a328f2bf401bc9b. Configure here.
| <Button | ||
| variant="transparent" | ||
| analyticsEventKey="project_creation.scm_connect_skip_clicked" | ||
| analyticsEventName="Project Creation: SCM Connect Skip Clicked" |
There was a problem hiding this comment.
Analytics skip event not registered in event map
Medium Severity
The analyticsEventKey "project_creation.scm_connect_skip_clicked" used on the skip button is not registered in ProjectCreationEventParameters or projectCreationEventMap in projectCreationAnalyticsEvents.tsx. Every other project_creation.scm_* event is registered there, suggesting this one was missed. Without registration, the event may not be tracked correctly through the analytics pipeline.
Reviewed by Cursor Bugbot for commit a02c19db2dd9c73d4975655b6a328f2bf401bc9b. Configure here.
There was a problem hiding this comment.
The key genuinely isn't registered, but this doesn't affect tracking. The skip button emits through the <Button analyticsEventKey/analyticsEventName> props, which route via useButtonTracking to rawTrackAnalyticsEvent with the explicit key and name. That path never consults projectCreationEventMap, so the event fires correctly at runtime regardless of registration.
The registered scm_* siblings (scm_connect_repo_selected, scm_connect_step_viewed) are a different mechanism: they go through trackAnalytics(), which requires registration for typing and to resolve the event name. The Button prop path is untyped and self-contained, so the "every other scm_* is registered" comparison doesn't apply here.
For reference, the onboarding flow this was extracted from uses the same untyped Button path with onboarding.scm_connect_skip_clicked, which is also unregistered. Mirroring that intentionally.
Marking non-blocking. May register both keys in a follow-up if we want the map to be a complete event catalog.
b9d1529 to
ee400b3
Compare
a02c19d to
44c9903
Compare
… creation Replaces the placeholder section 1 in the SCM-first project creation view with ScmIntegrationConnect, the integration-and-repo connection core extracted from ScmConnect in the previous PR. analyticsFlow is passed as "project-creation" so the component's events route through the project_creation.scm_* keys registered in VDY-77. The onboarding chrome (intro heading, lock/revoke text, benefits grid, Continue/Skip footer) does not render here. The wizard provides a "Continue without connecting a repo" button below the core for users who do not want to connect a repo; clicking it flips repoStepCompleted. Selecting a repo flips the flag eagerly via handleRepositoryChange. WizardState gains selectedIntegration and selectedRepository so a refresh restores the section 1 state. handleClearDerivedState is a no-op until VDY-75/76 wire platform/features/project state. Refs VDY-74
handleRepositoryChange's comment documented an eager flip of repoStepCompleted on non-null repo selection, but the code only updated selectedRepository. With showAllSteps deriving from "repoStepCompleted || selectedRepository", selecting then de-selecting a repo collapsed sections 2 and 3, contradicting the "later state edits do not collapse" invariant. Restore the eager flip and simplify showAllSteps to repoStepCompleted since the OR's second clause is now redundant. Refs VDY-74
44c9903 to
4b97df6
Compare
| }, [setState]); | ||
| }; | ||
|
|
||
| const handleIntegrationChange = useCallback( |
There was a problem hiding this comment.
does this actually need to be a useCallback?
There was a problem hiding this comment.
Yes in the sense that it is used as a dep of other useCallbacks
| (integration: Integration | undefined) => { | ||
| setState(s => ({...s, selectedIntegration: integration})); | ||
| }, | ||
| [setState] |
There was a problem hiding this comment.
I think the hook rules understand that setState has a consistent identity and you don't need to pass it as a dependency.
| repoStepCompleted: repository ? true : s.repoStepCompleted, | ||
| })); | ||
| }, | ||
| [setState] |
…ures (#116624) ## TL;DR Splits `ScmPlatformFeatures` into a reusable platform-and-features selection core (`ScmPlatformFeaturesCore`) and the onboarding chrome that wraps it. The core owns platform detection, feature metadata, the manual-picker toggle, all selection/toggle handlers, and the `step_viewed` / `platform_selected` / `feature_toggled` / `change_platform_clicked` analytics. The wrapper keeps the page headings, the Continue button, and the auto-create-on-skip-project-details path. Behavior preserved (modulo a small analyticsParam drop noted below); the existing 26 spec tests pass unchanged. This is the VDY-75 equivalent of PR 3's `ScmIntegrationConnect` extraction. The future wiring PR will consume `ScmPlatformFeaturesCore` from `scmCreateProject` for section 2 of the single-view wizard. --- ## Stack - [PR 1](#116434): `ScmAnalyticsFlow` + project_creation event registry - [PR 2](#116577): Single-view scaffold - [PR 3](#116581): Extract `ScmIntegrationConnect` - [PR 4](#116582): Wire `ScmIntegrationConnect` - **PR 5 (this):** Extract `ScmPlatformFeaturesCore` - PR 6 (next): Wire `ScmPlatformFeaturesCore` into section 2 (VDY-75 wiring) ## Summary - New `static/app/views/onboarding/components/scmPlatformFeaturesCore.tsx` owns the platform-and-features selection slice (auto-detected cards or manual picker + feature selection/info cards + the "unlimited volume" promo card). Props: `analyticsFlow`, `selectedRepository`, `selectedPlatform`, `selectedFeatures`, `onPlatformChange`, `onFeaturesChange`, `onClearProjectDetailsForm`. - New `static/app/views/onboarding/components/scmPlatformHelpers.tsx` holds shared constants and helpers (`FEATURE_DISPLAY_ORDER`, `getPlatformInfo`, `getPlatformName`, `platformOptions`, `toSelectedSdk`, `shouldSuggestFramework`, `ResolvedPlatform`) so both the wrapper and the core can import without a circular dependency. - `ScmPlatformFeatures` (wrapper) now imports and renders `<ScmPlatformFeaturesCore analyticsFlow="onboarding" .../>`. It still calls `useScmPlatformDetection` to derive `currentPlatformKey` for `handleContinue`'s auto-create path (React Query dedupes with the core's call). - The Continue button's `analyticsParams` drop the `source: 'detected' | 'manual'` field, since that lived in core-internal state and is already captured by per-selection `scm_platform_selected` events. - All 26 existing `scmPlatformFeatures.spec.tsx` tests pass unchanged; broader onboarding specs (`onboarding.spec.tsx`, `scmProjectDetails.spec.tsx`, `scmRepoSelector.spec.tsx`) also clean. ## Out of scope - Wiring `ScmPlatformFeaturesCore` into `scmCreateProject`. PR 6 will pick that up. Refs VDY-75 ## Test plan - [x] \`pnpm typecheck\` clean - [x] \`pnpm lint:js\` clean on touched files - [x] \`pnpm test-ci scmPlatformFeatures.spec.tsx\` (26 tests, all pass) - [x] \`pnpm test-ci onboarding.spec.tsx scmProjectDetails.spec.tsx scmRepoSelector.spec.tsx\` (59 tests, all pass) - [ ] In onboarding, the platform-features step renders identically: detected cards when SCM connected and platforms detected, manual picker otherwise, feature cards below, Continue footer - [ ] Selecting an auto-detected card or picking from manual fires \`onboarding.scm_platform_selected\` with the right \`source\` - [ ] Toggling a feature fires \`onboarding.scm_platform_feature_toggled\` - [ ] Clicking Continue fires \`onboarding.scm_platform_features_continue_clicked\` (without \`source\` param, with \`platform\` and \`features\`)
…ct creation (#116626) ## TL;DR Wires `ScmPlatformFeaturesCore` (extracted in PR 5) into section 2 of the SCM-first project creation wizard. The placeholder `PlatformFeaturesSection` is replaced by the real component, wrapped in the same bordered-card chrome as section 1, with `analyticsFlow="project-creation"` routing events through the `project_creation.scm_*` keys. Section 3 stays as a placeholder pending VDY-76. --- ## Stack - [PR 1](#116434): `ScmAnalyticsFlow` + project_creation event registry - [PR 2](#116577): Single-view scaffold - [PR 3](#116581): Extract `ScmIntegrationConnect` - [PR 4](#116582): Wire `ScmIntegrationConnect` - [PR 5](#116624): Extract `ScmPlatformFeaturesCore` - **PR 6 (this):** Wire `ScmPlatformFeaturesCore` into section 2 ## Summary - Replaces the placeholder `PlatformFeaturesSection` with `<ScmPlatformFeaturesCore analyticsFlow="project-creation" .../>`, wrapped in a `MotionStack` with `border="primary" radius="md" padding="lg"` to match section 1's card look. A "Platform & features" heading and short description sit above the core. - `WizardState` gains `selectedPlatform: OnboardingSelectedSDK | undefined` and `selectedFeatures: ProductSolution[] | undefined` so a refresh restores section 2's state through sessionStorage. - `handlePlatformChange` and `handleFeaturesChange` propagate user selections back into state. - `handleClearDerivedState` now clears `selectedPlatform` and `selectedFeatures` (in addition to its previous no-op) so changing the repo resets section 2's derived state. The core's contract for repo re-selection. - `handleClearProjectDetailsForm` stays a no-op until VDY-76 wires up project-details state. - Section 3 (`ProjectDetailsSection`) is still a placeholder. VDY-76 will replace it. ## Out of scope - Wiring `ScmProjectDetails` (or its future extracted core) into section 3. VDY-76 will pick that up. - Per-section progressive disclosure between sections 2 and 3. Per the design decision, both reveal together when `repoStepCompleted` flips. Refs VDY-75 ## Test plan - [x] \`pnpm typecheck\` clean - [x] \`pnpm lint:js\` clean on touched file - [ ] With the experiment flag on, after completing section 1, section 2 renders \`ScmPlatformFeaturesCore\` inside the bordered card - [ ] With a repo connected, auto-detected platform cards appear and clicking one fires \`project_creation.scm_platform_selected\` with \`source: 'detected'\` - [ ] Without a repo (skip path), the manual picker shows and selecting fires \`project_creation.scm_platform_selected\` with \`source: 'manual'\` - [ ] Toggling a feature fires \`project_creation.scm_platform_feature_toggled\` - [ ] Changing the repo in section 1 clears \`selectedPlatform\` and \`selectedFeatures\` (section 2 resets) - [ ] \`project_creation.scm_platform_features_step_viewed\` fires when section 2 mounts (i.e. after \`repoStepCompleted\` flips)


TL;DR
Replaces the placeholder section 1 in the SCM-first project creation view with
ScmIntegrationConnect(the integration-and-repo connection core extracted in PR 3).analyticsFlow="project-creation"routes its events throughproject_creation.scm_*keys. The wizard provides its own "Continue without connecting a repo" button below the core for users who do not want to connect a repo; selecting a repo flips the reveal flag eagerly. Sections 2 and 3 remain placeholders pending VDY-75/76.Stack
ScmIntegrationConnectfromScmConnectScmIntegrationConnectinto the single-view scaffoldSummary
ConnectRepositorySectioninscmCreateProject.tsxwith<ScmIntegrationConnect analyticsFlow="project-creation" ... />. None of the onboarding chrome (intro heading, lock/revoke text, benefits grid, Continue button) renders here.project_creation.scm_connect_skip_clickedand flipsrepoStepCompleted.handleRepositoryChangewraps the core'sonRepositoryChangeso a non-null repo selection eagerly flips the reveal flag.WizardStategainsselectedIntegrationandselectedRepositoryso a refresh restores section 1 state.handleClearDerivedStateis a no-op until VDY-75/76 wire platform/features/project state.Out of scope
<ScmPlatformFeatures />and<ScmProjectDetails />into sections 2 and 3. VDY-75/76 will pick those up.has_integrationanalyticsParam on the skip event. The wizard does not calluseScmProvidersto deriveeffectiveIntegration; if signal demands it later, we can plumb that through.Refs VDY-74
Test plan
pnpm typecheckcleanpnpm lint:jsclean on touched files/projects/new/rendersScmIntegrationConnectin section 1 (no big benefits grid, no onboarding header)project_creation.scm_connect_skip_clickedproject_creation.scm_connect_step_viewedfires on page load