From cf5b0921270e0a84a924eb6db471e3b782e835af Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 22 Jun 2022 11:58:09 +0100 Subject: [PATCH] Merge pull request #8505 from overleaf/jpa-compile-result-full [web] collect compile-result for ALL compiles GitOrigin-RevId: 5ba29fb352bc29d14c22e7e80fa556148d862137 --- .../src/Features/Compile/CompileController.js | 11 +++++--- .../src/Features/Compile/CompileManager.js | 9 ++++--- .../src/Compile/CompileControllerTests.js | 3 --- .../unit/src/Compile/CompileManagerTests.js | 26 +++++++++---------- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/services/web/app/src/Features/Compile/CompileController.js b/services/web/app/src/Features/Compile/CompileController.js index 51fe1725c4..91bbc2ad91 100644 --- a/services/web/app/src/Features/Compile/CompileController.js +++ b/services/web/app/src/Features/Compile/CompileController.js @@ -88,9 +88,12 @@ module.exports = CompileController = { if (pdfDownloadDomain && outputUrlPrefix) { pdfDownloadDomain += outputUrlPrefix } - let showFasterCompilesFeedbackUI = false - if (limits?.emitCompileResultEvent) { - showFasterCompilesFeedbackUI = true + + if (limits) { + // For a compile request to be sent to clsi we need limits. + // If we get here without having the limits object populated, it is + // a reasonable assumption to make that nothing was compiled. + // We need to know the limits in order to make use of the events. AnalyticsManager.recordEventForSession( req.session, 'compile-result-backend', @@ -113,7 +116,7 @@ module.exports = CompileController = { stats, timings, pdfDownloadDomain, - showFasterCompilesFeedbackUI, + showFasterCompilesFeedbackUI: limits?.showFasterCompilesFeedbackUI, }) } ) diff --git a/services/web/app/src/Features/Compile/CompileManager.js b/services/web/app/src/Features/Compile/CompileManager.js index 8d38617af0..62abd02059 100644 --- a/services/web/app/src/Features/Compile/CompileManager.js +++ b/services/web/app/src/Features/Compile/CompileManager.js @@ -171,10 +171,11 @@ module.exports = CompileManager = { CompileManager._getCompileBackendClassDetails( owner, limits.compileGroup, - (err, { compileBackendClass, emitCompileResultEvent }) => { + (err, { compileBackendClass, showFasterCompilesFeedbackUI }) => { if (err) return callback(err) limits.compileBackendClass = compileBackendClass - limits.emitCompileResultEvent = emitCompileResultEvent + limits.showFasterCompilesFeedbackUI = + showFasterCompilesFeedbackUI callback(null, limits) } ) @@ -250,7 +251,7 @@ module.exports = CompileManager = { if (compileGroup === 'standard') { return callback(null, { compileBackendClass: defaultBackendClass, - emitCompileResultEvent: false, + showFasterCompilesFeedbackUI: false, }) } SplitTestHandler.getAssignmentForMongoUser( @@ -263,7 +264,7 @@ module.exports = CompileManager = { callback(null, { compileBackendClass: variant === 'default' ? defaultBackendClass : variant, - emitCompileResultEvent: activeForUser, + showFasterCompilesFeedbackUI: activeForUser, }) } ) diff --git a/services/web/test/unit/src/Compile/CompileControllerTests.js b/services/web/test/unit/src/Compile/CompileControllerTests.js index 5bf949d082..e9897c6de8 100644 --- a/services/web/test/unit/src/Compile/CompileControllerTests.js +++ b/services/web/test/unit/src/Compile/CompileControllerTests.js @@ -114,7 +114,6 @@ describe('CompileController', function () { }, ], pdfDownloadDomain: 'https://compiles.overleaf.test', - showFasterCompilesFeedbackUI: false, }) ) }) @@ -156,7 +155,6 @@ describe('CompileController', function () { }, ], pdfDownloadDomain: 'https://compiles.overleaf.test/zone/b', - showFasterCompilesFeedbackUI: false, }) ) }) @@ -194,7 +192,6 @@ describe('CompileController', function () { JSON.stringify({ status: this.status, outputFiles: this.outputFiles, - showFasterCompilesFeedbackUI: false, }) ) }) diff --git a/services/web/test/unit/src/Compile/CompileManagerTests.js b/services/web/test/unit/src/Compile/CompileManagerTests.js index 3bbffe028d..3b7f7ef01c 100644 --- a/services/web/test/unit/src/Compile/CompileManagerTests.js +++ b/services/web/test/unit/src/Compile/CompileManagerTests.js @@ -221,7 +221,7 @@ describe('CompileManager', function () { compileGroup: this.group, compileBackendClass: 'e2', ownerAnalyticsId: 'abc', - emitCompileResultEvent: false, + showFasterCompilesFeedbackUI: false, }) .should.equal(true) }) @@ -245,13 +245,13 @@ describe('CompileManager', function () { beforeEach(function () { this.features.compileGroup = 'standard' }) - it('should return the default class and disable event', function (done) { + it('should return the default class and disable ui', function (done) { this.CompileManager.getProjectCompileLimits( this.project_id, - (err, { compileBackendClass, emitCompileResultEvent }) => { + (err, { compileBackendClass, showFasterCompilesFeedbackUI }) => { if (err) return done(err) expect(compileBackendClass).to.equal('e2') - expect(emitCompileResultEvent).to.equal(false) + expect(showFasterCompilesFeedbackUI).to.equal(false) done() } ) @@ -270,13 +270,13 @@ describe('CompileManager', function () { }) }) - it('should return the default class and disable event', function (done) { + it('should return the default class and disable ui', function (done) { this.CompileManager.getProjectCompileLimits( this.project_id, - (err, { compileBackendClass, emitCompileResultEvent }) => { + (err, { compileBackendClass, showFasterCompilesFeedbackUI }) => { if (err) return done(err) expect(compileBackendClass).to.equal('e2') - expect(emitCompileResultEvent).to.equal(false) + expect(showFasterCompilesFeedbackUI).to.equal(false) done() } ) @@ -291,13 +291,13 @@ describe('CompileManager', function () { variant: 'default', }) }) - it('should return the default class and enable event', function (done) { + it('should return the default class and enable ui', function (done) { this.CompileManager.getProjectCompileLimits( this.project_id, - (err, { compileBackendClass, emitCompileResultEvent }) => { + (err, { compileBackendClass, showFasterCompilesFeedbackUI }) => { if (err) return done(err) expect(compileBackendClass).to.equal('e2') - expect(emitCompileResultEvent).to.equal(true) + expect(showFasterCompilesFeedbackUI).to.equal(true) done() } ) @@ -311,13 +311,13 @@ describe('CompileManager', function () { variant: 'c2d', }) }) - it('should return the c2d class and enable event', function (done) { + it('should return the c2d class and enable ui', function (done) { this.CompileManager.getProjectCompileLimits( this.project_id, - (err, { compileBackendClass, emitCompileResultEvent }) => { + (err, { compileBackendClass, showFasterCompilesFeedbackUI }) => { if (err) return done(err) expect(compileBackendClass).to.equal('c2d') - expect(emitCompileResultEvent).to.equal(true) + expect(showFasterCompilesFeedbackUI).to.equal(true) done() } )