Merge pull request #16744 from overleaf/dp-mongoose-callback-user-feature-updater

Convert UserFeaturesUpdater and UserFeaturesUpdaterTests to async/await

GitOrigin-RevId: 65cbb57d463dd9557e7e8e7643a1be160f17eebc

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)
-    })
-  },
+  return { 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)
-    })
-  },
+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
+}
 
-  createFeaturesOverride(userId, featuresOverride, callback) {
-    User.updateOne(
-      { _id: userId },
-      { $push: { featuresOverrides: featuresOverride } },
-      callback
-    )
-  },
+async function createFeaturesOverride(userId, featuresOverride) {
+  return await User.updateOne(
+    { _id: userId },
+    { $push: { featuresOverrides: featuresOverride } }
+  ).exec()
 }
 
-module.exports.promises = promisifyAll(module.exports, {
-  multiResult: {
-    updateFeatures: ['features', 'featuresChanged'],
+module.exports = {
+  updateFeatures: callbackify(updateFeatures),
+  overrideFeatures: callbackify(overrideFeatures),
+  createFeaturesOverride: callbackify(createFeaturesOverride),
+  promises: {
+    updateFeatures,
+    overrideFeatures,
+    createFeaturesOverride,
   },
-})
+}

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)
     })
   })
 })