Răsfoiți Sursa

Logs UI variant without popup (#3794)

* Add logs UI without pop-up variant

* Implement frontend for logs UI without pop-up

* Add logs UI variants to admin panel and front user info

* Fix existing UI subvariant window global

GitOrigin-RevId: 075db56032354d782e489b5235925f55b1a67e0b
Paulo Jorge Reis 5 ani în urmă
părinte
comite
3f0e897e32

+ 51 - 9
services/web/app/src/Features/Helpers/NewLogsUI.js

@@ -1,29 +1,71 @@
 const { ObjectId } = require('mongodb')
 const { ObjectId } = require('mongodb')
 const Settings = require('settings-sharelatex')
 const Settings = require('settings-sharelatex')
 
 
-function shouldUserSeeNewLogsUI(user) {
+const EXISTING_UI = { newLogsUI: false, subvariant: null }
+const NEW_UI_WITH_POPUP = {
+  newLogsUI: true,
+  subvariant: 'new-logs-ui-with-popup'
+}
+const NEW_UI_WITHOUT_POPUP = {
+  newLogsUI: true,
+  subvariant: 'new-logs-ui-without-popup'
+}
+
+function _getVariantForPercentile(
+  percentile,
+  newLogsUIWithPopupPercentage,
+  newLogsUIWithoutPopupPercentage
+) {
+  // The thresholds below are upper thresholds
+  const newLogsUIThreshold = newLogsUIWithPopupPercentage
+  const newLogsUIWithoutPopupThreshold =
+    newLogsUIWithPopupPercentage + newLogsUIWithoutPopupPercentage
+
+  // The partitions for each of the variants (range is 0 to 99) are defined as:
+  // * New UI with pop-up: 0 to newLogsUIThreshold (exc)
+  // * New UI without pop-up: newLogsUIThreshold (inc) to newLogsUIWithoutPopupThreshold (exc)
+  // * Existing UI: newLogsUIWithoutPopupThreshold (inc) to 99
+  if (percentile < newLogsUIThreshold) {
+    return NEW_UI_WITH_POPUP
+  } else if (
+    percentile >= newLogsUIThreshold &&
+    percentile < newLogsUIWithoutPopupThreshold
+  ) {
+    return NEW_UI_WITHOUT_POPUP
+  } else {
+    return EXISTING_UI
+  }
+}
+
+function getNewLogsUIVariantForUser(user) {
   const {
   const {
     _id: userId,
     _id: userId,
     alphaProgram: isAlphaUser,
     alphaProgram: isAlphaUser,
     betaProgram: isBetaUser
     betaProgram: isBetaUser
   } = user
   } = user
   if (!userId) {
   if (!userId) {
-    return false
+    return EXISTING_UI
   }
   }
 
 
   const userIdAsPercentile = (ObjectId(userId).getTimestamp() / 1000) % 100
   const userIdAsPercentile = (ObjectId(userId).getTimestamp() / 1000) % 100
 
 
   if (isAlphaUser) {
   if (isAlphaUser) {
-    return true
-  } else if (isBetaUser && userIdAsPercentile < Settings.logsUIPercentageBeta) {
-    return true
-  } else if (userIdAsPercentile < Settings.logsUIPercentage) {
-    return true
+    return NEW_UI_WITH_POPUP
+  } else if (isBetaUser) {
+    return _getVariantForPercentile(
+      userIdAsPercentile,
+      Settings.logsUIPercentageBeta,
+      Settings.logsUIPercentageWithoutPopupBeta
+    )
   } else {
   } else {
-    return false
+    return _getVariantForPercentile(
+      userIdAsPercentile,
+      Settings.logsUIPercentage,
+      Settings.logsUIPercentageWithoutPopup
+    )
   }
   }
 }
 }
 
 
 module.exports = {
 module.exports = {
-  shouldUserSeeNewLogsUI
+  getNewLogsUIVariantForUser
 }
 }

+ 4 - 2
services/web/app/src/Features/Project/ProjectController.js

@@ -37,7 +37,7 @@ const BrandVariationsHandler = require('../BrandVariations/BrandVariationsHandle
 const UserController = require('../User/UserController')
 const UserController = require('../User/UserController')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const Modules = require('../../infrastructure/Modules')
 const Modules = require('../../infrastructure/Modules')
-const { shouldUserSeeNewLogsUI } = require('../Helpers/NewLogsUI')
+const { getNewLogsUIVariantForUser } = require('../Helpers/NewLogsUI')
 
 
 const _ssoAvailable = (affiliation, session, linkedInstitutionIds) => {
 const _ssoAvailable = (affiliation, session, linkedInstitutionIds) => {
   if (!affiliation.institution) return false
   if (!affiliation.institution) return false
@@ -800,7 +800,8 @@ const ProjectController = {
               })
               })
             }
             }
 
 
