From e1a55ea8e254a6db754dcfb594b6a97ec0ecacfb Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Tue, 9 Apr 2019 08:42:51 -0400 Subject: [PATCH] Revert "prepare analytics for move to separate repo" This reverts commit 846cea82dd1861a48619d1fc35dd8e104f13c50b. --- 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, 72 insertions(+), 395 deletions(-) delete mode 100644 src/config/analytics.js diff --git a/src/analytics/analytics.js b/src/analytics/analytics.js index c24dc7e..d6d09c6 100755 --- a/src/analytics/analytics.js +++ b/src/analytics/analytics.js @@ -1,133 +1,54 @@ 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'; -let config = {}; -let hasIdentifyBeenCalled = false; +const eventLogApiBaseUrl = `${configuration.LMS_BASE_URL}/event`; -/** - * 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, - }; +// Sends events to Segment and downstream +function handleTrackEvents(eventName, properties) { + // Simply forward track events to Segment + window.analytics.track(eventName, properties); } -function getTrackingLogApiBaseUrl() { - return `${config.analyticsApiBaseUrl}/event`; -} -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 }); +// 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 }); const serverData = { - event_type: eventName, + event_type: eventType, event: JSON.stringify(snakeEventData), page: window.location.href, }; - const loggingService = getLoggingService(); // verifies configuration early - return getAuthApiClient().post( - getTrackingLogApiBaseUrl(), + return apiClient.post( + eventLogApiBaseUrl, formurlencoded(serverData), { headers: { 'Content-Type': 'application/x-www-form-urlencoded', }, }, - ).catch((error) => { - loggingService.logAPIErrorResponse(error); - }); + ) + .catch((error) => { + LoggingService.logAPIErrorResponse(error); + }); } -/** - * 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 +function identifyUser() { + const authState = apiClient.getAuthenticationState(); if (authState.authentication && authState.authentication.userId) { // eslint-disable-next-line no-undef - window.analytics.identify(authState.authentication.userId, traits); - hasIdentifyBeenCalled = true; - } else { - loggingService.logError('UserId was not available for call to sendAuthenticatedIdentify.'); + window.analytics.identify(authState.authentication.userId); } } -/** - * Send anonymous identify call to Segment's identify. - * @param traits (optional) - */ -function identifyAnonymousUser(traits) { - window.analytics.identify(traits); - hasIdentifyBeenCalled = true; +function sendPageEvent() { + identifyUser(); + window.analytics.page(); } -/** - * 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, -}; +export { handleTrackEvents, identifyUser, logEvent, sendPageEvent }; diff --git a/src/analytics/analytics.test.js b/src/analytics/analytics.test.js index 0243e15..5d596bd 100644 --- a/src/analytics/analytics.test.js +++ b/src/analytics/analytics.test.js @@ -1,11 +1,11 @@ -import { - configureAnalytics, - identifyAnonymousUser, - identifyAuthenticatedUser, - sendPageEvent, - sendTrackEvent, - sendTrackingLogEvent, -} from './analytics'; +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'); const eventType = 'test.event'; const eventData = { @@ -14,269 +14,39 @@ const eventData = { testDeep: 'test-deep', }, }; -const testUserId = 99; -const testAnalyticsApiBaseUrl = '/analytics'; -let mockAuthApiClient; -let mockLoggingService; -function createMockLoggingService() { - mockLoggingService = { - logError: jest.fn(), - logAPIErrorResponse: jest.fn(), - }; -} +beforeAll(() => { + apiClient.mockClear(); + LoggingService.mockClear(); +}); -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', () => { - createMockLoggingService(); - createMockAuthApiClientPostResolved(); - configureAnalyticsWithMocks(); + jest.spyOn(apiClient, 'post').mockResolvedValue(undefined); expect.assertions(4); - return sendTrackingLogEvent(eventType, eventData) + return logEvent(eventType, eventData) .then(() => { - 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(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(config.headers['Content-Type']).toEqual('application/x-www-form-urlencoded'); }); }); - it('calls loggingService.logAPIErrorResponse on error', () => { - createMockLoggingService(); - createMockAuthApiClientPostRejected(); - configureAnalyticsWithMocks(); + it('calls LoggingService.logAPIErrorResponse on error', () => { + LoggingService.logAPIErrorResponse = jest.fn(); + jest.spyOn(apiClient, 'post').mockRejectedValue('test-error'); expect.assertions(2); - return sendTrackingLogEvent(eventType, eventData) + return logEvent(eventType, eventData) .then(() => { - expect(mockLoggingService.logAPIErrorResponse.mock.calls.length).toBe(1); - expect(mockLoggingService.logAPIErrorResponse).toBeCalledWith('test-error'); + expect(LoggingService.logAPIErrorResponse.mock.calls.length).toBe(1); + expect(LoggingService.logAPIErrorResponse.mock.calls[0][0]).toEqual('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 2b1ef01..73d6509 100644 --- a/src/analytics/segment.js +++ b/src/analytics/segment.js @@ -1,9 +1,10 @@ // The code in this file is from Segment's website, with the following update: -// - Takes the segment key as a parameter ( +// - Pulls the segment key from configuration. // https://segment.com/docs/sources/website/analytics.js/quickstart/ import { configuration } from '../config/environment'; -function initializeSegment(segmentKey) { +(function(){ + // Create a queue, but don't obliterate an existing one! var analytics = window.analytics = window.analytics || []; @@ -82,7 +83,6 @@ function initializeSegment(segmentKey) { // Load Analytics.js with your key, which will automatically // load the tools you've enabled for your account. Boosh! - analytics.load(segmentKey); -} + analytics.load(configuration.SEGMENT_KEY); -export { initializeSegment }; +})(); diff --git a/src/components/App.jsx b/src/components/App.jsx index 4d6f97a..05a54ce 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 { sendTrackEvent } from '../analytics/analytics'; +import { handleTrackEvents } 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={sendTrackEvent} + handleAllTrackEvents={handleTrackEvents} /> diff --git a/src/components/ProfilePage.jsx b/src/components/ProfilePage.jsx index 33ca7f5..c099cce 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 { sendTrackingLogEvent } from '../analytics/analytics'; +import { logEvent } from '../analytics/analytics'; // Actions import { @@ -53,7 +53,7 @@ export class ProfilePage extends React.Component { componentDidMount() { this.props.fetchProfile(this.props.match.params.username); - sendTrackingLogEvent('edx.profile.viewed', { + logEvent('edx.profile.viewed', { username: this.props.match.params.username, }); } diff --git a/src/components/ProfilePage.test.jsx b/src/components/ProfilePage.test.jsx index 582ac8a..3f15d3b 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.sendTrackingLogEvent = jest.fn(); + analytics.logEvent = jest.fn(); const tree = renderer .create(( @@ -46,7 +46,7 @@ describe('', () => { }); it('viewing own profile', () => { - analytics.sendTrackingLogEvent = jest.fn(); + analytics.logEvent = jest.fn(); const tree = renderer .create(( @@ -60,7 +60,7 @@ describe('', () => { }); it('viewing other profile', () => { - analytics.sendTrackingLogEvent = jest.fn(); + analytics.logEvent = jest.fn(); const tree = renderer .create(( @@ -74,7 +74,7 @@ describe('', () => { }); it('while saving an edited bio', () => { - analytics.sendTrackingLogEvent = jest.fn(); + analytics.logEvent = jest.fn(); const tree = renderer .create(( @@ -90,8 +90,8 @@ describe('', () => { describe('handles analytics', () => { - it('calls sendTrackingLogEvent when mounting', () => { - analytics.sendTrackingLogEvent = jest.fn(); + it('calls logEvent when mounting', () => { + analytics.logEvent = jest.fn(); mount(( @@ -103,9 +103,9 @@ describe('', () => { )); - 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({ + 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({ username: 'test-username', }); }); diff --git a/src/config/analytics.js b/src/config/analytics.js deleted file mode 100644 index 0490e00..0000000 --- a/src/config/analytics.js +++ /dev/null @@ -1,13 +0,0 @@ -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 e3872ad..7630ef8 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 { identifyAuthenticatedUser, sendPageEvent } from './analytics/analytics'; +import { sendPageEvent } from './analytics/analytics'; import './index.scss'; @@ -21,6 +21,5 @@ if (apiClient.ensurePublicOrAuthencationAndCookies(window.location.pathname)) { ReactDOM.render(, document.getElementById('root')); - identifyAuthenticatedUser(); sendPageEvent(); }