From ad4d6f8757627b9e752b758735ba1e1228a06f44 Mon Sep 17 00:00:00 2001
From: Oliver Lloyd <email@oliverlloyd.com>
Date: Mon, 15 Jun 2020 09:00:07 +0100
Subject: [PATCH] Use testId in place of passing the whole test object

---
 src/ab-core.test.ts | 26 ++++++++++++++++++--------
 src/ab-core.ts      |  4 ++--
 src/types.ts        |  5 ++++-
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/ab-core.test.ts b/src/ab-core.test.ts
index c0cc4e59..1c009ea8 100644
--- a/src/ab-core.test.ts
+++ b/src/ab-core.test.ts
@@ -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();
 		});
 
@@ -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();
 		});
 
@@ -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", () => {
@@ -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();
 		});
 	});
 });
diff --git a/src/ab-core.ts b/src/ab-core.ts
index c3c8a506..c3cfe0ac 100644
--- a/src/ab-core.ts
+++ b/src/ab-core.ts
@@ -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,
 		);
 
diff --git a/src/types.ts b/src/types.ts
index f45185ce..e78f6020 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -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 = {