From c5f15591352e1453f1d89055028bdf1fd97a34a5 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Wed, 11 Nov 2020 17:35:26 -0500 Subject: [PATCH 1/6] MM-30087 Remove direct dependency between Client4 and Rudder --- components/root/root.jsx | 6 ++++-- package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/components/root/root.jsx b/components/root/root.jsx index e395480b6da5..861e5a13ace4 100644 --- a/components/root/root.jsx +++ b/components/root/root.jsx @@ -1,7 +1,8 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {rudderAnalytics, Client4} from 'mattermost-redux/client'; +import {Client4} from 'mattermost-redux/client'; +import {rudderAnalytics, RudderTelemetryHandler} from 'mattermost-redux/client/rudder'; import PropTypes from 'prop-types'; import React from 'react'; import FastClick from 'fastclick'; @@ -148,7 +149,8 @@ export default class Root extends React.PureComponent { } if (rudderKey != null && rudderKey !== '' && this.props.telemetryEnabled) { - Client4.enableRudderEvents(); + Client4.setTelemetryHandler(new RudderTelemetryHandler()); + rudderAnalytics.load(rudderKey, rudderUrl); rudderAnalytics.identify(telemetryId, {}, { diff --git a/package-lock.json b/package-lock.json index ecb6153d98cc..195828b4b17a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17899,8 +17899,8 @@ "integrity": "sha512-6qE4B9deFBIa9YSpOc9O0Sgc43zTeVYbgDT5veRKSlB2+ZuHNoVVxA1L/ckMUayV9Ay9y7Z/SZCLcGteW9i7bg==" }, "mattermost-redux": { - "version": "github:mattermost/mattermost-redux#71950dee7ac3b93f2f2fe4a7e78de50e7b59b288", - "from": "github:mattermost/mattermost-redux#71950dee7ac3b93f2f2fe4a7e78de50e7b59b288", + "version": "github:mattermost/mattermost-redux#1c9d6bbac7569f2a2000758a2b40a4f009043694", + "from": "github:mattermost/mattermost-redux#1c9d6bbac7569f2a2000758a2b40a4f009043694", "requires": { "core-js": "3.6.5", "form-data": "3.0.0", diff --git a/package.json b/package.json index 5f8bc4c49f95..16384ef6c55b 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "localforage-observable": "2.0.1", "mark.js": "8.11.1", "marked": "github:mattermost/marked#87769262aa02e1784570f61f4f962050e07cc335", - "mattermost-redux": "github:mattermost/mattermost-redux#71950dee7ac3b93f2f2fe4a7e78de50e7b59b288", + "mattermost-redux": "github:mattermost/mattermost-redux#1c9d6bbac7569f2a2000758a2b40a4f009043694", "moment-timezone": "0.5.31", "p-queue": "6.6.1", "pdfjs-dist": "2.1.266", From 562e87abee15b0ef06d8bf3fe0ca884c4e157d70 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Thu, 12 Nov 2020 13:45:11 -0500 Subject: [PATCH 2/6] Update tests --- components/root/root.test.jsx | 69 ++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/components/root/root.test.jsx b/components/root/root.test.jsx index 66bdfbcacaae..3535c91d3b6e 100644 --- a/components/root/root.test.jsx +++ b/components/root/root.test.jsx @@ -5,6 +5,7 @@ import React from 'react'; import {shallow} from 'enzyme'; import {Client4} from 'mattermost-redux/client'; +import {rudderAnalytics} from 'mattermost-redux/client/rudder'; import Root from 'components/root/root'; import * as GlobalActions from 'actions/global_actions.jsx'; @@ -15,6 +16,13 @@ jest.mock('fastclick', () => ({ attach: () => {}, // eslint-disable-line no-empty-function })); +jest.mock('rudder-sdk-js', () => ({ + identify: jest.fn(), + load: jest.fn(), + page: jest.fn(), + track: jest.fn(), +})); + jest.mock('actions/telemetry_actions', () => ({ trackLoadTime: () => {}, // eslint-disable-line no-empty-function })); @@ -32,18 +40,6 @@ jest.mock('utils/utils', () => ({ jest.mock('mattermost-redux/actions/general', () => ({ setUrl: () => {}, })); -jest.mock('mattermost-redux/client', () => { - const original = jest.requireActual('mattermost-redux/client'); - - return { - ...original, - Client4: { - ...original.Client4, - setUrl: jest.fn(), - enableRudderEvents: jest.fn(), - }, - }; -}); describe('components/Root', () => { const baseProps = { @@ -193,21 +189,44 @@ describe('components/Root', () => { wrapper.unmount(); }); - test('should not call enableRudderEvents on call of onConfigLoaded if url and key for rudder is not set', () => { - const wrapper = shallow(); - wrapper.instance().onConfigLoaded(); - expect(Client4.enableRudderEvents).not.toHaveBeenCalled(); - wrapper.unmount(); - }); + describe('onConfigLoaded', () => { + // Replace loadMeAndConfig with an action that never resolves so we can control exactly when onConfigLoaded is called + const props = { + ...baseProps, + actions: { + ...baseProps.actions, + loadMeAndConfig: () => new Promise(() => {}), + }, + }; - test('should call for enableRudderEvents on call of onConfigLoaded if url and key for rudder is set', () => { - Constants.TELEMETRY_RUDDER_KEY = 'testKey'; - Constants.TELEMETRY_RUDDER_DATAPLANE_URL = 'url'; + test('should not send events to telemetry after onConfigLoaded is called if Rudder is not configured', () => { + const wrapper = shallow(); - const wrapper = shallow(); - wrapper.instance().onConfigLoaded(); - expect(Client4.enableRudderEvents).toHaveBeenCalled(); - wrapper.unmount(); + wrapper.instance().onConfigLoaded(); + + Client4.trackEvent('category', 'event'); + + expect(Client4.telemetryHandler).not.toBeDefined(); + expect(rudderAnalytics.track).not.toHaveBeenCalled(); + + wrapper.unmount(); + }); + + test('should send events to telemetry after onConfigLoaded is called if Rudder is configured', () => { + Constants.TELEMETRY_RUDDER_KEY = 'testKey'; + Constants.TELEMETRY_RUDDER_DATAPLANE_URL = 'url'; + + const wrapper = shallow(); + + wrapper.instance().onConfigLoaded(); + + Client4.trackEvent('category', 'event'); + + expect(Client4.telemetryHandler).toBeDefined(); + expect(rudderAnalytics.track).toHaveBeenCalled(); + + wrapper.unmount(); + }); }); test('should reload on focus after getting signal login event from another tab', () => { From 24f7aad7075dea0376f4a5ad5da07c81d4771434 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Mon, 23 Nov 2020 13:54:13 -0500 Subject: [PATCH 3/6] Switch mattermost-redux to master --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8a463692a279..ecd582cd017d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17899,8 +17899,8 @@ "integrity": "sha512-6qE4B9deFBIa9YSpOc9O0Sgc43zTeVYbgDT5veRKSlB2+ZuHNoVVxA1L/ckMUayV9Ay9y7Z/SZCLcGteW9i7bg==" }, "mattermost-redux": { - "version": "github:mattermost/mattermost-redux#bd63f65b85d3ec8c2636ed0365aad2ab6c246df2", - "from": "github:mattermost/mattermost-redux#bd63f65b85d3ec8c2636ed0365aad2ab6c246df2", + "version": "github:mattermost/mattermost-redux#0e4c1e624d57a744fe28a91e7cf45696d8b6e1c1", + "from": "github:mattermost/mattermost-redux#0e4c1e624d57a744fe28a91e7cf45696d8b6e1c1", "requires": { "core-js": "3.6.5", "form-data": "3.0.0", diff --git a/package.json b/package.json index 339c0ce50e75..7c4ed46f83d0 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "localforage-observable": "2.0.1", "mark.js": "8.11.1", "marked": "github:mattermost/marked#87769262aa02e1784570f61f4f962050e07cc335", - "mattermost-redux": "github:mattermost/mattermost-redux#bd63f65b85d3ec8c2636ed0365aad2ab6c246df2", + "mattermost-redux": "github:mattermost/mattermost-redux#0e4c1e624d57a744fe28a91e7cf45696d8b6e1c1", "moment-timezone": "0.5.31", "p-queue": "6.6.1", "pdfjs-dist": "2.1.266", From e268d2268ead35307ca95c1b38ce0872e8445561 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Mon, 23 Nov 2020 14:19:35 -0500 Subject: [PATCH 4/6] Fix missing types --- .../post_attachment_opengraph.test.tsx | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/components/post_view/post_attachment_opengraph/post_attachment_opengraph.test.tsx b/components/post_view/post_attachment_opengraph/post_attachment_opengraph.test.tsx index b27bce3dda11..8af5e598226a 100644 --- a/components/post_view/post_attachment_opengraph/post_attachment_opengraph.test.tsx +++ b/components/post_view/post_attachment_opengraph/post_attachment_opengraph.test.tsx @@ -21,6 +21,8 @@ describe('PostAttachmentOpenGraph', () => { metadata: { images: { [imageUrl]: { + format: 'png', + frameCount: 0, height: 100, width: 100, }, @@ -88,13 +90,23 @@ describe('PostAttachmentOpenGraph', () => { test('should be a large image', () => { const wrapper = shallow(); - expect(wrapper.instance().isLargeImage({width: 400, height: 180})).toBe(true); + expect(wrapper.instance().isLargeImage({ + format: 'png', + frameCount: 0, + height: 180, + width: 400, + })).toBe(true); }); test('should not be a large image', () => { const wrapper = shallow(); - expect(wrapper.instance().isLargeImage({width: 100, height: 100})).toBe(false); + expect(wrapper.instance().isLargeImage({ + format: 'png', + frameCount: 0, + height: 100, + width: 100, + })).toBe(false); }); }); @@ -115,6 +127,8 @@ describe('PostAttachmentOpenGraph', () => { ...post.metadata, images: { [imageUrl]: { + format: 'png', + frameCount: 0, height: 180, width: 400, }, @@ -232,10 +246,14 @@ describe('getBestImageUrl', () => { }; const imagesMetadata = { 'http://example.com/image.png': { + format: 'png', + frameCount: 0, height: 100, width: 100, }, 'http://example.com/image2.png': { + format: 'png', + frameCount: 0, height: 1000, width: 1000, }, From a5dabb2bc18bf2e75e2a456fba74c0462697c28a Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Tue, 24 Nov 2020 09:15:17 -0500 Subject: [PATCH 5/6] Change how we mock and import rudder --- components/root/root.test.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/root/root.test.jsx b/components/root/root.test.jsx index 3535c91d3b6e..a832f9c2ac00 100644 --- a/components/root/root.test.jsx +++ b/components/root/root.test.jsx @@ -1,11 +1,11 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React from 'react'; import {shallow} from 'enzyme'; +import React from 'react'; +import * as rudderAnalytics from 'rudder-sdk-js'; import {Client4} from 'mattermost-redux/client'; -import {rudderAnalytics} from 'mattermost-redux/client/rudder'; import Root from 'components/root/root'; import * as GlobalActions from 'actions/global_actions.jsx'; From 87543e3255524086d7174ceecd9907a55992e7a4 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Tue, 24 Nov 2020 09:33:48 -0500 Subject: [PATCH 6/6] Remove test that checks if events are sent to Rudder --- components/root/root.test.jsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/components/root/root.test.jsx b/components/root/root.test.jsx index a832f9c2ac00..39c01ae9fcd0 100644 --- a/components/root/root.test.jsx +++ b/components/root/root.test.jsx @@ -3,7 +3,6 @@ import {shallow} from 'enzyme'; import React from 'react'; -import * as rudderAnalytics from 'rudder-sdk-js'; import {Client4} from 'mattermost-redux/client'; @@ -199,7 +198,7 @@ describe('components/Root', () => { }, }; - test('should not send events to telemetry after onConfigLoaded is called if Rudder is not configured', () => { + test('should not set a TelemetryHandler when onConfigLoaded is called if Rudder is not configured', () => { const wrapper = shallow(); wrapper.instance().onConfigLoaded(); @@ -207,12 +206,11 @@ describe('components/Root', () => { Client4.trackEvent('category', 'event'); expect(Client4.telemetryHandler).not.toBeDefined(); - expect(rudderAnalytics.track).not.toHaveBeenCalled(); wrapper.unmount(); }); - test('should send events to telemetry after onConfigLoaded is called if Rudder is configured', () => { + test('should set a TelemetryHandler when onConfigLoaded is called if Rudder is configured', () => { Constants.TELEMETRY_RUDDER_KEY = 'testKey'; Constants.TELEMETRY_RUDDER_DATAPLANE_URL = 'url'; @@ -223,7 +221,6 @@ describe('components/Root', () => { Client4.trackEvent('category', 'event'); expect(Client4.telemetryHandler).toBeDefined(); - expect(rudderAnalytics.track).toHaveBeenCalled(); wrapper.unmount(); });