Skip to content
Draft
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
29 changes: 29 additions & 0 deletions products/desktop/packages/core/src/onboarding/steps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
type OnboardingStep,
previousStep,
stepDirection,
stepGatePending,
} from "./steps";

type StepGates = Parameters<typeof computeActiveSteps>[0];
Expand Down Expand Up @@ -76,6 +77,34 @@ describe("computeActiveSteps", () => {
});
});

describe("stepGatePending", () => {
it.each<{ step: OnboardingStep; gate: keyof StepGates }>([
{ step: "project-select", gate: "projectCount" },
{ step: "consent", gate: "consentRequired" },
{ step: "install-cli", gate: "hasGithubIntegration" },
{ step: "install-cli", gate: "cliReady" },
])("holds $step while $gate has not answered", ({ step, gate }) => {
const answered: StepGates = {
hasGithubIntegration: false,
cliReady: false,
projectCount: 2,
consentRequired: true,
};

expect(stepGatePending(step, answered)).toBe(false);
expect(stepGatePending(step, { ...answered, [gate]: undefined })).toBe(
true,
);
});

it.each<OnboardingStep>(["connect-github", "select-repo"])(
"never holds %s, which no gate can drop",
(step) => {
expect(stepGatePending(step, allSteps)).toBe(false);
},
);
});

