Merge pull request #4655 from overleaf/jel-gallery-event

Fix onclick event propagation

GitOrigin-RevId: bfcc3840e1768ed0f91e5fefbaee1738e36f6752

services/web/app/src/Features/Analytics/AnalyticsController.js

@@ -32,6 +32,7 @@ module.exports = {
     }
     const userId =
       SessionManager.getLoggedInUserId(req.session) || req.sessionID
+    delete req.body._csrf
     AnalyticsManager.recordEvent(userId, req.params.event, req.body)
     res.sendStatus(202)
   },

services/web/frontend/js/infrastructure/event-tracking.js

@@ -1,4 +1,3 @@
-import { postJSON } from './fetch-json'
 import sessionStorage from '../infrastructure/session-storage'
 
 const CACHE_KEY = 'mbEvents'
@@ -10,11 +9,7 @@ export function send(category, action, label, value) {
 }
 
 export function sendMB(key, segmentation = {}) {
-  postJSON(`/event/${key}`, { body: segmentation, keepalive: true }).catch(
+  sendBeacon(key, segmentation)
-    () => {
-      // ignore errors
-    }
-  )
 }
 
 export function sendMBOnce(key, segmentation = {}) {
@@ -39,3 +34,13 @@ export function sendMBSampled(key, body = {}, rate = 0.01) {
     sendMB(key, body)
   }
 }
+
+function sendBeacon(key, data) {
+  if (!navigator || !navigator.sendBeacon) return
+
+  data._csrf = window.csrfToken
+  const blob = new Blob([JSON.stringify(data)], {
+    type: 'application/json; charset=UTF-8',
+  })
+  navigator.sendBeacon(`/event/${key}`, blob)
+}

services/web/frontend/js/main/event.js

@@ -15,6 +15,7 @@
 import moment from 'moment'
 import App from '../base'
 import '../modules/localStorage'
+import { sendMB } from '../infrastructure/event-tracking'
 const CACHE_KEY = 'mbEvents'
 
 // keep track of how many heartbeats we've sent so we can calculate how
@@ -89,21 +90,7 @@ App.factory('eventTracking', function ($http, localStorage) {
       return (nextHeartbeat = moment().add(backoffSecs, 'seconds').toDate())
     },
 
-    sendMB(key, segmentation) {
+    sendMB,
-      if (segmentation == null) {
-        segmentation = {}
-      }
-      fetch(`/event/${key}`, {
-        method: 'POST',
-        body: JSON.stringify(segmentation),
-        keepalive: true,
-        headers: {
-          'X-CSRF-Token': window.csrfToken,
-          'Content-Type': 'application/json',
-          Accept: 'application/json',
-        },
-      })
-    },
 
     sendMBSampled(key, segmentation, rate = 0.01) {
       if (Math.random() < rate) {

services/web/test/unit/src/Analytics/AnalyticsControllerTests.js

@@ -61,21 +61,28 @@ describe('AnalyticsController', function () {
 
   describe('recordEvent', function () {
     beforeEach(function () {
+      const body = {
+        foo: 'stuff',
+        _csrf: 'atoken123',
+      }
       this.req = {
         params: {
           event: 'i_did_something',
         },
-        body: 'stuff',
+        body,
         sessionID: 'sessionIDHere',
         session: {},
       }
+
+      this.expectedData = Object.assign({}, body)
+      delete this.expectedData._csrf
     })
 
     it('should use the user_id', function (done) {
       this.SessionManager.getLoggedInUserId.returns('1234')
       this.controller.recordEvent(this.req, this.res)
       this.AnalyticsManager.recordEvent
-        .calledWith('1234', this.req.params.event, this.req.body)
+        .calledWith('1234', this.req.params.event, this.expectedData)
         .should.equal(true)
       done()
     })
@@ -83,7 +90,23 @@ describe('AnalyticsController', function () {
     it('should use the session id', function (done) {
       this.controller.recordEvent(this.req, this.res)
       this.AnalyticsManager.recordEvent
-        .calledWith(this.req.sessionID, this.req.params.event, this.req.body)
+        .calledWith(
+          this.req.sessionID,
+          this.req.params.event,
+          this.expectedData
+        )
+        .should.equal(true)
+      done()
+    })
+
+    it('should remove the CSRF token before sending to the manager', function (done) {
+      this.controller.recordEvent(this.req, this.res)
+      this.AnalyticsManager.recordEvent
+        .calledWith(
+          this.req.sessionID,
+          this.req.params.event,
+          this.expectedData
+        )
         .should.equal(true)
       done()
     })