Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions frontend/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,18 @@ const Constants = {
'category': 'User',
'event': `User oauth ${type}`,
}),
'ONBOARDING_AI_CONNECT': {
'category': 'Onboarding',
'event': 'Onboarding AI connect used',
},
'ONBOARDING_FLAG_TOGGLED': {
'category': 'Onboarding',
'event': 'Onboarding flag toggled',
},
'ONBOARDING_SNIPPET_COPIED': {
'category': 'Onboarding',
'event': 'Onboarding snippet copied',
},
'REFERRER_CONVERSION': (referrer: string) => ({
'category': 'Referrer',
'event': `${referrer} converted`,
Expand Down
10 changes: 10 additions & 0 deletions frontend/common/utils/getOnboardingVariant.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import Utils from './utils'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use the alias import path instead of a relative import.

As per coding guidelines, frontend/**/*.{js,jsx,ts,tsx} should "Use only common/, components/, and project/ import paths; do not use relative imports."

♻️ Proposed fix
-import Utils from './utils'
+import Utils from 'common/utils'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import Utils from './utils'
import Utils from 'common/utils'

Source: Coding guidelines


export type OnboardingVariant = 'control' | 'single_page'

// Which onboarding flow the user is in, from the onboarding_quickstart_flow
// flag the new flow is gated behind (#7738).
export const getOnboardingVariant = (): OnboardingVariant =>
Utils.getFlagsmithHasFeature('onboarding_quickstart_flow')
? 'single_page'
: 'control'
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useCopyFeedback } from 'components/pages/onboarding/hooks/useCopyFeedba
export type ConnectWithAiPanelProps = {
environmentKey: string
featureName: string
onCopyPrompt?: () => void
}

// "Connect with AI" tab: an agent-agnostic prompt the user pastes into their
Expand All @@ -18,6 +19,7 @@ export type ConnectWithAiPanelProps = {
const ConnectWithAiPanel: FC<ConnectWithAiPanelProps> = ({
environmentKey,
featureName,
onCopyPrompt,
}) => {
const { copied, copy } = useCopyFeedback()

Expand All @@ -39,7 +41,14 @@ Detect my stack, install the SDK, and wire ${featureName} into one place. Then r
<code className='onboarding-connect__prompt-text flex-fill'>
{aiPrompt}
</code>
<Button theme='outline' size='small' onClick={() => copy(aiPrompt)}>
<Button
theme='outline'
size='small'
onClick={() => {
copy(aiPrompt)
onCopyPrompt?.()
}}
>
<span
className='d-inline-flex align-items-center gap-1'
aria-live='polite'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export type OnboardingConnectPanelProps = {
featureName: string
onCopyInstall?: () => void
onCopyWire?: () => void
onCopyPrompt?: () => void
}

// Two ways to connect an app to the pre-created flag: paste an agent-agnostic
Expand All @@ -40,6 +41,7 @@ const OnboardingConnectPanel: FC<OnboardingConnectPanelProps> = ({
environmentKey,
featureName,
onCopyInstall,
onCopyPrompt,
onCopyWire,
}) => {
const [tab, setTab] = useState<ConnectTab>('manual')
Expand All @@ -61,6 +63,7 @@ const OnboardingConnectPanel: FC<OnboardingConnectPanelProps> = ({
<ConnectWithAiPanel
environmentKey={environmentKey}
featureName={featureName}
onCopyPrompt={onCopyPrompt}
/>
</OnboardingTabPanel>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import { useOnboardingFlag } from 'components/pages/onboarding/hooks/useOnboardi
import { useOnboardingConnection } from 'components/pages/onboarding/hooks/useOnboardingConnection'
import { useUpdateOrganisationMutation } from 'common/services/useOrganisation'
import { useUpdateProjectMutation } from 'common/services/useProject'
import API from 'project/api'
import Constants from 'common/constants'
import './OnboardingFlow.scss'

// The single-page onboarding flow, rendered at /getting-started when
Expand Down Expand Up @@ -139,6 +141,37 @@ const OnboardingFlow: FC = () => {
history.push(`${base}/features?feature=${flagId}&tab=${tab}`)
}

const diagnosticIds = {
environment_id: environment?.id,
organisation_id: organisationId,
project_id: projectId,
}
const trackSnippetCopied = (snippet: 'install' | 'wire') =>
API.trackEvent({
...Constants.events.ONBOARDING_SNIPPET_COPIED,
extra: { ...diagnosticIds, snippet },
})
Comment on lines +149 to +153

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Extract the inline union type.

As per coding guidelines, frontend/**/*.{ts,tsx} should "Extract inline union types into named types (for example, use type Status = 'A' | 'B' instead of an inline union)." The 'install' | 'wire' parameter type should be named.

♻️ Proposed fix
+type OnboardingSnippet = 'install' | 'wire'
+
-  const trackSnippetCopied = (snippet: 'install' | 'wire') =>
+  const trackSnippetCopied = (snippet: OnboardingSnippet) =>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const trackSnippetCopied = (snippet: 'install' | 'wire') =>
API.trackEvent({
...Constants.events.ONBOARDING_SNIPPET_COPIED,
extra: { ...diagnosticIds, snippet },
})
type OnboardingSnippet = 'install' | 'wire'
const trackSnippetCopied = (snippet: OnboardingSnippet) =>
API.trackEvent({
...Constants.events.ONBOARDING_SNIPPET_COPIED,
extra: { ...diagnosticIds, snippet },
})

Source: Coding guidelines

const handleCopyInstall = () => {
trackSnippetCopied('install')
setInstallCopied(true)
}
const handleCopyWire = () => {
trackSnippetCopied('wire')
setSnippetCopied(true)
}
const handleCopyPrompt = () =>
API.trackEvent({
...Constants.events.ONBOARDING_AI_CONNECT,
extra: diagnosticIds,
})
const handleToggleFlag = (enabled: boolean) => {
API.trackEvent({
...Constants.events.ONBOARDING_FLAG_TOGGLED,
extra: { ...diagnosticIds, enabled },
})
toggleFlag(enabled)
}

if (status === 'creating') {
return (
<div className='onboarding-flow mx-auto text-center'>
Expand Down Expand Up @@ -177,8 +210,9 @@ const OnboardingFlow: FC = () => {
<OnboardingConnectPanel
environmentKey={environmentKey}
featureName={featureName}
onCopyInstall={() => setInstallCopied(true)}
onCopyWire={() => setSnippetCopied(true)}
onCopyInstall={handleCopyInstall}
onCopyWire={handleCopyWire}
onCopyPrompt={handleCopyPrompt}
/>
<OnboardingTerminal
featureName={featureName}
Expand All @@ -196,7 +230,7 @@ const OnboardingFlow: FC = () => {
tags: flagTags,
},
]}
onToggle={(_flag, next) => toggleFlag(next)}
onToggle={(_flag, next) => handleToggleFlag(next)}
togglingFlag={isToggling ? featureName : null}
togglesReady={flagStateReady}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {
} from 'common/types/responses'
import { SmartDefaults } from './useSmartDefaults'
import { createOrganisationViaAccountStore } from './createOrganisationViaAccountStore'
import API from 'project/api'
import Constants from 'common/constants'

type Store = ReturnType<typeof getStore>

Expand Down Expand Up @@ -93,6 +95,10 @@ async function ensureProject(
}),
)
.unwrap()
// Auto-created setup is a milestone reached, not a user action: fire First
// Project created, not the generic Project created (reserved for projects a
// user makes by hand). First is safe here - we only get here with no project.
API.trackEvent(Constants.events.CREATE_FIRST_PROJECT)
Comment on lines +98 to +101

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Guard milestone tracking so analytics failures can't abort onboarding.

Neither API.trackEvent(Constants.events.CREATE_FIRST_PROJECT) nor the CREATE_FIRST_FEATURE call is guarded. If trackEvent throws synchronously (e.g. an uninitialised analytics SDK), the exception propagates through ensureProject/ensureFlag and aborts the whole bootstrap sequence — even though the project/flag were already successfully created. ensureOnboardingTag right below explicitly wraps its logic in try/catch because tagging is "cosmetic only - never block onboarding on tagging"; the same defensive rationale applies to these milestone events.

♻️ Suggested fix
-  API.trackEvent(Constants.events.CREATE_FIRST_PROJECT)
+  try {
+    API.trackEvent(Constants.events.CREATE_FIRST_PROJECT)
+  } catch {
+    // Analytics is best-effort - never block onboarding on tracking failures.
+  }
   if (isFirstFeature) {
-    API.trackEvent(Constants.events.CREATE_FIRST_FEATURE)
+    try {
+      API.trackEvent(Constants.events.CREATE_FIRST_FEATURE)
+    } catch {
+      // Analytics is best-effort - never block onboarding on tracking failures.
+    }
   }

Also applies to: 192-196

await store
.dispatch(
environmentService.endpoints.createEnvironment.initiate({
Expand Down Expand Up @@ -170,7 +176,8 @@ async function ensureFlag(
if (existing) {
return existing
}
return store
const isFirstFeature = !flags?.results?.length
const created = await store
.dispatch(
projectFlagService.endpoints.createProjectFlag.initiate({
body: {
Expand All @@ -182,6 +189,11 @@ async function ensureFlag(
}),
)
.unwrap()
// Milestone, not action (see the project create above): First only.
if (isFirstFeature) {
API.trackEvent(Constants.events.CREATE_FIRST_FEATURE)
}
return created
}

// Attach the "Onboarding" tag to the demo flag (find-or-create), so the flags
Expand Down
8 changes: 7 additions & 1 deletion frontend/web/project/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { AccountModel, User } from 'common/types/responses'
import AccountStore from 'common/stores/account-store'
import flagsmith from '@flagsmith/flagsmith'
import Utils from 'common/utils/utils'
import { getOnboardingVariant } from 'common/utils/getOnboardingVariant'
import loadChat, { identifyChatUser } from 'common/loadChat'

const API = {
Expand Down Expand Up @@ -247,7 +248,12 @@ const API = {
role: selectedRole,
tasks: user.onboarding?.tasks?.map((t) => t.name) || [],
})
API.flagsmithIdentify()
// 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(() =>
API.trackTraits({ onboarding_variant: getOnboardingVariant() }),
)
Comment on lines +251 to +256

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Add .catch() to avoid an unhandled promise rejection.

identify()'s try/catch is synchronous, so it won't catch a rejection from this async chain. If flagsmithIdentify() rejects, the appended .then() never runs and the rejection goes unhandled (console warnings, noisy error monitoring). The existing flagsmithIdentify internal chain (lines 151-173) has the same gap, but this new addition extends the unhandled surface further.

🛡️ Suggested fix
       API.flagsmithIdentify()?.then(() =>
         API.trackTraits({ onboarding_variant: getOnboardingVariant() }),
-      )
+      ).catch((err) => console.error('Error tracking onboarding_variant', err))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 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(() =>
API.trackTraits({ onboarding_variant: getOnboardingVariant() }),
)
// 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(() =>
API.trackTraits({ onboarding_variant: getOnboardingVariant() }),
).catch((err) => console.error('Error tracking onboarding_variant', err))

} catch (err) {
console.error('Error identifying', err)
}
Expand Down
Loading