Skip to content

Commit

Permalink
Create a transformer to enforce current terminology in reportEvent wi…
Browse files Browse the repository at this point in the history
…thout breaking Mixpanel events (#8774)

* wip

* fix remaining ts errors

* block/service -> brick/integration

* fix merge issues

* renaming

* renaming

* fix tests

* remove unused export

* improve typing

* refactoring

* bump logging database number

* add telemetryTypes to tsconfig.strictNullChecks

* pr review related changes
  • Loading branch information
grahamlangford authored Jul 9, 2024
1 parent 713af70 commit 6c441b8
Show file tree
Hide file tree
Showing 87 changed files with 408 additions and 290 deletions.
6 changes: 3 additions & 3 deletions src/__snapshots__/Storyshots.test.js.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/activation/useActivateMod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ type ActivationSource = "marketplace" | "extensionConsole";

function selectActivateEventData(modDefinition: ModDefinition) {
return {
blueprintId: modDefinition.metadata.id,
extensions: modDefinition.extensionPoints.map((x) => x.label),
modId: modDefinition.metadata.id,
modComponents: modDefinition.extensionPoints.map((x) => x.label),
};
}

Expand Down
4 changes: 2 additions & 2 deletions src/background/executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ const runBrickMock = jest.mocked(runBrick);
const optionsFactory = define<RemoteBrickOptions>({
ctxt: () => ({}),
messageContext: (i: number) => ({
extensionId: uuidSequence(i),
modComponentId: uuidSequence(i),
}),
meta: derive<RemoteBrickOptions, RemoteBrickOptions["meta"]>(
(options) => ({
extensionId: options.messageContext!.extensionId,
extensionId: options.messageContext!.modComponentId,
runId: null,
branches: [],
}),
Expand Down
2 changes: 1 addition & 1 deletion src/background/messenger/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export const recordLog = getNotifier("RECORD_LOG", bg);
export const clearLogs = getMethod("CLEAR_LOGS", bg);
export const clearLog = getMethod("CLEAR_LOG", bg);
export const clearExtensionDebugLogs = getMethod(
"CLEAR_EXTENSION_DEBUG_LOGS",
"CLEAR_MOD_COMPONENT_DEBUG_LOGS",
bg,
);

Expand Down
6 changes: 3 additions & 3 deletions src/background/messenger/registration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import * as packageRegistry from "@/registry/packageRegistry";
import integrationRegistry from "@/integrations/registry";
import { getUserData } from "@/auth/authStorage";
import {
clearExtensionDebugLogs,
clearModComponentDebugLogs,
clearLog,
clearLogs,
recordError,
Expand Down Expand Up @@ -110,7 +110,7 @@ declare global {
RECORD_ERROR: typeof recordError;
CLEAR_LOGS: typeof clearLogs;
CLEAR_LOG: typeof clearLog;
CLEAR_EXTENSION_DEBUG_LOGS: typeof clearExtensionDebugLogs;
CLEAR_MOD_COMPONENT_DEBUG_LOGS: typeof clearModComponentDebugLogs;

INTEGRATION_REGISTRY_CLEAR: typeof integrationRegistry.clear;
LOCATOR_FIND_ALL_SANITIZED_CONFIGS_FOR_INTEGRATION: typeof integrationConfigLocator.findAllSanitizedConfigsForIntegration;
Expand Down Expand Up @@ -183,7 +183,7 @@ export default function registerMessenger(): void {
RECORD_ERROR: recordError,
CLEAR_LOGS: clearLogs,
CLEAR_LOG: clearLog,
CLEAR_EXTENSION_DEBUG_LOGS: clearExtensionDebugLogs,
CLEAR_MOD_COMPONENT_DEBUG_LOGS: clearModComponentDebugLogs,

INTEGRATION_REGISTRY_CLEAR:
integrationRegistry.clear.bind(integrationRegistry),
Expand Down
2 changes: 1 addition & 1 deletion src/background/removeModComponentForEveryTab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,5 @@ export async function removeModComponentForEveryTab(
});
await uninstallContextMenu({ extensionId: modComponentId });
await clearModComponentTraces(modComponentId);
await clearLog({ extensionId: modComponentId });
await clearLog({ modComponentId });
}
4 changes: 2 additions & 2 deletions src/background/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,8 @@ async function getIntegrationMessageContext(
}

return {
serviceId: config.serviceId,
serviceVersion: resolvedIntegration?.version,
integrationId: config.serviceId,
integrationVersion: resolvedIntegration?.version,
authId: config.id,
};
}
Expand Down
8 changes: 4 additions & 4 deletions src/background/telemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
TEST_flushAll,
} from "@/background/telemetry";
import { appApiMock } from "@/testUtils/appApiMock";
import { type Event } from "@/telemetry/events";
import { type TelemetryEvent } from "@/telemetry/telemetryTypes";

const EXPECTED_RUNTIME_ID = "abc123";
const expectedManifestValues = {
Expand Down Expand Up @@ -53,13 +53,13 @@ beforeEach(async () => {

describe("recordEvent", () => {
test("runs", async () => {
await recordEvent({ event: "TestEvent" as Event, data: {} });
await recordEvent({ event: "TestEvent" as TelemetryEvent, data: {} });
const events = await flushEvents();
expect(events).toHaveLength(1);
});

test("includes expected default properties", async () => {
const testEvent = { event: "TestEvent" as Event, data: {} };
const testEvent = { event: "TestEvent" as TelemetryEvent, data: {} };
await recordEvent(testEvent);
const events = await flushEvents();
expect(events[0]).toMatchObject({
Expand All @@ -76,7 +76,7 @@ describe("recordEvent", () => {
test("successfully persists concurrent telemetry events to local storage", async () => {
// Easiest way to test race condition without having to mock
const recordTestEvents = Array.from({ length: 100 }, async () =>
recordEvent({ event: "TestEvent" as Event, data: {} }),
recordEvent({ event: "TestEvent" as TelemetryEvent, data: {} }),
);
await Promise.all(recordTestEvents);

Expand Down
4 changes: 2 additions & 2 deletions src/background/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { count as logSize } from "@/telemetry/logging";
import { count as traceSize } from "@/telemetry/trace";
import { getUUID } from "@/telemetry/telemetryHelpers";
import { getExtensionVersion, getTabsWithAccess } from "@/utils/extensionUtils";
import { type Event } from "@/telemetry/events";
import { type TelemetryEvent } from "@/telemetry/telemetryTypes";

const EVENT_BUFFER_DEBOUNCE_MS = 2000;
const EVENT_BUFFER_MAX_MS = 10_000;
Expand Down Expand Up @@ -333,7 +333,7 @@ export async function recordEvent({
event,
data = {},
}: {
event: Event;
event: TelemetryEvent;
data: UnknownObject | undefined;
}): Promise<void> {
if (await allowsTrack()) {
Expand Down
14 changes: 7 additions & 7 deletions src/bricks/effects/AddDynamicTextSnippet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ describe("AddDynamicTextSnippet", () => {
async (shortcut) => {
const extensionId = uuidv4();
const logger = new ConsoleLogger({
extensionId,
blueprintId: registryIdFactory(),
modComponentId: extensionId,
modId: registryIdFactory(),
});

const pipeline = {
Expand Down Expand Up @@ -81,8 +81,8 @@ describe("AddDynamicTextSnippet", () => {
componentId: extensionId,
context: {
...logger.context,
blockId: brick.id,
blockVersion: expect.toBeString(),
brickId: brick.id,
brickVersion: expect.toBeString(),
label: brick.name,
},
},
Expand All @@ -98,7 +98,7 @@ describe("AddDynamicTextSnippet", () => {
"passes preview directly: %s",
async (preview) => {
const extensionId = uuidv4();
const logger = new ConsoleLogger({ extensionId });
const logger = new ConsoleLogger({ modComponentId: extensionId });

const pipeline = {
id: brick.id,
Expand Down Expand Up @@ -127,8 +127,8 @@ describe("AddDynamicTextSnippet", () => {
componentId: extensionId,
context: {
...logger.context,
blockId: brick.id,
blockVersion: expect.toBeString(),
brickId: brick.id,
brickVersion: expect.toBeString(),
label: brick.name,
},
},
Expand Down
4 changes: 2 additions & 2 deletions src/bricks/effects/AddDynamicTextSnippet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,15 @@ class AddDynamicTextSnippet extends EffectABC {
return;
}

if (logger.context.extensionId == null) {
if (logger.context.modComponentId == null) {
throw new Error("Must be run in the context of a mod");
}

// Counter to keep track of the action run number for tracing
let counter = 0;

platform.snippetShortcutMenu.register({
componentId: logger.context.extensionId,
componentId: logger.context.modComponentId,
context: logger.context,
// Trim leading command key in shortcut to be resilient to user input
shortcut: normalizeShortcut(shortcut),
Expand Down
4 changes: 2 additions & 2 deletions src/bricks/effects/AddQuickBarAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,11 @@ class AddQuickBarAction extends EffectABC {
let counter = 0;

// Expected parent id from QuickBarProviderExtensionPoint
const parentId = `provider-${logger.context.extensionId}`;
const parentId = `provider-${logger.context.modComponentId}`;

const action: CustomAction = {
// XXX: old actions will still appear in the quick bar unless the extension point clears out the old actions
id: `${logger.context.extensionId}-${title}`,
id: `${logger.context.modComponentId}-${title}`,
// Additional metadata, for enabling clearing out old actions
modComponentRef: mapMessageContextToModComponentRef(logger.context),
// Can only provide a parent if the parent exists
Expand Down
2 changes: 1 addition & 1 deletion src/bricks/effects/AddTextSnippets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe("AddTextSnippets", () => {
title: "Test",
preview: "test text",
handler: expect.toBeFunction(),
componentId: options.logger.context.extensionId,
componentId: options.logger.context.modComponentId,
context: options.logger.context,
},
]);
Expand Down
4 changes: 2 additions & 2 deletions src/bricks/effects/AddTextSnippets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,13 @@ class AddTextSnippets extends EffectABC {
return;
}

if (logger.context.extensionId == null) {
if (logger.context.modComponentId == null) {
throw new Error("Must be run in the context of a mod component");
}

for (const { shortcut, title, text } of snippets) {
platform.snippetShortcutMenu.register({
componentId: logger.context.extensionId,
componentId: logger.context.modComponentId,
context: logger.context,
shortcut: normalizeShortcut(shortcut),
title,
Expand Down
4 changes: 2 additions & 2 deletions src/bricks/effects/assignModVariable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ const modId = validateRegistryId("test/123");
const brick = new AssignModVariable();

const logger = new ConsoleLogger({
extensionId: modComponentId,
blueprintId: modId,
modComponentId,
modId,
});

const brickOptions = brickOptionsFactory({ logger });
Expand Down
2 changes: 1 addition & 1 deletion src/bricks/effects/assignModVariable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class AssignModVariable extends EffectABC {
}>,
{ logger }: BrickOptions,
): Promise<void> {
const { blueprintId: modId, extensionId: modComponentId } = logger.context;
const { modId, modComponentId } = logger.context;

setState({
namespace: StateNamespaces.MOD,
Expand Down
2 changes: 1 addition & 1 deletion src/bricks/effects/attachAutocomplete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { brickOptionsFactory } from "@/testUtils/factories/runtimeFactories";
const brick = new AttachAutocomplete();

const logger = new ConsoleLogger({
extensionId: uuidSequence(0),
modComponentId: uuidSequence(0),
});

describe("AttachAutocomplete", () => {
Expand Down
2 changes: 1 addition & 1 deletion src/bricks/effects/customEvent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { brickOptionsFactory } from "@/testUtils/factories/runtimeFactories";
const brick = new CustomEventEffect();

const logger = new ConsoleLogger({
extensionId: uuidSequence(0),
modComponentId: uuidSequence(0),
});

describe("CustomEventEffect", () => {
Expand Down
18 changes: 9 additions & 9 deletions src/bricks/effects/pageState.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ describe("@pixiebrix/state/get", () => {
test("default to blueprint state", async () => {
const brick = new GetPageState();
const logger = new ConsoleLogger({
extensionId: uuidv4(),
blueprintId: validateRegistryId("test/123"),
modComponentId: uuidv4(),
modId: validateRegistryId("test/123"),
});
await brick.transform(
unsafeAssumeValidArg({}),
Expand All @@ -53,8 +53,8 @@ describe("@pixiebrix/state/set", () => {
test("shallow merge", async () => {
const brick = new SetPageState();
const logger = new ConsoleLogger({
extensionId: uuidv4(),
blueprintId: validateRegistryId("test/123"),
modComponentId: uuidv4(),
modId: validateRegistryId("test/123"),
});

let result = await brick.transform(
Expand All @@ -78,8 +78,8 @@ describe("@pixiebrix/state/set", () => {

const brick = new SetPageState();
const logger = new ConsoleLogger({
extensionId: uuidv4(),
blueprintId: validateRegistryId("test/123"),
modComponentId: uuidv4(),
modId: validateRegistryId("test/123"),
});

const original = {
Expand Down Expand Up @@ -164,8 +164,8 @@ describe("set and get", () => {
const setState = new SetPageState();
const getState = new GetPageState();
const logger = new ConsoleLogger({
extensionId: uuidv4(),
blueprintId: validateRegistryId("test/123"),
modComponentId: uuidv4(),
modId: validateRegistryId("test/123"),
});

await setState.transform(
Expand Down Expand Up @@ -196,7 +196,7 @@ describe("set and get", () => {
const setState = new SetPageState();
const getState = new GetPageState();
const logger = new ConsoleLogger({
extensionId: uuidv4(),
modComponentId: uuidv4(),
});

await setState.transform(
Expand Down
8 changes: 4 additions & 4 deletions src/bricks/effects/pageState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,15 @@ export class SetPageState extends TransformerABC {
}>,
{ logger, platform }: BrickOptions,
): Promise<JsonObject> {
const { blueprintId, extensionId } = logger.context;
const { modId, modComponentId } = logger.context;

return platform.state.setState({
namespace,
data,
mergeStrategy,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion -- TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/7891
modComponentId: extensionId!,
modId: blueprintId,
modComponentId: modComponentId!,
modId,
});
}
}
Expand Down Expand Up @@ -205,7 +205,7 @@ export class GetPageState extends TransformerABC {
}: BrickArgs<{ namespace?: StateNamespace }>,
{ logger, platform }: BrickOptions,
): Promise<JsonObject> {
const { blueprintId: modId, extensionId: modComponentId } = logger.context;
const { modId, modComponentId } = logger.context;

return platform.state.getState({
namespace,
Expand Down
2 changes: 1 addition & 1 deletion src/bricks/effects/sidebarEffects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export class ShowSidebar extends EffectABC {
sidebarController.updateSidebar({
force: forcePanel,
panelHeading,
blueprintId: logger.context.blueprintId,
blueprintId: logger.context.modId,
}),
);
}
Expand Down
Loading

0 comments on commit 6c441b8

Please sign in to comment.