Skip to content

Commit

Permalink
[FSSDK-9654] fix: Default to VUID without user(id) as Provider prop (#…
Browse files Browse the repository at this point in the history
…229)

* feat: Use vuid for user.id if user? provided

* test: Add missing Optimizely.getVuid jest.fn mock

* refactor: Move set user to componentDidMount

* test: Update for rejected optimizely.onReady

* refactor: Change use sync componentDidMount() lifecycle method

* fix: remove localStorage & getVuid use

* fix: Separate responsiblities of getUserContextInstance

* refactor: getUserContextWithOverrides

* fix: Update ReactSDKClient interface

* refactor: Update warns + clean up

* refactor: DRY + moves + warn updates

* refactor: No wait fetchQualifiedSegments on setUser

* fix(user): Allow latent setting of user

* test: WIP Fixing tests

* nit: Format with Prettier

* nit: More formatting with Prettier

* test: Fix now that instantiation is now async

* fix: Remove use sync lifecycle method

* test: WIP graceful promise reject test failing

* test: WIP replace timers in tests

* test: Better timeout handling in jest

* refactor: Move client onReady check slightly

It really is needed so that vuid in JS SDK is ready

* fix: Use FeatureVariableValue; Remove excess checks; Better user assignment

* refactor: Remove FeatureVariableValue for now

* test: Fix code supported by tests

* refactor: Move getUserWithOverrides() back to position

* refactor: Move isReady() for easier PR

* refactor: Change back to getUserContext()

* refactor: Make set and make user context private
  • Loading branch information
mikechu-optimizely authored Jan 16, 2024
1 parent abf633d commit 43613fc
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 203 deletions.
15 changes: 10 additions & 5 deletions src/Experiment.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/// <reference types="jest" />

