From 425e7b1e5bda8251220fbbb280337a60a544cef6 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 12 Jan 2026 11:55:10 +0000 Subject: [PATCH] [web] enable mongo notablescan in CI (#29501) * [monorepo] record ERROR/FATAL log messages in junit report * [web] put SaaS specific code behind feature flag * [web] use split test cache for getting user assignments The unit tests needed updating as they did not replicate any of the mongo filtering. The acceptance tests cover this logic. * [web] make better use of existing indexes * [web] avoid col-scan in tests of notifications module * [web] remove cleanup of empty feedbacks collection * [web] add assertion for reason of rejected request in launchpad test * [web] add missing indexes * [web] enable mongo notablescan * [web] make emailNotifications tests compatible with notablescan GitOrigin-RevId: b888f2feeb3a0e915f068ae1c4ea23ec17821221 --- .../test/mocha-multi-reporters.cjs | 1 + .../test/mocha-multi-reporters.cjs | 1 + libraries/logger/test-log-recorder.js | 20 +++++++++++ .../logger/test/mocha-multi-reporters.cjs | 1 + .../metrics/test/mocha-multi-reporters.cjs | 1 + .../o-error/test/mocha-multi-reporters.cjs | 1 + .../test/mocha-multi-reporters.cjs | 1 + .../test/mocha-multi-reporters.cjs | 1 + .../test/mocha-multi-reporters.cjs | 1 + .../test/mocha-multi-reporters.cjs | 1 + .../test/mocha-multi-reporters.cjs | 1 + .../test/mocha-multi-reporters.cjs | 1 + .../test/acceptance/js/helpers/ChatApp.js | 5 +++ services/chat/test/mocha-multi-reporters.cjs | 1 + services/clsi/test/mocha-multi-reporters.cjs | 1 + .../contacts/test/mocha-multi-reporters.cjs | 1 + .../docstore/test/mocha-multi-reporters.cjs | 1 + .../test/mocha-multi-reporters.cjs | 1 + .../filestore/test/mocha-multi-reporters.cjs | 1 + .../history-v1/test/mocha-multi-reporters.cjs | 1 + services/history-v1/test/setup.js | 2 ++ .../test/mocha-multi-reporters.cjs | 1 + .../real-time/test/mocha-multi-reporters.cjs | 1 + .../Institutions/InstitutionsFeatures.mjs | 5 +++ .../Features/SplitTests/SplitTestHandler.mjs | 7 ++-- .../Subscription/SubscriptionLocator.mjs | 18 +++++++++- .../src/Features/User/UserAuditLogHandler.mjs | 6 +++- .../web/app/src/Features/User/UserDeleter.mjs | 27 ++++++++------- .../UserMembership/UserMembershipsHandler.mjs | 2 ++ .../web/app/src/infrastructure/Features.mjs | 7 ++-- services/web/app/src/models/Feedback.mjs | 23 ------------- services/web/docker-compose.ci.yml | 2 +- services/web/docker-compose.yml | 2 +- .../test/acceptance/src/LaunchpadTests.mjs | 3 ++ .../web/test/acceptance/src/DeletionTests.mjs | 18 ++++++---- .../web/test/acceptance/src/ModelTests.mjs | 5 +++ .../web/test/acceptance/src/MongoHelper.mjs | 7 ++-- .../acceptance/src/PrimaryEmailCheckTests.mjs | 4 +++ .../src/UserMembershipAuthorizationTests.mjs | 9 +++++ .../test/acceptance/src/helpers/InitApp.mjs | 5 +++ services/web/test/mocha-multi-reporters.js | 1 + .../InstitutionsFeatures.test.mjs | 1 + .../src/SplitTests/SplitTestHandler.test.mjs | 5 --- .../test/unit/src/User/UserDeleter.test.mjs | 15 -------- ...03164405_institutions_managerIds_index.mjs | 34 +++++++++++++++++++ ...1103164405_publishers_managerIds_index.mjs | 34 +++++++++++++++++++ ...03164405_subscriptions_ssoConfig_index.mjs | 29 ++++++++++++++++ .../20251103164405_users_isAdmin_index.mjs | 29 ++++++++++++++++ 48 files changed, 268 insertions(+), 77 deletions(-) create mode 100644 libraries/logger/test-log-recorder.js delete mode 100644 services/web/app/src/models/Feedback.mjs create mode 100644 tools/migrations/20251103164405_institutions_managerIds_index.mjs create mode 100644 tools/migrations/20251103164405_publishers_managerIds_index.mjs create mode 100644 tools/migrations/20251103164405_subscriptions_ssoConfig_index.mjs create mode 100644 tools/migrations/20251103164405_users_isAdmin_index.mjs diff --git a/libraries/access-token-encryptor/test/mocha-multi-reporters.cjs b/libraries/access-token-encryptor/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/access-token-encryptor/test/mocha-multi-reporters.cjs +++ b/libraries/access-token-encryptor/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/fetch-utils/test/mocha-multi-reporters.cjs b/libraries/fetch-utils/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/fetch-utils/test/mocha-multi-reporters.cjs +++ b/libraries/fetch-utils/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/logger/test-log-recorder.js b/libraries/logger/test-log-recorder.js new file mode 100644 index 0000000000..8dbe11488e --- /dev/null +++ b/libraries/logger/test-log-recorder.js @@ -0,0 +1,20 @@ +const logger = require('./') +const bunyan = require('bunyan') +const serializers = require('./serializers') + +function testLogRecorder() { + const currentTest = this.currentTest + for (const level of ['error', 'fatal']) { + logger[level] = (info, msg) => { + const entry = { level, ...info, msg } + for (const [name, fn] of Object.entries(serializers)) { + if (name in entry) entry[name] = fn(entry[name]) + } + currentTest.consoleErrors = (currentTest.consoleErrors || []).concat( + JSON.stringify(info, bunyan.safeCycles()) + ) + } + } +} + +module.exports = testLogRecorder diff --git a/libraries/logger/test/mocha-multi-reporters.cjs b/libraries/logger/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/logger/test/mocha-multi-reporters.cjs +++ b/libraries/logger/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/metrics/test/mocha-multi-reporters.cjs b/libraries/metrics/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/metrics/test/mocha-multi-reporters.cjs +++ b/libraries/metrics/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/o-error/test/mocha-multi-reporters.cjs b/libraries/o-error/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/o-error/test/mocha-multi-reporters.cjs +++ b/libraries/o-error/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/object-persistor/test/mocha-multi-reporters.cjs b/libraries/object-persistor/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/object-persistor/test/mocha-multi-reporters.cjs +++ b/libraries/object-persistor/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/overleaf-editor-core/test/mocha-multi-reporters.cjs b/libraries/overleaf-editor-core/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/overleaf-editor-core/test/mocha-multi-reporters.cjs +++ b/libraries/overleaf-editor-core/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/promise-utils/test/mocha-multi-reporters.cjs b/libraries/promise-utils/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/promise-utils/test/mocha-multi-reporters.cjs +++ b/libraries/promise-utils/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/ranges-tracker/test/mocha-multi-reporters.cjs b/libraries/ranges-tracker/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/ranges-tracker/test/mocha-multi-reporters.cjs +++ b/libraries/ranges-tracker/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/redis-wrapper/test/mocha-multi-reporters.cjs b/libraries/redis-wrapper/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/redis-wrapper/test/mocha-multi-reporters.cjs +++ b/libraries/redis-wrapper/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/libraries/stream-utils/test/mocha-multi-reporters.cjs b/libraries/stream-utils/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/libraries/stream-utils/test/mocha-multi-reporters.cjs +++ b/libraries/stream-utils/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/chat/test/acceptance/js/helpers/ChatApp.js b/services/chat/test/acceptance/js/helpers/ChatApp.js index 32e6a25711..c67090ae0f 100644 --- a/services/chat/test/acceptance/js/helpers/ChatApp.js +++ b/services/chat/test/acceptance/js/helpers/ChatApp.js @@ -1,6 +1,7 @@ import { createServer } from '../../../../app/js/server.js' import { promisify } from 'node:util' import './MongoHelper.js' +import testLogRecorder from '@overleaf/logger/test-log-recorder.js' export { db } from '../../../../app/js/mongodb.js' @@ -14,3 +15,7 @@ export async function ensureRunning() { } return serverPromise } + +if (process.env.CI === 'true') { + beforeEach('record error logs in junit', testLogRecorder) +} diff --git a/services/chat/test/mocha-multi-reporters.cjs b/services/chat/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/services/chat/test/mocha-multi-reporters.cjs +++ b/services/chat/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/clsi/test/mocha-multi-reporters.cjs b/services/clsi/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/services/clsi/test/mocha-multi-reporters.cjs +++ b/services/clsi/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/contacts/test/mocha-multi-reporters.cjs b/services/contacts/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/services/contacts/test/mocha-multi-reporters.cjs +++ b/services/contacts/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/docstore/test/mocha-multi-reporters.cjs b/services/docstore/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/services/docstore/test/mocha-multi-reporters.cjs +++ b/services/docstore/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/document-updater/test/mocha-multi-reporters.cjs b/services/document-updater/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/services/document-updater/test/mocha-multi-reporters.cjs +++ b/services/document-updater/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/filestore/test/mocha-multi-reporters.cjs b/services/filestore/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/services/filestore/test/mocha-multi-reporters.cjs +++ b/services/filestore/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/history-v1/test/mocha-multi-reporters.cjs b/services/history-v1/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/services/history-v1/test/mocha-multi-reporters.cjs +++ b/services/history-v1/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/history-v1/test/setup.js b/services/history-v1/test/setup.js index 42bfdadeba..7f0e411cd1 100644 --- a/services/history-v1/test/setup.js +++ b/services/history-v1/test/setup.js @@ -5,6 +5,7 @@ const fetch = require('node-fetch') const { knex, redis } = require('../storage') const { exec } = require('node:child_process') const { promisify } = require('node:util') +const testLogRecorder = require('@overleaf/logger/test-log-recorder') // ensure every ObjectId has the id string as a property for correct comparisons require('mongodb').ObjectId.cacheHexString = true @@ -56,5 +57,6 @@ module.exports = { mochaHooks: { beforeAll: [setupPostgresDatabase, setupMongoDatabase, createGcsBuckets], afterAll: [tearDownConnectionPool], + beforeEach: process.env.CI === 'true' ? [testLogRecorder] : [], }, } diff --git a/services/project-history/test/mocha-multi-reporters.cjs b/services/project-history/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/services/project-history/test/mocha-multi-reporters.cjs +++ b/services/project-history/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/real-time/test/mocha-multi-reporters.cjs b/services/real-time/test/mocha-multi-reporters.cjs index 69342c7bde..674ff0e1cd 100644 --- a/services/real-time/test/mocha-multi-reporters.cjs +++ b/services/real-time/test/mocha-multi-reporters.cjs @@ -4,5 +4,6 @@ module.exports = { mochaFile: `reports/junit-mocha-${process.env.MOCHA_GREP}.xml`, includePending: true, jenkinsMode: true, + output: true, }, } diff --git a/services/web/app/src/Features/Institutions/InstitutionsFeatures.mjs b/services/web/app/src/Features/Institutions/InstitutionsFeatures.mjs index b206606a43..7b9119a1de 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsFeatures.mjs +++ b/services/web/app/src/Features/Institutions/InstitutionsFeatures.mjs @@ -4,8 +4,10 @@ import PlansLocator from '../Subscription/PlansLocator.mjs' import Settings from '@overleaf/settings' import InstitutionsGetter from './InstitutionsGetter.mjs' import FeaturesHelper from '../Subscription/FeaturesHelper.mjs' +import Features from '../../infrastructure/Features.mjs' async function _getInstitutionsAddons(userId) { + if (!Features.hasFeature('saas')) return {} const affiliates = await InstitutionsGetter.promises.getCurrentAffiliations(userId) // currently only addOn available to institutions is assist/WF bundle, @@ -17,6 +19,7 @@ async function _getInstitutionsAddons(userId) { } async function getInstitutionsFeatures(userId) { + if (!Features.hasFeature('saas')) return {} const planCode = await getInstitutionsPlan(userId) const plan = planCode && PlansLocator.findLocalPlanInSettings(planCode) let features = plan && plan.features @@ -28,6 +31,7 @@ async function getInstitutionsFeatures(userId) { } async function getInstitutionsPlan(userId) { + if (!Features.hasFeature('saas')) return null if (await hasLicence(userId)) { return Settings.institutionPlanCode } @@ -35,6 +39,7 @@ async function getInstitutionsPlan(userId) { } async function hasLicence(userId) { + if (!Features.hasFeature('saas')) return false const emailsData = await UserGetter.promises.getUserFullEmails(userId) return emailsData.some(emailData => emailData.emailHasInstitutionLicence) } diff --git a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs index 1a9a296c83..6447f9d4e5 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs @@ -232,12 +232,11 @@ async function getActiveAssignmentsForUser( return {} } - const splitTests = await SplitTest.find({ - $where: 'this.versions[this.versions.length - 1].active', - ...(removeArchived && { archived: { $ne: true } }), - }).exec() + const splitTests = (await SplitTestCache.get('')).values() const assignments = {} for (const splitTest of splitTests) { + if (!splitTest.versions[splitTest.versions.length - 1].active) continue + if (removeArchived && splitTest.archived) continue const { activeForUser, selectedVariantName, phase, versionNumber } = await _getAssignmentMetadata(user.analyticsId, user, splitTest) if (activeForUser) { diff --git a/services/web/app/src/Features/Subscription/SubscriptionLocator.mjs b/services/web/app/src/Features/Subscription/SubscriptionLocator.mjs index f5360281ad..172f5ea74a 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionLocator.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionLocator.mjs @@ -10,9 +10,11 @@ import { DeletedSubscription } from '../../models/DeletedSubscription.mjs' import logger from '@overleaf/logger' import { AI_ADD_ON_CODE, isStandaloneAiAddOnPlanCode } from './AiHelper.mjs' import './GroupPlansData.mjs' // make sure dynamic group plans are loaded +import Features from '../../infrastructure/Features.mjs' const SubscriptionLocator = { async getUsersSubscription(userOrId) { + if (!Features.hasFeature('saas')) return undefined const userId = SubscriptionLocator._getUserId(userOrId) const subscription = await Subscription.findOne({ admin_id: userId }).exec() logger.debug({ userId }, 'got users subscription') @@ -25,6 +27,7 @@ const SubscriptionLocator = { }, async getUserIndividualSubscription(userOrId) { + if (!Features.hasFeature('saas')) return undefined const userId = SubscriptionLocator._getUserId(userOrId) const subscription = await Subscription.findOne({ admin_id: userId, @@ -35,6 +38,7 @@ const SubscriptionLocator = { }, async getManagedGroupSubscriptions(userOrId) { + if (!Features.hasFeature('saas')) return [] return await Subscription.find({ manager_ids: userOrId, groupPlan: true, @@ -44,6 +48,7 @@ const SubscriptionLocator = { }, async getMemberSubscriptions(userOrId, populate = []) { + if (!Features.hasFeature('saas')) return [] const userId = SubscriptionLocator._getUserId(userOrId) // eslint-disable-next-line no-restricted-syntax return await Subscription.find({ member_ids: userId }) @@ -53,6 +58,7 @@ const SubscriptionLocator = { }, async getAdminEmail(subscriptionId) { + if (!Features.hasFeature('saas')) return undefined const subscription = await Subscription.findById(subscriptionId) .populate('admin_id', 'email') .exec() @@ -61,6 +67,7 @@ const SubscriptionLocator = { }, async getAdminEmailAndName(subscriptionId) { + if (!Features.hasFeature('saas')) return undefined const subscription = await Subscription.findById(subscriptionId) .populate('admin_id', ['email', 'first_name', 'last_name']) .exec() @@ -69,6 +76,7 @@ const SubscriptionLocator = { }, async hasRecurlyGroupSubscription(userOrId) { + if (!Features.hasFeature('saas')) return false const userId = SubscriptionLocator._getUserId(userOrId) return await Subscription.exists({ groupPlan: true, @@ -93,6 +101,7 @@ const SubscriptionLocator = { }, async getGroupSubscriptionsMemberOf(userId) { + if (!Features.hasFeature('saas')) return [] return await Subscription.find( { member_ids: userId }, { _id: 1, planCode: 1, userFeaturesDisabled: 1 } @@ -100,6 +109,7 @@ const SubscriptionLocator = { }, async getUniqueManagedSubscriptionMemberOf(userId) { + if (!Features.hasFeature('saas')) return null return await Subscription.findOne( { member_ids: userId, managedUsersEnabled: true }, { _id: 1 } @@ -125,8 +135,13 @@ const SubscriptionLocator = { }, async getGroupsWithTeamInvitesEmail(email) { + if (!Features.hasFeature('saas')) return [] return await Subscription.find( - { teamInvites: { $elemMatch: { email } } }, + { + teamInvites: { $elemMatch: { email } }, + // Add partialFilterExpression from index. + 'teamInvites.email': { $exists: true }, + }, { teamInvites: 1 } ).exec() }, @@ -136,6 +151,7 @@ const SubscriptionLocator = { }, async getUserDeletedSubscriptions(userId) { + if (!Features.hasFeature('saas')) return [] return await DeletedSubscription.find({ 'subscription.admin_id': userId, }).exec() diff --git a/services/web/app/src/Features/User/UserAuditLogHandler.mjs b/services/web/app/src/Features/User/UserAuditLogHandler.mjs index d72fd43fc1..9918792513 100644 --- a/services/web/app/src/Features/User/UserAuditLogHandler.mjs +++ b/services/web/app/src/Features/User/UserAuditLogHandler.mjs @@ -3,6 +3,7 @@ import logger from '@overleaf/logger' import { UserAuditLogEntry } from '../../models/UserAuditLogEntry.mjs' import { callbackify } from 'node:util' import SubscriptionLocator from '../Subscription/SubscriptionLocator.mjs' +import Features from '../../infrastructure/Features.mjs' function _canHaveNoIpAddressId(operation, info) { if (operation === 'add-email' && info.script) return true @@ -87,7 +88,10 @@ async function addEntry(userId, operation, initiatorId, ipAddress, info = {}) { ipAddress, } - if (MANAGED_GROUP_USER_EVENTS.includes(operation)) { + if ( + MANAGED_GROUP_USER_EVENTS.includes(operation) && + Features.hasFeature('saas') + ) { try { const managedSubscription = await SubscriptionLocator.promises.getUniqueManagedSubscriptionMemberOf( diff --git a/services/web/app/src/Features/User/UserDeleter.mjs b/services/web/app/src/Features/User/UserDeleter.mjs index 79eafa10df..d2a47e745c 100644 --- a/services/web/app/src/Features/User/UserDeleter.mjs +++ b/services/web/app/src/Features/User/UserDeleter.mjs @@ -4,7 +4,6 @@ import Settings from '@overleaf/settings' import { User } from '../../models/User.mjs' import { DeletedUser } from '../../models/DeletedUser.mjs' import { UserAuditLogEntry } from '../../models/UserAuditLogEntry.mjs' -import { Feedback } from '../../models/Feedback.mjs' import NewsletterManager from '../Newsletter/NewsletterManager.mjs' import ProjectDeleter from '../Project/ProjectDeleter.mjs' import SubscriptionHandler from '../Subscription/SubscriptionHandler.mjs' @@ -18,6 +17,7 @@ import Modules from '../../infrastructure/Modules.mjs' import Errors from '../Errors/Errors.js' import OnboardingDataCollectionManager from '../OnboardingDataCollection/OnboardingDataCollectionManager.mjs' import EmailHandler from '../Email/EmailHandler.mjs' +import Features from '../../infrastructure/Features.mjs' export default { deleteUser: callbackify(deleteUser), @@ -96,8 +96,6 @@ async function expireDeletedUser(userId) { try { logger.info({ userId }, 'firing expireDeletedUser hook') await Modules.promises.hooks.fire('expireDeletedUser', userId) - logger.info({ userId }, 'removing deleted user feedback records') - await Feedback.deleteMany({ userId }).exec() logger.info({ userId }, 'removing deleted user onboarding data') await OnboardingDataCollectionManager.deleteOnboardingDataCollection(userId) logger.info({ userId }, 'redacting PII from the deleted user record') @@ -157,6 +155,7 @@ async function expireDeletedUsersAfterDuration() { } async function ensureCanDeleteUser(user) { + if (!Features.hasFeature('saas')) return const subscription = await SubscriptionLocator.promises.getUsersSubscription(user) if (subscription) { @@ -212,16 +211,18 @@ async function _cleanupUser(user) { logger.info({ userId }, '[cleanupUser] removing user sessions from Redis') await UserSessionsManager.promises.removeSessionsFromRedis(user) - logger.info({ userId }, '[cleanupUser] unsubscribing from newsletters') - await NewsletterManager.promises.unsubscribe(user, { delete: true }) - logger.info({ userId }, '[cleanupUser] cancelling subscription') - await SubscriptionHandler.promises.cancelSubscription(user) - logger.info({ userId }, '[cleanupUser] deleting affiliations') - await InstitutionsAPI.promises.deleteAffiliations(userId) - logger.info({ userId }, '[cleanupUser] removing user from groups') - await SubscriptionUpdater.promises.removeUserFromAllGroups(userId) - logger.info({ userId }, '[cleanupUser] removing user from memberships') - await UserMembershipsHandler.promises.removeUserFromAllEntities(userId) + if (Features.hasFeature('saas')) { + logger.info({ userId }, '[cleanupUser] unsubscribing from newsletters') + await NewsletterManager.promises.unsubscribe(user, { delete: true }) + logger.info({ userId }, '[cleanupUser] cancelling subscription') + await SubscriptionHandler.promises.cancelSubscription(user) + logger.info({ userId }, '[cleanupUser] deleting affiliations') + await InstitutionsAPI.promises.deleteAffiliations(userId) + logger.info({ userId }, '[cleanupUser] removing user from groups') + await SubscriptionUpdater.promises.removeUserFromAllGroups(userId) + logger.info({ userId }, '[cleanupUser] removing user from memberships') + await UserMembershipsHandler.promises.removeUserFromAllEntities(userId) + } logger.info({ userId }, '[cleanupUser] removing personal access tokens') await Modules.promises.hooks.fire('cleanupPersonalAccessTokens', userId, [ 'collabratec', diff --git a/services/web/app/src/Features/UserMembership/UserMembershipsHandler.mjs b/services/web/app/src/Features/UserMembership/UserMembershipsHandler.mjs index fc1c6d0b8e..8d9643bd7e 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipsHandler.mjs +++ b/services/web/app/src/Features/UserMembership/UserMembershipsHandler.mjs @@ -16,6 +16,7 @@ import UserMembershipEntityConfigs from './UserMembershipEntityConfigs.mjs' import * as InstitutionModel from '../../models/Institution.mjs' import * as SubscriptionModel from '../../models/Subscription.mjs' import * as PublisherModel from '../../models/Publisher.mjs' +import Features from '../../infrastructure/Features.mjs' const EntityModels = { Institution: InstitutionModel.Institution, @@ -66,6 +67,7 @@ const UserMembershipsHandler = { if (callback == null) { callback = function () {} } + if (!Features.hasFeature('saas')) return callback(null, []) const query = Object.assign({}, entityConfig.baseQuery) query[entityConfig.fields.access] = userId EntityModels[entityConfig.modelName] diff --git a/services/web/app/src/infrastructure/Features.mjs b/services/web/app/src/infrastructure/Features.mjs index b6b9961962..6a398a60c1 100644 --- a/services/web/app/src/infrastructure/Features.mjs +++ b/services/web/app/src/infrastructure/Features.mjs @@ -1,13 +1,14 @@ import _ from 'lodash' import Settings from '@overleaf/settings' -const supportModuleAvailable = Settings.moduleImportSequence.includes('support') +const supportModuleAvailable = + Settings.moduleImportSequence?.includes('support') const symbolPaletteModuleAvailable = - Settings.moduleImportSequence.includes('symbol-palette') + Settings.moduleImportSequence?.includes('symbol-palette') const trackChangesModuleAvailable = - Settings.moduleImportSequence.includes('track-changes') + Settings.moduleImportSequence?.includes('track-changes') /** * @typedef {Object} Settings diff --git a/services/web/app/src/models/Feedback.mjs b/services/web/app/src/models/Feedback.mjs deleted file mode 100644 index be4e9bf7f2..0000000000 --- a/services/web/app/src/models/Feedback.mjs +++ /dev/null @@ -1,23 +0,0 @@ -import mongoose from '../infrastructure/Mongoose.mjs' -const { Schema } = mongoose -const { ObjectId } = Schema - -export const FeedbackSchema = new Schema( - { - userId: { - type: ObjectId, - ref: 'User', - }, - source: String, - createdAt: { - type: Date, - default() { - return new Date() - }, - }, - data: {}, - }, - { minimize: false } -) - -export const Feedback = mongoose.model('Feedback', FeedbackSchema) diff --git a/services/web/docker-compose.ci.yml b/services/web/docker-compose.ci.yml index a7976b2fd4..3882d2cbf2 100644 --- a/services/web/docker-compose.ci.yml +++ b/services/web/docker-compose.ci.yml @@ -130,7 +130,7 @@ services: mongo: image: mongo:8.0.11 - command: --replSet overleaf + command: --replSet overleaf --notablescan volumes: - ../../bin/shared/mongodb-init-replica-set.js:/docker-entrypoint-initdb.d/mongodb-init-replica-set.js - ../../bin/shared/mongodb-docker-entrypoint-wait.sh:/mongodb-docker-entrypoint-wait.sh diff --git a/services/web/docker-compose.yml b/services/web/docker-compose.yml index d766b9113f..0bd1032a8e 100644 --- a/services/web/docker-compose.yml +++ b/services/web/docker-compose.yml @@ -124,7 +124,7 @@ services: mongo: image: mongo:8.0.11 - command: --replSet overleaf + command: --replSet overleaf --notablescan volumes: - ../../bin/shared/mongodb-init-replica-set.js:/docker-entrypoint-initdb.d/mongodb-init-replica-set.js - ../../bin/shared/mongodb-docker-entrypoint-wait.sh:/mongodb-docker-entrypoint-wait.sh diff --git a/services/web/modules/launchpad/test/acceptance/src/LaunchpadTests.mjs b/services/web/modules/launchpad/test/acceptance/src/LaunchpadTests.mjs index e04d02cd80..9900338983 100644 --- a/services/web/modules/launchpad/test/acceptance/src/LaunchpadTests.mjs +++ b/services/web/modules/launchpad/test/acceptance/src/LaunchpadTests.mjs @@ -67,6 +67,9 @@ describe('Launchpad', function () { }), }) expect(badPostResponse.status).to.equal(403) + expect(await badPostResponse.json()).to.deep.equal({ + message: { type: 'error', text: 'admin user already exists' }, + }) // Log in as this new admin user const adminUser = await UserHelper.loginUser({ diff --git a/services/web/test/acceptance/src/DeletionTests.mjs b/services/web/test/acceptance/src/DeletionTests.mjs index b693c9b96d..da4b832818 100644 --- a/services/web/test/acceptance/src/DeletionTests.mjs +++ b/services/web/test/acceptance/src/DeletionTests.mjs @@ -45,12 +45,16 @@ describe('Deleting a user', function () { 'user', 'login', (results, cb) => { - const subscription = new Subscription({ - admin_id: results.user._id, - }) - subscription.ensureExists(err => { - cb(err, subscription) - }) + if (Features.hasFeature('saas')) { + const subscription = new Subscription({ + admin_id: results.user._id, + }) + subscription.ensureExists(err => { + cb(err, subscription) + }) + } else { + cb() + } }, ], }, @@ -88,7 +92,7 @@ describe('Deleting a user', function () { this.user.deleteUser(error => { expect(error).not.to.exist db.deletedUsers.findOne( - { 'user._id': user._id }, + { 'user.email': user.email }, (error, deletedUser) => { expect(error).not.to.exist expect(deletedUser).to.exist diff --git a/services/web/test/acceptance/src/ModelTests.mjs b/services/web/test/acceptance/src/ModelTests.mjs index 9ef32cf08c..651a55f320 100644 --- a/services/web/test/acceptance/src/ModelTests.mjs +++ b/services/web/test/acceptance/src/ModelTests.mjs @@ -1,6 +1,7 @@ import { expect } from 'chai' import { User } from '../../../app/src/models/User.mjs' import { Subscription } from '../../../app/src/models/Subscription.mjs' +import Features from '../../../app/src/infrastructure/Features.mjs' describe('mongoose', function () { describe('User', function () { @@ -50,6 +51,10 @@ describe('mongoose', function () { let user beforeEach(async function () { + if (!Features.hasFeature('saas')) { + this.skip() + } + user = await User.create({ email: 'wombat@potato.net' }) }) diff --git a/services/web/test/acceptance/src/MongoHelper.mjs b/services/web/test/acceptance/src/MongoHelper.mjs index ca22cecc3e..541fb7c569 100644 --- a/services/web/test/acceptance/src/MongoHelper.mjs +++ b/services/web/test/acceptance/src/MongoHelper.mjs @@ -163,9 +163,10 @@ describe('MongoTests', function () { }) describe('with an object as query', function () { + const signUpDate = new Date('2025-11-03T16:38:17.320Z') beforeEach(async function addHiddenFlag() { // add a mongo field that does not exist on the other users - await ghost.mongoUpdate({ $set: { hidden: 1 } }) + await ghost.mongoUpdate({ $set: { signUpDate } }) }) it('should pass through the query', function () { @@ -176,7 +177,7 @@ describe('MongoTests', function () { describe('when searching for hidden users', function () { it('should match the ghost only', async function () { - const query = normalizeMultiQuery({ hidden: 1 }) + const query = normalizeMultiQuery({ signUpDate }) const users = await db.users.find(query).toArray() expect(users).to.have.length(1) @@ -186,7 +187,7 @@ describe('MongoTests', function () { describe('when searching for non hidden users', function () { it('should find the three users', async function () { - const query = normalizeMultiQuery({ hidden: { $exists: false } }) + const query = normalizeMultiQuery({ signUpDate: { $ne: signUpDate } }) await expectToFindTheThreeUsers(query) }) diff --git a/services/web/test/acceptance/src/PrimaryEmailCheckTests.mjs b/services/web/test/acceptance/src/PrimaryEmailCheckTests.mjs index 16fcdf5ebb..c2f15014ba 100644 --- a/services/web/test/acceptance/src/PrimaryEmailCheckTests.mjs +++ b/services/web/test/acceptance/src/PrimaryEmailCheckTests.mjs @@ -265,6 +265,10 @@ describe('PrimaryEmailCheck', function () { describe('when the user is a managed user', function () { beforeEach(async function () { + if (!Features.hasFeature('saas')) { + this.skip() + } + const adminUser = await UserHelper.createUser() this.subscription = new Subscription({ adminId: adminUser._id, diff --git a/services/web/test/acceptance/src/UserMembershipAuthorizationTests.mjs b/services/web/test/acceptance/src/UserMembershipAuthorizationTests.mjs index 994cfbe455..1a4a59c538 100644 --- a/services/web/test/acceptance/src/UserMembershipAuthorizationTests.mjs +++ b/services/web/test/acceptance/src/UserMembershipAuthorizationTests.mjs @@ -6,15 +6,24 @@ import Subscription from './helpers/Subscription.mjs' import Publisher from './helpers/Publisher.mjs' import sinon from 'sinon' import RecurlyClient from '../../../app/src/Features/Subscription/RecurlyClient.mjs' +import Features from '../../../app/src/infrastructure/Features.mjs' describe('UserMembershipAuthorization', function () { beforeEach(function (done) { + if (!Features.hasFeature('saas')) { + this.skip() + } + this.user = new User() sinon.stub(RecurlyClient.promises, 'getSubscription').resolves({}) async.series([this.user.ensureUserExists.bind(this.user)], done) }) afterEach(function () { + if (!Features.hasFeature('saas')) { + return + } + RecurlyClient.promises.getSubscription.restore() }) diff --git a/services/web/test/acceptance/src/helpers/InitApp.mjs b/services/web/test/acceptance/src/helpers/InitApp.mjs index a85e050c16..6bd954a4bb 100644 --- a/services/web/test/acceptance/src/helpers/InitApp.mjs +++ b/services/web/test/acceptance/src/helpers/InitApp.mjs @@ -10,6 +10,7 @@ import { injectRouteAfter } from './injectRoute.mjs' import SplitTestHandler from '../../../../app/src/Features/SplitTests/SplitTestHandler.mjs' import SplitTestSessionHandler from '../../../../app/src/Features/SplitTests/SplitTestSessionHandler.mjs' import Modules from '../../../../app/src/infrastructure/Modules.mjs' +import testLogRecorder from '@overleaf/logger/test-log-recorder.js' const app = Server.app @@ -114,3 +115,7 @@ after('stop main app', async function () { Settings.gracefulShutdownDelayInMs = 1 await gracefulShutdown(server, 'tests') }) + +if (process.env.CI === 'true') { + beforeEach('record error logs in junit', testLogRecorder) +} diff --git a/services/web/test/mocha-multi-reporters.js b/services/web/test/mocha-multi-reporters.js index a98ffd2403..5cd81e9513 100644 --- a/services/web/test/mocha-multi-reporters.js +++ b/services/web/test/mocha-multi-reporters.js @@ -6,5 +6,6 @@ module.exports = { jenkinsMode: true, jenkinsClassnamePrefix: process.env.MOCHA_ROOT_SUITE_NAME, rootSuiteTitle: process.env.MOCHA_ROOT_SUITE_NAME, + outputs: true, }, } diff --git a/services/web/test/unit/src/Institutions/InstitutionsFeatures.test.mjs b/services/web/test/unit/src/Institutions/InstitutionsFeatures.test.mjs index fd2339f4fa..abd608a476 100644 --- a/services/web/test/unit/src/Institutions/InstitutionsFeatures.test.mjs +++ b/services/web/test/unit/src/Institutions/InstitutionsFeatures.test.mjs @@ -29,6 +29,7 @@ describe('InstitutionsFeatures', function () { vi.doMock('@overleaf/settings', () => ({ default: { institutionPlanCode: ctx.institutionPlanCode, + overleaf: {}, }, })) diff --git a/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs b/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs index fcf4194378..9a48c4735a 100644 --- a/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs +++ b/services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs @@ -301,11 +301,6 @@ describe('SplitTestHandler', function () { variantName: 'variant-1', versionNumber: 2, }, - 'not-active-test': { - phase: 'release', - variantName: 'variant-1', - versionNumber: 1, - }, }) }) diff --git a/services/web/test/unit/src/User/UserDeleter.test.mjs b/services/web/test/unit/src/User/UserDeleter.test.mjs index 70b5b4e721..efda58c232 100644 --- a/services/web/test/unit/src/User/UserDeleter.test.mjs +++ b/services/web/test/unit/src/User/UserDeleter.test.mjs @@ -99,10 +99,6 @@ describe('UserDeleter', function () { promises: { hooks: { fire: sinon.stub().resolves() } }, } - ctx.Feedback = { - deleteMany: sinon.stub().returns({ exec: sinon.stub().resolves() }), - } - ctx.OnboardingDataCollectionManager = { deleteOnboardingDataCollection: sinon.stub().resolves(), } @@ -127,10 +123,6 @@ describe('UserDeleter', function () { DeletedUser, })) - vi.doMock('../../../../app/src/models/Feedback', () => ({ - Feedback: ctx.Feedback, - })) - vi.doMock( '../../../../app/src/Features/Newsletter/NewsletterManager', () => ({ @@ -788,13 +780,6 @@ describe('UserDeleter', function () { ) }) - it('should delete Feeback', async function (ctx) { - await ctx.UserDeleter.promises.expireDeletedUser(ctx.userId) - expect(ctx.Feedback.deleteMany).to.have.been.calledWith({ - userId: ctx.userId, - }) - }) - describe('when called as a callback', function () { it('should expire the user', async function (ctx) { await new Promise(resolve => { diff --git a/tools/migrations/20251103164405_institutions_managerIds_index.mjs b/tools/migrations/20251103164405_institutions_managerIds_index.mjs new file mode 100644 index 0000000000..e249841446 --- /dev/null +++ b/tools/migrations/20251103164405_institutions_managerIds_index.mjs @@ -0,0 +1,34 @@ +import Helpers from './lib/helpers.mjs' + +const tags = ['saas'] + +const indexes = [ + { + key: { + managerIds: 1, + }, + name: 'managerIds_1', + }, +] + +const migrate = async client => { + const { db } = client + + await Helpers.addIndexesToCollection(db.institutions, indexes) +} + +const rollback = async client => { + const { db } = client + + try { + await Helpers.dropIndexesFromCollection(db.institutions, indexes) + } catch (err) { + console.error('Something went wrong rolling back the migrations', err) + } +} + +export default { + tags, + migrate, + rollback, +} diff --git a/tools/migrations/20251103164405_publishers_managerIds_index.mjs b/tools/migrations/20251103164405_publishers_managerIds_index.mjs new file mode 100644 index 0000000000..68e8008261 --- /dev/null +++ b/tools/migrations/20251103164405_publishers_managerIds_index.mjs @@ -0,0 +1,34 @@ +import Helpers from './lib/helpers.mjs' + +const tags = ['saas'] + +const indexes = [ + { + key: { + managerIds: 1, + }, + name: 'managerIds_1', + }, +] + +const migrate = async client => { + const { db } = client + + await Helpers.addIndexesToCollection(db.publishers, indexes) +} + +const rollback = async client => { + const { db } = client + + try { + await Helpers.dropIndexesFromCollection(db.publishers, indexes) + } catch (err) { + console.error('Something went wrong rolling back the migrations', err) + } +} + +export default { + tags, + migrate, + rollback, +} diff --git a/tools/migrations/20251103164405_subscriptions_ssoConfig_index.mjs b/tools/migrations/20251103164405_subscriptions_ssoConfig_index.mjs new file mode 100644 index 0000000000..945bd776dc --- /dev/null +++ b/tools/migrations/20251103164405_subscriptions_ssoConfig_index.mjs @@ -0,0 +1,29 @@ +import Helpers from './lib/helpers.mjs' + +const tags = ['saas'] + +const indexes = [ + { + name: 'ssoConfig_1', + key: { ssoConfig: 1 }, + partialFilterExpression: { + ssoConfig: { $exists: true }, + }, + }, +] + +const migrate = async client => { + const { db } = client + await Helpers.addIndexesToCollection(db.subscriptions, indexes) +} + +const rollback = async client => { + const { db } = client + await Helpers.dropIndexesFromCollection(db.subscriptions, indexes) +} + +export default { + tags, + migrate, + rollback, +} diff --git a/tools/migrations/20251103164405_users_isAdmin_index.mjs b/tools/migrations/20251103164405_users_isAdmin_index.mjs new file mode 100644 index 0000000000..4304452e53 --- /dev/null +++ b/tools/migrations/20251103164405_users_isAdmin_index.mjs @@ -0,0 +1,29 @@ +import Helpers from './lib/helpers.mjs' + +const tags = ['server-ce', 'server-pro'] + +const indexes = [ + { + name: 'isAdmin_1', + key: { isAdmin: 1 }, + partialFilterExpression: { + isAdmin: true, + }, + }, +] + +const migrate = async client => { + const { db } = client + await Helpers.addIndexesToCollection(db.users, indexes) +} + +const rollback = async client => { + const { db } = client + await Helpers.dropIndexesFromCollection(db.users, indexes) +} + +export default { + tags, + migrate, + rollback, +}