From d6f9f70d06f26a432ca9dd71c154547d6cf3aa2f Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 17 Jun 2021 18:26:52 +0100 Subject: [PATCH] Extend the new compile UI rollout, respecting existing allocations GitOrigin-RevId: 61bfba3a05780ec0b3a3461bba50cca6474a1274 --- .../web/app/src/Features/Helpers/NewLogsUI.js | 59 +++--- services/web/config/settings.defaults.js | 16 -- .../unit/src/HelperFiles/NewLogsUITests.js | 177 ++++++++---------- 3 files changed, 100 insertions(+), 152 deletions(-) diff --git a/services/web/app/src/Features/Helpers/NewLogsUI.js b/services/web/app/src/Features/Helpers/NewLogsUI.js index 64977d0f43..e8359bba9e 100644 --- a/services/web/app/src/Features/Helpers/NewLogsUI.js +++ b/services/web/app/src/Features/Helpers/NewLogsUI.js @@ -11,26 +11,27 @@ const NEW_UI_WITHOUT_POPUP = { subvariant: 'new-logs-ui-without-popup', } -function _getVariantForPercentile( - percentile, - newLogsUIWithPopupPercentage, - newLogsUIWithoutPopupPercentage -) { - // The thresholds below are upper thresholds - const newLogsUIThreshold = newLogsUIWithPopupPercentage - const newLogsUIWithoutPopupThreshold = - newLogsUIWithPopupPercentage + newLogsUIWithoutPopupPercentage +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%. - // 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) { + 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 >= newLogsUIThreshold && - percentile < newLogsUIWithoutPopupThreshold - ) { + } 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 @@ -38,12 +39,10 @@ function _getVariantForPercentile( } function getNewLogsUIVariantForUser(user) { - const { - _id: userId, - alphaProgram: isAlphaUser, - betaProgram: isBetaUser, - } = user - if (!userId) { + const { _id: userId, alphaProgram: isAlphaUser } = user + const isSaaS = Boolean(Settings.overleaf) + + if (!userId || !isSaaS) { return EXISTING_UI } @@ -51,18 +50,8 @@ function getNewLogsUIVariantForUser(user) { if (isAlphaUser) { return NEW_UI_WITH_POPUP - } else if (isBetaUser) { - return _getVariantForPercentile( - userIdAsPercentile, - Settings.logsUIPercentageBeta, - Settings.logsUIPercentageWithoutPopupBeta - ) } else { - return _getVariantForPercentile( - userIdAsPercentile, - Settings.logsUIPercentage, - Settings.logsUIPercentageWithoutPopup - ) + return _getVariantForPercentile(userIdAsPercentile) } } diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index 23b7ad823e..b81f545c67 100644 --- a/services/web/config/settings.defaults.js +++ b/services/web/config/settings.defaults.js @@ -393,22 +393,6 @@ module.exports = { ), wsRetryHandshake: parseInt(process.env.WEBSOCKET_RETRY_HANDSHAKE || '5', 10), - // Compile UI rollout percentages - 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), - logsUIPercentageWithoutPopup: parseInt( - process.env.LOGS_UI_WITHOUT_POPUP_PERCENTAGE || '0', - 10 - ), - // cookie domain // use full domain for cookies to only be accessible from that domain, // replace subdomain with dot to have them accessible on all subdomains diff --git a/services/web/test/unit/src/HelperFiles/NewLogsUITests.js b/services/web/test/unit/src/HelperFiles/NewLogsUITests.js index 4ed03c0e98..9fa942763d 100644 --- a/services/web/test/unit/src/HelperFiles/NewLogsUITests.js +++ b/services/web/test/unit/src/HelperFiles/NewLogsUITests.js @@ -27,18 +27,24 @@ describe('NewLogsUI helper', function () { ) } + function getTestInterval(lowerBoundary, upperBoundary) { + const midpoint = Math.floor( + lowerBoundary + (upperBoundary - lowerBoundary) / 2 + ) + return [lowerBoundary, midpoint, upperBoundary] + } + beforeEach(function () { this.user = { alphaProgram: false, - betaProgram: false, _id: ObjectId('60085414b76eeb00737d93aa'), } this.settings = { - logsUIPercentageBeta: 0, - logsUIPercentageWithoutPopupBeta: 0, - logsUIPercentage: 0, - logsUIPercentageWithoutPopup: 0, + overleaf: { + foo: 'bar', + }, } + NewLogsUI = SandboxedModule.require(MODULE_PATH, { requires: { mongodb: { ObjectId }, @@ -47,118 +53,87 @@ describe('NewLogsUI helper', function () { }) }) - it('should always show the new UI with popup for alpha users', function () { - this.user.alphaProgram = 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('In a non-SaaS context', function () { beforeEach(function () { - this.user.betaProgram = true + delete this.settings.overleaf }) - - 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 new UI rollout', function () { - const newUIWithPopupPercentage = 33 - const newUIWithoutPopupPercentage = 33 - - const newUIWithPopupThreshold = newUIWithPopupPercentage - const newUIWithoutPopupThreshold = - newUIWithPopupPercentage + newUIWithoutPopupPercentage - - beforeEach(function () { - 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 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 existing UI when the id is at the new UI without popup upper threshold (exc)', function () { - this.user._id = userIdFromTime(newUIWithoutPopupThreshold) + 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 - }) - 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 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('For alpha users', function () { + beforeEach(function () { + this.user.alphaProgram = true }) - - describe('with a new UI rollout', function () { - const newUIWithPopupPercentage = 33 - const newUIWithoutPopupPercentage = 33 - - const newUIWithPopupThreshold = newUIWithPopupPercentage - const newUIWithoutPopupThreshold = - newUIWithPopupPercentage + newUIWithoutPopupPercentage - - beforeEach(function () { - 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) + 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 - }) - 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) + } + }) + }) + + 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 - }) - 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) + } + 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 - }) - 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) + } + 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 - }) - 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 - }) + } + this.user._id = userIdFromTime(100) + const variant = NewLogsUI.getNewLogsUIVariantForUser(this.user) + expect(isExistingUI(variant)).to.be.false }) }) })