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

[FSSDK-9984] fix: initialization and setUser errors #255

Merged
merged 37 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
566daaf
fix: remove successful fetch requirement for onReady
Mar 18, 2024
77c4f56
Revert "fix: remove successful fetch requirement for onReady"
Mar 19, 2024
0f190d6
fix: error with OnReadyResult being undefined
Mar 21, 2024
2fc9703
fix: setUser should use VUID if possible
Mar 21, 2024
6da3ae3
Merge branch 'master' into mike/FSSDK-9984/fix-initialization-error
Mar 21, 2024
682e8a7
revert: timeout of undefined
Mar 21, 2024
bbea7ca
docs: update copyright year
Mar 21, 2024
21348c1
revert: Provider.tsx copyright since no code change
Mar 22, 2024
cb15720
build: bump JS SDK version
Mar 25, 2024
fd5cfd1
refactor: `res` should never be `undefined`
Mar 25, 2024
ae88a38
docs: add clarifying comment
Mar 25, 2024
1c1c501
revert: retrieval & use of current user context
Mar 27, 2024
92f09fb
wip: partial solution; needs collab
Mar 28, 2024
6bc5c45
refactor: setUser logic updated
Mar 29, 2024
8e9c122
revert: move setUser back to Provider constructor
Mar 29, 2024
7d5adc6
style: remove commented code
Mar 29, 2024
39b45c0
fix: fetchQualifiedSegments under SSR/sync scenario
Mar 29, 2024
b541ff4
ci: VS Code jest settings to run via extension
Apr 1, 2024
41c51fa
test: use NotReadyReason enum
Apr 1, 2024
8164582
test: use NotReadyReason & add missing getUserId in mock user context
Apr 1, 2024
286838f
fix: add onInitStateChange for default ready result
Apr 1, 2024
425de33
fix: logic in Promise.all user & client readiness
Apr 1, 2024
81be217
refactor: isReady() to isReactClientReady()
Apr 2, 2024
2d6dd19
test: fixes for uses of getUserId() in setUser()
Apr 2, 2024
e8116eb
wip: fixing tests
Apr 3, 2024
12a5b78
wip: fixed more tests
Apr 3, 2024
08ef8de
revert: refactor of isReactClientReady()
Apr 3, 2024
bdb469c
docs: Update copyrights
Apr 3, 2024
8967c15
fix: later setUser not getting new usercontext (manual testing)
Apr 3, 2024
01e72c7
fix: PR review changes
Apr 5, 2024
569cf66
test: add initial OptimizelyProvider tests
Apr 5, 2024
b0503cf
fix: PR review changes
Apr 5, 2024
f5b9a14
docs: add clarification inline comments
Apr 5, 2024
d349a05
build: bump underlying JS SDK version to 5.3.0
Apr 8, 2024
24f0ae8
fix: add missing getProjectConfig() from JS SDK
Apr 8, 2024
0a9951c
refactor: omit getProjectConfig instead of implementing it
Apr 8, 2024
ac5075c
style: remove unused import
Apr 8, 2024
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
25 changes: 25 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"version": "0.2.0",
"configurations": [
{
"type": "node",
"name": "vscode-jest-tests.v2.react-sdk",
"request": "launch",
"args": [
"--runInBand",
"--watchAll=false",
"--testNamePattern",
"${jest.testNamePattern}",
"--runTestsByPath",
"${jest.testFile}"
],
"cwd": "${workspaceFolder}",
"console": "integratedTerminal",
"internalConsoleOptions": "neverOpen",
"program": "${workspaceFolder}/node_modules/.bin/jest",
"windows": {
"program": "${workspaceFolder}/node_modules/jest/bin/jest"
}
}
]
}
5 changes: 2 additions & 3 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"jest.autoRun": {
"onStartup": ["all-tests"]
}
"jest.runMode": "on-demand",
"jest.jestCommandLine": "~/.nvm/nvm-exec yarn test"
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"access": "public"
},
"dependencies": {
"@optimizely/optimizely-sdk": "^5.2.0",
"@optimizely/optimizely-sdk": "^5.2.1",
"hoist-non-react-statics": "^3.3.0",
"prop-types": "^15.6.2",
"utility-types": "^2.1.0 || ^3.0.0"
Expand Down
6 changes: 3 additions & 3 deletions src/Feature.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2018-2019, 2023 Optimizely
* Copyright 2018-2019, 2023-2024 Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -22,7 +22,7 @@ import { render, screen, waitFor } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import { OptimizelyProvider } from './Provider';
import { ReactSDKClient, VariableValuesObject } from './client';
import { NotReadyReason, ReactSDKClient, VariableValuesObject } from './client';
import { OptimizelyFeature } from './Feature';

describe('<OptimizelyFeature>', () => {
Expand Down Expand Up @@ -298,7 +298,7 @@ describe('<OptimizelyFeature>', () => {

// while it's waiting for onReady()
expect(container.innerHTML).toBe('');
resolver.resolve({ success: false, reason: 'fail', dataReadyPromise: Promise.resolve() });
resolver.resolve({ success: false, reason: NotReadyReason.TIMEOUT, dataReadyPromise: Promise.resolve() });

// Simulate config update notification firing after datafile fetched
await optimizelyMock.onReady().then(res => res.dataReadyPromise);
Expand Down
6 changes: 2 additions & 4 deletions src/Provider.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2022-2023, Optimizely
* Copyright 2022-2024, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -43,9 +43,7 @@ interface OptimizelyProviderState {
export class OptimizelyProvider extends React.Component<OptimizelyProviderProps, OptimizelyProviderState> {
constructor(props: OptimizelyProviderProps) {
super(props);
}

componentDidMount(): void {
this.setUserInOptimizely();
}

Expand Down Expand Up @@ -81,9 +79,9 @@ export class OptimizelyProvider extends React.Component<OptimizelyProviderProps,
finalUser = DefaultUser;
mikechu-optimizely marked this conversation as resolved.
Show resolved Hide resolved
}

// if user is a promise, setUser occurs in the then block above
if (finalUser) {
try {
await optimizely.onReady();
await optimizely.setUser(finalUser);
} catch {
logger.error('Error while trying to set user.');
Expand Down
85 changes: 63 additions & 22 deletions src/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
Expand All @@ -28,7 +28,7 @@ jest.mock('./logger', () => {

import * as optimizely from '@optimizely/optimizely-sdk';

import { createInstance, OnReadyResult, ReactSDKClient } from './client';
import { createInstance, DefaultUser, NotReadyReason, OnReadyResult, ReactSDKClient } from './client';
import { logger } from './logger';

interface MockedReactSDKClient extends ReactSDKClient {
Expand All @@ -37,6 +37,7 @@ interface MockedReactSDKClient extends ReactSDKClient {
}

describe('ReactSDKClient', () => {
const validVuid = 'vuid_8de3bb278fce47f6b000cadc1ac';
const config: optimizely.Config = {
datafile: {},
};
Expand All @@ -51,6 +52,7 @@ describe('ReactSDKClient', () => {
decideAll: jest.fn(),
decideForKeys: jest.fn(),
fetchQualifiedSegments: jest.fn(),
getUserId: jest.fn(),
setForcedDecision: jest.fn(),
removeForcedDecision: jest.fn(),
removeAllForcedDecisions: jest.fn(),
Expand Down Expand Up @@ -144,32 +146,41 @@ describe('ReactSDKClient', () => {
});

it('fulfills the returned promise with success: true when a user is set', async () => {
jest.spyOn(instance, 'fetchQualifiedSegments').mockImplementation(async () => true);
jest.spyOn(mockInnerClient, 'onReady').mockResolvedValue({ success: true });
const instance = createInstance(config);
jest.spyOn(instance, 'fetchQualifiedSegments').mockResolvedValue(true);

await instance.setUser({
id: 'user12345',
});

const result = await instance.onReady();

expect(result.success).toBe(true);
});

it('fulfills the returned promise with success: false when fetchqualifiedsegment is false', async () => {
jest.spyOn(instance, 'fetchQualifiedSegments').mockImplementation(async () => false);
jest.spyOn(mockInnerClient, 'onReady').mockResolvedValue({ success: true });
const instance = createInstance(config);
jest.spyOn(instance, 'fetchQualifiedSegments').mockResolvedValue(false);

await instance.setUser({
id: 'user12345',
});
const result = await instance.onReady();

expect(result.success).toBe(false);
});

describe('if Optimizely client is null', () => {
beforeEach(() => {
// Mocks dataReadyPromise value instead of _client = null because test initialization of instance causes dataReadyPromise to return { success: true }
// Mocks clientAndUserReadyPromise value instead of _client = null because test initialization of
// instance causes clientAndUserReadyPromise to return { success: true }
// @ts-ignore
instance.dataReadyPromise = new Promise((resolve, reject) => {
instance.clientAndUserReadyPromise = new Promise(resolve => {
resolve({
success: false,
reason: 'NO_CLIENT',
reason: NotReadyReason.NO_CLIENT,
message: 'Optimizely client failed to initialize.',
});
});
Expand All @@ -185,16 +196,20 @@ describe('ReactSDKClient', () => {

it('waits for the inner client onReady to fulfill with success = false before fulfilling the returned promise', async () => {
const mockInnerClientOnReady = jest.spyOn(mockInnerClient, 'onReady');
let resolveInnerClientOnReady: (result: OnReadyResult) => void;
let resolveInnerClientOnReady: (result: OnReadyResult) => void = () => {};
const mockReadyPromise: Promise<OnReadyResult> = new Promise(res => {
resolveInnerClientOnReady = res;
});
mockInnerClientOnReady.mockReturnValueOnce(mockReadyPromise);
const userId = 'user999';
jest.spyOn(mockOptimizelyUserContext, 'getUserId').mockReturnValue(userId);
resolveInnerClientOnReady({ success: true });

await instance.setUser({
id: 'user999',
id: userId,
});
resolveInnerClientOnReady!({ success: true });
const result = await instance.onReady();

expect(result.success).toBe(false);
});
});
Expand All @@ -206,6 +221,7 @@ describe('ReactSDKClient', () => {
resolveInnerClientOnReady = res;
});
mockInnerClientOnReady.mockReturnValueOnce(mockReadyPromise);
const instance = createInstance(config);
jest.spyOn(instance, 'fetchQualifiedSegments').mockImplementation(async () => true);
await instance.setUser({
id: 'user999',
Expand All @@ -217,50 +233,69 @@ describe('ReactSDKClient', () => {
});

describe('setUser', () => {
it('can be called with no/default user set', async () => {
const instance = createInstance(config);
jest.spyOn(mockOptimizelyUserContext, 'getUserId').mockReturnValue(validVuid);

await instance.setUser(DefaultUser);

expect(instance.user.id).toEqual(validVuid);
});

it('updates the user object with id and attributes', async () => {
const userId = 'xxfueaojfe8&86';
jest.spyOn(mockOptimizelyUserContext, 'getUserId').mockReturnValue(userId);

const instance = createInstance(config);
await instance.setUser({
id: 'xxfueaojfe8&86',
id: userId,
attributes: {
foo: 'bar',
},
});

expect(instance.user).toEqual({
id: 'xxfueaojfe8&86',
id: userId,
attributes: {
foo: 'bar',
},
});
});

it('adds and removes update handlers', async () => {
const userId = 'newUser';
jest.spyOn(mockOptimizelyUserContext, 'getUserId').mockReturnValue(userId);
const instance = createInstance(config);
const onUserUpdateListener = jest.fn();
const dispose = instance.onUserUpdate(onUserUpdateListener);

await instance.setUser({
id: 'newUser',
id: userId,
});

expect(onUserUpdateListener).toBeCalledTimes(1);
expect(onUserUpdateListener).toBeCalledWith({
id: 'newUser',
id: userId,
attributes: {},
});

dispose();
await instance.setUser({
id: 'newUser2',
});

expect(onUserUpdateListener).toBeCalledTimes(1);
});

it('does not call fetchqualifiedsegements on setUser if onready is not calleed initially', async () => {
it('implicitly calls fetchqualifiedsegements', async () => {
const instance = createInstance(config);
jest.spyOn(instance, 'fetchQualifiedSegments').mockImplementation(async () => true);
jest.spyOn(instance, 'fetchQualifiedSegments').mockResolvedValue(true);

await instance.setUser({
id: 'xxfueaojfe8&86',
});

expect(instance.fetchQualifiedSegments).toBeCalledTimes(0);
expect(instance.fetchQualifiedSegments).toBeCalledTimes(1);
});

it('calls fetchqualifiedsegements internally on each setuser call after onready', async () => {
Expand All @@ -286,9 +321,11 @@ describe('ReactSDKClient', () => {
describe('pre-set user and user overrides', () => {
let instance: ReactSDKClient;
beforeEach(async () => {
const userId = 'user1';
jest.spyOn(mockOptimizelyUserContext, 'getUserId').mockReturnValue(userId);
instance = createInstance(config);
await instance.setUser({
id: 'user1',
id: userId,
attributes: {
foo: 'bar',
},
Expand Down Expand Up @@ -1067,9 +1104,11 @@ describe('ReactSDKClient', () => {
}
}
);
const userId = 'user1123';
jest.spyOn(mockOptimizelyUserContext, 'getUserId').mockReturnValue(userId);
const instance = createInstance(config);
await instance.setUser({
id: 'user1123',
id: userId,
});
const result = instance.getFeatureVariables('feat1');
expect(result).toEqual({
Expand Down Expand Up @@ -1353,9 +1392,11 @@ describe('ReactSDKClient', () => {
describe('setForcedDecision', () => {
let instance: ReactSDKClient;
beforeEach(async () => {
const userId = 'user1';
jest.spyOn(mockOptimizelyUserContext, 'getUserId').mockReturnValue(userId);
instance = createInstance(config);
await instance.setUser({
id: 'user1',
id: userId,
attributes: {
foo: 'bar',
},
Expand All @@ -1374,7 +1415,6 @@ describe('ReactSDKClient', () => {
variables: {},
variationKey: 'varition1',
});

// @ts-ignore
instance._client = null;

Expand Down Expand Up @@ -1462,6 +1502,8 @@ describe('ReactSDKClient', () => {
describe('removeForcedDecision', () => {
let instance: ReactSDKClient;
beforeEach(async () => {
const userId = 'user1';
jest.spyOn(mockOptimizelyUserContext, 'getUserId').mockReturnValue(userId);
instance = createInstance(config);
await instance.setUser({
id: 'user1',
Expand Down Expand Up @@ -1640,7 +1682,6 @@ describe('ReactSDKClient', () => {
});

it('should return a valid vuid', async () => {
const validVuid = 'vuid_8de3bb278fce47f6b000cadc1ac';
const mockGetVuid = mockInnerClient.getVuid as jest.Mock;
mockGetVuid.mockReturnValue(validVuid);

Expand Down
Loading
Loading