import * as React from 'react';
import { act } from 'react-dom/test-utils';
import { render, screen, waitFor } from '@testing-library/react';
Expand Down Expand Up @@ -43,10 +45,11 @@ describe('<OptimizelyExperiment>', () => {
optimizelyMock = ({
onReady: jest.fn().mockImplementation(() => onReadyPromise),
activate: jest.fn().mockImplementation(() => variationKey),
onUserUpdate: jest.fn().mockImplementation(() => () => { }),
onUserUpdate: jest.fn().mockImplementation(() => () => {}),
getVuid: jest.fn().mockImplementation(() => 'vuid_95bf72cebc774dfd8e8e580a5a1'),
notificationCenter: {
addNotificationListener: jest.fn().mockImplementation(() => { }),
removeNotificationListener: jest.fn().mockImplementation(() => { }),
addNotificationListener: jest.fn().mockImplementation(() => {}),
removeNotificationListener: jest.fn().mockImplementation(() => {}),
},
user: {
id: 'testuser',
Expand All @@ -55,7 +58,7 @@ describe('<OptimizelyExperiment>', () => {
isReady: jest.fn().mockImplementation(() => isReady),
getIsReadyPromiseFulfilled: () => true,
getIsUsingSdkKey: () => true,
onForcedVariationsUpdate: jest.fn().mockReturnValue(() => { }),
onForcedVariationsUpdate: jest.fn().mockReturnValue(() => {}),
} as unknown) as ReactSDKClient;
});

Expand Down Expand Up @@ -405,7 +408,9 @@ describe('<OptimizelyExperiment>', () => {
await optimizelyMock.onReady();

expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined);
await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation|true|false'));
await waitFor(() =>
expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation|true|false')
);
});

describe('when the onReady() promise return { success: false }', () => {
Expand Down
8 changes: 6 additions & 2 deletions src/Feature.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
/**
* Copyright 2018-2019, Optimizely
* Copyright 2018-2019, 2023 Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* 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,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/// <reference types="jest" />

import * as React from 'react';
import { act } from 'react-dom/test-utils';
import { render, screen, waitFor } from '@testing-library/react';
Expand Down Expand Up @@ -45,6 +48,7 @@ describe('<OptimizelyFeature>', () => {
getFeatureVariables: jest.fn().mockImplementation(() => featureVariables),
isFeatureEnabled: jest.fn().mockImplementation(() => isEnabledMock),
onUserUpdate: jest.fn().mockImplementation(handler => () => {}),
getVuid: jest.fn().mockImplementation(() => 'vuid_95bf72cebc774dfd8e8e580a5a1'),
notificationCenter: {
addNotificationListener: jest.fn().mockImplementation((type, handler) => {}),
removeNotificationListener: jest.fn().mockImplementation(id => {}),
Expand Down
35 changes: 23 additions & 12 deletions src/Provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as React from 'react';
import { UserAttributes } from '@optimizely/optimizely-sdk';
import { getLogger } from '@optimizely/optimizely-sdk';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
import { OptimizelyContextProvider } from './Context';
import { ReactSDKClient } from './client';
import { ReactSDKClient, DefaultUser } from './client';
import { areUsersEqual, UserInfo } from './utils';

const logger = getLogger('<OptimizelyProvider>');
Expand All @@ -42,9 +43,20 @@ interface OptimizelyProviderState {
export class OptimizelyProvider extends React.Component<OptimizelyProviderProps, OptimizelyProviderState> {
constructor(props: OptimizelyProviderProps) {
super(props);
const { optimizely, userId, userAttributes, user } = props;
}

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

async setUserInOptimizely(): Promise<void> {
const { optimizely, userId, userAttributes, user } = this.props;

if (!optimizely) {
logger.error('OptimizelyProvider must be passed an instance of the Optimizely SDK client');
return;
}

// check if user id/attributes are provided as props and set them ReactSDKClient
let finalUser: UserInfo | null = null;

if (user) {
Expand All @@ -65,17 +77,16 @@ export class OptimizelyProvider extends React.Component<OptimizelyProviderProps,
};
// deprecation warning
logger.warn('Passing userId and userAttributes as props is deprecated, please switch to using `user` prop');
} else {
finalUser = DefaultUser;
}

if (finalUser) {
if (!optimizely) {
logger.error(`Unable to set user ${finalUser} because optimizely object does not exist.`)
} else {
try {
optimizely.setUser(finalUser);
} catch (err) {
logger.error(`Unable to set user ${finalUser} because passed in optimizely object does not contain the setUser function.`)
}
try {
await optimizely.onReady();
await optimizely.setUser(finalUser);
} catch {
logger.error('Error while trying to set user.');
}
}
}
Expand Down Expand Up @@ -109,7 +120,7 @@ export class OptimizelyProvider extends React.Component<OptimizelyProviderProps,
}
}

render() {
render(): JSX.Element {
const { optimizely, children, timeout } = this.props;
const isServerSide = !!this.props.isServerSide;
const value = {
Expand Down
18 changes: 7 additions & 11 deletions src/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

jest.mock('@optimizely/optimizely-sdk');
jest.mock('./logger', () => {
return {
logger: {
warn: jest.fn(() => () => { }),
info: jest.fn(() => () => { }),
error: jest.fn(() => () => { }),
debug: jest.fn(() => () => { }),
warn: jest.fn(() => () => {}),
info: jest.fn(() => () => {}),
error: jest.fn(() => () => {}),
debug: jest.fn(() => () => {}),
},
};
});
Expand Down Expand Up @@ -1236,10 +1237,6 @@ describe('ReactSDKClient', () => {
const result = await instance.fetchQualifiedSegments();

expect(result).toEqual(false);
expect(logger.warn).toHaveBeenCalledTimes(1);
expect(logger.warn).toBeCalledWith(
'Unable to fetch qualified segments for user because Optimizely client failed to initialize.'
);
});

it('should return false if fetch fails', async () => {
Expand Down Expand Up @@ -1668,15 +1665,14 @@ describe('ReactSDKClient', () => {
instance.getUserContext();

expect(logger.warn).toHaveBeenCalledTimes(1);
expect(logger.warn).toBeCalledWith("Unable to get user context because Optimizely client failed to initialize.");
expect(logger.warn).toBeCalledWith('Unable to get user context. Optimizely client not initialized.');
});


it('should log a warning and return null if setUser is not called first', () => {
instance.getUserContext();

expect(logger.warn).toHaveBeenCalledTimes(1);
expect(logger.warn).toBeCalledWith("Unable to get user context because user was not set.");
expect(logger.warn).toBeCalledWith('Unable to get user context. User context not set.');
});

it('should return a userContext if setUser is called', () => {
Expand Down
Loading

0 comments on commit 43613fc

Please sign in to comment.