Merge pull request #4071 from overleaf/ab-subscription-decaf-cleanup

Subscription controller decaf cleanup

GitOrigin-RevId: 79b8adfabe30e4557a95b1aad71a5162e6f42cce

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

@@ -1,4 +1,3 @@
-let LimitationsManager
 const OError = require('@overleaf/o-error')
 const logger = require('logger-sharelatex')
 const ProjectGetter = require('../Project/ProjectGetter')
@@ -9,8 +8,9 @@ const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter')
 const CollaboratorsInvitesHandler = require('../Collaborators/CollaboratorsInviteHandler')
 const V1SubscriptionManager = require('./V1SubscriptionManager')
 const { V1ConnectionError } = require('../Errors/Errors')
+const { promisifyAll } = require('../../util/promises')
 
-module.exports = LimitationsManager = {
+const LimitationsManager = {
   allowedNumberOfCollaboratorsInProject(projectId, callback) {
     ProjectGetter.getProject(
       projectId,
@@ -226,3 +226,12 @@ module.exports = LimitationsManager = {
     )
   },
 }
+
+LimitationsManager.promises = promisifyAll(LimitationsManager, {
+  multiResult: {
+    userHasV2Subscription: ['hasSubscription', 'subscription'],
+    userIsMemberOfGroupSubscription: ['isMember', 'subscriptions'],
+    hasGroupMembersLimitReached: ['limitReached', 'subscription'],
+  },
+})
+module.exports = LimitationsManager

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

@@ -1,20 +1,3 @@
-/* eslint-disable
-    camelcase,
-    node/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
- * DS103: Rewrite code to no longer use __guard__
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let SubscriptionController
 const AuthenticationController = require('../Authentication/AuthenticationController')
 const SubscriptionHandler = require('./SubscriptionHandler')
 const PlansLocator = require('./PlansLocator')
@@ -24,7 +7,6 @@ const RecurlyWrapper = require('./RecurlyWrapper')
 const Settings = require('settings-sharelatex')
 const logger = require('logger-sharelatex')
 const GeoIpLookup = require('../../infrastructure/GeoIpLookup')
-const UserGetter = require('../User/UserGetter')
 const FeaturesUpdater = require('./FeaturesUpdater')
 const planFeatures = require('./planFeatures')
 const GroupPlansData = require('./GroupPlansData')
@@ -34,93 +16,64 @@ const HttpErrorHandler = require('../Errors/HttpErrorHandler')
 const SubscriptionErrors = require('./Errors')
 const SplitTestHandler = require('../SplitTests/SplitTestHandler')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
+const { expressify } = require('../../util/promises')
 const OError = require('@overleaf/o-error')
 const _ = require('lodash')
 
 const SUBSCRIPTION_PAGE_SPLIT_TEST = 'subscription-page'
 
-module.exports = SubscriptionController = {
+async function plansPage(req, res) {
-  plansPage(req, res, next) {
   const plans = SubscriptionViewModelBuilder.buildPlansList()
-    let currentUser = null
 
-    return GeoIpLookup.getCurrencyCode(
+  const recommendedCurrency = await GeoIpLookup.promises.getCurrencyCode(
-      (req.query != null ? req.query.ip : undefined) || req.ip,
+    (req.query ? req.query.ip : undefined) || req.ip
-      function (err, recomendedCurrency) {
+  )
-        if (err != null) {
+
-          return next(err)
-        }
-        const render = () =>
   res.render('subscriptions/plans', {
     title: 'plans_and_pricing',
     plans,
     gaExperiments: Settings.gaExperiments.plansPage,
     gaOptimize: true,
-            recomendedCurrency,
+    recomendedCurrency: recommendedCurrency,
     planFeatures,
     groupPlans: GroupPlansData,
   })
-        const user_id = AuthenticationController.getLoggedInUserId(req)
-        if (user_id != null) {
-          return UserGetter.getUser(
-            user_id,
-            { signUpDate: 1 },
-            function (err, user) {
-              if (err != null) {
-                return next(err)
 }
-              currentUser = user
-              return render()
-            }
-          )
-        } else {
-          return render()
-        }
-      }
-    )
-  },
 
 // get to show the recurly.js page
-  paymentPage(req, res, next) {
+async function paymentPage(req, res) {
   const user = AuthenticationController.getSessionUser(req)
   const plan = PlansLocator.findLocalPlanInSettings(req.query.planCode)
   if (!plan) {
     return HttpErrorHandler.unprocessableEntity(req, res, 'Plan not found')
   }
-    return LimitationsManager.userHasV1OrV2Subscription(
+  const hasSubscription = await LimitationsManager.promises.userHasV1OrV2Subscription(
-      user,
+    user
-      function (err, hasSubscription) {
+  )
-        if (err != null) {
-          return next(err)
-        }
   if (hasSubscription) {
-          return res.redirect('/user/subscription?hasSubscription=true')
+    res.redirect('/user/subscription?hasSubscription=true')
   } else {
     // LimitationsManager.userHasV2Subscription only checks Mongo. Double check with
     // Recurly as well at this point (we don't do this most places for speed).
-          return SubscriptionHandler.validateNoSubscriptionInRecurly(
+    const valid = await SubscriptionHandler.promises.validateNoSubscriptionInRecurly(
-            user._id,
+      user._id
-            function (error, valid) {
+    )
-              if (error != null) {
-                return next(error)
-              }
     if (!valid) {
       res.redirect('/user/subscription?hasSubscription=true')
     } else {
-                let currency =
+      let currency = req.query.currency
-                  req.query.currency != null
         ? req.query.currency.toUpperCase()
         : undefined
-                return GeoIpLookup.getCurrencyCode(
+      const {
-                  (req.query != null ? req.query.ip : undefined) || req.ip,
+        recomendedCurrency: recommendedCurrency,
-                  function (err, recomendedCurrency, countryCode) {
+        countryCode,
-                    if (err != null) {
+      } = await GeoIpLookup.promises.getCurrencyCode(
-                      return next(err)
+        (req.query ? req.query.ip : undefined) || req.ip
+      )
+      if (recommendedCurrency && currency == null) {
+        currency = recommendedCurrency
       }
-                    if (recomendedCurrency != null && currency == null) {
+      res.render('subscriptions/new', {
-                      currency = recomendedCurrency
-                    }
-                    return res.render('subscriptions/new', {
         title: 'subscribe',
         currency,
         countryCode,
@@ -135,21 +88,15 @@ module.exports = SubscriptionController = {
         gaOptimize: true,
       })
     }
-                )
   }
 }
-          )
-        }
-      }
-    )
-  },
 
-  userSubscriptionPage(req, res, next) {
+function userSubscriptionPage(req, res, next) {
   const user = AuthenticationController.getSessionUser(req)
-    return SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel(
+  SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel(
     user,
     function (error, results) {
-        if (error != null) {
+      if (error) {
         return next(error)
       }
       const {
@@ -161,10 +108,10 @@ module.exports = SubscriptionController = {
         managedPublishers,
         v1SubscriptionStatus,
       } = results
-        return LimitationsManager.userHasV1OrV2Subscription(
+      LimitationsManager.userHasV1OrV2Subscription(
         user,
         function (err, hasSubscription) {
-            if (error != null) {
+          if (error) {
             return next(error)
           }
           const fromPlansPage = req.query.hasSubscription
@@ -176,8 +123,7 @@ module.exports = SubscriptionController = {
           if (
             personalSubscription ||
             hasSubscription ||
-              (memberGroupSubscriptions &&
+            (memberGroupSubscriptions && memberGroupSubscriptions.length > 0) ||
-                memberGroupSubscriptions.length > 0) ||
             (confirmedMemberAffiliations &&
               confirmedMemberAffiliations.length > 0 &&
               _.find(confirmedMemberAffiliations, affiliation => {
@@ -193,14 +139,10 @@ module.exports = SubscriptionController = {
             if (testSegmentation.enabled) {
               subscriptionCopy = testSegmentation.variant
 
-                AnalyticsManager.recordEvent(
+              AnalyticsManager.recordEvent(user._id, 'subscription-page-view', {
-                  user._id,
-                  'subscription-page-view',
-                  {
                 splitTestId: SUBSCRIPTION_PAGE_SPLIT_TEST,
                 splitTestVariantId: testSegmentation.variant,
-                  }
+              })
-                )
             } else {
               AnalyticsManager.recordEvent(user._id, 'subscription-page-view')
             }
@@ -221,14 +163,14 @@ module.exports = SubscriptionController = {
             managedPublishers,
             v1SubscriptionStatus,
           }
-            return res.render('subscriptions/dashboard', data)
+          res.render('subscriptions/dashboard', data)
         }
       )
     }
   )
-  },
+}
 
-  createSubscription(req, res, next) {
+function createSubscription(req, res, next) {
   const user = AuthenticationController.getSessionUser(req)
   const recurlyTokenIds = {
     billing: req.body.recurly_token_id,
@@ -237,10 +179,10 @@ module.exports = SubscriptionController = {
   }
   const { subscriptionDetails } = req.body
 
-    return LimitationsManager.userHasV1OrV2Subscription(
+  LimitationsManager.userHasV1OrV2Subscription(
     user,
     function (err, hasSubscription) {
-        if (err != null) {
+      if (err) {
         return next(err)
       }
       if (hasSubscription) {
@@ -261,49 +203,50 @@ module.exports = SubscriptionController = {
             err instanceof Errors.InvalidError
           ) {
             logger.error({ err }, 'recurly transaction error, potential 422')
-              return HttpErrorHandler.unprocessableEntity(
+            HttpErrorHandler.unprocessableEntity(
               req,
               res,
               err.message,
               OError.getFullInfo(err).public
             )
-            }
+          } else {
-
             logger.warn(
               { err, user_id: user._id },
               'something went wrong creating subscription'
             )
             next(err)
           }
+        }
       )
     }
   )
-  },
+}
 
-  successful_subscription(req, res, next) {
+function successfulSubscription(req, res, next) {
   const user = AuthenticationController.getSessionUser(req)
   return SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel(
     user,
     function (error, { personalSubscription }) {
-        if (error != null) {
+      if (error) {
         return next(error)
       }
       if (personalSubscription == null) {
-          return res.redirect('/user/subscription/plans')
+        res.redirect('/user/subscription/plans')
-        }
+      } else {
-        return res.render('subscriptions/successful_subscription', {
+        res.render('subscriptions/successful_subscription', {
           title: 'thank_you',
           personalSubscription,
         })
       }
+    }
   )
-  },
+}
 
-  cancelSubscription(req, res, next) {
+function cancelSubscription(req, res, next) {
   const user = AuthenticationController.getSessionUser(req)
   logger.log({ user_id: user._id }, 'canceling subscription')
-    return SubscriptionHandler.cancelSubscription(user, function (err) {
+  SubscriptionHandler.cancelSubscription(user, function (err) {
-      if (err != null) {
+    if (err) {
       OError.tag(err, 'something went wrong canceling subscription', {
         user_id: user._id,
       })
@@ -311,66 +254,59 @@ module.exports = SubscriptionController = {
     }
     // Note: this redirect isn't used in the main flow as the redirection is
     // handled by Angular
-      return res.redirect('/user/subscription/canceled')
+    res.redirect('/user/subscription/canceled')
   })
-  },
+}
 
-  canceledSubscription(req, res, next) {
+function canceledSubscription(req, res, next) {
-    const user = AuthenticationController.getSessionUser(req)
   return res.render('subscriptions/canceled_subscription', {
     title: 'subscription_canceled',
   })
-  },
+}
 
-  cancelV1Subscription(req, res, next) {
+function cancelV1Subscription(req, res, next) {
-    const user_id = AuthenticationController.getLoggedInUserId(req)
+  const userId = AuthenticationController.getLoggedInUserId(req)
-    logger.log({ user_id }, 'canceling v1 subscription')
+  logger.log({ userId }, 'canceling v1 subscription')
-    return V1SubscriptionManager.cancelV1Subscription(user_id, function (err) {
+  V1SubscriptionManager.cancelV1Subscription(userId, function (err) {
-      if (err != null) {
+    if (err) {
       OError.tag(err, 'something went wrong canceling v1 subscription', {
-          user_id,
+        userId,
       })
       return next(err)
     }
-      return res.redirect('/user/subscription')
+    res.redirect('/user/subscription')
   })
-  },
+}
 
-  updateSubscription(req, res, next) {
+function updateSubscription(req, res, next) {
-    const _origin =
+  const origin = req && req.query ? req.query.origin : null
-      __guard__(req != null ? req.query : undefined, x => x.origin) || null
   const user = AuthenticationController.getSessionUser(req)
   const planCode = req.body.plan_code
   if (planCode == null) {
     const err = new Error('plan_code is not defined')
     logger.warn(
-        { user_id: user._id, err, planCode, origin: _origin, body: req.body },
+      { user_id: user._id, err, planCode, origin, body: req.body },
       '[Subscription] error in updateSubscription form'
     )
     return next(err)
   }
   logger.log({ planCode, user_id: user._id }, 'updating subscription')
-    return SubscriptionHandler.updateSubscription(
+  SubscriptionHandler.updateSubscription(user, planCode, null, function (err) {
-      user,
+    if (err) {
-      planCode,
-      null,
-      function (err) {
-        if (err != null) {
       OError.tag(err, 'something went wrong updating subscription', {
         user_id: user._id,
       })
       return next(err)
     }
-        return res.redirect('/user/subscription')
+    res.redirect('/user/subscription')
+  })
 }
-    )
-  },
 
-  cancelPendingSubscriptionChange(req, res, next) {
+function cancelPendingSubscriptionChange(req, res, next) {
   const user = AuthenticationController.getSessionUser(req)
   logger.log({ user_id: user._id }, 'canceling pending subscription change')
   SubscriptionHandler.cancelPendingSubscriptionChange(user, function (err) {
-      if (err != null) {
+    if (err) {
       OError.tag(
         err,
         'something went wrong canceling pending subscription change',
@@ -382,9 +318,9 @@ module.exports = SubscriptionController = {
     }
     res.redirect('/user/subscription')
   })
-  },
+}
 
-  updateAccountEmailAddress(req, res, next) {
+function updateAccountEmailAddress(req, res, next) {
   const user = AuthenticationController.getSessionUser(req)
   RecurlyWrapper.updateAccountEmailAddress(
     user._id,
@@ -396,13 +332,13 @@ module.exports = SubscriptionController = {
       res.sendStatus(200)
     }
   )
-  },
+}
 
-  reactivateSubscription(req, res, next) {
+function reactivateSubscription(req, res, next) {
   const user = AuthenticationController.getSessionUser(req)
   logger.log({ user_id: user._id }, 'reactivating subscription')
   SubscriptionHandler.reactivateSubscription(user, function (err) {
-      if (err != null) {
+    if (err) {
       OError.tag(err, 'something went wrong reactivating subscription', {
         user_id: user._id,
       })
@@ -410,9 +346,9 @@ module.exports = SubscriptionController = {
     }
     res.redirect('/user/subscription')
   })
-  },
+}
 
-  recurlyCallback(req, res, next) {
+function recurlyCallback(req, res, next) {
   logger.log({ data: req.body }, 'received recurly callback')
   const event = Object.keys(req.body)[0]
   const eventData = req.body[event]
@@ -424,72 +360,69 @@ module.exports = SubscriptionController = {
     ].includes(event)
   ) {
     const recurlySubscription = eventData.subscription
-      return SubscriptionHandler.syncSubscription(
+    SubscriptionHandler.syncSubscription(
       recurlySubscription,
       { ip: req.ip },
       function (err) {
-          if (err != null) {
+        if (err) {
           return next(err)
         }
-          return res.sendStatus(200)
+        res.sendStatus(200)
       }
     )
   } else if (event === 'billing_info_updated_notification') {
     const recurlyAccountCode = eventData.account.account_code
-      return SubscriptionHandler.attemptPaypalInvoiceCollection(
+    SubscriptionHandler.attemptPaypalInvoiceCollection(
       recurlyAccountCode,
       function (err) {
         if (err) {
           return next(err)
         }
-          return res.sendStatus(200)
+        res.sendStatus(200)
       }
     )
   } else {
-      return res.sendStatus(200)
+    res.sendStatus(200)
+  }
 }
-  },
 
-  renderUpgradeToAnnualPlanPage(req, res, next) {
+function renderUpgradeToAnnualPlanPage(req, res, next) {
   const user = AuthenticationController.getSessionUser(req)
-    return LimitationsManager.userHasV2Subscription(
+  LimitationsManager.userHasV2Subscription(
     user,
     function (err, hasSubscription, subscription) {
       let planName
-        if (err != null) {
+      if (err) {
         return next(err)
       }
-        const planCode =
+      const planCode = subscription
-          subscription != null ? subscription.planCode.toLowerCase() : undefined
+        ? subscription.planCode.toLowerCase()
-        if (
+        : undefined
-          (planCode != null ? planCode.indexOf('annual') : undefined) !== -1
+      if ((planCode ? planCode.indexOf('annual') : undefined) !== -1) {
-        ) {
         planName = 'annual'
-        } else if (
+      } else if ((planCode ? planCode.indexOf('student') : undefined) !== -1) {
-          (planCode != null ? planCode.indexOf('student') : undefined) !== -1
-        ) {
         planName = 'student'
       } else if (
-          (planCode != null ? planCode.indexOf('collaborator') : undefined) !==
+        (planCode ? planCode.indexOf('collaborator') : undefined) !== -1
-          -1
       ) {
         planName = 'collaborator'
       }
-        if (!hasSubscription) {
+      if (hasSubscription) {
-          return res.redirect('/user/subscription/plans')
+        res.render('subscriptions/upgradeToAnnual', {
-        }
-        return res.render('subscriptions/upgradeToAnnual', {
           title: 'Upgrade to annual',
           planName,
         })
+      } else {
+        res.redirect('/user/subscription/plans')
+      }
     }
   )
-  },
+}
 
-  processUpgradeToAnnualPlan(req, res, next) {
+function processUpgradeToAnnualPlan(req, res, next) {
   const user = AuthenticationController.getSessionUser(req)
   const { planName } = req.body
-    const coupon_code = Settings.coupon_codes.upgradeToAnnualPromo[planName]
+  const couponCode = Settings.coupon_codes.upgradeToAnnualPromo[planName]
   const annualPlanName = `${planName}-annual`
   logger.log(
     { user_id: user._id, planName: annualPlanName },
@@ -498,73 +431,70 @@ module.exports = SubscriptionController = {
   return SubscriptionHandler.updateSubscription(
     user,
     annualPlanName,
-      coupon_code,
+    couponCode,
     function (err) {
-        if (err != null) {
+      if (err) {
         OError.tag(err, 'error updating subscription', {
           user_id: user._id,
         })
         return next(err)
       }
-        return res.sendStatus(200)
+      res.sendStatus(200)
     }
   )
-  },
+}
 
-  extendTrial(req, res, next) {
+async function extendTrial(req, res) {
   const user = AuthenticationController.getSessionUser(req)
-    return LimitationsManager.userHasV2Subscription(
+  const {
-      user,
-      function (err, hasSubscription, subscription) {
-        if (err != null) {
-          return next(err)
-        }
-        return SubscriptionHandler.extendTrial(
     subscription,
-          14,
+  } = await LimitationsManager.promises.userHasV2Subscription(user)
-          function (err) {
-            if (err != null) {
-              return res.sendStatus(500)
-            } else {
-              return res.sendStatus(200)
-            }
-          }
-        )
-      }
-    )
-  },
 
-  recurlyNotificationParser(req, res, next) {
+  try {
+    await SubscriptionHandler.promises.extendTrial(subscription, 14)
+  } catch (error) {
+    return res.sendStatus(500)
+  }
+  res.sendStatus(200)
+}
+
+function recurlyNotificationParser(req, res, next) {
   let xml = ''
   req.on('data', chunk => (xml += chunk))
-    return req.on('end', () =>
+  req.on('end', () =>
     RecurlyWrapper._parseXml(xml, function (error, body) {
-        if (error != null) {
+      if (error) {
         return next(error)
       }
       req.body = body
-        return next()
+      next()
     })
   )
-  },
-
-  refreshUserFeatures(req, res, next) {
-    const { user_id } = req.params
-    return FeaturesUpdater.refreshFeatures(
-      user_id,
-      'subscription-controller',
-      function (error) {
-        if (error != null) {
-          return next(error)
-        }
-        return res.sendStatus(200)
-      }
-    )
-  },
 }
 
-function __guard__(value, transform) {
+async function refreshUserFeatures(req, res) {
-  return typeof value !== 'undefined' && value !== null
+  const { user_id: userId } = req.params
-    ? transform(value)
+  await FeaturesUpdater.promises.refreshFeatures(userId)
-    : undefined
+  res.sendStatus(200)
+}
+
+module.exports = {
+  plansPage: expressify(plansPage),
+  paymentPage: expressify(paymentPage),
+  userSubscriptionPage,
+  createSubscription,
+  successfulSubscription,
+  cancelSubscription,
+  canceledSubscription,
+  cancelV1Subscription,
+  updateSubscription,
+  cancelPendingSubscriptionChange,
+  updateAccountEmailAddress,
+  reactivateSubscription,
+  recurlyCallback,
+  renderUpgradeToAnnualPlanPage,
+  processUpgradeToAnnualPlan,
+  extendTrial: expressify(extendTrial),
+  recurlyNotificationParser,
+  refreshUserFeatures: expressify(refreshUserFeatures),
 }

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

@@ -38,7 +38,7 @@ module.exports = {
     webRouter.get(
       '/user/subscription/thank-you',
       AuthenticationController.requireLogin(),
-      SubscriptionController.successful_subscription
+      SubscriptionController.successfulSubscription
     )
 
     webRouter.get(

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

@@ -10,6 +10,7 @@ const sanitizeHtml = require('sanitize-html')
 const _ = require('underscore')
 const async = require('async')
 const SubscriptionHelper = require('./SubscriptionHelper')
+const { promisify } = require('../../util/promises')
 
 function buildHostedLink(recurlySubscription, type) {
   const recurlySubdomain = Settings.apis.recurly.subdomain
@@ -29,8 +30,7 @@ function buildHostedLink(recurlySubscription, type) {
   }
 }
 
-module.exports = {
+function buildUsersSubscriptionViewModel(user, callback) {
-  buildUsersSubscriptionViewModel(user, callback) {
   async.auto(
     {
       personalSubscription(cb) {
@@ -276,9 +276,9 @@ module.exports = {
       })
     }
   )
-  },
+}
 
-  buildPlansList(currentPlan) {
+function buildPlansList(currentPlan) {
   const { plans } = Settings
 
   const allPlans = {}
@@ -292,10 +292,7 @@ module.exports = {
     result.planCodesChangingAtTermEnd = _.pluck(
       _.filter(plans, plan => {
         if (!plan.hideFromUsers) {
-            return SubscriptionHelper.shouldPlanChangeAtTermEnd(
+          return SubscriptionHelper.shouldPlanChangeAtTermEnd(currentPlan, plan)
-              currentPlan,
-              plan
-            )
         }
       }),
       'planCode'
@@ -329,11 +326,16 @@ module.exports = {
   result.individualAnnualPlans = _.filter(
     plans,
     plan =>
-        !plan.groupPlan &&
+      !plan.groupPlan && plan.annual && plan.planCode.indexOf('student') === -1
-        plan.annual &&
-        plan.planCode.indexOf('student') === -1
   )
 
   return result
+}
+
+module.exports = {
+  buildUsersSubscriptionViewModel,
+  buildPlansList,
+  promises: {
+    buildUsersSubscriptionViewModel: promisify(buildUsersSubscriptionViewModel),
   },
 }

services/web/app/src/infrastructure/GeoIpLookup.js

@@ -1,22 +1,9 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-*/
-// 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
- * DS103: Rewrite code to no longer use __guard__
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let GeoIpLookup
 const request = require('request')
 const settings = require('settings-sharelatex')
 const _ = require('underscore')
 const logger = require('logger-sharelatex')
 const URL = require('url')
+const { promisify } = require('../util/promises')
 
 const currencyMappings = {
   GB: 'GBP',
@@ -61,8 +48,7 @@ const EuroCountries = [
 
 _.each(EuroCountries, country => (currencyMappings[country] = 'EUR'))
 
-module.exports = GeoIpLookup = {
+function getDetails(ip, callback) {
-  getDetails(ip, callback) {
   if (ip == null) {
     const e = new Error('no ip passed')
     return callback(e)
@@ -74,36 +60,38 @@ module.exports = GeoIpLookup = {
     json: true,
   }
   logger.log({ ip, opts }, 'getting geo ip details')
-    return request.get(opts, function (err, res, ipDetails) {
+  request.get(opts, function (err, res, ipDetails) {
-      if (err != null) {
+    if (err) {
       logger.warn({ err, ip }, 'error getting ip details')
     }
-      return callback(err, ipDetails)
+    callback(err, ipDetails)
   })
-  },
+}
 
-  getCurrencyCode(ip, callback) {
+function getCurrencyCode(ip, callback) {
-    return GeoIpLookup.getDetails(ip, function (err, ipDetails) {
+  getDetails(ip, function (err, ipDetails) {
-      if (err != null || ipDetails == null) {
+    if (err || !ipDetails) {
       logger.err(
         { err, ip },
         'problem getting currencyCode for ip, defaulting to USD'
       )
       return callback(null, 'USD')
     }
-      const countryCode = __guard__(
+    const countryCode =
-        ipDetails != null ? ipDetails.country_code : undefined,
+      ipDetails && ipDetails.countryCode
-        x => x.toUpperCase()
+        ? ipDetails.countryCode.toUpperCase()
-      )
+        : undefined
     const currencyCode = currencyMappings[countryCode] || 'USD'
     logger.log({ ip, currencyCode, ipDetails }, 'got currencyCode for ip')
     return callback(err, currencyCode, countryCode)
   })
-  },
 }
 
-function __guard__(value, transform) {
+module.exports = {
-  return typeof value !== 'undefined' && value !== null
+  getDetails,
-    ? transform(value)
+  getCurrencyCode,
-    : undefined
+  promises: {
+    getDetails: promisify(getDetails),
+    getCurrencyCode: promisify(getCurrencyCode),
+  },
 }

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

@@ -61,6 +61,15 @@ describe('SubscriptionController', function () {
       syncSubscription: sinon.stub().yields(),
       attemptPaypalInvoiceCollection: sinon.stub().yields(),
       startFreeTrial: sinon.stub(),
+      promises: {
+        createSubscription: sinon.stub().resolves(),
+        updateSubscription: sinon.stub().resolves(),
+        reactivateSubscription: sinon.stub().resolves(),
+        cancelSubscription: sinon.stub().resolves(),
+        syncSubscription: sinon.stub().resolves(),
+        attemptPaypalInvoiceCollection: sinon.stub().resolves(),
+        startFreeTrial: sinon.stub().resolves(),
+      },
     }
 
     this.PlansLocator = { findLocalPlanInSettings: sinon.stub() }
@@ -69,11 +78,19 @@ describe('SubscriptionController', function () {
       hasPaidSubscription: sinon.stub(),
       userHasV1OrV2Subscription: sinon.stub(),
       userHasV2Subscription: sinon.stub(),
+      promises: {
+        hasPaidSubscription: sinon.stub().resolves(),
+        userHasV1OrV2Subscription: sinon.stub().resolves(),
+        userHasV2Subscription: sinon.stub().resolves(),
+      },
     }
 
     this.SubscriptionViewModelBuilder = {
       buildUsersSubscriptionViewModel: sinon.stub().callsArgWith(1, null, {}),
       buildPlansList: sinon.stub(),
+      promises: {
+        buildUsersSubscriptionViewModel: sinon.stub().resolves({}),
+      },
     }
     this.settings = {
       coupon_codes: {
@@ -90,9 +107,17 @@ describe('SubscriptionController', function () {
       siteUrl: 'http://de.sharelatex.dev:3000',
       gaExperiments: {},
     }
-    this.GeoIpLookup = { getCurrencyCode: sinon.stub() }
+    this.GeoIpLookup = {
+      getCurrencyCode: sinon.stub(),
+      promises: {
+        getCurrencyCode: sinon.stub(),
+      },
+    }
     this.UserGetter = {
       getUser: sinon.stub().callsArgWith(2, null, this.user),
+      promises: {
+        getUser: sinon.stub().resolves(this.user),
+      },
     }
     this.SubscriptionController = SandboxedModule.require(modulePath, {
       requires: {
@@ -135,75 +160,26 @@ describe('SubscriptionController', function () {
   describe('plansPage', function () {
     beforeEach(function () {
       this.req.ip = '1234.3123.3131.333 313.133.445.666 653.5345.5345.534'
-      return this.GeoIpLookup.getCurrencyCode.callsArgWith(
+      return this.GeoIpLookup.promises.getCurrencyCode.resolves(
-        1,
-        null,
         this.stubbedCurrencyCode
       )
     })
-
-    describe('when user is logged in', function (done) {
-      beforeEach(function (done) {
-        this.res.callback = done
-        return this.SubscriptionController.plansPage(this.req, this.res)
-      })
-      it('should fetch the current user', function (done) {
-        this.UserGetter.getUser.callCount.should.equal(1)
-        return done()
-      })
-
-      describe('not dependant on logged in state', function (done) {
-        // these could have been put in 'when user is not logged in' too
-        it('should set the recommended currency from the geoiplookup', function (done) {
-          this.res.renderedVariables.recomendedCurrency.should.equal(
-            this.stubbedCurrencyCode
-          )
-          this.GeoIpLookup.getCurrencyCode
-            .calledWith(this.req.ip)
-            .should.equal(true)
-          return done()
-        })
-        it('should include data for features table', function (done) {
-          this.res.renderedVariables.planFeatures.length.should.not.equal(0)
-          return done()
-        })
-      })
-    })
-
-    describe('when user is not logged in', function (done) {
-      beforeEach(function (done) {
-        this.res.callback = done
-        this.AuthenticationController.getLoggedInUserId = sinon
-          .stub()
-          .returns(null)
-        return this.SubscriptionController.plansPage(this.req, this.res)
-      })
-
-      it('should not fetch the current user', function (done) {
-        this.UserGetter.getUser.callCount.should.equal(0)
-        return done()
-      })
-    })
   })
 
   describe('paymentPage', function () {
     beforeEach(function () {
       this.req.headers = {}
-      this.SubscriptionHandler.validateNoSubscriptionInRecurly = sinon
+      this.SubscriptionHandler.promises.validateNoSubscriptionInRecurly = sinon
         .stub()
-        .yields(null, true)
+        .resolves(true)
-      return this.GeoIpLookup.getCurrencyCode.callsArgWith(
+      return this.GeoIpLookup.promises.getCurrencyCode.resolves({
-        1,
+        recomendedCurrency: this.stubbedCurrencyCode,
-        null,
+      })
-        this.stubbedCurrencyCode
-      )
     })
 
     describe('with a user without a subscription', function () {
       beforeEach(function () {
-        this.LimitationsManager.userHasV1OrV2Subscription.callsArgWith(
+        this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(
-          1,
-          null,
           false
         )
         return this.PlansLocator.findLocalPlanInSettings.returns({})
@@ -213,9 +189,9 @@ describe('SubscriptionController', function () {
         it('should render the new subscription page', function (done) {
           this.res.render = (page, opts) => {
             page.should.equal('subscriptions/new')
-            return done()
+            done()
           }
-          return this.SubscriptionController.paymentPage(this.req, this.res)
+          this.SubscriptionController.paymentPage(this.req, this.res)
         })
       })
     })
@@ -223,9 +199,7 @@ describe('SubscriptionController', function () {
     describe('with a user with subscription', function () {
       it('should redirect to the subscription dashboard', function (done) {
         this.PlansLocator.findLocalPlanInSettings.returns({})
-        this.LimitationsManager.userHasV1OrV2Subscription.callsArgWith(
+        this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(
-          1,
-          null,
           true
         )
         this.res.redirect = url => {
@@ -238,9 +212,7 @@ describe('SubscriptionController', function () {
 
     describe('with an invalid plan code', function () {
       it('should return 422 error - Unprocessable Entity', function (done) {
-        this.LimitationsManager.userHasV1OrV2Subscription.callsArgWith(
+        this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(
-          1,
-          null,
           false
         )
         this.PlansLocator.findLocalPlanInSettings.returns(null)
@@ -252,19 +224,13 @@ describe('SubscriptionController', function () {
             done()
           }
         )
-        return this.SubscriptionController.paymentPage(
+        return this.SubscriptionController.paymentPage(this.req, this.res)
-          this.req,
-          this.res,
-          this.next
-        )
       })
     })
 
     describe('which currency to use', function () {
       beforeEach(function () {
-        this.LimitationsManager.userHasV1OrV2Subscription.callsArgWith(
+        this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(
-          1,
-          null,
           false
         )
         return this.PlansLocator.findLocalPlanInSettings.returns({})
@@ -293,23 +259,21 @@ describe('SubscriptionController', function () {
         this.req.query.currency = null
         this.res.render = (page, opts) => {
           opts.currency.should.equal(this.stubbedCurrencyCode)
-          return done()
+          done()
         }
-        return this.SubscriptionController.paymentPage(this.req, this.res)
+        this.SubscriptionController.paymentPage(this.req, this.res)
       })
     })
 
     describe('with a recurly subscription already', function () {
       it('should redirect to the subscription dashboard', function (done) {
         this.PlansLocator.findLocalPlanInSettings.returns({})
-        this.LimitationsManager.userHasV1OrV2Subscription.callsArgWith(
+        this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(
-          1,
+          false
-          null,
+        )
+        this.SubscriptionHandler.promises.validateNoSubscriptionInRecurly.resolves(
           false
         )
-        this.SubscriptionHandler.validateNoSubscriptionInRecurly = sinon
-          .stub()
-          .yields(null, false)
         this.res.redirect = url => {
           url.should.equal('/user/subscription?hasSubscription=true')
           return done()
@@ -319,7 +283,7 @@ describe('SubscriptionController', function () {
     })
   })
 
-  describe('successful_subscription', function () {
+  describe('successfulSubscription', function () {
     beforeEach(function (done) {
       this.SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel.callsArgWith(
         1,
@@ -327,7 +291,7 @@ describe('SubscriptionController', function () {
         {}
       )
       this.res.callback = done
-      return this.SubscriptionController.successful_subscription(
+      return this.SubscriptionController.successfulSubscription(
         this.req,
         this.res
       )
@@ -359,12 +323,9 @@ describe('SubscriptionController', function () {
       this.res.render = (view, data) => {
         this.data = data
         expect(view).to.equal('subscriptions/dashboard')
-        return done()
+        done()
       }
-      return this.SubscriptionController.userSubscriptionPage(
+      this.SubscriptionController.userSubscriptionPage(this.req, this.res)
-        this.req,
-        this.res
-      )
     })
 
     it('should load the personal, groups and v1 subscriptions', function () {

services/web/test/unit/src/infrastructure/GeoIpLookupTests.js

@@ -1,194 +0,0 @@
-/* eslint-disable
-    node/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 SandboxedModule = require('sandboxed-module')
-const assert = require('assert')
-const path = require('path')
-const sinon = require('sinon')
-const modulePath = path.join(
-  __dirname,
-  '../../../../app/src/infrastructure/GeoIpLookup'
-)
-const { expect } = require('chai')
-
-describe('GeoIpLookup', function () {
-  beforeEach(function () {
-    this.settings = {
-      apis: {
-        geoIpLookup: {
-          url: 'http://lookup.com',
-        },
-      },
-    }
-    this.request = { get: sinon.stub() }
-    this.GeoIpLookup = SandboxedModule.require(modulePath, {
-      requires: {
-        request: this.request,
-        'settings-sharelatex': this.settings,
-      },
-    })
-    this.ipAddress = '123.456.789.123'
-
-    return (this.stubbedResponse = {
-      ip: this.ipAddress,
-      country_code: 'GB',
-      country_name: 'United Kingdom',
-      region_code: 'H9',
-      region_name: 'London, City of',
-      city: 'London',
-      zipcode: 'SE16',
-      latitude: 51.0,
-      longitude: -0.0493,
-      metro_code: '',
-      area_code: '',
-    })
-  })
-
-  describe('getDetails', function () {
-    beforeEach(function () {
-      return this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-    })
-
-    it('should request the details using the ip', function (done) {
-      return this.GeoIpLookup.getDetails(this.ipAddress, err => {
-        this.request.get
-          .calledWith({
-            url: this.settings.apis.geoIpLookup.url + '/' + this.ipAddress,
-            timeout: 1000,
-            json: true,
-          })
-          .should.equal(true)
-        return done()
-      })
-    })
-
-    it('should return the ip details', function (done) {
-      return this.GeoIpLookup.getDetails(
-        this.ipAddress,
-        (err, returnedDetails) => {
-          assert.deepEqual(returnedDetails, this.stubbedResponse)
-          return done()
-        }
-      )
-    })
-
-    it('should take the first ip in the string', function (done) {
-      return this.GeoIpLookup.getDetails(
-        ` ${this.ipAddress} 456.312.452.102 432.433.888.234`,
-        err => {
-          this.request.get
-            .calledWith({
-              url: this.settings.apis.geoIpLookup.url + '/' + this.ipAddress,
-              timeout: 1000,
-              json: true,
-            })
-            .should.equal(true)
-          return done()
-        }
-      )
-    })
-  })
-
-  describe('getCurrencyCode', function () {
-    it('should return GBP for GB country', function (done) {
-      this.GeoIpLookup.getDetails = sinon
-        .stub()
-        .callsArgWith(1, null, this.stubbedResponse)
-      return this.GeoIpLookup.getCurrencyCode(
-        this.ipAddress,
-        (err, currencyCode) => {
-          currencyCode.should.equal('GBP')
-          return done()
-        }
-      )
-    })
-
-    it('should return GBP for gb country', function (done) {
-      this.stubbedResponse.country_code = 'gb'
-      this.GeoIpLookup.getDetails = sinon
-        .stub()
-        .callsArgWith(1, null, this.stubbedResponse)
-      return this.GeoIpLookup.getCurrencyCode(
-        this.ipAddress,
-        (err, currencyCode) => {
-          currencyCode.should.equal('GBP')
-          return done()
-        }
-      )
-    })
-
-    it('should return USD for US', function (done) {
-      this.stubbedResponse.country_code = 'US'
-      this.GeoIpLookup.getDetails = sinon
-        .stub()
-        .callsArgWith(1, null, this.stubbedResponse)
-      return this.GeoIpLookup.getCurrencyCode(
-        this.ipAddress,
-        (err, currencyCode) => {
-          currencyCode.should.equal('USD')
-          return done()
-        }
-      )
-    })
-
-    it('should return EUR for DE', function (done) {
-      this.stubbedResponse.country_code = 'DE'
-      this.GeoIpLookup.getDetails = sinon
-        .stub()
-        .callsArgWith(1, null, this.stubbedResponse)
-      return this.GeoIpLookup.getCurrencyCode(
-        this.ipAddress,
-        (err, currencyCode) => {
-          currencyCode.should.equal('EUR')
-          return done()
-        }
-      )
-    })
-
-    it('should default to USD if there is an error', function (done) {
-      this.GeoIpLookup.getDetails = sinon.stub().callsArgWith(1, 'problem')
-      return this.GeoIpLookup.getCurrencyCode(
-        this.ipAddress,
-        (err, currencyCode) => {
-          currencyCode.should.equal('USD')
-          return done()
-        }
-      )
-    })
-
-    it('should default to USD if there are no details', function (done) {
-      this.GeoIpLookup.getDetails = sinon.stub().callsArgWith(1)
-      return this.GeoIpLookup.getCurrencyCode(
-        this.ipAddress,
-        (err, currencyCode) => {
-          currencyCode.should.equal('USD')
-          return done()
-        }
-      )
-    })
-
-    it('should default to USD if there is no match for their country', function (done) {
-      this.stubbedResponse.country_code = 'Non existant'
-      this.GeoIpLookup.getDetails = sinon
-        .stub()
-        .callsArgWith(1, null, this.stubbedResponse)
-      return this.GeoIpLookup.getCurrencyCode(
-        this.ipAddress,
-        (err, currencyCode) => {
-          currencyCode.should.equal('USD')
-          return done()
-        }
-      )
-    })
-  })
-})