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
4 changes: 2 additions & 2 deletions products/desktop/packages/shared/src/analytics-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -642,8 +642,8 @@ export type InboxReportOpenMethod =
export type InboxReportCloseMethod =
| "next_report"
| "deselected"
| "navigated_away"
| "unmount";
| "unmount"
| "page_unload";

export type InboxReportActionType =
| "dismiss"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import type { SignalReport } from "@posthog/shared/types";
import { renderHook } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";

const mockTrack = vi.hoisted(() => vi.fn());

vi.mock("@posthog/ui/shell/analytics", () => ({ track: mockTrack }));

vi.mock("@posthog/ui/features/inbox/hooks/useInboxAllReports", () => ({
useInboxAllReports: () => ({ scopedReports: [] }),
}));

import { useReportOpenTracker } from "./useReportOpenTracker";

function report(id: string): SignalReport {
return {
id,
title: `Report ${id}`,
summary: null,
status: "ready",
total_weight: 1,
signal_count: 1,
created_at: "2024-01-01T00:00:00Z",
updated_at: "2024-01-01T00:00:00Z",
artefact_count: 0,
implementation_pr_url: null,
} as SignalReport;
}

function closeCalls(): Array<[string, Record<string, unknown>, unknown]> {
return mockTrack.mock.calls.filter(
(call) => call[0] === "Inbox report closed",
) as Array<[string, Record<string, unknown>, unknown]>;
}

