Skip to content
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.

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

AB tests followup #16243

Merged
merged 3 commits into from
Jan 14, 2025
Merged
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
6 changes: 6 additions & 0 deletions docs/features/message-system.md
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,12 @@ Structure of config, types and optionality of specific keys can be found in the
"desktop": ">=24.5.1",
"mobile": "!",
"web": ">=24.5.1"
},
// It's very useful to define a duration for each experiment that starts the next day at the earliest,
// to minimize changing the application under the hands of those who have it open at the moment.
"duration": {
"from": "2021-03-01T12:10:00.000Z",
"to": "2022-01-31T12:10:00.000Z"
}
}
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import { ReactElement } from 'react';

import { ExperimentNameType } from '@suite-common/message-system';

import { useExperiment } from 'src/hooks/experiment/useExperiment';
import { useExperiment, ExperimentId } from '@suite-common/message-system';

interface ExperimentWrapperProps {
id: ExperimentNameType;
id: ExperimentId;
components: Array<{
variant: string;
element: ReactElement;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('Message system middleware', () => {
message3,
message4,
]);
jest.spyOn(messageSystemUtils, 'getValidExperiments').mockImplementation(() => []);
jest.spyOn(messageSystemUtils, 'getValidExperimentIds').mockImplementation(() => []);

const store = initStore(getInitialState(undefined, undefined));
store.dispatch({
Expand Down Expand Up @@ -128,7 +128,7 @@ describe('Message system middleware', () => {

it('saves messages even if there are no valid messages', () => {
jest.spyOn(messageSystemUtils, 'getValidMessages').mockImplementation(() => []);
jest.spyOn(messageSystemUtils, 'getValidExperiments').mockImplementation(() => []);
jest.spyOn(messageSystemUtils, 'getValidExperimentIds').mockImplementation(() => []);

const store = initStore(getInitialState(undefined, undefined));
store.dispatch({
Expand Down Expand Up @@ -168,8 +168,8 @@ describe('Message system middleware', () => {
],
};

jest.spyOn(messageSystemUtils, 'getValidExperiments').mockImplementation(() => [
experiment1,
jest.spyOn(messageSystemUtils, 'getValidExperimentIds').mockImplementation(() => [
experiment1.id,
]);

const store = initStore(getInitialState(undefined, undefined));
Expand Down
19 changes: 7 additions & 12 deletions packages/suite/src/middlewares/suite/messageSystemMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
messageSystemActions,
categorizeMessages,
getValidMessages,
getValidExperiments,
getValidExperimentIds,
} from '@suite-common/message-system';

import { SUITE } from 'src/actions/suite/constants';
Expand Down Expand Up @@ -38,27 +38,22 @@ const messageSystemMiddleware =
const device = selectSelectedDevice(api.getState());
const { enabledNetworks } = api.getState().wallet.settings;

const validMessages = getValidMessages(config, {
const validationParams = {
device,
transports,
settings: {
tor: getIsTorEnabled(torStatus),
enabledNetworks,
},
});
};

const validMessages = getValidMessages(config, validationParams);
const categorizedValidMessages = categorizeMessages(validMessages);

const validExperiments = getValidExperiments(config, {
device,
transports,
settings: {
tor: getIsTorEnabled(torStatus),
enabledNetworks,
},
}).map(item => item.id);
const validExperimentIds = getValidExperimentIds(config, validationParams);

api.dispatch(messageSystemActions.updateValidMessages(categorizedValidMessages));
api.dispatch(messageSystemActions.updateValidExperiments(validExperiments));
api.dispatch(messageSystemActions.updateValidExperiments(validExperimentIds));
}

return action;
Expand Down
2 changes: 2 additions & 0 deletions suite-common/message-system/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
"fs-extra": "^11.2.0",
"json-schema-to-typescript": "^13.1.2",
"jws": "^4.0.0",
"react": "18.2.0",
"react-redux": "8.0.7",
"semver": "^7.6.3"
},
"devDependencies": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { getWeakRandomId } from '@trezor/utils';

import { ExperimentIdType } from '../experiments';
import { ExperimentId } from '../messageSystemTypes';

// getWeakRandomId is also used for generating instanceId
export const getArrayOfInstanceIds = (count: number) =>
Array.from({ length: count }, () => getWeakRandomId(10));

export const experimentTest = {
id: 'e2e8d05f-1469-4e47-9ab0-53544e5cad07' as ExperimentIdType,
id: 'e2e8d05f-1469-4e47-9ab0-53544e5cad07' as ExperimentId,
groups: [
{
variant: 'A',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1945,9 +1945,9 @@ export const validateExperiments = [
},
];

export const getValidExperiments = [
export const getValidExperimentIds = [
{
description: 'getValidExperiments - case 1',
description: 'getValidExperimentIds - case 1',
currentDate: '2021-04-01T12:10:00.000Z',
userAgent:
'Mozilla/5.0 (Macintosh; Intel Mac OS X 11_1_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.90 Safari/537.36',
Expand All @@ -1957,7 +1957,7 @@ export const getValidExperiments = [
config: getMessageSystemConfig(),
result: [
...(getMessageSystemConfig().experiments ?? []).map(
experiment => experiment.experiment,
experiment => experiment.experiment.id,
),
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import {
getExperimentGroupByInclusion,
getInclusionFromInstanceId,
selectActiveExperimentGroup,
} from '../';
import { experimentTest, getArrayOfInstanceIds } from '../__fixtures__';
import { ExperimentIdType } from '../experiments';
} from '../experimentUtils';
import { experimentTest, getArrayOfInstanceIds } from '../__fixtures__/experimentUtils';
import { ExperimentId } from '../messageSystemTypes';

describe('testing experiment utils', () => {
const experimentId = 'e2e8d05f-1469-4e47-9ab0-53544e5cad07' as ExperimentIdType;
const experimentId = 'e2e8d05f-1469-4e47-9ab0-53544e5cad07' as ExperimentId;

it('test getInclusionFromInstanceId whether returns percentage between 0 and 99', () => {
const arrayOfIds = getArrayOfInstanceIds(100);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ describe('Message system utils', () => {
});
});

describe('getValidExperiments', () => {
describe('getValidExperimentIds', () => {
let userAgentGetter: any;
const OLD_ENV = { ...process.env };

Expand All @@ -140,7 +140,7 @@ describe('Message system utils', () => {
process.env = OLD_ENV;
});

fixtures.getValidExperiments.forEach(f => {
fixtures.getValidExperimentIds.forEach(f => {
it(f.description, () => {
jest.spyOn(Date, 'now').mockImplementation(() => new Date(f.currentDate).getTime());
// @ts-expect-error (getOsName returns union of string literals)
Expand All @@ -151,7 +151,7 @@ describe('Message system utils', () => {
process.env.VERSION = f.suiteVersion;

// @ts-expect-error
expect(messageSystem.getValidExperiments(f.config, f.options)).toEqual(f.result);
expect(messageSystem.getValidExperimentIds(f.config, f.options)).toEqual(f.result);
});
});
});
Expand Down
6 changes: 0 additions & 6 deletions suite-common/message-system/src/experiment/experiments.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
import { createHash } from 'crypto';

import { ExperimentsItem } from '@suite-common/suite-types';

import { ExperimentIdType } from './experiments';

export type ExperimentsItemUuidType = Omit<ExperimentsItem, 'id'> & { id: ExperimentIdType };
import { ExperimentId, ExperimentsItemType } from './messageSystemTypes';

type ExperimentCategoriesProps = {
experiment: ExperimentsItemUuidType | undefined;
experiment: ExperimentsItemType | undefined;
instanceId: string | undefined;
};

type ExperimentsGroupsType = ExperimentsItemUuidType['groups'];
type ExperimentsGroupsType = ExperimentsItemType['groups'];
type ExperimentsGroupType = ExperimentsGroupsType[number];

type ExperimentGetGroupByInclusion = {
Expand All @@ -22,7 +18,7 @@ type ExperimentGetGroupByInclusion = {
/**
* @returns number between 0 and 99 generated from instanceId and experimentId
*/
export const getInclusionFromInstanceId = (instanceId: string, experimentId: ExperimentIdType) => {
export const getInclusionFromInstanceId = (instanceId: string, experimentId: ExperimentId) => {
const combinedId = `${instanceId}-${experimentId}`;
const hash = createHash('sha256').update(combinedId).digest('hex').slice(0, 8);

Expand Down
4 changes: 2 additions & 2 deletions suite-common/message-system/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ export * from './messageSystemThunks';
export * from './messageSystemTypes';
export * from './messageSystemUtils';

export * from './experiment';
export * from './experiment/experiments';
export * from './experimentUtils';
export * from './useExperiment';
14 changes: 9 additions & 5 deletions suite-common/message-system/src/messageSystemSelectors.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { createWeakMapSelector, returnStableArrayIfEmpty } from '@suite-common/redux-utils';
import { Message, Category } from '@suite-common/suite-types';
import { ExperimentId } from '@suite-common/message-system';

import { ContextDomain, FeatureDomain, MessageSystemRootState } from './messageSystemTypes';
import { ExperimentIdType } from './experiment/experiments';
import { ExperimentsItemUuidType } from './experiment';
import {
ContextDomain,
ExperimentsItemType,
FeatureDomain,
MessageSystemRootState,
} from './messageSystemTypes';

// Create app-specific selectors with correct types
export const createMemoizedSelector = createWeakMapSelector.withTypes<MessageSystemRootState>();
Expand Down Expand Up @@ -167,9 +171,9 @@ export const selectAllValidExperiments = createMemoizedSelector(
},
);

export const selectExperimentById = (id: ExperimentIdType) =>
export const selectExperimentById = (id: ExperimentId) =>
createMemoizedSelector([selectAllValidExperiments], allValidExperiments =>
allValidExperiments.find(
(experiment): experiment is ExperimentsItemUuidType => experiment.id === id,
(experiment): experiment is ExperimentsItemType => experiment.id === id,
),
);
10 changes: 9 additions & 1 deletion suite-common/message-system/src/messageSystemTypes.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { MessageSystem, Category } from '@suite-common/suite-types';
import { ExperimentsItem, MessageSystem, Category } from '@suite-common/suite-types';

export type MessageState = { [key in Category]: boolean };

Expand Down Expand Up @@ -35,3 +35,11 @@ export const Context = {
} as const;

export type ContextDomain = (typeof Context)[keyof typeof Context];

export const Experiment = {
// e.g. orangeSendButton: 'fb0eb1bc-8ec3-44d4-98eb-53301d73d981',
} as const;

export type ExperimentId = (typeof Experiment)[keyof typeof Experiment];

export type ExperimentsItemType = Omit<ExperimentsItem, 'id'> & { id: ExperimentId };
8 changes: 2 additions & 6 deletions suite-common/message-system/src/messageSystemUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import type {
Device,
Environment,
Condition,
ExperimentsItem,
} from '@suite-common/suite-types';
import type { NetworkSymbol } from '@suite-common/wallet-config';
import type { TransportInfo } from '@trezor/connect';
Expand Down Expand Up @@ -289,10 +288,7 @@ export const getValidMessages = (config: MessageSystem | null, options: Options)
.map(action => action.message);
};

export const getValidExperiments = (
config: MessageSystem | null,
options: Options,
): ExperimentsItem[] => {
export const getValidExperimentIds = (config: MessageSystem | null, options: Options): string[] => {
if (!config?.experiments) {
return [];
}
Expand All @@ -303,5 +299,5 @@ export const getValidExperiments = (
!experiment.conditions.length ||
experiment.conditions.some(condition => validateConditions(condition, options)),
)
.map(experiment => experiment.experiment);
.map(experiment => experiment?.experiment?.id);
};
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import { useMemo } from 'react';
import { useSelector } from 'react-redux';

import { selectAnalyticsInstanceId } from '@suite-common/analytics';
import {
ExperimentNameType,
experiments,
selectActiveExperimentGroup,
selectExperimentById,
} from '@suite-common/message-system';

import { useSelector } from 'src/hooks/suite';
import { selectActiveExperimentGroup } from './experimentUtils';
import { selectExperimentById } from './messageSystemSelectors';
import { ExperimentId } from './messageSystemTypes';

export const useExperiment = (id: ExperimentNameType) => {
const experimentUuid = experiments[id];
export const useExperiment = (experimentId: ExperimentId) => {
const instanceId = useSelector(selectAnalyticsInstanceId);
const experiment = useSelector(selectExperimentById(experimentUuid));
const experiment = useSelector(selectExperimentById(experimentId));
const activeExperimentVariant = useMemo(
() => selectActiveExperimentGroup({ instanceId, experiment }),
[instanceId, experiment],
Expand Down
9 changes: 7 additions & 2 deletions suite-native/message-system/src/messageSystemMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
categorizeMessages,
getValidMessages,
selectMessageSystemConfig,
getValidExperimentIds,
} from '@suite-common/message-system';
import { deviceActions, selectSelectedDevice } from '@suite-common/wallet-core';
import {
Expand All @@ -30,17 +31,21 @@ export const messageSystemMiddleware = createMiddleware((action, { next, dispatc
const device = selectSelectedDevice(getState());
const enabledNetworks = selectDeviceEnabledDiscoveryNetworkSymbols(getState());

const validMessages = getValidMessages(config, {
const validationParams = {
device,
settings: {
tor: false, // not supported in suite-native
enabledNetworks,
},
});
};

const validMessages = getValidMessages(config, validationParams);
const categorizedValidMessages = categorizeMessages(validMessages);

const validExperimentIds = getValidExperimentIds(config, validationParams);

dispatch(messageSystemActions.updateValidMessages(categorizedValidMessages));
dispatch(messageSystemActions.updateValidExperiments(validExperimentIds));
}

return action;
Expand Down
2 changes: 2 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9610,6 +9610,8 @@ __metadata:
fs-extra: "npm:^11.2.0"
json-schema-to-typescript: "npm:^13.1.2"
jws: "npm:^4.0.0"
react: "npm:18.2.0"
react-redux: "npm:8.0.7"
semver: "npm:^7.6.3"
tsx: "npm:^4.16.3"
languageName: unknown
Expand Down
Loading