Skip to content

Commit

Permalink
[Analytics][Interactive Graph] Adding new events for interactive grap…
Browse files Browse the repository at this point in the history
…h and migration to team-internal events. (#2205)

## Summary:
Updating analytics events to align with changes to event-schema. This includes:
- Updating schema versions with new analtyics metrics like, message, userAgent, and widgetSubType.
- Adding Team Internal events to allow our team to switch over to team controlled events, especially for PerseusErrorOccurred.

Issue: LEMS-2534

## Test plan:

Author: catandthemachines

Reviewers: catandthemachines, nishasy, SonicScrewdriver, jeremywiebe, benchristel, handeyeco

Required Reviewers:

Approved By: SonicScrewdriver

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x)

Pull Request URL: #2205
  • Loading branch information
catandthemachines authored Feb 13, 2025
1 parent 91cede4 commit ae29e2b
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 3 deletions.
6 changes: 6 additions & 0 deletions .changeset/forty-spiders-pretend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": patch
"@khanacademy/perseus-core": patch
---

Updating perseus analytics and events for better metrics.
38 changes: 38 additions & 0 deletions packages/perseus-core/src/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,28 @@ export type PerseusAnalyticsEvent =
type: "perseus:expression-focused";
payload: null;
}
// TODO(LEMS-2826): Remove this error type in LEMS-2826
| {
type: "perseus:widget-rendering-error";
payload: {
widgetSubType: string;
widgetType: string;
widgetId: string;
message: string;
userAgent: string;
};
}
| {
type: "perseus:widget-rendering-error:ti";
payload: {
widgetSubType: string;
widgetType: string;
widgetId: string;
message: string;
userAgent: string;
};
}
// TODO(LEMS-2827): Remove this error type in LEMS-2827
| {
type: "perseus:interactive-graph-widget:rendered";
payload: {
Expand All @@ -31,18 +46,41 @@ export type PerseusAnalyticsEvent =
widgetId: string;
};
}
| {
type: "perseus:widget:rendered:ti";
payload: {
widgetSubType: string;
widgetType: string;
widgetId: string;
};
}
// TODO(LEMS-2831): Remove this error type in LEMS-2831
| {
type: "perseus:label-image:toggle-answers-hidden";
payload: null;
}
// TODO(LEMS-2830): Remove this error type in LEMS-2830
| {
type: "perseus:label-image:marker-interacted-with";
payload: null;
}
// TODO(LEMS-2829): Remove this error type in LEMS-2829
| {
type: "perseus:label-image:choiced-interacted-with";
payload: null;
}
| {
type: "perseus:label-image:toggle-answers-hidden:ti";
payload: null;
}
| {
type: "perseus:label-image:marker-interacted-with:ti";
payload: null;
}
| {
type: "perseus:label-image:choiced-interacted-with:ti";
payload: null;
}
| {
type: "math-input:keypad-closed";
payload: {
Expand Down
18 changes: 17 additions & 1 deletion packages/perseus/src/__tests__/widget-container.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ describe("widget-container", () => {

it("should send analytics even when widget rendering errors", () => {
// Arrange
jest.spyOn(window.navigator, "userAgent", "get").mockReturnValue(
"userAgent",
);
jest.spyOn(Dependencies, "getDependencies").mockReturnValue(
testDependencies,
);
Expand Down Expand Up @@ -120,11 +123,24 @@ describe("widget-container", () => {
);

// Assert
expect(onAnalyticsEventSpy).toHaveBeenCalledWith({
expect(onAnalyticsEventSpy).toHaveBeenNthCalledWith(1, {
type: "perseus:widget-rendering-error",
payload: {
widgetSubType: "null",
widgetType: "mock-widget",
widgetId: "mock-widget 1",
message: "MockWidget failed to render",
userAgent: "userAgent",
},
});
expect(onAnalyticsEventSpy).toHaveBeenNthCalledWith(2, {
type: "perseus:widget-rendering-error:ti",
payload: {
widgetSubType: "null",
widgetType: "mock-widget",
widgetId: "mock-widget 1",
message: "MockWidget failed to render",
userAgent: "userAgent",
},
});
});
Expand Down
1 change: 1 addition & 0 deletions packages/perseus/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ export {
} from "./util/test-utils";
export {
getWidgetTypeByWidgetId,
getWidgetSubTypeByWidgetId,
contentHasWidgetType,
getWidgetsMapFromItemData,
getWidgetFromWidgetMap,
Expand Down
25 changes: 24 additions & 1 deletion packages/perseus/src/widget-container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ class WidgetContainer extends React.Component<Props, State> {
});

const type = this.props.type;
const userAgent = navigator.userAgent;

const WidgetType = Widgets.getWidget(type);
if (WidgetType == null) {
// This is for the good of all people!!
Expand All @@ -117,6 +119,13 @@ class WidgetContainer extends React.Component<Props, State> {
return <div className={className} />;
}

let subType = "null";
if (type === "interactive-graph") {
const props = this.state.widgetProps;

subType = props.graph?.type ?? "null";
}

let alignment = this.state.widgetProps.alignment;
if (alignment === "default") {
alignment = CoreWidgetRegistry.getDefaultAlignment(type);
Expand Down Expand Up @@ -174,12 +183,26 @@ class WidgetContainer extends React.Component<Props, State> {
widget_type: type,
widget_id: this.props.id,
}}
onError={() => {
onError={(error: Error) => {
// TODO(LEMS-2826): Remove analytics event in LEMS-2826 in favor of ti below.
analytics.onAnalyticsEvent({
type: "perseus:widget-rendering-error",
payload: {
widgetSubType: subType,
widgetType: type,
widgetId: this.props.id,
message: error.message,
userAgent: userAgent,
},
});
analytics.onAnalyticsEvent({
type: "perseus:widget-rendering-error:ti",
payload: {
widgetSubType: subType,
widgetType: type,
widgetId: this.props.id,
message: error.message,
userAgent: userAgent,
},
});
}}
Expand Down
18 changes: 18 additions & 0 deletions packages/perseus/src/widget-type-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
type PerseusItem,
type PerseusWidget,
type PerseusWidgetsMap,
type PerseusGraphType,
} from "@khanacademy/perseus-core";

/**
Expand All @@ -21,6 +22,23 @@ export function getWidgetTypeByWidgetId(
return widget?.type ?? null;
}

export function getWidgetSubTypeByWidgetId(
widgetId: string,
widgetMap: PerseusWidgetsMap,
): string | null {
const widget = widgetMap[widgetId];
const widgetType = widget?.type ?? null;

switch (widgetType) {
case "interactive-graph":
const graph: PerseusGraphType = widget.options.graph;

return graph?.type ?? null;
default:
return widgetType;
}
}

/**
* Does the content have a specific type of widget?
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import {vec} from "mafs";
import React from "react";
import invariant from "tiny-invariant";

import {testDependencies} from "../../../../../testing/test-dependencies";
import {
testDependencies,
testDependenciesV2,
} from "../../../../../testing/test-dependencies";
import * as Dependencies from "../../dependencies";

import {calculateNestedSVGCoords, MafsGraph} from "./mafs-graph";
Expand All @@ -14,6 +17,7 @@ import {getBaseMafsGraphPropsForTests} from "./utils";

import type {MafsGraphProps} from "./mafs-graph";
import type {InteractiveGraphState} from "./types";
import type {PerseusDependenciesV2} from "@khanacademy/perseus";
import type {GraphRange} from "@khanacademy/perseus-core";
import type {UserEvent} from "@testing-library/user-event";

Expand Down Expand Up @@ -107,6 +111,61 @@ describe("MafsGraph", () => {
expect(line.getAttribute("y2")).toBe(-expectedY2 + "");
});

it("should send analytics even when widget is rendered", () => {
const mockDispatch = jest.fn();
const state: InteractiveGraphState = {
type: "segment",
hasBeenInteractedWith: true,
range: [
[-10, 10],
[-10, 10],
],
snapStep: [0.5, 0.5],
coords: [
[
[0, 0],
[-7, 0.5],
],
],
};

const onAnalyticsEventSpy = jest.fn();
const depsV2: PerseusDependenciesV2 = {
...testDependenciesV2,
analytics: {onAnalyticsEvent: onAnalyticsEventSpy},
};

const baseMafsGraphProps = getBaseMafsGraphPropsForTests();

render(
<Dependencies.DependenciesContext.Provider value={depsV2}>
<MafsGraph
{...baseMafsGraphProps}
state={state}
dispatch={mockDispatch}
/>
</Dependencies.DependenciesContext.Provider>,
);

// Assert
expect(onAnalyticsEventSpy).toHaveBeenNthCalledWith(1, {
type: "perseus:interactive-graph-widget:rendered",
payload: {
type: "segment",
widgetType: "INTERACTIVE_GRAPH",
widgetId: "interactive-graph",
},
});
expect(onAnalyticsEventSpy).toHaveBeenNthCalledWith(2, {
type: "perseus:widget:rendered:ti",
payload: {
widgetSubType: "segment",
widgetType: "INTERACTIVE_GRAPH",
widgetId: "interactive-graph",
},
});
});

it("renders TeX in axis Labels", () => {
const basePropsWithTexLabels = {
...getBaseMafsGraphPropsForTests(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,22 @@ export const MafsGraph = (props: MafsGraphProps) => {

useOnMountEffect(() => {
analytics.onAnalyticsEvent({
// TODO(LEMS-2827): Remove analytics event in LEMS-2827 in favor of ti below.
type: "perseus:interactive-graph-widget:rendered",
payload: {
type,
widgetType: "INTERACTIVE_GRAPH",
widgetId: "interactive-graph",
},
});
analytics.onAnalyticsEvent({
type: "perseus:widget:rendered:ti",
payload: {
widgetSubType: type,
widgetType: "INTERACTIVE_GRAPH",
widgetId: "interactive-graph",
},
});
});

const {graph, interactiveElementsDescription} = renderGraphElements({
Expand Down
16 changes: 16 additions & 0 deletions packages/perseus/src/widgets/label-image/label-image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,17 @@ export class LabelImage
}

activateMarker(index: number, opened: boolean) {
// TODO(LEMS-2830): Remove analytics event in LEMS-2830 in favor of ti below.
this.props.analytics?.onAnalyticsEvent({
type: "perseus:label-image:marker-interacted-with",
payload: null,
});

this.props.analytics?.onAnalyticsEvent({
type: "perseus:label-image:marker-interacted-with:ti",
payload: null,
});

const {activeMarkerIndex} = this.state;
// Set index of opened marker
if (activeMarkerIndex !== index && opened) {
Expand Down Expand Up @@ -531,10 +537,15 @@ export class LabelImage
}))}
multipleSelect={this.props.multipleAnswers}
onChange={(selection) => {
// TODO(LEMS-2829): Remove analytics event in LEMS-2829 in favor of ti below.
this.props.analytics?.onAnalyticsEvent({
type: "perseus:label-image:choiced-interacted-with",
payload: null,
});
this.props.analytics?.onAnalyticsEvent({
type: "perseus:label-image:choiced-interacted-with:ti",
payload: null,
});
this.handleAnswerChoicesChangeForMarker(
index,
selection,
Expand Down Expand Up @@ -667,10 +678,15 @@ export class LabelImage
<HideAnswersToggle
areAnswersHidden={this.state.hideAnswers}
onChange={(hideAnswers) => {
// TODO(LEMS-2831): Remove analytics event in LEMS-2831 in favor of ti below.
this.props.analytics?.onAnalyticsEvent({
type: "perseus:label-image:toggle-answers-hidden",
payload: null,
});
this.props.analytics?.onAnalyticsEvent({
type: "perseus:label-image:toggle-answers-hidden:ti",
payload: null,
});
this.setState({hideAnswers});
}}
/>
Expand Down

0 comments on commit ae29e2b

Please sign in to comment.