describe("useReportOpenTracker", () => {
beforeEach(() => {
vi.clearAllMocks();
});

it("flushes the close on pagehide and does not fire it again on unmount", () => {
const { unmount } = renderHook(() =>
useReportOpenTracker(report("r1"), "reports"),
);

window.dispatchEvent(new Event("pagehide"));
unmount();

expect(closeCalls()).toHaveLength(1);
const [, properties, options] = closeCalls()[0];
expect(properties).toMatchObject({
report_id: "r1",
close_method: "page_unload",
});
// The unload flush must leave before the page goes.
expect(options).toEqual({ send_instantly: true });
});

it("labels an in-app unmount close `unmount`", () => {
const { unmount } = renderHook(() =>
useReportOpenTracker(report("r1"), "reports"),
);

unmount();

expect(closeCalls()).toHaveLength(1);
expect(closeCalls()[0][1]).toMatchObject({
report_id: "r1",
close_method: "unmount",
});
});

it("labels a report鈫抮eport switch close `next_report`", () => {
const { rerender, unmount } = renderHook(
({ id }: { id: string }) => useReportOpenTracker(report(id), "reports"),
{ initialProps: { id: "r1" } },
);

rerender({ id: "r2" });

expect(closeCalls()).toHaveLength(1);
expect(closeCalls()[0][1]).toMatchObject({
report_id: "r1",
close_method: "next_report",
});

// The switch resets the default, so the next teardown is a plain unmount.
unmount();
expect(closeCalls()[1][1]).toMatchObject({
report_id: "r2",
close_method: "unmount",
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { InboxReportCloseMethod } from "@posthog/shared/analytics-events";
import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events";
import type { SignalReport } from "@posthog/shared/types";
import { useInboxAllReports } from "@posthog/ui/features/inbox/hooks/useInboxAllReports";
import type { TrackOptions } from "@posthog/ui/shell/analytics";
import { track } from "@posthog/ui/shell/analytics";
import { useEffect, useRef } from "react";

Expand Down Expand Up @@ -78,11 +79,11 @@ export function useReportOpenTracker(
reportRef.current = report;

// Detect a report鈫抮eport switch during render so the close cleanup can label
// it `next_report` rather than `navigated_away`. Writing a ref during render
// it `next_report` rather than `unmount`. Writing a ref during render
// is the React-sanctioned "track the previous prop" pattern, and crucially it
// runs before the outgoing effect's cleanup, which is where we read it.
const renderedIdRef = useRef<string | null>(null);
const closeMethodRef = useRef<InboxReportCloseMethod>("navigated_away");
const closeMethodRef = useRef<InboxReportCloseMethod>("unmount");
if (renderedIdRef.current !== null && renderedIdRef.current !== report.id) {
closeMethodRef.current = "next_report";
}
Expand Down Expand Up @@ -110,19 +111,47 @@ export function useReportOpenTracker(
});
lastOpenedReportId = opened.id;

// Emit the dwell-time close once. The effect cleanup (in-app teardown or a
// report switch) and the `pagehide` flush (tab/window close) both call this,
// so whichever fires first wins and the other is a no-op.
let closed = false;
const close = (
closeMethod: InboxReportCloseMethod,
options?: TrackOptions,
): void => {
if (closed) {
return;
}
closed = true;
track(
ANALYTICS_EVENTS.INBOX_REPORT_CLOSED,
{
report_id: opened.id,
report_title: opened.title ?? null,
report_age_hours: reportAgeHours(opened.created_at),
priority: opened.priority ?? null,
actionability: opened.actionability ?? null,
time_spent_ms: Date.now() - openedAt,
scrolled: false,
close_method: closeMethod,
},
options,
);
};

// A tab or window close never unmounts the detail route, so the cleanup
// below never runs and the close is lost. Flush on `pagehide` with
// `send_instantly` so the event leaves before the page goes. Mirrors the
// cloud inbox logic.
const onPageHide = (): void =>
close("page_unload", { send_instantly: true });
window.addEventListener("pagehide", onPageHide);

return () => {
track(ANALYTICS_EVENTS.INBOX_REPORT_CLOSED, {
report_id: opened.id,
report_title: opened.title ?? null,
report_age_hours: reportAgeHours(opened.created_at),
priority: opened.priority ?? null,
actionability: opened.actionability ?? null,
time_spent_ms: Date.now() - openedAt,
scrolled: false,
close_method: closeMethodRef.current,
});
window.removeEventListener("pagehide", onPageHide);
close(closeMethodRef.current);
// Reset to the exit default; a subsequent switch re-sets it during render.
closeMethodRef.current = "navigated_away";
closeMethodRef.current = "unmount";
};
}, [report.id]);
}
9 changes: 7 additions & 2 deletions products/desktop/packages/ui/src/shell/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,17 @@ import type {
} from "@posthog/shared/analytics-events";
import type { Task } from "@posthog/shared/domain-types";

/** Per-call capture options. `send_instantly` bypasses batching so an event fired during page unload leaves before the page goes. */
export interface TrackOptions {
send_instantly?: boolean;
}

type TrackArgs<K extends keyof EventPropertyMap> =
EventPropertyMap[K] extends never
? []
: EventPropertyMap[K] extends undefined
? [properties?: EventPropertyMap[K]]
: [properties: EventPropertyMap[K]];
? [properties?: EventPropertyMap[K], options?: TrackOptions]
: [properties: EventPropertyMap[K], options?: TrackOptions];

export interface AnalyticsUserGroups {
team?: { id: number; uuid: string; name: string } | null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ describe("track", () => {
expect(mockPosthog.capture).toHaveBeenCalledWith(
ANALYTICS_EVENTS.SIGNAL_SOURCE_CONNECTED,
expect.objectContaining({ inbox_client: "code" }),
undefined,
);
});

Expand All @@ -154,6 +155,7 @@ describe("track", () => {
expect(mockPosthog.capture).toHaveBeenCalledWith(
ANALYTICS_EVENTS.PROMPT_HISTORY_OPENED,
expect.not.objectContaining({ inbox_client: expect.anything() }),
undefined,
);
});

Expand Down
16 changes: 11 additions & 5 deletions products/desktop/packages/ui/src/shell/posthogAnalyticsImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type { PermissionRequest } from "@posthog/ui/features/sessions/sessionLog
import type {
AnalyticsTracker,
AnalyticsUserGroups,
TrackOptions,
} from "@posthog/ui/shell/analytics";
import { logger } from "@posthog/ui/shell/logger";

Expand Down Expand Up @@ -283,20 +284,25 @@ export function track<K extends keyof EventPropertyMap>(
...args: EventPropertyMap[K] extends never
? []
: EventPropertyMap[K] extends undefined
? [properties?: EventPropertyMap[K]]
: [properties: EventPropertyMap[K]]
? [properties?: EventPropertyMap[K], options?: TrackOptions]
: [properties: EventPropertyMap[K], options?: TrackOptions]
) {
if (!isInitialized) {
return;
}

const [rawProperties, options] = args as [
EventPropertyMap[K]?,
TrackOptions?,
];

// Stamp inbox events with the client discriminator. Spread first so a caller
// could override it, matching posthog's inboxAnalytics.ts (none do today).
const properties = isInboxAnalyticsEvent(eventName)
? { inbox_client: INBOX_CLIENT, ...args[0] }
: args[0];
? { inbox_client: INBOX_CLIENT, ...rawProperties }
: rawProperties;

posthog.capture(eventName, properties);
posthog.capture(eventName, properties, options);
}

/**
Expand Down
Loading