-            const userShouldSeeNewLogsUI = shouldUserSeeNewLogsUI(user)
+            const logsUIVariant = getNewLogsUIVariantForUser(user)
+            const userShouldSeeNewLogsUI = logsUIVariant.newLogsUI
             const wantsOldLogsUI =
             const wantsOldLogsUI =
               req.query && req.query.new_logs_ui === 'false'
               req.query && req.query.new_logs_ui === 'false'
 
 
@@ -860,6 +861,7 @@ const ProjectController = {
               wsUrl,
               wsUrl,
               showSupport: Features.hasFeature('support'),
               showSupport: Features.hasFeature('support'),
               showNewLogsUI: userShouldSeeNewLogsUI && !wantsOldLogsUI,
               showNewLogsUI: userShouldSeeNewLogsUI && !wantsOldLogsUI,
+              logsUISubvariant: logsUIVariant.subvariant,
               showNewNavigationUI:
               showNewNavigationUI:
                 req.query && req.query.new_navigation_ui === 'true',
                 req.query && req.query.new_navigation_ui === 'true',
               showReactFileTree: !wantsOldFileTreeUI,
               showReactFileTree: !wantsOldFileTreeUI,

+ 2 - 0
services/web/app/views/project/editor/pdf.pug

@@ -30,6 +30,7 @@ div.full-size.pdf(ng-controller="PdfController")
 			on-set-split-layout="setPdfSplitLayout"
 			on-set-split-layout="setPdfSplitLayout"
 			on-set-full-layout="setPdfFullLayout"
 			on-set-full-layout="setPdfFullLayout"
 			on-stop-compilation="stop"
 			on-stop-compilation="stop"
+			variant-with-first-error-popup="logsUISubvariant === 'new-logs-ui-with-popup'"
 			show-logs="shouldShowLogs"
 			show-logs="shouldShowLogs"
 			on-log-entry-location-click="openInEditor"
 			on-log-entry-location-click="openInEditor"
 		)
 		)
@@ -415,3 +416,4 @@ script(type='text/ng-template', id='clearCacheModalTemplate')
 
 
 script(type="text/javascript").
 script(type="text/javascript").
 	window.showNewLogsUI = #{showNewLogsUI || false}
 	window.showNewLogsUI = #{showNewLogsUI || false}
+	window.logsUISubvariant = !{logsUISubvariant ? '"' + logsUISubvariant + '"' : 'null'}

+ 3 - 0
services/web/config/settings.defaults.coffee

@@ -227,7 +227,10 @@ module.exports = settings =
 
 
 	# Compile UI rollout percentages
 	# Compile UI rollout percentages
 	logsUIPercentageBeta: parseInt(process.env['LOGS_UI_PERCENTAGE_BETA'] || '0', 10)
 	logsUIPercentageBeta: parseInt(process.env['LOGS_UI_PERCENTAGE_BETA'] || '0', 10)
+	logsUIPercentageWithoutPopupBeta: parseInt(process.env['LOGS_UI_WITHOUT_POPUP_PERCENTAGE_BETA'] || '0', 10)
+
 	logsUIPercentage: parseInt(process.env['LOGS_UI_PERCENTAGE'] || '0', 10)
 	logsUIPercentage: parseInt(process.env['LOGS_UI_PERCENTAGE'] || '0', 10)