describe("nearestActiveStep", () => {
const withoutConditionals = computeActiveSteps({
hasGithubIntegration: true,
Expand Down
27 changes: 25 additions & 2 deletions products/desktop/packages/core/src/onboarding/steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@ export interface DetectedRepo {
branch?: string;
}

export function computeActiveSteps(options: {
export interface StepGates {
/** Undefined while the integrations query is loading; the step only drops on a confirmed connection. */
hasGithubIntegration: boolean | undefined;
/** Undefined while the local git and gh checks are loading. */
cliReady: boolean | undefined;
/** Undefined until the project list has loaded, so a slow list cannot skip a real choice. */
projectCount: number | undefined;
consentRequired: boolean | undefined;
}): OnboardingStep[] {
}

export function computeActiveSteps(options: StepGates): OnboardingStep[] {
return ONBOARDING_STEPS.filter((step) => {
if (step === "project-select" && options.projectCount === 1) return false;
if (step === "consent" && options.consentRequired === false) return false;
Expand All @@ -45,6 +47,27 @@ export function computeActiveSteps(options: {
});
}

/**
* Whether a gate that governs `step` has not answered yet. An unanswered gate
* keeps its step in the active set, so the step is on screen but a later answer
* can still take it away. Analytics waits for this to be false, or it records a
* view for a step the person never had to complete.
*/
export function stepGatePending(
step: OnboardingStep,
options: StepGates,
): boolean {
if (step === "project-select") return options.projectCount === undefined;
if (step === "consent") return options.consentRequired === undefined;
if (step === "install-cli") {
return (
options.hasGithubIntegration === undefined ||
options.cliReady === undefined
);
}
return false;
}

export function stepIndexOf(
activeSteps: OnboardingStep[],
step: OnboardingStep,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { ConnectGitHubStep } from "@posthog/ui/features/onboarding/components/Co
import { InstallCliStep } from "@posthog/ui/features/onboarding/components/InstallCliStep";
import { useOnboardingFlow } from "@posthog/ui/features/onboarding/hooks/useOnboardingFlow";
import { useOnboardingStore } from "@posthog/ui/features/onboarding/onboardingStore";
import type { OnboardingStep } from "@posthog/ui/features/onboarding/types";
import { useSettingsStore } from "@posthog/ui/features/settings/settingsStore";
import { shipIt } from "@posthog/ui/primitives/confetti";
import { FullScreenLayout } from "@posthog/ui/primitives/FullScreenLayout";
Expand Down Expand Up @@ -77,6 +78,7 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) {
hasGithubIntegration,
consentSatisfied,
consentRequirement,
currentStepPending,
} = useOnboardingFlow();
const completeOnboarding = useOnboardingStore(
(state) => state.completeOnboarding,
Expand Down Expand Up @@ -156,15 +158,36 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) {
const flowStartedAtRef = useRef(Date.now());
const stepEnteredAtRef = useRef(Date.now());

// biome-ignore lint/correctness/useExhaustiveDependencies: fires once on mount; subsequent step views fire from handleNext/handleBack
useEffect(() => {
track(ANALYTICS_EVENTS.ONBOARDING_STARTED);
}, []);

// Entry is when the person arrives on the step, which is not always when the
// view is recorded: a pending gate delays the view but not the reading.
// biome-ignore lint/correctness/useExhaustiveDependencies: currentStep is the trigger, not a value the body reads
useEffect(() => {
stepEnteredAtRef.current = Date.now();
}, [currentStep]);

const viewedStepRef = useRef<OnboardingStep | null>(null);
const recordStepViewed = () => {
if (currentIndex < 0 || viewedStepRef.current === currentStep) return;
viewedStepRef.current = currentStep;
track(ANALYTICS_EVENTS.ONBOARDING_STEP_VIEWED, {
step_id: currentStep,
step_index: currentIndex,
total_steps: activeSteps.length,
});
}, []);
};

// The ordinary path: the step settles while the person is reading it. This
// also covers a step entered by the self-heal in useOnboardingFlow, which
// reaches the person without passing through handleNext.
// biome-ignore lint/correctness/useExhaustiveDependencies: recordStepViewed reads the same values these deps carry
useEffect(() => {
if (currentStepPending) return;
recordStepViewed();
}, [currentStep, currentIndex, currentStepPending, activeSteps.length]);

useEffect(() => {
const handleBeforeUnload = () => {
Expand Down Expand Up @@ -195,17 +218,6 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) {
);
};

const trackStepViewed = (stepIndex: number) => {
const stepId = activeSteps[stepIndex];
if (!stepId) return;
track(ANALYTICS_EVENTS.ONBOARDING_STEP_VIEWED, {
step_id: stepId,
step_index: stepIndex,
total_steps: activeSteps.length,
});
stepEnteredAtRef.current = Date.now();
};

const handleNext = (context?: StepCompletedContext) => {
if (
currentStep === "consent" &&
Expand All @@ -217,14 +229,15 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) {
// into capture properties poisons the whole analytics batch.
const safeContext =
context && "nativeEvent" in context ? undefined : context;
// A person can leave a step before its gate answers, which the effect above
// skips. Record the view first, so a completion never arrives without one.
recordStepViewed();
trackStepCompleted(safeContext);
trackStepViewed(currentIndex + 1);
next();
};

const handleBack = () => {
if (currentStep === "consent" && consentSubmitting) return;
trackStepViewed(currentIndex - 1);
back();
};

Expand All @@ -238,6 +251,7 @@ export function OnboardingFlow({ onOpenSupport }: OnboardingFlowProps) {
useHotkeys("left", handleBack, { enableOnFormTags: false }, [handleBack]);

const handleComplete = (repoSkipped: boolean) => {
recordStepViewed();
if (repoSkipped) {
track(ANALYTICS_EVENTS.ONBOARDING_STEP_SKIPPED, {
step_id: currentStep,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
nearestActiveStep,
type OnboardingStep,
stepDirection,
stepGatePending,
} from "@posthog/core/onboarding/steps";
import { useHostTRPC, useHostTRPCClient } from "@posthog/host-router/react";
import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events";
Expand Down Expand Up @@ -134,7 +135,8 @@ export function useOnboardingFlow() {
],
);

const { data: githubUserIntegrations } = useUserGithubIntegrations();
const { data: githubUserIntegrations, isPending: githubIntegrationsPending } =
useUserGithubIntegrations();
// The install-cli step only offers git and gh, so a ready toolchain skips it.
// InstallCliStep reuses these cached results when the step does render.
const trpc = useHostTRPC();
Expand Down Expand Up @@ -165,9 +167,12 @@ export function useOnboardingFlow() {
gitStatus.installed && ghStatus.installed && ghStatus.authenticated,
);
}, [cliReady, localWorkspaces, gitStatus, ghStatus]);
const hasGithubIntegration = githubUserIntegrations
? githubUserIntegrations.length > 0
: undefined;
// Read the pending state, not the data: the query retries and then leaves
// `data` undefined, which would hold the install-cli gate open for the rest
// of the session. A failed lookup keeps the step, same as an unanswered one.
const hasGithubIntegration = githubIntegrationsPending
? undefined
: (githubUserIntegrations?.length ?? 0) > 0;
// Counted off the store rather than through useProjects, whose auto-select
// effect would then run in a second place and re-clear the query cache.
const orgProjectsMap = useAuthStateValue((state) => state.orgProjectsMap);
Expand Down Expand Up @@ -205,25 +210,24 @@ export function useOnboardingFlow() {
);
}, [consent]);

// A failed lookup keeps the step, same as an unanswered one, but it answers
// the gate. Otherwise the step shows with its view never recorded.
const consentRequired =
consentRequirement?.organizationId === consent.organizationId
? consentRequirement?.required
: undefined;
consent.status === "error"
? true
: consentRequirement?.organizationId === consent.organizationId
? consentRequirement?.required
: undefined;
const sampledConsentRequirement =
consentRequirement?.organizationId === consent.organizationId
? consentRequirement
: undefined;

const activeSteps = useMemo(
() =>
computeActiveSteps({
hasGithubIntegration,
cliReady,
projectCount,
consentRequired,
}),
const stepGates = useMemo(
() => ({ hasGithubIntegration, cliReady, projectCount, consentRequired }),
[hasGithubIntegration, cliReady, projectCount, consentRequired],
);
const activeSteps = useMemo(() => computeActiveSteps(stepGates), [stepGates]);
Comment thread
adboio marked this conversation as resolved.

useEffect(() => {
if (!activeSteps.includes(currentStep)) {
Expand Down Expand Up @@ -259,6 +263,7 @@ export function useOnboardingFlow() {
return {
currentStep,
currentIndex,
currentStepPending: stepGatePending(currentStep, stepGates),
totalSteps: activeSteps.length,
activeSteps,
isFirstStep,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,31 @@ import { describe, expect, it } from "vitest";
import { migrateOnboardingState } from "./onboardingStore";

describe("migrateOnboardingState", () => {
const persisted = (currentStep: string) => ({
currentStep,
hasCompletedOnboarding: false,
hasShippedFirstPr: false,
selectedProjectId: 1,
});

it("moves a persisted invite-code step forward", () => {
const migrated = migrateOnboardingState({
currentStep: "invite-code",
hasCompletedOnboarding: false,
hasShippedFirstPr: false,
selectedProjectId: 1,
});

expect(migrated.currentStep).toBe("consent");
expect(migrateOnboardingState(persisted("invite-code")).currentStep).toBe(
"consent",
);
});

it.each(["welcome", "import-config"])(
"resets the retired %s step to the start of the flow",
(step) => {
expect(migrateOnboardingState(persisted(step)).currentStep).toBe(
"project-select",
);
},
);

it("leaves a step of the current flow alone", () => {
expect(migrateOnboardingState(persisted("select-repo")).currentStep).toBe(
"select-repo",
);
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import type { OnboardingStep } from "@posthog/ui/features/onboarding/types";
import {
ONBOARDING_STEPS,
type OnboardingStep,
} from "@posthog/ui/features/onboarding/types";
import { logger } from "@posthog/ui/shell/logger";
import { create } from "zustand";
import { persist } from "zustand/middleware";
Expand Down Expand Up @@ -37,6 +40,11 @@ export function migrateOnboardingState(
if ((state.currentStep as string) === "invite-code") {
return { ...state, currentStep: "consent" };
}
// A step id from a retired set, for example "welcome", renders no branch in
// the flow, so the person sees an empty card until the self-heal moves them.
if (!ONBOARDING_STEPS.includes(state.currentStep)) {
return { ...state, currentStep: ONBOARDING_STEPS[0] };
}
return state;
}

Expand All @@ -61,7 +69,7 @@ export const useOnboardingStore = create<OnboardingStore>()(
}),
{
name: "onboarding-store",
version: 1,
version: 2,
migrate: migrateOnboardingState,
partialize: (state) => ({
currentStep: state.currentStep,
Expand Down
Loading