mirror of
https://github.com/yu-i-i/overleaf-cep.git
synced 2026-11-12 13:13:45 +01:00
[web] Gate labs experiments behind premium compiles (#36339)
* feat: gating labs experiments behind premium compile feature * feat: move filtering to the labs module, and only propogate experiments that user can opt into on the FE GitOrigin-RevId: 984aa0339c7dc8d61f0f6830780a599508306d5e
This commit is contained in:
committed by
Copybot
parent
2c8e2781c7
commit
1c0bc6c50a
@@ -9,6 +9,7 @@ import SplitTestCache from './SplitTestCache.mjs'
|
||||
import { SplitTest } from '../../models/SplitTest.mjs'
|
||||
import UserAnalyticsDataCache from '../Analytics/UserAnalyticsDataCache.mjs'
|
||||
import Features from '../../infrastructure/Features.mjs'
|
||||
import Modules from '../../infrastructure/Modules.mjs'
|
||||
import SplitTestUtils from './SplitTestUtils.mjs'
|
||||
import Settings from '@overleaf/settings'
|
||||
import SessionManager from '../Authentication/SessionManager.mjs'
|
||||
@@ -923,7 +924,10 @@ async function _loadSplitTestInfoInLocals(locals, splitTestName, session) {
|
||||
badgeInfo: splitTest.badgeInfo?.[phase],
|
||||
}
|
||||
|
||||
if (phase === 'labs') {
|
||||
if (
|
||||
phase === 'labs' &&
|
||||
(await _userMeetsLabsRequirements(splitTest, session))
|
||||
) {
|
||||
const variant = currentVersion.variants?.[0]
|
||||
info.labsDetails = {
|
||||
title: splitTest.labsTitle || '',
|
||||
@@ -959,6 +963,38 @@ async function _loadSplitTestInfoInLocals(locals, splitTestName, session) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the user meets the requirements a labs experiment declares, e.g.
|
||||
* having premium compiles available.
|
||||
*
|
||||
* The check lives in the labs module, which owns both the list of requirements
|
||||
* and the user lookup they need, so this is a no-op when that module is not
|
||||
* loaded. Experiments the user does not qualify for are left out of the labs
|
||||
* details, and so never reach the frontend.
|
||||
*
|
||||
* @param {object} splitTest a labs-phase split test
|
||||
* @param {object} session the request session
|
||||
* @returns {Promise<boolean>}
|
||||
*/
|
||||
async function _userMeetsLabsRequirements(splitTest, session) {
|
||||
try {
|
||||
const results = await Modules.promises.hooks.fire(
|
||||
'userMeetsLabsExperimentRequirements',
|
||||
session,
|
||||
splitTest
|
||||
)
|
||||
return results.every(result => result !== false)
|
||||
} catch (error) {
|
||||
// leave the experiment out rather than advertise one the user may not be
|
||||
// able to opt into. The opt-in endpoint enforces the same requirements.
|
||||
logger.warn(
|
||||
{ err: error, splitTestName: splitTest.name },
|
||||
'failed to check labs experiment requirements'
|
||||
)
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
function _getNonSaasAssignment(splitTestName) {
|
||||
if (Settings.splitTestOverrides?.[splitTestName]) {
|
||||
return {
|
||||
|
||||
@@ -98,6 +98,9 @@ async function createSplitTest(
|
||||
labsDescription: labsInfo.description,
|
||||
labsIcon: labsInfo.icon,
|
||||
labsSuccessNotification: labsInfo.successNotification,
|
||||
labsRequirePremiumCompiles: labsInfo.requirePremiumCompiles
|
||||
? true
|
||||
: undefined,
|
||||
versions: [
|
||||
{
|
||||
versionNumber: 1,
|
||||
@@ -177,6 +180,11 @@ async function updateSplitTestInfo(name, info, labsInfo) {
|
||||
splitTest.labsDescription = labsInfo.description
|
||||
splitTest.labsIcon = labsInfo.icon
|
||||
splitTest.labsSuccessNotification = labsInfo.successNotification
|
||||
// unset rather than stored as false, so turning the requirement back off
|
||||
// leaves the split test as it was before it was ever set
|
||||
splitTest.labsRequirePremiumCompiles = labsInfo.requirePremiumCompiles
|
||||
? true
|
||||
: undefined
|
||||
}
|
||||
return _saveSplitTest(splitTest)
|
||||
}
|
||||
|
||||
@@ -111,6 +111,18 @@ function compareFeatures(currentFeatures, expectedFeatures) {
|
||||
return mismatchReasons
|
||||
}
|
||||
|
||||
// Compile groups that count as having premium compiles available.
|
||||
const PREMIUM_COMPILE_GROUPS = ['alpha', 'priority']
|
||||
|
||||
/**
|
||||
* Whether a feature set (or a resolved set of compile limits) has premium
|
||||
* compiles available. Takes anything with a `compileGroup`, so it works for
|
||||
* both a user's features and the limits resolved for a compile.
|
||||
*/
|
||||
function hasPremiumCompiles(features) {
|
||||
return PREMIUM_COMPILE_GROUPS.includes(features?.compileGroup)
|
||||
}
|
||||
|
||||
function getMatchedFeatureSet(features) {
|
||||
for (const [name, featureSet] of Object.entries(Settings.features)) {
|
||||
if (_.isEqual(features, featureSet)) {
|
||||
@@ -125,5 +137,6 @@ export default {
|
||||
computeFeatureSet,
|
||||
isFeatureSetBetter,
|
||||
compareFeatures,
|
||||
hasPremiumCompiles,
|
||||
getMatchedFeatureSet,
|
||||
}
|
||||
|
||||
@@ -213,6 +213,14 @@ export const SplitTestSchema = new Schema(
|
||||
type: LabsSuccessNotificationSchema,
|
||||
required: false,
|
||||
},
|
||||
// When set, only users with a premium compile group can see this
|
||||
// experiment on the Labs page or opt into it. Left unset rather than
|
||||
// defaulted to false, so split tests that never use it don't carry a
|
||||
// labs-only field.
|
||||
labsRequirePremiumCompiles: {
|
||||
type: Boolean,
|
||||
required: false,
|
||||
},
|
||||
},
|
||||
{ minimize: false }
|
||||
)
|
||||
|
||||
@@ -68,6 +68,14 @@ describe('SplitTestHandler', function () {
|
||||
ctx.SessionManager = {
|
||||
isUserLoggedIn: sinon.stub().returns(false),
|
||||
}
|
||||
// no module gates labs experiments unless a test says otherwise
|
||||
ctx.Modules = {
|
||||
promises: {
|
||||
hooks: {
|
||||
fire: sinon.stub().resolves([]),
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
Features = {
|
||||
hasFeature: vi.fn().mockReturnValue(true),
|
||||
@@ -77,6 +85,10 @@ describe('SplitTestHandler', function () {
|
||||
default: Features,
|
||||
}))
|
||||
|
||||
vi.doMock('../../../../app/src/infrastructure/Modules', () => ({
|
||||
default: ctx.Modules,
|
||||
}))
|
||||
|
||||
vi.doMock('../../../../app/src/Features/User/UserGetter', () => ({
|
||||
default: ctx.UserGetter,
|
||||
}))
|
||||
@@ -1084,6 +1096,108 @@ describe('SplitTestHandler', function () {
|
||||
sinon.match(info => info.labsDetails === undefined)
|
||||
)
|
||||
})
|
||||
|
||||
describe('experiment requirements', function () {
|
||||
beforeEach(function (ctx) {
|
||||
ctx.labsSplitTest = {
|
||||
name: 'labs-requirements-test',
|
||||
labsTitle: 'Needs premium compiles',
|
||||
versions: [
|
||||
{
|
||||
active: true,
|
||||
analyticsEnabled: true,
|
||||
phase: 'labs',
|
||||
versionNumber: 1,
|
||||
createdAt: new Date('2024-06-15T12:00:00.000Z'),
|
||||
variants: [
|
||||
{
|
||||
name: 'variant-1',
|
||||
rolloutPercent: 100,
|
||||
rolloutStripes: [{ start: 0, end: 100 }],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
}
|
||||
ctx.cachedSplitTests.set('labs-requirements-test', ctx.labsSplitTest)
|
||||
})
|
||||
|
||||
it('asks the labs module whether the user meets the requirements', async function (ctx) {
|
||||
await ctx.SplitTestHandler.promises.getAssignment(
|
||||
ctx.req,
|
||||
ctx.res,
|
||||
'labs-requirements-test'
|
||||
)
|
||||
|
||||
expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
|
||||
'userMeetsLabsExperimentRequirements',
|
||||
ctx.req.session,
|
||||
ctx.labsSplitTest
|
||||
)
|
||||
})
|
||||
|
||||
it('does not set labsDetails when the user does not meet them', async function (ctx) {
|
||||
ctx.Modules.promises.hooks.fire.resolves([false])
|
||||
|
||||
await ctx.SplitTestHandler.promises.getAssignment(
|
||||
ctx.req,
|
||||
ctx.res,
|
||||
'labs-requirements-test'
|
||||
)
|
||||
|
||||
expect(ctx.LocalsHelper.setSplitTestInfo).to.have.been.calledWith(
|
||||
ctx.res.locals,
|
||||
'labs-requirements-test',
|
||||
sinon.match(info => info.labsDetails === undefined)
|
||||
)
|
||||
})
|
||||
|
||||
it('sets labsDetails when the user meets them', async function (ctx) {
|
||||
ctx.Modules.promises.hooks.fire.resolves([true])
|
||||
|
||||
await ctx.SplitTestHandler.promises.getAssignment(
|
||||
ctx.req,
|
||||
ctx.res,
|
||||
'labs-requirements-test'
|
||||
)
|
||||
|
||||
expect(ctx.LocalsHelper.setSplitTestInfo).to.have.been.calledWith(
|
||||
ctx.res.locals,
|
||||
'labs-requirements-test',
|
||||
sinon.match({
|
||||
labsDetails: sinon.match({ title: 'Needs premium compiles' }),
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
// fail closed rather than advertise an experiment the opt-in endpoint
|
||||
// would then refuse
|
||||
it('does not set labsDetails when the check fails', async function (ctx) {
|
||||
ctx.Modules.promises.hooks.fire.rejects(new Error('mongo is down'))
|
||||
|
||||
await ctx.SplitTestHandler.promises.getAssignment(
|
||||
ctx.req,
|
||||
ctx.res,
|
||||
'labs-requirements-test'
|
||||
)
|
||||
|
||||
expect(ctx.LocalsHelper.setSplitTestInfo).to.have.been.calledWith(
|
||||
ctx.res.locals,
|
||||
'labs-requirements-test',
|
||||
sinon.match(info => info.labsDetails === undefined)
|
||||
)
|
||||
})
|
||||
|
||||
it('does not ask about a release-phase split test', async function (ctx) {
|
||||
await ctx.SplitTestHandler.promises.getAssignment(
|
||||
ctx.req,
|
||||
ctx.res,
|
||||
'active-test'
|
||||
)
|
||||
|
||||
expect(ctx.Modules.promises.hooks.fire).to.not.have.been.called
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('labs phase assignment (gradual rollout)', function () {
|
||||
|
||||
@@ -150,4 +150,26 @@ describe('FeaturesHelper', function () {
|
||||
expect(result3).to.be.true
|
||||
})
|
||||
})
|
||||
|
||||
describe('hasPremiumCompiles', function () {
|
||||
it('should be true for premium compile groups', function (ctx) {
|
||||
expect(
|
||||
ctx.FeaturesHelper.hasPremiumCompiles({ compileGroup: 'priority' })
|
||||
).to.be.true
|
||||
expect(ctx.FeaturesHelper.hasPremiumCompiles({ compileGroup: 'alpha' }))
|
||||
.to.be.true
|
||||
})
|
||||
|
||||
it('should be false for the standard compile group', function (ctx) {
|
||||
expect(
|
||||
ctx.FeaturesHelper.hasPremiumCompiles({ compileGroup: 'standard' })
|
||||
).to.be.false
|
||||
})
|
||||
|
||||
it('should be false when the compile group is missing', function (ctx) {
|
||||
expect(ctx.FeaturesHelper.hasPremiumCompiles({})).to.be.false
|
||||
expect(ctx.FeaturesHelper.hasPremiumCompiles(undefined)).to.be.false
|
||||
expect(ctx.FeaturesHelper.hasPremiumCompiles(null)).to.be.false
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user