Merge pull request #2994 from overleaf/msm-add-user-projection-subscription-handler

Added projection to User.find() queries in SubscriptionHandler

GitOrigin-RevId: f74e3fcd2138306dc35b5c4d0da314046ab48e35

services/web/app/src/Features/Subscription/SubscriptionHandler.js

@@ -1,21 +1,5 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-unused-vars,
-    standard/no-callback-literal,
-*/
-// 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
- */
 const async = require('async')
 const RecurlyWrapper = require('./RecurlyWrapper')
-const Settings = require('settings-sharelatex')
 const { User } = require('../../models/User')
 const { promisifyAll } = require('../../util/promises')
 const logger = require('logger-sharelatex')
@@ -26,11 +10,11 @@ const Events = require('../../infrastructure/Events')
 const Analytics = require('../Analytics/AnalyticsManager')
 
 const SubscriptionHandler = {
-  validateNoSubscriptionInRecurly(user_id, callback) {
+  validateNoSubscriptionInRecurly(userId, callback) {
     if (callback == null) {
-      callback = function(error, valid) {}
+      callback = function() {}
     }
-    return RecurlyWrapper.listAccountActiveSubscriptions(user_id, function(
+    RecurlyWrapper.listAccountActiveSubscriptions(userId, function(
       error,
       subscriptions
     ) {
@@ -41,83 +25,85 @@ const SubscriptionHandler = {
         return callback(error)
       }
       if (subscriptions.length > 0) {
-        return SubscriptionUpdater.syncSubscription(
-          subscriptions[0],
-          user_id,
-          function(error) {
-            if (error != null) {
-              return callback(error)
-            }
-            return callback(null, false)
+        SubscriptionUpdater.syncSubscription(subscriptions[0], userId, function(
+          error
+        ) {
+          if (error != null) {
+            return callback(error)
           }
-        )
+          callback(null, false)
+        })
       } else {
-        return callback(null, true)
+        callback(null, true)
       }
     })
   },
 
   createSubscription(user, subscriptionDetails, recurlyTokenIds, callback) {
-    const clientTokenId = ''
-    return SubscriptionHandler.validateNoSubscriptionInRecurly(
-      user._id,
-      function(error, valid) {
-        if (error != null) {
-          return callback(error)
-        }
-        if (!valid) {
-          return callback(new Error('user already has subscription in recurly'))
-        }
-        return RecurlyWrapper.createSubscription(
-          user,
-          subscriptionDetails,
-          recurlyTokenIds,
-          function(error, recurlySubscription) {
-            if (error != null) {
-              return callback(error)
-            }
-            return SubscriptionUpdater.syncSubscription(
-              recurlySubscription,
-              user._id,
-              function(error) {
-                if (error != null) {
-                  return callback(error)
-                }
-                return callback()
-              }
-            )
-          }
-        )
+    SubscriptionHandler.validateNoSubscriptionInRecurly(user._id, function(
+      error,
+      valid
+    ) {
+      if (error != null) {
+        return callback(error)
       }
-    )
+      if (!valid) {
+        return callback(new Error('user already has subscription in recurly'))
+      }
+      RecurlyWrapper.createSubscription(
+        user,
+        subscriptionDetails,
+        recurlyTokenIds,
+        function(error, recurlySubscription) {
+          if (error != null) {
+            return callback(error)
+          }
+          return SubscriptionUpdater.syncSubscription(
+            recurlySubscription,
+            user._id,
+            function(error) {
+              if (error != null) {
+                return callback(error)
+              }
+              return callback()
+            }
+          )
+        }
+      )
+    })
   },
 
-  updateSubscription(user, plan_code, coupon_code, callback) {
-    return LimitationsManager.userHasV2Subscription(user, function(
+  updateSubscription(user, planCode, couponCode, callback) {
+    LimitationsManager.userHasV2Subscription(user, function(
       err,
       hasSubscription,
       subscription
     ) {
+      if (err) {
+        logger.warn(
+          { err, user_id: user._id, hasSubscription },
+          'there was an error checking user v2 subscription'
+        )
+      }
       if (!hasSubscription) {
         return callback()
       } else {
         return async.series(
           [
             function(cb) {
-              if (coupon_code == null) {
+              if (couponCode == null) {
                 return cb()
               }
-              return RecurlyWrapper.getSubscription(
+              RecurlyWrapper.getSubscription(
                 subscription.recurlySubscription_id,
                 { includeAccount: true },
                 function(err, usersSubscription) {
                   if (err != null) {
                     return callback(err)
                   }
-                  const { account_code } = usersSubscription.account
-                  return RecurlyWrapper.redeemCoupon(
-                    account_code,
-                    coupon_code,
+                  RecurlyWrapper.redeemCoupon(
+                    usersSubscription.account.account_code,
+                    couponCode,
                     cb
                   )
                 }
@@ -126,12 +112,12 @@ const SubscriptionHandler = {
             cb =>
               RecurlyWrapper.updateSubscription(
                 subscription.recurlySubscription_id,
-                { plan_code, timeframe: 'now' },
+                { plan_code: planCode, timeframe: 'now' },
                 function(error, recurlySubscription) {
                   if (error != null) {
                     return callback(error)
                   }
-                  return SubscriptionUpdater.syncSubscription(
+                  SubscriptionUpdater.syncSubscription(
                     recurlySubscription,
                     user._id,
                     cb
@@ -146,13 +132,19 @@ const SubscriptionHandler = {
   },
 
   cancelSubscription(user, callback) {
-    return LimitationsManager.userHasV2Subscription(user, function(
+    LimitationsManager.userHasV2Subscription(user, function(
       err,
       hasSubscription,
       subscription
     ) {
+      if (err) {
+        logger.warn(
+          { err, user_id: user._id, hasSubscription },
+          'there was an error checking user v2 subscription'
+        )
+      }
       if (hasSubscription) {
-        return RecurlyWrapper.cancelSubscription(
+        RecurlyWrapper.cancelSubscription(
           subscription.recurlySubscription_id,
           function(error) {
             if (error != null) {
@@ -181,23 +173,29 @@ const SubscriptionHandler = {
             )
             Events.emit('cancelSubscription', user._id)
             Analytics.recordEvent(user._id, 'subscription-canceled')
-            return callback()
+            callback()
           }
         )
       } else {
-        return callback()
+        callback()
       }
     })
   },
 
   reactivateSubscription(user, callback) {
-    return LimitationsManager.userHasV2Subscription(user, function(
+    LimitationsManager.userHasV2Subscription(user, function(
       err,
       hasSubscription,
       subscription
     ) {
+      if (err) {
+        logger.warn(
+          { err, user_id: user._id, hasSubscription },
+          'there was an error checking user v2 subscription'
+        )
+      }
       if (hasSubscription) {
-        return RecurlyWrapper.reactivateSubscription(
+        RecurlyWrapper.reactivateSubscription(
           subscription.recurlySubscription_id,
           function(error) {
             if (error != null) {
@@ -216,40 +214,41 @@ const SubscriptionHandler = {
               }
             )
             Analytics.recordEvent(user._id, 'subscription-reactivated')
-            return callback()
+            callback()
           }
         )
       } else {
-        return callback()
+        callback()
       }
     })
   },
 
   syncSubscription(recurlySubscription, requesterData, callback) {
-    return RecurlyWrapper.getSubscription(
+    RecurlyWrapper.getSubscription(
       recurlySubscription.uuid,
       { includeAccount: true },
       function(error, recurlySubscription) {
         if (error != null) {
           return callback(error)
         }
-        return User.findById(recurlySubscription.account.account_code, function(
-          error,
-          user
-        ) {
-          if (error != null) {
-            return callback(error)
-          }
-          if (user == null) {
-            return callback(new Error('no user found'))
+        User.findById(
+          recurlySubscription.account.account_code,
+          { _id: 1 },
+          function(error, user) {
+            if (error != null) {
+              return callback(error)
+            }
+            if (user == null) {
+              return callback(new Error('no user found'))
+            }
+            SubscriptionUpdater.syncSubscription(
+              recurlySubscription,
+              user != null ? user._id : undefined,
+              requesterData,
+              callback
+            )
           }
-          return SubscriptionUpdater.syncSubscription(
-            recurlySubscription,
-            user != null ? user._id : undefined,
-            requesterData,
-            callback
-          )
-        })
+        )
       }
     )
   },

services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js

@@ -1,19 +1,6 @@
-/* eslint-disable
-    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 SandboxedModule = require('sandboxed-module')
-const should = require('chai').should()
+require('chai').should()
 const sinon = require('sinon')
-const querystring = require('querystring')
 const modulePath =
   '../../../../app/src/Features/Subscription/SubscriptionHandler'
 
@@ -109,9 +96,9 @@ describe('SubscriptionHandler', function() {
       }
     })
 
-    return (this.SubscriptionHandler.syncSubscriptionToUser = sinon
+    this.SubscriptionHandler.syncSubscriptionToUser = sinon
       .stub()
-      .callsArgWith(2))
+      .callsArgWith(2)
   })
 
   describe('createSubscription', function() {
@@ -122,14 +109,14 @@ describe('SubscriptionHandler', function() {
         number: '12345'
       }
       this.recurlyTokenIds = { billing: '45555666' }
-      return (this.SubscriptionHandler.validateNoSubscriptionInRecurly = sinon
+      this.SubscriptionHandler.validateNoSubscriptionInRecurly = sinon
         .stub()
-        .yields(null, true))
+        .yields(null, true)
     })
 
     describe('successfully', function() {
       beforeEach(function() {
-        return this.SubscriptionHandler.createSubscription(
+        this.SubscriptionHandler.createSubscription(
           this.user,
           this.subscriptionDetails,
           this.recurlyTokenIds,
@@ -138,7 +125,7 @@ describe('SubscriptionHandler', function() {
       })
 
       it('should create the subscription with the wrapper', function() {
-        return this.RecurlyWrapper.createSubscription
+        this.RecurlyWrapper.createSubscription
           .calledWith(this.user, this.subscriptionDetails, this.recurlyTokenIds)
           .should.equal(true)
       })
@@ -148,7 +135,7 @@ describe('SubscriptionHandler', function() {
         this.SubscriptionUpdater.syncSubscription.args[0][0].should.deep.equal(
           this.activeRecurlySubscription
         )
-        return this.SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal(
+        this.SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal(
           this.user._id
         )
       })
@@ -159,7 +146,7 @@ describe('SubscriptionHandler', function() {
         this.SubscriptionHandler.validateNoSubscriptionInRecurly = sinon
           .stub()
           .yields(null, false)
-        return this.SubscriptionHandler.createSubscription(
+        this.SubscriptionHandler.createSubscription(
           this.user,
           this.subscriptionDetails,
           this.recurlyTokenIds,
@@ -167,8 +154,8 @@ describe('SubscriptionHandler', function() {
         )
       })
 
-      it('should return an error', function() {
-        return this.callback.calledWith(
+      it('should  an error', function() {
+        this.callback.calledWith(
           new Error('user already has subscription in recurly')
         )
       })
@@ -186,7 +173,7 @@ describe('SubscriptionHandler', function() {
             true,
             this.subscription
           )
-          return this.SubscriptionHandler.updateSubscription(
+          this.SubscriptionHandler.updateSubscription(
             this.user,
             this.plan_code,
             null,
@@ -200,13 +187,13 @@ describe('SubscriptionHandler', function() {
             .should.equal(true)
           const updateOptions = this.RecurlyWrapper.updateSubscription
             .args[0][1]
-          return updateOptions.plan_code.should.equal(this.plan_code)
+          updateOptions.plan_code.should.equal(this.plan_code)
         })
 
         it('should update immediately', function() {
           const updateOptions = this.RecurlyWrapper.updateSubscription
             .args[0][1]
-          return updateOptions.timeframe.should.equal('now')
+          updateOptions.timeframe.should.equal('now')
         })
 
         it('should sync the new subscription to the user', function() {
@@ -216,7 +203,7 @@ describe('SubscriptionHandler', function() {
           this.SubscriptionUpdater.syncSubscription.args[0][0].should.deep.equal(
             this.activeRecurlySubscription
           )
-          return this.SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal(
+          this.SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal(
             this.user._id
           )
         })
@@ -230,7 +217,7 @@ describe('SubscriptionHandler', function() {
           null,
           false
         )
-        return this.SubscriptionHandler.updateSubscription(
+        this.SubscriptionHandler.updateSubscription(
           this.user,
           this.plan_code,
           null,
@@ -240,7 +227,7 @@ describe('SubscriptionHandler', function() {
 
       it('should redirect to the subscription dashboard', function() {
         this.RecurlyWrapper.updateSubscription.called.should.equal(false)
-        return this.SubscriptionHandler.syncSubscriptionToUser.called.should.equal(
+        this.SubscriptionHandler.syncSubscriptionToUser.called.should.equal(
           false
         )
       })
@@ -256,7 +243,7 @@ describe('SubscriptionHandler', function() {
           true,
           this.subscription
         )
-        return this.SubscriptionHandler.updateSubscription(
+        this.SubscriptionHandler.updateSubscription(
           this.user,
           this.plan_code,
           this.coupon_code,
@@ -265,7 +252,7 @@ describe('SubscriptionHandler', function() {
       })
 
       it('should get the users account', function() {
-        return this.RecurlyWrapper.getSubscription
+        this.RecurlyWrapper.getSubscription
           .calledWith(this.activeRecurlySubscription.uuid)
           .should.equal(true)
       })
@@ -277,7 +264,7 @@ describe('SubscriptionHandler', function() {
             this.coupon_code
           )
           .should.equal(true)
-        return done()
+        done()
       })
 
       it('should update the subscription', function() {
@@ -285,7 +272,7 @@ describe('SubscriptionHandler', function() {
           .calledWith(this.subscription.recurlySubscription_id)
           .should.equal(true)
         const updateOptions = this.RecurlyWrapper.updateSubscription.args[0][1]
-        return updateOptions.plan_code.should.equal(this.plan_code)
+        updateOptions.plan_code.should.equal(this.plan_code)
       })
     })
   })
@@ -299,11 +286,11 @@ describe('SubscriptionHandler', function() {
           false,
           this.subscription
         )
-        return this.SubscriptionHandler.cancelSubscription(this.user, done)
+        this.SubscriptionHandler.cancelSubscription(this.user, done)
       })
 
       it('should redirect to the subscription dashboard', function() {
-        return this.RecurlyWrapper.cancelSubscription.called.should.equal(false)
+        this.RecurlyWrapper.cancelSubscription.called.should.equal(false)
       })
     })
 
@@ -315,18 +302,18 @@ describe('SubscriptionHandler', function() {
           true,
           this.subscription
         )
-        return this.SubscriptionHandler.cancelSubscription(this.user, done)
+        this.SubscriptionHandler.cancelSubscription(this.user, done)
       })
 
       it('should cancel the subscription', function() {
         this.RecurlyWrapper.cancelSubscription.called.should.equal(true)
-        return this.RecurlyWrapper.cancelSubscription
+        this.RecurlyWrapper.cancelSubscription
           .calledWith(this.subscription.recurlySubscription_id)
           .should.equal(true)
       })
 
       it('should trigger the cancel subscription event', function() {
-        return this.Events.emit
+        this.Events.emit
           .calledWith('cancelSubscription', this.user._id)
           .should.equal(true)
       })
@@ -342,17 +329,15 @@ describe('SubscriptionHandler', function() {
           false,
           this.subscription
         )
-        return this.SubscriptionHandler.reactivateSubscription(this.user, done)
+        this.SubscriptionHandler.reactivateSubscription(this.user, done)
       })
 
       it('should redirect to the subscription dashboard', function() {
-        return this.RecurlyWrapper.reactivateSubscription.called.should.equal(
-          false
-        )
+        this.RecurlyWrapper.reactivateSubscription.called.should.equal(false)
       })
 
       it('should not send a notification email', function() {
-        return sinon.assert.notCalled(this.EmailHandler.sendEmail)
+        sinon.assert.notCalled(this.EmailHandler.sendEmail)
       })
     })
 
@@ -364,18 +349,18 @@ describe('SubscriptionHandler', function() {
           true,
           this.subscription
         )
-        return this.SubscriptionHandler.reactivateSubscription(this.user, done)
+        this.SubscriptionHandler.reactivateSubscription(this.user, done)
       })
 
       it('should reactivate the subscription', function() {
         this.RecurlyWrapper.reactivateSubscription.called.should.equal(true)
-        return this.RecurlyWrapper.reactivateSubscription
+        this.RecurlyWrapper.reactivateSubscription
           .calledWith(this.subscription.recurlySubscription_id)
           .should.equal(true)
       })
 
       it('should send a notification email', function() {
-        return sinon.assert.calledWith(
+        sinon.assert.calledWith(
           this.EmailHandler.sendEmail,
           'reactivatedSubscription'
         )
@@ -388,11 +373,11 @@ describe('SubscriptionHandler', function() {
       beforeEach(function(done) {
         this.user.id = this.activeRecurlySubscription.account.account_code
 
-        this.User.findById = (userId, callback) => {
+        this.User.findById = (userId, projection, callback) => {
           userId.should.equal(this.user.id)
-          return callback(null, this.user)
+          callback(null, this.user)
         }
-        return this.SubscriptionHandler.syncSubscription(
+        this.SubscriptionHandler.syncSubscription(
           this.activeRecurlySubscription,
           {},
           done
@@ -400,14 +385,14 @@ describe('SubscriptionHandler', function() {
       })
 
       it('should request the affected subscription from the API', function() {
-        return this.RecurlyWrapper.getSubscription
+        this.RecurlyWrapper.getSubscription
           .calledWith(this.activeRecurlySubscription.uuid)
           .should.equal(true)
       })
 
       it('should request the account details of the subscription', function() {
         const options = this.RecurlyWrapper.getSubscription.args[0][1]
-        return options.includeAccount.should.equal(true)
+        options.includeAccount.should.equal(true)
       })
 
       it('should sync the subscription to the user', function() {
@@ -415,7 +400,7 @@ describe('SubscriptionHandler', function() {
         this.SubscriptionUpdater.syncSubscription.args[0][0].should.deep.equal(
           this.activeRecurlySubscription
         )
-        return this.SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal(
+        this.SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal(
           this.user._id
         )
       })
@@ -483,51 +468,49 @@ describe('SubscriptionHandler', function() {
         .stub()
         .yields(null, this.subscriptions)
       this.SubscriptionUpdater.syncSubscription = sinon.stub().yields()
-      return (this.callback = sinon.stub())
+      this.callback = sinon.stub()
     })
 
     describe('with no subscription in recurly', function() {
       beforeEach(function() {
         this.subscriptions.push((this.subscription = { mock: 'subscription' }))
-        return this.SubscriptionHandler.validateNoSubscriptionInRecurly(
+        this.SubscriptionHandler.validateNoSubscriptionInRecurly(
           this.user_id,
           this.callback
         )
       })
 
       it('should call RecurlyWrapper.listAccountActiveSubscriptions with the user id', function() {
-        return this.RecurlyWrapper.listAccountActiveSubscriptions
+        this.RecurlyWrapper.listAccountActiveSubscriptions
           .calledWith(this.user_id)
           .should.equal(true)
       })
 
       it('should sync the subscription', function() {
-        return this.SubscriptionUpdater.syncSubscription
+        this.SubscriptionUpdater.syncSubscription
           .calledWith(this.subscription, this.user_id)
           .should.equal(true)
       })
 
       it('should call the callback with valid == false', function() {
-        return this.callback.calledWith(null, false).should.equal(true)
+        this.callback.calledWith(null, false).should.equal(true)
       })
     })
 
     describe('with a subscription in recurly', function() {
       beforeEach(function() {
-        return this.SubscriptionHandler.validateNoSubscriptionInRecurly(
+        this.SubscriptionHandler.validateNoSubscriptionInRecurly(
           this.user_id,
           this.callback
         )
       })
 
       it('should not sync the subscription', function() {
-        return this.SubscriptionUpdater.syncSubscription.called.should.equal(
-          false
-        )
+        this.SubscriptionUpdater.syncSubscription.called.should.equal(false)
       })
 
       it('should call the callback with valid == true', function() {
-        return this.callback.calledWith(null, true).should.equal(true)
+        this.callback.calledWith(null, true).should.equal(true)
       })
     })
   })