From 9d6fb2ca2650f680faaa24a872aa07abba4de46c Mon Sep 17 00:00:00 2001 From: Sabe Jones Date: Thu, 5 Nov 2020 15:44:30 -0600 Subject: [PATCH] Revert "Analytics: track generic events through the server (#12735)" This reverts commit 48dbe547c0640a39fd350239b4a4691267cfbade. --- .../POST-analytics_track_eventName.test.js | 28 ----------- test/helpers/api-integration/requester.js | 1 - .../achievements/dropCapReached.vue | 4 +- .../header/notifications/dropCapReached.vue | 2 +- .../src/components/header/userDropdown.vue | 2 +- website/client/src/libs/analytics.js | 13 ++--- website/client/src/store/actions/analytics.js | 7 --- website/client/src/store/actions/index.js | 2 - website/client/vue.config.js | 4 -- .../server/controllers/api-v3/challenges.js | 5 -- website/server/controllers/api-v3/quests.js | 8 ++-- .../server/controllers/top-level/analytics.js | 47 ------------------- .../controllers/top-level/payments/paypal.js | 2 +- .../controllers/top-level/payments/stripe.js | 2 +- website/server/libs/analyticsService.js | 8 ++-- website/server/libs/cron.js | 2 +- 16 files changed, 17 insertions(+), 120 deletions(-) delete mode 100644 test/api/v3/integration/analytics/POST-analytics_track_eventName.test.js delete mode 100644 website/client/src/store/actions/analytics.js delete mode 100644 website/server/controllers/top-level/analytics.js diff --git a/test/api/v3/integration/analytics/POST-analytics_track_eventName.test.js b/test/api/v3/integration/analytics/POST-analytics_track_eventName.test.js deleted file mode 100644 index eb00ee8927..0000000000 --- a/test/api/v3/integration/analytics/POST-analytics_track_eventName.test.js +++ /dev/null @@ -1,28 +0,0 @@ -import { - generateUser, - requester, - translate as t, -} from '../../../../helpers/api-integration/v3'; -import { mockAnalyticsService as analytics } from '../../../../../website/server/libs/analyticsService'; - -describe('POST /analytics/track/:eventName', () => { - it('requires authentication', async () => { - await expect(requester().post('/analytics/track/event')).to.eventually.be.rejected.and.eql({ - code: 401, - error: 'NotAuthorized', - message: t('missingAuthHeaders'), - }); - }); - - it('calls res.analytics', async () => { - const user = await generateUser(); - sandbox.spy(analytics, 'track'); - - const requestWithHeaders = requester(user, { 'x-client': 'habitica-web' }); - await requestWithHeaders.post('/analytics/track/eventName', { data: 'example' }, { 'x-client': 'habitica-web' }); - expect(analytics.track).to.be.calledOnce; - expect(analytics.track).to.be.calledWith('eventName', sandbox.match({ data: 'example' })); - - sandbox.restore(); - }); -}); diff --git a/test/helpers/api-integration/requester.js b/test/helpers/api-integration/requester.js index f5c63b9d14..a01aa1e227 100644 --- a/test/helpers/api-integration/requester.js +++ b/test/helpers/api-integration/requester.js @@ -41,7 +41,6 @@ function _requestMaker (user, method, additionalSets = {}) { || route.indexOf('/amazon') === 0 || route.indexOf('/stripe') === 0 || route.indexOf('/qr-code') === 0 - || route.indexOf('/analytics') === 0 ) { url += `${route}`; } else { diff --git a/website/client/src/components/achievements/dropCapReached.vue b/website/client/src/components/achievements/dropCapReached.vue index 0c01057752..a804fd78fa 100644 --- a/website/client/src/components/achievements/dropCapReached.vue +++ b/website/client/src/components/achievements/dropCapReached.vue @@ -232,7 +232,7 @@ export default { eventCategory: 'drop-cap-reached', eventAction: 'click', eventLabel: 'Drop Cap Reached > Modal > Wiki', - }, { trackOnServer: true }); + }); }, toLearnMore () { Analytics.track({ @@ -240,7 +240,7 @@ export default { eventCategory: 'drop-cap-reached', eventAction: 'click', eventLabel: 'Drop Cap Reached > Modal > Subscriptions', - }, { trackOnServer: true }); + }); this.close(); this.$router.push('/user/settings/subscription'); diff --git a/website/client/src/components/header/notifications/dropCapReached.vue b/website/client/src/components/header/notifications/dropCapReached.vue index 6fcab511d2..34a133b64e 100644 --- a/website/client/src/components/header/notifications/dropCapReached.vue +++ b/website/client/src/components/header/notifications/dropCapReached.vue @@ -36,7 +36,7 @@ export default { eventCategory: 'drop-cap-reached', eventAction: 'click', eventLabel: 'Drop Cap Reached > Notification Click', - }, { trackOnServer: true }); + }); }, }, }; diff --git a/website/client/src/components/header/userDropdown.vue b/website/client/src/components/header/userDropdown.vue index cb058247b0..8434defbef 100644 --- a/website/client/src/components/header/userDropdown.vue +++ b/website/client/src/components/header/userDropdown.vue @@ -183,7 +183,7 @@ export default { eventCategory: 'button', eventAction: 'click', eventLabel: 'User Dropdown > Subscriptions', - }, { trackOnServer: true }); + }); this.$router.push({ name: 'subscription' }); }, diff --git a/website/client/src/libs/analytics.js b/website/client/src/libs/analytics.js index 3b2bed28ec..d7af60b8a1 100644 --- a/website/client/src/libs/analytics.js +++ b/website/client/src/libs/analytics.js @@ -82,21 +82,14 @@ export function setUser () { window.ga('set', { userId: user._id }); } -export function track (properties, options = {}) { +export function track (properties) { // Use nextTick to avoid blocking the UI Vue.nextTick(() => { if (_doesNotHaveRequiredFields(properties)) return; if (_doesNotHaveAllowedHitType(properties)) return; - const trackOnServer = options && options.trackOnServer === true; - if (trackOnServer === true) { - // Track an event on the server - const store = getStore(); - store.dispatch('analytics:trackEvent', properties); - } else { - amplitude.getInstance().logEvent(properties.eventAction, properties); - window.ga('send', properties); - } + amplitude.getInstance().logEvent(properties.eventAction, properties); + window.ga('send', properties); }); } diff --git a/website/client/src/store/actions/analytics.js b/website/client/src/store/actions/analytics.js deleted file mode 100644 index 1d104d749c..0000000000 --- a/website/client/src/store/actions/analytics.js +++ /dev/null @@ -1,7 +0,0 @@ -import axios from 'axios'; - -export async function trackEvent (store, params) { - const url = `/analytics/track/${params.eventAction}`; - - await axios.post(url, params); -} diff --git a/website/client/src/store/actions/index.js b/website/client/src/store/actions/index.js index adc7a38fb2..947be42878 100644 --- a/website/client/src/store/actions/index.js +++ b/website/client/src/store/actions/index.js @@ -17,7 +17,6 @@ import * as shops from './shops'; import * as snackbars from './snackbars'; import * as worldState from './worldState'; import * as news from './news'; -import * as analytics from './analytics'; // Actions should be named as 'actionName' and can be accessed as 'namespace:actionName' // Example: fetch in user.js -> 'user:fetch' @@ -40,7 +39,6 @@ const actions = flattenAndNamespace({ snackbars, worldState, news, - analytics, }); export default actions; diff --git a/website/client/vue.config.js b/website/client/vue.config.js index 6d446cb1f6..aae8725c33 100644 --- a/website/client/vue.config.js +++ b/website/client/vue.config.js @@ -148,10 +148,6 @@ module.exports = { target: DEV_BASE_URL, changeOrigin: true, }, - '^/analytics': { - target: DEV_BASE_URL, - changeOrigin: true, - }, }, }, }; diff --git a/website/server/controllers/api-v3/challenges.js b/website/server/controllers/api-v3/challenges.js index 84801e28a5..3e8bea0a6c 100644 --- a/website/server/controllers/api-v3/challenges.js +++ b/website/server/controllers/api-v3/challenges.js @@ -221,7 +221,6 @@ api.createChallenge = { groupID: group._id, groupName: group.privacy === 'private' ? null : group.name, groupType: group._id === TAVERN_ID ? 'tavern' : group.type, - headers: req.headers, }); res.respond(201, response); @@ -287,7 +286,6 @@ api.joinChallenge = { groupID: group._id, groupName: group.privacy === 'private' ? null : group.name, groupType: group._id === TAVERN_ID ? 'tavern' : group.type, - headers: req.headers, }); res.respond(200, response); @@ -337,7 +335,6 @@ api.leaveChallenge = { groupID: challenge.group._id, groupName: challenge.group.privacy === 'private' ? null : challenge.group.name, groupType: challenge.group._id === TAVERN_ID ? 'tavern' : challenge.group.type, - headers: req.headers, }); res.respond(200, {}); @@ -751,7 +748,6 @@ api.deleteChallenge = { groupID: challenge.group._id, groupName: challenge.group.privacy === 'private' ? null : challenge.group.name, groupType: challenge.group._id === TAVERN_ID ? 'tavern' : challenge.group.type, - headers: req.headers, }); res.respond(200, {}); @@ -802,7 +798,6 @@ api.selectChallengeWinner = { groupID: challenge.group._id, groupName: challenge.group.privacy === 'private' ? null : challenge.group.name, groupType: challenge.group._id === TAVERN_ID ? 'tavern' : challenge.group.type, - headers: req.headers, }); res.respond(200, {}); diff --git a/website/server/controllers/api-v3/quests.js b/website/server/controllers/api-v3/quests.js index b0e2c08e6c..dd1105e58d 100644 --- a/website/server/controllers/api-v3/quests.js +++ b/website/server/controllers/api-v3/quests.js @@ -156,7 +156,7 @@ api.inviteToQuest = { questName: questKey, uuid: user._id, headers: req.headers, - }, true); + }); }, }; @@ -217,7 +217,7 @@ api.acceptQuest = { questName: group.quest.key, uuid: user._id, headers: req.headers, - }, true); + }); }, }; @@ -278,7 +278,7 @@ api.rejectQuest = { questName: group.quest.key, uuid: user._id, headers: req.headers, - }, true); + }); }, }; @@ -338,7 +338,7 @@ api.forceStart = { questName: group.quest.key, uuid: user._id, headers: req.headers, - }, true); + }); }, }; diff --git a/website/server/controllers/top-level/analytics.js b/website/server/controllers/top-level/analytics.js deleted file mode 100644 index f663a10dcb..0000000000 --- a/website/server/controllers/top-level/analytics.js +++ /dev/null @@ -1,47 +0,0 @@ -import { - NotAuthorized, -} from '../../libs/errors'; -import { - authWithHeaders, -} from '../../middlewares/auth'; - -const api = {}; - -/** - * @apiIgnore Analytics are considered part of the private API - * @api {post} /analytics/track/:eventName Track a generic analytics event - * @apiName AnalyticsTrack - * @apiGroup Analytics - * - * @apiSuccess {Object} data An empty object - * */ -api.trackEvent = { - method: 'POST', - url: '/analytics/track/:eventName', - // we authenticate these requests to make sure they actually came from a real user - middlewares: [authWithHeaders()], - async handler (req, res) { - // As of now only web can track events using this route - if (req.headers['x-client'] !== 'habitica-web') { - throw new NotAuthorized('Only habitica.com is allowed to track analytics events.'); - } - - const { user } = res.locals; - const eventProperties = req.body; - - res.analytics.track(req.params.eventName, { - uuid: user._id, - headers: req.headers, - category: 'behaviour', - gaLabel: 'local', - // hitType: 'event', sent from the client - ...eventProperties, - }); - - // not using res.respond - // because we don't want to send back notifications and other user-related data - res.status(200).send({}); - }, -}; - -export default api; diff --git a/website/server/controllers/top-level/payments/paypal.js b/website/server/controllers/top-level/payments/paypal.js index 1432ceea90..ba3eaf64b3 100644 --- a/website/server/controllers/top-level/payments/paypal.js +++ b/website/server/controllers/top-level/payments/paypal.js @@ -63,7 +63,7 @@ api.checkoutSuccess = { if (!customerId) throw new BadRequest(apiError('missingCustomerId')); await paypalPayments.checkoutSuccess({ - user, gemsBlock, gift, paymentId, customerId, headers: req.headers, + user, gemsBlock, gift, paymentId, customerId, }); if (req.query.noRedirect) { diff --git a/website/server/controllers/top-level/payments/stripe.js b/website/server/controllers/top-level/payments/stripe.js index fec9230524..4a01108306 100644 --- a/website/server/controllers/top-level/payments/stripe.js +++ b/website/server/controllers/top-level/payments/stripe.js @@ -35,7 +35,7 @@ api.checkout = { const { groupId, coupon, gemsBlock } = req.query; await stripePayments.checkout({ - token, user, gemsBlock, gift, sub, groupId, coupon, headers: req.headers, + token, user, gemsBlock, gift, sub, groupId, coupon, }); res.respond(200, {}); diff --git a/website/server/libs/analyticsService.js b/website/server/libs/analyticsService.js index f990b60a9d..45de7fef6c 100644 --- a/website/server/libs/analyticsService.js +++ b/website/server/libs/analyticsService.js @@ -174,7 +174,7 @@ function _formatDataForAmplitude (data) { return ampData; } -function _sendDataToAmplitude (eventType, data, loggerOnly) { +function _sendDataToAmplitude (eventType, data) { const amplitudeData = _formatDataForAmplitude(data); amplitudeData.event_type = eventType; @@ -183,8 +183,6 @@ function _sendDataToAmplitude (eventType, data, loggerOnly) { logger.info('Amplitude Event', amplitudeData); } - if (loggerOnly) return Promise.resolve(null); - return amplitude .track(amplitudeData) .catch(err => logger.error(err, 'Error while sending data to Amplitude.')); @@ -314,9 +312,9 @@ function _setOnce (dataToSetOnce, uuid) { } // There's no error handling directly here because it's handled inside _sendDataTo{Amplitude|Google} -async function track (eventType, data, loggerOnly = false) { +async function track (eventType, data) { const promises = [ - _sendDataToAmplitude(eventType, data, loggerOnly), + _sendDataToAmplitude(eventType, data), _sendDataToGoogle(eventType, data), ]; if (data.user && data.user.registeredThrough) { diff --git a/website/server/libs/cron.js b/website/server/libs/cron.js index f8755f7337..67804c705e 100644 --- a/website/server/libs/cron.js +++ b/website/server/libs/cron.js @@ -225,7 +225,7 @@ function trackCronAnalytics (analytics, user, _progress, options) { user, questName: user.party.quest.key, headers: options.headers, - }, true); + }); } }