From 846cea82dd1861a48619d1fc35dd8e104f13c50b Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Mon, 8 Apr 2019 11:06:21 -0400 Subject: [PATCH] prepare analytics for move to separate repo - Make analytics and segment files configurable from outside. - Add checks for previously called identify on page/track. - Add some additional parameters to tracking events. ARCH-517 --- src/analytics/analytics.js | 133 ++++++++++--- src/analytics/analytics.test.js | 282 +++++++++++++++++++++++++--- src/analytics/segment.js | 10 +- src/components/App.jsx | 4 +- src/components/ProfilePage.jsx | 4 +- src/components/ProfilePage.test.jsx | 18 +- src/config/analytics.js | 13 ++ src/index.jsx | 3 +- 8 files changed, 395 insertions(+), 72 deletions(-) create mode 100644 src/config/analytics.js diff --git a/src/analytics/analytics.js b/src/analytics/analytics.js index d6d09c6..c24dc7e 100755 --- a/src/analytics/analytics.js +++ b/src/analytics/analytics.js @@ -1,54 +1,133 @@ import formurlencoded from 'form-urlencoded'; -import LoggingService from '@edx/frontend-logging'; - -import apiClient from '../config/apiClient'; -import { configuration } from '../config/environment'; import { snakeCaseObject } from '../services/utils'; -const eventLogApiBaseUrl = `${configuration.LMS_BASE_URL}/event`; +let config = {}; +let hasIdentifyBeenCalled = false; -// Sends events to Segment and downstream -function handleTrackEvents(eventName, properties) { - // Simply forward track events to Segment - window.analytics.track(eventName, properties); +/** + * Configures analytics module for an application. + * Note: this is using a module method, rather than a class+constructor, so functions + * can easily be passed around without this-binding concerns. + */ +function configureAnalytics(newConfig) { + hasIdentifyBeenCalled = false; + config = { + loggingService: newConfig.loggingService, + authApiClient: newConfig.authApiClient, + analyticsApiBaseUrl: newConfig.analyticsApiBaseUrl, + }; } +function getTrackingLogApiBaseUrl() { + return `${config.analyticsApiBaseUrl}/event`; +} -// Sends events to tracking log and downstream -// TODO: Determine consistent naming for eventName vs eventType and properties v eventData. -function logEvent(eventType, eventData) { - const snakeEventData = snakeCaseObject(eventData, { deep: true }); +function getAuthApiClient() { + if (!config.authApiClient) { + throw Error('You must configure the authApiClient.'); + } + return config.authApiClient; +} + +function getLoggingService() { + if (!config.loggingService) { + throw Error('You must configure the loggingService.'); + } + return config.loggingService; +} + +/** + * Checks that identify was first called. Otherwise, logs error. + */ +function checkIdentifyCalled() { + const loggingService = getLoggingService(); // verifies configuration early + if (!hasIdentifyBeenCalled) { + loggingService.logError('Identify must be called before other tracking events.'); + } +} + +/** + * Logs events to tracking log and downstream. + * For tracking log event documentation, see + * https://openedx.atlassian.net/wiki/spaces/AN/pages/13205895/Event+Design+and+Review+Process + * @param eventName (event_type on backend, but named to match Segment api) + * @param properties (event on backend, but named properties to match Segment api) + * @returns The promise returned by apiClient.post. + */ +function sendTrackingLogEvent(eventName, properties) { + const snakeEventData = snakeCaseObject(properties, { deep: true }); const serverData = { - event_type: eventType, + event_type: eventName, event: JSON.stringify(snakeEventData), page: window.location.href, }; - return apiClient.post( - eventLogApiBaseUrl, + const loggingService = getLoggingService(); // verifies configuration early + return getAuthApiClient().post( + getTrackingLogApiBaseUrl(), formurlencoded(serverData), { headers: { 'Content-Type': 'application/x-www-form-urlencoded', }, }, - ) - .catch((error) => { - LoggingService.logAPIErrorResponse(error); - }); + ).catch((error) => { + loggingService.logAPIErrorResponse(error); + }); } -function identifyUser() { - const authState = apiClient.getAuthenticationState(); +/** + * Send identify call to Segment, using userId from authApiClient. + * @param traits (optional) + */ +function identifyAuthenticatedUser(traits) { + const authState = getAuthApiClient().getAuthenticationState(); + const loggingService = getLoggingService(); // verifies configuration early if (authState.authentication && authState.authentication.userId) { // eslint-disable-next-line no-undef - window.analytics.identify(authState.authentication.userId); + window.analytics.identify(authState.authentication.userId, traits); + hasIdentifyBeenCalled = true; + } else { + loggingService.logError('UserId was not available for call to sendAuthenticatedIdentify.'); } } -function sendPageEvent() { - identifyUser(); - window.analytics.page(); +/** + * Send anonymous identify call to Segment's identify. + * @param traits (optional) + */ +function identifyAnonymousUser(traits) { + window.analytics.identify(traits); + hasIdentifyBeenCalled = true; } -export { handleTrackEvents, identifyUser, logEvent, sendPageEvent }; +/** + * Sends a track event to Segment and downstream. + * Note: For links and forms, you should use trackLink and trackForm instead. + * @param eventName + * @param properties (optional) + */ +function sendTrackEvent(eventName, properties) { + checkIdentifyCalled(); + window.analytics.track(eventName, properties); +} + +/** + * Sends a page event to Segment and downstream. + * @param category (optional) Name is required to pass a category. + * @param name (optional) If only one string arg provided, assumed to be name. + * @param properties (optional) + */ +function sendPageEvent(category, name, properties) { + checkIdentifyCalled(); + window.analytics.page(category, name, properties); +} + +export { + configureAnalytics, + identifyAnonymousUser, + identifyAuthenticatedUser, + sendPageEvent, + sendTrackEvent, + sendTrackingLogEvent, +}; diff --git a/src/analytics/analytics.test.js b/src/analytics/analytics.test.js index 5d596bd..0243e15 100644 --- a/src/analytics/analytics.test.js +++ b/src/analytics/analytics.test.js @@ -1,11 +1,11 @@ -import LoggingService from '@edx/frontend-logging'; - -import { logEvent } from './analytics'; -import { configuration } from '../config/environment'; -import apiClient from '../config/apiClient'; - -jest.mock('@edx/frontend-logging'); -jest.mock('../config/apiClient'); +import { + configureAnalytics, + identifyAnonymousUser, + identifyAuthenticatedUser, + sendPageEvent, + sendTrackEvent, + sendTrackingLogEvent, +} from './analytics'; const eventType = 'test.event'; const eventData = { @@ -14,39 +14,269 @@ const eventData = { testDeep: 'test-deep', }, }; +const testUserId = 99; +const testAnalyticsApiBaseUrl = '/analytics'; +let mockAuthApiClient; +let mockLoggingService; -beforeAll(() => { - apiClient.mockClear(); - LoggingService.mockClear(); -}); +function createMockLoggingService() { + mockLoggingService = { + logError: jest.fn(), + logAPIErrorResponse: jest.fn(), + }; +} +function createMockAuthApiClientAuthenticated() { + mockAuthApiClient = { + getAuthenticationState: + jest.fn(() => ({ + authentication: { userId: testUserId }, + })), + }; +} + +function createMockAuthApiClientAuthenticationIncomplete() { + mockAuthApiClient = { + getAuthenticationState: + jest.fn(() => ({ + authentication: {}, + })), + }; +} + +function createMockAuthApiClientPostResolved() { + mockAuthApiClient = { + post: jest.fn().mockResolvedValue(undefined), + }; +} + +function createMockAuthApiClientPostRejected() { + mockAuthApiClient = { + post: jest.fn().mockRejectedValue('test-error'), + }; +} + +function configureAnalyticsWithMocks() { + configureAnalytics({ + loggingService: mockLoggingService, + authApiClient: mockAuthApiClient, + analyticsApiBaseUrl: testAnalyticsApiBaseUrl, + }); +} + +describe('analytics sendTrackingLogEvent', () => { + it('fails when loggingService is not configured', () => { + mockLoggingService = undefined; + createMockAuthApiClientPostResolved(); + configureAnalyticsWithMocks(); + + expect(() => sendTrackingLogEvent(eventType, eventData)) + .toThrowError('You must configure the loggingService.'); + }); + + it('fails when authApiClient is not configured', () => { + createMockLoggingService(); + mockAuthApiClient = undefined; + configureAnalyticsWithMocks(); + + expect(() => sendTrackingLogEvent(eventType, eventData)) + .toThrowError('You must configure the authApiClient.'); + }); -describe('analytics logEvent', () => { it('posts expected data when successful', () => { - jest.spyOn(apiClient, 'post').mockResolvedValue(undefined); + createMockLoggingService(); + createMockAuthApiClientPostResolved(); + configureAnalyticsWithMocks(); expect.assertions(4); - return logEvent(eventType, eventData) + return sendTrackingLogEvent(eventType, eventData) .then(() => { - expect(apiClient.post.mock.calls.length).toEqual(1); - expect(apiClient.post.mock.calls[0][0]).toEqual(`${configuration.LMS_BASE_URL}/event`); - const data = 'event_type=test.event&event=%7B%22test_shallow%22%3A%22test-shallow%22%2C%22test_object%22%3A%7B%22test_deep%22%3A%22test-deep%22%7D%7D&page=http%3A%2F%2Flocalhost%2F'; - expect(apiClient.post.mock.calls[0][1]).toEqual(data); - const config = apiClient.post.mock.calls[0][2]; + expect(mockAuthApiClient.post.mock.calls.length).toEqual(1); + expect(mockAuthApiClient.post.mock.calls[0][0]).toEqual('/analytics/event'); + const expectedData = 'event_type=test.event&event=%7B%22test_shallow%22%3A%22test-shallow%22%2C%22test_object%22%3A%7B%22test_deep%22%3A%22test-deep%22%7D%7D&page=http%3A%2F%2Flocalhost%2F'; + expect(mockAuthApiClient.post.mock.calls[0][1]).toEqual(expectedData); + const config = mockAuthApiClient.post.mock.calls[0][2]; expect(config.headers['Content-Type']).toEqual('application/x-www-form-urlencoded'); }); }); - it('calls LoggingService.logAPIErrorResponse on error', () => { - LoggingService.logAPIErrorResponse = jest.fn(); - jest.spyOn(apiClient, 'post').mockRejectedValue('test-error'); + it('calls loggingService.logAPIErrorResponse on error', () => { + createMockLoggingService(); + createMockAuthApiClientPostRejected(); + configureAnalyticsWithMocks(); expect.assertions(2); - return logEvent(eventType, eventData) + return sendTrackingLogEvent(eventType, eventData) .then(() => { - expect(LoggingService.logAPIErrorResponse.mock.calls.length).toBe(1); - expect(LoggingService.logAPIErrorResponse.mock.calls[0][0]).toEqual('test-error'); + expect(mockLoggingService.logAPIErrorResponse.mock.calls.length).toBe(1); + expect(mockLoggingService.logAPIErrorResponse).toBeCalledWith('test-error'); }); }); }); + +describe('analytics identifyAuthenticatedUser', () => { + beforeEach(() => { + window.analytics = { + identify: jest.fn(), + }; + }); + + it('fails when loggingService is not configured', () => { + mockLoggingService = undefined; + createMockAuthApiClientAuthenticated(); + configureAnalyticsWithMocks(); + + expect(() => identifyAuthenticatedUser()) + .toThrowError('You must configure the loggingService.'); + }); + + it('fails when authApiClient is not configured', () => { + createMockLoggingService(); + mockAuthApiClient = undefined; + configureAnalyticsWithMocks(); + + expect(() => identifyAuthenticatedUser()) + .toThrowError('You must configure the authApiClient.'); + }); + + it('calls Segment identify on success', () => { + createMockLoggingService(); + createMockAuthApiClientAuthenticated(); + configureAnalyticsWithMocks(); + + const testTraits = { anything: 'Yay!' }; + identifyAuthenticatedUser(testTraits); + + expect(window.analytics.identify.mock.calls.length).toBe(1); + expect(window.analytics.identify).toBeCalledWith(testUserId, testTraits); + }); + + it('logs error when authentication problem.', () => { + createMockLoggingService(); + createMockAuthApiClientAuthenticationIncomplete(); + configureAnalyticsWithMocks(); + + identifyAuthenticatedUser(); + + expect(mockLoggingService.logError.mock.calls.length).toBe(1); + expect(mockLoggingService.logError).toBeCalledWith('UserId was not available for call to sendAuthenticatedIdentify.'); + }); +}); + +describe('analytics identifyAnonymousUser', () => { + beforeEach(() => { + window.analytics = { + identify: jest.fn(), + }; + }); + + it('calls Segment identify on success', () => { + const testTraits = { anything: 'Yay!' }; + identifyAnonymousUser(testTraits); + + expect(window.analytics.identify.mock.calls.length).toBe(1); + expect(window.analytics.identify).toBeCalledWith(testTraits); + }); +}); + +function testSendPageAfterIdentify(identifyFunction) { + createMockLoggingService(); + createMockAuthApiClientAuthenticated(); + configureAnalyticsWithMocks(); + identifyFunction(); + + const testCategory = 'test-category'; + const testName = 'test-name'; + const testProperties = { anything: 'Yay!' }; + sendPageEvent(testCategory, testName, testProperties); + + expect(window.analytics.page.mock.calls.length).toBe(1); + expect(window.analytics.page).toBeCalledWith(testCategory, testName, testProperties); +} + +describe('analytics send Page event', () => { + beforeEach(() => { + window.analytics = { + identify: jest.fn(), + page: jest.fn(), + }; + }); + + it('fails when loggingService is not configured', () => { + mockLoggingService = undefined; + mockAuthApiClient = undefined; + configureAnalyticsWithMocks(); + + expect(() => sendPageEvent()).toThrowError('You must configure the loggingService.'); + }); + + it('calls Segment page on success after identifyAuthenticatedUser', () => { + testSendPageAfterIdentify(identifyAuthenticatedUser); + }); + + it('calls Segment page on success after identifyAnonymousUser', () => { + testSendPageAfterIdentify(identifyAnonymousUser); + }); + + it('fails if page called with no identify', () => { + createMockLoggingService(); + mockAuthApiClient = undefined; + configureAnalyticsWithMocks(); + + sendPageEvent(); + + expect(mockLoggingService.logError.mock.calls.length).toBe(1); + expect(mockLoggingService.logError).toBeCalledWith('Identify must be called before other tracking events.'); + }); +}); + +function testSendTrackEventAfterIdentify(identifyFunction) { + createMockLoggingService(); + createMockAuthApiClientAuthenticated(); + configureAnalyticsWithMocks(); + identifyFunction(); + + const testName = 'test-name'; + const testProperties = { anything: 'Yay!' }; + sendTrackEvent(testName, testProperties); + + expect(window.analytics.track.mock.calls.length).toBe(1); + expect(window.analytics.track).toBeCalledWith(testName, testProperties); +} + +describe('analytics send Track event', () => { + beforeEach(() => { + window.analytics = { + identify: jest.fn(), + track: jest.fn(), + }; + }); + + it('fails when loggingService is not configured', () => { + mockLoggingService = undefined; + mockAuthApiClient = undefined; + configureAnalyticsWithMocks(); + + expect(() => sendTrackEvent()).toThrowError('You must configure the loggingService.'); + }); + + it('calls Segment track on success after identifyAuthenticatedUser', () => { + testSendTrackEventAfterIdentify(identifyAuthenticatedUser); + }); + + it('calls Segment track on success after identifyAnonymousUser', () => { + testSendTrackEventAfterIdentify(identifyAnonymousUser); + }); + + it('fails if track called with no identify', () => { + createMockLoggingService(); + mockAuthApiClient = undefined; + configureAnalyticsWithMocks(); + + sendTrackEvent(); + + expect(mockLoggingService.logError.mock.calls.length).toBe(1); + expect(mockLoggingService.logError).toBeCalledWith('Identify must be called before other tracking events.'); + }); +}); diff --git a/src/analytics/segment.js b/src/analytics/segment.js index 73d6509..2b1ef01 100644 --- a/src/analytics/segment.js +++ b/src/analytics/segment.js @@ -1,10 +1,9 @@ // The code in this file is from Segment's website, with the following update: -// - Pulls the segment key from configuration. +// - Takes the segment key as a parameter ( // https://segment.com/docs/sources/website/analytics.js/quickstart/ import { configuration } from '../config/environment'; -(function(){ - +function initializeSegment(segmentKey) { // Create a queue, but don't obliterate an existing one! var analytics = window.analytics = window.analytics || []; @@ -83,6 +82,7 @@ import { configuration } from '../config/environment'; // Load Analytics.js with your key, which will automatically // load the tools you've enabled for your account. Boosh! - analytics.load(configuration.SEGMENT_KEY); + analytics.load(segmentKey); +} -})(); +export { initializeSegment }; diff --git a/src/components/App.jsx b/src/components/App.jsx index 05a54ce..4d6f97a 100644 --- a/src/components/App.jsx +++ b/src/components/App.jsx @@ -8,7 +8,7 @@ import SiteFooter from '@edx/frontend-component-footer'; import { fetchUserAccount, UserAccountApiService } from '@edx/frontend-auth'; import apiClient from '../config/apiClient'; -import { handleTrackEvents } from '../analytics/analytics'; +import { sendTrackEvent } from '../analytics/analytics'; import { getLocale, getMessages } from '../i18n/i18n-loader'; import SiteHeader from './common/SiteHeader'; import ConnectedProfilePage from './ProfilePage'; @@ -56,7 +56,7 @@ class App extends Component { redditUrl={process.env.REDDIT_URL} appleAppStoreUrl={process.env.APPLE_APP_STORE_URL} googlePlayUrl={process.env.GOOGLE_PLAY_URL} - handleAllTrackEvents={handleTrackEvents} + handleAllTrackEvents={sendTrackEvent} /> diff --git a/src/components/ProfilePage.jsx b/src/components/ProfilePage.jsx index c099cce..33ca7f5 100644 --- a/src/components/ProfilePage.jsx +++ b/src/components/ProfilePage.jsx @@ -5,7 +5,7 @@ import { connect } from 'react-redux'; import { injectIntl, intlShape } from 'react-intl'; // Analytics -import { logEvent } from '../analytics/analytics'; +import { sendTrackingLogEvent } from '../analytics/analytics'; // Actions import { @@ -53,7 +53,7 @@ export class ProfilePage extends React.Component { componentDidMount() { this.props.fetchProfile(this.props.match.params.username); - logEvent('edx.profile.viewed', { + sendTrackingLogEvent('edx.profile.viewed', { username: this.props.match.params.username, }); } diff --git a/src/components/ProfilePage.test.jsx b/src/components/ProfilePage.test.jsx index 3f15d3b..582ac8a 100644 --- a/src/components/ProfilePage.test.jsx +++ b/src/components/ProfilePage.test.jsx @@ -32,7 +32,7 @@ const requiredProfilePageProps = { describe('', () => { describe('Renders correctly in various states', () => { it('app loading', () => { - analytics.logEvent = jest.fn(); + analytics.sendTrackingLogEvent = jest.fn(); const tree = renderer .create(( @@ -46,7 +46,7 @@ describe('', () => { }); it('viewing own profile', () => { - analytics.logEvent = jest.fn(); + analytics.sendTrackingLogEvent = jest.fn(); const tree = renderer .create(( @@ -60,7 +60,7 @@ describe('', () => { }); it('viewing other profile', () => { - analytics.logEvent = jest.fn(); + analytics.sendTrackingLogEvent = jest.fn(); const tree = renderer .create(( @@ -74,7 +74,7 @@ describe('', () => { }); it('while saving an edited bio', () => { - analytics.logEvent = jest.fn(); + analytics.sendTrackingLogEvent = jest.fn(); const tree = renderer .create(( @@ -90,8 +90,8 @@ describe('', () => { describe('handles analytics', () => { - it('calls logEvent when mounting', () => { - analytics.logEvent = jest.fn(); + it('calls sendTrackingLogEvent when mounting', () => { + analytics.sendTrackingLogEvent = jest.fn(); mount(( @@ -103,9 +103,9 @@ describe('', () => { )); - expect(analytics.logEvent.mock.calls.length).toBe(1); - expect(analytics.logEvent.mock.calls[0][0]).toEqual('edx.profile.viewed'); - expect(analytics.logEvent.mock.calls[0][1]).toEqual({ + expect(analytics.sendTrackingLogEvent.mock.calls.length).toBe(1); + expect(analytics.sendTrackingLogEvent.mock.calls[0][0]).toEqual('edx.profile.viewed'); + expect(analytics.sendTrackingLogEvent.mock.calls[0][1]).toEqual({ username: 'test-username', }); }); diff --git a/src/config/analytics.js b/src/config/analytics.js new file mode 100644 index 0000000..0490e00 --- /dev/null +++ b/src/config/analytics.js @@ -0,0 +1,13 @@ +import LoggingService from '@edx/frontend-logging'; + +import configureAnalytics from '../analytics/analytics'; +import initializeSegment from '../analytics/segment'; +import apiClient from '../config/apiClient'; +import { configuration } from '../config/environment'; + +initializeSegment(configuration.SEGMENT_KEY); +configureAnalytics({ + loggingService: LoggingService, + authApiClient: apiClient, + analyticsApiBaseUrl: configuration.LMS_BASE_URL, +}); diff --git a/src/index.jsx b/src/index.jsx index 7630ef8..e3872ad 100755 --- a/src/index.jsx +++ b/src/index.jsx @@ -6,7 +6,7 @@ import ReactDOM from 'react-dom'; import configureStore from './config/configureStore'; import apiClient from './config/apiClient'; import { handleRtl } from './i18n/i18n-loader'; -import { sendPageEvent } from './analytics/analytics'; +import { identifyAuthenticatedUser, sendPageEvent } from './analytics/analytics'; import './index.scss'; @@ -21,5 +21,6 @@ if (apiClient.ensurePublicOrAuthencationAndCookies(window.location.pathname)) { ReactDOM.render(, document.getElementById('root')); + identifyAuthenticatedUser(); sendPageEvent(); }