Merge pull request #6727 from overleaf/em-revert-logs-ui-split-test

Revert "Move new logs UI toggle to a split test"

GitOrigin-RevId: 5f2aecb53d2df21c89da594b0a64c8cf8ca04bc6

services/web/app/src/Features/Helpers/NewLogsUI.js

@@ -0,0 +1,63 @@
+const { ObjectId } = require('mongodb')
+const Settings = require('@overleaf/settings')
+
+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) {
+  // The current percentages are:
+  // - 33% New UI with pop-up (originally, 5%)
+  // - 33% New UI without pop-up (originally, 5%)
+  // - 34% Existing UI
+  // To ensure group stability, the implementation below respects the original partitions
+  // for the new UI variants: [0, 5[ and [5,10[.
+  // Two new partitions are added: [10, 38[ and [38, 66[. These represent an extra 28p.p.
+  // which, with to the original 5%, add up to 33%.
+
+  if (percentile < 5) {
+    // This partition represents the "New UI with pop-up" group in the original roll-out (5%)
+    return NEW_UI_WITH_POPUP
+  } else if (percentile >= 5 && percentile < 10) {
+    // This partition represents the "New UI without pop-up" group in the original roll-out (5%)
+    return NEW_UI_WITHOUT_POPUP
+  } else if (percentile >= 10 && percentile < 38) {
+    // This partition represents an extra 28% of users getting the "New UI with pop-up"
+    return NEW_UI_WITH_POPUP
+  } else if (percentile >= 38 && percentile < 66) {
+    // This partition represents an extra 28% of users getting the "New UI without pop-up"
+    return NEW_UI_WITHOUT_POPUP
+  } else {
+    return EXISTING_UI
+  }
+}
+
+// eslint-disable-next-line no-unused-vars
+function getNewLogsUIVariantForUser(user) {
+  const { _id: userId, alphaProgram: isAlphaUser } = user
+  const isSaaS = Boolean(Settings.overleaf)
+
+  if (!userId || !isSaaS) {
+    return EXISTING_UI
+  }
+
+  const userIdAsPercentile = (ObjectId(userId).getTimestamp() / 1000) % 100
+
+  if (isAlphaUser) {
+    return NEW_UI_WITH_POPUP
+  } else {
+    return _getVariantForPercentile(userIdAsPercentile)
+  }
+}
+
+module.exports = {
+  // We're disabling the split tests while rolling out the PDF Preview
+  // https://github.com/overleaf/internal/issues/5553
+  getNewLogsUIVariantForUser: () => EXISTING_UI,
+}

services/web/app/src/Features/Project/ProjectController.js

@@ -36,6 +36,7 @@ const UserController = require('../User/UserController')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const Modules = require('../../infrastructure/Modules')
 const SplitTestHandler = require('../SplitTests/SplitTestHandler')
+const { getNewLogsUIVariantForUser } = require('../Helpers/NewLogsUI')
 const FeaturesUpdater = require('../Subscription/FeaturesUpdater')
 const SpellingHandler = require('../Spelling/SpellingHandler')
 const UserPrimaryEmailCheckHandler = require('../User/UserPrimaryEmailCheckHandler')
@@ -828,21 +829,6 @@ const ProjectController = {
             }
           )
         },
-        logsUIAssignment(cb) {
-          SplitTestHandler.getAssignment(
-            req,
-            'logs-ui',
-            {},
-            (error, assignment) => {
-              // do not fail editor load if assignment fails
-              if (error) {
-                cb(null, { variant: 'default' })
-              } else {
-                cb(null, assignment)
-              }
-            }
-          )
-        },
       },
       (
         err,
@@ -856,7 +842,6 @@ const ProjectController = {
           newPdfPreviewAssignment,
           newSourceEditorAssignment,
           pdfDetachAssignment,
-          logsUIAssignment,
         }
       ) => {
         if (err != null) {
@@ -921,6 +906,8 @@ const ProjectController = {
               })
             }
 
