diff --git a/services/web/app/src/Features/Helpers/NewLogsUI.js b/services/web/app/src/Features/Helpers/NewLogsUI.js index 8d57f603f4..2d17b2f465 100644 --- a/services/web/app/src/Features/Helpers/NewLogsUI.js +++ b/services/web/app/src/Features/Helpers/NewLogsUI.js @@ -1,29 +1,71 @@ const { ObjectId } = require('mongodb') 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 { _id: userId, alphaProgram: isAlphaUser, betaProgram: isBetaUser } = user if (!userId) { - return false + return EXISTING_UI } const userIdAsPercentile = (ObjectId(userId).getTimestamp() / 1000) % 100 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 { - return false + return _getVariantForPercentile( + userIdAsPercentile, + Settings.logsUIPercentage, + Settings.logsUIPercentageWithoutPopup + ) } } module.exports = { - shouldUserSeeNewLogsUI + getNewLogsUIVariantForUser } diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index fcbbdca59f..4e6b2df830 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -37,7 +37,7 @@ const BrandVariationsHandler = require('../BrandVariations/BrandVariationsHandle const UserController = require('../User/UserController') const AnalyticsManager = require('../Analytics/AnalyticsManager') const Modules = require('../../infrastructure/Modules') -const { shouldUserSeeNewLogsUI } = require('../Helpers/NewLogsUI') +const { getNewLogsUIVariantForUser } = require('../Helpers/NewLogsUI') const _ssoAvailable = (affiliation, session, linkedInstitutionIds) => { 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 = req.query && req.query.new_logs_ui === 'false' @@ -860,6 +861,7 @@ const ProjectController = { wsUrl, showSupport: Features.hasFeature('support'), showNewLogsUI: userShouldSeeNewLogsUI && !wantsOldLogsUI, + logsUISubvariant: logsUIVariant.subvariant, showNewNavigationUI: req.query && req.query.new_navigation_ui === 'true', showReactFileTree: !wantsOldFileTreeUI, diff --git a/services/web/app/views/project/editor/pdf.pug b/services/web/app/views/project/editor/pdf.pug index 3132bf78da..5c9f07706e 100644 --- a/services/web/app/views/project/editor/pdf.pug +++ b/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-full-layout="setPdfFullLayout" on-stop-compilation="stop" + variant-with-first-error-popup="logsUISubvariant === 'new-logs-ui-with-popup'" show-logs="shouldShowLogs" on-log-entry-location-click="openInEditor" ) @@ -415,3 +416,4 @@ script(type='text/ng-template', id='clearCacheModalTemplate') script(type="text/javascript"). window.showNewLogsUI = #{showNewLogsUI || false} + window.logsUISubvariant = !{logsUISubvariant ? '"' + logsUISubvariant + '"' : 'null'} diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index 45614db807..f5c964af32 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -227,7 +227,10 @@ module.exports = settings = # 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, diff --git a/services/web/frontend/js/features/preview/components/preview-pane.js b/services/web/frontend/js/features/preview/components/preview-pane.js index 1d2bac6233..9c0c575b53 100644 --- a/services/web/frontend/js/features/preview/components/preview-pane.js +++ b/services/web/frontend/js/features/preview/components/preview-pane.js @@ -22,6 +22,7 @@ function PreviewPane({ pdfDownloadUrl, onLogEntryLocationClick, showLogs, + variantWithFirstErrorPopup = true, splitLayout }) { const { t } = useTranslation() @@ -67,6 +68,7 @@ function PreviewPane({ !compilerState.isCompiling const showFirstErrorPopUp = + variantWithFirstErrorPopup && nErrors > 0 && !seenLogsForCurrentCompile && !dismissedFirstErrorPopUp && @@ -168,6 +170,7 @@ PreviewPane.propTypes = { outputFiles: PropTypes.array, pdfDownloadUrl: PropTypes.string, showLogs: PropTypes.bool.isRequired, + variantWithFirstErrorPopup: PropTypes.bool, splitLayout: PropTypes.bool.isRequired } diff --git a/services/web/frontend/js/ide/pdf/controllers/PdfController.js b/services/web/frontend/js/ide/pdf/controllers/PdfController.js index 5c9a2bf486..422ced8e46 100644 --- a/services/web/frontend/js/ide/pdf/controllers/PdfController.js +++ b/services/web/frontend/js/ide/pdf/controllers/PdfController.js @@ -32,6 +32,8 @@ App.controller('PdfController', function( $scope.pdf.clearingCache = false $scope.shouldShowLogs = false + $scope.logsUISubvariant = window.logsUISubvariant + // view logic to check whether the files dropdown should "drop up" or "drop down" $scope.shouldDropUp = false @@ -708,7 +710,8 @@ App.controller('PdfController', function( errors: $scope.pdf.logEntries.errors.length, warnings: $scope.pdf.logEntries.warnings.length, typesetting: $scope.pdf.logEntries.typesetting.length, - newLogsUI: window.showNewLogsUI + newLogsUI: window.showNewLogsUI, + subvariant: window.logsUISubvariant } eventTracking.sendMBSampled( 'compile-result', diff --git a/services/web/test/unit/src/HelperFiles/NewLogsUITests.js b/services/web/test/unit/src/HelperFiles/NewLogsUITests.js index bcb5c78718..7c5fea9c69 100644 --- a/services/web/test/unit/src/HelperFiles/NewLogsUITests.js +++ b/services/web/test/unit/src/HelperFiles/NewLogsUITests.js @@ -13,6 +13,20 @@ describe('NewLogsUI helper', function() { 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() { this.user = { alphaProgram: false, @@ -21,7 +35,9 @@ describe('NewLogsUI helper', function() { } this.settings = { logsUIPercentageBeta: 0, - logsUIPercentage: 0 + logsUIPercentageWithoutPopupBeta: 0, + logsUIPercentage: 0, + logsUIPercentageWithoutPopup: 0 } NewLogsUI = SandboxedModule.require(MODULE_PATH, { 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 - 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() { beforeEach(function() { 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() { - this.settings.logsUIPercentageBeta = percentileThresold + this.settings.logsUIPercentageBeta = newUIWithPopupPercentage + this.settings.logsUIPercentageWithoutPopupBeta = newUIWithoutPopupPercentage }) - 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 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 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 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) + 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() { - this.settings.logsUIPercentage = percentileThresold + this.settings.logsUIPercentage = newUIWithPopupPercentage + this.settings.logsUIPercentageWithoutPopup = newUIWithoutPopupPercentage }) - 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 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 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 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) + 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 }) }) }) diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index cda5e7015c..2fde847650 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -121,7 +121,9 @@ describe('ProjectController', function() { inc: sinon.stub() } this.NewLogsUIHelper = { - shouldUserSeeNewLogsUI: sinon.stub().returns(false) + getNewLogsUIVariantForUser: sinon + .stub() + .returns({ newLogsUI: false, subvariant: null }) } this.ProjectController = SandboxedModule.require(MODULE_PATH, {