From 7decfd0a62fb6889bc3846c3ebefbb7d45df0c80 Mon Sep 17 00:00:00 2001 From: Jiang Bohan Date: Wed, 22 Apr 2026 16:39:19 +0800 Subject: [PATCH] fix(analytics): preserve super-props on reset + cover overlay/login pageviews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two blockers from PR review: 1. `posthog.reset()` wipes persisted super-properties, so after logout or account switch the next session's events silently dropped `client_type` and `app_version` until a full reload. Cache the set at init time and re-register it inside `resetAnalytics()` so the breakdown survives the auth transition. Added unit tests to pin the invariant. 2. Desktop `PageviewTracker` only watched the active tab path, which missed pre-workspace overlays (`/onboarding`, `/workspaces/new`, `/invite/`) — those aren't tab routes on desktop — and also missed the logged-out `/login` state. Move the tracker to the app root and derive the visible path from `(user, overlay, activeTabPath)` with overlay > tab precedence so the `$pageview` stream matches the surface the user actually sees. --- apps/desktop/src/renderer/src/App.tsx | 12 ++- .../src/components/desktop-layout.tsx | 2 - .../src/components/pageview-tracker.tsx | 56 ++++++++++-- packages/core/analytics/index.test.ts | 88 +++++++++++++++++++ packages/core/analytics/index.ts | 21 +++-- 5 files changed, 161 insertions(+), 18 deletions(-) create mode 100644 packages/core/analytics/index.test.ts diff --git a/apps/desktop/src/renderer/src/App.tsx b/apps/desktop/src/renderer/src/App.tsx index 853751a57..7e43ee21a 100644 --- a/apps/desktop/src/renderer/src/App.tsx +++ b/apps/desktop/src/renderer/src/App.tsx @@ -10,6 +10,7 @@ import { MulticaIcon } from "@multica/ui/components/common/multica-icon"; import { Toaster } from "sonner"; import { DesktopLoginPage } from "./pages/login"; import { DesktopShell } from "./components/desktop-layout"; +import { PageviewTracker } from "./components/pageview-tracker"; import { UpdateNotification } from "./components/update-notification"; import { useTabStore } from "./stores/tab-store"; import { useWindowOverlayStore } from "./stores/window-overlay-store"; @@ -160,8 +161,15 @@ function AppContent() { ); } - if (!user) return ; - return ; + // Pageview tracker sits at the app root so it covers every visible + // surface (login, overlays, tab paths) — mounting it inside DesktopShell + // would miss the logged-out and overlay states. + return ( + <> + + {user ? : } + + ); } // Backend the daemon should connect to — same URL the renderer talks to. diff --git a/apps/desktop/src/renderer/src/components/desktop-layout.tsx b/apps/desktop/src/renderer/src/components/desktop-layout.tsx index d2d54250c..15f488a8f 100644 --- a/apps/desktop/src/renderer/src/components/desktop-layout.tsx +++ b/apps/desktop/src/renderer/src/components/desktop-layout.tsx @@ -20,7 +20,6 @@ import { DesktopNavigationProvider } from "@/platform/navigation"; import { TabBar } from "./tab-bar"; import { TabContent } from "./tab-content"; import { WindowOverlay } from "./window-overlay"; -import { PageviewTracker } from "./pageview-tracker"; function SidebarTopBar() { const { canGoBack, canGoForward, goBack, goForward } = useTabHistory(); @@ -110,7 +109,6 @@ export function DesktopShell() { return ( - {/* WorkspaceSlugProvider accepts null — components that need slug use useWorkspaceSlug() (nullable) or useRequiredWorkspaceSlug() (throws). TabContent MUST always render so the tab router can diff --git a/apps/desktop/src/renderer/src/components/pageview-tracker.tsx b/apps/desktop/src/renderer/src/components/pageview-tracker.tsx index ae8667ea0..abaf2b901 100644 --- a/apps/desktop/src/renderer/src/components/pageview-tracker.tsx +++ b/apps/desktop/src/renderer/src/components/pageview-tracker.tsx @@ -1,20 +1,35 @@ import { useEffect } from "react"; import { capturePageview } from "@multica/core/analytics"; +import { useAuthStore } from "@multica/core/auth"; import { useTabStore } from "@/stores/tab-store"; +import { useWindowOverlayStore, type WindowOverlay } from "@/stores/window-overlay-store"; /** - * Fires a PostHog $pageview whenever the visible (active) tab's path changes. - * Desktop routing lives in per-tab memory routers, so the user's "current - * URL" is whichever path the active tab is on — the tab store keeps that in - * sync via `useTabRouterSync`. Switching tabs or workspaces is a page - * transition from the user's perspective and fires too. + * Fires a PostHog $pageview whenever the user's visible surface changes. + * + * Desktop has three layers that can own the visible page: + * + * 1. Logged-out state → `/login`. No workspace context, no tabs. + * 2. Window overlays (onboarding, new-workspace, invite) → synthetic paths + * that match the equivalent web routes. Overlays are NOT tab routes on + * desktop (see `stores/window-overlay-store.ts` + `routes.tsx`), so the + * tab path alone would either miss them or mislabel them as "/". + * 3. Otherwise → the active tab's path (workspace-scoped, e.g. + * `/acme/issues/123`). Kept in sync by `useTabRouterSync`. + * + * The overlay takes precedence over the tab path because it is visually in + * front of the tab system; the logged-out state shadows both because the + * shell doesn't render at all yet. This keeps the `$pageview` stream aligned + * with what the user actually sees. * * PostHog's `capture_pageview: true` auto-capture is intentionally off (see * `initAnalytics`) so this component owns the event shape, matching the web * implementation in `apps/web/components/pageview-tracker.tsx`. */ export function PageviewTracker() { - const activePath = useTabStore((s) => { + const user = useAuthStore((s) => s.user); + const overlay = useWindowOverlayStore((s) => s.overlay); + const activeTabPath = useTabStore((s) => { const slug = s.activeWorkspaceSlug; if (!slug) return null; const group = s.byWorkspace[slug]; @@ -22,10 +37,33 @@ export function PageviewTracker() { return group.tabs.find((t) => t.id === group.activeTabId)?.path ?? null; }); + const path = resolvePath(user, overlay, activeTabPath); + useEffect(() => { - if (!activePath) return; - capturePageview(activePath); - }, [activePath]); + if (!path) return; + capturePageview(path); + }, [path]); return null; } + +function resolvePath( + user: unknown, + overlay: WindowOverlay | null, + activeTabPath: string | null, +): string | null { + if (!user) return "/login"; + if (overlay) return overlayPath(overlay); + return activeTabPath; +} + +function overlayPath(overlay: WindowOverlay): string { + switch (overlay.type) { + case "new-workspace": + return "/workspaces/new"; + case "onboarding": + return "/onboarding"; + case "invite": + return `/invite/${overlay.invitationId}`; + } +} diff --git a/packages/core/analytics/index.test.ts b/packages/core/analytics/index.test.ts new file mode 100644 index 000000000..90ffe3ae3 --- /dev/null +++ b/packages/core/analytics/index.test.ts @@ -0,0 +1,88 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// Mock posthog-js before importing the module under test so the module's +// top-level `import posthog from "posthog-js"` resolves to the mock. +vi.mock("posthog-js", () => { + const mock = { + init: vi.fn(), + register: vi.fn(), + reset: vi.fn(), + identify: vi.fn(), + capture: vi.fn(), + }; + return { default: mock }; +}); + +// Re-import per test so module-level `initialized` / cached super-props +// don't leak between cases. +async function loadModule() { + vi.resetModules(); + const analytics = await import("./index"); + const posthog = (await import("posthog-js")).default as unknown as { + init: ReturnType; + register: ReturnType; + reset: ReturnType; + }; + posthog.init.mockClear(); + posthog.register.mockClear(); + posthog.reset.mockClear(); + return { analytics, posthog }; +} + +beforeEach(() => { + vi.stubGlobal("window", {}); + vi.stubGlobal("navigator", { userAgent: "Mozilla/5.0" }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("initAnalytics super-properties", () => { + it("registers client_type and app_version after posthog.init", async () => { + const { analytics, posthog } = await loadModule(); + analytics.initAnalytics({ key: "k", host: "", appVersion: "1.2.3" }); + expect(posthog.register).toHaveBeenCalledWith({ + client_type: "web", + app_version: "1.2.3", + }); + }); + + it("omits app_version when not provided", async () => { + const { analytics, posthog } = await loadModule(); + analytics.initAnalytics({ key: "k", host: "" }); + expect(posthog.register).toHaveBeenCalledWith({ client_type: "web" }); + }); + + it("detects desktop when window.electron is present", async () => { + vi.stubGlobal("window", { electron: {} }); + const { analytics, posthog } = await loadModule(); + analytics.initAnalytics({ key: "k", host: "" }); + expect(posthog.register).toHaveBeenCalledWith({ client_type: "desktop" }); + }); +}); + +describe("resetAnalytics", () => { + it("re-registers super-properties after reset so subsequent events keep client_type", async () => { + const { analytics, posthog } = await loadModule(); + analytics.initAnalytics({ key: "k", host: "", appVersion: "1.2.3" }); + posthog.register.mockClear(); + + analytics.resetAnalytics(); + + // reset() wipes persisted super-props; we re-register the cached set so + // the next session's events keep client_type + app_version. + expect(posthog.reset).toHaveBeenCalledTimes(1); + expect(posthog.register).toHaveBeenCalledWith({ + client_type: "web", + app_version: "1.2.3", + }); + }); + + it("is a no-op when analytics was never initialized", async () => { + const { analytics, posthog } = await loadModule(); + analytics.resetAnalytics(); + expect(posthog.reset).not.toHaveBeenCalled(); + expect(posthog.register).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/analytics/index.ts b/packages/core/analytics/index.ts index 7a81ab9b0..34e04a749 100644 --- a/packages/core/analytics/index.ts +++ b/packages/core/analytics/index.ts @@ -39,6 +39,11 @@ let pendingIdentify: { userId: string; props?: Record } | null // config fetch resolves. We keep the first pending pageview so that step // doesn't silently drop. let pendingPageview: string | undefined | null = null; +// Cached super-properties so resetAnalytics() can re-register them after +// posthog.reset() wipes the persisted set. Without this, logout / account +// switch silently drops client_type + app_version from every subsequent +// event until a full reload. +let superProperties: Record = {}; export interface AnalyticsConfig { key: string; @@ -112,11 +117,11 @@ export function initAnalytics(config: AnalyticsConfig | null | undefined): boole // (PostHog's own `$lib` reports "web" for both because Electron renderers // are Chromium). `app_version` is optional so self-hosted or local dev // builds without a version don't pollute the property. - const superProps: Record = { - client_type: detectClientType(), - }; - if (config.appVersion) superProps.app_version = config.appVersion; - posthog.register(superProps); + // We cache the set so resetAnalytics() can re-apply it after + // posthog.reset() — reset() clears persisted super-properties otherwise. + superProperties = { client_type: detectClientType() }; + if (config.appVersion) superProperties.app_version = config.appVersion; + posthog.register(superProperties); initialized = true; // Flush any identify() that arrived before init resolved. @@ -158,6 +163,12 @@ export function resetAnalytics(): void { pendingPageview = null; if (!initialized) return; posthog.reset(); + // reset() wipes persisted super-properties too, so re-register the ones + // set at init time. Otherwise every event after logout / account-switch + // would be missing client_type + app_version until a full reload. + if (Object.keys(superProperties).length > 0) { + posthog.register(superProperties); + } } /**