From 8bd917242bc1301ea4dfbf852d33b0643088fff1 Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Mon, 5 Feb 2024 09:28:35 +0000 Subject: [PATCH] Merge pull request #16744 from overleaf/dp-mongoose-callback-user-feature-updater Convert UserFeaturesUpdater and UserFeaturesUpdaterTests to async/await GitOrigin-RevId: 65cbb57d463dd9557e7e8e7643a1be160f17eebc --- .../Subscription/UserFeaturesUpdater.js | 90 +++++++++--------- .../Subscription/UserFeaturesUpdaterTests.js | 94 ++++++++----------- 2 files changed, 84 insertions(+), 100 deletions(-) diff --git a/services/web/app/src/Features/Subscription/UserFeaturesUpdater.js b/services/web/app/src/Features/Subscription/UserFeaturesUpdater.js index e3fb74970a..e29eb8e0e2 100644 --- a/services/web/app/src/Features/Subscription/UserFeaturesUpdater.js +++ b/services/web/app/src/Features/Subscription/UserFeaturesUpdater.js @@ -1,5 +1,5 @@ const { User } = require('../../models/User') -const { promisifyAll } = require('@overleaf/promise-utils') +const { callbackify } = require('util') const Settings = require('@overleaf/settings') function _featuresChanged(newFeatures, featuresBefore) { @@ -11,51 +11,51 @@ function _featuresChanged(newFeatures, featuresBefore) { return false } -module.exports = { - updateFeatures(userId, features, callback) { - const update = { - featuresUpdatedAt: new Date(), - } - // record the system-wide features epoch, if defined - if (Settings.featuresEpoch) { - update.featuresEpoch = Settings.featuresEpoch - } - for (const key in features) { - const value = features[key] - update[`features.${key}`] = value - } - User.findByIdAndUpdate(userId, update, (err, docBeforeUpdate) => { - let featuresChanged = false - if (docBeforeUpdate) { - featuresChanged = _featuresChanged(features, docBeforeUpdate.features) - } +async function updateFeatures(userId, features) { + const update = { + featuresUpdatedAt: new Date(), + } + // record the system-wide features epoch, if defined + if (Settings.featuresEpoch) { + update.featuresEpoch = Settings.featuresEpoch + } + for (const key in features) { + const value = features[key] + update[`features.${key}`] = value + } + const docBeforeUpdate = await User.findByIdAndUpdate(userId, update).exec() + let featuresChanged = false + if (docBeforeUpdate) { + featuresChanged = _featuresChanged(features, docBeforeUpdate.features) + } - callback(err, features, featuresChanged) - }) - }, - - overrideFeatures(userId, features, callback) { - const update = { features, featuresUpdatedAt: new Date() } - User.findByIdAndUpdate(userId, update, (err, docBeforeUpdate) => { - let featuresChanged = false - if (docBeforeUpdate) { - featuresChanged = _featuresChanged(features, docBeforeUpdate.features) - } - callback(err, featuresChanged) - }) - }, - - createFeaturesOverride(userId, featuresOverride, callback) { - User.updateOne( - { _id: userId }, - { $push: { featuresOverrides: featuresOverride } }, - callback - ) - }, + return { features, featuresChanged } } -module.exports.promises = promisifyAll(module.exports, { - multiResult: { - updateFeatures: ['features', 'featuresChanged'], +async function overrideFeatures(userId, features) { + const update = { features, featuresUpdatedAt: new Date() } + const docBeforeUpdate = await User.findByIdAndUpdate(userId, update).exec() + let featuresChanged = false + if (docBeforeUpdate) { + featuresChanged = _featuresChanged(features, docBeforeUpdate.features) + } + return featuresChanged +} + +async function createFeaturesOverride(userId, featuresOverride) { + return await User.updateOne( + { _id: userId }, + { $push: { featuresOverrides: featuresOverride } } + ).exec() +} + +module.exports = { + updateFeatures: callbackify(updateFeatures), + overrideFeatures: callbackify(overrideFeatures), + createFeaturesOverride: callbackify(createFeaturesOverride), + promises: { + updateFeatures, + overrideFeatures, + createFeaturesOverride, }, -}) +} diff --git a/services/web/test/unit/src/Subscription/UserFeaturesUpdaterTests.js b/services/web/test/unit/src/Subscription/UserFeaturesUpdaterTests.js index 607226993d..0f0b15b7e8 100644 --- a/services/web/test/unit/src/Subscription/UserFeaturesUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/UserFeaturesUpdaterTests.js @@ -1,6 +1,5 @@ const SandboxedModule = require('sandboxed-module') const { expect } = require('chai') -const assert = require('assert') const sinon = require('sinon') const modulePath = '../../../../app/src/Features/Subscription/UserFeaturesUpdater' @@ -24,7 +23,9 @@ describe('UserFeaturesUpdater', function () { symbolPalette: true, } this.User = { - findByIdAndUpdate: sinon.stub().yields(null, { features: this.features }), + findByIdAndUpdate: sinon.stub().returns({ + exec: sinon.stub().resolves({ features: this.features }), + }), } this.UserFeaturesUpdater = SandboxedModule.require(modulePath, { requires: { @@ -37,77 +38,60 @@ describe('UserFeaturesUpdater', function () { }) describe('updateFeatures', function () { - it('should send the users features', function (done) { + it('should send the users features', async function () { const userId = '5208dd34438842e2db000005' const update = { versioning: true, collaborators: 10, } - this.UserFeaturesUpdater.updateFeatures( - userId, - update, - (err, features) => { - const updateArgs = this.User.findByIdAndUpdate.lastCall.args - expect(updateArgs[0]).to.deep.equal(userId) - assert.equal(err, null) - expect(Object.keys(updateArgs[1]).length).to.equal(3) - expect(updateArgs[1]['features.versioning']).to.equal( - update.versioning - ) - expect(updateArgs[1]['features.collaborators']).to.equal( - update.collaborators - ) - expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true - features.should.deep.equal(update) - expect(updateArgs[1].featuresEpoch).to.be.undefined - done() - } + + const { features } = + await this.UserFeaturesUpdater.promises.updateFeatures(userId, update) + + const updateArgs = this.User.findByIdAndUpdate.lastCall.args + expect(updateArgs[0]).to.deep.equal(userId) + expect(Object.keys(updateArgs[1]).length).to.equal(3) + expect(updateArgs[1]['features.versioning']).to.equal(update.versioning) + expect(updateArgs[1]['features.collaborators']).to.equal( + update.collaborators ) + expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true + features.should.deep.equal(update) + expect(updateArgs[1].featuresEpoch).to.be.undefined }) - it('should set the featuresEpoch when present', function (done) { + + it('should set the featuresEpoch when present', async function () { const userId = '5208dd34438842e2db000005' const update = { versioning: true, } this.Settings.featuresEpoch = 'epoch-1' - this.UserFeaturesUpdater.updateFeatures( - userId, - update, - (err, features) => { - const updateArgs = this.User.findByIdAndUpdate.lastCall.args - expect(updateArgs[0]).to.deep.equal(userId) - assert.equal(err, null) - expect(Object.keys(updateArgs[1]).length).to.equal(3) - expect(updateArgs[1]['features.versioning']).to.equal( - update.versioning - ) - expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true - features.should.deep.equal(update) - expect(updateArgs[1].featuresEpoch).to.equal('epoch-1') - done() - } - ) + const { features } = + await this.UserFeaturesUpdater.promises.updateFeatures(userId, update) + + const updateArgs = this.User.findByIdAndUpdate.lastCall.args + expect(updateArgs[0]).to.deep.equal(userId) + expect(Object.keys(updateArgs[1]).length).to.equal(3) + expect(updateArgs[1]['features.versioning']).to.equal(update.versioning) + expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true + features.should.deep.equal(update) + expect(updateArgs[1].featuresEpoch).to.equal('epoch-1') }) }) describe('overrideFeatures', function () { - it('should send the users features', function (done) { + it('should send the users features', async function () { const userId = '5208dd34438842e2db000005' const update = Object.assign({}, { mendeley: !this.features.mendeley }) - this.UserFeaturesUpdater.overrideFeatures( - userId, - update, - (err, featuresChanged) => { - assert.equal(err, null) - const updateArgs = this.User.findByIdAndUpdate.lastCall.args - expect(updateArgs[0]).to.equal(userId) - expect(Object.keys(updateArgs[1]).length).to.equal(2) - expect(updateArgs[1].features).to.deep.equal(update) - expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true - featuresChanged.should.equal(true) - done() - } - ) + const featuresChanged = + await this.UserFeaturesUpdater.promises.overrideFeatures(userId, update) + + const updateArgs = this.User.findByIdAndUpdate.lastCall.args + expect(updateArgs[0]).to.equal(userId) + expect(Object.keys(updateArgs[1]).length).to.equal(2) + expect(updateArgs[1].features).to.deep.equal(update) + expect(updateArgs[1].featuresUpdatedAt instanceof Date).to.be.true + featuresChanged.should.equal(true) }) }) })