Skip to content

feat: Add telemetry tracking to DismissibleUpsell component #8309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 25, 2025
Merged

feat: Add telemetry tracking to DismissibleUpsell component #8309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ecbf671
feat: add telemetry tracking to DismissibleUpsell component
roomote Sep 25, 2025
0e88233
refactor(webview): make handleDismiss synchronous in DismissibleUpsell
roomote Sep 25, 2025
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
5 changes: 5 additions & 0 deletions packages/types/src/telemetry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ export enum TelemetryEventName {
ACCOUNT_LOGOUT_CLICKED = "Account Logout Clicked",
ACCOUNT_LOGOUT_SUCCESS = "Account Logout Success",

UPSELL_DISMISSED = "Upsell Dismissed",
UPSELL_CLICKED = "Upsell Clicked",

SCHEMA_VALIDATION_ERROR = "Schema Validation Error",
DIFF_APPLICATION_ERROR = "Diff Application Error",
SHELL_INTEGRATION_ERROR = "Shell Integration Error",
Expand Down Expand Up @@ -181,6 +184,8 @@ export const rooCodeTelemetryEventSchema = z.discriminatedUnion("type", [
TelemetryEventName.ACCOUNT_CONNECT_SUCCESS,
TelemetryEventName.ACCOUNT_LOGOUT_CLICKED,
TelemetryEventName.ACCOUNT_LOGOUT_SUCCESS,
TelemetryEventName.UPSELL_DISMISSED,
TelemetryEventName.UPSELL_CLICKED,
TelemetryEventName.SCHEMA_VALIDATION_ERROR,
TelemetryEventName.DIFF_APPLICATION_ERROR,
TelemetryEventName.SHELL_INTEGRATION_ERROR,
Expand Down
16 changes: 15 additions & 1 deletion webview-ui/src/components/common/DismissibleUpsell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { memo, ReactNode, useEffect, useState, useRef } from "react"
import { vscode } from "@src/utils/vscode"
import { useAppTranslation } from "@src/i18n/TranslationContext"
import { telemetryClient } from "@src/utils/TelemetryClient"
import { TelemetryEventName } from "@roo-code/types"

interface DismissibleUpsellProps {
/** Required unique identifier for this upsell */
Expand Down Expand Up @@ -76,7 +78,12 @@ const DismissibleUpsell = memo(
}
}, [upsellId])

const handleDismiss = async () => {
const handleDismiss = () => {
// Track telemetry for dismissal
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: handleDismiss is marked async but doesn’t await anything. Making it synchronous avoids misleading consumers and minor overhead.

Suggested change
// Track telemetry for dismissal
const handleDismiss = () => {

telemetryClient.capture(TelemetryEventName.UPSELL_DISMISSED, {
upsellId: upsellId,
})

// First notify the extension to persist the dismissal
// This ensures the message is sent even if the component unmounts quickly
vscode.postMessage({
Expand Down Expand Up @@ -134,6 +141,13 @@ const DismissibleUpsell = memo(
<div
className={containerClasses}
onClick={() => {
// Track telemetry for click
if (onClick) {
telemetryClient.capture(TelemetryEventName.UPSELL_CLICKED, {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Intentional behavior: UPSELL_CLICKED is emitted only when onClick exists. If analytics should also capture container-dismiss without onClick (dismissOnClick=true), consider adding a separate event or documenting this decision.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, let's make sure to track these as dismissals.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if Roomote implemented this change, it doesn't seem like it did

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be right, wdyt?

upsellId: upsellId,
})
}

// Call the onClick handler if provided
onClick?.()
// Also dismiss if dismissOnClick is true
Expand Down
74 changes: 71 additions & 3 deletions webview-ui/src/components/common/__tests__/DismissibleUpsell.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { render, screen, fireEvent, waitFor, act } from "@testing-library/react"
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
import DismissibleUpsell from "../DismissibleUpsell"
import { TelemetryEventName } from "@roo-code/types"

// Mock the vscode API
const mockPostMessage = vi.fn()
Expand All @@ -10,6 +11,14 @@ vi.mock("@src/utils/vscode", () => ({
},
}))

// Mock telemetryClient
const mockCapture = vi.fn()
vi.mock("@src/utils/TelemetryClient", () => ({
telemetryClient: {
capture: (eventName: string, properties?: Record<string, any>) => mockCapture(eventName, properties),
},
}))

// Mock the translation hook
vi.mock("@src/i18n/TranslationContext", () => ({
useAppTranslation: () => ({
Expand All @@ -26,6 +35,7 @@ vi.mock("@src/i18n/TranslationContext", () => ({
describe("DismissibleUpsell", () => {
beforeEach(() => {
mockPostMessage.mockClear()
mockCapture.mockClear()
vi.clearAllTimers()
})

Expand Down Expand Up @@ -72,7 +82,7 @@ describe("DismissibleUpsell", () => {
})
})

it("hides the upsell when dismiss button is clicked", async () => {
it("hides the upsell when dismiss button is clicked and tracks telemetry", async () => {
const onDismiss = vi.fn()
const { container } = render(
<DismissibleUpsell upsellId="test-upsell" onDismiss={onDismiss}>
Expand All @@ -92,6 +102,11 @@ describe("DismissibleUpsell", () => {
const dismissButton = screen.getByRole("button", { name: /dismiss/i })
fireEvent.click(dismissButton)

// Check that telemetry was tracked
expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_DISMISSED, {
upsellId: "test-upsell",
})

// Check that the dismiss message was sent BEFORE hiding
expect(mockPostMessage).toHaveBeenCalledWith({
type: "dismissUpsell",
Expand Down Expand Up @@ -351,7 +366,7 @@ describe("DismissibleUpsell", () => {
})
})

it("calls onClick when the container is clicked", async () => {
it("calls onClick when the container is clicked and tracks telemetry", async () => {
const onClick = vi.fn()
render(
<DismissibleUpsell upsellId="test-upsell" onClick={onClick}>
Expand All @@ -372,6 +387,11 @@ describe("DismissibleUpsell", () => {
fireEvent.click(container)

expect(onClick).toHaveBeenCalledTimes(1)

// Check that telemetry was tracked
expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_CLICKED, {
upsellId: "test-upsell",
})
})

it("does not call onClick when dismiss button is clicked", async () => {
Expand Down Expand Up @@ -470,7 +490,7 @@ describe("DismissibleUpsell", () => {
})
})

it("dismisses when clicked if dismissOnClick is true", async () => {
it("dismisses when clicked if dismissOnClick is true and tracks both telemetry events", async () => {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Add a test for the scenario where dismissOnClick is true and no onClick is provided. Expect only UPSELL_DISMISSED to be captured (and not UPSELL_CLICKED) to lock in the intended behavior.

const onClick = vi.fn()
const onDismiss = vi.fn()
const { container } = render(
Expand All @@ -493,6 +513,14 @@ describe("DismissibleUpsell", () => {
expect(onClick).toHaveBeenCalledTimes(1)
expect(onDismiss).toHaveBeenCalledTimes(1)

// Check that both telemetry events were tracked
expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_CLICKED, {
upsellId: "test-upsell",
})
expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_DISMISSED, {
upsellId: "test-upsell",
})

expect(mockPostMessage).toHaveBeenCalledWith({
type: "dismissUpsell",
upsellId: "test-upsell",
Expand All @@ -503,6 +531,46 @@ describe("DismissibleUpsell", () => {
})
})

it("dismisses on container click when dismissOnClick is true and no onClick is provided; tracks only dismissal", async () => {
const onDismiss = vi.fn()
const { container } = render(
<DismissibleUpsell upsellId="test-upsell" onDismiss={onDismiss} dismissOnClick={true}>
<div>Test content</div>
</DismissibleUpsell>,
)

// Make component visible
makeUpsellVisible()

// Wait for component to be visible
await waitFor(() => {
expect(screen.getByText("Test content")).toBeInTheDocument()
})

// Click on the container (not the dismiss button)
const containerDiv = screen.getByText("Test content").parentElement as HTMLElement
fireEvent.click(containerDiv)

// onDismiss should be called
expect(onDismiss).toHaveBeenCalledTimes(1)

// Telemetry: only dismissal should be tracked
expect(mockCapture).toHaveBeenCalledWith(TelemetryEventName.UPSELL_DISMISSED, {
upsellId: "test-upsell",
})
expect(mockCapture).not.toHaveBeenCalledWith(TelemetryEventName.UPSELL_CLICKED, expect.anything())

// Dismiss message should be sent
expect(mockPostMessage).toHaveBeenCalledWith({
type: "dismissUpsell",
upsellId: "test-upsell",
})

// Component should be hidden
await waitFor(() => {
expect(container.firstChild).toBeNull()
})
})
it("does not dismiss when clicked if dismissOnClick is false", async () => {
const onClick = vi.fn()
const onDismiss = vi.fn()
Expand Down
Loading