+            const logsUIVariant = getNewLogsUIVariantForUser(user)
+
             function shouldDisplayFeature(name, variantFlag) {
               if (req.query && req.query[name]) {
                 return req.query[name] === 'true'
@@ -1017,7 +1004,11 @@ const ProjectController = {
               gitBridgePublicBaseUrl: Settings.gitBridgePublicBaseUrl,
               wsUrl,
               showSupport: Features.hasFeature('support'),
-              logsUIVariant: logsUIAssignment.variant,
+              showNewLogsUI: shouldDisplayFeature(
+                'new_logs_ui',
+                logsUIVariant.newLogsUI
+              ),
+              logsUISubvariant: logsUIVariant.subvariant,
               showPdfDetach,
               debugPdfDetach,
               showNewPdfPreview,

services/web/app/views/project/editor.pug

@@ -141,7 +141,8 @@ block append meta
 	//- used in public/js/libs/sharejs.js
 	meta(name="ol-useShareJsHash" data-type="boolean" content=true)
 	meta(name="ol-wsRetryHandshake" data-type="json" content=settings.wsRetryHandshake)
-	meta(name="ol-logsUIVariant" data-type="boolean" content=logsUIVariant)
+	meta(name="ol-showNewLogsUI" data-type="boolean" content=showNewLogsUI)
+	meta(name="ol-logsUISubvariant" content=logsUISubvariant)
 	meta(name="ol-showPdfDetach" data-type="boolean" content=showPdfDetach)
 	meta(name="ol-debugPdfDetach" data-type="boolean" content=debugPdfDetach)
 	meta(name="ol-showNewPdfPreview" data-type="boolean" content=showNewPdfPreview)

services/web/app/views/project/editor/pdf.pug

@@ -1,5 +1,5 @@
 div.full-size.pdf(ng-controller="PdfController")
-	if logsUIVariant !== 'default'
+	if showNewLogsUI
 		preview-pane(
 			compiler-state=`{
 				autoCompileHasChanges: changesToAutoCompile,
@@ -43,7 +43,7 @@ div.full-size.pdf(ng-controller="PdfController")
 			on-set-split-layout="setPdfSplitLayout"
 			on-set-full-layout="setPdfFullLayout"
 			on-stop-compilation="stop"
-			variant-with-first-error-popup="logsUIVariant === 'new-logs-ui-with-popup'"
+			variant-with-first-error-popup="logsUISubvariant === 'new-logs-ui-with-popup'"
 			show-logs="shouldShowLogs"
 			on-log-entry-location-click="openInEditor"
 		)
@@ -303,7 +303,7 @@ div.full-size.pdf(ng-controller="PdfController")
 				ng-if="settings.pdfViewer == 'native'"
 			)
 
-		if logsUIVariant === 'default'
+		if !showNewLogsUI
 			.pdf-validation-problems(ng-switch-when="validation-problems")
 
 				.alert.alert-danger(ng-show="pdf.validation.sizeCheck")

services/web/frontend/js/ide/pdf/controllers/PdfController.js

@@ -36,7 +36,7 @@ App.controller(
     $scope.pdf.clearingCache = false
     $scope.shouldShowLogs = false
 
-    $scope.logsUIVariant = window.logsUIVariant
+    $scope.logsUISubvariant = window.logsUISubvariant
 
     // view logic to check whether the files dropdown should "drop up" or "drop down"
     $scope.shouldDropUp = false
@@ -538,7 +538,7 @@ App.controller(
     // In the existing compile UI, errors and validation problems are shown in the PDF pane, whereas in the new
     // one they're shown in the logs pane. This `$watch`er makes sure we change the view in the new logs UI.
     // This should be removed once we stop supporting the two different log UIs.
-    if (window.logsUIVariant !== 'default') {
+    if (window.showNewLogsUI) {
       $scope.$watch(
         () =>
           $scope.pdf.view === 'errors' ||
@@ -732,7 +732,8 @@ App.controller(
           errors: $scope.pdf.logEntries.errors.length,
           warnings: $scope.pdf.logEntries.warnings.length,
           typesetting: $scope.pdf.logEntries.typesetting.length,
-          logsUIVariant: window.logsUIVariant,
+          newLogsUI: window.showNewLogsUI,
+          subvariant: window.showNewLogsUI ? window.logsUISubvariant : null,
         }
         eventTracking.sendMBSampled('compile-result', metadata, 0.01)
       }

services/web/test/unit/src/HelperFiles/NewLogsUITests.js

@@ -0,0 +1,144 @@
+const SandboxedModule = require('sandboxed-module')
+const { expect } = require('chai')
+const { ObjectId } = require('mongodb')
+const MODULE_PATH = require('path').join(
+  __dirname,
+  '../../../../app/src/Features/Helpers/NewLogsUI.js'
+)
+
+describe('NewLogsUI helper', function () {
+  let NewLogsUI
+
+  before(function () {
+    // We're disabling the Logs UI split test while rolling out the PDF Preview
+    this.skip()
+  })
+
+  function userIdFromTime(time) {
+    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'
+    )
+  }
+
+  function getTestInterval(lowerBoundary, upperBoundary) {
+    const midpoint = Math.floor(
+      lowerBoundary + (upperBoundary - lowerBoundary) / 2
+    )
+    return [lowerBoundary, midpoint, upperBoundary]
+  }
+
+  beforeEach(function () {
+    this.user = {
+      alphaProgram: false,
+      _id: ObjectId('60085414b76eeb00737d93aa'),
+    }
+    this.settings = {
+      overleaf: {
+        foo: 'bar',
+      },
+    }
+
+    NewLogsUI = SandboxedModule.require(MODULE_PATH, {
+      requires: {
+        mongodb: { ObjectId },
+        '@overleaf/settings': this.settings,
+      },
+    })
+  })
+
+  describe('In a non-SaaS context', function () {
+    beforeEach(function () {
+      delete this.settings.overleaf
+    })
+    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('For alpha users', function () {
+    beforeEach(function () {
+      this.user.alphaProgram = true
+    })
+    it('should always show the new UI with popup', function () {
+      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 regular users', function () {
+    it('should show the new UI with popup when the id is in the [0, 5[ interval', function () {
+      const testInterval = getTestInterval(0, 4)
+      for (const percentile of testInterval) {
+        this.user._id = userIdFromTime(percentile)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithPopup(variant)).to.be.true
+      }
+      this.user._id = userIdFromTime(5)
+      const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+      expect(isNewUIWithPopup(variant)).to.be.false
+    })
+    it('should show the new UI without popup when the id is in the [5, 10[ interval', function () {
+      const testInterval = getTestInterval(5, 9)
+      for (const percentile of testInterval) {
+        this.user._id = userIdFromTime(percentile)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithoutPopup(variant)).to.be.true
+      }
+      this.user._id = userIdFromTime(10)
+      const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+      expect(isNewUIWithoutPopup(variant)).to.be.false
+    })
+    it('should show the new UI with popup when the id is in the [10, 38[ interval', function () {
+      const testInterval = getTestInterval(10, 37)
+      for (const percentile of testInterval) {
+        this.user._id = userIdFromTime(percentile)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithPopup(variant)).to.be.true
+      }
+      this.user._id = userIdFromTime(38)
+      const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+      expect(isNewUIWithPopup(variant)).to.be.false
+    })
+    it('should show the new UI without popup when the id is in the [38, 66[ interval', function () {
+      const testInterval = getTestInterval(38, 65)
+      for (const percentile of testInterval) {
+        this.user._id = userIdFromTime(percentile)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isNewUIWithoutPopup(variant)).to.be.true
+      }
+      this.user._id = userIdFromTime(66)
+      const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+      expect(isNewUIWithoutPopup(variant)).to.be.false
+    })
+    it('should show the existing UI when the id is in the [66, 99] interval', function () {
+      const testInterval = getTestInterval(66, 99)
+      for (const percentile of testInterval) {
+        this.user._id = userIdFromTime(percentile)
+        const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+        expect(isExistingUI(variant)).to.be.true
+      }
+      this.user._id = userIdFromTime(100)
+      const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user)
+      expect(isExistingUI(variant)).to.be.false
+    })
+  })
+})

services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -123,6 +123,11 @@ describe('ProjectController', function () {
       },
       inc: sinon.stub(),
     }
+    this.NewLogsUIHelper = {
+      getNewLogsUIVariantForUser: sinon
+        .stub()
+        .returns({ newLogsUI: false, subvariant: null }),
+    }
     this.SplitTestHandler = {
       promises: {
         getAssignment: sinon.stub().resolves({ variant: 'default' }),
@@ -170,6 +175,7 @@ describe('ProjectController', function () {
         '../../infrastructure/Modules': {
           hooks: { fire: sinon.stub().yields(null, []) },
         },
+        '../Helpers/NewLogsUI': this.NewLogsUIHelper,
         '../Spelling/SpellingHandler': {
           getUserDictionary: sinon.stub().yields(null, []),
         },