diff --git a/services/web/app/src/Features/BetaProgram/BetaProgramController.js b/services/web/app/src/Features/BetaProgram/BetaProgramController.js index 528d40062e..1b1e7e24d4 100644 --- a/services/web/app/src/Features/BetaProgram/BetaProgramController.js +++ b/services/web/app/src/Features/BetaProgram/BetaProgramController.js @@ -37,7 +37,7 @@ const BetaProgramController = { optInPage(req, res, next) { const userId = SessionManager.getLoggedInUserId(req.session) logger.debug({ userId }, 'showing beta participation page for user') - UserGetter.getUser(userId, function (err, user) { + UserGetter.getUser(userId, { betaProgram: 1 }, function (err, user) { if (err) { OError.tag(err, 'error fetching user', { userId, diff --git a/services/web/app/src/Features/Institutions/InstitutionsManager.js b/services/web/app/src/Features/Institutions/InstitutionsManager.js index ce1da66ebd..4e543123bb 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsManager.js +++ b/services/web/app/src/Features/Institutions/InstitutionsManager.js @@ -322,7 +322,7 @@ function refreshFeaturesAndNotify(affiliation, callback) { const getUserInfo = (userId, callback) => async.waterfall( [ - cb => UserGetter.getUser(userId, cb), + cb => UserGetter.getUser(userId, { _id: 1 }, cb), (user, cb) => SubscriptionLocator.getUsersSubscription(user, (err, subscription) => cb(err, user, subscription) diff --git a/services/web/app/src/Features/Subscription/ManagedUsersHandler.js b/services/web/app/src/Features/Subscription/ManagedUsersHandler.js index 4365b93975..4e6bfd1581 100644 --- a/services/web/app/src/Features/Subscription/ManagedUsersHandler.js +++ b/services/web/app/src/Features/Subscription/ManagedUsersHandler.js @@ -116,10 +116,13 @@ async function getEnrollmentForUser(requestedUser) { async function enrollInSubscription(userId, subscription) { // check whether the user is already enrolled in a subscription - const user = await User.findOne({ - _id: userId, - 'enrollment.managedBy': { $exists: true }, - }).exec() + const user = await User.findOne( + { + _id: userId, + 'enrollment.managedBy': { $exists: true }, + }, + { _id: 1 } + ).exec() if (user != null) { throw new OError('User is already enrolled in a subscription', { userId, diff --git a/services/web/app/src/Features/User/UserController.js b/services/web/app/src/Features/User/UserController.js index bc667070ec..bd93da530d 100644 --- a/services/web/app/src/Features/User/UserController.js +++ b/services/web/app/src/Features/User/UserController.js @@ -288,44 +288,52 @@ const UserController = { subscribe(req, res, next) { const userId = SessionManager.getLoggedInUserId(req.session) - UserGetter.getUser(userId, (err, user) => { - if (err != null) { - return next(err) - } - NewsletterManager.subscribe(user, err => { + UserGetter.getUser( + userId, + { _id: 1, email: 1, first_name: 1, last_name: 1 }, + (err, user) => { if (err != null) { - OError.tag(err, 'error subscribing to newsletter') return next(err) } - return res.json({ - message: req.i18n.translate('thanks_settings_updated'), - }) - }) - }) - }, - - unsubscribe(req, res, next) { - const userId = SessionManager.getLoggedInUserId(req.session) - UserGetter.getUser(userId, (err, user) => { - if (err != null) { - return next(err) - } - NewsletterManager.unsubscribe(user, err => { - if (err != null) { - OError.tag(err, 'error unsubscribing to newsletter') - return next(err) - } - Modules.hooks.fire('newsletterUnsubscribed', user, err => { - if (err) { - OError.tag(err, 'error firing "newsletterUnsubscribed" hook') + NewsletterManager.subscribe(user, err => { + if (err != null) { + OError.tag(err, 'error subscribing to newsletter') return next(err) } return res.json({ message: req.i18n.translate('thanks_settings_updated'), }) }) - }) - }) + } + ) + }, + + unsubscribe(req, res, next) { + const userId = SessionManager.getLoggedInUserId(req.session) + UserGetter.getUser( + userId, + { _id: 1, email: 1, first_name: 1, last_name: 1 }, + (err, user) => { + if (err != null) { + return next(err) + } + NewsletterManager.unsubscribe(user, err => { + if (err != null) { + OError.tag(err, 'error unsubscribing to newsletter') + return next(err) + } + Modules.hooks.fire('newsletterUnsubscribed', user, err => { + if (err) { + OError.tag(err, 'error firing "newsletterUnsubscribed" hook') + return next(err) + } + return res.json({ + message: req.i18n.translate('thanks_settings_updated'), + }) + }) + }) + } + ) }, updateUserSettings(req, res, next) { diff --git a/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js b/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js index f204482d54..b73344c76b 100644 --- a/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js +++ b/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js @@ -90,7 +90,7 @@ const UserEmailsConfirmationHandler = { if (!userId || email !== EmailHelper.parseEmail(email)) { return callback(new Errors.NotFoundError('invalid data')) } - UserGetter.getUser(userId, {}, function (error, user) { + UserGetter.getUser(userId, { emails: 1 }, function (error, user) { if (error) { return callback(error) } diff --git a/services/web/app/src/Features/User/UserGetter.js b/services/web/app/src/Features/User/UserGetter.js index de3ec8c8f1..805257720a 100644 --- a/services/web/app/src/Features/User/UserGetter.js +++ b/services/web/app/src/Features/User/UserGetter.js @@ -111,6 +111,14 @@ const UserGetter = { } }, + getUserFeatures(userId, callback) { + this.getUser(userId, { features: 1 }, (error, user) => { + if (error) return callback(error) + if (!user) return callback(new Errors.NotFoundError('user not found')) + callback(null, user.features) + }) + }, + getUserEmail(userId, callback) { this.getUser(userId, { email: 1 }, (error, user) => callback(error, user && user.email) diff --git a/services/web/app/src/Features/User/UserPagesController.js b/services/web/app/src/Features/User/UserPagesController.js index 7f96fe837a..28dbbdac87 100644 --- a/services/web/app/src/Features/User/UserPagesController.js +++ b/services/web/app/src/Features/User/UserPagesController.js @@ -223,21 +223,25 @@ const UserPagesController = { emailPreferencesPage(req, res, next) { const userId = SessionManager.getLoggedInUserId(req.session) - UserGetter.getUser(userId, (err, user) => { - if (err != null) { - return next(err) - } - NewsletterManager.subscribed(user, (err, subscribed) => { + UserGetter.getUser( + userId, + { _id: 1, email: 1, first_name: 1, last_name: 1 }, + (err, user) => { if (err != null) { - OError.tag(err, 'error getting newsletter subscription status') return next(err) } - res.render('user/email-preferences', { - title: 'newsletter_info_title', - subscribed, + NewsletterManager.subscribed(user, (err, subscribed) => { + if (err != null) { + OError.tag(err, 'error getting newsletter subscription status') + return next(err) + } + res.render('user/email-preferences', { + title: 'newsletter_info_title', + subscribed, + }) }) - }) - }) + } + ) }, _restructureThirdPartyIds(user) { diff --git a/services/web/modules/server-ce-scripts/scripts/delete-user.js b/services/web/modules/server-ce-scripts/scripts/delete-user.js index 5ec1065081..7abcc376c6 100644 --- a/services/web/modules/server-ce-scripts/scripts/delete-user.js +++ b/services/web/modules/server-ce-scripts/scripts/delete-user.js @@ -12,7 +12,7 @@ async function main() { } await new Promise((resolve, reject) => { - UserGetter.getUser({ email }, function (error, user) { + UserGetter.getUser({ email }, { _id: 1 }, function (error, user) { if (error) { return reject(error) } diff --git a/services/web/test/acceptance/src/ModelTests.js b/services/web/test/acceptance/src/ModelTests.js index 4e57e1585f..ac0c473fde 100644 --- a/services/web/test/acceptance/src/ModelTests.js +++ b/services/web/test/acceptance/src/ModelTests.js @@ -9,7 +9,7 @@ describe('mongoose', function () { it('allows the creation of a user', async function () { await expect(User.create({ email })).to.be.fulfilled - await expect(User.findOne({ email })).to.eventually.exist + await expect(User.findOne({ email }, { _id: 1 })).to.eventually.exist }) it('does not allow the creation of multiple users with the same email', async function () { @@ -41,7 +41,7 @@ describe('mongoose', function () { }) ).to.be.fulfilled - const user = await User.findOne({ email }) + const user = await User.findOne({ email }, { splitTests: 1 }) expect(user.splitTests['some-test'][0].assignedAt).to.be.a('date') expect(user.splitTests['some-test'][1].assignedAt).to.be.a('date') }) diff --git a/services/web/test/unit/src/BetaProgram/BetaProgramControllerTests.js b/services/web/test/unit/src/BetaProgram/BetaProgramControllerTests.js index 933d6d2200..a4332bdd36 100644 --- a/services/web/test/unit/src/BetaProgram/BetaProgramControllerTests.js +++ b/services/web/test/unit/src/BetaProgram/BetaProgramControllerTests.js @@ -127,7 +127,7 @@ describe('BetaProgramController', function () { describe('optInPage', function () { beforeEach(function () { - this.UserGetter.getUser.callsArgWith(1, null, this.user) + this.UserGetter.getUser.yields(null, this.user) }) it('should render the opt-in page', function () { @@ -139,7 +139,7 @@ describe('BetaProgramController', function () { describe('when UserGetter.getUser produces an error', function () { beforeEach(function () { - this.UserGetter.getUser.callsArgWith(1, new Error('woops')) + this.UserGetter.getUser.yields(new Error('woops')) }) it('should not render the opt-in page', function () { diff --git a/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js b/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js index 05928f8f23..46e34e4fe2 100644 --- a/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js +++ b/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js @@ -45,7 +45,7 @@ describe('InstitutionsManager', function () { this.UserGetter = { getUsersByAnyConfirmedEmail: sinon.stub().yields(), - getUser: sinon.stub().callsArgWith(1, null, this.user), + getUser: sinon.stub().yields(null, this.user), promises: { getUsers: sinon.stub().resolves(this.users), getUsersByAnyConfirmedEmail: sinon.stub().resolves(), @@ -164,18 +164,10 @@ describe('InstitutionsManager', function () { this.user3 = { _id: this.user3Id } this.user4 = { _id: this.user4Id } - this.UserGetter.getUser - .withArgs(this.user1Id) - .callsArgWith(1, null, this.user1) - this.UserGetter.getUser - .withArgs(this.user2Id) - .callsArgWith(1, null, this.user2) - this.UserGetter.getUser - .withArgs(this.user3Id) - .callsArgWith(1, null, this.user3) - this.UserGetter.getUser - .withArgs(this.user4Id) - .callsArgWith(1, null, this.user4) + this.UserGetter.getUser.withArgs(this.user1Id).yields(null, this.user1) + this.UserGetter.getUser.withArgs(this.user2Id).yields(null, this.user2) + this.UserGetter.getUser.withArgs(this.user3Id).yields(null, this.user3) + this.UserGetter.getUser.withArgs(this.user4Id).yields(null, this.user4) this.SubscriptionLocator.getUsersSubscription .withArgs(this.user2) diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index c41156624a..0e73c49716 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/services/web/test/unit/src/User/UserControllerTests.js @@ -38,10 +38,10 @@ describe('UserController', function () { this.UserDeleter = { deleteUser: sinon.stub().yields() } this.UserGetter = { - getUser: sinon.stub().callsArgWith(1, null, this.user), + getUser: sinon.stub().yields(null, this.user), promises: { getUser: sinon.stub().resolves(this.user) }, } - this.User = { findById: sinon.stub().callsArgWith(1, null, this.user) } + this.User = { findById: sinon.stub().yields(null, this.user) } this.NewsLetterManager = { subscribe: sinon.stub().yields(), unsubscribe: sinon.stub().yields(),