+	logsUIPercentageWithoutPopup: parseInt(process.env['LOGS_UI_WITHOUT_POPUP_PERCENTAGE'] || '0', 10)
 
 
 	# cookie domain
 	# cookie domain
 	# use full domain for cookies to only be accessible from that domain,
 	# use full domain for cookies to only be accessible from that domain,

+ 3 - 0
services/web/frontend/js/features/preview/components/preview-pane.js

@@ -22,6 +22,7 @@ function PreviewPane({
   pdfDownloadUrl,
   pdfDownloadUrl,
   onLogEntryLocationClick,
   onLogEntryLocationClick,
   showLogs,
   showLogs,
+  variantWithFirstErrorPopup = true,
   splitLayout
   splitLayout
 }) {
 }) {
   const { t } = useTranslation()
   const { t } = useTranslation()
@@ -67,6 +68,7 @@ function PreviewPane({
     !compilerState.isCompiling
     !compilerState.isCompiling
 
 
   const showFirstErrorPopUp =
   const showFirstErrorPopUp =
+    variantWithFirstErrorPopup &&
     nErrors > 0 &&
     nErrors > 0 &&
     !seenLogsForCurrentCompile &&
     !seenLogsForCurrentCompile &&
     !dismissedFirstErrorPopUp &&
     !dismissedFirstErrorPopUp &&
@@ -168,6 +170,7 @@ PreviewPane.propTypes = {
   outputFiles: PropTypes.array,
   outputFiles: PropTypes.array,
   pdfDownloadUrl: PropTypes.string,
   pdfDownloadUrl: PropTypes.string,
   showLogs: PropTypes.bool.isRequired,
   showLogs: PropTypes.bool.isRequired,
+  variantWithFirstErrorPopup: PropTypes.bool,
   splitLayout: PropTypes.bool.isRequired
   splitLayout: PropTypes.bool.isRequired
 }
 }
 
 

+ 4 - 1
services/web/frontend/js/ide/pdf/controllers/PdfController.js

@@ -32,6 +32,8 @@ App.controller('PdfController', function(
   $scope.pdf.clearingCache = false
   $scope.pdf.clearingCache = false
   $scope.shouldShowLogs = false
   $scope.shouldShowLogs = false
 
 
+  $scope.logsUISubvariant = window.logsUISubvariant
+
   // view logic to check whether the files dropdown should "drop up" or "drop down"
   // view logic to check whether the files dropdown should "drop up" or "drop down"
   $scope.shouldDropUp = false
   $scope.shouldDropUp = false
 
 
@@ -708,7 +710,8 @@ App.controller('PdfController', function(
         errors: $scope.pdf.logEntries.errors.length,
         errors: $scope.pdf.logEntries.errors.length,
         warnings: $scope.pdf.logEntries.warnings.length,
         warnings: $scope.pdf.logEntries.warnings.length,
         typesetting: $scope.pdf.logEntries.typesetting.length,
         typesetting: $scope.pdf.logEntries.typesetting.length,
-        newLogsUI: window.showNewLogsUI
+        newLogsUI: window.showNewLogsUI,
+        subvariant: window.logsUISubvariant
       }
       }
       eventTracking.sendMBSampled(
       eventTracking.sendMBSampled(
         'compile-result',
         'compile-result',

+ 109 - 28
services/web/test/unit/src/HelperFiles/NewLogsUITests.js

@@ -13,6 +13,20 @@ describe('NewLogsUI helper', function() {
     return ObjectId.createFromTime(time).toString()
     return ObjectId.createFromTime(time).toString()
   }
   }
 
 
+  function isExistingUI(variant) {
+    return !variant.newLogsUI && !variant.subvariant
+  }
+
+  function isNewUIWithPopup(variant) {
+    return variant.newLogsUI && variant.subvariant === 'new-logs-ui-with-popup'
+  }
+
+  function isNewUIWithoutPopup(variant) {
+    return (
+      variant.newLogsUI && variant.subvariant === 'new-logs-ui-without-popup'
+    )
+  }
+
   beforeEach(function() {
   beforeEach(function() {
     this.user = {
     this.user = {
       alphaProgram: false,
       alphaProgram: false,
@@ -21,7 +35,9 @@ describe('NewLogsUI helper', function() {
     }
     }
     this.settings = {
     this.settings = {
       logsUIPercentageBeta: 0,
       logsUIPercentageBeta: 0,
-      logsUIPercentage: 0
+      logsUIPercentageWithoutPopupBeta: 0,
+      logsUIPercentage: 0,
+      logsUIPercentageWithoutPopup: 0
     }
     }
     NewLogsUI = SandboxedModule.require(MODULE_PATH, {
     NewLogsUI = SandboxedModule.require(MODULE_PATH, {
       requires: {
       requires: {
@@ -31,52 +47,117 @@ describe('NewLogsUI helper', function() {
     })
     })
   })
   })
 
 
-  it('should show the new logs ui for alpha users', function() {
+  it('should always show the new UI with popup for alpha users', function() {
     this.user.alphaProgram = true
     this.user.alphaProgram = true
-    expect(NewLogsUI.shouldUserSeeNewLogsUI(this.user)).to.be.true
+    for (const percentile of [0, 20, 40, 60, 80]) {
+      this.user._id = userIdFromTime(percentile)
+      const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+      expect(isNewUIWithPopup(variant)).to.be.true
+    }
   })
   })
 
 
   describe('for beta users', function() {
   describe('for beta users', function() {
     beforeEach(function() {
     beforeEach(function() {
       this.user.betaProgram = true
       this.user.betaProgram = true
     })
     })
-    it('should not show the new logs ui with a beta rollout percentage of 0', function() {
-      this.settings.logsUIPercentageBeta = 0
-      expect(NewLogsUI.shouldUserSeeNewLogsUI(this.user)).to.be.false
+
+    describe('with a 0% rollout', function() {
+      it('should always show the existing UI', function() {
+        for (const percentile of [0, 20, 40, 60, 80]) {
+          this.user._id = userIdFromTime(percentile)
+          const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+          expect(isExistingUI(variant)).to.be.true
+        }
+      })
     })
     })
-    describe('with a beta rollout percentage > 0', function() {
-      const percentileThresold = 50
+
+    describe('with a new UI rollout', function() {
+      const newUIWithPopupPercentage = 33
+      const newUIWithoutPopupPercentage = 33
+
+      const newUIWithPopupThreshold = newUIWithPopupPercentage
+      const newUIWithoutPopupThreshold =
+        newUIWithPopupPercentage + newUIWithoutPopupPercentage
+
       beforeEach(function() {
       beforeEach(function() {
-        this.settings.logsUIPercentageBeta = percentileThresold
+        this.settings.logsUIPercentageBeta = newUIWithPopupPercentage
+        this.settings.logsUIPercentageWithoutPopupBeta = newUIWithoutPopupPercentage
+      })
+      it('should show the new UI with popup when the id is below the new UI with popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithPopupThreshold - 1)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithPopup(variant)).to.be.true
+      })
+      it('should show the new UI without popup when the id is at the new UI with popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithPopupThreshold)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithoutPopup(variant)).to.be.true
       })
       })
-      it('should not show the new logs ui when the user id is higher than the percent threshold', function() {
-        this.user._id = userIdFromTime(percentileThresold + 1)
-        expect(NewLogsUI.shouldUserSeeNewLogsUI(this.user)).to.be.false
+      it('should show the new UI without popup when the id is above the new UI with popup upper threshold (inc) and below the new UI without popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithoutPopupThreshold - 1)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithoutPopup(variant)).to.be.true
       })
       })
-      it('should show the new logs ui when the user id is lower than the percent threshold', function() {
-        this.user._id = userIdFromTime(percentileThresold - 1)
-        expect(NewLogsUI.shouldUserSeeNewLogsUI(this.user)).to.be.true
+      it('should show the existing UI when the id is at the new UI without popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithoutPopupThreshold)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isExistingUI(variant)).to.be.true
+      })
+      it('should show the existing UI when the id is above the new UI without popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithoutPopupThreshold + 1)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isExistingUI(variant)).to.be.true
       })
       })
     })
     })
   })
   })
 
 
-  describe('for normal users', function() {
-    it('should not show the new logs ui rollout percentage of 0', function() {
-      this.settings.logsUIPercentage = 0
-      expect(NewLogsUI.shouldUserSeeNewLogsUI(this.user)).to.be.false
+  describe('for regular users', function() {
+    describe('with a 0% rollout', function() {
+      it('should always show the existing UI', function() {
+        for (const percentile of [0, 20, 40, 60, 80]) {
+          this.user._id = userIdFromTime(percentile)
+          const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+          expect(isExistingUI(variant)).to.be.true
+        }
+      })
     })
     })
-    describe('with a rollout percentage > 0', function() {
-      const percentileThresold = 50
+
+    describe('with a new UI rollout', function() {
+      const newUIWithPopupPercentage = 33
+      const newUIWithoutPopupPercentage = 33
+
+      const newUIWithPopupThreshold = newUIWithPopupPercentage
+      const newUIWithoutPopupThreshold =
+        newUIWithPopupPercentage + newUIWithoutPopupPercentage
+
       beforeEach(function() {
       beforeEach(function() {
-        this.settings.logsUIPercentage = percentileThresold
+        this.settings.logsUIPercentage = newUIWithPopupPercentage
+        this.settings.logsUIPercentageWithoutPopup = newUIWithoutPopupPercentage
+      })
+      it('should show the new UI with popup when the id is below the new UI with popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithPopupThreshold - 1)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithPopup(variant)).to.be.true
+      })
+      it('should show the new UI without popup when the id is at the new UI with popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithPopupThreshold)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithoutPopup(variant)).to.be.true
+      })
+      it('should show the new UI without popup when the id is above the new UI with popup upper threshold (inc) and below the new UI without popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithoutPopupThreshold - 1)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithoutPopup(variant)).to.be.true
       })
       })
-      it('should not show the new logs ui when the user id is higher than the percent threshold', function() {
-        this.user._id = userIdFromTime(percentileThresold + 1)
-        expect(NewLogsUI.shouldUserSeeNewLogsUI(this.user)).to.be.false
+      it('should show the existing UI when the id is at the new UI without popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithoutPopupThreshold)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isExistingUI(variant)).to.be.true
       })
       })
-      it('should show the new logs ui when the user id is lower than the percent threshold', function() {
-        this.user._id = userIdFromTime(percentileThresold - 1)
-        expect(NewLogsUI.shouldUserSeeNewLogsUI(this.user)).to.be.true
+      it('should show the existing UI when the id is above the new UI without popup upper threshold (exc)', function() {
+        this.user._id = userIdFromTime(newUIWithoutPopupThreshold + 1)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isExistingUI(variant)).to.be.true
       })
       })
     })
     })
   })
   })

+ 3 - 1
services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -121,7 +121,9 @@ describe('ProjectController', function() {
       inc: sinon.stub()
       inc: sinon.stub()
     }
     }
     this.NewLogsUIHelper = {
     this.NewLogsUIHelper = {
-      shouldUserSeeNewLogsUI: sinon.stub().returns(false)
+      getNewLogsUIVariantForUser: sinon
+        .stub()
+        .returns({ newLogsUI: false, subvariant: null })
     }
     }
 
 
     this.ProjectController = SandboxedModule.require(MODULE_PATH, {
     this.ProjectController = SandboxedModule.require(MODULE_PATH, {