diff --git a/services/web/app/src/Features/Project/ProjectController.mjs b/services/web/app/src/Features/Project/ProjectController.mjs index 2fe0c388c2..620ef30024 100644 --- a/services/web/app/src/Features/Project/ProjectController.mjs +++ b/services/web/app/src/Features/Project/ProjectController.mjs @@ -617,7 +617,7 @@ const _ProjectController = { req, projectId ) - const imageNames = ProjectHelper.getAllowedImagesForUser(user) + const imageNames = await ProjectHelper.getAllowedImagesForUser(user) const privilegeLevel = await AuthorizationManager.promises.getPrivilegeLevelForProject( diff --git a/services/web/app/src/Features/Project/ProjectHelper.mjs b/services/web/app/src/Features/Project/ProjectHelper.mjs index 0517639596..25951d39bb 100644 --- a/services/web/app/src/Features/Project/ProjectHelper.mjs +++ b/services/web/app/src/Features/Project/ProjectHelper.mjs @@ -3,6 +3,7 @@ import mongodb from 'mongodb-legacy' import _ from 'lodash' import Settings from '@overleaf/settings' import OError from '@overleaf/o-error' +import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs' const { ObjectId } = mongodb @@ -148,25 +149,45 @@ function _addNumericSuffixToProjectName(name, allProjectNames, maxLength) { return null } -function _imageAllowed(user, image) { - if (image.alphaOnly) { - return Boolean(user?.alphaProgram) +async function _monthlyExperimentalImageAllowed(user) { + const userId = user?._id?.toString() + if (!userId) return false + const { variant } = await SplitTestHandler.promises.getAssignmentForUser( + userId, + 'monthly-texlive' + ) + return variant === 'enabled' +} + +function _imageAllowed( + image, + alphaImagesAllowed, + monthlyExperimentalImagesAllowed +) { + if (image.alphaOnly && !alphaImagesAllowed) { + return false } - if (image.monthlyExperimental) { - return Boolean( - user?.labsProgram && user.labsExperiments.includes('monthly-texlive') - ) + if (image.monthlyExperimental && !monthlyExperimentalImagesAllowed) { + return false } return true } -function getAllowedImagesForUser(user) { +async function getAllowedImagesForUser(user) { let images = Settings.allowedImageNames || [] + const alphaImagesAllowed = Boolean(user?.alphaProgram) + const monthlyExperimentalImagesAllowed = + await _monthlyExperimentalImageAllowed(user) + images = images.map(image => { return { ...image, - allowed: _imageAllowed(user, image), + allowed: _imageAllowed( + image, + alphaImagesAllowed, + monthlyExperimentalImagesAllowed + ), rolling: image.monthlyExperimental, } }) diff --git a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs index c333511d03..a1ebb7c5dd 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs @@ -15,7 +15,6 @@ import SessionManager from '../Authentication/SessionManager.mjs' import logger from '@overleaf/logger' import SplitTestSessionHandler from './SplitTestSessionHandler.mjs' import SplitTestUserGetter from './SplitTestUserGetter.mjs' -import SplitTestManager from './SplitTestManager.mjs' /** * @import { Assignment } from "./types" @@ -23,6 +22,7 @@ import SplitTestManager from './SplitTestManager.mjs' const DEFAULT_VARIANT = 'default' const ALPHA_PHASE = 'alpha' +const LABS_PHASE = 'labs' const BETA_PHASE = 'beta' const RELEASE_PHASE = 'release' const DEFAULT_ASSIGNMENT = { @@ -417,8 +417,10 @@ async function _getAssignment( } if (activeForUser) { - if (_isSplitTest(splitTest)) { - // if the user is logged in, persist the assignment + const hasUserLimit = _currentVersionHasUserLimit(splitTest) + + if (_isSplitTest(splitTest) || hasUserLimit) { + // if the user is logged in, persist the assignment (and increment user count if needed) if (userId) { const assignmentData = { user, @@ -447,7 +449,7 @@ async function _getAssignment( } } // otherwise this is an anonymous user, we store assignments in session to persist them on registration - else { + else if (_isSplitTest(splitTest)) { await SplitTestSessionHandler.promises.appendAssignment(session, { splitTestId: splitTest._id, splitTestName, @@ -458,23 +460,25 @@ async function _getAssignment( }) } - const effectiveAnalyticsId = user?.analyticsId || analyticsId || userId - AnalyticsManager.setUserPropertyForAnalyticsId( - effectiveAnalyticsId, - `split-test-${splitTestName}-${versionNumber}`, - selectedVariantName - ).catch(err => { - logger.warn( - { - err, - analyticsId: effectiveAnalyticsId, - splitTest: splitTestName, - versionNumber, - variant: selectedVariantName, - }, - 'failed to set user property for analytics id' - ) - }) + if (_isSplitTest(splitTest)) { + const effectiveAnalyticsId = user?.analyticsId || analyticsId || userId + AnalyticsManager.setUserPropertyForAnalyticsId( + effectiveAnalyticsId, + `split-test-${splitTestName}-${versionNumber}`, + selectedVariantName + ).catch(err => { + logger.warn( + { + err, + analyticsId: effectiveAnalyticsId, + splitTest: splitTestName, + versionNumber, + variant: selectedVariantName, + }, + 'failed to set user property for analytics id' + ) + }) + } } let isFirstNonDefaultAssignment if (userId) { @@ -501,11 +505,15 @@ async function _getAssignmentMetadata(analyticsId, user, splitTest) { const phase = currentVersion.phase // For continuity on phase rollout for gradual rollouts, we keep all users from the previous phase enrolled to the variant. - // In beta, all alpha users are cohorted to the variant, and the same in release phase all alpha & beta users. + // In beta, all alpha and labs users are cohorted to the variant, and the same in release phase all alpha, labs & beta users. if ( _isGradualRollout(splitTest) && ((phase === BETA_PHASE && user?.alphaProgram) || - (phase === RELEASE_PHASE && (user?.alphaProgram || user?.betaProgram))) + (phase === RELEASE_PHASE && + (user?.alphaProgram || + user?.betaProgram || + (user?.labsProgram && + user?.labsExperiments?.includes(splitTest.name))))) ) { return { activeForUser: true, @@ -516,6 +524,27 @@ async function _getAssignmentMetadata(analyticsId, user, splitTest) { } } + // Labs phase: user must be in labs program AND have opted into this experiment. + // The userCount/userLimit check is enforced at enrollment time (see + // incrementLabsVariantCounterIfBelowLimit), so we trust the enrollment here. + if (phase === LABS_PHASE) { + if (user?.labsProgram && user?.labsExperiments?.includes(splitTest.name)) { + const selectedVariant = currentVersion.variants[0] + const selectedVariantName = selectedVariant.name + + return { + activeForUser: true, + selectedVariantName, + phase, + versionNumber, + isFirstNonDefaultAssignment: false, + } + } + return { + activeForUser: false, + } + } + if ( (phase === ALPHA_PHASE && !user?.alphaProgram) || (phase === BETA_PHASE && !user?.betaProgram) @@ -549,8 +578,7 @@ async function _getAssignmentMetadata(analyticsId, user, splitTest) { : null if (!existingAssignment) { - const currentCount = - selectedVariant.userCount ?? Number.MAX_SAFE_INTEGER + const currentCount = selectedVariant.userCount ?? 0 if (currentCount >= userLimit) { selectedVariantName = DEFAULT_VARIANT @@ -674,6 +702,12 @@ async function _shouldIncrementVariantCounter( return false } + // Labs variant counters are managed at enrollment/unenrollment time, + // not at assignment time. + if (phase === LABS_PHASE) { + return false + } + const splitTest = await _getSplitTest(splitTestName) if (!splitTest) { return false @@ -704,42 +738,6 @@ async function _shouldIncrementVariantCounter( return !existingPhaseAssignment } -/** - * Increment the user counter for a specific variant - * @param {string} splitTestName - The name of the split test - * @param {string} variantName - The name of the variant - * @param {number} versionNumber - The version to update - */ -async function _incrementVariantCounter( - splitTestName, - variantName, - versionNumber -) { - try { - await SplitTest.updateOne( - { - name: splitTestName, - 'versions.versionNumber': versionNumber, - 'versions.variants.name': variantName, - }, - { - $inc: { - 'versions.$.variants.$[variant].userCount': 1, - }, - }, - { - arrayFilters: [{ 'variant.name': variantName }], - } - ).exec() - await SplitTestManager.clearCache() - } catch (error) { - logger.error( - { err: error, splitTestName, variantName, versionNumber }, - 'Failed to increment variant counter' - ) - } -} - function _makeAssignment({ variant, currentVersion, @@ -812,6 +810,138 @@ function _isGradualRollout(featureFlag) { return !SplitTestUtils.getCurrentVersion(featureFlag).analyticsEnabled } +function _currentVersionHasUserLimit(featureFlag) { + const currentVersion = SplitTestUtils.getCurrentVersion(featureFlag) + return currentVersion.variants.some( + v => v.userLimit && typeof v.userLimit === 'number' + ) +} + +/** + * Increment the user counter for a specific variant + * @param {string} splitTestName - The name of the split test + * @param {string} variantName - The name of the variant + * @param {number} versionNumber - The version to update + */ +async function _incrementVariantCounter( + splitTestName, + variantName, + versionNumber +) { + try { + await SplitTest.updateOne( + { + name: splitTestName, + 'versions.versionNumber': versionNumber, + 'versions.variants.name': variantName, + }, + { + $inc: { + 'versions.$.variants.$[variant].userCount': 1, + }, + }, + { + arrayFilters: [{ 'variant.name': variantName }], + } + ).exec() + } catch (error) { + logger.error( + { err: error, splitTestName, variantName, versionNumber }, + 'Failed to increment variant counter' + ) + } +} + +/** + * Atomically increment the labs variant counter only if below the user limit. + * Returns true if a slot was claimed, false if the limit has been reached. + * When there is no userLimit, enrollment is always allowed (returns true). + * @param {string} splitTestName + * @returns {Promise} + */ +async function incrementLabsVariantCounterIfBelowLimit(splitTestName) { + const splitTest = await _getSplitTest(splitTestName) + if (!splitTest) return false + + const currentVersion = SplitTestUtils.getCurrentVersion(splitTest) + if (!currentVersion || currentVersion.phase !== LABS_PHASE) return false + + const variant = currentVersion.variants[0] + if (!variant) return false + + if (!variant.userLimit || typeof variant.userLimit !== 'number') { + return true + } + + const result = await SplitTest.updateOne( + { + name: splitTestName, + 'versions.versionNumber': currentVersion.versionNumber, + }, + { + $inc: { + 'versions.$.variants.$[variant].userCount': 1, + }, + }, + { + arrayFilters: [ + { + 'variant.name': variant.name, + 'variant.userCount': { $not: { $gte: variant.userLimit } }, + }, + ], + } + ).exec() + + return result.modifiedCount > 0 +} + +/** + * Decrement the user counter for a labs experiment when a user opts out. + * This frees up a slot so another user can enroll. + * @param {string} splitTestName + */ +async function decrementLabsVariantCounter(splitTestName) { + const splitTest = await _getSplitTest(splitTestName) + if (!splitTest) return + + const currentVersion = SplitTestUtils.getCurrentVersion(splitTest) + if (!currentVersion || currentVersion.phase !== LABS_PHASE) return + + const variant = currentVersion.variants[0] + if (!variant?.userLimit || typeof variant.userLimit !== 'number') return + if (!variant.userCount || variant.userCount <= 0) return + + try { + const result = await SplitTest.updateOne( + { + name: splitTestName, + 'versions.versionNumber': currentVersion.versionNumber, + 'versions.variants.name': variant.name, + }, + { + $inc: { + 'versions.$.variants.$[variant].userCount': -1, + }, + }, + { + arrayFilters: [{ 'variant.name': variant.name }], + } + ).exec() + if (result.modifiedCount === 0) { + logger.warn( + { splitTestName }, + 'Labs variant counter decrement matched no documents' + ) + } + } catch (error) { + logger.error( + { err: error, splitTestName }, + 'Failed to decrement labs variant counter' + ) + } +} + export default { getPercentile, getAssignment: callbackify(getAssignment), @@ -829,5 +959,7 @@ export default { getOneTimeAssignment, getActiveAssignmentsForUser, hasUserBeenAssignedToVariant, + decrementLabsVariantCounter, + incrementLabsVariantCounterIfBelowLimit, }, } diff --git a/services/web/app/src/Features/SplitTests/SplitTestManager.mjs b/services/web/app/src/Features/SplitTests/SplitTestManager.mjs index 27b05dc6e1..1d67149f82 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestManager.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestManager.mjs @@ -1,10 +1,12 @@ import { SplitTest } from '../../models/SplitTest.mjs' +import Errors from '../Errors/Errors.js' import SplitTestUtils from './SplitTestUtils.mjs' import OError from '@overleaf/o-error' import _ from 'lodash' import { CacheFlow } from 'cache-flow' const ALPHA_PHASE = 'alpha' +const LABS_PHASE = 'labs' const BETA_PHASE = 'beta' const RELEASE_PHASE = 'release' @@ -34,7 +36,7 @@ async function getSplitTests({ name, phase, type, active, archived }) { filters.$where = query } } - if (['alpha', 'beta', 'release'].includes(phase)) { + if (['alpha', 'labs', 'beta', 'release'].includes(phase)) { const query = `this.versions[this.versions.length - 1].phase === "${phase}"` if (filters.$where) { filters.$where += `&& ${query}` @@ -78,9 +80,18 @@ async function getSplitTest(query) { } async function createSplitTest( - { name, configuration, badgeInfo = {}, info = {} }, + { name, configuration, badgeInfo = {}, info = {}, labsInfo = {} }, userId ) { + // Labs phase requires exactly one variant + if (configuration.phase === LABS_PHASE) { + if (configuration.variants.length !== 1) { + throw new Errors.InvalidError('Labs phase requires exactly one variant') + } + // Auto-set 100% rollout for labs + configuration.variants[0].rolloutPercent = 100 + } + const stripedVariants = [] let stripeStart = 0 @@ -117,6 +128,9 @@ async function createSplitTest( reportsUrls: info.reportsUrls, winningVariant: info.winningVariant, badgeInfo, + labsTitle: labsInfo.title, + labsDescription: labsInfo.description, + labsIcon: labsInfo.icon, versions: [ { versionNumber: 1, @@ -138,14 +152,18 @@ async function createSplitTest( async function updateSplitTestConfig({ name, configuration, comment }, userId) { const splitTest = await getSplitTest({ name }) if (!splitTest) { - throw new OError(`Cannot update split test '${name}': not found`) + throw new Errors.NotFoundError( + `Cannot update split test '${name}': not found` + ) } if (splitTest.archived) { - throw new OError('Cannot update an archived split test', { name }) + throw new Errors.InvalidError('Cannot update an archived split test', { + name, + }) } const lastVersion = SplitTestUtils.getCurrentVersion(splitTest).toObject() if (configuration.phase !== lastVersion.phase) { - throw new OError( + throw new Errors.InvalidError( `Cannot update with different phase - use /switch-to-next-phase endpoint instead` ) } @@ -171,23 +189,32 @@ async function updateSplitTestConfig({ name, configuration, comment }, userId) { return _saveSplitTest(splitTest) } -async function updateSplitTestInfo(name, info) { +async function updateSplitTestInfo(name, info, labsInfo) { const splitTest = await getSplitTest({ name }) if (!splitTest) { - throw new OError(`Cannot update split test '${name}': not found`) + throw new Errors.NotFoundError( + `Cannot update split test '${name}': not found` + ) } splitTest.description = info.description splitTest.expectedEndDate = info.expectedEndDate splitTest.ticketUrl = info.ticketUrl splitTest.reportsUrls = info.reportsUrls splitTest.winningVariant = info.winningVariant + if (labsInfo) { + splitTest.labsTitle = labsInfo.title + splitTest.labsDescription = labsInfo.description + splitTest.labsIcon = labsInfo.icon + } return _saveSplitTest(splitTest) } async function updateSplitTestBadgeInfo(name, badgeInfo) { const splitTest = await getSplitTest({ name }) if (!splitTest) { - throw new OError(`Cannot update split test '${name}': not found`) + throw new Errors.NotFoundError( + `Cannot update split test '${name}': not found` + ) } splitTest.badgeInfo = badgeInfo return _saveSplitTest(splitTest) @@ -232,41 +259,105 @@ async function mergeSplitTests(incomingTests, overWriteLocal) { } } -async function switchToNextPhase({ name, comment }, userId) { +async function switchToNextPhase( + { name, comment, targetPhase, labsUserLimit, clearUserLimit }, + userId +) { const splitTest = await getSplitTest({ name }) if (!splitTest) { - throw new OError( + throw new Errors.NotFoundError( `Cannot switch split test with ID '${name}' to next phase: not found` ) } if (splitTest.archived) { - throw new OError('Cannot switch an archived split test to next phase', { - name, - }) + throw new Errors.InvalidError( + 'Cannot switch an archived split test to next phase', + { + name, + } + ) } const lastVersionCopy = SplitTestUtils.getCurrentVersion(splitTest).toObject() lastVersionCopy.versionNumber++ - if (lastVersionCopy.phase === ALPHA_PHASE) { - lastVersionCopy.phase = BETA_PHASE - } else if (lastVersionCopy.phase === BETA_PHASE) { - if (splitTest.forbidReleasePhase) { - throw new OError('Switch to release phase is disabled for this test', { - name, - }) + + const currentPhase = lastVersionCopy.phase + + // Determine and validate target phase + if (targetPhase) { + const validTransitions = { + [ALPHA_PHASE]: [LABS_PHASE, BETA_PHASE], + [LABS_PHASE]: [BETA_PHASE], + [BETA_PHASE]: [RELEASE_PHASE], } - lastVersionCopy.phase = RELEASE_PHASE - } else if (splitTest.phase === RELEASE_PHASE) { - throw new OError( - `Split test with ID '${name}' is already in the release phase` + const allowed = validTransitions[currentPhase] + if (!allowed || !allowed.includes(targetPhase)) { + throw new Errors.InvalidError( + `Cannot switch from '${currentPhase}' to '${targetPhase}'`, + { name } + ) + } + lastVersionCopy.phase = targetPhase + } else { + // Default transitions (skip labs) + if (currentPhase === ALPHA_PHASE) { + lastVersionCopy.phase = BETA_PHASE + } else if (currentPhase === LABS_PHASE) { + lastVersionCopy.phase = BETA_PHASE + } else if (currentPhase === BETA_PHASE) { + if (splitTest.forbidReleasePhase) { + throw new Errors.ForbiddenError( + 'Switch to release phase is disabled for this test', + { + name, + } + ) + } + lastVersionCopy.phase = RELEASE_PHASE + } else if (currentPhase === RELEASE_PHASE) { + throw new Errors.InvalidError( + `Split test with ID '${name}' is already in the release phase` + ) + } + } + + if (splitTest.forbidReleasePhase && lastVersionCopy.phase === RELEASE_PHASE) { + throw new Errors.ForbiddenError( + 'Switch to release phase is disabled for this test', + { + name, + } ) } - for (const variant of lastVersionCopy.variants) { - variant.rolloutPercent = 0 - variant.rolloutStripes = [] - if (variant.userCount) { - variant.userCount = 0 + + // Labs phase requires exactly one variant + if (lastVersionCopy.phase === LABS_PHASE) { + if (lastVersionCopy.variants.length !== 1) { + throw new Errors.InvalidError('Labs phase requires exactly one variant', { + name, + variantCount: lastVersionCopy.variants.length, + }) + } + // Auto-set variant to 100% rollout for labs + const variant = lastVersionCopy.variants[0] + variant.rolloutPercent = 100 + variant.rolloutStripes = [{ start: 0, end: 100 }] + if (variant.userLimit === undefined) { + variant.userLimit = labsUserLimit + } + variant.userCount = 0 + } else { + for (const variant of lastVersionCopy.variants) { + variant.rolloutPercent = 0 + variant.rolloutStripes = [] + if (clearUserLimit) { + delete variant.userLimit + delete variant.userCount + } else if (variant.userCount) { + variant.userCount = 0 + } } } + lastVersionCopy.author = userId lastVersionCopy.comment = comment lastVersionCopy.createdAt = new Date() @@ -280,12 +371,12 @@ async function revertToPreviousVersion( ) { const splitTest = await getSplitTest({ name }) if (!splitTest) { - throw new OError( + throw new Errors.NotFoundError( `Cannot revert split test with ID '${name}' to previous version: not found` ) } if (splitTest.archived) { - throw new OError( + throw new Errors.InvalidError( 'Cannot revert an archived split test to previous version', { name, @@ -293,13 +384,13 @@ async function revertToPreviousVersion( ) } if (splitTest.versions.length <= 1) { - throw new OError( + throw new Errors.InvalidError( `Cannot revert split test with ID '${name}' to previous version: split test must have at least 2 versions` ) } const previousVersion = SplitTestUtils.getVersion(splitTest, versionNumber) if (!previousVersion) { - throw new OError( + throw new Errors.NotFoundError( `Cannot revert split test with ID '${name}' to version number ${versionNumber}: version not found` ) } @@ -345,10 +436,14 @@ async function revertToPreviousVersion( async function archive(name, userId) { const splitTest = await getSplitTest({ name }) if (!splitTest) { - throw new OError(`Cannot archive split test with ID '${name}': not found`) + throw new Errors.NotFoundError( + `Cannot archive split test with ID '${name}': not found` + ) } if (splitTest.archived) { - throw new OError(`Split test with ID '${name}' is already archived`) + throw new Errors.InvalidError( + `Split test with ID '${name}' is already archived` + ) } splitTest.archived = true splitTest.archivedAt = new Date() @@ -366,28 +461,32 @@ function _checkNewVariantsConfiguration( analyticsEnabled ) { if (newVariantsConfiguration?.length > 1 && !analyticsEnabled) { - throw new OError(`Gradual rollouts can only have a single variant`) + throw new Errors.InvalidError( + `Gradual rollouts can only have a single variant` + ) } const totalRolloutPercentage = _getTotalRolloutPercentage( newVariantsConfiguration ) if (totalRolloutPercentage > 100) { - throw new OError(`Total variants rollout percentage cannot exceed 100`) + throw new Errors.InvalidError( + `Total variants rollout percentage cannot exceed 100` + ) } for (const variant of variants) { const newVariantConfiguration = _.find(newVariantsConfiguration, { name: variant.name, }) if (!newVariantConfiguration) { - throw new OError( + throw new Errors.InvalidError( `Variant defined in previous version as ${JSON.stringify( variant )} cannot be removed in new configuration: either set it inactive or create a new split test` ) } if (newVariantConfiguration.rolloutPercent < variant.rolloutPercent) { - throw new OError( + throw new Errors.InvalidError( `Rollout percentage for variant defined in previous version as ${JSON.stringify( variant )} cannot be decreased: revert to a previous configuration instead` @@ -399,14 +498,14 @@ function _checkNewVariantsConfiguration( newVariantConfiguration.userLimit !== undefined && newVariantConfiguration.userLimit < variant.userLimit ) { - throw new OError( + throw new Errors.InvalidError( `User limit for variant '${variant.name}' cannot be decreased: revert to a previous configuration instead` ) } } else { // Existing variant has no user limit - cannot add one if (newVariantConfiguration.userLimit !== undefined) { - throw new OError( + throw new Errors.InvalidError( `User limit cannot be added to variant '${variant.name}' after creation: user limits can only be set when the split test is created` ) } @@ -496,7 +595,7 @@ async function _saveSplitTest(splitTest) { */ function _checkEnvIsSafe(operation) { if (process.env.NODE_ENV !== 'development') { - throw new OError( + throw new Errors.ForbiddenError( `Attempted to ${operation} all feature flags outside of local env` ) } diff --git a/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.mjs b/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.mjs index 0f4ab589bb..d6248b3771 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.mjs @@ -114,6 +114,16 @@ function getCachedVariant(session, splitTestName, currentVersion) { return session.cachedSplitTestAssignments[cacheKey] } +function clearCachedVariant(session, splitTestName) { + if (!session.cachedSplitTestAssignments) return + for (const cacheKey of Object.keys(session.cachedSplitTestAssignments)) { + const name = cacheKey.split('-').slice(0, -1).join('-') + if (name === splitTestName) { + delete session.cachedSplitTestAssignments[cacheKey] + } + } +} + function setVariantInCache({ session, splitTestName, @@ -248,6 +258,7 @@ export default { getAssignments: callbackify(getAssignments), appendAssignment: callbackify(appendAssignment), getCachedVariant, + clearCachedVariant, setVariantInCache, sessionMaintenance: callbackify(sessionMaintenance), collectSessionStats, diff --git a/services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs b/services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs index 2454a51f6c..e77150c2ce 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs @@ -7,6 +7,8 @@ async function getUser(id, splitTestName) { analyticsId: 1, alphaProgram: 1, betaProgram: 1, + labsProgram: 1, + labsExperiments: 1, } if (splitTestName) { projection[`splitTests.${splitTestName}`] = 1 diff --git a/services/web/app/src/models/SplitTest.mjs b/services/web/app/src/models/SplitTest.mjs index dc6446e0c9..e3b5dcdfe8 100644 --- a/services/web/app/src/models/SplitTest.mjs +++ b/services/web/app/src/models/SplitTest.mjs @@ -33,6 +33,7 @@ const BadgeSchema = new Schema( const BadgeInfoSchema = new Schema( { alpha: BadgeSchema, + labs: BadgeSchema, beta: BadgeSchema, release: BadgeSchema, }, @@ -85,7 +86,7 @@ const VersionSchema = new Schema( phase: { type: String, default: 'alpha', - enum: ['alpha', 'beta', 'release'], + enum: ['alpha', 'labs', 'beta', 'release'], required: true, }, active: { @@ -174,6 +175,18 @@ export const SplitTestSchema = new Schema( type: BadgeInfoSchema, required: false, }, + labsTitle: { + type: String, + required: false, + }, + labsDescription: { + type: String, + required: false, + }, + labsIcon: { + type: String, + required: false, + }, }, { minimize: false } ) diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index f517d0314b..f9c2013116 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -1573,7 +1573,6 @@ "revoke_license": "", "right": "", "role": "", - "rolling_texlive_build": "", "saml_auth_error": "", "saml_email_not_in_account_error_managed_users_2": "", "saml_identity_exists_error": "", @@ -1886,7 +1885,6 @@ "tex_live_version": "", "thank_you": "", "thank_you_exclamation": "", - "thank_you_for_joining_the_rolling_texlive": "", "thank_you_for_your_feedback": "", "thanks_for_confirming_your_email_address": "", "thanks_for_getting_in_touch": "", @@ -1933,7 +1931,6 @@ "this_action_will_also_disable_domain_capture": "", "this_address_will_be_shown_on_the_invoice": "", "this_could_be_because_we_cant_support_some_elements_of_the_table": "", - "this_experiment_gives_you_access_to_new_versions_of_latex": "", "this_field_is_required": "", "this_grants_access_to_features_2": "", "this_is_the_file_that_references_pulled_from_your_reference_manager_will_be_added_to": "", diff --git a/services/web/frontend/js/features/ide-react/components/editor/editor-panel.tsx b/services/web/frontend/js/features/ide-react/components/editor/editor-panel.tsx index 17c5f7fd83..f65f45b74a 100644 --- a/services/web/frontend/js/features/ide-react/components/editor/editor-panel.tsx +++ b/services/web/frontend/js/features/ide-react/components/editor/editor-panel.tsx @@ -5,13 +5,11 @@ import FileView from '@/features/file-view/components/file-view' import { fileViewFile } from '@/features/ide-react/util/file-view' import MultipleSelectionPane from '@/features/ide-react/components/editor/multiple-selection-pane' import { TabsContainer } from '@/features/source-editor/components/tabs/tabs-container' -import { isInExperiment } from '@/utils/labs-utils' import { isSplitTestEnabled } from '@/utils/splitTestUtils' export default function EditorPanel() { const { selectedEntityCount, openEntity } = useFileTreeOpenContext() - const tabsEnabled = - isInExperiment('editor-tabs') && isSplitTestEnabled('editor-tabs') + const tabsEnabled = isSplitTestEnabled('editor-tabs') return (
diff --git a/services/web/frontend/js/features/ide-react/components/layout/editor.tsx b/services/web/frontend/js/features/ide-react/components/layout/editor.tsx index e534dc6a06..d208669fdc 100644 --- a/services/web/frontend/js/features/ide-react/components/layout/editor.tsx +++ b/services/web/frontend/js/features/ide-react/components/layout/editor.tsx @@ -10,7 +10,7 @@ import { FullSizeLoadingSpinner } from '@/shared/components/loading-spinner' import SymbolPalettePane from '@/features/ide-react/components/editor/symbol-palette-pane' import { useEditorPropertiesContext } from '@/features/ide-react/context/editor-properties-context' import { PythonEditorSplit } from '@/features/ide-react/components/layout/python-editor-split' -import { isInExperiment } from '@/utils/labs-utils' +import { isSplitTestEnabled } from '@/utils/splitTestUtils' export const Editor = () => { const { opening, errorState, showSymbolPalette } = @@ -20,7 +20,7 @@ export const Editor = () => { const isPythonDocument = openEntity?.type === 'doc' && openEntity.entity.name.toLowerCase().endsWith('.py') - const pythonExecutionEnabled = isInExperiment('overleaf-code') + const pythonExecutionEnabled = isSplitTestEnabled('overleaf-code') if (!currentDocumentId) { return null diff --git a/services/web/frontend/js/features/ide-react/context/tabs-context.tsx b/services/web/frontend/js/features/ide-react/context/tabs-context.tsx index 0be66bc1ff..c5d2e307a3 100644 --- a/services/web/frontend/js/features/ide-react/context/tabs-context.tsx +++ b/services/web/frontend/js/features/ide-react/context/tabs-context.tsx @@ -6,7 +6,6 @@ import React, { FC, useCallback, useContext, useEffect, useMemo } from 'react' import { useFileTreeOpenContext } from './file-tree-open-context' import { useEditorManagerContext } from './editor-manager-context' import { debugConsole } from '@/utils/debugging' -import { isInExperiment } from '@/utils/labs-utils' import { disambiguatePaths } from '../util/disambiguate-paths' import { isSplitTestEnabled } from '@/utils/splitTestUtils' @@ -43,8 +42,7 @@ export const TabsProvider: FC = ({ children }) => { const { openEntity } = useFileTreeOpenContext() const { openDocWithId, openFileWithId } = useEditorManagerContext() - const tabsEnabled = - isInExperiment('editor-tabs') && isSplitTestEnabled('editor-tabs') + const tabsEnabled = isSplitTestEnabled('editor-tabs') const [openTabs, setOpenTabs] = usePersistedState( `open-tabs:${projectId}`, diff --git a/services/web/frontend/js/features/monthly-texlive/labs-widget.tsx b/services/web/frontend/js/features/monthly-texlive/labs-widget.tsx deleted file mode 100644 index fcadc9507c..0000000000 --- a/services/web/frontend/js/features/monthly-texlive/labs-widget.tsx +++ /dev/null @@ -1,99 +0,0 @@ -import { useCallback, useState } from 'react' -import LabsExperimentWidget from '../../shared/components/labs/labs-experiments-widget' -import { isInExperiment } from '@/utils/labs-utils' -import { useTranslation, Trans } from 'react-i18next' -import MaterialIcon from '@/shared/components/material-icon' -import { isSplitTestEnabled } from '@/utils/splitTestUtils' -import { postJSON } from '@/infrastructure/fetch-json' -import { debugConsole } from '@/utils/debugging' - -export const TUTORIAL_KEY = 'rolling-compile-image-changed' - -const MonthlyTexliveLabsWidget = ({ - labsProgram, - setErrorMessage, -}: { - labsProgram: boolean - setErrorMessage: (err: string) => void -}) => { - const { t } = useTranslation() - const [optedIn, setOptedIn] = useState(isInExperiment('monthly-texlive')) - - const optInWithCompletedTutorial = useCallback( - async (shouldOptIn: boolean) => { - try { - await postJSON(`/tutorial/${TUTORIAL_KEY}/complete`) - } catch (err) { - debugConsole.error(err) - } - setOptedIn(shouldOptIn) - }, - [setOptedIn] - ) - - const monthlyTexLiveSplitTestEnabled = isSplitTestEnabled('monthly-texlive') - if (!monthlyTexLiveSplitTestEnabled) { - return null - } - - const logo = ( - - ) - - const optedInDescription = ( - , - ]} - /> - ) - - const description = ( - , - // eslint-disable-next-line jsx-a11y/anchor-has-content - , - ]} - /> - ) - - return ( - - ) -} - -export const hidden = () => !isSplitTestEnabled('monthly-texlive') - -export default MonthlyTexliveLabsWidget diff --git a/services/web/frontend/js/features/overleaf-code/labs-widget.tsx b/services/web/frontend/js/features/overleaf-code/labs-widget.tsx deleted file mode 100644 index a30e5302bc..0000000000 --- a/services/web/frontend/js/features/overleaf-code/labs-widget.tsx +++ /dev/null @@ -1,48 +0,0 @@ -import LabsExperimentWidget, { - LabsExperimentWidgetProps, -} from '@/shared/components/labs/labs-experiments-widget' -import MaterialIcon from '@/shared/components/material-icon' -import { isInExperiment } from '@/utils/labs-utils' -import { isSplitTestEnabled } from '@/utils/splitTestUtils' -import { useState } from 'react' - -type LabsWidgetProps = Pick & { - labsProgram: boolean -} - -const LabsWidget = ({ setErrorMessage, labsProgram }: LabsWidgetProps) => { - const [optedIn, setOptedIn] = useState(isInExperiment('overleaf-code')) - - if (!isSplitTestEnabled('overleaf-code')) { - return null - } - - const description = ( - - Run Python code while editing .py files - - ) - - return ( - - } - optedInDescription={description} - labsEnabled={labsProgram} - /> - ) -} - -export const hidden = () => !isSplitTestEnabled('overleaf-code') - -export default LabsWidget diff --git a/services/web/frontend/js/features/source-editor/components/tabs/tabs-experiment-widget.tsx b/services/web/frontend/js/features/source-editor/components/tabs/tabs-experiment-widget.tsx deleted file mode 100644 index 4f73385ff1..0000000000 --- a/services/web/frontend/js/features/source-editor/components/tabs/tabs-experiment-widget.tsx +++ /dev/null @@ -1,42 +0,0 @@ -import LabsExperimentWidget, { - LabsExperimentWidgetProps, -} from '@/shared/components/labs/labs-experiments-widget' -import { isInExperiment } from '@/utils/labs-utils' -import { useState } from 'react' -import MaterialIcon from '@/shared/components/material-icon' -import { isSplitTestEnabled } from '@/utils/splitTestUtils' - -type LabsWidgetProps = Pick & { - labsProgram: boolean -} - -const LabsWidget = ({ setErrorMessage, labsProgram }: LabsWidgetProps) => { - const [optedIn, setOptedIn] = useState(isInExperiment('editor-tabs')) - - const description = ( - Quickly switch between open files and documents using tabs. - ) - - return ( - - } - optedInDescription={description} - labsEnabled={labsProgram} - /> - ) -} - -export const hidden = () => !isSplitTestEnabled('editor-tabs') - -export default LabsWidget diff --git a/services/web/frontend/js/shared/components/beta-badge.tsx b/services/web/frontend/js/shared/components/beta-badge.tsx index 0d44d14a05..c2b260adad 100644 --- a/services/web/frontend/js/shared/components/beta-badge.tsx +++ b/services/web/frontend/js/shared/components/beta-badge.tsx @@ -18,25 +18,21 @@ type LinkProps = { onMouseDown?: MouseEventHandler } -const defaultHref = '/beta/participate' - const BetaBadge: FC<{ tooltip?: TooltipProps link?: LinkProps description?: ReactNode phase?: string -}> = ({ - tooltip, - link = { href: defaultHref }, - description, - phase = 'beta', -}) => { +}> = ({ tooltip, link = {}, description, phase = 'beta' }) => { const { href, ...linkProps } = link + const linkedBadge = ( {description || tooltip?.text} diff --git a/services/web/frontend/js/shared/components/labs/labs-description.tsx b/services/web/frontend/js/shared/components/labs/labs-description.tsx new file mode 100644 index 0000000000..f28405a444 --- /dev/null +++ b/services/web/frontend/js/shared/components/labs/labs-description.tsx @@ -0,0 +1,38 @@ +import { FC, useMemo } from 'react' +import { micromark } from 'micromark' +import DOMPurify from 'dompurify' + +const PURIFY_CONFIG = { + ALLOWED_TAGS: ['#text', 'p', 'em', 'strong', 'a'], + ALLOWED_ATTR: ['href'], +} + +const LINK_REL = 'noreferrer noopener' +const LINK_TARGET = '_BLANK' + +function sanitizeDescription(description: string) { + DOMPurify.addHook('afterSanitizeAttributes', node => { + if (node.nodeName === 'A') { + node.setAttribute('rel', LINK_REL) + node.setAttribute('target', LINK_TARGET) + } + }) + + try { + return DOMPurify.sanitize(micromark(description), PURIFY_CONFIG) + } finally { + DOMPurify.removeHook('afterSanitizeAttributes') + } +} + +/** + * Renders a labs experiment description from markdown to sanitized HTML. + * Only bold, italic, and links are supported. + */ +export const LabsDescription: FC<{ description: string }> = ({ + description, +}) => { + const html = useMemo(() => sanitizeDescription(description), [description]) + + return
+} diff --git a/services/web/frontend/js/shared/components/labs/labs-experiment-icon.tsx b/services/web/frontend/js/shared/components/labs/labs-experiment-icon.tsx new file mode 100644 index 0000000000..7c02e77fb1 --- /dev/null +++ b/services/web/frontend/js/shared/components/labs/labs-experiment-icon.tsx @@ -0,0 +1,21 @@ +import React from 'react' +import MaterialIcon from '@/shared/components/material-icon' + +type LabsExperimentIconProps = { + icon: string +} + +const LabsExperimentIcon: React.FC = ({ icon }) => { + if (!icon) { + return null + } + + return ( + + ) +} + +export default LabsExperimentIcon diff --git a/services/web/frontend/js/shared/components/labs/labs-experiments-widget.tsx b/services/web/frontend/js/shared/components/labs/labs-experiments-widget.tsx index db7ea358e4..7a9f882b41 100644 --- a/services/web/frontend/js/shared/components/labs/labs-experiments-widget.tsx +++ b/services/web/frontend/js/shared/components/labs/labs-experiments-widget.tsx @@ -73,14 +73,14 @@ export function LabsExperimentWidget({

{title}

{optedIn && {t('enabled')}}
-

+

{optedIn && feedbackLink && ( diff --git a/services/web/frontend/js/utils/labs-utils.ts b/services/web/frontend/js/utils/labs-utils.ts deleted file mode 100644 index 5ec880726f..0000000000 --- a/services/web/frontend/js/utils/labs-utils.ts +++ /dev/null @@ -1,15 +0,0 @@ -import getMeta from './meta' - -// Should be `never` when no experiments are active. Otherwise it should be a -// union of active experiment names e.g. `'experiment1' | 'experiment2'` - -export type ActiveExperiment = - | 'monthly-texlive' - | 'bibtex-visual-editor' - | 'overleaf-code' - | 'editor-tabs' - -export const isInExperiment = (experiment: ActiveExperiment): boolean => { - const experiments = getMeta('ol-labsExperiments') - return Boolean(experiments?.includes(experiment)) -} diff --git a/services/web/frontend/js/utils/meta.ts b/services/web/frontend/js/utils/meta.ts index 6b0b077ed3..166fb376ad 100644 --- a/services/web/frontend/js/utils/meta.ts +++ b/services/web/frontend/js/utils/meta.ts @@ -62,7 +62,6 @@ import { SubscriptionCreationPreview } from '../../../types/subscription/subscri import { DefaultNavbarMetadata } from '@/shared/components/types/default-navbar-metadata' import { FooterMetadata } from '@/shared/components/types/footer-metadata' import type { ScriptLogType } from '../../../modules/admin-panel/frontend/js/features/script-logs/script-log' -import { ActiveExperiment } from './labs-utils' import { Subscription as AdminSubscription } from '../../../types/admin/subscription' import { AdminCapability } from '../../../types/admin-capabilities' import { AlgoliaConfig } from '../../../modules/algolia-search/frontend/js/types' @@ -192,7 +191,15 @@ export interface Meta { 'ol-itm_referrer': string 'ol-joinedGroupName': string 'ol-labs': boolean - 'ol-labsExperiments': ActiveExperiment[] | undefined + 'ol-labsExperiments': Array<{ + name: string + title: string + description: string + icon: string + surveyLink: string + isFull: boolean + optedIn: boolean + }> 'ol-languages': SpellCheckLanguage[] 'ol-learnedWords': string[] 'ol-legacyEditorThemes': { name: string; dark: boolean }[] diff --git a/services/web/locales/en.json b/services/web/locales/en.json index af664c184d..26c7796bf7 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -2031,7 +2031,6 @@ "right": "Right", "ro": "Romanian", "role": "Role", - "rolling_texlive_build": "Rolling TeX Live releases (advanced)", "ru": "Russian", "saml": "SAML", "saml_auth_error": "Sorry, your identity provider responded with an error. Please contact your administrator for more information.", @@ -2411,7 +2410,6 @@ "thank_you_email_confirmed": "Thank you, your email is now confirmed", "thank_you_exclamation": "Thank you!", "thank_you_for_being_part_of_our_beta_program": "Thank you for being part of our beta program, where you can have <0>early access to new features and help us understand your needs better", - "thank_you_for_joining_the_rolling_texlive": "Thank you for joining the rolling TeX Live build’s experiment. To get started, check out <0>our guide on how to change the TeX Live image for a project. The “Rolling TeX Live (Labs)” option should now be present. Note that after leaving the experiment, projects set to the rolling image will continue to compile using that image unless manually switched to an older TeX Live version.", "thank_you_for_your_feedback": "Thank you for your feedback!", "thanks": "Thanks", "thanks_for_confirming_your_email_address": "Thanks for confirming your email address", @@ -2468,7 +2466,6 @@ "this_action_will_also_disable_domain_capture": "This action will also disable domain capture.", "this_address_will_be_shown_on_the_invoice": "This address will be shown on the invoice", "this_could_be_because_we_cant_support_some_elements_of_the_table": "This could be because we can’t yet support some elements of the table in the table preview. Or there may be an error in the table’s LaTeX code.", - "this_experiment_gives_you_access_to_new_versions_of_latex": "For advanced users only. This experiment gives you regular access to new, untested versions of TeX Live (the LaTeX engine used for compiling). You can then <0>choose this as your compiler on a project-by-project basis. <1>Find out more about rolling TeX Live", "this_field_is_required": "This field is required", "this_grants_access_to_features_2": "This grants you access to <0>__appName__ <0>__featureType__ features.", "this_is_the_file_that_references_pulled_from_your_reference_manager_will_be_added_to": "This is the file that references pulled from your reference manager will be added to.", diff --git a/services/web/test/frontend/shared/components/labs-description.test.tsx b/services/web/test/frontend/shared/components/labs-description.test.tsx new file mode 100644 index 0000000000..7d964288ea --- /dev/null +++ b/services/web/test/frontend/shared/components/labs-description.test.tsx @@ -0,0 +1,30 @@ +import { expect } from 'chai' +import { render } from '@testing-library/react' + +import { LabsDescription } from '@/shared/components/labs/labs-description' + +describe('', function () { + it('adds rel and target attributes to rendered links', function () { + const { container } = render( + + ) + + const link = container.querySelector('a') + expect(link).to.not.equal(null) + expect(link?.getAttribute('href')).to.equal('https://example.com') + expect(link?.getAttribute('rel')).to.equal('noreferrer noopener') + expect(link?.getAttribute('target')).to.equal('_BLANK') + }) + + it('preserves href sanitization for unsafe links', function () { + const { container } = render( + + ) + + const link = container.querySelector('a') + expect(link).to.not.equal(null) + expect(link?.getAttribute('href')).to.equal('') + expect(link?.getAttribute('rel')).to.equal('noreferrer noopener') + expect(link?.getAttribute('target')).to.equal('_BLANK') + }) +}) diff --git a/services/web/test/unit/src/Project/ProjectHelper.test.mjs b/services/web/test/unit/src/Project/ProjectHelper.test.mjs index e8f8564e66..a4a7ebcdba 100644 --- a/services/web/test/unit/src/Project/ProjectHelper.test.mjs +++ b/services/web/test/unit/src/Project/ProjectHelper.test.mjs @@ -49,6 +49,12 @@ describe('ProjectHelper', function () { ], } + ctx.SplitTestHandler = { + promises: { + getAssignmentForUser: vi.fn().mockResolvedValue({ variant: 'default' }), + }, + } + vi.doMock('mongodb-legacy', () => ({ default: { ObjectId }, })) @@ -57,6 +63,13 @@ describe('ProjectHelper', function () { default: ctx.Settings, })) + vi.doMock( + '../../../../app/src/Features/SplitTests/SplitTestHandler.mjs', + () => ({ + default: ctx.SplitTestHandler, + }) + ) + ctx.ProjectHelper = (await import(MODULE_PATH)).default }) @@ -145,8 +158,8 @@ describe('ProjectHelper', function () { }) describe('getAllowedImagesForUser', function () { - it('marks alpha only images as not allowed when the user is anonymous', function (ctx) { - const images = ctx.ProjectHelper.getAllowedImagesForUser(null) + it('marks alpha only images as not allowed when the user is anonymous', async function (ctx) { + const images = await ctx.ProjectHelper.getAllowedImagesForUser(null) const imageNames = _mapToAllowed(images) expect(imageNames).to.deep.equal([ { imageName: 'texlive-full:2018.1', allowed: true }, @@ -156,8 +169,8 @@ describe('ProjectHelper', function () { ]) }) - it('marks monthly labs images as not allowed when the user is anonymous', function (ctx) { - const images = ctx.ProjectHelper.getAllowedImagesForUser(null) + it('marks monthly labs images as not allowed when the user is anonymous', async function (ctx) { + const images = await ctx.ProjectHelper.getAllowedImagesForUser(null) const imageNames = _mapToAllowed(images) expect(imageNames).to.deep.equal([ { imageName: 'texlive-full:2018.1', allowed: true }, @@ -167,8 +180,11 @@ describe('ProjectHelper', function () { ]) }) - it('marks monthly labs images as allowed when the user is enrolled', function (ctx) { - const images = ctx.ProjectHelper.getAllowedImagesForUser(ctx.user) + it('marks monthly labs images as allowed when the user is enrolled', async function (ctx) { + ctx.SplitTestHandler.promises.getAssignmentForUser.mockResolvedValue({ + variant: 'enabled', + }) + const images = await ctx.ProjectHelper.getAllowedImagesForUser(ctx.user) const imageNames = _mapToAllowed(images) expect(imageNames).to.deep.equal([ { imageName: 'texlive-full:2018.1', allowed: true }, @@ -178,19 +194,21 @@ describe('ProjectHelper', function () { ]) }) - it('marks alpha only images as not allowed when when the user is not admin', function (ctx) { - const images = ctx.ProjectHelper.getAllowedImagesForUser(ctx.user) + it('marks alpha only images as not allowed when when the user is not admin', async function (ctx) { + const images = await ctx.ProjectHelper.getAllowedImagesForUser(ctx.user) const imageNames = _mapToAllowed(images) expect(imageNames).to.deep.equal([ { imageName: 'texlive-full:2018.1', allowed: true }, { imageName: 'texlive-full:2019.1', allowed: true }, { imageName: 'texlive-full:2020.1', allowed: false }, - { imageName: 'texlive-full:2021.1', allowed: true }, + { imageName: 'texlive-full:2021.1', allowed: false }, ]) }) - it('returns all images when the user is admin', function (ctx) { - const images = ctx.ProjectHelper.getAllowedImagesForUser(ctx.adminUser) + it('returns all images when the user is admin', async function (ctx) { + const images = await ctx.ProjectHelper.getAllowedImagesForUser( + ctx.adminUser + ) const imageNames = _mapToAllowed(images) expect(imageNames).to.deep.equal([ { imageName: 'texlive-full:2018.1', allowed: true }, diff --git a/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs b/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs index 9a48c4735a..2d19b128ff 100644 --- a/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs +++ b/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs @@ -499,21 +499,322 @@ describe('SplitTestHandler', function () { expect(assignment.variant).to.equal('variant-1') }) + }) - it('should assign to default if userCount is undefined', async function (ctx) { + describe('labs phase assignment (split test)', function () { + beforeEach(function (ctx) { + ctx.AnalyticsManager.getIdsFromSession.returns({ + userId: 'abc123abc123', + }) ctx.cachedSplitTests.set( - 'active-test', - makeSplitTest('active-test', { userLimit: 100, userCount: undefined }) + 'labs-experiment', + makeSplitTest('labs-experiment', { phase: 'labs' }) ) + }) + + it('should assign to variant when user is in labs program and has opted into the experiment', async function (ctx) { + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: true, + labsExperiments: ['labs-experiment'], + }) const assignment = await ctx.SplitTestHandler.promises.getAssignment( ctx.req, ctx.res, - 'active-test' + 'labs-experiment' + ) + + expect(assignment.variant).to.equal('variant-1') + }) + + it('should assign to default when user is in labs program but has not opted into the experiment', async function (ctx) { + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: true, + labsExperiments: ['some-other-experiment'], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment' ) expect(assignment.variant).to.equal('default') }) + + it('should assign to default when user is not in labs program', async function (ctx) { + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: false, + labsExperiments: [], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment' + ) + + expect(assignment.variant).to.equal('default') + }) + + it('should assign to default when user has no labsExperiments field', async function (ctx) { + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: true, + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment' + ) + + expect(assignment.variant).to.equal('default') + }) + + it('should assign to variant when under user limit', async function (ctx) { + ctx.cachedSplitTests.set( + 'labs-experiment', + makeSplitTest('labs-experiment', { + phase: 'labs', + userLimit: 10, + userCount: 5, + }) + ) + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: true, + labsExperiments: ['labs-experiment'], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment' + ) + + expect(assignment.variant).to.equal('variant-1') + }) + + it('should assign enrolled user to variant even when user limit is reached', async function (ctx) { + ctx.cachedSplitTests.set( + 'labs-experiment', + makeSplitTest('labs-experiment', { + phase: 'labs', + userLimit: 10, + userCount: 10, + }) + ) + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: true, + labsExperiments: ['labs-experiment'], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment' + ) + + expect(assignment.variant).to.equal('variant-1') + }) + + it('should allow already-assigned user even when limit is reached', async function (ctx) { + ctx.cachedSplitTests.set( + 'labs-experiment', + makeSplitTest('labs-experiment', { + phase: 'labs', + userLimit: 10, + userCount: 10, + }) + ) + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: { + 'labs-experiment': [ + { + variantName: 'variant-1', + versionNumber: 1, + assignedAt: new Date(), + phase: 'labs', + }, + ], + }, + labsProgram: true, + labsExperiments: ['labs-experiment'], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment' + ) + + expect(assignment.variant).to.equal('variant-1') + }) + }) + + describe('labs phase assignment (gradual rollout)', function () { + beforeEach(function (ctx) { + ctx.AnalyticsManager.getIdsFromSession.returns({ + userId: 'abc123abc123', + }) + ctx.cachedSplitTests.set( + 'labs-experiment-gr', + makeSplitTest('labs-experiment-gr', { + phase: 'labs', + analyticsEnabled: false, + }) + ) + }) + + it('should assign to variant when user is in labs program and has opted in', async function (ctx) { + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: true, + labsExperiments: ['labs-experiment-gr'], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment-gr' + ) + + expect(assignment.variant).to.equal('variant-1') + }) + + it('should assign to default when user has not opted in', async function (ctx) { + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: true, + labsExperiments: [], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment-gr' + ) + + expect(assignment.variant).to.equal('default') + }) + + it('should assign to default when user is not in labs program', async function (ctx) { + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: false, + labsExperiments: [], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment-gr' + ) + + expect(assignment.variant).to.equal('default') + }) + + it('should assign to variant when under user limit', async function (ctx) { + ctx.cachedSplitTests.set( + 'labs-experiment-gr', + makeSplitTest('labs-experiment-gr', { + phase: 'labs', + analyticsEnabled: false, + userLimit: 10, + userCount: 5, + }) + ) + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: true, + labsExperiments: ['labs-experiment-gr'], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment-gr' + ) + + expect(assignment.variant).to.equal('variant-1') + }) + + it('should assign enrolled user to variant even when user limit is reached', async function (ctx) { + ctx.cachedSplitTests.set( + 'labs-experiment-gr', + makeSplitTest('labs-experiment-gr', { + phase: 'labs', + analyticsEnabled: false, + userLimit: 10, + userCount: 10, + }) + ) + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: {}, + labsProgram: true, + labsExperiments: ['labs-experiment-gr'], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment-gr' + ) + + expect(assignment.variant).to.equal('variant-1') + }) + + it('should allow already-assigned user even when limit is reached', async function (ctx) { + ctx.cachedSplitTests.set( + 'labs-experiment-gr', + makeSplitTest('labs-experiment-gr', { + phase: 'labs', + analyticsEnabled: false, + userLimit: 10, + userCount: 10, + }) + ) + ctx.SplitTestUserGetter.promises.getUser.resolves({ + _id: new ObjectId('abc123abc123abc123abc123'), + splitTests: { + 'labs-experiment-gr': [ + { + variantName: 'variant-1', + versionNumber: 1, + assignedAt: new Date(), + phase: 'labs', + }, + ], + }, + labsProgram: true, + labsExperiments: ['labs-experiment-gr'], + }) + + const assignment = await ctx.SplitTestHandler.promises.getAssignment( + ctx.req, + ctx.res, + 'labs-experiment-gr' + ) + + expect(assignment.variant).to.equal('variant-1') + }) }) }) diff --git a/services/web/test/unit/src/SplitTests/SplitTestSessionHandler.test.mjs b/services/web/test/unit/src/SplitTests/SplitTestSessionHandler.test.mjs index a29bf22b22..8ad2addb23 100644 --- a/services/web/test/unit/src/SplitTests/SplitTestSessionHandler.test.mjs +++ b/services/web/test/unit/src/SplitTests/SplitTestSessionHandler.test.mjs @@ -325,4 +325,52 @@ describe('SplitTestSessionHandler', function () { ], }) }) + + describe('clearCachedVariant', function () { + it('should remove all cached entries for a given split test name', function (ctx) { + const session = { + cachedSplitTestAssignments: { + 'my-test-1': 'variant-1', + 'my-test-2': 'variant-2', + 'other-test-1': 'variant-1', + }, + } + ctx.SplitTestSessionHandler.clearCachedVariant(session, 'my-test') + expect(session.cachedSplitTestAssignments).to.deep.equal({ + 'other-test-1': 'variant-1', + }) + }) + + it('should handle split test names with hyphens correctly', function (ctx) { + const session = { + cachedSplitTestAssignments: { + 'monthly-texlive-1': 'enabled', + 'monthly-texlive-2': 'enabled', + 'monthly-1': 'variant-1', + }, + } + ctx.SplitTestSessionHandler.clearCachedVariant(session, 'monthly-texlive') + expect(session.cachedSplitTestAssignments).to.deep.equal({ + 'monthly-1': 'variant-1', + }) + }) + + it('should do nothing when session has no cached assignments', function (ctx) { + const session = {} + ctx.SplitTestSessionHandler.clearCachedVariant(session, 'my-test') + expect(session.cachedSplitTestAssignments).to.be.undefined + }) + + it('should do nothing when there are no matching entries', function (ctx) { + const session = { + cachedSplitTestAssignments: { + 'other-test-1': 'variant-1', + }, + } + ctx.SplitTestSessionHandler.clearCachedVariant(session, 'my-test') + expect(session.cachedSplitTestAssignments).to.deep.equal({ + 'other-test-1': 'variant-1', + }) + }) + }) })