Skip to content

Commit

Permalink
AB testing client condition calculation (#297)
Browse files Browse the repository at this point in the history
Also now we know how to handle predicates in conditions
+ tests
fixes descope/etc#2333
  • Loading branch information
aviadl authored Nov 1, 2023
1 parent 5d8d33c commit 8860b56
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 10 deletions.
1 change: 1 addition & 0 deletions packages/core-js-sdk/src/sdk/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ export type Options = {
samlIdpStateId?: string;
samlIdpUsername?: string;
ssoAppId?: string;
abTestingKey?: number;
};

export type ResponseData = Record<string, any>;
Expand Down
8 changes: 6 additions & 2 deletions packages/web-component/src/lib/descope-wc/DescopeWc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from '../helpers';
import { calculateConditions, calculateCondition } from '../helpers/conditions';
import { getLastAuth, setLastAuth } from '../helpers/lastAuth';
import { getABTestingKey } from '../helpers/abTestingKey';
import { IsChanged } from '../helpers/state';
import { disableWebauthnButtons } from '../helpers/templates';
import {
Expand Down Expand Up @@ -160,6 +161,7 @@ class DescopeWc extends BaseDescopeWc {

let startScreenId: string;
let conditionInteractionId: string;
const abTestingKey = getABTestingKey();
const loginId = this.sdk.getLastUserLoginId();
const flowConfig = await this.getFlowConfig();

Expand All @@ -175,13 +177,13 @@ class DescopeWc extends BaseDescopeWc {
if (!executionId) {
if (flowConfig.conditions) {
({ startScreenId, conditionInteractionId } = calculateConditions(
{ loginId, code, token },
{ loginId, code, token, abTestingKey },
flowConfig.conditions
));
} else if (flowConfig.condition) {
({ startScreenId, conditionInteractionId } = calculateCondition(
flowConfig.condition,
{ loginId, code, token }
{ loginId, code, token, abTestingKey }
));
} else {
startScreenId = flowConfig.startScreenId;
Expand Down Expand Up @@ -219,6 +221,7 @@ class DescopeWc extends BaseDescopeWc {
ssoAppId,
...(redirectUrl && { redirectUrl }),
lastAuth: getLastAuth(loginId),
abTestingKey,
},
conditionInteractionId,
'',
Expand Down Expand Up @@ -421,6 +424,7 @@ class DescopeWc extends BaseDescopeWc {
ssoAppId,
lastAuth,
preview: this.preview,
abTestingKey,
...(redirectUrl && { redirectUrl }),
},
conditionInteractionId,
Expand Down
11 changes: 11 additions & 0 deletions packages/web-component/src/lib/helpers/abTestingKey.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const LOCAL_STORAGE_AB_TESTING_KEY = 'dls_ab_testing_id';

export const getABTestingKey = (): number => {
const abTestingKey = localStorage.getItem(LOCAL_STORAGE_AB_TESTING_KEY);
if (!abTestingKey) {
const generatedKey = Math.floor(Math.random() * 100 + 1);
localStorage.setItem(LOCAL_STORAGE_AB_TESTING_KEY, generatedKey.toString());
return generatedKey;
}
return Number(abTestingKey);
};
14 changes: 11 additions & 3 deletions packages/web-component/src/lib/helpers/conditions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ const conditionsMap: ConditionsMap = {
'is-true': (ctx) => !!ctx.token,
'is-false': (ctx) => !ctx.token,
},
abTestingKey: {
'greater-than': (ctx, predicate: number) =>
(ctx.abTestingKey || 0) > predicate,
'less-than': (ctx, predicate: number) =>
(ctx.abTestingKey || 0) < predicate,
},
};

export const calculateCondition = (
Expand All @@ -25,7 +31,9 @@ export const calculateCondition = (
if (!checkFunc) {
return {};
}
const conditionResult = checkFunc(ctx) ? condition.met : condition.unmet;
const conditionResult = checkFunc(ctx, condition.predicate)
? condition.met
: condition.unmet;
return {
startScreenId: conditionResult?.screenId,
conditionInteractionId: conditionResult?.interactionId,
Expand All @@ -37,12 +45,12 @@ export const calculateConditions = (
ctx: Context,
conditions?: ClientCondition[]
) => {
const conditionResult = conditions?.find(({ key, operator }) => {
const conditionResult = conditions?.find(({ key, operator, predicate }) => {
if (key === elseInteractionId) {
return true;
}
const checkFunc = conditionsMap[key]?.[operator];
return !!checkFunc?.(ctx);
return !!checkFunc?.(ctx, predicate);
});
return !conditionResult
? {}
Expand Down
10 changes: 8 additions & 2 deletions packages/web-component/src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export interface ClientConditionResult {
export interface ClientCondition {
operator: Operator;
key: string;
predicate?: string | number;
met: ClientConditionResult;
unmet?: ClientConditionResult;
}
Expand All @@ -118,9 +119,13 @@ export type AutoFocusOptions = true | false | 'skipFirstScreen';

export type ThemeOptions = 'light' | 'dark' | 'os';

export type Key = 'lastAuth.loginId' | 'idpInitiated' | 'externalToken';
export type Key =
| 'lastAuth.loginId'
| 'idpInitiated'
| 'externalToken'
| 'abTestingKey';

type CheckFunction = (ctx: Context) => boolean;
type CheckFunction = (ctx: Context, predicate?: string | number) => boolean;

export type ConditionsMap = {
[key in Key]: {
Expand All @@ -132,6 +137,7 @@ export interface Context {
loginId?: string;
code?: string;
token?: string;
abTestingKey?: number;
}

export interface ILogger {
Expand Down
22 changes: 19 additions & 3 deletions packages/web-component/test/descope-wc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import DescopeWc from '../src/lib/descope-wc';
import * as helpers from '../src/lib/helpers/helpers';
// eslint-disable-next-line import/no-namespace
import { generateSdkResponse } from './testUtils';
import { getABTestingKey } from '../src/lib/helpers/abTestingKey';

jest.mock('@descope/web-js-sdk');

Expand Down Expand Up @@ -97,6 +98,8 @@ Object.defineProperty(window.history, 'replaceState', {
},
});

const abTestingKey = getABTestingKey();

describe('web-component', () => {
beforeEach(() => {
configContent = {
Expand Down Expand Up @@ -540,6 +543,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
lastAuth: {},
redirectUrl: 'http://custom.url',
oidcIdpStateId: null,
Expand Down Expand Up @@ -1829,6 +1833,7 @@ describe('web-component', () => {
expect(startMock).toBeCalledWith(
'sign-in',
{
abTestingKey,
lastAuth: { authMethod: 'otp' },
oidcIdpStateId: null,
samlIdpStateId: null,
Expand Down Expand Up @@ -1876,6 +1881,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: null,
samlIdpStateId: null,
samlIdpUsername: null,
Expand Down Expand Up @@ -1962,6 +1968,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: null,
redirectAuth: undefined,
tenant: undefined,
Expand Down Expand Up @@ -2032,6 +2039,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: null,
samlIdpStateId: null,
samlIdpUsername: null,
Expand Down Expand Up @@ -2068,6 +2076,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: null,
redirectAuth: { callbackUrl: callback, codeChallenge: challenge },
tenant: undefined,
Expand Down Expand Up @@ -2102,6 +2111,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: 'abcdefgh',
samlIdpStateId: null,
samlIdpUsername: null,
Expand Down Expand Up @@ -2217,6 +2227,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: null,
samlIdpStateId: 'abcdefgh',
samlIdpUsername: null,
Expand Down Expand Up @@ -2253,6 +2264,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: null,
samlIdpStateId: 'abcdefgh',
samlIdpUsername: 'dummyUser',
Expand Down Expand Up @@ -2314,6 +2326,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: null,
samlIdpStateId: null,
samlIdpUsername: null,
Expand Down Expand Up @@ -2363,6 +2376,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: null,
samlIdpStateId: null,
samlIdpUsername: null,
Expand Down Expand Up @@ -2422,6 +2436,7 @@ describe('web-component', () => {
expect(startMock).toHaveBeenCalledWith(
'sign-in',
{
abTestingKey,
oidcIdpStateId: null,
samlIdpStateId: null,
samlIdpUsername: null,
Expand All @@ -2441,7 +2456,7 @@ describe('web-component', () => {
);
});

it('Should fetch met screen when second condition is met', async () => {
it('Should fetch met screen when second condition is met (also checks conditions with predicates)', async () => {
startMock.mockReturnValueOnce(generateSdkResponse());
localStorage.setItem(
DESCOPE_LAST_AUTH_LOCAL_STORAGE_KEY,
Expand All @@ -2465,12 +2480,13 @@ describe('web-component', () => {
},
},
{
key: 'lastAuth.loginId',
key: 'abTestingKey',
met: {
interactionId: 'gbutpyzvtgs',
screenId: 'met',
},
operator: 'not-empty',
operator: 'greater-than',
predicate: abTestingKey - 1,
unmet: {
interactionId: 'ELSE',
screenId: 'unmet',
Expand Down
1 change: 1 addition & 0 deletions packages/web-js-sdk/src/sdk/flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type Options = Pick<
| 'samlIdpUsername'
| 'ssoAppId'
| 'preview'
| 'abTestingKey'
> & {
lastAuth?: Omit<CoreSdkFlowStartArgs[1]['lastAuth'], 'loginId' | 'name'>;
};
Expand Down

0 comments on commit 8860b56

Please sign in to comment.