diff --git a/products/desktop/packages/shared/src/analytics-events.ts b/products/desktop/packages/shared/src/analytics-events.ts index d5c3b2eaf9b3..15a158d28f40 100644 --- a/products/desktop/packages/shared/src/analytics-events.ts +++ b/products/desktop/packages/shared/src/analytics-events.ts @@ -642,8 +642,8 @@ export type InboxReportOpenMethod = export type InboxReportCloseMethod = | "next_report" | "deselected" - | "navigated_away" - | "unmount"; + | "unmount" + | "page_unload"; export type InboxReportActionType = | "dismiss" diff --git a/products/desktop/packages/ui/src/features/inbox/hooks/useReportOpenTracker.test.tsx b/products/desktop/packages/ui/src/features/inbox/hooks/useReportOpenTracker.test.tsx new file mode 100644 index 000000000000..1948ad6edd92 --- /dev/null +++ b/products/desktop/packages/ui/src/features/inbox/hooks/useReportOpenTracker.test.tsx @@ -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, unknown]> { + return mockTrack.mock.calls.filter( + (call) => call[0] === "Inbox report closed", + ) as Array<[string, Record, 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→report 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", + }); + }); +}); diff --git a/products/desktop/packages/ui/src/features/inbox/hooks/useReportOpenTracker.ts b/products/desktop/packages/ui/src/features/inbox/hooks/useReportOpenTracker.ts index 578bc51c164a..15aa83e952cf 100644 --- a/products/desktop/packages/ui/src/features/inbox/hooks/useReportOpenTracker.ts +++ b/products/desktop/packages/ui/src/features/inbox/hooks/useReportOpenTracker.ts @@ -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"; @@ -78,11 +79,11 @@ export function useReportOpenTracker( reportRef.current = report; // Detect a report→report 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(null); - const closeMethodRef = useRef("navigated_away"); + const closeMethodRef = useRef("unmount"); if (renderedIdRef.current !== null && renderedIdRef.current !== report.id) { closeMethodRef.current = "next_report"; } @@ -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]); } diff --git a/products/desktop/packages/ui/src/shell/analytics.ts b/products/desktop/packages/ui/src/shell/analytics.ts index 011ddc64e960..6fbe5c6f8568 100644 --- a/products/desktop/packages/ui/src/shell/analytics.ts +++ b/products/desktop/packages/ui/src/shell/analytics.ts @@ -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 = 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; diff --git a/products/desktop/packages/ui/src/shell/posthogAnalyticsImpl.test.ts b/products/desktop/packages/ui/src/shell/posthogAnalyticsImpl.test.ts index 45b2b84892c6..13e467c2e818 100644 --- a/products/desktop/packages/ui/src/shell/posthogAnalyticsImpl.test.ts +++ b/products/desktop/packages/ui/src/shell/posthogAnalyticsImpl.test.ts @@ -142,6 +142,7 @@ describe("track", () => { expect(mockPosthog.capture).toHaveBeenCalledWith( ANALYTICS_EVENTS.SIGNAL_SOURCE_CONNECTED, expect.objectContaining({ inbox_client: "code" }), + undefined, ); }); @@ -154,6 +155,7 @@ describe("track", () => { expect(mockPosthog.capture).toHaveBeenCalledWith( ANALYTICS_EVENTS.PROMPT_HISTORY_OPENED, expect.not.objectContaining({ inbox_client: expect.anything() }), + undefined, ); }); diff --git a/products/desktop/packages/ui/src/shell/posthogAnalyticsImpl.ts b/products/desktop/packages/ui/src/shell/posthogAnalyticsImpl.ts index 8b58b9872be1..d24a76160680 100644 --- a/products/desktop/packages/ui/src/shell/posthogAnalyticsImpl.ts +++ b/products/desktop/packages/ui/src/shell/posthogAnalyticsImpl.ts @@ -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"; @@ -283,20 +284,25 @@ export function track( ...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); } /**