Merge pull request #2974 from overleaf/jel-ns-user-projections-beta

Remove user projections in BetaProgramHandler

GitOrigin-RevId: 88b7bc3b6f11ae9f8314543ee538c84d25cde7cd

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

@@ -1,62 +1,28 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let BetaProgramHandler
-const { User } = require('../../models/User')
-const logger = require('logger-sharelatex')
+const { callbackify } = require('util')
 const metrics = require('metrics-sharelatex')
+const UserUpdater = require('../User/UserUpdater')
 
-module.exports = BetaProgramHandler = {
-  optIn(user_id, callback) {
-    if (callback == null) {
-      callback = function(err) {}
-    }
-    return User.findById(user_id, function(err, user) {
-      if (err) {
-        logger.warn({ err, user_id }, 'problem adding user to beta')
-        return callback(err)
-      }
-      metrics.inc('beta-program.opt-in')
-      user.betaProgram = true
-      return user.save(function(err) {
-        if (err) {
-          logger.warn({ err, user_id }, 'problem adding user to beta')
-          return callback(err)
-        }
-        return callback(null)
-      })
-    })
-  },
+async function optIn(userId) {
+  await UserUpdater.promises.updateUser(userId, { $set: { betaProgram: true } })
+  metrics.inc('beta-program.opt-in')
+}
+
+async function optOut(userId) {
+  await UserUpdater.promises.updateUser(userId, {
+    $set: { betaProgram: false }
+  })
+  metrics.inc('beta-program.opt-out')
+}
+
+const BetaProgramHandler = {
+  optIn: callbackify(optIn),
 
-  optOut(user_id, callback) {
-    if (callback == null) {
-      callback = function(err) {}
-    }
-    return User.findById(user_id, function(err, user) {
-      if (err) {
-        logger.warn({ err, user_id }, 'problem removing user from beta')
-        return callback(err)
-      }
-      metrics.inc('beta-program.opt-out')
-      user.betaProgram = false
-      return user.save(function(err) {
-        if (err) {
-          logger.warn({ err, user_id }, 'problem removing user from beta')
-          return callback(err)
-        }
-        return callback(null)
-      })
-    })
-  }
+  optOut: callbackify(optOut)
 }
+
+BetaProgramHandler.promises = {
+  optIn,
+  optOut
+}
+
+module.exports = BetaProgramHandler

services/web/test/acceptance/src/BetaProgramTests.js

@@ -0,0 +1,39 @@
+const { expect } = require('chai')
+const UserHelper = require('../src/helpers/UserHelper')
+
+describe('BetaProgram', function() {
+  let email, userHelper
+  beforeEach(async function() {
+    userHelper = new UserHelper()
+    email = userHelper.getDefaultEmail()
+    userHelper = await UserHelper.createUser({ email })
+    userHelper = await UserHelper.loginUser({
+      email,
+      password: userHelper.getDefaultPassword()
+    })
+  })
+  it('should opt in', async function() {
+    const response = await userHelper.request.post('/beta/opt-in', {
+      simple: false
+    })
+    expect(response.statusCode).to.equal(302)
+    response.statusCode.should.equal(302)
+    expect(response.headers.location).to.equal('/beta/participate')
+    const user = (await UserHelper.getUser({
+      email
+    })).user
+    expect(user.betaProgram).to.equal(true)
+  })
+  it('should opt out', async function() {
+    const response = await userHelper.request.post('/beta/opt-out', {
+      simple: false
+    })
+    expect(response.statusCode).to.equal(302)
+    response.statusCode.should.equal(302)
+    expect(response.headers.location).to.equal('/beta/participate')
+    const user = (await UserHelper.getUser({
+      email
+    })).user
+    expect(user.betaProgram).to.equal(false)
+  })
+})

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

@@ -1,19 +1,4 @@
-/* eslint-disable
-    handle-callback-err,
-    max-len,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const should = require('chai').should()
 const SandboxedModule = require('sandboxed-module')
-const assert = require('assert')
 const path = require('path')
 const modulePath = path.join(
   __dirname,
@@ -32,68 +17,56 @@ describe('BetaProgramHandler', function() {
       betaProgram: false,
       save: sinon.stub().callsArgWith(0, null)
     }
-    return (this.handler = SandboxedModule.require(modulePath, {
+    this.handler = SandboxedModule.require(modulePath, {
       globals: {
         console: console
       },
       requires: {
-        '../../models/User': {
-          User: {
-            findById: sinon.stub().callsArgWith(1, null, this.user)
-          }
-        },
-        'logger-sharelatex': (this.logger = {
-          log: sinon.stub(),
-          warn: sinon.stub(),
-          err: sinon.stub()
-        }),
         'metrics-sharelatex': (this.logger = {
           inc: sinon.stub()
+        }),
+        '../User/UserUpdater': (this.UserUpdater = {
+          promises: {
+            updateUser: sinon.stub().resolves()
+          }
         })
       }
-    }))
+    })
   })
 
   describe('optIn', function() {
     beforeEach(function() {
       this.user.betaProgram = false
-      return (this.call = callback => {
-        return this.handler.optIn(this.user_id, callback)
-      })
-    })
-
-    it('should set betaProgram = true on user object', function(done) {
-      return this.call(err => {
-        this.user.betaProgram.should.equal(true)
-        return done()
-      })
+      this.call = callback => {
+        this.handler.optIn(this.user_id, callback)
+      }
     })
 
-    it('should call user.save', function(done) {
-      return this.call(err => {
-        this.user.save.callCount.should.equal(1)
-        return done()
+    it('should call userUpdater', function(done) {
+      this.call(err => {
+        expect(err).to.not.exist
+        this.UserUpdater.promises.updateUser.callCount.should.equal(1)
+        done()
       })
     })
 
     it('should not produce an error', function(done) {
-      return this.call(err => {
-        expect(err).to.equal(null)
-        expect(err).to.not.be.instanceof(Error)
-        return done()
+      this.call(err => {
+        expect(err).to.not.exist
+        done()
       })
     })
 
-    describe('when user.save produces an error', function() {
+    describe('when userUpdater produces an error', function() {
       beforeEach(function() {
-        return this.user.save.callsArgWith(0, new Error('woops'))
+        this.UserUpdater.promises.updateUser.rejects()
       })
 
       it('should produce an error', function(done) {
-        return this.call(err => {
-          expect(err).to.not.equal(null)
+        this.call(err => {
+          expect(err).to.exist
           expect(err).to.be.instanceof(Error)
-          return done()
+          done()
         })
       })
     })
@@ -102,43 +75,36 @@ describe('BetaProgramHandler', function() {
   describe('optOut', function() {
     beforeEach(function() {
       this.user.betaProgram = true
-      return (this.call = callback => {
-        return this.handler.optOut(this.user_id, callback)
-      })
-    })
-
-    it('should set betaProgram = true on user object', function(done) {
-      return this.call(err => {
-        this.user.betaProgram.should.equal(false)
-        return done()
-      })
+      this.call = callback => {
+        this.handler.optOut(this.user_id, callback)
+      }
     })
 
-    it('should call user.save', function(done) {
-      return this.call(err => {
-        this.user.save.callCount.should.equal(1)
-        return done()
+    it('should call userUpdater', function(done) {
+      this.call(err => {
+        expect(err).to.not.exist
+        this.UserUpdater.promises.updateUser.callCount.should.equal(1)
+        done()
       })
     })
 
     it('should not produce an error', function(done) {
-      return this.call(err => {
-        expect(err).to.equal(null)
-        expect(err).to.not.be.instanceof(Error)
-        return done()
+      this.call(err => {
+        expect(err).to.not.exist
+        done()
       })
     })
 
-    describe('when user.save produces an error', function() {
+    describe('when userUpdater produces an error', function() {
       beforeEach(function() {
-        return this.user.save.callsArgWith(0, new Error('woops'))
+        this.UserUpdater.promises.updateUser.rejects()
       })
 
       it('should produce an error', function(done) {
-        return this.call(err => {
-          expect(err).to.not.equal(null)
+        this.call(err => {
+          expect(err).to.exist
           expect(err).to.be.instanceof(Error)
-          return done()
+          done()
         })
       })
     })