[web] cleanup archived split-test assignments from user record on login (#33365)
* [web] cleanup archived split-test assignments from user record on login Co-authored-by: Anna Claire Fields <anna.fields@overleaf.com> * [migrations] purge archived split tests from all users Co-authored-by: Anna Claire Fields <anna.fields@overleaf.com> * [web] add missing mock and update snapshot test * [web] gracefully access db.users.splitTests --------- Co-authored-by: Anna Claire Fields <anna.fields@overleaf.com> GitOrigin-RevId: bd185074a402556d7b7c812208cf834dd52b27a5
This commit is contained in:
@@ -25,6 +25,7 @@ import Modules from '../../infrastructure/Modules.mjs'
|
|||||||
import { expressify, promisify } from '@overleaf/promise-utils'
|
import { expressify, promisify } from '@overleaf/promise-utils'
|
||||||
import { handleAuthenticateErrors } from './AuthenticationErrors.mjs'
|
import { handleAuthenticateErrors } from './AuthenticationErrors.mjs'
|
||||||
import EmailHelper from '../Helpers/EmailHelper.mjs'
|
import EmailHelper from '../Helpers/EmailHelper.mjs'
|
||||||
|
import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs'
|
||||||
|
|
||||||
const { hasAdminAccess } = AdminAuthorizationHelper
|
const { hasAdminAccess } = AdminAuthorizationHelper
|
||||||
|
|
||||||
@@ -647,6 +648,10 @@ function _loginAsyncHandlers(req, user, anonymousAnalyticsId, isNewUser) {
|
|||||||
UserHandler.promises.populateTeamInvites(user).catch(err => {
|
UserHandler.promises.populateTeamInvites(user).catch(err => {
|
||||||
logger.warn({ err }, 'error setting up login data')
|
logger.warn({ err }, 'error setting up login data')
|
||||||
})
|
})
|
||||||
|
SplitTestHandler.promises.userMaintenanceOnLogin(user).catch(err => {
|
||||||
|
const userId = user._id
|
||||||
|
logger.warn({ err, userId }, 'error cleaning up split-tests on login')
|
||||||
|
})
|
||||||
LoginRateLimiter.recordSuccessfulLogin(user.email, () => {})
|
LoginRateLimiter.recordSuccessfulLogin(user.email, () => {})
|
||||||
AuthenticationController._recordSuccessfulLogin(user._id, () => {})
|
AuthenticationController._recordSuccessfulLogin(user._id, () => {})
|
||||||
AuthenticationController.ipMatchCheck(req, user)
|
AuthenticationController.ipMatchCheck(req, user)
|
||||||
|
|||||||
@@ -978,6 +978,21 @@ async function decrementLabsVariantCounter(splitTestName) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function userMaintenanceOnLogin(user) {
|
||||||
|
const splitTests = (await SplitTestCache.get('')).values()
|
||||||
|
const toCleanup = {}
|
||||||
|
for (const splitTest of splitTests) {
|
||||||
|
if (splitTest.archived && user.splitTests?.[splitTest.name]) {
|
||||||
|
toCleanup[`splitTests.${splitTest.name}`] = 1
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (Object.keys(toCleanup).length > 0) {
|
||||||
|
await UserUpdater.promises.updateUser(user._id, {
|
||||||
|
$unset: toCleanup,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export default {
|
export default {
|
||||||
getPercentile,
|
getPercentile,
|
||||||
getAssignment: callbackify(getAssignment),
|
getAssignment: callbackify(getAssignment),
|
||||||
@@ -997,5 +1012,6 @@ export default {
|
|||||||
hasUserBeenAssignedToVariant,
|
hasUserBeenAssignedToVariant,
|
||||||
decrementLabsVariantCounter,
|
decrementLabsVariantCounter,
|
||||||
incrementLabsVariantCounterIfBelowLimit,
|
incrementLabsVariantCounterIfBelowLimit,
|
||||||
|
userMaintenanceOnLogin,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,121 @@
|
|||||||
|
import { expect } from 'chai'
|
||||||
|
import { db, ObjectId } from '../../../app/src/infrastructure/mongodb.mjs'
|
||||||
|
import { exec } from 'node:child_process'
|
||||||
|
|
||||||
|
describe('CleanupUsersSplitTestsMigration', function () {
|
||||||
|
beforeEach('insert data', async function () {
|
||||||
|
await db.splittests.insertMany([
|
||||||
|
{ name: 'archived-test', archived: true },
|
||||||
|
{ name: 'non-archived-test', archived: false },
|
||||||
|
])
|
||||||
|
await db.users.insertMany([
|
||||||
|
{
|
||||||
|
_id: new ObjectId('50e434d90000000000000000'),
|
||||||
|
email: 'foo0@bar.com',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
_id: new ObjectId('50e434d90000000000000001'),
|
||||||
|
email: 'foo1@bar.com',
|
||||||
|
splitTests: {
|
||||||
|
'archived-test': [
|
||||||
|
{
|
||||||
|
variantName: 'default',
|
||||||
|
versionNumber: 2,
|
||||||
|
phase: 'release',
|
||||||
|
assignedAt: new Date('2025-10-22T14:23:29.738Z'),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
_id: new ObjectId('50e434d90000000000000002'),
|
||||||
|
email: 'foo2@bar.com',
|
||||||
|
splitTests: {
|
||||||
|
'non-archived-test': [
|
||||||
|
{
|
||||||
|
variantName: 'default',
|
||||||
|
versionNumber: 2,
|
||||||
|
phase: 'release',
|
||||||
|
assignedAt: new Date('2025-10-22T14:23:29.738Z'),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
_id: new ObjectId('50e434d90000000000000003'),
|
||||||
|
email: 'foo3@bar.com',
|
||||||
|
splitTests: {
|
||||||
|
'non-archived-test': [
|
||||||
|
{
|
||||||
|
variantName: 'default',
|
||||||
|
versionNumber: 2,
|
||||||
|
phase: 'release',
|
||||||
|
assignedAt: new Date('2025-10-22T14:23:29.738Z'),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
'archived-test': [
|
||||||
|
{
|
||||||
|
variantName: 'default',
|
||||||
|
versionNumber: 2,
|
||||||
|
phase: 'release',
|
||||||
|
assignedAt: new Date('2025-10-22T14:23:29.738Z'),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
|
beforeEach('run migration', function (done) {
|
||||||
|
exec(
|
||||||
|
'cd ../../tools/migrations && yarn run migrations migrate -t saas --force 20260504100000_cleanup_users_split_tests',
|
||||||
|
done
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('should update the users', async function () {
|
||||||
|
expect(
|
||||||
|
await db.users
|
||||||
|
.find({}, { projection: { _id: 1, email: 1, splitTests: 1 } })
|
||||||
|
.toArray()
|
||||||
|
).to.deep.equal([
|
||||||
|
{
|
||||||
|
_id: new ObjectId('50e434d90000000000000000'),
|
||||||
|
email: 'foo0@bar.com',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
_id: new ObjectId('50e434d90000000000000001'),
|
||||||
|
email: 'foo1@bar.com',
|
||||||
|
splitTests: {},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
_id: new ObjectId('50e434d90000000000000002'),
|
||||||
|
email: 'foo2@bar.com',
|
||||||
|
splitTests: {
|
||||||
|
'non-archived-test': [
|
||||||
|
{
|
||||||
|
variantName: 'default',
|
||||||
|
versionNumber: 2,
|
||||||
|
phase: 'release',
|
||||||
|
assignedAt: new Date('2025-10-22T14:23:29.738Z'),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
_id: new ObjectId('50e434d90000000000000003'),
|
||||||
|
email: 'foo3@bar.com',
|
||||||
|
splitTests: {
|
||||||
|
'non-archived-test': [
|
||||||
|
{
|
||||||
|
variantName: 'default',
|
||||||
|
versionNumber: 2,
|
||||||
|
phase: 'release',
|
||||||
|
assignedAt: new Date('2025-10-22T14:23:29.738Z'),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
])
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -102,6 +102,17 @@ describe('AuthenticationController', function () {
|
|||||||
})
|
})
|
||||||
)
|
)
|
||||||
|
|
||||||
|
vi.doMock(
|
||||||
|
'../../../../app/src/Features/SplitTests/SplitTestHandler',
|
||||||
|
() => ({
|
||||||
|
default: (ctx.SplitTestHandler = {
|
||||||
|
promises: {
|
||||||
|
userMaintenanceOnLogin: sinon.stub().resolves(),
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
})
|
||||||
|
)
|
||||||
|
|
||||||
vi.doMock('../../../../app/src/Features/User/UserUpdater', () => ({
|
vi.doMock('../../../../app/src/Features/User/UserUpdater', () => ({
|
||||||
default: (ctx.UserUpdater = {
|
default: (ctx.UserUpdater = {
|
||||||
updateUser: sinon.stub(),
|
updateUser: sinon.stub(),
|
||||||
|
|||||||
@@ -0,0 +1,30 @@
|
|||||||
|
import { db } from './lib/mongodb.mjs'
|
||||||
|
import { batchedUpdate } from '@overleaf/mongo-utils/batchedUpdate.js'
|
||||||
|
|
||||||
|
const tags = ['saas', 'server-ce', 'server-pro']
|
||||||
|
|
||||||
|
const migrate = async () => {
|
||||||
|
const splitTests = await db.splittests.find().toArray()
|
||||||
|
const toCleanup = {}
|
||||||
|
for (const splitTest of splitTests) {
|
||||||
|
if (splitTest.archived) {
|
||||||
|
toCleanup[`splitTests.${splitTest.name}`] = 1
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Take the easy route and unset all the archived split tests on all the users with any split test assignment.
|
||||||
|
await batchedUpdate(
|
||||||
|
db.users,
|
||||||
|
{ splitTests: { $exists: true } },
|
||||||
|
{ $unset: toCleanup }
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
const rollback = async () => {
|
||||||
|
// The data is gone. Nothing to revert to.
|
||||||
|
}
|
||||||
|
|
||||||
|
export default {
|
||||||
|
tags,
|
||||||
|
migrate,
|
||||||
|
rollback,
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user