Commit 864dc798 authored by Robert Knight's avatar Robert Knight

Remove Google Analytics from the client

We have had Google Analytics disabled for the Hypothesis client for over
a year and do not plan to re-enable it. We may need analytics for other
purposes in future (for example, to help inform product design). In that
case we will likely implement a solution that is a better fit for
whatever those needs end up being.

This commit removes the existing analytics code in the meantime so that
we don't spend time maintaining something that is not used.

Fixes #3027
parent cc7f8f08
......@@ -23,7 +23,6 @@ import ShareLinks from '../ShareLinks';
* FIXME: Refactor after root cause is addressed.
* See https://github.com/hypothesis/client/issues/1542
* @prop {string} shareUri - The URI to view the annotation on its own
* @prop {Object} analytics - Injected service
* @prop {Object} toastMessenger - Injected service
*/
......@@ -40,7 +39,6 @@ function selectionOverflowsInputElement() {
*/
function AnnotationShareControl({
annotation,
analytics,
toastMessenger,
group,
shareUri,
......@@ -162,12 +160,7 @@ function AnnotationShareControl({
link shares the annotation by itself.
</div>
)}
{showShareLinks && (
<ShareLinks
shareURI={shareUri}
analyticsEventName={analytics.events.ANNOTATION_SHARED}
/>
)}
{showShareLinks && <ShareLinks shareURI={shareUri} />}
</div>
<SvgIcon
name="pointer"
......@@ -180,6 +173,6 @@ function AnnotationShareControl({
);
}
AnnotationShareControl.injectedProps = ['analytics', 'toastMessenger'];
AnnotationShareControl.injectedProps = ['toastMessenger'];
export default withServices(AnnotationShareControl);
......@@ -8,7 +8,6 @@ import AnnotationShareControl, { $imports } from '../AnnotationShareControl';
describe('AnnotationShareControl', () => {
let fakeAnnotation;
let fakeAnalytics;
let fakeCopyToClipboard;
let fakeToastMessenger;
let fakeGroup;
......@@ -27,7 +26,6 @@ describe('AnnotationShareControl', () => {
return mount(
<AnnotationShareControl
annotation={fakeAnnotation}
analytics={fakeAnalytics}
toastMessenger={fakeToastMessenger}
group={fakeGroup}
shareUri={fakeShareUri}
......@@ -64,11 +62,6 @@ describe('AnnotationShareControl', () => {
uri: 'http://www.example.com',
};
fakeAnalytics = {
events: {
ANNOTATION_SHARED: 'whatever',
},
};
fakeCopyToClipboard = {
copyText: sinon.stub(),
};
......
......@@ -15,7 +15,6 @@ import MenuItem from './MenuItem';
* @prop {boolean} [isExpanded] - Whether the submenu for this group is expanded
* @prop {(expand: boolean) => any} onExpand -
* Callback invoked to expand or collapse the current group
* @prop {Object} analytics - Injected service
* @prop {Object} groups - Injected service
* @prop {Object} toastMessenger - Injected service
*/
......@@ -29,7 +28,6 @@ import MenuItem from './MenuItem';
* @param {GroupListItemProps} props
*/
function GroupListItem({
analytics,
isExpanded,
group,
groups: groupsService,
......@@ -46,7 +44,6 @@ function GroupListItem({
const isSelected = group.id === focusedGroupId;
const focusGroup = () => {
analytics.track(analytics.events.GROUP_SWITCH);
store.clearDirectLinkedGroupFetchFailed();
store.clearDirectLinkedIds();
groupsService.focus(group.id);
......@@ -55,7 +52,6 @@ function GroupListItem({
const leaveGroup = () => {
const message = `Are you sure you want to leave the group "${group.name}"?`;
if (window.confirm(message)) {
analytics.track(analytics.events.GROUP_LEAVE);
groupsService.leave(group.id);
}
};
......@@ -146,6 +142,6 @@ function GroupListItem({
);
}
GroupListItem.injectedProps = ['analytics', 'groups', 'toastMessenger'];
GroupListItem.injectedProps = ['groups', 'toastMessenger'];
export default withServices(GroupListItem);
......@@ -13,7 +13,6 @@ import Spinner from './Spinner';
/**
* @typedef ShareAnnotationsPanelProps
* @prop {Object} analytics - Injected service
* @prop {Object} toastMessenger - Injected service
*/
......@@ -26,7 +25,7 @@ import Spinner from './Spinner';
*
* @param {ShareAnnotationsPanelProps} props
*/
function ShareAnnotationsPanel({ analytics, toastMessenger }) {
function ShareAnnotationsPanel({ toastMessenger }) {
const store = useStoreProxy();
const mainFrame = store.mainFrame();
const focusedGroup = store.focusedGroup();
......@@ -105,10 +104,7 @@ function ShareAnnotationsPanel({ analytics, toastMessenger }) {
<em>Only Me</em>) annotations are only visible to you.
</span>
</p>
<ShareLinks
shareURI={shareURI}
analyticsEventName={analytics.events.DOCUMENT_SHARED}
/>
<ShareLinks shareURI={shareURI} />
</>
) : (
<p>
......@@ -122,6 +118,6 @@ function ShareAnnotationsPanel({ analytics, toastMessenger }) {
);
}
ShareAnnotationsPanel.injectedProps = ['analytics', 'toastMessenger'];
ShareAnnotationsPanel.injectedProps = ['toastMessenger'];
export default withServices(ShareAnnotationsPanel);
......@@ -7,7 +7,6 @@ import { withServices } from '../service-context';
* @prop {string} iconName - The name of the SVG icon to use for this link
* @prop {string} label - Accessible label/tooltip for link
* @prop {string} uri - URI for sharing this annotation
* @prop {() => void} onClick - Callback for analytics tracking
*/
/**
......@@ -15,14 +14,13 @@ import { withServices } from '../service-context';
*
* @param {ShareLinkProps} props
*/
function ShareLink({ label, iconName, uri, onClick }) {
function ShareLink({ label, iconName, uri }) {
return (
<li className="ShareLinks__link">
<a
aria-label={label}
href={uri}
title={label}
onClick={onClick}
target="_blank"
rel="noopener noreferrer"
>
......@@ -34,23 +32,13 @@ function ShareLink({ label, iconName, uri, onClick }) {
/**
* @typedef ShareLinksProps
* @prop {object} analytics
* @prop {string} analyticsEventName
* @prop {string} shareURI - The URL to share
*/
/**
* A list of share links to social-media platforms.
*/
function ShareLinks({ analytics, analyticsEventName, shareURI }) {
// Return a click callback that will track click events for the given
// social platform (`shareTarget`)
const trackShareClick = shareTarget => {
return () => {
analytics.track(analyticsEventName, shareTarget);
};
};
function ShareLinks({ shareURI }) {
// This is the double-encoded format needed for other services (the entire
// URI needs to be encoded because it's used as the value of querystring params)
const encodedURI = encodeURIComponent(shareURI);
......@@ -61,14 +49,12 @@ function ShareLinks({ analytics, analyticsEventName, shareURI }) {
iconName="twitter"
label="Tweet share link"
uri={`https://twitter.com/intent/tweet?url=${encodedURI}&hashtags=annotated`}
onClick={trackShareClick('twitter')}
/>
<ShareLink
iconName="facebook"
label="Share on Facebook"
uri={`https://www.facebook.com/sharer/sharer.php?u=${encodedURI}`}
onClick={trackShareClick('facebook')}
/>
<ShareLink
......@@ -77,12 +63,9 @@ function ShareLinks({ analytics, analyticsEventName, shareURI }) {
uri={`mailto:?subject=${encodeURIComponent(
"Let's Annotate"
)}&body=${encodedURI}`}
onClick={trackShareClick('email')}
/>
</ul>
);
}
ShareLinks.injectedProps = ['analytics'];
export default withServices(ShareLinks);
import { mount } from 'enzyme';
import { act } from 'preact/test-utils';
import { events } from '../../services/analytics';
import GroupListItem from '../GroupListItem';
import { $imports } from '../GroupListItem';
describe('GroupListItem', () => {
let fakeAnalytics;
let fakeCopyText;
let fakeToastMessenger;
let fakeGroupsService;
......@@ -34,11 +32,6 @@ describe('GroupListItem', () => {
clearDirectLinkedGroupFetchFailed: sinon.stub(),
};
fakeAnalytics = {
track: sinon.stub(),
events,
};
fakeToastMessenger = {
success: sinon.stub(),
error: sinon.stub(),
......@@ -88,7 +81,6 @@ describe('GroupListItem', () => {
toastMessenger={fakeToastMessenger}
group={fakeGroup}
groups={fakeGroupsService}
analytics={fakeAnalytics}
{...props}
/>
);
......@@ -106,7 +98,6 @@ describe('GroupListItem', () => {
wrapper.find('MenuItem').props().onClick();
assert.calledWith(fakeGroupsService.focus, fakeGroup.id);
assert.calledWith(fakeAnalytics.track, fakeAnalytics.events.GROUP_SWITCH);
});
it('clears the direct linked ids from the store when the group is clicked', () => {
......
......@@ -8,7 +8,6 @@ import mockImportedComponents from '../../../test-util/mock-imported-components'
describe('ShareAnnotationsPanel', () => {
let fakeStore;
let fakeAnalytics;
let fakeBouncerLink;
let fakePageSharingLink;
let fakeToastMessenger;
......@@ -22,20 +21,10 @@ describe('ShareAnnotationsPanel', () => {
const createShareAnnotationsPanel = props =>
mount(
<ShareAnnotationsPanel
analytics={fakeAnalytics}
toastMessenger={fakeToastMessenger}
{...props}
/>
<ShareAnnotationsPanel toastMessenger={fakeToastMessenger} {...props} />
);
beforeEach(() => {
fakeAnalytics = {
events: {
DOCUMENT_SHARED: 'whatever',
},
track: sinon.stub(),
};
fakeBouncerLink = 'http://hyp.is/go?url=http%3A%2F%2Fwww.example.com';
fakeCopyToClipboard = {
copyText: sinon.stub(),
......
......@@ -7,25 +7,13 @@ import { checkAccessibility } from '../../../test-util/accessibility';
import mockImportedComponents from '../../../test-util/mock-imported-components';
describe('ShareLinks', () => {
let fakeAnalytics;
const shareLink =
'https://hyp.is/go?url=https%3A%2F%2Fwww.example.com&group=testprivate';
const createComponent = props =>
mount(
<ShareLinks
analyticsEventName="potato-peeling"
analytics={fakeAnalytics}
shareURI={shareLink}
{...props}
/>
);
mount(<ShareLinks shareURI={shareLink} {...props} />);
beforeEach(() => {
fakeAnalytics = {
track: sinon.stub(),
};
$imports.$mock(mockImportedComponents());
});
......@@ -53,25 +41,12 @@ describe('ShareLinks', () => {
title: 'Share via email',
},
].forEach(testCase => {
it(`creates a share link for ${testCase.service} and tracks clicks`, () => {
it(`creates a share link for ${testCase.service}`, () => {
const wrapper = createComponent({ shareURI: shareLink });
const link = wrapper.find(`a[title="${testCase.title}"]`);
assert.equal(link.prop('href'), testCase.expectedURI);
// Assure tracking doesn't happen until clicked
// See https://github.com/hypothesis/client/issues/1566
assert.notCalled(fakeAnalytics.track);
// Now click...
link.simulate('click');
assert.calledWith(
fakeAnalytics.track,
'potato-peeling',
testCase.service
);
});
});
......
// @ts-nocheck - The `ga` property is unknown.
let loaded = false;
export default function loadGoogleAnalytics(trackingId) {
// small measure to make we do not accidentally
// load the analytics scripts more than once
if (loaded) {
return;
}
loaded = true;
/* eslint-disable */
// Google Analytics snippet to load the analytics script
(function (i, s, o, g, r, a, m) {
i['GoogleAnalyticsObject'] = r;
(i[r] =
i[r] ||
function () {
(i[r].q = i[r].q || []).push(arguments);
}),
(i[r].l = 1 * new Date());
(a = s.createElement(o)), (m = s.getElementsByTagName(o)[0]);
a.async = 1;
a.src = g;
m.parentNode.insertBefore(a, m);
})(
window,
document,
'script',
'https://www.google-analytics.com/analytics.js',
'ga'
);
ga('create', trackingId, 'auto');
// overrides helper that requires http or https protocols.
// obvious issue when it comes to extensions with protocols
// like "chrome-extension://" but isn't a huge need for us
// anywhere else as well.
// https://developers.google.com/analytics/devguides/collection/analyticsjs/tasks#disabling
ga('set', 'checkProtocolTask', null);
// anonymize collected IP addresses for GDPR
// https://developers.google.com/analytics/devguides/collection/analyticsjs/ip-anonymization
ga('set', 'anonymizeIp', true);
/* eslint-enable */
}
......@@ -7,7 +7,6 @@ import {
startServer as startRPCServer,
preStartServer as preStartRPCServer,
} from './cross-origin-rpc.js';
import addAnalytics from './ga';
import disableOpenerForExternalLinks from './util/disable-opener-for-external-links';
import { fetchConfig } from './config/fetch-config';
import * as sentry from './util/sentry';
......@@ -34,10 +33,6 @@ if (process.env.NODE_ENV !== 'production') {
require('preact/debug');
}
if (appConfig.googleAnalytics) {
addAnalytics(appConfig.googleAnalytics);
}
// Install Preact renderer options to work around browser quirks
rendererOptions.setupBrowserFixes();
......@@ -57,17 +52,6 @@ function setupRoute(groups, session, router) {
router.sync();
}
/**
* Send a page view event when the app starts up.
*
* We don't bother tracking route changes later because the client only uses a
* single route in a given session.
*/
// @inject
function sendPageView(analytics) {
analytics.sendPageView();
}
/**
* Fetch any persisted client-side defaults, and persist any app-state changes to
* those defaults
......@@ -107,7 +91,6 @@ import { ServiceContext } from './service-context';
// Services.
import bridgeService from '../shared/bridge';
import analyticsService from './services/analytics';
import annotationsService from './services/annotations';
import apiService from './services/api';
import apiRoutesService from './services/api-routes';
......@@ -145,7 +128,6 @@ function startApp(config, appEl) {
// Register services.
container
.register('analytics', analyticsService)
.register('annotationsService', annotationsService)
.register('api', apiService)
.register('apiRoutes', apiRoutesService)
......@@ -179,7 +161,6 @@ function startApp(config, appEl) {
// Initialize services.
container.run(persistDefaults);
container.run(autosave);
container.run(sendPageView);
container.run(setupApi);
container.run(setupRoute);
container.run(startRPCServer);
......
const VIA_REFERRER = /^https:\/\/(qa-)?via.hypothes.is\//;
export const events = {
ANNOTATION_CREATED: 'annotationCreated',
ANNOTATION_DELETED: 'annotationDeleted',
ANNOTATION_FLAGGED: 'annotationFlagged',
ANNOTATION_SHARED: 'annotationShared',
ANNOTATION_UPDATED: 'annotationUpdated',
DOCUMENT_SHARED: 'documentShared',
GROUP_LEAVE: 'groupLeave',
GROUP_SWITCH: 'groupSwitch',
GROUP_VIEW_ACTIVITY: 'groupViewActivity',
HIGHLIGHT_CREATED: 'highlightCreated',
HIGHLIGHT_UPDATED: 'highlightUpdated',
HIGHLIGHT_DELETED: 'highlightDeleted',
LOGIN_FAILURE: 'loginFailure',
LOGIN_SUCCESS: 'loginSuccessful',
LOGOUT_FAILURE: 'logoutFailure',
LOGOUT_SUCCESS: 'logoutSuccessful',
PAGE_NOTE_CREATED: 'pageNoteCreated',
PAGE_NOTE_UPDATED: 'pageNoteUpdated',
PAGE_NOTE_DELETED: 'pageNoteDeleted',
REPLY_CREATED: 'replyCreated',
REPLY_UPDATED: 'replyUpdated',
REPLY_DELETED: 'replyDeleted',
SIDEBAR_OPENED: 'sidebarOpened',
SIGN_UP_REQUESTED: 'signUpRequested',
};
/**
* Return a string identifying the context in which the client is being used.
*
* This is used as the "category" for analytics events to support comparing
* behavior across different environments in which the client is used.
*
* @param {Window} win
* @return {string}
*/
function clientType(win, settings = {}) {
const validTypes = [
'chrome-extension',
'firefox-extension',
'embed',
'bookmarklet',
'via',
];
let type;
// The preferred method for deciding what type of app is running is
// through the setting of the appType to one of the valid types above.
// However, we also want to capture app types where we were not given
// the appType setting explicitly - these are the app types that were
// added before we added the analytics logic
if (validTypes.indexOf((settings.appType || '').toLowerCase()) > -1) {
type = settings.appType.toLowerCase();
} else if (win.location.protocol === 'chrome-extension:') {
type = 'chrome-extension';
} else if (VIA_REFERRER.test(win.document.referrer)) {
type = 'via';
} else {
type = 'embed';
}
return type;
}
/**
* @typedef Analytics
* @prop {() => any} sendPageView
* @prop {(action: string, label?: string, value?: number) => void} track
* @prop {Object.<string, string>} events
*/
/**
* Analytics service for tracking page views and user interactions with the
* application.
* @param {Window} $window - Test seam
* @return {Analytics}
*/
// @inject
export default function analytics($window, settings) {
const category = clientType($window, settings);
const noop = () => {};
// Return the current analytics.js command queue function. This function
// is replaced when analytics.js fully loads.
//
// See https://developers.google.com/analytics/devguides/collection/analyticsjs/command-queue-reference
// @ts-ignore The window interface needs to be expanded to include this property
const commandQueue = () => $window.ga || noop;
return {
/**
* Track a page view when the app initially loads or changes route.
*
* See https://developers.google.com/analytics/devguides/collection/analyticsjs/pages
*/
sendPageView() {
const queue = commandQueue();
queue('send', 'pageview');
},
/**
* Track an event using Google Analytics.
*
* GA events have a category, action, label and value. The category is set
* to a string indicating the distribution method of the client (embed,
* browser extension, proxy service etc.).
*
* See https://developers.google.com/analytics/devguides/collection/analyticsjs/events
*
* @param {string} action -
* The event which happened. This should be a value from the `events` enum.
* @param [string] label
* A string argument to associate with the event. The meaning depends upon
* the event.
* @param [number] value
* A numeric value to associate with the event. The meaning depends upon
* the event.
*/
track(action, label, value) {
const queue = commandQueue();
queue('send', 'event', category, action, label, value);
},
events,
};
}
......@@ -14,14 +14,7 @@ const CACHE_TTL = 5 * 60 * 1000; // 5 minutes
*
* @inject
*/
export default function session(
analytics,
store,
api,
auth,
settings,
toastMessenger
) {
export default function session(store, api, auth, settings, toastMessenger) {
// Cache the result of load()
let lastLoad;
let lastLoadTime;
......@@ -129,15 +122,10 @@ export default function session(
return reload();
});
return loggedOut
.catch(function (err) {
toastMessenger.error('Log out failed');
analytics.track(analytics.events.LOGOUT_FAILURE);
throw new Error(err);
})
.then(function () {
analytics.track(analytics.events.LOGOUT_SUCCESS);
});
return loggedOut.catch(err => {
toastMessenger.error('Log out failed');
throw new Error(err);
});
}
/**
......
import analyticsService from '../analytics';
describe('analytics', function () {
let $windowStub;
let svc;
beforeEach(function () {
$windowStub = {
ga: sinon.stub(),
location: {
href: '',
protocol: 'https:',
},
document: {
referrer: '',
},
};
svc = analyticsService($windowStub, { appType: 'embed' });
});
function checkEventSent(
category,
event,
label = undefined,
value = undefined
) {
assert.calledWith(
$windowStub.ga,
'send',
'event',
category,
event,
label,
value
);
}
describe('applying global category based on environment contexts', function () {
it('sets the category to match the appType setting value', function () {
const validTypes = ['chrome-extension', 'embed', 'bookmarklet', 'via'];
validTypes.forEach(function (appType, index) {
analyticsService($windowStub, { appType: appType }).track(
'event' + index
);
checkEventSent(appType, 'event' + index);
});
});
it('sets category as embed if no other matches can be made', function () {
analyticsService($windowStub).track('eventA');
checkEventSent('embed', 'eventA');
});
it('sets category as via if url matches the via uri pattern', function () {
$windowStub.document.referrer = 'https://via.hypothes.is/';
analyticsService($windowStub).track('eventA');
checkEventSent('via', 'eventA');
// match staging as well
$windowStub.document.referrer = 'https://qa-via.hypothes.is/';
analyticsService($windowStub).track('eventB');
checkEventSent('via', 'eventB');
});
it('sets category as chrome-extension if protocol matches chrome-extension:', function () {
$windowStub.location.protocol = 'chrome-extension:';
analyticsService($windowStub).track('eventA');
checkEventSent('chrome-extension', 'eventA');
});
});
describe('#track', () => {
it('allows custom labels to be sent for an event', function () {
svc.track('eventA', 'labelA');
checkEventSent('embed', 'eventA', 'labelA');
});
it('allows custom metricValues to be sent for an event', function () {
svc.track('eventA', null, 242.2);
checkEventSent('embed', 'eventA', null, 242.2);
});
it('allows custom metricValues and labels to be sent for an event', function () {
svc.track('eventA', 'labelabc', 242.2);
checkEventSent('embed', 'eventA', 'labelabc', 242.2);
});
});
describe('#sendPageView', () => {
it('sends a page view hit', () => {
svc.sendPageView();
assert.calledWith($windowStub.ga, 'send', 'pageview');
});
});
context('when Google Analytics is not loaded', () => {
it('analytics methods can be called but do nothing', () => {
const ga = $windowStub.ga;
delete $windowStub.ga;
const svc = analyticsService($windowStub, {});
svc.track('someEvent');
svc.sendPageView();
assert.notCalled(ga);
});
});
it('sends events to the current analytics.js command queue', () => {
const initialQueue = $windowStub.ga;
const queueAfterLoad = sinon.stub();
// Send a page view hit before analytics.js loads.
svc.sendPageView();
assert.called($windowStub.ga);
// Simulate analytics.js loading, which will replace the command queue.
$windowStub.ga = queueAfterLoad;
initialQueue.reset();
// Report a user interaction after analytics.js loads.
svc.track('someEvent');
// Check that the event was passed to the right queue.
assert.notCalled(initialQueue);
assert.called(queueAfterLoad);
checkEventSent('embed', 'someEvent');
});
});
import EventEmitter from 'tiny-emitter';
import { events as analyticsEvents } from '../analytics';
import sessionFactory from '../session';
import { $imports } from '../session';
import { Injector } from '../../../shared/injector';
describe('sidebar/services/session', function () {
let fakeAnalytics;
let fakeAuth;
let fakeSentry;
let fakeServiceConfig;
......@@ -26,10 +24,6 @@ describe('sidebar/services/session', function () {
userid: null,
};
fakeAnalytics = {
track: sinon.stub(),
events: analyticsEvents,
};
fakeStore = {
profile: sinon.stub().returns(currentProfile),
updateProfile: sinon.stub().callsFake(newProfile => {
......@@ -61,7 +55,6 @@ describe('sidebar/services/session', function () {
});
session = new Injector()
.register('analytics', { value: fakeAnalytics })
.register('store', { value: fakeStore })
.register('api', { value: fakeApi })
.register('auth', { value: fakeAuth })
......@@ -274,15 +267,6 @@ describe('sidebar/services/session', function () {
});
});
it('tracks successful logout actions in analytics', () => {
return session.logout().then(() => {
assert.calledWith(
fakeAnalytics.track,
fakeAnalytics.events.LOGOUT_SUCCESS
);
});
});
it('updates the profile after logging out', () => {
return session.logout().then(() => {
assert.calledOnce(fakeStore.updateProfile);
......
......@@ -46,7 +46,6 @@
* @typedef SidebarConfig
* @prop {string} apiUrl
* @prop {string} authDomain
* @prop {string} [googleAnalytics]
* @prop {string} oauthClientId
* @prop {string[]} rpcAllowedOrigins
* @prop {SentryConfig} [sentry]
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment