diff --git a/services/web/app/src/Features/Institutions/InstitutionsFeatures.js b/services/web/app/src/Features/Institutions/InstitutionsFeatures.js index dc8bcba353..eaa48dcda9 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsFeatures.js +++ b/services/web/app/src/Features/Institutions/InstitutionsFeatures.js @@ -2,6 +2,7 @@ let InstitutionsFeatures const UserGetter = require('../User/UserGetter') const PlansLocator = require('../Subscription/PlansLocator') const Settings = require('@overleaf/settings') +const { promisifyAll } = require('../../util/promises') module.exports = InstitutionsFeatures = { getInstitutionsFeatures(userId, callback) { @@ -44,3 +45,5 @@ module.exports = InstitutionsFeatures = { }) }, } + +module.exports.promises = promisifyAll(module.exports) diff --git a/services/web/app/src/Features/Institutions/InstitutionsManager.js b/services/web/app/src/Features/Institutions/InstitutionsManager.js index dba36b7ac8..bdd6a790b4 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsManager.js +++ b/services/web/app/src/Features/Institutions/InstitutionsManager.js @@ -7,6 +7,7 @@ const { promises: InstitutionsAPIPromises, } = require('./InstitutionsAPI') const FeaturesUpdater = require('../Subscription/FeaturesUpdater') +const FeaturesHelper = require('../Subscription/FeaturesHelper') const UserGetter = require('../User/UserGetter') const NotificationsBuilder = require('../Notifications/NotificationsBuilder') const SubscriptionLocator = require('../Subscription/SubscriptionLocator') @@ -55,7 +56,7 @@ async function _checkUsersFeatures(userIds) { } users.forEach(user => { - const hasProFeaturesOrBetter = FeaturesUpdater.isFeatureSetBetter( + const hasProFeaturesOrBetter = FeaturesHelper.isFeatureSetBetter( user.features, Settings.features.professional ) diff --git a/services/web/app/src/Features/Referal/ReferalFeatures.js b/services/web/app/src/Features/Referal/ReferalFeatures.js index 14b921bd71..3b353fb185 100644 --- a/services/web/app/src/Features/Referal/ReferalFeatures.js +++ b/services/web/app/src/Features/Referal/ReferalFeatures.js @@ -1,4 +1,5 @@ const _ = require('underscore') +const { promisify } = require('util') const { User } = require('../../models/User') const Settings = require('@overleaf/settings') @@ -47,3 +48,7 @@ module.exports = ReferalFeatures = { return highestBonusLevel }, } + +module.exports.promises = { + getBonusFeatures: promisify(module.exports.getBonusFeatures), +} diff --git a/services/web/app/src/Features/Subscription/FeaturesHelper.js b/services/web/app/src/Features/Subscription/FeaturesHelper.js new file mode 100644 index 0000000000..303a5fde13 --- /dev/null +++ b/services/web/app/src/Features/Subscription/FeaturesHelper.js @@ -0,0 +1,94 @@ +const _ = require('lodash') + +/** + * Merge feature sets coming from different sources + */ +function mergeFeatures(featuresA, featuresB) { + const features = Object.assign({}, featuresA) + for (const key in featuresB) { + // Special merging logic for non-boolean features + if (key === 'compileGroup') { + if ( + features.compileGroup === 'priority' || + featuresB.compileGroup === 'priority' + ) { + features.compileGroup = 'priority' + } else { + features.compileGroup = 'standard' + } + } else if (key === 'collaborators') { + if (features.collaborators === -1 || featuresB.collaborators === -1) { + features.collaborators = -1 + } else { + features.collaborators = Math.max( + features.collaborators || 0, + featuresB.collaborators || 0 + ) + } + } else if (key === 'compileTimeout') { + features.compileTimeout = Math.max( + features.compileTimeout || 0, + featuresB.compileTimeout || 0 + ) + } else { + // Boolean keys, true is better + features[key] = features[key] || featuresB[key] + } + } + return features +} + +/** + * Returns whether `featuresA` is a better feature set than `featuresB` + */ +function isFeatureSetBetter(featuresA, featuresB) { + const mergedFeatures = mergeFeatures(featuresA, featuresB) + return _.isEqual(featuresA, mergedFeatures) +} + +/** + * Return what's missing from `currentFeatures` to equal `expectedFeatures` + */ +function compareFeatures(currentFeatures, expectedFeatures) { + currentFeatures = _.clone(currentFeatures) + expectedFeatures = _.clone(expectedFeatures) + if (_.isEqual(currentFeatures, expectedFeatures)) { + return {} + } + + const mismatchReasons = {} + const featureKeys = [ + ...new Set([ + ...Object.keys(currentFeatures), + ...Object.keys(expectedFeatures), + ]), + ] + featureKeys.sort().forEach(key => { + if (expectedFeatures[key] !== currentFeatures[key]) { + mismatchReasons[key] = expectedFeatures[key] + } + }) + + if (mismatchReasons.compileTimeout) { + // store the compile timeout difference instead of the new compile timeout + mismatchReasons.compileTimeout = + expectedFeatures.compileTimeout - currentFeatures.compileTimeout + } + + if (mismatchReasons.collaborators) { + // store the collaborators difference instead of the new number only + // replace -1 by 100 to make it clearer + if (expectedFeatures.collaborators === -1) { + expectedFeatures.collaborators = 100 + } + if (currentFeatures.collaborators === -1) { + currentFeatures.collaborators = 100 + } + mismatchReasons.collaborators = + expectedFeatures.collaborators - currentFeatures.collaborators + } + + return mismatchReasons +} + +module.exports = { mergeFeatures, isFeatureSetBetter, compareFeatures } diff --git a/services/web/app/src/Features/Subscription/FeaturesUpdater.js b/services/web/app/src/Features/Subscription/FeaturesUpdater.js index b9af77dbac..3e73f84d99 100644 --- a/services/web/app/src/Features/Subscription/FeaturesUpdater.js +++ b/services/web/app/src/Features/Subscription/FeaturesUpdater.js @@ -1,9 +1,10 @@ -const async = require('async') -const OError = require('@overleaf/o-error') -const PlansLocator = require('./PlansLocator') const _ = require('lodash') +const { callbackify } = require('util') +const { callbackifyMultiResult } = require('../../util/promises') +const PlansLocator = require('./PlansLocator') const SubscriptionLocator = require('./SubscriptionLocator') const UserFeaturesUpdater = require('./UserFeaturesUpdater') +const FeaturesHelper = require('./FeaturesHelper') const Settings = require('@overleaf/settings') const logger = require('logger-sharelatex') const ReferalFeatures = require('../Referal/ReferalFeatures') @@ -12,339 +13,187 @@ const InstitutionsFeatures = require('../Institutions/InstitutionsFeatures') const UserGetter = require('../User/UserGetter') const AnalyticsManager = require('../Analytics/AnalyticsManager') -const FeaturesUpdater = { - refreshFeatures(userId, reason, callback = () => {}) { - UserGetter.getUser(userId, { _id: 1, features: 1 }, (err, user) => { - if (err) { - return callback(err) - } - const oldFeatures = _.clone(user.features) - FeaturesUpdater._computeFeatures(userId, (error, features) => { - if (error) { - return callback(error) - } - logger.log({ userId, features }, 'updating user features') +async function refreshFeatures(userId, reason) { + const user = await UserGetter.promises.getUser(userId, { + _id: 1, + features: 1, + }) + const oldFeatures = _.clone(user.features) + const features = await computeFeatures(userId) + logger.log({ userId, features }, 'updating user features') - const matchedFeatureSet = FeaturesUpdater._getMatchedFeatureSet( - features - ) - AnalyticsManager.setUserPropertyForUser( - userId, - 'feature-set', - matchedFeatureSet - ) + const matchedFeatureSet = _getMatchedFeatureSet(features) + AnalyticsManager.setUserPropertyForUser( + userId, + 'feature-set', + matchedFeatureSet + ) - UserFeaturesUpdater.updateFeatures( - userId, - features, - (err, newFeatures, featuresChanged) => { - if (err) { - return callback(err) - } - if (oldFeatures.dropbox === true && features.dropbox === false) { - logger.log({ userId }, '[FeaturesUpdater] must unlink dropbox') - const Modules = require('../../infrastructure/Modules') - Modules.hooks.fire('removeDropbox', userId, reason, err => { - if (err) { - logger.error(err) - } - - return callback(null, newFeatures, featuresChanged) - }) - } else { - return callback(null, newFeatures, featuresChanged) - } - } - ) - }) - }) - }, - - _computeFeatures(userId, callback) { - const jobs = { - individualFeatures(cb) { - FeaturesUpdater._getIndividualFeatures(userId, cb) - }, - groupFeatureSets(cb) { - FeaturesUpdater._getGroupFeatureSets(userId, cb) - }, - institutionFeatures(cb) { - InstitutionsFeatures.getInstitutionsFeatures(userId, cb) - }, - v1Features(cb) { - FeaturesUpdater._getV1Features(userId, cb) - }, - bonusFeatures(cb) { - ReferalFeatures.getBonusFeatures(userId, cb) - }, - featuresOverrides(cb) { - FeaturesUpdater._getFeaturesOverrides(userId, cb) - }, + const { + features: newFeatures, + featuresChanged, + } = await UserFeaturesUpdater.promises.updateFeatures(userId, features) + if (oldFeatures.dropbox === true && features.dropbox === false) { + logger.log({ userId }, '[FeaturesUpdater] must unlink dropbox') + const Modules = require('../../infrastructure/Modules') + try { + await Modules.promises.hooks.fire('removeDropbox', userId, reason) + } catch (err) { + logger.error(err) } - async.series(jobs, function (err, results) { - if (err) { - OError.tag( - err, - 'error getting subscription or group for refreshFeatures', - { - userId, - } - ) - return callback(err) - } + } + return { features: newFeatures, featuresChanged } +} - const { - individualFeatures, - groupFeatureSets, - institutionFeatures, - v1Features, - bonusFeatures, - featuresOverrides, - } = results - logger.log( - { - userId, - individualFeatures, - groupFeatureSets, - institutionFeatures, - v1Features, - bonusFeatures, - featuresOverrides, - }, - 'merging user features' - ) - const featureSets = groupFeatureSets.concat([ - individualFeatures, - institutionFeatures, - v1Features, - bonusFeatures, - featuresOverrides, - ]) - const features = _.reduce( - featureSets, - FeaturesUpdater._mergeFeatures, - Settings.defaultFeatures - ) - callback(null, features) - }) - }, - - _getIndividualFeatures(userId, callback) { - SubscriptionLocator.getUserIndividualSubscription(userId, (err, sub) => - callback(err, FeaturesUpdater._subscriptionToFeatures(sub)) - ) - }, - - _getGroupFeatureSets(userId, callback) { - SubscriptionLocator.getGroupSubscriptionsMemberOf(userId, (err, subs) => - callback(err, (subs || []).map(FeaturesUpdater._subscriptionToFeatures)) - ) - }, - - _getFeaturesOverrides(userId, callback) { - UserGetter.getUser(userId, { featuresOverrides: 1 }, (error, user) => { - if (error) { - return callback(error) - } - if ( - !user || - !user.featuresOverrides || - user.featuresOverrides.length === 0 - ) { - return callback(null, {}) - } - const activeFeaturesOverrides = [] - for (const featuresOverride of user.featuresOverrides) { - if ( - !featuresOverride.expiresAt || - featuresOverride.expiresAt > new Date() - ) { - activeFeaturesOverrides.push(featuresOverride.features) - } - } - const features = _.reduce( - activeFeaturesOverrides, - FeaturesUpdater._mergeFeatures, - {} - ) - callback(null, features) - }) - }, - - _getV1Features(userId, callback) { - V1SubscriptionManager.getPlanCodeFromV1( +async function computeFeatures(userId) { + const individualFeatures = await _getIndividualFeatures(userId) + const groupFeatureSets = await _getGroupFeatureSets(userId) + const institutionFeatures = await InstitutionsFeatures.promises.getInstitutionsFeatures( + userId + ) + const v1Features = await _getV1Features(userId) + const bonusFeatures = await ReferalFeatures.promises.getBonusFeatures(userId) + const featuresOverrides = await _getFeaturesOverrides(userId) + logger.log( + { userId, - function (err, planCode, v1Id) { - if (err) { - if ((err ? err.name : undefined) === 'NotFoundError') { - return callback(null, []) - } - return callback(err) - } + individualFeatures, + groupFeatureSets, + institutionFeatures, + v1Features, + bonusFeatures, + featuresOverrides, + }, + 'merging user features' + ) + const featureSets = groupFeatureSets.concat([ + individualFeatures, + institutionFeatures, + v1Features, + bonusFeatures, + featuresOverrides, + ]) + const features = _.reduce( + featureSets, + FeaturesHelper.mergeFeatures, + Settings.defaultFeatures + ) + return features +} - callback( - err, - FeaturesUpdater._mergeFeatures( - V1SubscriptionManager.getGrandfatheredFeaturesForV1User(v1Id) || {}, - FeaturesUpdater.planCodeToFeatures(planCode) - ) - ) - } - ) - }, +async function _getIndividualFeatures(userId) { + const sub = await SubscriptionLocator.promises.getUserIndividualSubscription( + userId + ) + return _subscriptionToFeatures(sub) +} - _mergeFeatures(featuresA, featuresB) { - const features = Object.assign({}, featuresA) - for (const key in featuresB) { - // Special merging logic for non-boolean features - if (key === 'compileGroup') { - if ( - features.compileGroup === 'priority' || - featuresB.compileGroup === 'priority' - ) { - features.compileGroup = 'priority' - } else { - features.compileGroup = 'standard' - } - } else if (key === 'collaborators') { - if (features.collaborators === -1 || featuresB.collaborators === -1) { - features.collaborators = -1 - } else { - features.collaborators = Math.max( - features.collaborators || 0, - featuresB.collaborators || 0 - ) - } - } else if (key === 'compileTimeout') { - features.compileTimeout = Math.max( - features.compileTimeout || 0, - featuresB.compileTimeout || 0 - ) - } else { - // Boolean keys, true is better - features[key] = features[key] || featuresB[key] - } +async function _getGroupFeatureSets(userId) { + const subs = await SubscriptionLocator.promises.getGroupSubscriptionsMemberOf( + userId + ) + return (subs || []).map(_subscriptionToFeatures) +} + +async function _getFeaturesOverrides(userId) { + const user = await UserGetter.promises.getUser(userId, { + featuresOverrides: 1, + }) + if (!user || !user.featuresOverrides || user.featuresOverrides.length === 0) { + return {} + } + const activeFeaturesOverrides = [] + for (const featuresOverride of user.featuresOverrides) { + if ( + !featuresOverride.expiresAt || + featuresOverride.expiresAt > new Date() + ) { + activeFeaturesOverrides.push(featuresOverride.features) } - return features - }, + } + const features = _.reduce( + activeFeaturesOverrides, + FeaturesHelper.mergeFeatures, + {} + ) + return features +} - isFeatureSetBetter(featuresA, featuresB) { - const mergedFeatures = FeaturesUpdater._mergeFeatures(featuresA, featuresB) - return _.isEqual(featuresA, mergedFeatures) - }, - - _subscriptionToFeatures(subscription) { - return FeaturesUpdater.planCodeToFeatures( - subscription ? subscription.planCode : undefined - ) - }, - - planCodeToFeatures(planCode) { - if (!planCode) { - return {} - } - const plan = PlansLocator.findLocalPlanInSettings(planCode) - if (!plan) { +async function _getV1Features(userId) { + let planCode, v1Id + try { + ;({ + planCode, + v1Id, + } = await V1SubscriptionManager.promises.getPlanCodeFromV1(userId)) + } catch (err) { + if (err.name === 'NotFoundError') { return {} } else { - return plan.features + throw err } - }, - - compareFeatures(currentFeatures, expectedFeatures) { - currentFeatures = _.clone(currentFeatures) - expectedFeatures = _.clone(expectedFeatures) - if (_.isEqual(currentFeatures, expectedFeatures)) { - return {} - } - - const mismatchReasons = {} - const featureKeys = [ - ...new Set([ - ...Object.keys(currentFeatures), - ...Object.keys(expectedFeatures), - ]), - ] - featureKeys.sort().forEach(key => { - if (expectedFeatures[key] !== currentFeatures[key]) { - mismatchReasons[key] = expectedFeatures[key] - } - }) - - if (mismatchReasons.compileTimeout) { - // store the compile timeout difference instead of the new compile timeout - mismatchReasons.compileTimeout = - expectedFeatures.compileTimeout - currentFeatures.compileTimeout - } - - if (mismatchReasons.collaborators) { - // store the collaborators difference instead of the new number only - // replace -1 by 100 to make it clearer - if (expectedFeatures.collaborators === -1) { - expectedFeatures.collaborators = 100 - } - if (currentFeatures.collaborators === -1) { - currentFeatures.collaborators = 100 - } - mismatchReasons.collaborators = - expectedFeatures.collaborators - currentFeatures.collaborators - } - - return mismatchReasons - }, - - doSyncFromV1(v1UserId, callback) { - logger.log({ v1UserId }, '[AccountSync] starting account sync') - return UserGetter.getUser( - { 'overleaf.id': v1UserId }, - { _id: 1 }, - function (err, user) { - if (err != null) { - OError.tag(err, '[AccountSync] error getting user', { - v1UserId, - }) - return callback(err) - } - if ((user != null ? user._id : undefined) == null) { - logger.warn({ v1UserId }, '[AccountSync] no user found for v1 id') - return callback(null) - } - logger.log( - { v1UserId, userId: user._id }, - '[AccountSync] updating user subscription and features' - ) - return FeaturesUpdater.refreshFeatures(user._id, 'sync-v1', callback) - } - ) - }, - - _getMatchedFeatureSet(features) { - for (const [name, featureSet] of Object.entries(Settings.features)) { - if (_.isEqual(features, featureSet)) { - return name - } - } - return 'mixed' - }, + } + return FeaturesHelper.mergeFeatures( + V1SubscriptionManager.getGrandfatheredFeaturesForV1User(v1Id) || {}, + _planCodeToFeatures(planCode) + ) } -const refreshFeaturesPromise = (userId, reason) => - new Promise(function (resolve, reject) { - FeaturesUpdater.refreshFeatures( - userId, - reason, - (error, features, featuresChanged) => { - if (error) { - reject(error) - } else { - resolve({ features, featuresChanged }) - } - } - ) - }) - -FeaturesUpdater.promises = { - refreshFeatures: refreshFeaturesPromise, +function _subscriptionToFeatures(subscription) { + return _planCodeToFeatures(subscription && subscription.planCode) } -module.exports = FeaturesUpdater +function _planCodeToFeatures(planCode) { + if (!planCode) { + return {} + } + const plan = PlansLocator.findLocalPlanInSettings(planCode) + if (!plan) { + return {} + } else { + return plan.features + } +} + +async function doSyncFromV1(v1UserId) { + logger.log({ v1UserId }, '[AccountSync] starting account sync') + const user = await UserGetter.promises.getUser( + { 'overleaf.id': v1UserId }, + { _id: 1 } + ) + if (user == null) { + logger.warn({ v1UserId }, '[AccountSync] no user found for v1 id') + return + } + logger.log( + { v1UserId, userId: user._id }, + '[AccountSync] updating user subscription and features' + ) + return refreshFeatures(user._id, 'sync-v1') +} + +function _getMatchedFeatureSet(features) { + for (const [name, featureSet] of Object.entries(Settings.features)) { + if (_.isEqual(features, featureSet)) { + return name + } + } + return 'mixed' +} + +module.exports = { + computeFeatures: callbackify(computeFeatures), + refreshFeatures: callbackifyMultiResult(refreshFeatures, [ + 'features', + 'featuresChanged', + ]), + doSyncFromV1: callbackifyMultiResult(doSyncFromV1, [ + 'features', + 'featuresChanged', + ]), + promises: { + computeFeatures, + refreshFeatures, + doSyncFromV1, + }, +} diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index 95a1126acc..fbca68e115 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -447,7 +447,7 @@ function recurlyNotificationParser(req, res, next) { async function refreshUserFeatures(req, res) { const { user_id: userId } = req.params - await FeaturesUpdater.promises.refreshFeatures(userId) + await FeaturesUpdater.promises.refreshFeatures(userId, 'acceptance-test') res.sendStatus(200) } diff --git a/services/web/app/src/Features/Subscription/SubscriptionHandler.js b/services/web/app/src/Features/Subscription/SubscriptionHandler.js index 4d7027a261..69c305d184 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionHandler.js +++ b/services/web/app/src/Features/Subscription/SubscriptionHandler.js @@ -1,8 +1,8 @@ const async = require('async') +const { promisify } = require('util') const RecurlyWrapper = require('./RecurlyWrapper') const RecurlyClient = require('./RecurlyClient') const { User } = require('../../models/User') -const { promisifyAll } = require('../../util/promises') const logger = require('logger-sharelatex') const SubscriptionUpdater = require('./SubscriptionUpdater') const LimitationsManager = require('./LimitationsManager') @@ -10,334 +10,352 @@ const EmailHandler = require('../Email/EmailHandler') const PlansLocator = require('./PlansLocator') const SubscriptionHelper = require('./SubscriptionHelper') -const SubscriptionHandler = { - validateNoSubscriptionInRecurly(userId, callback) { - if (callback == null) { - callback = function () {} - } - RecurlyWrapper.listAccountActiveSubscriptions( - userId, - function (error, subscriptions) { - if (subscriptions == null) { - subscriptions = [] - } - if (error != null) { - return callback(error) - } - if (subscriptions.length > 0) { - SubscriptionUpdater.syncSubscription( - subscriptions[0], - userId, - function (error) { - if (error != null) { - return callback(error) - } - callback(null, false) - } - ) - } else { - callback(null, true) - } +function validateNoSubscriptionInRecurly(userId, callback) { + if (callback == null) { + callback = function () {} + } + RecurlyWrapper.listAccountActiveSubscriptions( + userId, + function (error, subscriptions) { + if (subscriptions == null) { + subscriptions = [] } - ) - }, - - createSubscription(user, subscriptionDetails, recurlyTokenIds, 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) + } + if (subscriptions.length > 0) { + SubscriptionUpdater.syncSubscription( + subscriptions[0], + userId, + function (error) { if (error != null) { return callback(error) } - return SubscriptionUpdater.syncSubscription( - recurlySubscription, - user._id, - function (error) { - if (error != null) { - return callback(error) - } - return callback() - } - ) + callback(null, false) } ) + } else { + callback(null, true) } - ) - }, + } + ) +} - updateSubscription(user, planCode, couponCode, callback) { - LimitationsManager.userHasV2Subscription( +function createSubscription( + user, + subscriptionDetails, + recurlyTokenIds, + callback +) { + 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, - 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 (couponCode == null) { - return cb() - } - RecurlyWrapper.getSubscription( - subscription.recurlySubscription_id, - { includeAccount: true }, - function (err, usersSubscription) { - if (err != null) { - return cb(err) - } - RecurlyWrapper.redeemCoupon( - usersSubscription.account.account_code, - couponCode, - cb - ) - } - ) - }, - function (cb) { - let changeAtTermEnd - const currentPlan = PlansLocator.findLocalPlanInSettings( - subscription.planCode - ) - const newPlan = PlansLocator.findLocalPlanInSettings(planCode) - if (currentPlan && newPlan) { - changeAtTermEnd = SubscriptionHelper.shouldPlanChangeAtTermEnd( - currentPlan, - newPlan - ) - } else { - logger.error( - { currentPlan: subscription.planCode, newPlan: planCode }, - 'unable to locate both plans in settings' - ) - return cb( - new Error('unable to locate both plans in settings') - ) - } - const timeframe = changeAtTermEnd ? 'term_end' : 'now' - RecurlyClient.changeSubscriptionByUuid( - subscription.recurlySubscription_id, - { planCode: planCode, timeframe: timeframe }, - function (error, subscriptionChange) { - if (error != null) { - return cb(error) - } - // v2 recurly API wants a UUID, but UUID isn't included in the subscription change response - // we got the UUID from the DB using userHasV2Subscription() - it is the only property - // we need to be able to build a 'recurlySubscription' object for syncSubscription() - SubscriptionHandler.syncSubscription( - { uuid: subscription.recurlySubscription_id }, - user._id, - cb - ) - } - ) - }, - ], - callback - ) - } - } - ) - }, - - cancelPendingSubscriptionChange(user, callback) { - LimitationsManager.userHasV2Subscription( - user, - function (err, hasSubscription, subscription) { - if (err) { - return callback(err) - } - if (hasSubscription) { - RecurlyClient.removeSubscriptionChangeByUuid( - subscription.recurlySubscription_id, - function (error) { - if (error != null) { - return callback(error) - } - callback() - } - ) - } else { - callback() - } - } - ) - }, - - cancelSubscription(user, 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) { - RecurlyClient.cancelSubscriptionByUuid( - subscription.recurlySubscription_id, - function (error) { - if (error != null) { - return callback(error) - } - const emailOpts = { - to: user.email, - first_name: user.first_name, - } - const ONE_HOUR_IN_MS = 1000 * 60 * 60 - setTimeout( - () => - EmailHandler.sendEmail( - 'canceledSubscription', - emailOpts, - err => { - if (err != null) { - logger.warn( - { err }, - 'failed to send confirmation email for subscription cancellation' - ) - } - } - ), - ONE_HOUR_IN_MS - ) - callback() - } - ) - } else { - callback() - } - } - ) - }, - - reactivateSubscription(user, 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) { - RecurlyClient.reactivateSubscriptionByUuid( - subscription.recurlySubscription_id, - function (error) { - if (error != null) { - return callback(error) - } - EmailHandler.sendEmail( - 'reactivatedSubscription', - { to: user.email }, - err => { - if (err != null) { - logger.warn( - { err }, - 'failed to send reactivation confirmation email' - ) - } - } - ) - callback() - } - ) - } else { - callback() - } - } - ) - }, - - syncSubscription(recurlySubscription, requesterData, callback) { - RecurlyWrapper.getSubscription( - recurlySubscription.uuid, - { includeAccount: true }, + subscriptionDetails, + recurlyTokenIds, function (error, recurlySubscription) { if (error != null) { return callback(error) } - User.findById( - recurlySubscription.account.account_code, - { _id: 1 }, - function (error, user) { + return SubscriptionUpdater.syncSubscription( + recurlySubscription, + user._id, + function (error) { 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 callback() } ) } ) - }, + }) +} - // attempt to collect past due invoice for customer. Only do that when a) the - // customer is using Paypal and b) there is only one past due invoice. - // This is used because Recurly doesn't always attempt collection of paast due - // invoices after Paypal billing info were updated. - attemptPaypalInvoiceCollection(recurlyAccountCode, callback) { - RecurlyWrapper.getBillingInfo(recurlyAccountCode, (error, billingInfo) => { - if (error) { +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 (couponCode == null) { + return cb() + } + RecurlyWrapper.getSubscription( + subscription.recurlySubscription_id, + { includeAccount: true }, + function (err, usersSubscription) { + if (err != null) { + return cb(err) + } + RecurlyWrapper.redeemCoupon( + usersSubscription.account.account_code, + couponCode, + cb + ) + } + ) + }, + function (cb) { + let changeAtTermEnd + const currentPlan = PlansLocator.findLocalPlanInSettings( + subscription.planCode + ) + const newPlan = PlansLocator.findLocalPlanInSettings(planCode) + if (currentPlan && newPlan) { + changeAtTermEnd = SubscriptionHelper.shouldPlanChangeAtTermEnd( + currentPlan, + newPlan + ) + } else { + logger.error( + { currentPlan: subscription.planCode, newPlan: planCode }, + 'unable to locate both plans in settings' + ) + return cb(new Error('unable to locate both plans in settings')) + } + const timeframe = changeAtTermEnd ? 'term_end' : 'now' + RecurlyClient.changeSubscriptionByUuid( + subscription.recurlySubscription_id, + { planCode: planCode, timeframe: timeframe }, + function (error, subscriptionChange) { + if (error != null) { + return cb(error) + } + // v2 recurly API wants a UUID, but UUID isn't included in the subscription change response + // we got the UUID from the DB using userHasV2Subscription() - it is the only property + // we need to be able to build a 'recurlySubscription' object for syncSubscription() + syncSubscription( + { uuid: subscription.recurlySubscription_id }, + user._id, + cb + ) + } + ) + }, + ], + callback + ) + } + } + ) +} + +function cancelPendingSubscriptionChange(user, callback) { + LimitationsManager.userHasV2Subscription( + user, + function (err, hasSubscription, subscription) { + if (err) { + return callback(err) + } + if (hasSubscription) { + RecurlyClient.removeSubscriptionChangeByUuid( + subscription.recurlySubscription_id, + function (error) { + if (error != null) { + return callback(error) + } + callback() + } + ) + } else { + callback() + } + } + ) +} + +function cancelSubscription(user, 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) { + RecurlyClient.cancelSubscriptionByUuid( + subscription.recurlySubscription_id, + function (error) { + if (error != null) { + return callback(error) + } + const emailOpts = { + to: user.email, + first_name: user.first_name, + } + const ONE_HOUR_IN_MS = 1000 * 60 * 60 + setTimeout( + () => + EmailHandler.sendEmail( + 'canceledSubscription', + emailOpts, + err => { + if (err != null) { + logger.warn( + { err }, + 'failed to send confirmation email for subscription cancellation' + ) + } + } + ), + ONE_HOUR_IN_MS + ) + callback() + } + ) + } else { + callback() + } + } + ) +} + +function reactivateSubscription(user, 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) { + RecurlyClient.reactivateSubscriptionByUuid( + subscription.recurlySubscription_id, + function (error) { + if (error != null) { + return callback(error) + } + EmailHandler.sendEmail( + 'reactivatedSubscription', + { to: user.email }, + err => { + if (err != null) { + logger.warn( + { err }, + 'failed to send reactivation confirmation email' + ) + } + } + ) + callback() + } + ) + } else { + callback() + } + } + ) +} + +function syncSubscription(recurlySubscription, requesterData, callback) { + RecurlyWrapper.getSubscription( + recurlySubscription.uuid, + { includeAccount: true }, + function (error, recurlySubscription) { + if (error != null) { return callback(error) } - if (!billingInfo.paypal_billing_agreement_id) { - // this is not a Paypal user - return callback() - } - RecurlyWrapper.getAccountPastDueInvoices( - recurlyAccountCode, - (error, pastDueInvoices) => { - if (error) { + User.findById( + recurlySubscription.account.account_code, + { _id: 1 }, + function (error, user) { + if (error != null) { return callback(error) } - if (pastDueInvoices.length !== 1) { - // no past due invoices, or more than one. Ignore. - return callback() + if (user == null) { + return callback(new Error('no user found')) } - RecurlyWrapper.attemptInvoiceCollection( - pastDueInvoices[0].invoice_number, + SubscriptionUpdater.syncSubscription( + recurlySubscription, + user != null ? user._id : undefined, + requesterData, callback ) } ) - }) - }, - - extendTrial(subscription, daysToExend, callback) { - return RecurlyWrapper.extendTrial( - subscription.recurlySubscription_id, - daysToExend, - callback - ) - }, + } + ) } -SubscriptionHandler.promises = promisifyAll(SubscriptionHandler) -module.exports = SubscriptionHandler +// attempt to collect past due invoice for customer. Only do that when a) the +// customer is using Paypal and b) there is only one past due invoice. +// This is used because Recurly doesn't always attempt collection of paast due +// invoices after Paypal billing info were updated. +function attemptPaypalInvoiceCollection(recurlyAccountCode, callback) { + RecurlyWrapper.getBillingInfo(recurlyAccountCode, (error, billingInfo) => { + if (error) { + return callback(error) + } + if (!billingInfo.paypal_billing_agreement_id) { + // this is not a Paypal user + return callback() + } + RecurlyWrapper.getAccountPastDueInvoices( + recurlyAccountCode, + (error, pastDueInvoices) => { + if (error) { + return callback(error) + } + if (pastDueInvoices.length !== 1) { + // no past due invoices, or more than one. Ignore. + return callback() + } + RecurlyWrapper.attemptInvoiceCollection( + pastDueInvoices[0].invoice_number, + callback + ) + } + ) + }) +} + +function extendTrial(subscription, daysToExend, callback) { + return RecurlyWrapper.extendTrial( + subscription.recurlySubscription_id, + daysToExend, + callback + ) +} + +module.exports = { + validateNoSubscriptionInRecurly, + createSubscription, + updateSubscription, + cancelPendingSubscriptionChange, + cancelSubscription, + reactivateSubscription, + syncSubscription, + attemptPaypalInvoiceCollection, + extendTrial, + promises: { + validateNoSubscriptionInRecurly: promisify(validateNoSubscriptionInRecurly), + createSubscription: promisify(createSubscription), + updateSubscription: promisify(updateSubscription), + cancelPendingSubscriptionChange: promisify(cancelPendingSubscriptionChange), + cancelSubscription: promisify(cancelSubscription), + reactivateSubscription: promisify(reactivateSubscription), + syncSubscription: promisify(syncSubscription), + attemptPaypalInvoiceCollection: promisify(attemptPaypalInvoiceCollection), + extendTrial: promisify(extendTrial), + }, +} diff --git a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js index b15e496c2c..008d99d1ab 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js +++ b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js @@ -1,12 +1,10 @@ const { db, ObjectId } = require('../../infrastructure/mongodb') -const OError = require('@overleaf/o-error') -const async = require('async') -const { promisify, callbackify } = require('../../util/promises') +const { callbackify } = require('../../util/promises') const { Subscription } = require('../../models/Subscription') const SubscriptionLocator = require('./SubscriptionLocator') -const UserGetter = require('../User/UserGetter') const PlansLocator = require('./PlansLocator') const FeaturesUpdater = require('./FeaturesUpdater') +const FeaturesHelper = require('./FeaturesHelper') const AnalyticsManager = require('../Analytics/AnalyticsManager') const { DeletedSubscription } = require('../../models/DeletedSubscription') const logger = require('logger-sharelatex') @@ -25,7 +23,7 @@ const logger = require('logger-sharelatex') * * If the subscription is Recurly, we silently do nothing. */ -function updateAdmin(subscription, adminId, callback) { +async function updateAdmin(subscription, adminId) { const query = { _id: ObjectId(subscription._id), customAccount: true, @@ -38,207 +36,115 @@ function updateAdmin(subscription, adminId, callback) { } else { update.$set.manager_ids = [ObjectId(adminId)] } - Subscription.updateOne(query, update, callback) + await Subscription.updateOne(query, update).exec() } -function syncSubscription( +async function syncSubscription( recurlySubscription, adminUserId, - requesterData, - callback + requesterData = {} ) { - if (!callback) { - callback = requesterData - requesterData = {} + let subscription = await SubscriptionLocator.promises.getUsersSubscription( + adminUserId + ) + if (subscription == null) { + subscription = await _createNewSubscription(adminUserId) } - SubscriptionLocator.getUsersSubscription( - adminUserId, - function (err, subscription) { - if (err != null) { - return callback(err) - } - if (subscription != null) { - updateSubscriptionFromRecurly( - recurlySubscription, - subscription, - requesterData, - callback - ) - } else { - _createNewSubscription(adminUserId, function (err, subscription) { - if (err != null) { - return callback(err) - } - updateSubscriptionFromRecurly( - recurlySubscription, - subscription, - requesterData, - callback - ) - }) - } - } + await updateSubscriptionFromRecurly( + recurlySubscription, + subscription, + requesterData ) } -function addUserToGroup(subscriptionId, userId, callback) { - Subscription.updateOne( +async function addUserToGroup(subscriptionId, userId) { + await Subscription.updateOne( { _id: subscriptionId }, - { $addToSet: { member_ids: userId } }, - function (err) { - if (err != null) { - return callback(err) - } - FeaturesUpdater.refreshFeatures(userId, 'add-to-group', function () { - callbackify(_sendUserGroupPlanCodeUserProperty)(userId, callback) - }) - } - ) + { $addToSet: { member_ids: userId } } + ).exec() + await FeaturesUpdater.promises.refreshFeatures(userId, 'add-to-group') + await _sendUserGroupPlanCodeUserProperty(userId) } -function removeUserFromGroup(subscriptionId, userId, callback) { - Subscription.updateOne( +async function removeUserFromGroup(subscriptionId, userId) { + await Subscription.updateOne( { _id: subscriptionId }, - { $pull: { member_ids: userId } }, - function (error) { - if (error) { - OError.tag(error, 'error removing user from group', { - subscriptionId, - userId, - }) - return callback(error) - } - UserGetter.getUser(userId, function (error, user) { - if (error) { - return callback(error) - } - FeaturesUpdater.refreshFeatures( - userId, - 'remove-user-from-group', - function () { - callbackify(_sendUserGroupPlanCodeUserProperty)(userId, callback) - } - ) - }) - } - ) -} - -function removeUserFromAllGroups(userId, callback) { - SubscriptionLocator.getMemberSubscriptions( + { $pull: { member_ids: userId } } + ).exec() + await FeaturesUpdater.promises.refreshFeatures( userId, - function (error, subscriptions) { - if (error) { - return callback(error) - } - if (!subscriptions) { - return callback() - } - const subscriptionIds = subscriptions.map(sub => sub._id) - const removeOperation = { $pull: { member_ids: userId } } - Subscription.updateMany( - { _id: subscriptionIds }, - removeOperation, - function (error) { - if (error) { - OError.tag(error, 'error removing user from groups', { - userId, - subscriptionIds, - }) - return callback(error) - } - UserGetter.getUser(userId, function (error, user) { - if (error) { - return callback(error) - } - FeaturesUpdater.refreshFeatures( - userId, - 'remove-user-from-groups', - function () { - callbackify(_sendUserGroupPlanCodeUserProperty)( - userId, - callback - ) - } - ) - }) - } - ) - } + 'remove-user-from-group' ) + await _sendUserGroupPlanCodeUserProperty(userId) } -function deleteWithV1Id(v1TeamId, callback) { - Subscription.deleteOne({ 'overleaf.id': v1TeamId }, callback) -} - -function deleteSubscription(subscription, deleterData, callback) { - if (callback == null) { - callback = function () {} +async function removeUserFromAllGroups(userId) { + const subscriptions = await SubscriptionLocator.promises.getMemberSubscriptions( + userId + ) + if (subscriptions.length === 0) { + return } - async.series( - [ - cb => - // 1. create deletedSubscription - createDeletedSubscription(subscription, deleterData, cb), - cb => - // 2. remove subscription - Subscription.deleteOne({ _id: subscription._id }, cb), - cb => - // 3. refresh users features - refreshUsersFeatures(subscription, cb), - ], - callback + const subscriptionIds = subscriptions.map(sub => sub._id) + const removeOperation = { $pull: { member_ids: userId } } + await Subscription.updateMany( + { _id: subscriptionIds }, + removeOperation + ).exec() + await FeaturesUpdater.promises.refreshFeatures( + userId, + 'remove-user-from-groups' ) + await _sendUserGroupPlanCodeUserProperty(userId) } -function restoreSubscription(subscriptionId, callback) { - SubscriptionLocator.getDeletedSubscription( - subscriptionId, - function (err, deletedSubscription) { - if (err) { - return callback(err) - } - const subscription = deletedSubscription.subscription - async.series( - [ - cb => - // 1. upsert subscription - db.subscriptions.updateOne( - { _id: subscription._id }, - subscription, - { upsert: true }, - cb - ), - cb => - // 2. refresh users features. Do this before removing the - // subscription so the restore can be retried if this fails - refreshUsersFeatures(subscription, cb), - cb => - // 3. remove deleted subscription - DeletedSubscription.deleteOne( - { 'subscription._id': subscription._id }, - callback - ), - ], - callback - ) - } - ) +async function deleteWithV1Id(v1TeamId) { + await Subscription.deleteOne({ 'overleaf.id': v1TeamId }).exec() } -function refreshUsersFeatures(subscription, callback) { +async function deleteSubscription(subscription, deleterData) { + // 1. create deletedSubscription + await createDeletedSubscription(subscription, deleterData) + + // 2. remove subscription + await Subscription.deleteOne({ _id: subscription._id }).exec() + + // 3. refresh users features + await refreshUsersFeatures(subscription) +} + +async function restoreSubscription(subscriptionId) { + const deletedSubscription = await SubscriptionLocator.promises.getDeletedSubscription( + subscriptionId + ) + const subscription = deletedSubscription.subscription + + // 1. upsert subscription + await db.subscriptions.updateOne({ _id: subscription._id }, subscription, { + upsert: true, + }) + + // 2. refresh users features. Do this before removing the + // subscription so the restore can be retried if this fails + await refreshUsersFeatures(subscription) + + // 3. remove deleted subscription + await DeletedSubscription.deleteOne({ + 'subscription._id': subscription._id, + }).exec() +} + +async function refreshUsersFeatures(subscription) { const userIds = [subscription.admin_id].concat(subscription.member_ids || []) - async.mapSeries( - userIds, - function (userId, cb) { - FeaturesUpdater.refreshFeatures(userId, 'subscription-updater', cb) - }, - callback - ) + for (const userId of userIds) { + await FeaturesUpdater.promises.refreshFeatures( + userId, + 'subscription-updater' + ) + } } -function createDeletedSubscription(subscription, deleterData, callback) { +async function createDeletedSubscription(subscription, deleterData) { subscription.teamInvites = [] subscription.invited_emails = [] const filter = { 'subscription._id': subscription._id } @@ -250,65 +156,56 @@ function createDeletedSubscription(subscription, deleterData, callback) { subscription: subscription, } const options = { upsert: true, new: true, setDefaultsOnInsert: true } - DeletedSubscription.findOneAndUpdate(filter, data, options, callback) + await DeletedSubscription.findOneAndUpdate(filter, data, options).exec() } -function _createNewSubscription(adminUserId, callback) { +async function _createNewSubscription(adminUserId) { const subscription = new Subscription({ admin_id: adminUserId, manager_ids: [adminUserId], }) - subscription.save(err => callback(err, subscription)) + await subscription.save() + return subscription } -function _deleteAndReplaceSubscriptionFromRecurly( +async function _deleteAndReplaceSubscriptionFromRecurly( recurlySubscription, subscription, - requesterData, - callback + requesterData ) { const adminUserId = subscription.admin_id - deleteSubscription(subscription, requesterData, err => { - if (err) { - return callback(err) - } - _createNewSubscription(adminUserId, (err, newSubscription) => { - if (err) { - return callback(err) - } - updateSubscriptionFromRecurly( - recurlySubscription, - newSubscription, - requesterData, - callback - ) - }) - }) + await deleteSubscription(subscription, requesterData) + const newSubscription = await _createNewSubscription(adminUserId) + await updateSubscriptionFromRecurly( + recurlySubscription, + newSubscription, + requesterData + ) } -function updateSubscriptionFromRecurly( +async function updateSubscriptionFromRecurly( recurlySubscription, subscription, - requesterData, - callback + requesterData ) { if (recurlySubscription.state === 'expired') { - return deleteSubscription(subscription, requesterData, callback) + await deleteSubscription(subscription, requesterData) + return } const updatedPlanCode = recurlySubscription.plan.plan_code const plan = PlansLocator.findLocalPlanInSettings(updatedPlanCode) if (plan == null) { - return callback(new Error(`plan code not found: ${updatedPlanCode}`)) + throw new Error(`plan code not found: ${updatedPlanCode}`) } if (!plan.groupPlan && subscription.groupPlan) { // If downgrading from group to individual plan, delete group sub and create a new one - return _deleteAndReplaceSubscriptionFromRecurly( + await _deleteAndReplaceSubscriptionFromRecurly( recurlySubscription, subscription, - requesterData, - callback + requesterData ) + return } subscription.recurlySubscription_id = recurlySubscription.uuid @@ -336,25 +233,22 @@ function updateSubscriptionFromRecurly( }) } } - subscription.save(function (error) { - if (error) { - return callback(error) - } - refreshUsersFeatures(subscription, callback) - }) + await subscription.save() + await refreshUsersFeatures(subscription) } async function _sendUserGroupPlanCodeUserProperty(userId) { try { - const subscriptions = - (await SubscriptionLocator.promises.getMemberSubscriptions(userId)) || [] + const subscriptions = await SubscriptionLocator.promises.getMemberSubscriptions( + userId + ) let bestPlanCode = null let bestFeatures = {} for (const subscription of subscriptions) { const plan = PlansLocator.findLocalPlanInSettings(subscription.planCode) if ( plan && - FeaturesUpdater.isFeatureSetBetter(plan.features, bestFeatures) + FeaturesHelper.isFeatureSetBetter(plan.features, bestFeatures) ) { bestPlanCode = plan.planCode bestFeatures = plan.features @@ -374,28 +268,28 @@ async function _sendUserGroupPlanCodeUserProperty(userId) { } module.exports = { - updateAdmin, - syncSubscription, - deleteSubscription, - createDeletedSubscription, - addUserToGroup, - refreshUsersFeatures, - removeUserFromGroup, - removeUserFromAllGroups, - deleteWithV1Id, - restoreSubscription, - updateSubscriptionFromRecurly, + updateAdmin: callbackify(updateAdmin), + syncSubscription: callbackify(syncSubscription), + deleteSubscription: callbackify(deleteSubscription), + createDeletedSubscription: callbackify(createDeletedSubscription), + addUserToGroup: callbackify(addUserToGroup), + refreshUsersFeatures: callbackify(refreshUsersFeatures), + removeUserFromGroup: callbackify(removeUserFromGroup), + removeUserFromAllGroups: callbackify(removeUserFromAllGroups), + deleteWithV1Id: callbackify(deleteWithV1Id), + restoreSubscription: callbackify(restoreSubscription), + updateSubscriptionFromRecurly: callbackify(updateSubscriptionFromRecurly), promises: { - updateAdmin: promisify(updateAdmin), - syncSubscription: promisify(syncSubscription), - addUserToGroup: promisify(addUserToGroup), - refreshUsersFeatures: promisify(refreshUsersFeatures), - removeUserFromGroup: promisify(removeUserFromGroup), - removeUserFromAllGroups: promisify(removeUserFromAllGroups), - deleteSubscription: promisify(deleteSubscription), - createDeletedSubscription: promisify(createDeletedSubscription), - deleteWithV1Id: promisify(deleteWithV1Id), - restoreSubscription: promisify(restoreSubscription), - updateSubscriptionFromRecurly: promisify(updateSubscriptionFromRecurly), + updateAdmin, + syncSubscription, + addUserToGroup, + refreshUsersFeatures, + removeUserFromGroup, + removeUserFromAllGroups, + deleteSubscription, + createDeletedSubscription, + deleteWithV1Id, + restoreSubscription, + updateSubscriptionFromRecurly, }, } diff --git a/services/web/app/src/Features/Subscription/UserFeaturesUpdater.js b/services/web/app/src/Features/Subscription/UserFeaturesUpdater.js index 51619d6f1a..cc72632b8c 100644 --- a/services/web/app/src/Features/Subscription/UserFeaturesUpdater.js +++ b/services/web/app/src/Features/Subscription/UserFeaturesUpdater.js @@ -1,4 +1,5 @@ const { User } = require('../../models/User') +const { promisifyAll } = require('../../util/promises') function _featuresChanged(newFeatures, featuresBefore) { for (const feature in newFeatures) { @@ -39,3 +40,9 @@ module.exports = { }) }, } + +module.exports.promises = promisifyAll(module.exports, { + multiResult: { + updateFeatures: ['features', 'featuresChanged'], + }, +}) diff --git a/services/web/app/src/Features/Subscription/V1SubscriptionManager.js b/services/web/app/src/Features/Subscription/V1SubscriptionManager.js index ec56934f26..3f2ca77124 100644 --- a/services/web/app/src/Features/Subscription/V1SubscriptionManager.js +++ b/services/web/app/src/Features/Subscription/V1SubscriptionManager.js @@ -16,6 +16,7 @@ const UserGetter = require('../User/UserGetter') const request = require('request') const settings = require('@overleaf/settings') const { V1ConnectionError, NotFoundError } = require('../Errors/Errors') +const { promisifyAll } = require('../../util/promises') module.exports = V1SubscriptionManager = { // Returned planCode = 'v1_pro' | 'v1_pro_plus' | 'v1_student' | 'v1_free' | null @@ -214,3 +215,10 @@ function __guard__(value, transform) { ? transform(value) : undefined } + +module.exports.promises = promisifyAll(module.exports, { + without: ['getGrandfatheredFeaturesForV1User'], + multiResult: { + getPlanCodeFromV1: ['planCode', 'v1Id'], + }, +}) diff --git a/services/web/scripts/refresh_features.js b/services/web/scripts/refresh_features.js index fceb61bbf9..6e54e5a0b0 100644 --- a/services/web/scripts/refresh_features.js +++ b/services/web/scripts/refresh_features.js @@ -3,6 +3,7 @@ const minimist = require('minimist') const _ = require('lodash') const async = require('async') const FeaturesUpdater = require('../app/src/Features/Subscription/FeaturesUpdater') +const FeaturesHelper = require('../app/src/Features/Subscription/FeaturesHelper') const UserFeaturesUpdater = require('../app/src/Features/Subscription/UserFeaturesUpdater') const ScriptLogger = { @@ -54,12 +55,12 @@ const ScriptLogger = { } const checkAndUpdateUser = (user, callback) => - FeaturesUpdater._computeFeatures(user._id, (error, freshFeatures) => { + FeaturesUpdater.computeFeatures(user._id, (error, freshFeatures) => { if (error) { return callback(error) } - const mismatchReasons = FeaturesUpdater.compareFeatures( + const mismatchReasons = FeaturesHelper.compareFeatures( user.features, freshFeatures ) diff --git a/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js b/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js index a07134d375..b8b1dc96d2 100644 --- a/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js +++ b/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js @@ -106,6 +106,8 @@ describe('InstitutionsManager', function () { }, '../Subscription/FeaturesUpdater': { refreshFeatures: this.refreshFeatures, + }, + '../Subscription/FeaturesHelper': { isFeatureSetBetter: (this.isFeatureSetBetter = sinon.stub()), }, '../User/UserGetter': this.UserGetter, diff --git a/services/web/test/unit/src/Subscription/FeaturesHelperTests.js b/services/web/test/unit/src/Subscription/FeaturesHelperTests.js new file mode 100644 index 0000000000..755ad08b88 --- /dev/null +++ b/services/web/test/unit/src/Subscription/FeaturesHelperTests.js @@ -0,0 +1,126 @@ +const SandboxedModule = require('sandboxed-module') +const { expect } = require('chai') + +const MODULE_PATH = '../../../../app/src/Features/Subscription/FeaturesHelper' + +describe('FeaturesHelper', function () { + beforeEach(function () { + this.FeaturesHelper = SandboxedModule.require(MODULE_PATH) + }) + + describe('mergeFeatures', function () { + it('should prefer priority over standard for compileGroup', function () { + expect( + this.FeaturesHelper.mergeFeatures( + { compileGroup: 'priority' }, + { compileGroup: 'standard' } + ) + ).to.deep.equal({ compileGroup: 'priority' }) + expect( + this.FeaturesHelper.mergeFeatures( + { compileGroup: 'standard' }, + { compileGroup: 'priority' } + ) + ).to.deep.equal({ compileGroup: 'priority' }) + expect( + this.FeaturesHelper.mergeFeatures( + { compileGroup: 'priority' }, + { compileGroup: 'priority' } + ) + ).to.deep.equal({ compileGroup: 'priority' }) + expect( + this.FeaturesHelper.mergeFeatures( + { compileGroup: 'standard' }, + { compileGroup: 'standard' } + ) + ).to.deep.equal({ compileGroup: 'standard' }) + }) + + it('should prefer -1 over any other for collaborators', function () { + expect( + this.FeaturesHelper.mergeFeatures( + { collaborators: -1 }, + { collaborators: 10 } + ) + ).to.deep.equal({ collaborators: -1 }) + expect( + this.FeaturesHelper.mergeFeatures( + { collaborators: 10 }, + { collaborators: -1 } + ) + ).to.deep.equal({ collaborators: -1 }) + expect( + this.FeaturesHelper.mergeFeatures( + { collaborators: 4 }, + { collaborators: 10 } + ) + ).to.deep.equal({ collaborators: 10 }) + }) + + it('should prefer the higher of compileTimeout', function () { + expect( + this.FeaturesHelper.mergeFeatures( + { compileTimeout: 20 }, + { compileTimeout: 10 } + ) + ).to.deep.equal({ compileTimeout: 20 }) + expect( + this.FeaturesHelper.mergeFeatures( + { compileTimeout: 10 }, + { compileTimeout: 20 } + ) + ).to.deep.equal({ compileTimeout: 20 }) + }) + + it('should prefer the true over false for other keys', function () { + expect( + this.FeaturesHelper.mergeFeatures({ github: true }, { github: false }) + ).to.deep.equal({ github: true }) + expect( + this.FeaturesHelper.mergeFeatures({ github: false }, { github: true }) + ).to.deep.equal({ github: true }) + expect( + this.FeaturesHelper.mergeFeatures({ github: true }, { github: true }) + ).to.deep.equal({ github: true }) + expect( + this.FeaturesHelper.mergeFeatures({ github: false }, { github: false }) + ).to.deep.equal({ github: false }) + }) + }) + + describe('isFeatureSetBetter', function () { + it('simple comparisons', function () { + const result1 = this.FeaturesHelper.isFeatureSetBetter( + { dropbox: true }, + { dropbox: false } + ) + expect(result1).to.be.true + + const result2 = this.FeaturesHelper.isFeatureSetBetter( + { dropbox: false }, + { dropbox: true } + ) + expect(result2).to.be.false + }) + + it('compound comparisons with same features', function () { + const result1 = this.FeaturesHelper.isFeatureSetBetter( + { collaborators: 9, dropbox: true }, + { collaborators: 10, dropbox: true } + ) + expect(result1).to.be.false + + const result2 = this.FeaturesHelper.isFeatureSetBetter( + { collaborators: -1, dropbox: true }, + { collaborators: 10, dropbox: true } + ) + expect(result2).to.be.true + + const result3 = this.FeaturesHelper.isFeatureSetBetter( + { collaborators: -1, compileTimeout: 60, dropbox: true }, + { collaborators: 10, compileTimeout: 60, dropbox: true } + ) + expect(result3).to.be.true + }) + }) +}) diff --git a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js index 40b27b5773..1e6e6c32f9 100644 --- a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js @@ -1,556 +1,244 @@ const SandboxedModule = require('sandboxed-module') -const { assert, expect } = require('chai') +const { expect } = require('chai') const sinon = require('sinon') -const modulePath = '../../../../app/src/Features/Subscription/FeaturesUpdater' const { ObjectId } = require('mongodb') +const MODULE_PATH = '../../../../app/src/Features/Subscription/FeaturesUpdater' + describe('FeaturesUpdater', function () { beforeEach(function () { - this.user_id = ObjectId().toString() + this.user = { + _id: new ObjectId(), + features: {}, + } + this.v1UserId = 12345 + this.subscriptions = { + individual: { planCode: 'individual-plan' }, + group1: { planCode: 'group-plan-1' }, + group2: { planCode: 'group-plan-2' }, + noDropbox: { planCode: 'no-dropbox' }, + } - this.FeaturesUpdater = SandboxedModule.require(modulePath, { + this.UserFeaturesUpdater = { + promises: { + updateFeatures: sinon + .stub() + .resolves({ features: { some: 'features' }, featuresChanged: true }), + }, + } + + this.SubscriptionLocator = { + promises: { + getUserIndividualSubscription: sinon.stub(), + getGroupSubscriptionsMemberOf: sinon.stub(), + }, + } + this.SubscriptionLocator.promises.getUserIndividualSubscription + .withArgs(this.user._id) + .resolves(this.subscriptions.individual) + this.SubscriptionLocator.promises.getGroupSubscriptionsMemberOf + .withArgs(this.user._id) + .resolves([this.subscriptions.group1, this.subscriptions.group2]) + + this.Settings = { + defaultFeatures: { default: 'features' }, + plans: [ + { planCode: 'individual-plan', features: { individual: 'features' } }, + { planCode: 'group-plan-1', features: { group1: 'features' } }, + { planCode: 'group-plan-2', features: { group2: 'features' } }, + { planCode: 'v1-plan', features: { v1: 'features' } }, + { planCode: 'no-dropbox', features: { dropbox: false } }, + ], + features: { + all: { + default: 'features', + individual: 'features', + group1: 'features', + group2: 'features', + institutions: 'features', + v1: 'features', + grandfathered: 'features', + bonus: 'features', + }, + }, + } + + this.ReferalFeatures = { + promises: { + getBonusFeatures: sinon.stub().resolves({ bonus: 'features' }), + }, + } + this.V1SubscriptionManager = { + getGrandfatheredFeaturesForV1User: sinon.stub(), + promises: { + getPlanCodeFromV1: sinon.stub(), + }, + } + this.V1SubscriptionManager.promises.getPlanCodeFromV1 + .withArgs(this.user._id) + .resolves({ planCode: 'v1-plan', v1Id: this.v1UserId }) + this.V1SubscriptionManager.getGrandfatheredFeaturesForV1User + .withArgs(this.v1UserId) + .returns({ grandfathered: 'features' }) + + this.InstitutionsFeatures = { + promises: { + getInstitutionsFeatures: sinon + .stub() + .resolves({ institutions: 'features' }), + }, + } + + this.UserGetter = { + promises: { + getUser: sinon.stub().resolves(null), + }, + } + this.UserGetter.promises.getUser.withArgs(this.user._id).resolves(this.user) + this.UserGetter.promises.getUser + .withArgs({ 'overleaf.id': this.v1UserId }) + .resolves(this.user) + + this.AnalyticsManager = { + setUserPropertyForUser: sinon.stub(), + } + this.Modules = { + promises: { hooks: { fire: sinon.stub().resolves() } }, + } + this.FeaturesHelper = { + mergeFeatures: sinon.stub().callsFake((a, b) => ({ ...a, ...b })), + } + + this.FeaturesUpdater = SandboxedModule.require(MODULE_PATH, { requires: { - './UserFeaturesUpdater': (this.UserFeaturesUpdater = {}), - './SubscriptionLocator': (this.SubscriptionLocator = {}), - './PlansLocator': (this.PlansLocator = {}), - '@overleaf/settings': (this.Settings = { - features: { - personal: { - collaborators: 1, - dropbox: false, - compileTimeout: 60, - compileGroup: 'standard', - }, - collaborator: { - collaborators: 10, - dropbox: true, - compileTimeout: 240, - compileGroup: 'priority', - }, - professional: { - collaborators: -1, - dropbox: true, - compileTimeout: 240, - compileGroup: 'priority', - }, - }, - }), - '../Referal/ReferalFeatures': (this.ReferalFeatures = {}), - './V1SubscriptionManager': (this.V1SubscriptionManager = {}), - '../Institutions/InstitutionsFeatures': (this.InstitutionsFeatures = {}), - '../User/UserGetter': (this.UserGetter = {}), - '../Analytics/AnalyticsManager': (this.AnalyticsManager = { - setUserPropertyForUser: sinon.stub(), - }), - '../../infrastructure/Modules': (this.Modules = { - hooks: { fire: sinon.stub() }, - }), + './UserFeaturesUpdater': this.UserFeaturesUpdater, + './SubscriptionLocator': this.SubscriptionLocator, + './FeaturesHelper': this.FeaturesHelper, + '@overleaf/settings': this.Settings, + '../Referal/ReferalFeatures': this.ReferalFeatures, + './V1SubscriptionManager': this.V1SubscriptionManager, + '../Institutions/InstitutionsFeatures': this.InstitutionsFeatures, + '../User/UserGetter': this.UserGetter, + '../Analytics/AnalyticsManager': this.AnalyticsManager, + '../../infrastructure/Modules': this.Modules, }, }) }) describe('refreshFeatures', function () { - beforeEach(function () { - this.user = { - _id: this.user_id, - features: {}, - } - this.UserFeaturesUpdater.updateFeatures = sinon - .stub() - .yields(null, { some: 'features' }, true) - this.FeaturesUpdater._getIndividualFeatures = sinon - .stub() - .yields(null, { individual: 'features' }) - this.FeaturesUpdater._getGroupFeatureSets = sinon - .stub() - .yields(null, [{ group: 'features' }, { group: 'features2' }]) - this.InstitutionsFeatures.getInstitutionsFeatures = sinon - .stub() - .yields(null, { institutions: 'features' }) - this.FeaturesUpdater._getV1Features = sinon - .stub() - .yields(null, { v1: 'features' }) - this.ReferalFeatures.getBonusFeatures = sinon - .stub() - .yields(null, { bonus: 'features' }) - this.FeaturesUpdater._mergeFeatures = sinon - .stub() - .returns({ merged: 'features' }) - this.UserGetter.getUser = sinon.stub().yields(null, this.user) - this.callback = sinon.stub() - }) - it('should return features and featuresChanged', function () { - this.FeaturesUpdater.refreshFeatures( - this.user_id, - 'test', - (err, features, featuresChanged) => { - expect(err).to.not.exist - expect(features).to.exist - expect(featuresChanged).to.exist - } + it('should return features and featuresChanged', async function () { + const { + features, + featuresChanged, + } = await this.FeaturesUpdater.promises.refreshFeatures( + this.user._id, + 'test' ) + expect(features).to.exist + expect(featuresChanged).to.exist }) + describe('normally', function () { - beforeEach(function () { - this.FeaturesUpdater.refreshFeatures( - this.user_id, - 'test', - this.callback + beforeEach(async function () { + await this.FeaturesUpdater.promises.refreshFeatures( + this.user._id, + 'test' ) }) - it('should get the individual features', function () { - this.FeaturesUpdater._getIndividualFeatures - .calledWith(this.user_id) - .should.equal(true) - }) - it('should get the group features', function () { - this.FeaturesUpdater._getGroupFeatureSets - .calledWith(this.user_id) - .should.equal(true) - }) - it('should get the institution features', function () { - this.InstitutionsFeatures.getInstitutionsFeatures - .calledWith(this.user_id) - .should.equal(true) - }) - it('should get the v1 features', function () { - this.FeaturesUpdater._getV1Features - .calledWith(this.user_id) - .should.equal(true) - }) - it('should get the bonus features', function () { - this.ReferalFeatures.getBonusFeatures - .calledWith(this.user_id) - .should.equal(true) - }) - it('should merge from the default features', function () { - this.FeaturesUpdater._mergeFeatures - .calledWith(this.Settings.defaultFeatures) - .should.equal(true) - }) - - it('should merge the individual features', function () { - this.FeaturesUpdater._mergeFeatures - .calledWith(sinon.match.any, { individual: 'features' }) - .should.equal(true) - }) - - it('should merge the group features', function () { - this.FeaturesUpdater._mergeFeatures - .calledWith(sinon.match.any, { group: 'features' }) - .should.equal(true) - this.FeaturesUpdater._mergeFeatures - .calledWith(sinon.match.any, { group: 'features2' }) - .should.equal(true) - }) - - it('should merge the institutions features', function () { - this.FeaturesUpdater._mergeFeatures - .calledWith(sinon.match.any, { institutions: 'features' }) - .should.equal(true) - }) - - it('should merge the v1 features', function () { - this.FeaturesUpdater._mergeFeatures - .calledWith(sinon.match.any, { v1: 'features' }) - .should.equal(true) - }) - - it('should merge the bonus features', function () { - this.FeaturesUpdater._mergeFeatures - .calledWith(sinon.match.any, { bonus: 'features' }) - .should.equal(true) - }) it('should update the user with the merged features', function () { - this.UserFeaturesUpdater.updateFeatures - .calledWith(this.user_id, { merged: 'features' }) - .should.equal(true) + expect( + this.UserFeaturesUpdater.promises.updateFeatures + ).to.have.been.calledWith(this.user._id, this.Settings.features.all) + }) + + it('should send the corresponding feature set user property', function () { + expect( + this.AnalyticsManager.setUserPropertyForUser + ).to.have.been.calledWith(this.user._id, 'feature-set', 'all') }) }) - describe('analytics user properties', function () { - it('should send the corresponding feature set user property', function () { - this.FeaturesUpdater._mergeFeatures = sinon - .stub() - .returns(this.Settings.features.personal) - - this.FeaturesUpdater.refreshFeatures( - this.user_id, - 'test', - this.callback - ) - - sinon.assert.calledWith( - this.AnalyticsManager.setUserPropertyForUser, - this.user_id, - 'feature-set', - 'personal' + describe('with a non-standard feature set', async function () { + beforeEach(async function () { + this.SubscriptionLocator.promises.getGroupSubscriptionsMemberOf + .withArgs(this.user._id) + .resolves(null) + await this.FeaturesUpdater.promises.refreshFeatures( + this.user._id, + 'test' ) }) it('should send mixed feature set user property', function () { - this.FeaturesUpdater._mergeFeatures = sinon - .stub() - .returns({ dropbox: true, feature: 'some' }) - - this.FeaturesUpdater.refreshFeatures( - this.user_id, - 'test', - this.callback - ) - sinon.assert.calledWith( this.AnalyticsManager.setUserPropertyForUser, - this.user_id, + this.user._id, 'feature-set', 'mixed' ) }) }) - describe('when losing dropbox feature', function () { - beforeEach(function () { - this.user = { - _id: this.user_id, - features: { dropbox: true }, - } - this.UserGetter.getUser = sinon.stub().yields(null, this.user) - this.FeaturesUpdater._mergeFeatures = sinon - .stub() - .returns({ dropbox: false }) - this.FeaturesUpdater.refreshFeatures( - this.user_id, - 'test', - this.callback + describe('when losing dropbox feature', async function () { + beforeEach(async function () { + this.user.features = { dropbox: true } + this.SubscriptionLocator.promises.getUserIndividualSubscription + .withArgs(this.user._id) + .resolves(this.subscriptions.noDropbox) + await this.FeaturesUpdater.promises.refreshFeatures( + this.user._id, + 'test' ) }) + it('should fire module hook to unlink dropbox', function () { - this.Modules.hooks.fire - .calledWith('removeDropbox', this.user._id, 'test') - .should.equal(true) - }) - }) - }) - - describe('_mergeFeatures', function () { - it('should prefer priority over standard for compileGroup', function () { - expect( - this.FeaturesUpdater._mergeFeatures( - { - compileGroup: 'priority', - }, - { - compileGroup: 'standard', - } + expect(this.Modules.promises.hooks.fire).to.have.been.calledWith( + 'removeDropbox', + this.user._id, + 'test' ) - ).to.deep.equal({ - compileGroup: 'priority', - }) - expect( - this.FeaturesUpdater._mergeFeatures( - { - compileGroup: 'standard', - }, - { - compileGroup: 'priority', - } - ) - ).to.deep.equal({ - compileGroup: 'priority', - }) - expect( - this.FeaturesUpdater._mergeFeatures( - { - compileGroup: 'priority', - }, - { - compileGroup: 'priority', - } - ) - ).to.deep.equal({ - compileGroup: 'priority', - }) - expect( - this.FeaturesUpdater._mergeFeatures( - { - compileGroup: 'standard', - }, - { - compileGroup: 'standard', - } - ) - ).to.deep.equal({ - compileGroup: 'standard', - }) - }) - - it('should prefer -1 over any other for collaborators', function () { - expect( - this.FeaturesUpdater._mergeFeatures( - { - collaborators: -1, - }, - { - collaborators: 10, - } - ) - ).to.deep.equal({ - collaborators: -1, - }) - expect( - this.FeaturesUpdater._mergeFeatures( - { - collaborators: 10, - }, - { - collaborators: -1, - } - ) - ).to.deep.equal({ - collaborators: -1, - }) - expect( - this.FeaturesUpdater._mergeFeatures( - { - collaborators: 4, - }, - { - collaborators: 10, - } - ) - ).to.deep.equal({ - collaborators: 10, - }) - }) - - it('should prefer the higher of compileTimeout', function () { - expect( - this.FeaturesUpdater._mergeFeatures( - { - compileTimeout: 20, - }, - { - compileTimeout: 10, - } - ) - ).to.deep.equal({ - compileTimeout: 20, - }) - expect( - this.FeaturesUpdater._mergeFeatures( - { - compileTimeout: 10, - }, - { - compileTimeout: 20, - } - ) - ).to.deep.equal({ - compileTimeout: 20, - }) - }) - - it('should prefer the true over false for other keys', function () { - expect( - this.FeaturesUpdater._mergeFeatures( - { - github: true, - }, - { - github: false, - } - ) - ).to.deep.equal({ - github: true, - }) - expect( - this.FeaturesUpdater._mergeFeatures( - { - github: false, - }, - { - github: true, - } - ) - ).to.deep.equal({ - github: true, - }) - expect( - this.FeaturesUpdater._mergeFeatures( - { - github: true, - }, - { - github: true, - } - ) - ).to.deep.equal({ - github: true, - }) - expect( - this.FeaturesUpdater._mergeFeatures( - { - github: false, - }, - { - github: false, - } - ) - ).to.deep.equal({ - github: false, }) }) }) describe('doSyncFromV1', function () { - beforeEach(function () { - this.v1UserId = 1 - this.user = { - _id: this.user_id, - email: 'user@example.com', - overleaf: { - id: this.v1UserId, - }, - } - - this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, this.user) - this.FeaturesUpdater.refreshFeatures = sinon.stub().yields(null) - this.call = cb => { - this.FeaturesUpdater.doSyncFromV1(this.v1UserId, cb) - } - }) - describe('when all goes well', function () { - it('should call getUser', function (done) { - this.call(() => { - expect(this.UserGetter.getUser.callCount).to.equal(1) - expect( - this.UserGetter.getUser.calledWith({ 'overleaf.id': this.v1UserId }) - ).to.equal(true) - done() - }) + beforeEach(async function () { + await this.FeaturesUpdater.promises.doSyncFromV1(this.v1UserId) }) - it('should call refreshFeatures', function (done) { - this.call(() => { - expect(this.FeaturesUpdater.refreshFeatures.callCount).to.equal(1) - expect( - this.FeaturesUpdater.refreshFeatures.calledWith(this.user_id) - ).to.equal(true) - done() - }) - }) - - it('should not produce an error', function (done) { - this.call(err => { - expect(err).to.not.exist - done() - }) + it('should update the user with the merged features', function () { + expect( + this.UserFeaturesUpdater.promises.updateFeatures + ).to.have.been.calledWith(this.user._id, this.Settings.features.all) }) }) describe('when getUser produces an error', function () { beforeEach(function () { - this.UserGetter.getUser = sinon - .stub() - .callsArgWith(2, new Error('woops')) + this.UserGetter.promises.getUser.rejects(new Error('woops')) }) - it('should not call refreshFeatures', function () { - expect(this.FeaturesUpdater.refreshFeatures.callCount).to.equal(0) - }) - - it('should produce an error', function (done) { - this.call(err => { - expect(err).to.exist - done() - }) + it('should propagate the error', async function () { + const someId = 9090 + await expect(this.FeaturesUpdater.promises.doSyncFromV1(someId)).to.be + .rejected + expect(this.UserFeaturesUpdater.promises.updateFeatures).not.to.have + .been.called }) }) describe('when getUser does not find a user', function () { - beforeEach(function () { - this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, null) + beforeEach(async function () { + const someOtherId = 987 + await this.FeaturesUpdater.promises.doSyncFromV1(someOtherId) }) - it('should not call refreshFeatures', function (done) { - this.call(() => { - expect(this.FeaturesUpdater.refreshFeatures.callCount).to.equal(0) - done() - }) + it('should not update the user', function () { + expect(this.UserFeaturesUpdater.promises.updateFeatures).not.to.have + .been.called }) - - it('should not produce an error', function (done) { - this.call(err => { - expect(err).to.not.exist - done() - }) - }) - }) - }) - - describe('isFeatureSetBetter', function () { - it('simple comparisons', function () { - const result1 = this.FeaturesUpdater.isFeatureSetBetter( - { - dropbox: true, - }, - { - dropbox: false, - } - ) - assert.isTrue(result1) - - const result2 = this.FeaturesUpdater.isFeatureSetBetter( - { - dropbox: false, - }, - { - dropbox: true, - } - ) - assert.isFalse(result2) - }) - - it('compound comparisons with same features', function () { - const result1 = this.FeaturesUpdater.isFeatureSetBetter( - { - collaborators: 9, - dropbox: true, - }, - { - collaborators: 10, - dropbox: true, - } - ) - assert.isFalse(result1) - - const result2 = this.FeaturesUpdater.isFeatureSetBetter( - { - collaborators: -1, - dropbox: true, - }, - { - collaborators: 10, - dropbox: true, - } - ) - assert.isTrue(result2) - - const result3 = this.FeaturesUpdater.isFeatureSetBetter( - { - collaborators: -1, - compileTimeout: 60, - dropbox: true, - }, - { - collaborators: 10, - compileTimeout: 60, - dropbox: true, - } - ) - assert.isTrue(result3) }) }) }) diff --git a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js index a9671559f0..c2f9448c18 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js @@ -2,7 +2,8 @@ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') const chai = require('chai') const { expect } = chai -const modulePath = + +const MODULE_PATH = '../../../../app/src/Features/Subscription/SubscriptionHandler' const mockRecurlySubscriptions = { @@ -47,6 +48,8 @@ const mockSubscriptionChanges = { describe('SubscriptionHandler', function () { beforeEach(function () { + this.callback = sinon.stub() + this.Settings = { plans: [ { @@ -85,6 +88,7 @@ describe('SubscriptionHandler', function () { getBillingInfo: sinon.stub().yields(), getAccountPastDueInvoices: sinon.stub().yields(), attemptInvoiceCollection: sinon.stub().yields(), + listAccountActiveSubscriptions: sinon.stub().yields(null, []), } this.RecurlyClient = { changeSubscriptionByUuid: sinon @@ -118,7 +122,7 @@ describe('SubscriptionHandler', function () { shouldPlanChangeAtTermEnd: sinon.stub(), } - this.SubscriptionHandler = SandboxedModule.require(modulePath, { + this.SubscriptionHandler = SandboxedModule.require(MODULE_PATH, { requires: { './RecurlyWrapper': this.RecurlyWrapper, './RecurlyClient': this.RecurlyClient, @@ -134,23 +138,15 @@ describe('SubscriptionHandler', function () { './SubscriptionHelper': this.SubscriptionHelper, }, }) - - this.SubscriptionHandler.syncSubscriptionToUser = sinon - .stub() - .callsArgWith(2) }) describe('createSubscription', function () { beforeEach(function () { - this.callback = sinon.stub() this.subscriptionDetails = { cvv: '123', number: '12345', } this.recurlyTokenIds = { billing: '45555666' } - this.SubscriptionHandler.validateNoSubscriptionInRecurly = sinon - .stub() - .yields(null, true) }) describe('successfully', function () { @@ -182,9 +178,9 @@ describe('SubscriptionHandler', function () { describe('when there is already a subscription in Recurly', function () { beforeEach(function () { - this.SubscriptionHandler.validateNoSubscriptionInRecurly = sinon - .stub() - .yields(null, false) + this.RecurlyWrapper.listAccountActiveSubscriptions.yields(null, [ + this.subscription, + ]) this.SubscriptionHandler.createSubscription( this.user, this.subscriptionDetails, @@ -235,9 +231,9 @@ describe('SubscriptionHandler', function () { callback(null, this.user) } this.plan_code = 'collaborator' - this.SubscriptionHelper.shouldPlanChangeAtTermEnd = sinon - .stub() - .returns(shouldPlanChangeAtTermEnd) + this.SubscriptionHelper.shouldPlanChangeAtTermEnd.returns( + shouldPlanChangeAtTermEnd + ) this.LimitationsManager.userHasV2Subscription.callsArgWith( 1, null, @@ -277,14 +273,13 @@ describe('SubscriptionHandler', function () { callback(null, this.user) } this.plan_code = 'collaborator' - this.PlansLocator.findLocalPlanInSettings = sinon.stub().returns(null) + this.PlansLocator.findLocalPlanInSettings.returns(null) this.LimitationsManager.userHasV2Subscription.callsArgWith( 1, null, true, this.subscription ) - this.callback = sinon.stub() this.SubscriptionHandler.updateSubscription( this.user, this.plan_code, @@ -322,9 +317,7 @@ describe('SubscriptionHandler', function () { it('should redirect to the subscription dashboard', function () { this.RecurlyClient.changeSubscriptionByUuid.called.should.equal(false) - this.SubscriptionHandler.syncSubscriptionToUser.called.should.equal( - false - ) + this.SubscriptionUpdater.syncSubscription.called.should.equal(false) }) }) @@ -561,18 +554,11 @@ describe('SubscriptionHandler', function () { }) describe('validateNoSubscriptionInRecurly', function () { - beforeEach(function () { - this.subscriptions = [] - this.RecurlyWrapper.listAccountActiveSubscriptions = sinon - .stub() - .yields(null, this.subscriptions) - this.SubscriptionUpdater.syncSubscription = sinon.stub().yields() - this.callback = sinon.stub() - }) - - describe('with no subscription in recurly', function () { + describe('with a subscription in recurly', function () { beforeEach(function () { - this.subscriptions.push((this.subscription = { mock: 'subscription' })) + this.RecurlyWrapper.listAccountActiveSubscriptions.yields(null, [ + this.subscription, + ]) this.SubscriptionHandler.validateNoSubscriptionInRecurly( this.user_id, this.callback @@ -596,7 +582,7 @@ describe('SubscriptionHandler', function () { }) }) - describe('with a subscription in recurly', function () { + describe('with no subscription in recurly', function () { beforeEach(function () { this.SubscriptionHandler.validateNoSubscriptionInRecurly( this.user_id, diff --git a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js index b4c41c0671..f60de1d3ae 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js @@ -7,12 +7,14 @@ const { ObjectId } = require('mongodb') describe('SubscriptionUpdater', function () { beforeEach(function () { + this.recurlyPlan = { planCode: 'recurly-plan' } this.recurlySubscription = { uuid: '1238uoijdasjhd', plan: { - plan_code: 'kjhsakjds', + plan_code: this.recurlyPlan.planCode, }, } + this.adminUser = { _id: (this.adminuser_id = '5208dd34438843e2db000007') } this.otherUserId = '5208dd34438842e2db000005' this.allUserIds = ['13213', 'dsadas', 'djsaiud89'] @@ -21,7 +23,7 @@ describe('SubscriptionUpdater', function () { admin_id: this.adminUser._id, manager_ids: [this.adminUser._id], member_ids: [], - save: sinon.stub().callsArgWith(0), + save: sinon.stub().resolves(), planCode: 'student_or_something', } this.user_id = this.adminuser_id @@ -31,7 +33,7 @@ describe('SubscriptionUpdater', function () { admin_id: this.adminUser._id, manager_ids: [this.adminUser._id], member_ids: this.allUserIds, - save: sinon.stub().callsArgWith(0), + save: sinon.stub().resolves(), groupPlan: true, planCode: 'group_subscription', } @@ -40,17 +42,11 @@ describe('SubscriptionUpdater', function () { admin_id: this.adminUser._id, manager_ids: [this.adminUser._id], member_ids: [this.otherUserId], - save: sinon.stub().callsArgWith(0), + save: sinon.stub().resolves(), groupPlan: true, planCode: 'better_group_subscription', } - this.updateStub = sinon.stub().callsArgWith(2, null) - this.updateManyStub = sinon.stub().callsArgWith(2, null) - this.findOneAndUpdateStub = sinon - .stub() - .callsArgWith(2, null, this.subscription) - const subscription = this.subscription this.SubscriptionModel = class { constructor(opts) { @@ -61,62 +57,93 @@ describe('SubscriptionUpdater', function () { } save() { - return subscription + return Promise.resolve(subscription) } } - this.SubscriptionModel.deleteOne = sinon.stub().yields() - this.SubscriptionModel.updateOne = this.updateStub - this.SubscriptionModel.updateMany = this.updateManyStub - this.SubscriptionModel.findOneAndUpdate = this.findOneAndUpdateStub + this.SubscriptionModel.deleteOne = sinon + .stub() + .returns({ exec: sinon.stub().resolves() }) + this.SubscriptionModel.updateOne = sinon + .stub() + .returns({ exec: sinon.stub().resolves() }) + this.SubscriptionModel.updateMany = sinon + .stub() + .returns({ exec: sinon.stub().resolves() }) + this.SubscriptionModel.findOneAndUpdate = sinon.stub().returns({ + exec: sinon.stub().resolves(this.subscription), + }) this.SubscriptionLocator = { - getUsersSubscription: sinon.stub(), - getGroupSubscriptionMemberOf: sinon.stub(), - getMemberSubscriptions: sinon.stub().yields(null, []), promises: { - getMemberSubscriptions: sinon.stub().returns([this.groupSubscription]), + getUsersSubscription: sinon.stub(), + getGroupSubscriptionMemberOf: sinon.stub(), + getMemberSubscriptions: sinon.stub().resolves([]), + getSubscription: sinon.stub(), }, } + this.SubscriptionLocator.promises.getSubscription + .withArgs(this.subscription._id) + .resolves(this.subscription) + this.Settings = { defaultPlanCode: 'personal', defaultFeatures: { default: 'features' }, + plans: [ + this.recurlyPlan, + { planCode: this.subscription.planCode, features: {} }, + { + planCode: this.groupSubscription.planCode, + features: { + collaborators: 10, + compileTimeout: 60, + dropbox: true, + }, + }, + { + planCode: this.betterGroupSubscription.planCode, + features: { + collaborators: -1, + compileTimeout: 240, + dropbox: true, + }, + }, + ], } - this.UserFeaturesUpdater = { updateFeatures: sinon.stub().yields() } - - this.PlansLocator = { findLocalPlanInSettings: sinon.stub().returns({}) } - - this.UserGetter = { - getUsers(memberIds, projection, callback) { - const users = memberIds.map(id => ({ _id: id })) - callback(null, users) + this.UserFeaturesUpdater = { + promises: { + updateFeatures: sinon.stub().resolves(), + }, + } + + this.ReferalFeatures = { + promises: { + getBonusFeatures: sinon.stub().resolves(), }, - getUser: sinon.stub(), } - this.ReferalFeatures = { getBonusFeatures: sinon.stub().callsArgWith(1) } - this.Modules = { hooks: { fire: sinon.stub().callsArgWith(2, null, null) } } this.FeaturesUpdater = { - refreshFeatures: sinon.stub().yields(), - planCodeToFeatures: sinon.stub(), + promises: { + refreshFeatures: sinon.stub().resolves({}), + }, } + this.DeletedSubscription = { - findOneAndUpdate: sinon.stub().yields(), + findOneAndUpdate: sinon.stub().returns({ exec: sinon.stub().resolves() }), } + this.AnalyticsManager = { setUserPropertyForUser: sinon.stub(), } + this.SubscriptionUpdater = SandboxedModule.require(modulePath, { requires: { - mongodb: { ObjectId }, '../../models/Subscription': { Subscription: this.SubscriptionModel, }, './UserFeaturesUpdater': this.UserFeaturesUpdater, './SubscriptionLocator': this.SubscriptionLocator, - '../User/UserGetter': this.UserGetter, - './PlansLocator': this.PlansLocator, '@overleaf/settings': this.Settings, '../../infrastructure/mongodb': { db: {}, ObjectId }, './FeaturesUpdater': this.FeaturesUpdater, @@ -129,266 +156,184 @@ describe('SubscriptionUpdater', function () { }) describe('updateAdmin', function () { - it('should update the subscription admin', function (done) { + it('should update the subscription admin', async function () { this.subscription.groupPlan = true - this.SubscriptionUpdater.updateAdmin( + await this.SubscriptionUpdater.promises.updateAdmin( this.subscription, - this.otherUserId, - err => { - if (err != null) { - return done(err) - } - const query = { - _id: ObjectId(this.subscription._id), - customAccount: true, - } - const update = { - $set: { admin_id: ObjectId(this.otherUserId) }, - $addToSet: { manager_ids: ObjectId(this.otherUserId) }, - } - this.SubscriptionModel.updateOne.should.have.been.calledOnce - this.SubscriptionModel.updateOne.should.have.been.calledWith( - query, - update - ) - done() - } + this.otherUserId + ) + const query = { + _id: ObjectId(this.subscription._id), + customAccount: true, + } + const update = { + $set: { admin_id: ObjectId(this.otherUserId) }, + $addToSet: { manager_ids: ObjectId(this.otherUserId) }, + } + this.SubscriptionModel.updateOne.should.have.been.calledOnce + this.SubscriptionModel.updateOne.should.have.been.calledWith( + query, + update ) }) - it('should remove the manager for non-group subscriptions', function (done) { - this.SubscriptionUpdater.updateAdmin( + it('should remove the manager for non-group subscriptions', async function () { + await this.SubscriptionUpdater.promises.updateAdmin( this.subscription, - this.otherUserId, - err => { - if (err != null) { - return done(err) - } - const query = { - _id: ObjectId(this.subscription._id), - customAccount: true, - } - const update = { - $set: { - admin_id: ObjectId(this.otherUserId), - manager_ids: [ObjectId(this.otherUserId)], - }, - } - this.SubscriptionModel.updateOne.should.have.been.calledOnce - this.SubscriptionModel.updateOne.should.have.been.calledWith( - query, - update - ) - done() - } + this.otherUserId + ) + const query = { + _id: ObjectId(this.subscription._id), + customAccount: true, + } + const update = { + $set: { + admin_id: ObjectId(this.otherUserId), + manager_ids: [ObjectId(this.otherUserId)], + }, + } + this.SubscriptionModel.updateOne.should.have.been.calledOnce + this.SubscriptionModel.updateOne.should.have.been.calledWith( + query, + update ) }) }) describe('syncSubscription', function () { beforeEach(function () { - this.SubscriptionLocator.getUsersSubscription.callsArgWith( - 1, - null, + this.SubscriptionLocator.promises.getUsersSubscription.resolves( this.subscription ) - this.SubscriptionUpdater.updateSubscriptionFromRecurly = sinon - .stub() - .yields() }) - it('should update the subscription if the user already is admin of one', function (done) { - this.SubscriptionUpdater.syncSubscription( + it('should update the subscription if the user already is admin of one', async function () { + await this.SubscriptionUpdater.promises.syncSubscription( this.recurlySubscription, - this.adminUser._id, - err => { - if (err != null) { - return done(err) - } - this.SubscriptionLocator.getUsersSubscription - .calledWith(this.adminUser._id) - .should.equal(true) - done() - } + this.adminUser._id ) + this.SubscriptionLocator.promises.getUsersSubscription + .calledWith(this.adminUser._id) + .should.equal(true) }) - it('should not call updateFeatures with group subscription if recurly subscription is not expired', function (done) { - this.SubscriptionUpdater.syncSubscription( + it('should not call updateFeatures with group subscription if recurly subscription is not expired', async function () { + await this.SubscriptionUpdater.promises.syncSubscription( this.recurlySubscription, - this.adminUser._id, - err => { - if (err != null) { - return done(err) - } - this.SubscriptionLocator.getUsersSubscription - .calledWith(this.adminUser._id) - .should.equal(true) - this.UserFeaturesUpdater.updateFeatures.called.should.equal(false) - done() - } + this.adminUser._id + ) + this.SubscriptionLocator.promises.getUsersSubscription + .calledWith(this.adminUser._id) + .should.equal(true) + this.UserFeaturesUpdater.promises.updateFeatures.called.should.equal( + false ) }) }) describe('updateSubscriptionFromRecurly', function () { - beforeEach(function () { - this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(2) - }) - afterEach(function () { this.subscription.member_ids = [] }) - it('should update the subscription with token etc when not expired', function (done) { - this.SubscriptionUpdater.updateSubscriptionFromRecurly( + it('should update the subscription with token etc when not expired', async function () { + await this.SubscriptionUpdater.promises.updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, - {}, - err => { - if (err != null) { - return done(err) - } - this.subscription.recurlySubscription_id.should.equal( - this.recurlySubscription.uuid - ) - this.subscription.planCode.should.equal( - this.recurlySubscription.plan.plan_code - ) - this.subscription.save.called.should.equal(true) - this.FeaturesUpdater.refreshFeatures - .calledWith(this.adminUser._id) - .should.equal(true) - done() - } + {} ) + this.subscription.recurlySubscription_id.should.equal( + this.recurlySubscription.uuid + ) + this.subscription.planCode.should.equal( + this.recurlySubscription.plan.plan_code + ) + this.subscription.save.called.should.equal(true) + this.FeaturesUpdater.promises.refreshFeatures + .calledWith(this.adminUser._id) + .should.equal(true) }) - it('should remove the subscription when expired', function (done) { + it('should remove the subscription when expired', async function () { this.recurlySubscription.state = 'expired' - this.SubscriptionUpdater.updateSubscriptionFromRecurly( + await this.SubscriptionUpdater.promises.updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, - {}, - err => { - if (err != null) { - return done(err) - } - done() - } + {} ) }) - it('should update all the users features', function (done) { + it('should update all the users features', async function () { this.subscription.member_ids = this.allUserIds - this.SubscriptionUpdater.updateSubscriptionFromRecurly( + await this.SubscriptionUpdater.promises.updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, - {}, - err => { - if (err != null) { - return done(err) - } - this.FeaturesUpdater.refreshFeatures - .calledWith(this.adminUser._id) - .should.equal(true) - this.FeaturesUpdater.refreshFeatures - .calledWith(this.allUserIds[0]) - .should.equal(true) - this.FeaturesUpdater.refreshFeatures - .calledWith(this.allUserIds[1]) - .should.equal(true) - this.FeaturesUpdater.refreshFeatures - .calledWith(this.allUserIds[2]) - .should.equal(true) - done() - } + {} ) + this.FeaturesUpdater.promises.refreshFeatures + .calledWith(this.adminUser._id) + .should.equal(true) + this.FeaturesUpdater.promises.refreshFeatures + .calledWith(this.allUserIds[0]) + .should.equal(true) + this.FeaturesUpdater.promises.refreshFeatures + .calledWith(this.allUserIds[1]) + .should.equal(true) + this.FeaturesUpdater.promises.refreshFeatures + .calledWith(this.allUserIds[2]) + .should.equal(true) }) - it('should set group to true and save how many members can be added to group', function (done) { - this.PlansLocator.findLocalPlanInSettings - .withArgs(this.recurlySubscription.plan.plan_code) - .returns({ groupPlan: true, membersLimit: 5 }) - this.SubscriptionUpdater.updateSubscriptionFromRecurly( + it('should set group to true and save how many members can be added to group', async function () { + this.recurlyPlan.groupPlan = true + this.recurlyPlan.membersLimit = 5 + await this.SubscriptionUpdater.promises.updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, - {}, - err => { - if (err != null) { - return done(err) - } - this.subscription.membersLimit.should.equal(5) - this.subscription.groupPlan.should.equal(true) - this.subscription.member_ids.should.deep.equal([ - this.subscription.admin_id, - ]) - done() - } + {} ) + this.subscription.membersLimit.should.equal(5) + this.subscription.groupPlan.should.equal(true) + this.subscription.member_ids.should.deep.equal([ + this.subscription.admin_id, + ]) }) - it('should delete and replace subscription when downgrading from group to individual plan', function (done) { - this.PlansLocator.findLocalPlanInSettings - .withArgs(this.recurlySubscription.plan.plan_code) - .returns({ groupPlan: false }) - this.SubscriptionUpdater._deleteAndReplaceSubscriptionFromRecurly = sinon - .stub() - .yields() - this.SubscriptionUpdater.updateSubscriptionFromRecurly( + it('should delete and replace subscription when downgrading from group to individual plan', async function () { + this.recurlyPlan.groupPlan = false + await this.SubscriptionUpdater.promises.updateSubscriptionFromRecurly( this.recurlySubscription, this.groupSubscription, - {}, - err => { - if (err != null) { - return done(err) - } - done() - } + {} ) }) - it('should not set group to true or set groupPlan', function (done) { - this.SubscriptionUpdater.updateSubscriptionFromRecurly( + it('should not set group to true or set groupPlan', async function () { + await this.SubscriptionUpdater.promises.updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, - {}, - err => { - if (err != null) { - return done(err) - } - assert.notEqual(this.subscription.membersLimit, 5) - assert.notEqual(this.subscription.groupPlan, true) - done() - } + {} ) + assert.notEqual(this.subscription.membersLimit, 5) + assert.notEqual(this.subscription.groupPlan, true) }) describe('when the plan allows adding more seats', function () { beforeEach(function () { this.membersLimitAddOn = 'add_on1' - this.PlansLocator.findLocalPlanInSettings - .withArgs(this.recurlySubscription.plan.plan_code) - .returns({ - groupPlan: true, - membersLimit: 5, - membersLimitAddOn: this.membersLimitAddOn, - }) + this.recurlyPlan.groupPlan = true + this.recurlyPlan.membersLimit = 5 + this.recurlyPlan.membersLimitAddOn = this.membersLimitAddOn }) function expectMembersLimit(limit) { - it('should set the membersLimit accordingly', function (done) { - this.SubscriptionUpdater.updateSubscriptionFromRecurly( + it('should set the membersLimit accordingly', async function () { + await this.SubscriptionUpdater.promises.updateSubscriptionFromRecurly( this.recurlySubscription, this.subscription, - {}, - error => { - if (error) return done(error) - - expect(this.subscription.membersLimit).to.equal(limit) - done() - } + {} ) + expect(this.subscription.membersLimit).to.equal(limit) }) } @@ -431,117 +376,34 @@ describe('SubscriptionUpdater', function () { }) describe('addUserToGroup', function () { - beforeEach(function () { - this.FeaturesUpdater.refreshFeatures = sinon.stub().yields(null, {}) - this.FeaturesUpdater.isFeatureSetBetter = sinon.stub() - this.FeaturesUpdater.isFeatureSetBetter - .withArgs( - { - collaborators: 10, - compileTimeout: 60, - dropbox: true, - }, - {} - ) - .returns(true) - this.FeaturesUpdater.isFeatureSetBetter - .withArgs( - { - collaborators: -1, - compileTimeout: 240, - dropbox: true, - }, - { - collaborators: 10, - compileTimeout: 60, - dropbox: true, - } - ) - .returns(true) - this.FeaturesUpdater.isFeatureSetBetter - .withArgs( - { - collaborators: -1, - compileTimeout: 240, - dropbox: true, - }, - {} - ) - .returns(true) - this.PlansLocator.findLocalPlanInSettings = sinon.stub() - this.PlansLocator.findLocalPlanInSettings - .withArgs(this.groupSubscription.planCode) - .returns({ - planCode: this.groupSubscription.planCode, - features: { - collaborators: 10, - compileTimeout: 60, - dropbox: true, - }, - }) - this.PlansLocator.findLocalPlanInSettings - .withArgs(this.betterGroupSubscription.planCode) - .returns({ - planCode: this.betterGroupSubscription.planCode, - features: { - collaborators: -1, - compileTimeout: 240, - dropbox: true, - }, - }) + it('should add the user ids to the group as a set', async function () { + await this.SubscriptionUpdater.promises.addUserToGroup( + this.subscription._id, + this.otherUserId + ) + const searchOps = { _id: this.subscription._id } + const insertOperation = { + $addToSet: { member_ids: this.otherUserId }, + } + this.SubscriptionModel.updateOne + .calledWith(searchOps, insertOperation) + .should.equal(true) }) - it('should add the user ids to the group as a set', function (done) { - this.SubscriptionUpdater.addUserToGroup( + it('should update the users features', async function () { + await this.SubscriptionUpdater.promises.addUserToGroup( this.subscription._id, - this.otherUserId, - () => { - const searchOps = { _id: this.subscription._id } - const insertOperation = { - $addToSet: { member_ids: this.otherUserId }, - } - this.updateStub - .calledWith(searchOps, insertOperation) - .should.equal(true) - done() - } - ) - }) - - it('should update the users features', function (done) { - this.SubscriptionUpdater.addUserToGroup( - this.subscription._id, - this.otherUserId, - () => { - this.FeaturesUpdater.refreshFeatures - .calledWith(this.otherUserId) - .should.equal(true) - done() - } - ) - }) - - it('should set the group plan code user property', function (done) { - this.SubscriptionUpdater.addUserToGroup( - this.subscription._id, - this.otherUserId, - () => { - sinon.assert.calledWith( - this.AnalyticsManager.setUserPropertyForUser, - this.otherUserId, - 'group-subscription-plan-code', - 'group_subscription' - ) - done() - } + this.otherUserId ) + this.FeaturesUpdater.promises.refreshFeatures + .calledWith(this.otherUserId) + .should.equal(true) }) it('should set the group plan code user property to the best plan with 1 group subscription', async function () { - this.SubscriptionLocator.promises.getMemberSubscriptions = sinon - .stub() + this.SubscriptionLocator.promises.getMemberSubscriptions .withArgs(this.otherUserId) - .returns([this.groupSubscription]) + .resolves([this.groupSubscription]) await this.SubscriptionUpdater.promises.addUserToGroup( this.groupSubscription._id, this.otherUserId @@ -555,10 +417,9 @@ describe('SubscriptionUpdater', function () { }) it('should set the group plan code user property to the best plan with 2 group subscriptions', async function () { - this.SubscriptionLocator.promises.getMemberSubscriptions = sinon - .stub() + this.SubscriptionLocator.promises.getMemberSubscriptions .withArgs(this.otherUserId) - .returns([this.groupSubscription, this.betterGroupSubscription]) + .resolves([this.groupSubscription, this.betterGroupSubscription]) await this.SubscriptionUpdater.promises.addUserToGroup( this.betterGroupSubscription._id, this.otherUserId @@ -572,10 +433,9 @@ describe('SubscriptionUpdater', function () { }) it('should set the group plan code user property to the best plan with 2 group subscriptions in reverse order', async function () { - this.SubscriptionLocator.promises.getMemberSubscriptions = sinon - .stub() + this.SubscriptionLocator.promises.getMemberSubscriptions .withArgs(this.otherUserId) - .returns([this.betterGroupSubscription, this.groupSubscription]) + .resolves([this.betterGroupSubscription, this.groupSubscription]) await this.SubscriptionUpdater.promises.addUserToGroup( this.betterGroupSubscription._id, this.otherUserId @@ -591,95 +451,84 @@ describe('SubscriptionUpdater', function () { describe('removeUserFromGroups', function () { beforeEach(function () { - this.FeaturesUpdater.refreshFeatures = sinon.stub().yields(null, {}) - this.FeaturesUpdater.isFeatureSetBetter = sinon.stub().returns(true) - this.UserGetter.getUser.yields(null, {}) this.fakeSubscriptions = [{ _id: 'fake-id-1' }, { _id: 'fake-id-2' }] - this.SubscriptionLocator.getMemberSubscriptions.yields( - null, + this.SubscriptionLocator.promises.getMemberSubscriptions.resolves( this.fakeSubscriptions ) - this.SubscriptionLocator.promises.getMemberSubscriptions.returns([]) }) - it('should pull the users id from the group', function (done) { - this.SubscriptionUpdater.removeUserFromGroup( + it('should pull the users id from the group', async function () { + await this.SubscriptionUpdater.promises.removeUserFromGroup( this.subscription._id, + this.otherUserId + ) + const removeOperation = { $pull: { member_ids: this.otherUserId } } + this.SubscriptionModel.updateOne + .calledWith({ _id: this.subscription._id }, removeOperation) + .should.equal(true) + }) + + it('should set the group plan code user property when removing user from group', async function () { + await this.SubscriptionUpdater.promises.removeUserFromGroup( + this.subscription._id, + this.otherUserId + ) + sinon.assert.calledWith( + this.AnalyticsManager.setUserPropertyForUser, this.otherUserId, - () => { - const removeOperation = { $pull: { member_ids: this.otherUserId } } - this.updateStub - .calledWith({ _id: this.subscription._id }, removeOperation) - .should.equal(true) - done() - } + 'group-subscription-plan-code', + null ) }) - it('should set the group plan code user property when removing user from group', function (done) { - this.SubscriptionUpdater.removeUserFromGroup( - this.subscription._id, + it('should set the group plan code user property when removing user from all groups', async function () { + await this.SubscriptionUpdater.promises.removeUserFromAllGroups( + this.otherUserId + ) + sinon.assert.calledWith( + this.AnalyticsManager.setUserPropertyForUser, this.otherUserId, - () => { - sinon.assert.calledWith( - this.AnalyticsManager.setUserPropertyForUser, - this.otherUserId, - 'group-subscription-plan-code', - null - ) - done() - } + 'group-subscription-plan-code', + null ) }) - it('should set the group plan code user property when removing user from all groups', function (done) { - this.SubscriptionUpdater.removeUserFromAllGroups(this.otherUserId, () => { - sinon.assert.calledWith( - this.AnalyticsManager.setUserPropertyForUser, - this.otherUserId, - 'group-subscription-plan-code', - null - ) - done() - }) - }) - - it('should pull the users id from all groups', function (done) { - this.SubscriptionUpdater.removeUserFromAllGroups(this.otherUserId, () => { - const filter = { _id: ['fake-id-1', 'fake-id-2'] } - const removeOperation = { $pull: { member_ids: this.otherUserId } } - sinon.assert.calledWith(this.updateManyStub, filter, removeOperation) - done() - }) - }) - - it('should update the users features', function (done) { - this.SubscriptionUpdater.removeUserFromGroup( - this.subscription._id, - this.otherUserId, - () => { - this.FeaturesUpdater.refreshFeatures - .calledWith(this.otherUserId) - .should.equal(true) - done() - } + it('should pull the users id from all groups', async function () { + await this.SubscriptionUpdater.promises.removeUserFromAllGroups( + this.otherUserId ) + const filter = { _id: ['fake-id-1', 'fake-id-2'] } + const removeOperation = { $pull: { member_ids: this.otherUserId } } + sinon.assert.calledWith( + this.SubscriptionModel.updateMany, + filter, + removeOperation + ) + }) + + it('should update the users features', async function () { + await this.SubscriptionUpdater.promises.removeUserFromGroup( + this.subscription._id, + this.otherUserId + ) + this.FeaturesUpdater.promises.refreshFeatures + .calledWith(this.otherUserId) + .should.equal(true) }) }) describe('deleteSubscription', function () { - beforeEach(function (done) { + beforeEach(async function () { this.subscription = { _id: ObjectId().toString(), mock: 'subscription', admin_id: ObjectId(), member_ids: [ObjectId(), ObjectId(), ObjectId()], } - this.SubscriptionLocator.getSubscription = sinon - .stub() - .yields(null, this.subscription) - this.FeaturesUpdater.refreshFeatures = sinon.stub().yields() - this.SubscriptionUpdater.deleteSubscription(this.subscription, {}, done) + await this.SubscriptionUpdater.promises.deleteSubscription( + this.subscription, + {} + ) }) it('should remove the subscription', function () { @@ -689,14 +538,14 @@ describe('SubscriptionUpdater', function () { }) it('should downgrade the admin_id', function () { - this.FeaturesUpdater.refreshFeatures + this.FeaturesUpdater.promises.refreshFeatures .calledWith(this.subscription.admin_id) .should.equal(true) }) it('should downgrade all of the members', function () { for (const userId of this.subscription.member_ids) { - this.FeaturesUpdater.refreshFeatures + this.FeaturesUpdater.promises.refreshFeatures .calledWith(userId) .should.equal(true) }