feat(onboarding): funnel analytics for the new flow#7960
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds onboarding event definitions, feature-flag-based variant helpers, one-time variant trait tracking, and analytics for prompt copies, snippets, flag toggles, project creation, and first-feature creation. Connect-panel callbacks now propagate copy actions, while flag toggling reports success through a boolean promise. Event tracking also validates payloads and catches dispatch errors. Estimated code review effort: 3 (Moderate) | ~25 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
97fe9e6 to
10fa440
Compare
3a4e76d to
0a430fe
Compare
0a430fe to
5492b47
Compare
Docker builds report
|
Zaimwa9
left a comment
There was a problem hiding this comment.
Mmh, sorry overall I feel that the whole flag/variant/tracking management is done at the wrong place and should be using a multivariate flag (if not an experimentation but that can be done later on).
I don't think the tracking should happen in identify:
API.identifyruns on every user model load and the variant could change over time- The real tracking should happen when we consume the variant (
GettingStartedGate.tsxseems like the right place) - We would track it on 3 years old accounts for example and/or on users that never reach the onboarding, meaning we would corrupt the data
Let me know if i'm missing something
| // Which onboarding flow the user is in, from the onboarding_quickstart_flow | ||
| // flag the new flow is gated behind (#7738). |
There was a problem hiding this comment.
We can remove this comment
| }), | ||
| ) | ||
| .unwrap() | ||
| // Milestone, not action (see the project create above): First only. |
There was a problem hiding this comment.
Same. Can you go over the comments and only leave the ones clearly stating what is happening?
There was a problem hiding this comment.
Done removed in 30ffd6c. Trimmed the narrative comments, left only the two flagging
non-obvious behaviour.
| export const getOnboardingVariant = (): OnboardingVariant => | ||
| Utils.getFlagsmithHasFeature('onboarding_quickstart_flow') | ||
| ? 'single_page' | ||
| : 'control' |
There was a problem hiding this comment.
How do we plan to release this one ? Should we directly have 2 variants in the feature and grab the variant name from the flag ?
| // Pin the onboarding variant as a user property once the flag has | ||
| // resolved, so every event (incl. autocapture pageviews) is attributable | ||
| // to control vs single_page in one funnel (#7738). | ||
| API.flagsmithIdentify()?.then(() => |
There was a problem hiding this comment.
we could have it in a finally maybe to also grab the default value ?
This means we are setting it each time we identify a user ?
All users are getting an onboardingVariant trait while they haven't gone through the onboarding ?
There was a problem hiding this comment.
Just checked, we have this amplitude method to Identify.setOnce to avoid mutating it
There was a problem hiding this comment.
Good call @Zaimwa9. I will revert the changes on api.ts in order to keep this PR clean with the funnel analytics and will create follow up PR's to address your concerns.
There was a problem hiding this comment.
I did it on a follow-up #8061, to keep this PR onboarding-only.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f40ac417-b58c-44c5-bc90-9f9d8ece7b77
📒 Files selected for processing (9)
frontend/common/utils/getOnboardingVariant.tsfrontend/e2e/tests/initialise-tests.pw.tsfrontend/e2e/tests/onboarding-tests.pw.tsfrontend/web/components/App.jsfrontend/web/components/pages/onboarding/GettingStartedGate.tsxfrontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.tsxfrontend/web/components/pages/onboarding/hooks/bootstrapOnboarding.tsfrontend/web/components/pages/onboarding/hooks/useOnboardingFlag.tsfrontend/web/project/api.ts
af03032 to
71aecb7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/web/project/api.ts (1)
313-323: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMove validation outside the Google Analytics check and handle promise rejection.
The event validation and debug logging are nested inside
if (Project.ga), meaning they won't run for Amplitude-only setups. Furthermore,API.postEventreturns a promise that could reject, but the synchronoustry/catchwon't catch it, leading to an unhandled promise rejection.♻️ Proposed fix
- if (Project.ga) { - if (Project.logAnalytics) console.log('ANALYTICS EVENT', data) - if (!data || !data.category || !data.event) - return console.error('Invalid event provided', data) - if (data.category === 'First') - API.postEvent( - `${data.event}${ - data.extra ? ` ${JSON.stringify(data.extra)}` : '' - }`, - 'first_events', - ) + if (Project.logAnalytics) console.log('ANALYTICS EVENT', data) + if (!data || !data.category || !data.event) + return console.error('Invalid event provided', data) + + if (data.category === 'First') { + API.postEvent( + `${data.event}${data.extra ? ` ${JSON.stringify(data.extra)}` : ''}`, + 'first_events', + )?.catch((err) => console.error('Error posting first event', err)) + } + + if (Project.ga) { ga('send', {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2035470c-64b7-49b0-8914-284551309112
📒 Files selected for processing (9)
frontend/common/constants.tsfrontend/common/utils/getOnboardingVariant.tsfrontend/web/components/pages/onboarding/GettingStartedGate.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/ConnectWithAiPanel.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/OnboardingConnectPanel.tsxfrontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.tsxfrontend/web/components/pages/onboarding/hooks/bootstrapOnboarding.tsfrontend/web/components/pages/onboarding/hooks/useOnboardingFlag.tsfrontend/web/project/api.ts
✅ private-cloud · depot-ubuntu-latest-16 — run #18636 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18636 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18636 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18636 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18634 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18634 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18633 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18633 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18634 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18634 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #18633 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18633 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
|
✅ oss · depot-ubuntu-latest-arm-16 — run #18572 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
|
…otstrap The single-page flow auto-creates the project and flag, which emitted nothing, so the funnel had no project/feature step. Fire First Project created / First Feature created on a real create. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Onboarding snippet copied (install/wire), Onboarding AI connect used, and Onboarding flag toggled (fired only after the toggle saves; toggle now returns success). For measuring internal drop-off in the new flow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Set the onboarding_variant user property (control | single_page) when the user reaches the gate, derived from the onboarding_quickstart_flow flag. Tagged at the point of consumption, not in identify, so only users who reach onboarding get it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
da1ace9 to
c67bc32
Compare
Use amplitude setOnce for onboarding_variant so it's pinned to the user's first exposure and doesn't get overwritten on later loads. Adds API.trackTraitsOnce; the getting-started gate uses it. Addresses Wadii's review note on #7960. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A failing tracker (GA/amplitude) can't break the caller, e.g. the onboarding bootstrap firing milestone events. Addresses kyle's note on #7960. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| const variant = getOnboardingVariant() | ||
|
|
||
| useEffect(() => { | ||
| API.trackTraits({ onboarding_variant: variant }) |
There was a problem hiding this comment.
Tagged at the gate (only users who reach onboarding). set for now; immutable setOnce in #8061.
Zaimwa9
left a comment
There was a problem hiding this comment.
One change to use the right property
| export const getOnboardingVariant = (): OnboardingVariant => { | ||
| if (!Utils.getFlagsmithHasFeature('onboarding_quickstart_flow')) { | ||
| return 'control' | ||
| } | ||
| const value = Utils.getFlagsmithValue('onboarding_quickstart_flow') | ||
| return isOnboardingVariant(value) ? value : 'single_page' | ||
| } |
There was a problem hiding this comment.
Sorry, see here https://github.com/Flagsmith/flagsmith/pull/7979/changes#diff-da830800eab2c18085ef679517e14d18836f9d0db7940a925117f4bd8c5c565bR27-R28
We should have access to the variant NAME directly, so no need to use the value at all normally
There was a problem hiding this comment.
Pushed df6090f, reading the arm from flag.variant. Is that what you meant? Left getExperimentFlag for the experimentation follow-up, glad to hear your thoughts.
Review feedback: the identity flags response names the served variant (the MV option key, or the reserved 'control' for the default arm), so the arm comes from flag.variant and the flag's value stays free. An enabled flag with no variant is the pre-conversion boolean, meaning the new flow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
docs/if required so people know about the feature.Changes
Contributes to #7738.
Funnel analytics for the new single-page onboarding flow. It now emits the same milestone events as the current flow, tags the variant it served, and adds new-flow-only diagnostics. Funnel analytics only. The multivariate flag and a true A/B split (tagging the control arm) are a follow-up.
onboarding_variantuser property (control|single_page), set at the getting-started gate when a user reaches onboarding (not at identify), from theonboarding_quickstart_flowflag. Tags which flow the user saw, so onboarding events can be grouped by it.First Project created/First Feature createdon a real create (org already fired). Milestone events only, not the genericProject/Feature createdactions, which stay reserved for resources a user makes by hand.Onboarding snippet copied(install / wire),Onboarding AI connect used,Onboarding flag toggled.Follow-ups: multivariate flag plus control-arm tagging for a true A/B split; activation (first SDK evaluation, pending #7623); shared-analytics hardening (
setOnce,trackEventtry/catch) in #8061.How did you test this code?
Manually, with
onboarding_quickstart_flowon and analytics logging enabled (or by spying onAPI.trackEvent/API.trackTraitsin the console):onboarding_variant = single_pageis set once at the gate.First Project created/First Feature createdfire once on the auto-bootstrap, and not again on revisit.