Merge pull request #14881 from overleaf/ab-beta-program-async-await

[web] Convert BetaProgramController to async/await

GitOrigin-RevId: 2423cb00b78b2f3fddb7bcd317d34ed4dbc12b95

services/web/app/src/Features/BetaProgram/BetaProgramController.js

@@ -1,61 +1,57 @@
 const BetaProgramHandler = require('./BetaProgramHandler')
 const OError = require('@overleaf/o-error')
 const UserGetter = require('../User/UserGetter')
-const Settings = require('@overleaf/settings')
 const logger = require('@overleaf/logger')
 const SessionManager = require('../Authentication/SessionManager')
 const SplitTestHandler = require('../SplitTests/SplitTestHandler')
+const { expressify } = require('../../util/promises')
 
-const BetaProgramController = {
-  optIn(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    logger.debug({ userId }, 'user opting in to beta program')
-    if (userId == null) {
-      return next(new Error('no user id in session'))
-    }
-    BetaProgramHandler.optIn(userId, function (err) {
-      if (err) {
-        return next(err)
-      }
-      SplitTestHandler.sessionMaintenance(req, null, () =>
-        res.redirect('/beta/participate')
-      )
-    })
-  },
+async function optIn(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  await BetaProgramHandler.promises.optIn(userId)
+  try {
+    await SplitTestHandler.promises.sessionMaintenance(req, null)
+  } catch (error) {
+    logger.error(
+      { err: error },
+      'Failed to perform session maintenance after beta program opt in'
+    )
+  }
+  res.redirect('/beta/participate')
+}
 
-  optOut(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    logger.debug({ userId }, 'user opting out of beta program')
-    if (userId == null) {
-      return next(new Error('no user id in session'))
-    }
-    BetaProgramHandler.optOut(userId, function (err) {
-      if (err) {
-        return next(err)
-      }
-      SplitTestHandler.sessionMaintenance(req, null, () =>
-        res.redirect('/beta/participate')
-      )
-    })
-  },
+async function optOut(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  await BetaProgramHandler.promises.optOut(userId)
+  try {
+    await SplitTestHandler.promises.sessionMaintenance(req, null)
+  } catch (error) {
+    logger.error(
+      { err: error },
+      'Failed to perform session maintenance after beta program opt out'
+    )
+  }
+  res.redirect('/beta/participate')
+}
 
-  optInPage(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    logger.debug({ userId }, 'showing beta participation page for user')
-    UserGetter.getUser(userId, { betaProgram: 1 }, function (err, user) {
-      if (err) {
-        OError.tag(err, 'error fetching user', {
-          userId,
-        })
-        return next(err)
-      }
-      res.render('beta_program/opt_in', {
-        title: 'sharelatex_beta_program',
-        user,
-        languages: Settings.languages,
-      })
+async function optInPage(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  let user
+  try {
+    user = await UserGetter.promises.getUser(userId, { betaProgram: 1 })
+  } catch (error) {
+    throw OError.tag(error, 'error fetching user', {
+      userId,
     })
-  },
+  }
+  res.render('beta_program/opt_in', {
+    title: 'sharelatex_beta_program',
+    user,
+  })
 }
 
-module.exports = BetaProgramController
+module.exports = {
+  optIn: expressify(optIn),
+  optOut: expressify(optOut),
+  optInPage: expressify(optInPage),
+}

services/web/app/src/Features/BetaProgram/BetaProgramHandler.js

@@ -17,15 +17,11 @@ async function optOut(userId) {
   AnalyticsManager.setUserPropertyForUser(userId, 'beta-program', false)
 }
 
-const BetaProgramHandler = {
+module.exports = {
   optIn: callbackify(optIn),
-
   optOut: callbackify(optOut),
+  promises: {
+    optIn,
+    optOut,
+  },
 }
-
-BetaProgramHandler.promises = {
-  optIn,
-  optOut,
-}
-
-module.exports = BetaProgramHandler

services/web/test/unit/src/BetaProgram/BetaProgramControllerTests.js

@@ -1,6 +1,8 @@
 const SandboxedModule = require('sandboxed-module')
 const path = require('path')
 const sinon = require('sinon')
+const { expect } = require('chai')
+const MockResponse = require('../helpers/MockResponse')
 
 const modulePath = path.join(
   __dirname,
@@ -22,17 +24,23 @@ describe('BetaProgramController', function () {
       },
     }
     this.SplitTestHandler = {
-      sessionMaintenance: sinon.stub().yields(),
+      promises: {
+        sessionMaintenance: sinon.stub(),
+      },
     }
     this.BetaProgramController = SandboxedModule.require(modulePath, {
       requires: {
         '../SplitTests/SplitTestHandler': this.SplitTestHandler,
         './BetaProgramHandler': (this.BetaProgramHandler = {
-          optIn: sinon.stub(),
-          optOut: sinon.stub(),
+          promises: {
+            optIn: sinon.stub().resolves(),
+            optOut: sinon.stub().resolves(),
+          },
         }),
         '../User/UserGetter': (this.UserGetter = {
-          getUser: sinon.stub(),
+          promises: {
+            getUser: sinon.stub().resolves(),
+          },
         }),
         '@overleaf/settings': (this.settings = {
           languages: {},
@@ -43,23 +51,17 @@ describe('BetaProgramController', function () {
           }),
       },
     })
-    this.res = {
-      send: sinon.stub(),
-      redirect: sinon.stub(),
-      render: sinon.stub(),
-    }
+    this.res = new MockResponse()
     this.next = sinon.stub()
   })
 
   describe('optIn', function () {
-    beforeEach(function () {
-      this.BetaProgramHandler.optIn.callsArgWith(1, null)
-    })
-
-    it("should redirect to '/beta/participate'", function () {
-      this.BetaProgramController.optIn(this.req, this.res, this.next)
-      this.res.redirect.callCount.should.equal(1)
-      this.res.redirect.firstCall.args[0].should.equal('/beta/participate')
+    it("should redirect to '/beta/participate'", function (done) {
+      this.res.callback = () => {
+        this.res.redirectedTo.should.equal('/beta/participate')
+        done()
+      }
+      this.BetaProgramController.optIn(this.req, this.res, done)
     })
 
     it('should not call next with an error', function () {
@@ -69,19 +71,22 @@ describe('BetaProgramController', function () {
 
     it('should call BetaProgramHandler.optIn', function () {
       this.BetaProgramController.optIn(this.req, this.res, this.next)
-      this.BetaProgramHandler.optIn.callCount.should.equal(1)
+      this.BetaProgramHandler.promises.optIn.callCount.should.equal(1)
     })
 
-    it('should invoke the session maintenance', function () {
-      this.BetaProgramController.optIn(this.req, this.res, this.next)
-      this.SplitTestHandler.sessionMaintenance.should.have.been.calledWith(
-        this.req
-      )
+    it('should invoke the session maintenance', function (done) {
+      this.res.callback = () => {
+        this.SplitTestHandler.promises.sessionMaintenance.should.have.been.calledWith(
+          this.req
+        )
+        done()
+      }
+      this.BetaProgramController.optIn(this.req, this.res, done)
     })
 
     describe('when BetaProgramHandler.opIn produces an error', function () {
       beforeEach(function () {
-        this.BetaProgramHandler.optIn.callsArgWith(1, new Error('woops'))
+        this.BetaProgramHandler.promises.optIn.throws(new Error('woops'))
       })
 
       it("should not redirect to '/beta/participate'", function () {
@@ -89,75 +94,89 @@ describe('BetaProgramController', function () {
         this.res.redirect.callCount.should.equal(0)
       })
 
-      it('should produce an error', function () {
-        this.BetaProgramController.optIn(this.req, this.res, this.next)
-        this.next.callCount.should.equal(1)
-        this.next.firstCall.args[0].should.be.instanceof(Error)
+      it('should produce an error', function (done) {
+        this.BetaProgramController.optIn(this.req, this.res, err => {
+          expect(err).to.be.instanceof(Error)
+          done()
+        })
       })
     })
   })
 
   describe('optOut', function () {
-    beforeEach(function () {
-      this.BetaProgramHandler.optOut.callsArgWith(1, null)
-    })
-
-    it("should redirect to '/beta/participate'", function () {
-      this.BetaProgramController.optOut(this.req, this.res, this.next)
-      this.res.redirect.callCount.should.equal(1)
-      this.res.redirect.firstCall.args[0].should.equal('/beta/participate')
+    it("should redirect to '/beta/participate'", function (done) {
+      this.res.callback = () => {
+        expect(this.res.redirectedTo).to.equal('/beta/participate')
+        done()
+      }
+      this.BetaProgramController.optOut(this.req, this.res, done)
     })
 
-    it('should not call next with an error', function () {
-      this.BetaProgramController.optOut(this.req, this.res, this.next)
-      this.next.callCount.should.equal(0)
+    it('should not call next with an error', function (done) {
+      this.res.callback = () => {
+        this.next.callCount.should.equal(0)
+        done()
+      }
+      this.BetaProgramController.optOut(this.req, this.res, done)
     })
 
-    it('should call BetaProgramHandler.optOut', function () {
-      this.BetaProgramController.optOut(this.req, this.res, this.next)
-      this.BetaProgramHandler.optOut.callCount.should.equal(1)
+    it('should call BetaProgramHandler.optOut', function (done) {
+      this.res.callback = () => {
+        this.BetaProgramHandler.promises.optOut.callCount.should.equal(1)
+        done()
+      }
+      this.BetaProgramController.optOut(this.req, this.res, done)
     })
 
-    it('should invoke the session maintenance', function () {
-      this.BetaProgramController.optOut(this.req, this.res, this.next)
-      this.SplitTestHandler.sessionMaintenance.should.have.been.calledWith(
-        this.req
-      )
+    it('should invoke the session maintenance', function (done) {
+      this.res.callback = () => {
+        this.SplitTestHandler.promises.sessionMaintenance.should.have.been.calledWith(
+          this.req,
+          null
+        )
+        done()
+      }
+      this.BetaProgramController.optOut(this.req, this.res, done)
     })
 
     describe('when BetaProgramHandler.optOut produces an error', function () {
       beforeEach(function () {
-        this.BetaProgramHandler.optOut.callsArgWith(1, new Error('woops'))
+        this.BetaProgramHandler.promises.optOut.throws(new Error('woops'))
       })
 
-      it("should not redirect to '/beta/participate'", function () {
-        this.BetaProgramController.optOut(this.req, this.res, this.next)
-        this.res.redirect.callCount.should.equal(0)
+      it("should not redirect to '/beta/participate'", function (done) {
+        this.BetaProgramController.optOut(this.req, this.res, error => {
+          expect(error).to.exist
+          expect(this.res.redirected).to.equal(false)
+          done()
+        })
       })
 
-      it('should produce an error', function () {
-        this.BetaProgramController.optOut(this.req, this.res, this.next)
-        this.next.callCount.should.equal(1)
-        this.next.firstCall.args[0].should.be.instanceof(Error)
+      it('should produce an error', function (done) {
+        this.BetaProgramController.optOut(this.req, this.res, error => {
+          expect(error).to.exist
+          done()
+        })
       })
     })
   })
 
   describe('optInPage', function () {
     beforeEach(function () {
-      this.UserGetter.getUser.yields(null, this.user)
+      this.UserGetter.promises.getUser.resolves(this.user)
     })
 
-    it('should render the opt-in page', function () {
-      this.BetaProgramController.optInPage(this.req, this.res, this.next)
-      this.res.render.callCount.should.equal(1)
-      const { args } = this.res.render.firstCall
-      args[0].should.equal('beta_program/opt_in')
+    it('should render the opt-in page', function (done) {
+      this.res.callback = () => {
+        expect(this.res.renderedTemplate).to.equal('beta_program/opt_in')
+        done()
+      }
+      this.BetaProgramController.optInPage(this.req, this.res, done)
     })
 
     describe('when UserGetter.getUser produces an error', function () {
       beforeEach(function () {
-        this.UserGetter.getUser.yields(new Error('woops'))
+        this.UserGetter.promises.getUser.throws(new Error('woops'))
       })
 
       it('should not render the opt-in page', function () {
@@ -165,10 +184,12 @@ describe('BetaProgramController', function () {
         this.res.render.callCount.should.equal(0)
       })
 
-      it('should produce an error', function () {
-        this.BetaProgramController.optInPage(this.req, this.res, this.next)
-        this.next.callCount.should.equal(1)
-        this.next.firstCall.args[0].should.be.instanceof(Error)
+      it('should produce an error', function (done) {
+        this.BetaProgramController.optInPage(this.req, this.res, error => {
+          expect(error).to.exist
+          expect(error).to.be.instanceof(Error)
+          done()
+        })
       })
     })
   })