From 54bddc750478efeda0d76d12dbefda26c6766021 Mon Sep 17 00:00:00 2001 From: Josh Dover <1813008+joshdover@users.noreply.github.com> Date: Tue, 20 Jul 2021 15:14:47 +0200 Subject: [PATCH 1/3] Improve unit test coverage of FS API calls --- x-pack/plugins/cloud/public/fullstory.ts | 28 ++------ .../plugins/cloud/public/plugin.test.mocks.ts | 10 ++- x-pack/plugins/cloud/public/plugin.test.ts | 67 ++++++++++++++++--- x-pack/plugins/cloud/public/plugin.ts | 29 +++++++- 4 files changed, 97 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/cloud/public/fullstory.ts b/x-pack/plugins/cloud/public/fullstory.ts index a5b735bce9387..3c1b326341de6 100644 --- a/x-pack/plugins/cloud/public/fullstory.ts +++ b/x-pack/plugins/cloud/public/fullstory.ts @@ -5,27 +5,24 @@ * 2.0. */ -import { sha256 } from 'js-sha256'; import type { IBasePath, PackageInfo } from '../../../../src/core/public'; export interface FullStoryDeps { basePath: IBasePath; orgId: string; packageInfo: PackageInfo; - userId?: string; } -interface FullStoryApi { +export interface FullStoryApi { identify(userId: string, userVars?: Record): void; event(eventName: string, eventProperties: Record): void; } -export const initializeFullStory = async ({ +export const initializeFullStory = ({ basePath, orgId, packageInfo, - userId, -}: FullStoryDeps) => { +}: FullStoryDeps): FullStoryApi => { // @ts-expect-error window._fs_debug = false; // @ts-expect-error @@ -75,22 +72,5 @@ export const initializeFullStory = async ({ // @ts-expect-error const fullStory: FullStoryApi = window.FSKibana; - try { - // This needs to be called syncronously to be sure that we populate the user ID soon enough to make sessions merging - // across domains work - if (userId) { - // Do the hashing here to keep it at clear as possible in our source code that we do not send literal user IDs - const hashedId = sha256(userId.toString()); - fullStory.identify(hashedId); - } - } catch (e) { - // eslint-disable-next-line no-console - console.error(`[cloud.full_story] Could not call FS.identify due to error: ${e.toString()}`, e); - } - - // Record an event that Kibana was opened so we can easily search for sessions that use Kibana - fullStory.event('Loaded Kibana', { - // `str` suffix is required, see docs: https://help.fullstory.com/hc/en-us/articles/360020623234 - kibana_version_str: packageInfo.version, - }); + return fullStory; }; diff --git a/x-pack/plugins/cloud/public/plugin.test.mocks.ts b/x-pack/plugins/cloud/public/plugin.test.mocks.ts index 889b8492d5b1b..40c9ff9ab57f4 100644 --- a/x-pack/plugins/cloud/public/plugin.test.mocks.ts +++ b/x-pack/plugins/cloud/public/plugin.test.mocks.ts @@ -5,9 +5,15 @@ * 2.0. */ -import type { FullStoryDeps } from './fullstory'; +import type { FullStoryDeps, FullStoryApi } from './fullstory'; -export const initializeFullStoryMock = jest.fn(); +export const fullStoryApiMock: jest.Mocked = { + event: jest.fn(), + identify: jest.fn(), +}; +export const initializeFullStoryMock = jest.fn( + () => fullStoryApiMock +); jest.doMock('./fullstory', () => { return { initializeFullStory: initializeFullStoryMock }; }); diff --git a/x-pack/plugins/cloud/public/plugin.test.ts b/x-pack/plugins/cloud/public/plugin.test.ts index 264ae61c050e8..9b3ddc8e7294e 100644 --- a/x-pack/plugins/cloud/public/plugin.test.ts +++ b/x-pack/plugins/cloud/public/plugin.test.ts @@ -9,14 +9,14 @@ import { nextTick } from '@kbn/test/jest'; import { coreMock } from 'src/core/public/mocks'; import { homePluginMock } from 'src/plugins/home/public/mocks'; import { securityMock } from '../../security/public/mocks'; -import { initializeFullStoryMock } from './plugin.test.mocks'; +import { fullStoryApiMock, initializeFullStoryMock } from './plugin.test.mocks'; import { CloudPlugin, CloudConfigType, loadFullStoryUserId } from './plugin'; describe('Cloud Plugin', () => { describe('#setup', () => { describe('setupFullstory', () => { beforeEach(() => { - initializeFullStoryMock.mockReset(); + jest.clearAllMocks(); }); const setupPlugin = async ({ @@ -63,23 +63,72 @@ describe('Cloud Plugin', () => { }); expect(initializeFullStoryMock).toHaveBeenCalled(); - const { basePath, orgId, packageInfo, userId } = initializeFullStoryMock.mock.calls[0][0]; + const { basePath, orgId, packageInfo } = initializeFullStoryMock.mock.calls[0][0]; expect(basePath.prepend).toBeDefined(); expect(orgId).toEqual('foo'); expect(packageInfo).toEqual(initContext.env.packageInfo); - expect(userId).toEqual('1234'); }); - it('passes undefined user ID when security is not available', async () => { + it('calls FS.identify with hashed user ID when security is available', async () => { + await setupPlugin({ + config: { full_story: { enabled: true, org_id: 'foo' } }, + currentUserProps: { + username: '1234', + }, + }); + + expect(fullStoryApiMock.identify).toHaveBeenCalledWith( + '03ac674216f3e15c761ee1a5e255f067953623c8b388b4459e13f978d7c846f4' + ); + }); + + it('does not call FS.identify when security is not available', async () => { await setupPlugin({ config: { full_story: { enabled: true, org_id: 'foo' } }, securityEnabled: false, }); - expect(initializeFullStoryMock).toHaveBeenCalled(); - const { orgId, userId } = initializeFullStoryMock.mock.calls[0][0]; - expect(orgId).toEqual('foo'); - expect(userId).toEqual(undefined); + expect(fullStoryApiMock.identify).not.toHaveBeenCalled(); + }); + + it('calls FS.event when security is available', async () => { + const { initContext } = await setupPlugin({ + config: { full_story: { enabled: true, org_id: 'foo' } }, + currentUserProps: { + username: '1234', + }, + }); + + expect(fullStoryApiMock.event).toHaveBeenCalledWith('Loaded Kibana', { + kibana_version_str: initContext.env.packageInfo.version, + }); + }); + + it('calls FS.event when security is not available', async () => { + const { initContext } = await setupPlugin({ + config: { full_story: { enabled: true, org_id: 'foo' } }, + securityEnabled: false, + }); + + expect(fullStoryApiMock.event).toHaveBeenCalledWith('Loaded Kibana', { + kibana_version_str: initContext.env.packageInfo.version, + }); + }); + + it('calls FS.event when FS.identify throws an error', async () => { + fullStoryApiMock.identify.mockImplementationOnce(() => { + throw new Error(`identify failed!`); + }); + const { initContext } = await setupPlugin({ + config: { full_story: { enabled: true, org_id: 'foo' } }, + currentUserProps: { + username: '1234', + }, + }); + + expect(fullStoryApiMock.event).toHaveBeenCalledWith('Loaded Kibana', { + kibana_version_str: initContext.env.packageInfo.version, + }); }); it('does not call initializeFullStory when enabled=false', async () => { diff --git a/x-pack/plugins/cloud/public/plugin.ts b/x-pack/plugins/cloud/public/plugin.ts index 98017d09ef807..93c91c62e004d 100644 --- a/x-pack/plugins/cloud/public/plugin.ts +++ b/x-pack/plugins/cloud/public/plugin.ts @@ -14,6 +14,7 @@ import { IBasePath, } from 'src/core/public'; import { i18n } from '@kbn/i18n'; +import { sha256 } from 'js-sha256'; import type { AuthenticatedUser, SecurityPluginSetup, @@ -162,7 +163,7 @@ export class CloudPlugin implements Plugin { }: CloudSetupDependencies & { basePath: IBasePath }) { const { enabled, org_id: orgId } = this.config.full_story; if (!enabled || !orgId) { - return; + return; // do not load any fullstory code in the browser if not enabled } // Keep this import async so that we do not load any FullStory code into the browser when it is disabled. @@ -171,17 +172,41 @@ export class CloudPlugin implements Plugin { ? loadFullStoryUserId({ getCurrentUser: security.authc.getCurrentUser }) : Promise.resolve(undefined); + // We need to call FS.identify synchronously after FullStory is initialized, so we must load the user upfront const [{ initializeFullStory }, userId] = await Promise.all([ fullStoryChunkPromise, userIdPromise, ]); - initializeFullStory({ + const fullStory = initializeFullStory({ basePath, orgId, packageInfo: this.initializerContext.env.packageInfo, userId, }); + + // Very defensive try/catch to avoid any UnhandledPromiseRejections + try { + // This needs to be called syncronously to be sure that we populate the user ID soon enough to make sessions merging + // across domains work + if (userId) { + // Do the hashing here to keep it at clear as possible in our source code that we do not send literal user IDs + const hashedId = sha256(userId.toString()); + fullStory.identify(hashedId); + } + } catch (e) { + // eslint-disable-next-line no-console + console.error( + `[cloud.full_story] Could not call FS.identify due to error: ${e.toString()}`, + e + ); + } + + // Record an event that Kibana was opened so we can easily search for sessions that use Kibana + fullStory.event('Loaded Kibana', { + // `str` suffix is required, see docs: https://help.fullstory.com/hc/en-us/articles/360020623234 + kibana_version_str: this.initializerContext.env.packageInfo.version, + }); } } From fc1a52bf4674a0dc2d47ec6ec2f42db6e64c940e Mon Sep 17 00:00:00 2001 From: Josh Dover <1813008+joshdover@users.noreply.github.com> Date: Tue, 20 Jul 2021 17:44:12 +0200 Subject: [PATCH 2/3] fix type error --- x-pack/plugins/cloud/public/plugin.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/cloud/public/plugin.ts b/x-pack/plugins/cloud/public/plugin.ts index 93c91c62e004d..a0849eb2a217b 100644 --- a/x-pack/plugins/cloud/public/plugin.ts +++ b/x-pack/plugins/cloud/public/plugin.ts @@ -182,7 +182,6 @@ export class CloudPlugin implements Plugin { basePath, orgId, packageInfo: this.initializerContext.env.packageInfo, - userId, }); // Very defensive try/catch to avoid any UnhandledPromiseRejections From 8ccd241fc9520deffb72ffe054f4e763b8b6e441 Mon Sep 17 00:00:00 2001 From: Josh Dover <1813008+joshdover@users.noreply.github.com> Date: Tue, 20 Jul 2021 17:52:49 +0200 Subject: [PATCH 3/3] async load js-sha256 --- x-pack/plugins/cloud/public/fullstory.ts | 13 +++++++++++-- x-pack/plugins/cloud/public/plugin.test.mocks.ts | 10 ++++++---- x-pack/plugins/cloud/public/plugin.ts | 3 +-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/cloud/public/fullstory.ts b/x-pack/plugins/cloud/public/fullstory.ts index 3c1b326341de6..25d5320a063bd 100644 --- a/x-pack/plugins/cloud/public/fullstory.ts +++ b/x-pack/plugins/cloud/public/fullstory.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { sha256 } from 'js-sha256'; // loaded here to reduce page load bundle size when FullStory is disabled import type { IBasePath, PackageInfo } from '../../../../src/core/public'; export interface FullStoryDeps { @@ -18,11 +19,16 @@ export interface FullStoryApi { event(eventName: string, eventProperties: Record): void; } +export interface FullStoryService { + fullStory: FullStoryApi; + sha256: typeof sha256; +} + export const initializeFullStory = ({ basePath, orgId, packageInfo, -}: FullStoryDeps): FullStoryApi => { +}: FullStoryDeps): FullStoryService => { // @ts-expect-error window._fs_debug = false; // @ts-expect-error @@ -72,5 +78,8 @@ export const initializeFullStory = ({ // @ts-expect-error const fullStory: FullStoryApi = window.FSKibana; - return fullStory; + return { + fullStory, + sha256, + }; }; diff --git a/x-pack/plugins/cloud/public/plugin.test.mocks.ts b/x-pack/plugins/cloud/public/plugin.test.mocks.ts index 40c9ff9ab57f4..4eb206d07bf85 100644 --- a/x-pack/plugins/cloud/public/plugin.test.mocks.ts +++ b/x-pack/plugins/cloud/public/plugin.test.mocks.ts @@ -5,15 +5,17 @@ * 2.0. */ -import type { FullStoryDeps, FullStoryApi } from './fullstory'; +import { sha256 } from 'js-sha256'; +import type { FullStoryDeps, FullStoryApi, FullStoryService } from './fullstory'; export const fullStoryApiMock: jest.Mocked = { event: jest.fn(), identify: jest.fn(), }; -export const initializeFullStoryMock = jest.fn( - () => fullStoryApiMock -); +export const initializeFullStoryMock = jest.fn(() => ({ + fullStory: fullStoryApiMock, + sha256, +})); jest.doMock('./fullstory', () => { return { initializeFullStory: initializeFullStoryMock }; }); diff --git a/x-pack/plugins/cloud/public/plugin.ts b/x-pack/plugins/cloud/public/plugin.ts index a0849eb2a217b..16c11d569c5f7 100644 --- a/x-pack/plugins/cloud/public/plugin.ts +++ b/x-pack/plugins/cloud/public/plugin.ts @@ -14,7 +14,6 @@ import { IBasePath, } from 'src/core/public'; import { i18n } from '@kbn/i18n'; -import { sha256 } from 'js-sha256'; import type { AuthenticatedUser, SecurityPluginSetup, @@ -178,7 +177,7 @@ export class CloudPlugin implements Plugin { userIdPromise, ]); - const fullStory = initializeFullStory({ + const { fullStory, sha256 } = initializeFullStory({ basePath, orgId, packageInfo: this.initializerContext.env.packageInfo,