From 249381b6a84f6a10bd568274f66025c550e99ddd Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 24 Jun 2016 11:42:58 +0100 Subject: [PATCH 01/17] WIP: initial work on sending address to recurly for paypal subscriptions --- .../Subscription/RecurlyWrapper.coffee | 159 +++++++++++++++++- .../coffee/main/new-subscription.coffee | 7 + 2 files changed, 157 insertions(+), 9 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index 680905ca6c..edf4dc8841 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -4,11 +4,140 @@ request = require 'request' Settings = require "settings-sharelatex" xml2js = require "xml2js" logger = require("logger-sharelatex") +Async = require('async') module.exports = RecurlyWrapper = apiUrl : "https://api.recurly.com/v2" - createSubscription: (user, subscriptionDetails, recurly_token_id, callback)-> + _createPaypalSubscription: (user, subscriptionDetails, recurly_token_id, callback) -> + logger.log {user_id: user._id, recurly_token_id}, "starting process of creating paypal subscription" + Async.waterfall([ + (next) -> # create account + logger.log {user_id: user._id, recurly_token_id}, "creating user in recurly" + address = subscriptionDetails.address + if !address + return next(new Error('no address in subscriptionDetails at createAccount stage')) + requestBody = """ + + #{user._id} + #{user.email} + #{user.first_name} + #{user.last_name} +
+ #{address.address1} + #{address.address2} + #{address.city} + #{address.state} + #{address.zip} + #{address.country} +
+ + """ + RecurlyWrapper.apiRequest({ + url : "accounts" + method : "POST" + body : requestBody + }, (error, response, responseBody) => + return next(error) if error? + RecurlyWrapper._parseAccountXml responseBody, (err, account) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error creating account" + return next(err) + result = {account} + return next(null, result) + ) + , (result, next) -> # create billing info + logger.log {user_id: user._id, recurly_token_id}, "creating billing info in recurly" + accountCode = result?.account?.account_code + if !accountCode + return next(new Error('no account code at createBillingInfo stage')) + requestBody = """ + + #{recurly_token_id} + + """ + RecurlyWrapper.apiRequest({ + url: "accounts/#{accountCode}/billing_info" + method: "POST" + body: requestBody + }, (error, response, responseBody) => + return next(error) if error? + RecurlyWrapper._parseBillingInfoXml responseBody, (err, billingInfo) -> + if err + logger.error {err, user_id: user._id, accountCode, recurly_token_id}, "error creating billing info" + return next(err) + result.billingInfo = billingInfo + return next(null, result) + ) + , (result, next) -> # set address + logger.log {user_id: user._id, recurly_token_id}, "setting billing address in recurly" + accountCode = result?.account?.account_code + if !accountCode + return next(new Error('no account code at setAddress stage')) + address = subscriptionDetails.address + if !address + return next(new Error('no address in subscriptionDetails at setAddress stage')) + requestBody = """ + + #{address.address1} + #{address.address2} + #{address.city} + #{address.state} + #{address.zip} + #{address.country} + + """ + RecurlyWrapper.apiRequest({ + url: "accounts/#{accountCode}/billing_info" + method: "PUT" + body: requestBody + }, (error, response, responseBody) => + return next(error) if error? + RecurlyWrapper._parseBillingInfoXml responseBody, (err, billingInfo) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error updating billing info" + return next(err) + result.billingInfo = billingInfo + return next(null, result) + ) + , (result, next) -> # create subscription + logger.log {user_id: user._id, recurly_token_id}, "creating subscription in recurly" + requestBody = """ + + #{subscriptionDetails.plan_code} + #{subscriptionDetails.currencyCode} + #{subscriptionDetails.coupon_code} + + #{user._id} + + + """ # TODO: check account details and billing + RecurlyWrapper.apiRequest({ + url : "subscriptions" + method : "POST" + body : requestBody + }, (error, response, responseBody) => + return next(error) if error? + RecurlyWrapper._parseSubscriptionXml responseBody, (err, subscription) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error creating subscription" + return next(err) + result.subscription = subscription + return next(null, result) + ) + ], (err, result) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error in paypal subscription creation process" + return callback(err) + if !result.subscription + err = new Error('no subscription object in result') + logger.error {err, user_id: user._id, recurly_token_id}, "error in paypal subscription creation process" + return callback(err) + logger.log {user_id: user._id, recurly_token_id}, "done creating paypal subscription for user" + callback(null, result.subscription) + ) + + _createCreditCardSubscription: (user, subscriptionDetails, recurly_token_id, callback) -> requestBody = """ #{subscriptionDetails.plan_code} @@ -32,7 +161,13 @@ module.exports = RecurlyWrapper = }, (error, response, responseBody) => return callback(error) if error? @_parseSubscriptionXml responseBody, callback - ) + ) + + createSubscription: (user, subscriptionDetails, recurly_token_id, callback)-> + isPaypal = subscriptionDetails.isPaypal + logger.log {user_id: user._id, isPaypal, recurly_token_id}, "setting up subscription in recurly" + fn = if isPaypal then RecurlyWrapper._createPaypalSubscription else RecurlyWrapper._createCreditCardSubscription + fn user, subscriptionDetails, recurly_token_id, callback apiRequest : (options, callback) -> options.url = @apiUrl + "/" + options.url @@ -60,7 +195,7 @@ module.exports = RecurlyWrapper = newAttributes[key] = value else newAttributes[newKey] = value - + return newAttributes crypto.randomBytes 32, (error, buffer) -> @@ -74,7 +209,7 @@ module.exports = RecurlyWrapper = signature = "#{signed}|#{unsignedQuery}" callback null, signature - + getSubscriptions: (accountId, callback)-> @apiRequest({ @@ -227,7 +362,7 @@ module.exports = RecurlyWrapper = }, (error, response, body) -> callback(error) ) - + redeemCoupon: (account_code, coupon_code, callback)-> requestBody = """ @@ -278,6 +413,15 @@ module.exports = RecurlyWrapper = return callback "I don't understand the response from Recurly" callback null, account + _parseBillingInfoXml: (xml, callback) -> + @_parseXml xml, (error, data) -> + return callback(error) if error? + if data? and data.account? + billingInfo = data.billing_info + else + return callback "I don't understand the response from Recurly" + callback null, billingInfo + _parseXml: (xml, callback) -> convertDataTypes = (data) -> if data? and data["$"]? @@ -299,7 +443,7 @@ module.exports = RecurlyWrapper = else array.push(convertDataTypes(value)) data = array - + if data instanceof Array data = (convertDataTypes(entry) for entry in data) else if typeof data == "object" @@ -315,6 +459,3 @@ module.exports = RecurlyWrapper = return callback(error) if error? result = convertDataTypes(data) callback null, result - - - diff --git a/services/web/public/coffee/main/new-subscription.coffee b/services/web/public/coffee/main/new-subscription.coffee index 404b512e7b..9547508337 100644 --- a/services/web/public/coffee/main/new-subscription.coffee +++ b/services/web/public/coffee/main/new-subscription.coffee @@ -115,6 +115,13 @@ define [ currencyCode:pricing.items.currency plan_code:pricing.items.plan.code coupon_code:pricing.items?.coupon?.code || "" + isPaypal: $scope.paymentMethod == 'paypal' + address: + address1: $scope.data.address1 + address2: $scope.data.address2 + country: $scope.data.country + state: $scope.data.state + zip: $scope.data.zip $http.post("/user/subscription/create", postData) .success (data, status, headers)-> sixpack.convert "in-editor-free-trial-plan", pricing.items.plan.code, (err)-> From 56bc840b88014a8868711e52e6378914bc478430 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 24 Jun 2016 14:11:28 +0100 Subject: [PATCH 02/17] WIP: fix up error handling and account for possibility of account already existing. --- .../Subscription/RecurlyWrapper.coffee | 49 ++++++++++++++++--- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index edf4dc8841..75d1f1667a 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -12,7 +12,32 @@ module.exports = RecurlyWrapper = _createPaypalSubscription: (user, subscriptionDetails, recurly_token_id, callback) -> logger.log {user_id: user._id, recurly_token_id}, "starting process of creating paypal subscription" Async.waterfall([ - (next) -> # create account + (next) -> # check if account exists + logger.log {user_id: user._id, recurly_token_id}, "checking if recurly account exists for user" + RecurlyWrapper.apiRequest({ + url: "accounts/#{user._id}" + method: "GET" + }, (error, response, responseBody) -> + if error + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while checking account" + return next(error) + result = {userExists: true} + if response.statusCode == 404 + result.userExists = false + return next(null, result) + logger.log {user_id: user._id, recurly_token_id}, "user appears to exist in recurly" + RecurlyWrapper._parseAccountXml responseBody, (err, account) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error parsing account" + return next(err) + result.account = account + return next(null, result) + ) + + , (result, next) -> # create account + if !result.userExists + logger.log {user_id: user._id, recurly_token_id}, "user already exists in recurly" + return next(null, result) logger.log {user_id: user._id, recurly_token_id}, "creating user in recurly" address = subscriptionDetails.address if !address @@ -38,14 +63,17 @@ module.exports = RecurlyWrapper = method : "POST" body : requestBody }, (error, response, responseBody) => - return next(error) if error? + if error + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while creating account" + return next(error) RecurlyWrapper._parseAccountXml responseBody, (err, account) -> if err logger.error {err, user_id: user._id, recurly_token_id}, "error creating account" return next(err) - result = {account} + result.account = account return next(null, result) ) + , (result, next) -> # create billing info logger.log {user_id: user._id, recurly_token_id}, "creating billing info in recurly" accountCode = result?.account?.account_code @@ -61,7 +89,9 @@ module.exports = RecurlyWrapper = method: "POST" body: requestBody }, (error, response, responseBody) => - return next(error) if error? + if error + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while creating billing info" + return next(error) RecurlyWrapper._parseBillingInfoXml responseBody, (err, billingInfo) -> if err logger.error {err, user_id: user._id, accountCode, recurly_token_id}, "error creating billing info" @@ -69,6 +99,7 @@ module.exports = RecurlyWrapper = result.billingInfo = billingInfo return next(null, result) ) + , (result, next) -> # set address logger.log {user_id: user._id, recurly_token_id}, "setting billing address in recurly" accountCode = result?.account?.account_code @@ -92,7 +123,9 @@ module.exports = RecurlyWrapper = method: "PUT" body: requestBody }, (error, response, responseBody) => - return next(error) if error? + if error + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while setting address" + return next(error) RecurlyWrapper._parseBillingInfoXml responseBody, (err, billingInfo) -> if err logger.error {err, user_id: user._id, recurly_token_id}, "error updating billing info" @@ -100,6 +133,7 @@ module.exports = RecurlyWrapper = result.billingInfo = billingInfo return next(null, result) ) + , (result, next) -> # create subscription logger.log {user_id: user._id, recurly_token_id}, "creating subscription in recurly" requestBody = """ @@ -117,7 +151,9 @@ module.exports = RecurlyWrapper = method : "POST" body : requestBody }, (error, response, responseBody) => - return next(error) if error? + if error + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while creating subscription" + return next(error) RecurlyWrapper._parseSubscriptionXml responseBody, (err, subscription) -> if err logger.error {err, user_id: user._id, recurly_token_id}, "error creating subscription" @@ -125,6 +161,7 @@ module.exports = RecurlyWrapper = result.subscription = subscription return next(null, result) ) + ], (err, result) -> if err logger.error {err, user_id: user._id, recurly_token_id}, "error in paypal subscription creation process" From 026e9f46c8d75bb8571ac8fb6c1aff07c6c74d90 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 24 Jun 2016 15:03:46 +0100 Subject: [PATCH 03/17] WIP: process appears to work, setting address correctly --- .../Features/Subscription/RecurlyWrapper.coffee | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index 75d1f1667a..5f0c7877f4 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -35,7 +35,7 @@ module.exports = RecurlyWrapper = ) , (result, next) -> # create account - if !result.userExists + if result.userExists logger.log {user_id: user._id, recurly_token_id}, "user already exists in recurly" return next(null, result) logger.log {user_id: user._id, recurly_token_id}, "creating user in recurly" @@ -51,9 +51,9 @@ module.exports = RecurlyWrapper =
#{address.address1} #{address.address2} - #{address.city} - #{address.state} - #{address.zip} + #{address.city || ''} + #{address.state || ''} + #{address.zip || ''} #{address.country}
@@ -112,9 +112,9 @@ module.exports = RecurlyWrapper = #{address.address1} #{address.address2} - #{address.city} - #{address.state} - #{address.zip} + #{address.city || ''} + #{address.state || ''} + #{address.zip || ''} #{address.country} """ @@ -455,6 +455,8 @@ module.exports = RecurlyWrapper = return callback(error) if error? if data? and data.account? billingInfo = data.billing_info + else if data? and data.billing_info? + billingInfo = data.billing_info else return callback "I don't understand the response from Recurly" callback null, billingInfo From 72c73809f61508efe1ff5cb3819bd30bb94ab20c Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Jun 2016 09:44:40 +0100 Subject: [PATCH 04/17] Generate address xml from object. --- .../Subscription/RecurlyWrapper.coffee | 20 +++++++------- .../coffee/main/new-subscription.coffee | 11 ++++---- .../Subscription/RecurlyWrapperTests.coffee | 26 +++++++++++++++++++ 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index 5f0c7877f4..e49cb12b39 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -9,6 +9,15 @@ Async = require('async') module.exports = RecurlyWrapper = apiUrl : "https://api.recurly.com/v2" + _addressToXml: (address) -> + allowedKeys = ['address1', 'address2', 'city', 'country', 'state', 'zip', 'postal_code'] + resultString = "\n" + for k, v of address + if v and (k in allowedKeys) + resultString += "<#{k}#{if k == 'address2' then ' nil="nil"' else ''}>#{v || ''}\n" + resultString += "\n" + return resultString + _createPaypalSubscription: (user, subscriptionDetails, recurly_token_id, callback) -> logger.log {user_id: user._id, recurly_token_id}, "starting process of creating paypal subscription" Async.waterfall([ @@ -108,16 +117,7 @@ module.exports = RecurlyWrapper = address = subscriptionDetails.address if !address return next(new Error('no address in subscriptionDetails at setAddress stage')) - requestBody = """ - - #{address.address1} - #{address.address2} - #{address.city || ''} - #{address.state || ''} - #{address.zip || ''} - #{address.country} - - """ + requestBody = RecurlyWrapper._addressToXml(address) RecurlyWrapper.apiRequest({ url: "accounts/#{accountCode}/billing_info" method: "PUT" diff --git a/services/web/public/coffee/main/new-subscription.coffee b/services/web/public/coffee/main/new-subscription.coffee index 9547508337..0dc2d68e52 100644 --- a/services/web/public/coffee/main/new-subscription.coffee +++ b/services/web/public/coffee/main/new-subscription.coffee @@ -117,11 +117,12 @@ define [ coupon_code:pricing.items?.coupon?.code || "" isPaypal: $scope.paymentMethod == 'paypal' address: - address1: $scope.data.address1 - address2: $scope.data.address2 - country: $scope.data.country - state: $scope.data.state - zip: $scope.data.zip + address1: $scope.data.address1 + address2: $scope.data.address2 + country: $scope.data.country + state: $scope.data.state + postal_code: $scope.date.postal_code + zip: $scope.data.zip $http.post("/user/subscription/create", postData) .success (data, status, headers)-> sixpack.convert "in-editor-free-trial-plan", pricing.items.plan.code, (err)-> diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index e05040772d..143085b402 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -329,5 +329,31 @@ describe "RecurlyWrapper", -> @apiRequest.called.should.equal true + describe "_addressToXml", -> + + beforeEach -> + @address = + address1: "addr_one" + address2: "addr_two" + country: "some_country" + state: "some_state" + zip: "some_zip" + nonsenseKey: "rubbish" + + it 'should generate the correct xml', () -> + result = RecurlyWrapper._addressToXml @address + should.equal( + result, + """ + + addr_one + addr_two + some_country + some_state + some_zip + \n + """ + ) + From 9938787e4acfeb266519cfb9d85c5bd0aef4df4b Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Jun 2016 10:38:10 +0100 Subject: [PATCH 05/17] fix a few silly issues. Appears to work now --- .../coffee/Features/Subscription/RecurlyWrapper.coffee | 10 +++++----- .../web/public/coffee/main/new-subscription.coffee | 2 +- .../coffee/Subscription/RecurlyWrapperTests.coffee | 2 ++ 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index e49cb12b39..69210cbd52 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -10,7 +10,7 @@ module.exports = RecurlyWrapper = apiUrl : "https://api.recurly.com/v2" _addressToXml: (address) -> - allowedKeys = ['address1', 'address2', 'city', 'country', 'state', 'zip', 'postal_code'] + allowedKeys = ['address1', 'address2', 'city', 'country', 'state', 'zip'] resultString = "\n" for k, v of address if v and (k in allowedKeys) @@ -27,13 +27,13 @@ module.exports = RecurlyWrapper = url: "accounts/#{user._id}" method: "GET" }, (error, response, responseBody) -> + result = {userExists: true} if error + if response.statusCode == 404 # actually not an error in this case, just no existing account + result.userExists = false + return next(null, result) logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while checking account" return next(error) - result = {userExists: true} - if response.statusCode == 404 - result.userExists = false - return next(null, result) logger.log {user_id: user._id, recurly_token_id}, "user appears to exist in recurly" RecurlyWrapper._parseAccountXml responseBody, (err, account) -> if err diff --git a/services/web/public/coffee/main/new-subscription.coffee b/services/web/public/coffee/main/new-subscription.coffee index 0dc2d68e52..b507690f4e 100644 --- a/services/web/public/coffee/main/new-subscription.coffee +++ b/services/web/public/coffee/main/new-subscription.coffee @@ -121,7 +121,7 @@ define [ address2: $scope.data.address2 country: $scope.data.country state: $scope.data.state - postal_code: $scope.date.postal_code + postal_code: $scope.data.postal_code zip: $scope.data.zip $http.post("/user/subscription/create", postData) .success (data, status, headers)-> diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index 143085b402..3fb5590018 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -338,6 +338,7 @@ describe "RecurlyWrapper", -> country: "some_country" state: "some_state" zip: "some_zip" + postal_code: "some_postal_code" nonsenseKey: "rubbish" it 'should generate the correct xml', () -> @@ -351,6 +352,7 @@ describe "RecurlyWrapper", -> some_country some_state some_zip + some_postal_code \n """ ) From eb92cfe8e09dde49957c1160a16bc4f44f502f89 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Jun 2016 10:40:08 +0100 Subject: [PATCH 06/17] Remove the postal_code from test, until recurly get back to us. --- .../UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index 3fb5590018..143085b402 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -338,7 +338,6 @@ describe "RecurlyWrapper", -> country: "some_country" state: "some_state" zip: "some_zip" - postal_code: "some_postal_code" nonsenseKey: "rubbish" it 'should generate the correct xml', () -> @@ -352,7 +351,6 @@ describe "RecurlyWrapper", -> some_country some_state some_zip - some_postal_code \n """ ) From 401565ba23df3c2173a9085c8370b328d2966fc2 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Jun 2016 12:14:43 +0100 Subject: [PATCH 07/17] Fix references to the RecurlyWrapper object. --- .../Subscription/RecurlyWrapper.coffee | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index 69210cbd52..41435eabe4 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -191,13 +191,13 @@ module.exports = RecurlyWrapper = """ - @apiRequest({ + RecurlyWrapper.apiRequest({ url : "subscriptions" method : "POST" body : requestBody }, (error, response, responseBody) => return callback(error) if error? - @_parseSubscriptionXml responseBody, callback + RecurlyWrapper._parseSubscriptionXml responseBody, callback ) createSubscription: (user, subscriptionDetails, recurly_token_id, callback)-> @@ -207,7 +207,7 @@ module.exports = RecurlyWrapper = fn user, subscriptionDetails, recurly_token_id, callback apiRequest : (options, callback) -> - options.url = @apiUrl + "/" + options.url + options.url = RecurlyWrapper.apiUrl + "/" + options.url options.headers = "Authorization" : "Basic " + new Buffer(Settings.apis.recurly.apiKey).toString("base64") "Accept" : "application/xml" @@ -249,11 +249,11 @@ module.exports = RecurlyWrapper = getSubscriptions: (accountId, callback)-> - @apiRequest({ + RecurlyWrapper.apiRequest({ url: "accounts/#{accountId}/subscriptions" }, (error, response, body) => return callback(error) if error? - @_parseXml body, callback + RecurlyWrapper._parseXml body, callback ) @@ -266,11 +266,11 @@ module.exports = RecurlyWrapper = else url = "subscriptions/#{subscriptionId}" - @apiRequest({ + RecurlyWrapper.apiRequest({ url: url }, (error, response, body) => return callback(error) if error? - @_parseSubscriptionXml body, (error, recurlySubscription) => + RecurlyWrapper._parseSubscriptionXml body, (error, recurlySubscription) => return callback(error) if error? if options.includeAccount if recurlySubscription.account? and recurlySubscription.account.url? @@ -278,7 +278,7 @@ module.exports = RecurlyWrapper = else return callback "I don't understand the response from Recurly" - @getAccount accountId, (error, account) -> + RecurlyWrapper.getAccount accountId, (error, account) -> return callback(error) if error? recurlySubscription.account = account callback null, recurlySubscription @@ -296,9 +296,9 @@ module.exports = RecurlyWrapper = per_page:200 if cursor? opts.qs.cursor = cursor - @apiRequest opts, (error, response, body) => + RecurlyWrapper.apiRequest opts, (error, response, body) => return callback(error) if error? - @_parseXml body, (err, data)-> + RecurlyWrapper._parseXml body, (err, data)-> if err? logger.err err:err, "could not get accoutns" callback(err) @@ -314,19 +314,19 @@ module.exports = RecurlyWrapper = getAccount: (accountId, callback) -> - @apiRequest({ + RecurlyWrapper.apiRequest({ url: "accounts/#{accountId}" }, (error, response, body) => return callback(error) if error? - @_parseAccountXml body, callback + RecurlyWrapper._parseAccountXml body, callback ) getBillingInfo: (accountId, callback)-> - @apiRequest({ + RecurlyWrapper.apiRequest({ url: "accounts/#{accountId}/billing_info" }, (error, response, body) => return callback(error) if error? - @_parseXml body, callback + RecurlyWrapper._parseXml body, callback ) @@ -338,13 +338,13 @@ module.exports = RecurlyWrapper = #{options.timeframe} """ - @apiRequest({ + RecurlyWrapper.apiRequest({ url : "subscriptions/#{subscriptionId}" method : "put" body : requestBody }, (error, response, responseBody) => return callback(error) if error? - @_parseSubscriptionXml responseBody, callback + RecurlyWrapper._parseSubscriptionXml responseBody, callback ) createFixedAmmountCoupon: (coupon_code, name, currencyCode, discount_in_cents, plan_code, callback)-> @@ -363,7 +363,7 @@ module.exports = RecurlyWrapper = """ logger.log coupon_code:coupon_code, requestBody:requestBody, "creating coupon" - @apiRequest({ + RecurlyWrapper.apiRequest({ url : "coupons" method : "post" body : requestBody @@ -375,16 +375,16 @@ module.exports = RecurlyWrapper = lookupCoupon: (coupon_code, callback)-> - @apiRequest({ + RecurlyWrapper.apiRequest({ url: "coupons/#{coupon_code}" }, (error, response, body) => return callback(error) if error? - @_parseXml body, callback + RecurlyWrapper._parseXml body, callback ) cancelSubscription: (subscriptionId, callback) -> logger.log subscriptionId:subscriptionId, "telling recurly to cancel subscription" - @apiRequest({ + RecurlyWrapper.apiRequest({ url: "subscriptions/#{subscriptionId}/cancel", method: "put" }, (error, response, body) -> @@ -393,7 +393,7 @@ module.exports = RecurlyWrapper = reactivateSubscription: (subscriptionId, callback) -> logger.log subscriptionId:subscriptionId, "telling recurly to reactivating subscription" - @apiRequest({ + RecurlyWrapper.apiRequest({ url: "subscriptions/#{subscriptionId}/reactivate", method: "put" }, (error, response, body) -> @@ -409,7 +409,7 @@ module.exports = RecurlyWrapper = """ logger.log account_code:account_code, coupon_code:coupon_code, requestBody:requestBody, "redeeming coupon for user" - @apiRequest({ + RecurlyWrapper.apiRequest({ url : "coupons/#{coupon_code}/redeem" method : "post" body : requestBody @@ -423,7 +423,7 @@ module.exports = RecurlyWrapper = next_renewal_date = new Date() next_renewal_date.setDate(next_renewal_date.getDate() + daysUntilExpire) logger.log subscriptionId:subscriptionId, daysUntilExpire:daysUntilExpire, "Exending Free trial for user" - @apiRequest({ + RecurlyWrapper.apiRequest({ url : "/subscriptions/#{subscriptionId}/postpone?next_renewal_date=#{next_renewal_date}&bulk=false" method : "put" }, (error, response, responseBody) => @@ -433,7 +433,7 @@ module.exports = RecurlyWrapper = ) _parseSubscriptionXml: (xml, callback) -> - @_parseXml xml, (error, data) -> + RecurlyWrapper._parseXml xml, (error, data) -> return callback(error) if error? if data? and data.subscription? recurlySubscription = data.subscription @@ -442,7 +442,7 @@ module.exports = RecurlyWrapper = callback null, recurlySubscription _parseAccountXml: (xml, callback) -> - @_parseXml xml, (error, data) -> + RecurlyWrapper._parseXml xml, (error, data) -> return callback(error) if error? if data? and data.account? account = data.account @@ -451,7 +451,7 @@ module.exports = RecurlyWrapper = callback null, account _parseBillingInfoXml: (xml, callback) -> - @_parseXml xml, (error, data) -> + RecurlyWrapper._parseXml xml, (error, data) -> return callback(error) if error? if data? and data.account? billingInfo = data.billing_info From d853eb5916876ae65a0a4f7397c99b22631dc189 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Jun 2016 13:54:54 +0100 Subject: [PATCH 08/17] Update existing RecurlyWrapper tests. --- .../Subscription/RecurlyWrapperTests.coffee | 165 ++++++++++++------ 1 file changed, 115 insertions(+), 50 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index 143085b402..4b1880cf00 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -2,8 +2,8 @@ should = require('chai').should() sinon = require 'sinon' crypto = require 'crypto' querystring = require 'querystring' -RecurlyWrapper = require "../../../../app/js/Features/Subscription/RecurlyWrapper" -Settings = require "settings-sharelatex" +modulePath = "../../../../app/js/Features/Subscription/RecurlyWrapper" +SandboxedModule = require('sandboxed-module') tk = require("timekeeper") fixtures = @@ -97,22 +97,37 @@ mockApiRequest = (options, callback) -> describe "RecurlyWrapper", -> - beforeEach -> - Settings.plans = [{ - planCode: "collaborator" - name: "Collaborator" - features: - collaborators: -1 - versioning: true - }] - Settings.defaultPlanCode = - collaborators: 0 - versioning: false + + before -> + @settings = + plans: [{ + planCode: "collaborator" + name: "Collaborator" + features: + collaborators: -1 + versioning: true + }] + defaultPlanCode: + collaborators: 0 + versioning: false + apis: + recurly: + apiKey: 'nonsense' + privateKey: 'private_nonsense' + + @RecurlyWrapper = RecurlyWrapper = SandboxedModule.require modulePath, requires: + "settings-sharelatex": @settings + "logger-sharelatex": + err: sinon.stub() + error: sinon.stub() + log: sinon.stub() + "request": sinon.stub() describe "sign", -> + before (done) -> tk.freeze Date.now() # freeze the time for these tests - RecurlyWrapper.sign({ + @RecurlyWrapper.sign({ subscription : plan_code : "gold" name : "$$$" @@ -127,7 +142,7 @@ describe "RecurlyWrapper", -> it "should be signed correctly", -> signed = @signature.split("|")[0] query = @signature.split("|")[1] - crypto.createHmac("sha1", Settings.apis.recurly.privateKey).update(query).digest("hex").should.equal signed + crypto.createHmac("sha1", @settings.apis.recurly.privateKey).update(query).digest("hex").should.equal signed it "should be url escaped", -> query = @signature.split("|")[1] @@ -180,7 +195,7 @@ describe "RecurlyWrapper", -> " " + " " + "" - RecurlyWrapper._parseXml xml, (error, data) -> + @RecurlyWrapper._parseXml xml, (error, data) -> data.subscription.plan.plan_code.should.equal "gold" data.subscription.plan.name.should.equal "Gold plan" data.subscription.uuid.should.equal "44f83d7cba354d5b84812419f923ea96" @@ -188,32 +203,36 @@ describe "RecurlyWrapper", -> data.subscription.unit_amount_in_cents.should.equal 800 data.subscription.currency.should.equal "EUR" data.subscription.quantity.should.equal 1 + data.subscription.activated_at.should.deep.equal new Date("2011-05-27T07:00:00Z") should.equal data.subscription.canceled_at, null should.equal data.subscription.expires_at, null + data.subscription.current_period_started_at.should.deep.equal new Date("2011-06-27T07:00:00Z") + data.subscription.current_period_ends_at.should.deep.equal new Date("2011-07-27T07:00:00Z") should.equal data.subscription.trial_started_at, null should.equal data.subscription.trial_ends_at, null - data.subscription.subscription_add_ons.should.deep.equal [{ + + data.subscription.subscription_add_ons[0].should.deep.equal { add_on_code: "ipaddresses" quantity: "10" unit_amount_in_cents: "150" - }] + } data.subscription.account.url.should.equal "https://api.recurly.com/v2/accounts/1" data.subscription.url.should.equal "https://api.recurly.com/v2/subscriptions/44f83d7cba354d5b84812419f923ea96" data.subscription.plan.url.should.equal "https://api.recurly.com/v2/plans/gold" done() - + describe "getSubscription", -> describe "with proper subscription id", -> before -> - @apiRequest = sinon.stub(RecurlyWrapper, "apiRequest", mockApiRequest) - RecurlyWrapper.getSubscription "44f83d7cba354d5b84812419f923ea96", (error, recurlySubscription) => + @apiRequest = sinon.stub(@RecurlyWrapper, "apiRequest", mockApiRequest) + @RecurlyWrapper.getSubscription "44f83d7cba354d5b84812419f923ea96", (error, recurlySubscription) => @recurlySubscription = recurlySubscription after -> - RecurlyWrapper.apiRequest.restore() - + @RecurlyWrapper.apiRequest.restore() + it "should look up the subscription at the normal API end point", -> @apiRequest.args[0][0].url.should.equal "subscriptions/44f83d7cba354d5b84812419f923ea96" @@ -222,12 +241,12 @@ describe "RecurlyWrapper", -> describe "with ReculyJS token", -> before -> - @apiRequest = sinon.stub(RecurlyWrapper, "apiRequest", mockApiRequest) - RecurlyWrapper.getSubscription "70db44b10f5f4b238669480c9903f6f5", {recurlyJsResult: true}, (error, recurlySubscription) => + @apiRequest = sinon.stub(@RecurlyWrapper, "apiRequest", mockApiRequest) + @RecurlyWrapper.getSubscription "70db44b10f5f4b238669480c9903f6f5", {recurlyJsResult: true}, (error, recurlySubscription) => @recurlySubscription = recurlySubscription after -> - RecurlyWrapper.apiRequest.restore() - + @RecurlyWrapper.apiRequest.restore() + it "should return the subscription", -> @recurlySubscription.uuid.should.equal "44f83d7cba354d5b84812419f923ea96" @@ -236,30 +255,30 @@ describe "RecurlyWrapper", -> describe "with includeAccount", -> beforeEach -> - @apiRequest = sinon.stub(RecurlyWrapper, "apiRequest", mockApiRequest) - RecurlyWrapper.getSubscription "44f83d7cba354d5b84812419f923ea96", {includeAccount: true}, (error, recurlySubscription) => + @apiRequest = sinon.stub(@RecurlyWrapper, "apiRequest", mockApiRequest) + @RecurlyWrapper.getSubscription "44f83d7cba354d5b84812419f923ea96", {includeAccount: true}, (error, recurlySubscription) => @recurlySubscription = recurlySubscription afterEach -> - RecurlyWrapper.apiRequest.restore() + @RecurlyWrapper.apiRequest.restore() it "should request the account from the API", -> @apiRequest.args[1][0].url.should.equal "accounts/104" - + it "should populate the account attribute", -> @recurlySubscription.account.account_code.should.equal "104" - + describe "updateSubscription", -> beforeEach (done) -> @recurlySubscriptionId = "subscription-id-123" - @apiRequest = sinon.stub RecurlyWrapper, "apiRequest", (options, callback) => + @apiRequest = sinon.stub @RecurlyWrapper, "apiRequest", (options, callback) => @requestOptions = options callback null, {}, fixtures["subscriptions/44f83d7cba354d5b84812419f923ea96"] - RecurlyWrapper.updateSubscription @recurlySubscriptionId, { plan_code : "silver", timeframe: "now" }, (error, recurlySubscription) => + @RecurlyWrapper.updateSubscription @recurlySubscriptionId, { plan_code : "silver", timeframe: "now" }, (error, recurlySubscription) => @recurlySubscription = recurlySubscription done() afterEach -> - RecurlyWrapper.apiRequest.restore() + @RecurlyWrapper.apiRequest.restore() it "should send an update request to the API", -> @apiRequest.called.should.equal true @@ -275,59 +294,58 @@ describe "RecurlyWrapper", -> it "should return the updated subscription", -> should.exist @recurlySubscription @recurlySubscription.plan.plan_code.should.equal "gold" - + describe "cancelSubscription", -> beforeEach (done) -> @recurlySubscriptionId = "subscription-id-123" - @apiRequest = sinon.stub RecurlyWrapper, "apiRequest", (options, callback) => + @apiRequest = sinon.stub @RecurlyWrapper, "apiRequest", (options, callback) => options.url.should.equal "subscriptions/#{@recurlySubscriptionId}/cancel" options.method.should.equal "put" callback() - RecurlyWrapper.cancelSubscription(@recurlySubscriptionId, done) + @RecurlyWrapper.cancelSubscription(@recurlySubscriptionId, done) afterEach -> - RecurlyWrapper.apiRequest.restore() + @RecurlyWrapper.apiRequest.restore() it "should send a cancel request to the API", -> @apiRequest.called.should.equal true - + describe "reactivateSubscription", -> beforeEach (done) -> @recurlySubscriptionId = "subscription-id-123" - @apiRequest = sinon.stub RecurlyWrapper, "apiRequest", (options, callback) => + @apiRequest = sinon.stub @RecurlyWrapper, "apiRequest", (options, callback) => options.url.should.equal "subscriptions/#{@recurlySubscriptionId}/reactivate" options.method.should.equal "put" callback() - RecurlyWrapper.reactivateSubscription(@recurlySubscriptionId, done) + @RecurlyWrapper.reactivateSubscription(@recurlySubscriptionId, done) afterEach -> - RecurlyWrapper.apiRequest.restore() + @RecurlyWrapper.apiRequest.restore() it "should send a cancel request to the API", -> @apiRequest.called.should.equal true - - + + describe "redeemCoupon", -> beforeEach (done) -> @recurlyAccountId = "account-id-123" @coupon_code = "312321312" - @apiRequest = sinon.stub RecurlyWrapper, "apiRequest", (options, callback) => + @apiRequest = sinon.stub @RecurlyWrapper, "apiRequest", (options, callback) => options.url.should.equal "coupons/#{@coupon_code}/redeem" options.body.indexOf("#{@recurlyAccountId}").should.not.equal -1 options.body.indexOf("USD").should.not.equal -1 options.method.should.equal "post" callback() - RecurlyWrapper.redeemCoupon(@recurlyAccountId, @coupon_code, done) + @RecurlyWrapper.redeemCoupon(@recurlyAccountId, @coupon_code, done) afterEach -> - RecurlyWrapper.apiRequest.restore() + @RecurlyWrapper.apiRequest.restore() it "should send the request to redem the coupon", -> @apiRequest.called.should.equal true - describe "_addressToXml", -> @@ -341,7 +359,7 @@ describe "RecurlyWrapper", -> nonsenseKey: "rubbish" it 'should generate the correct xml', () -> - result = RecurlyWrapper._addressToXml @address + result = @RecurlyWrapper._addressToXml @address should.equal( result, """ @@ -355,5 +373,52 @@ describe "RecurlyWrapper", -> """ ) + describe 'createSubscription', -> + + beforeEach -> + @user = + _id: 'some_id' + email: 'user@example.com' + @subscriptionDetails = + currencyCode: "EUR" + plan_code: "some_plan_code" + coupon_code: "" + isPaypal: true + address: + address1: "addr_one" + address2: "addr_two" + country: "some_country" + state: "some_state" + zip: "some_zip" + @recurly_token_id = "a-token-id" + describe 'when all goes well', -> + + beforeEach -> + + describe 'when paypal', -> + beforeEach -> + + describe 'when not paypal', -> + beforeEach -> + + describe '_createCreditCardSubscription', -> + + beforeEach -> + + describe 'when all goes well', -> + + beforeEach -> + + describe 'when api request produces an error', -> + + beforeEach -> + + describe '_createPaypalSubscription', -> + + beforeEach -> + + describe 'when all goes well', -> + + beforeEach -> From 95d85538cc1bca4c19699341d9890be6fc846539 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Jun 2016 14:00:30 +0100 Subject: [PATCH 09/17] Clean up xml text block in test. --- .../Subscription/RecurlyWrapperTests.coffee | 64 ++++++++++--------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index 4b1880cf00..821fddb3e9 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -164,37 +164,38 @@ describe "RecurlyWrapper", -> describe "_parseXml", -> it "should convert different data types into correct representations", (done) -> - xml = - "" + - "" + - " " + - " " + - " gold" + - " Gold plan" + - " " + - " 44f83d7cba354d5b84812419f923ea96" + - " active" + - " 800" + - " EUR" + - " 1" + - " 2011-05-27T07:00:00Z" + - " " + - " " + - " 2011-06-27T07:00:00Z" + - " 2011-07-27T07:00:00Z" + - " " + - " " + - " " + - " " + - " ipaddresses" + - " 10" + - " 150" + - " " + - " " + - " " + - " " + - " " + - "" + xml = """ + + + + + gold + Gold plan + + 44f83d7cba354d5b84812419f923ea96 + active + 800 + EUR + 1 + 2011-05-27T07:00:00Z + + + 2011-06-27T07:00:00Z + 2011-07-27T07:00:00Z + + + + + ipaddresses + 10 + 150 + + + + + + + """ @RecurlyWrapper._parseXml xml, (error, data) -> data.subscription.plan.plan_code.should.equal "gold" data.subscription.plan.name.should.equal "Gold plan" @@ -225,6 +226,7 @@ describe "RecurlyWrapper", -> done() describe "getSubscription", -> + describe "with proper subscription id", -> before -> @apiRequest = sinon.stub(@RecurlyWrapper, "apiRequest", mockApiRequest) From 2c1b3266818c022ed6ef3ce5e9c9f1f3fa6dc130 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Jun 2016 14:29:19 +0100 Subject: [PATCH 10/17] test the `createSubscription` function. --- .../Subscription/RecurlyWrapperTests.coffee | 75 ++++++++++++++++++- 1 file changed, 71 insertions(+), 4 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index 821fddb3e9..f19a4389aa 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -1,4 +1,5 @@ should = require('chai').should() +expect = require('chai').expect sinon = require 'sinon' crypto = require 'crypto' querystring = require 'querystring' @@ -392,18 +393,84 @@ describe "RecurlyWrapper", -> country: "some_country" state: "some_state" zip: "some_zip" + @subscription = {} @recurly_token_id = "a-token-id" + @call = (callback) => + @RecurlyWrapper.createSubscription(@user, @subscriptionDetails, @recurly_token_id, callback) - describe 'when all goes well', -> + describe 'when paypal', -> beforeEach -> + @subscriptionDetails.isPaypal = true + @_createPaypalSubscription = sinon.stub(@RecurlyWrapper, '_createPaypalSubscription') + @_createPaypalSubscription.callsArgWith(3, null, @subscription) - describe 'when paypal', -> - beforeEach -> + afterEach -> + @_createPaypalSubscription.restore() + + it 'should not produce an error', (done) -> + @call (err, sub) => + expect(err).to.equal null + expect(err).to.not.be.instanceof Error + done() + + it 'should produce a subscription object', (done) -> + @call (err, sub) => + expect(sub).to.deep.equal @subscription + done() + + it 'should call _createPaypalSubscription', (done) -> + @call (err, sub) => + @_createPaypalSubscription.callCount.should.equal 1 + done() + + describe "when _createPaypalSubscription produces an error", -> - describe 'when not paypal', -> beforeEach -> + @_createPaypalSubscription.callsArgWith(3, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err, sub) => + expect(err).to.be.instanceof Error + done() + + describe 'when not paypal', -> + + beforeEach -> + @subscriptionDetails.isPaypal = false + @_createCreditCardSubscription = sinon.stub(@RecurlyWrapper, '_createCreditCardSubscription') + @_createCreditCardSubscription.callsArgWith(3, null, @subscription) + + afterEach -> + @_createCreditCardSubscription.restore() + + it 'should not produce an error', (done) -> + @call (err, sub) => + expect(err).to.equal null + expect(err).to.not.be.instanceof Error + done() + + it 'should produce a subscription object', (done) -> + @call (err, sub) => + expect(sub).to.deep.equal @subscription + done() + + it 'should call _createCreditCardSubscription', (done) -> + @call (err, sub) => + @_createCreditCardSubscription.callCount.should.equal 1 + done() + + describe "when _createCreditCardSubscription produces an error", -> + + beforeEach -> + @_createCreditCardSubscription.callsArgWith(3, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err, sub) => + expect(err).to.be.instanceof Error + done() + describe '_createCreditCardSubscription', -> From 3bf8da3e83725faf80b18b75e1861b3967d50a2e Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Jun 2016 14:45:17 +0100 Subject: [PATCH 11/17] test _createCreditCardSubscription --- .../Subscription/RecurlyWrapperTests.coffee | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index f19a4389aa..fd8bbe25ed 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -475,14 +475,66 @@ describe "RecurlyWrapper", -> describe '_createCreditCardSubscription', -> beforeEach -> + @user = + _id: 'some_id' + email: 'user@example.com' + @subscriptionDetails = + currencyCode: "EUR" + plan_code: "some_plan_code" + coupon_code: "" + isPaypal: true + address: + address1: "addr_one" + address2: "addr_two" + country: "some_country" + state: "some_state" + zip: "some_zip" + @subscription = {} + @recurly_token_id = "a-token-id" + @apiRequest = sinon.stub(@RecurlyWrapper, 'apiRequest') + @response = + statusCode: 200 + @body = "is_bad" + @apiRequest.callsArgWith(1, null, @response, @body) + @_parseSubscriptionXml = sinon.stub(@RecurlyWrapper, '_parseSubscriptionXml') + @_parseSubscriptionXml.callsArgWith(1, null, @subscription) + @call = (callback) => + @RecurlyWrapper._createCreditCardSubscription(@user, @subscriptionDetails, @recurly_token_id, callback) - describe 'when all goes well', -> + afterEach -> + @apiRequest.restore() + @_parseSubscriptionXml.restore() - beforeEach -> + it 'should not produce an error', (done) -> + @call (err, sub) => + expect(err).to.not.be.instanceof Error + expect(err).to.equal null + done() + + it 'should produce a subscription', (done) -> + @call (err, sub) => + expect(sub).to.equal @subscription + done() describe 'when api request produces an error', -> beforeEach -> + @apiRequest.callsArgWith(1, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err, sub) => + expect(err).to.be.instanceof Error + done() + + describe 'when parse xml produces an error', -> + + beforeEach -> + @_parseSubscriptionXml.callsArgWith(1, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err, sub) => + expect(err).to.be.instanceof Error + done() describe '_createPaypalSubscription', -> From 709f8f2bea6b273e3b966768b15db427a717fc33 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Jun 2016 16:34:00 +0100 Subject: [PATCH 12/17] start testing the paypal workflow. --- .../Subscription/RecurlyWrapper.coffee | 320 +++++++++--------- .../Subscription/RecurlyWrapperTests.coffee | 86 ++++- 2 files changed, 253 insertions(+), 153 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index 41435eabe4..841ee0a77e 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -18,160 +18,178 @@ module.exports = RecurlyWrapper = resultString += "\n" return resultString + _paypal: + checkAccountExists: (cache, next) -> + user = cache.user + recurly_token_id = cache.recurly_token_id + subscriptionDetails = cache.subscriptionDetails + logger.log {user_id: user._id, recurly_token_id}, "checking if recurly account exists for user" + RecurlyWrapper.apiRequest({ + url: "accounts/#{user._id}" + method: "GET" + }, (error, response, responseBody) -> + if error + if response.statusCode == 404 # actually not an error in this case, just no existing account + cache.userExists = false + return next(null, cache) + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while checking account" + return next(error) + logger.log {user_id: user._id, recurly_token_id}, "user appears to exist in recurly" + RecurlyWrapper._parseAccountXml responseBody, (err, account) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error parsing account" + return next(err) + cache.account = account + return next(null, cache) + ) + createAccount: (cache, next) -> + user = cache.user + recurly_token_id = cache.recurly_token_id + subscriptionDetails = cache.subscriptionDetails + if cache.userExists + logger.log {user_id: user._id, recurly_token_id}, "user already exists in recurly" + return next(null, cache) + logger.log {user_id: user._id, recurly_token_id}, "creating user in recurly" + address = subscriptionDetails.address + if !address + return next(new Error('no address in subscriptionDetails at createAccount stage')) + requestBody = """ + + #{user._id} + #{user.email} + #{user.first_name} + #{user.last_name} +
+ #{address.address1} + #{address.address2} + #{address.city || ''} + #{address.state || ''} + #{address.zip || ''} + #{address.country} +
+ + """ + RecurlyWrapper.apiRequest({ + url : "accounts" + method : "POST" + body : requestBody + }, (error, response, responseBody) => + if error + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while creating account" + return next(error) + RecurlyWrapper._parseAccountXml responseBody, (err, account) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error creating account" + return next(err) + cache.account = account + return next(null, cache) + ) + createBillingInfo: (cache, next) -> + user = cache.user + recurly_token_id = cache.recurly_token_id + subscriptionDetails = cache.subscriptionDetails + logger.log {user_id: user._id, recurly_token_id}, "creating billing info in recurly" + accountCode = cache?.account?.account_code + if !accountCode + return next(new Error('no account code at createBillingInfo stage')) + requestBody = """ + + #{recurly_token_id} + + """ + RecurlyWrapper.apiRequest({ + url: "accounts/#{accountCode}/billing_info" + method: "POST" + body: requestBody + }, (error, response, responseBody) => + if error + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while creating billing info" + return next(error) + RecurlyWrapper._parseBillingInfoXml responseBody, (err, billingInfo) -> + if err + logger.error {err, user_id: user._id, accountCode, recurly_token_id}, "error creating billing info" + return next(err) + cache.billingInfo = billingInfo + return next(null, cache) + ) + + setAddress: (cache, next) -> + user = cache.user + recurly_token_id = cache.recurly_token_id + subscriptionDetails = cache.subscriptionDetails + logger.log {user_id: user._id, recurly_token_id}, "setting billing address in recurly" + accountCode = cache?.account?.account_code + if !accountCode + return next(new Error('no account code at setAddress stage')) + address = subscriptionDetails.address + if !address + return next(new Error('no address in subscriptionDetails at setAddress stage')) + requestBody = RecurlyWrapper._addressToXml(address) + RecurlyWrapper.apiRequest({ + url: "accounts/#{accountCode}/billing_info" + method: "PUT" + body: requestBody + }, (error, response, responseBody) => + if error + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while setting address" + return next(error) + RecurlyWrapper._parseBillingInfoXml responseBody, (err, billingInfo) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error updating billing info" + return next(err) + cache.billingInfo = billingInfo + return next(null, cache) + ) + createSubscription: (cache, next) -> + user = cache.user + recurly_token_id = cache.recurly_token_id + subscriptionDetails = cache.subscriptionDetails + logger.log {user_id: user._id, recurly_token_id}, "creating subscription in recurly" + requestBody = """ + + #{subscriptionDetails.plan_code} + #{subscriptionDetails.currencyCode} + #{subscriptionDetails.coupon_code} + + #{user._id} + + + """ # TODO: check account details and billing + RecurlyWrapper.apiRequest({ + url : "subscriptions" + method : "POST" + body : requestBody + }, (error, response, responseBody) => + if error + logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while creating subscription" + return next(error) + RecurlyWrapper._parseSubscriptionXml responseBody, (err, subscription) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error creating subscription" + return next(err) + cache.subscription = subscription + return next(null, cache) + ) + _createPaypalSubscription: (user, subscriptionDetails, recurly_token_id, callback) -> logger.log {user_id: user._id, recurly_token_id}, "starting process of creating paypal subscription" + cache = {user, recurly_token_id, subscriptionDetails} Async.waterfall([ - (next) -> # check if account exists - logger.log {user_id: user._id, recurly_token_id}, "checking if recurly account exists for user" - RecurlyWrapper.apiRequest({ - url: "accounts/#{user._id}" - method: "GET" - }, (error, response, responseBody) -> - result = {userExists: true} - if error - if response.statusCode == 404 # actually not an error in this case, just no existing account - result.userExists = false - return next(null, result) - logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while checking account" - return next(error) - logger.log {user_id: user._id, recurly_token_id}, "user appears to exist in recurly" - RecurlyWrapper._parseAccountXml responseBody, (err, account) -> - if err - logger.error {err, user_id: user._id, recurly_token_id}, "error parsing account" - return next(err) - result.account = account - return next(null, result) - ) - - , (result, next) -> # create account - if result.userExists - logger.log {user_id: user._id, recurly_token_id}, "user already exists in recurly" - return next(null, result) - logger.log {user_id: user._id, recurly_token_id}, "creating user in recurly" - address = subscriptionDetails.address - if !address - return next(new Error('no address in subscriptionDetails at createAccount stage')) - requestBody = """ - - #{user._id} - #{user.email} - #{user.first_name} - #{user.last_name} -
- #{address.address1} - #{address.address2} - #{address.city || ''} - #{address.state || ''} - #{address.zip || ''} - #{address.country} -
- - """ - RecurlyWrapper.apiRequest({ - url : "accounts" - method : "POST" - body : requestBody - }, (error, response, responseBody) => - if error - logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while creating account" - return next(error) - RecurlyWrapper._parseAccountXml responseBody, (err, account) -> - if err - logger.error {err, user_id: user._id, recurly_token_id}, "error creating account" - return next(err) - result.account = account - return next(null, result) - ) - - , (result, next) -> # create billing info - logger.log {user_id: user._id, recurly_token_id}, "creating billing info in recurly" - accountCode = result?.account?.account_code - if !accountCode - return next(new Error('no account code at createBillingInfo stage')) - requestBody = """ - - #{recurly_token_id} - - """ - RecurlyWrapper.apiRequest({ - url: "accounts/#{accountCode}/billing_info" - method: "POST" - body: requestBody - }, (error, response, responseBody) => - if error - logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while creating billing info" - return next(error) - RecurlyWrapper._parseBillingInfoXml responseBody, (err, billingInfo) -> - if err - logger.error {err, user_id: user._id, accountCode, recurly_token_id}, "error creating billing info" - return next(err) - result.billingInfo = billingInfo - return next(null, result) - ) - - , (result, next) -> # set address - logger.log {user_id: user._id, recurly_token_id}, "setting billing address in recurly" - accountCode = result?.account?.account_code - if !accountCode - return next(new Error('no account code at setAddress stage')) - address = subscriptionDetails.address - if !address - return next(new Error('no address in subscriptionDetails at setAddress stage')) - requestBody = RecurlyWrapper._addressToXml(address) - RecurlyWrapper.apiRequest({ - url: "accounts/#{accountCode}/billing_info" - method: "PUT" - body: requestBody - }, (error, response, responseBody) => - if error - logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while setting address" - return next(error) - RecurlyWrapper._parseBillingInfoXml responseBody, (err, billingInfo) -> - if err - logger.error {err, user_id: user._id, recurly_token_id}, "error updating billing info" - return next(err) - result.billingInfo = billingInfo - return next(null, result) - ) - - , (result, next) -> # create subscription - logger.log {user_id: user._id, recurly_token_id}, "creating subscription in recurly" - requestBody = """ - - #{subscriptionDetails.plan_code} - #{subscriptionDetails.currencyCode} - #{subscriptionDetails.coupon_code} - - #{user._id} - - - """ # TODO: check account details and billing - RecurlyWrapper.apiRequest({ - url : "subscriptions" - method : "POST" - body : requestBody - }, (error, response, responseBody) => - if error - logger.error {error, user_id: user._id, recurly_token_id}, "error response from recurly while creating subscription" - return next(error) - RecurlyWrapper._parseSubscriptionXml responseBody, (err, subscription) -> - if err - logger.error {err, user_id: user._id, recurly_token_id}, "error creating subscription" - return next(err) - result.subscription = subscription - return next(null, result) - ) - - ], (err, result) -> - if err - logger.error {err, user_id: user._id, recurly_token_id}, "error in paypal subscription creation process" - return callback(err) - if !result.subscription - err = new Error('no subscription object in result') - logger.error {err, user_id: user._id, recurly_token_id}, "error in paypal subscription creation process" - return callback(err) - logger.log {user_id: user._id, recurly_token_id}, "done creating paypal subscription for user" - callback(null, result.subscription) + Async.apply(RecurlyWrapper._paypal.checkAccountExists, cache), + RecurlyWrapper._paypal.createAccount, + RecurlyWrapper._paypal.createBillingInfo, + RecurlyWrapper._paypal.setAddress, + RecurlyWrapper._paypal.createSubscription, + ], (err, result) -> + if err + logger.error {err, user_id: user._id, recurly_token_id}, "error in paypal subscription creation process" + return callback(err) + if !result.subscription + err = new Error('no subscription object in result') + logger.error {err, user_id: user._id, recurly_token_id}, "error in paypal subscription creation process" + return callback(err) + logger.log {user_id: user._id, recurly_token_id}, "done creating paypal subscription for user" + callback(null, result.subscription) ) _createCreditCardSubscription: (user, subscriptionDetails, recurly_token_id, callback) -> diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index fd8bbe25ed..e9dae2cd66 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -516,6 +516,16 @@ describe "RecurlyWrapper", -> expect(sub).to.equal @subscription done() + it 'should call apiRequest', (done) -> + @call (err, sub) => + @apiRequest.callCount.should.equal 1 + done() + + it 'should call _parseSubscriptionXml', (done) -> + @call (err, sub) => + @_parseSubscriptionXml.callCount.should.equal 1 + done() + describe 'when api request produces an error', -> beforeEach -> @@ -526,6 +536,16 @@ describe "RecurlyWrapper", -> expect(err).to.be.instanceof Error done() + it 'should call apiRequest', (done) -> + @call (err, sub) => + @apiRequest.callCount.should.equal 1 + done() + + it 'should not _parseSubscriptionXml', (done) -> + @call (err, sub) => + @_parseSubscriptionXml.callCount.should.equal 0 + done() + describe 'when parse xml produces an error', -> beforeEach -> @@ -539,7 +559,69 @@ describe "RecurlyWrapper", -> describe '_createPaypalSubscription', -> beforeEach -> + @checkAccountExists = sinon.stub(@RecurlyWrapper._paypal, 'checkAccountExists') + @createAccount = sinon.stub(@RecurlyWrapper._paypal, 'createAccount') + @createBillingInfo = sinon.stub(@RecurlyWrapper._paypal, 'createBillingInfo') + @setAddress = sinon.stub(@RecurlyWrapper._paypal, 'setAddress') + @createSubscription = sinon.stub(@RecurlyWrapper._paypal, 'createSubscription') + @user = + _id: 'some_id' + email: 'user@example.com' + @subscriptionDetails = + currencyCode: "EUR" + plan_code: "some_plan_code" + coupon_code: "" + isPaypal: true + address: + address1: "addr_one" + address2: "addr_two" + country: "some_country" + state: "some_state" + zip: "some_zip" + @subscription = {} + @recurly_token_id = "a-token-id" - describe 'when all goes well', -> + # set up data callbacks + user = @user + subscriptionDetails = @subscriptionDetails + recurly_token_id = @recurly_token_id - beforeEach -> + @checkAccountExists.callsArgWith(1, null, + {user, subscriptionDetails, recurly_token_id, + userExists: false, account: {accountCode: 'xx'}} + ) + + @createAccount.callsArgWith(1, null, + {user, subscriptionDetails, recurly_token_id, + userExists: false, account: {accountCode: 'xx'}} + ) + + @createBillingInfo.callsArgWith(1, null, + {user, subscriptionDetails, recurly_token_id, + userExists: false, account: {accountCode: 'xx'}, billingInfo: {token_id: 'abc'}} + ) + + @setAddress.callsArgWith(1, null, + {user, subscriptionDetails, recurly_token_id, + userExists: false, account: {accountCode: 'xx'}, billingInfo: {token_id: 'abc'}} + ) + + @createSubscription.callsArgWith(1, null, + {user, subscriptionDetails, recurly_token_id, + userExists: false, account: {accountCode: 'xx'}, billingInfo: {token_id: 'abc'}, subscription: {}} + ) + + @call = (callback) => + @RecurlyWrapper._createPaypalSubscription @user, @subscriptionDetails, @recurly_token_id, callback + + afterEach -> + @checkAccountExists.restore() + @createAccount.restore() + @createBillingInfo.restore() + @setAddress.restore() + @createSubscription.restore() + + it 'should not produce an error', (done) -> + @call (err, sub) => + expect(err).to.not.be.instanceof Error + done() From d21eb1b07f0edbc91d6a1f6827416265ca5b9a9b Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 28 Jun 2016 09:04:19 +0100 Subject: [PATCH 13/17] Add tests --- .../Subscription/RecurlyWrapperTests.coffee | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index e9dae2cd66..02450ea984 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -590,25 +590,21 @@ describe "RecurlyWrapper", -> {user, subscriptionDetails, recurly_token_id, userExists: false, account: {accountCode: 'xx'}} ) - @createAccount.callsArgWith(1, null, {user, subscriptionDetails, recurly_token_id, userExists: false, account: {accountCode: 'xx'}} ) - @createBillingInfo.callsArgWith(1, null, {user, subscriptionDetails, recurly_token_id, userExists: false, account: {accountCode: 'xx'}, billingInfo: {token_id: 'abc'}} ) - @setAddress.callsArgWith(1, null, {user, subscriptionDetails, recurly_token_id, userExists: false, account: {accountCode: 'xx'}, billingInfo: {token_id: 'abc'}} ) - @createSubscription.callsArgWith(1, null, {user, subscriptionDetails, recurly_token_id, - userExists: false, account: {accountCode: 'xx'}, billingInfo: {token_id: 'abc'}, subscription: {}} + userExists: false, account: {accountCode: 'xx'}, billingInfo: {token_id: 'abc'}, subscription: @subscription} ) @call = (callback) => @@ -625,3 +621,18 @@ describe "RecurlyWrapper", -> @call (err, sub) => expect(err).to.not.be.instanceof Error done() + + it 'should produce a subscription object', (done) -> + @call (err, sub) => + expect(sub).to.not.equal null + expect(sub).to.equal @subscription + done() + + it 'should call each of the paypal functions', (done) -> + @call (err, sub) => + @checkAccountExists.callCount.should.equal 1 + @createAccount.callCount.should.equal 1 + @createBillingInfo.callCount.should.equal 1 + @setAddress.callCount.should.equal 1 + @createSubscription.callCount.should.equal 1 + done() From 6bdfedc1b0587316ad4900a27432481e2661fd34 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 28 Jun 2016 09:09:57 +0100 Subject: [PATCH 14/17] Test when a paypal stage produces an error --- .../Subscription/RecurlyWrapperTests.coffee | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index 02450ea984..d7099f255c 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -628,7 +628,7 @@ describe "RecurlyWrapper", -> expect(sub).to.equal @subscription done() - it 'should call each of the paypal functions', (done) -> + it 'should call each of the paypal stages', (done) -> @call (err, sub) => @checkAccountExists.callCount.should.equal 1 @createAccount.callCount.should.equal 1 @@ -636,3 +636,22 @@ describe "RecurlyWrapper", -> @setAddress.callCount.should.equal 1 @createSubscription.callCount.should.equal 1 done() + + describe 'when one of the paypal stages produces an error', -> + + beforeEach -> + @createAccount.callsArgWith(1, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err, sub) => + expect(err).to.be.instanceof Error + done() + + it 'should stop calling the paypal stages after the error', (done) -> + @call (err, sub) => + @checkAccountExists.callCount.should.equal 1 + @createAccount.callCount.should.equal 1 + @createBillingInfo.callCount.should.equal 0 + @setAddress.callCount.should.equal 0 + @createSubscription.callCount.should.equal 0 + done() From e194de50cf1e6e3952c54af81955a4ac59ad877c Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 28 Jun 2016 10:17:06 +0100 Subject: [PATCH 15/17] test `_paypal.checAccountExists`. --- .../Subscription/RecurlyWrapperTests.coffee | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index d7099f255c..4ab866b408 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -655,3 +655,103 @@ describe "RecurlyWrapper", -> @setAddress.callCount.should.equal 0 @createSubscription.callCount.should.equal 0 done() + + describe '_paypal.checkAccountExists', -> + + beforeEach -> + @apiRequest = sinon.stub(@RecurlyWrapper, 'apiRequest') + @_parseAccountXml = sinon.spy(@RecurlyWrapper, '_parseAccountXml') + @cache = + user: @user = {_id: 'some_id'} + recurly_token_id: @recurly_token_id = "some_token" + subscriptionDetails: @subscriptionDetails = + currencyCode: "EUR" + plan_code: "some_plan_code" + coupon_code: "" + isPaypal: true + address: + address1: "addr_one" + address2: "addr_two" + country: "some_country" + state: "some_state" + zip: "some_zip" + @call = (callback) => + @RecurlyWrapper._paypal.checkAccountExists @cache, callback + + afterEach -> + @apiRequest.restore() + @_parseAccountXml.restore() + + describe 'when the account exists', -> + + beforeEach -> + resultXml = 'abc' + @apiRequest.callsArgWith(1, null, {statusCode: 200}, resultXml) + + it 'should not produce an error', (done) -> + @call (err, result) => + expect(err).to.not.be.instanceof Error + done() + + it 'should call apiRequest', (done) -> + @call (err, result) => + @apiRequest.callCount.should.equal 1 + done() + + it 'should call _parseAccountXml', (done) -> + @call (err, result) => + @RecurlyWrapper._parseAccountXml.callCount.should.equal 1 + done() + + it 'should add the account to the cumulative result', (done) -> + @call (err, result) => + expect(result.account).to.not.equal null + expect(result.account).to.not.equal undefined + expect(result.account).to.deep.equal { + account_code: 'abc' + } + done() + + describe 'when the account does not exist', -> + + beforeEach -> + @apiRequest.callsArgWith(1, new Error('not found'), {statusCode: 404}, '') + + it 'should not produce an error', (done) -> + @call (err, result) => + expect(err).to.not.be.instanceof Error + done() + + it 'should call apiRequest', (done) -> + @call (err, result) => + @apiRequest.callCount.should.equal 1 + done() + + it 'should not call _parseAccountXml', (done) -> + @call (err, result) => + @RecurlyWrapper._parseAccountXml.callCount.should.equal 0 + done() + + it 'should not add the account to result', (done) -> + @call (err, result) => + expect(result.account).to.equal undefined + done() + + # describe '_paypal.createAccount', -> + + # beforeEach -> + + + # describe '_paypal.createBillingInfo', -> + + # beforeEach -> + + + # describe '_paypal.setAddress', -> + + # beforeEach -> + + + # describe '_paypal.createSubscription', -> + + # beforeEach -> From 465d09dcfed10add64a7e14a8b3be9f5d675d055 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 28 Jun 2016 11:10:51 +0100 Subject: [PATCH 16/17] Test the createSubscription action (+4 squashed commits) Squashed commits: [fc9c8f9] Add tests for createBillingInfo [db9f90e] Test the createSubscription stage [c17151d] Check that userExists gets set [1367c96] Act on buddy-check feedback --- .../Subscription/RecurlyWrapper.coffee | 14 +- .../Subscription/RecurlyWrapperTests.coffee | 364 +++++++++++++++--- 2 files changed, 321 insertions(+), 57 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index 841ee0a77e..0d7aa99131 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -39,6 +39,7 @@ module.exports = RecurlyWrapper = if err logger.error {err, user_id: user._id, recurly_token_id}, "error parsing account" return next(err) + cache.userExists = true cache.account = account return next(null, cache) ) @@ -46,13 +47,13 @@ module.exports = RecurlyWrapper = user = cache.user recurly_token_id = cache.recurly_token_id subscriptionDetails = cache.subscriptionDetails + address = subscriptionDetails.address + if !address + return next(new Error('no address in subscriptionDetails at createAccount stage')) if cache.userExists logger.log {user_id: user._id, recurly_token_id}, "user already exists in recurly" return next(null, cache) logger.log {user_id: user._id, recurly_token_id}, "creating user in recurly" - address = subscriptionDetails.address - if !address - return next(new Error('no address in subscriptionDetails at createAccount stage')) requestBody = """ #{user._id} @@ -173,6 +174,9 @@ module.exports = RecurlyWrapper = _createPaypalSubscription: (user, subscriptionDetails, recurly_token_id, callback) -> logger.log {user_id: user._id, recurly_token_id}, "starting process of creating paypal subscription" + # We use `async.waterfall` to run each of these actions in sequence + # passing a `cache` object along the way. The cache is initialized + # with required data, and `async.apply` to pass the cache to the first function cache = {user, recurly_token_id, subscriptionDetails} Async.waterfall([ Async.apply(RecurlyWrapper._paypal.checkAccountExists, cache), @@ -471,9 +475,7 @@ module.exports = RecurlyWrapper = _parseBillingInfoXml: (xml, callback) -> RecurlyWrapper._parseXml xml, (error, data) -> return callback(error) if error? - if data? and data.account? - billingInfo = data.billing_info - else if data? and data.billing_info? + if data? and data.billing_info? billingInfo = data.billing_info else return callback "I don't understand the response from Recurly" diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index 4ab866b408..549f6e0250 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -656,11 +656,13 @@ describe "RecurlyWrapper", -> @createSubscription.callCount.should.equal 0 done() - describe '_paypal.checkAccountExists', -> + describe 'paypal actions', -> beforeEach -> @apiRequest = sinon.stub(@RecurlyWrapper, 'apiRequest') @_parseAccountXml = sinon.spy(@RecurlyWrapper, '_parseAccountXml') + @_parseBillingInfoXml = sinon.spy(@RecurlyWrapper, '_parseBillingInfoXml') + @_parseSubscriptionXml = sinon.spy(@RecurlyWrapper, '_parseSubscriptionXml') @cache = user: @user = {_id: 'some_id'} recurly_token_id: @recurly_token_id = "some_token" @@ -675,83 +677,343 @@ describe "RecurlyWrapper", -> country: "some_country" state: "some_state" zip: "some_zip" - @call = (callback) => - @RecurlyWrapper._paypal.checkAccountExists @cache, callback afterEach -> @apiRequest.restore() @_parseAccountXml.restore() + @_parseBillingInfoXml.restore() + @_parseSubscriptionXml.restore() - describe 'when the account exists', -> + describe '_paypal.checkAccountExists', -> beforeEach -> - resultXml = 'abc' - @apiRequest.callsArgWith(1, null, {statusCode: 200}, resultXml) + @call = (callback) => + @RecurlyWrapper._paypal.checkAccountExists @cache, callback - it 'should not produce an error', (done) -> - @call (err, result) => - expect(err).to.not.be.instanceof Error - done() + describe 'when the account exists', -> - it 'should call apiRequest', (done) -> - @call (err, result) => - @apiRequest.callCount.should.equal 1 - done() + beforeEach -> + resultXml = 'abc' + @apiRequest.callsArgWith(1, null, {statusCode: 200}, resultXml) - it 'should call _parseAccountXml', (done) -> - @call (err, result) => - @RecurlyWrapper._parseAccountXml.callCount.should.equal 1 - done() + it 'should not produce an error', (done) -> + @call (err, result) => + expect(err).to.not.be.instanceof Error + done() - it 'should add the account to the cumulative result', (done) -> - @call (err, result) => - expect(result.account).to.not.equal null - expect(result.account).to.not.equal undefined - expect(result.account).to.deep.equal { + it 'should call apiRequest', (done) -> + @call (err, result) => + @apiRequest.callCount.should.equal 1 + done() + + it 'should call _parseAccountXml', (done) -> + @call (err, result) => + @RecurlyWrapper._parseAccountXml.callCount.should.equal 1 + done() + + it 'should add the account to the cumulative result', (done) -> + @call (err, result) => + expect(result.account).to.not.equal null + expect(result.account).to.not.equal undefined + expect(result.account).to.deep.equal { + account_code: 'abc' + } + done() + + it 'should set userExists to true', (done) -> + @call (err, result) => + expect(result.userExists).to.equal true + done() + + describe 'when the account does not exist', -> + + beforeEach -> + @apiRequest.callsArgWith(1, new Error('not found'), {statusCode: 404}, '') + + it 'should not produce an error', (done) -> + @call (err, result) => + expect(err).to.not.be.instanceof Error + done() + + it 'should call apiRequest', (done) -> + @call (err, result) => + @apiRequest.callCount.should.equal 1 + @apiRequest.firstCall.args[0].method.should.equal 'GET' + done() + + it 'should not call _parseAccountXml', (done) -> + @call (err, result) => + @RecurlyWrapper._parseAccountXml.callCount.should.equal 0 + done() + + it 'should not add the account to result', (done) -> + @call (err, result) => + expect(result.account).to.equal undefined + done() + + it 'should set userExists to false', (done) -> + @call (err, result) => + expect(result.userExists).to.equal false + done() + + describe 'when apiRequest produces an error', -> + + beforeEach -> + @apiRequest.callsArgWith(1, new Error('woops'), {statusCode: 500}) + + it 'should produce an error', (done) -> + @call (err, result) => + expect(err).to.be.instanceof Error + done() + + describe '_paypal.createAccount', -> + + beforeEach -> + @call = (callback) => + @RecurlyWrapper._paypal.createAccount @cache, callback + + describe 'when address is missing from subscriptionDetails', -> + + beforeEach -> + @cache.subscriptionDetails.address = null + + it 'should produce an error', (done) -> + @call (err, result) => + expect(err).to.be.instanceof Error + done() + + describe 'when account already exists', -> + + beforeEach -> + @cache.userExists = true + @cache.account = account_code: 'abc' - } - done() - describe 'when the account does not exist', -> + it 'should not produce an error', (done) -> + @call (err, result) => + expect(err).to.not.be.instanceof Error + done() + + it 'should produce cache object', (done) -> + @call (err, result) => + expect(result).to.deep.equal @cache + expect(result.account).to.deep.equal { + account_code: 'abc' + } + done() + + it 'should not call apiRequest', (done) -> + @call (err, result) => + @apiRequest.callCount.should.equal 0 + done() + + it 'should not call _parseAccountXml', (done) -> + @call (err, result) => + @RecurlyWrapper._parseAccountXml.callCount.should.equal 0 + done() + + describe 'when account does not exist', -> + + beforeEach -> + @cache.userExists = false + resultXml = 'abc' + @apiRequest.callsArgWith(1, null, {statusCode: 200}, resultXml) + + it 'should not produce an error', (done) -> + @call (err, result) => + expect(err).to.not.be.instanceof Error + done() + + it 'should call apiRequest', (done) -> + @call (err, result) => + @apiRequest.callCount.should.equal 1 + @apiRequest.firstCall.args[0].method.should.equal 'POST' + done() + + it 'should call _parseAccountXml', (done) -> + @call (err, result) => + @RecurlyWrapper._parseAccountXml.callCount.should.equal 1 + done() + + describe 'when apiRequest produces an error', -> + + beforeEach -> + @apiRequest.callsArgWith(1, new Error('woops'), {statusCode: 500}) + + it 'should produce an error', (done) -> + @call (err, result) => + expect(err).to.be.instanceof Error + done() + + describe '_paypal.createBillingInfo', -> beforeEach -> - @apiRequest.callsArgWith(1, new Error('not found'), {statusCode: 404}, '') + @cache.account = + account_code: 'abc' + @call = (callback) => + @RecurlyWrapper._paypal.createBillingInfo @cache, callback - it 'should not produce an error', (done) -> - @call (err, result) => - expect(err).to.not.be.instanceof Error - done() + describe 'when account_code is missing from cache', -> - it 'should call apiRequest', (done) -> - @call (err, result) => - @apiRequest.callCount.should.equal 1 - done() + beforeEach -> + @cache.account.account_code = null - it 'should not call _parseAccountXml', (done) -> - @call (err, result) => - @RecurlyWrapper._parseAccountXml.callCount.should.equal 0 - done() + it 'should produce an error', (done) -> + @call (err, result) => + expect(err).to.be.instanceof Error + done() - it 'should not add the account to result', (done) -> - @call (err, result) => - expect(result.account).to.equal undefined - done() + describe 'when all goes well', -> - # describe '_paypal.createAccount', -> + beforeEach -> + resultXml = '1' + @apiRequest.callsArgWith(1, null, {statusCode: 200}, resultXml) - # beforeEach -> + it 'should not produce an error', (done) -> + @call (err, result) => + expect(err).to.not.be.instanceof Error + done() + it 'should call apiRequest', (done) -> + @call (err, result) => + @apiRequest.callCount.should.equal 1 + @apiRequest.firstCall.args[0].method.should.equal 'POST' + done() - # describe '_paypal.createBillingInfo', -> + it 'should call _parseBillingInfoXml', (done) -> + @call (err, result) => + @RecurlyWrapper._parseBillingInfoXml.callCount.should.equal 1 + done() - # beforeEach -> + it 'should set billingInfo on cache', (done) -> + @call (err, result) => + expect(result.billingInfo).to.deep.equal { + a: "1" + } + done() + describe 'when apiRequest produces an error', -> - # describe '_paypal.setAddress', -> + beforeEach -> + @apiRequest.callsArgWith(1, new Error('woops'), {statusCode: 500}) - # beforeEach -> + it 'should produce an error', (done) -> + @call (err, result) => + expect(err).to.be.instanceof Error + done() + describe '_paypal.setAddress', -> - # describe '_paypal.createSubscription', -> + beforeEach -> + @cache.account = + account_code: 'abc' + @cache.billingInfo = {} + @call = (callback) => + @RecurlyWrapper._paypal.setAddress @cache, callback - # beforeEach -> + describe 'when account_code is missing from cache', -> + + beforeEach -> + @cache.account.account_code = null + + it 'should produce an error', (done) -> + @call (err, result) => + expect(err).to.be.instanceof Error + done() + + describe 'when address is missing from subscriptionDetails', -> + + beforeEach -> + @cache.subscriptionDetails.address = null + + it 'should produce an error', (done) -> + @call (err, result) => + expect(err).to.be.instanceof Error + done() + + describe 'when all goes well', -> + + beforeEach -> + resultXml = 'London' + @apiRequest.callsArgWith(1, null, {statusCode: 200}, resultXml) + + it 'should not produce an error', (done) -> + @call (err, result) => + expect(err).to.not.be.instanceof Error + done() + + it 'should call apiRequest', (done) -> + @call (err, result) => + @apiRequest.callCount.should.equal 1 + @apiRequest.firstCall.args[0].method.should.equal 'PUT' + done() + + it 'should call _parseBillingInfoXml', (done) -> + @call (err, result) => + @RecurlyWrapper._parseBillingInfoXml.callCount.should.equal 1 + done() + + it 'should set billingInfo on cache', (done) -> + @call (err, result) => + expect(result.billingInfo).to.deep.equal { + city: 'London' + } + done() + + describe 'when apiRequest produces an error', -> + + beforeEach -> + @apiRequest.callsArgWith(1, new Error('woops'), {statusCode: 500}) + + it 'should produce an error', (done) -> + @call (err, result) => + expect(err).to.be.instanceof Error + done() + + describe '_paypal.createSubscription', -> + + beforeEach -> + @cache.account = + account_code: 'abc' + @cache.billingInfo = {} + @call = (callback) => + @RecurlyWrapper._paypal.createSubscription @cache, callback + + describe 'when all goes well', -> + + beforeEach -> + resultXml = '1' + @apiRequest.callsArgWith(1, null, {statusCode: 200}, resultXml) + + it 'should not produce an error', (done) -> + @call (err, result) => + expect(err).to.not.be.instanceof Error + done() + + it 'should call apiRequest', (done) -> + @call (err, result) => + @apiRequest.callCount.should.equal 1 + @apiRequest.firstCall.args[0].method.should.equal 'POST' + done() + + it 'should call _parseSubscriptionXml', (done) -> + @call (err, result) => + @RecurlyWrapper._parseSubscriptionXml.callCount.should.equal 1 + done() + + it 'should set subscription on cache', (done) -> + @call (err, result) => + expect(result.subscription).to.deep.equal { + a: "1" + } + done() + + describe 'when apiRequest produces an error', -> + + beforeEach -> + @apiRequest.callsArgWith(1, new Error('woops'), {statusCode: 500}) + + it 'should produce an error', (done) -> + @call (err, result) => + expect(err).to.be.instanceof Error + done() From 6581bc4ecfc85437333ad5bdf8537d17b4c21a7d Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 28 Jun 2016 14:15:47 +0100 Subject: [PATCH 17/17] set postal_code as zip --- .../coffee/Features/Subscription/RecurlyWrapper.coffee | 4 +++- .../web/public/coffee/main/new-subscription.coffee | 1 - .../coffee/Subscription/RecurlyWrapperTests.coffee | 10 +++++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index 0d7aa99131..059c5fb02c 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -10,9 +10,11 @@ module.exports = RecurlyWrapper = apiUrl : "https://api.recurly.com/v2" _addressToXml: (address) -> - allowedKeys = ['address1', 'address2', 'city', 'country', 'state', 'zip'] + allowedKeys = ['address1', 'address2', 'city', 'country', 'state', 'zip', 'postal_code'] resultString = "\n" for k, v of address + if k == 'postal_code' + k = 'zip' if v and (k in allowedKeys) resultString += "<#{k}#{if k == 'address2' then ' nil="nil"' else ''}>#{v || ''}\n" resultString += "\n" diff --git a/services/web/public/coffee/main/new-subscription.coffee b/services/web/public/coffee/main/new-subscription.coffee index b507690f4e..93c2c0a7cc 100644 --- a/services/web/public/coffee/main/new-subscription.coffee +++ b/services/web/public/coffee/main/new-subscription.coffee @@ -122,7 +122,6 @@ define [ country: $scope.data.country state: $scope.data.state postal_code: $scope.data.postal_code - zip: $scope.data.zip $http.post("/user/subscription/create", postData) .success (data, status, headers)-> sixpack.convert "in-editor-free-trial-plan", pricing.items.plan.code, (err)-> diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index 549f6e0250..5a5e9fc40c 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee @@ -354,11 +354,11 @@ describe "RecurlyWrapper", -> beforeEach -> @address = - address1: "addr_one" - address2: "addr_two" - country: "some_country" - state: "some_state" - zip: "some_zip" + address1: "addr_one" + address2: "addr_two" + country: "some_country" + state: "some_state" + postal_code: "some_zip" nonsenseKey: "rubbish" it 'should generate the correct xml', () ->