Skip to content
This repository has been archived by the owner on Jan 19, 2023. It is now read-only.

Commit

Permalink
Merge pull request #12 from guardian/oliver/use-variant-id
Browse files Browse the repository at this point in the history
Use testId in place of passing the whole test object
  • Loading branch information
oliverlloyd committed Jun 15, 2020
2 parents 5227c88 + ad4d6f8 commit ec2a747
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 11 deletions.
26 changes: 18 additions & 8 deletions src/ab-core.test.ts
Expand Up @@ -177,10 +177,10 @@ describe('A/B test core', () => {
// The user mvtId is 1234, which puts them into the 'control' bucket
// with two variants, as it is an even number
expect(
abTestLibDefault.isUserInVariant(DummyTest, 'control'),
abTestLibDefault.isUserInVariant(DummyTest.id, 'control'),
).toBeTruthy();
expect(
abTestLibDefault.isUserInVariant(DummyTest, 'variant'),
abTestLibDefault.isUserInVariant(DummyTest.id, 'variant'),
).toBeFalsy();
});

Expand All @@ -194,9 +194,11 @@ describe('A/B test core', () => {
});
// The user mvtId is 1235
// so the user should not in the variant bucket
expect(abTestLib.isUserInVariant(DummyTest, 'control')).toBeFalsy();
expect(
abTestLib.isUserInVariant(DummyTest, 'variant'),
abTestLib.isUserInVariant(DummyTest.id, 'control'),
).toBeFalsy();
expect(
abTestLib.isUserInVariant(DummyTest.id, 'variant'),
).toBeTruthy();
});

Expand All @@ -212,8 +214,12 @@ describe('A/B test core', () => {
});
// The user mvtId is 1234, and the test audience is 90-100%
// so the user should not be in any variants
expect(abTestLib.isUserInVariant(DummyTest, 'control')).toBeFalsy();
expect(abTestLib.isUserInVariant(DummyTest, 'variant')).toBeFalsy();
expect(
abTestLib.isUserInVariant(DummyTest.id, 'control'),
).toBeFalsy();
expect(
abTestLib.isUserInVariant(DummyTest.id, 'variant'),
).toBeFalsy();
});

test("Returns false when test can't run", () => {
Expand All @@ -225,8 +231,12 @@ describe('A/B test core', () => {
...initCoreDefaultConfig,
...{ arrayOfTestObjects: [DummyTest] },
});
expect(abTestLib.isUserInVariant(DummyTest, 'control')).toBeFalsy();
expect(abTestLib.isUserInVariant(DummyTest, 'variant')).toBeFalsy();
expect(
abTestLib.isUserInVariant(DummyTest.id, 'control'),
).toBeFalsy();
expect(
abTestLib.isUserInVariant(DummyTest.id, 'variant'),
).toBeFalsy();
});
});
});
4 changes: 2 additions & 2 deletions src/ab-core.ts
Expand Up @@ -121,10 +121,10 @@ export const initCore = (config: ConfigType): CoreAPI => {
.map((test: ABTest) => runnableTest(test)) // I will remove these comments
.find((rt: Runnable<ABTest> | null) => rt !== null) || null; // so that this API can be reviewed seperate

const isUserInVariant: CoreAPI['isUserInVariant'] = (test, variantId) =>
const isUserInVariant: CoreAPI['isUserInVariant'] = (testId, variantId) =>
allRunnableTests(arrayOfTestObjects).some(
(runnableTest: ABTest & { variantToRun: Variant }) =>
runnableTest.id === test.id &&
runnableTest.id === testId &&
runnableTest.variantToRun.id === variantId,
);

Expand Down
5 changes: 4 additions & 1 deletion src/types.ts
Expand Up @@ -17,7 +17,10 @@ export type CoreAPI = {
firstRunnableTest: (
tests: ReadonlyArray<ABTest>,
) => Runnable<ABTest> | null;
isUserInVariant: (test: ABTest, variantId: Variant['id']) => boolean;
isUserInVariant: (
testId: ABTest['id'],
variantId?: Variant['id'],
) => boolean;
};

export type OphanAPIConfig = {
Expand Down

0 comments on commit ec2a747

Please sign in to comment.