mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 13:29:44 +02:00
fix(analytics): preserve super-props on reset + cover overlay/login pageviews
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/<id>`) — 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.
This commit is contained in:
@@ -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 <DesktopLoginPage />;
|
||||
return <DesktopShell />;
|
||||
// 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 (
|
||||
<>
|
||||
<PageviewTracker />
|
||||
{user ? <DesktopShell /> : <DesktopLoginPage />}
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
// Backend the daemon should connect to — same URL the renderer talks to.
|
||||
|
||||
@@ -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 (
|
||||
<DesktopNavigationProvider>
|
||||
<PageviewTracker />
|
||||
{/* WorkspaceSlugProvider accepts null — components that need slug
|
||||
use useWorkspaceSlug() (nullable) or useRequiredWorkspaceSlug()
|
||||
(throws). TabContent MUST always render so the tab router can
|
||||
|
||||
@@ -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}`;
|
||||
}
|
||||
}
|
||||
|
||||
88
packages/core/analytics/index.test.ts
Normal file
88
packages/core/analytics/index.test.ts
Normal file
@@ -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<typeof vi.fn>;
|
||||
register: ReturnType<typeof vi.fn>;
|
||||
reset: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -39,6 +39,11 @@ let pendingIdentify: { userId: string; props?: Record<string, unknown> } | 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<string, unknown> = {};
|
||||
|
||||
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<string, unknown> = {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user