diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index 60387c0dc0..ce40847a27 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -469,33 +469,35 @@ module.exports = RecurlyWrapper = logger.err err:error, subscriptionId:subscriptionId, daysUntilExpire:daysUntilExpire, "error exending trial" callback(error) ) + + listAccountActiveSubscriptions: (account_id, callback = (error, subscriptions) ->) -> + RecurlyWrapper.apiRequest { + url: "accounts/#{account_id}/subscriptions" + qs: + state: "active" + }, (error, response, body) -> + return callback(error) if error? + RecurlyWrapper._parseSubscriptionsXml body, callback + + _parseSubscriptionsXml: (xml, callback) -> + RecurlyWrapper._parseXmlAndGetAttribute xml, "subscriptions", callback _parseSubscriptionXml: (xml, callback) -> - RecurlyWrapper._parseXml xml, (error, data) -> - return callback(error) if error? - if data? and data.subscription? - recurlySubscription = data.subscription - else - return callback "I don't understand the response from Recurly" - callback null, recurlySubscription + RecurlyWrapper._parseXmlAndGetAttribute xml, "subscription", callback _parseAccountXml: (xml, callback) -> - RecurlyWrapper._parseXml xml, (error, data) -> - return callback(error) if error? - if data? and data.account? - account = data.account - else - return callback "I don't understand the response from Recurly" - callback null, account + RecurlyWrapper._parseXmlAndGetAttribute xml, "account", callback _parseBillingInfoXml: (xml, callback) -> + RecurlyWrapper._parseXmlAndGetAttribute xml, "billing_info", callback + + _parseXmlAndGetAttribute: (xml, attribute, callback) -> RecurlyWrapper._parseXml xml, (error, data) -> return callback(error) if error? - if data? and data.billing_info? - billingInfo = data.billing_info + if data? and data[attribute]? + return callback null, data[attribute] else - return callback "I don't understand the response from Recurly" - callback null, billingInfo + return callback(new Error("I don't understand the response from Recurly")) _parseXml: (xml, callback) -> convertDataTypes = (data) -> diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee index 84de417bb6..cc3013a42d 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee @@ -46,31 +46,39 @@ module.exports = SubscriptionController = if hasSubscription or !plan? res.redirect "/user/subscription" else - currency = req.query.currency?.toUpperCase() - GeoIpLookup.getCurrencyCode req.query?.ip || req.ip, (err, recomendedCurrency, countryCode)-> - return next(err) if err? - if recomendedCurrency? and !currency? - currency = recomendedCurrency - RecurlyWrapper.sign { - subscription: - plan_code : req.query.planCode - currency: currency - account_code: user._id - }, (error, signature) -> - return next(error) if error? - res.render "subscriptions/new", - title : "subscribe" - plan_code: req.query.planCode - currency: currency - countryCode:countryCode - plan:plan - showStudentPlan: req.query.ssp - recurlyConfig: JSON.stringify - currency: currency - subdomain: Settings.apis.recurly.subdomain - showCouponField: req.query.scf - showVatField: req.query.svf - couponCode: req.query.cc or "" + # LimitationsManager.userHasSubscription only checks Mongo. Double check with + # Recurly as well at this point (we don't do this most places for speed). + SubscriptionHandler.validateNoSubscriptionInRecurly user._id, (error, valid) -> + return next(error) if error? + if !valid + res.redirect "/user/subscription" + return + else + currency = req.query.currency?.toUpperCase() + GeoIpLookup.getCurrencyCode req.query?.ip || req.ip, (err, recomendedCurrency, countryCode)-> + return next(err) if err? + if recomendedCurrency? and !currency? + currency = recomendedCurrency + RecurlyWrapper.sign { + subscription: + plan_code : req.query.planCode + currency: currency + account_code: user._id + }, (error, signature) -> + return next(error) if error? + res.render "subscriptions/new", + title : "subscribe" + plan_code: req.query.planCode + currency: currency + countryCode:countryCode + plan:plan + showStudentPlan: req.query.ssp + recurlyConfig: JSON.stringify + currency: currency + subdomain: Settings.apis.recurly.subdomain + showCouponField: req.query.scf + showVatField: req.query.svf + couponCode: req.query.cc or "" diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionHandler.coffee index ba95895dcd..99da3270a8 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionHandler.coffee @@ -11,15 +11,28 @@ Analytics = require("../Analytics/AnalyticsManager") module.exports = + validateNoSubscriptionInRecurly: (user_id, callback = (error, valid) ->) -> + RecurlyWrapper.listAccountActiveSubscriptions user_id, (error, subscriptions = []) -> + return callback(error) if error? + if subscriptions.length > 0 + SubscriptionUpdater.syncSubscription subscriptions[0], user_id, (error) -> + return callback(error) if error? + return callback(null, false) + else + return callback(null, true) createSubscription: (user, subscriptionDetails, recurly_token_id, callback)-> self = @ clientTokenId = "" - RecurlyWrapper.createSubscription user, subscriptionDetails, recurly_token_id, (error, recurlySubscription)-> + @validateNoSubscriptionInRecurly user._id, (error, valid) -> return callback(error) if error? - SubscriptionUpdater.syncSubscription recurlySubscription, user._id, (error) -> + if !valid + return callback(new Error("user already has subscription in recurly")) + RecurlyWrapper.createSubscription user, subscriptionDetails, recurly_token_id, (error, recurlySubscription)-> return callback(error) if error? - callback() + SubscriptionUpdater.syncSubscription recurlySubscription, user._id, (error) -> + return callback(error) if error? + callback() updateSubscription: (user, plan_code, coupon_code, callback)-> logger.log user:user, plan_code:plan_code, coupon_code:coupon_code, "updating subscription" diff --git a/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee index 3a43c8988e..b95fba1cdd 100644 --- a/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee @@ -17,8 +17,7 @@ mockSubscriptions = account: account_code: "user-123" -describe "SubscriptionController sanboxed", -> - +describe "SubscriptionController", -> beforeEach -> @user = {email:"tom@yahoo.com", _id: 'one', signUpDate: new Date('2000-10-01')} @activeRecurlySubscription = mockSubscriptions["subscription-123-active"] @@ -150,6 +149,7 @@ describe "SubscriptionController sanboxed", -> describe "paymentPage", -> beforeEach -> @req.headers = {} + @SubscriptionHandler.validateNoSubscriptionInRecurly = sinon.stub().yields(null, true) @GeoIpLookup.getCurrencyCode.callsArgWith(1, null, @stubbedCurrencyCode) describe "with a user without a subscription", -> @@ -209,6 +209,16 @@ describe "SubscriptionController sanboxed", -> opts.currency.should.equal @stubbedCurrencyCode done() @SubscriptionController.paymentPage @req, @res + + describe "with a recurly subscription already", -> + it "should redirect to the subscription dashboard", (done)-> + @LimitationsManager.userHasSubscription.callsArgWith(1, null, false) + @SubscriptionHandler.validateNoSubscriptionInRecurly = sinon.stub().yields(null, false) + @res.redirect = (url)=> + url.should.equal "/user/subscription" + done() + @SubscriptionController.paymentPage(@req, @res) + describe "successful_subscription", -> beforeEach (done) -> diff --git a/services/web/test/UnitTests/coffee/Subscription/SubscriptionHandlerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/SubscriptionHandlerTests.coffee index 935ed6d025..1e2cec4bfa 100644 --- a/services/web/test/UnitTests/coffee/Subscription/SubscriptionHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/SubscriptionHandlerTests.coffee @@ -16,7 +16,7 @@ mockRecurlySubscriptions = account: account_code: "user-123" -describe "Subscription Handler sanboxed", -> +describe "SubscriptionHandler", -> beforeEach -> @Settings = @@ -33,7 +33,7 @@ describe "Subscription Handler sanboxed", -> @activeRecurlySubscription = mockRecurlySubscriptions["subscription-123-active"] @User = {} @user = - _id: "user_id_here_" + _id: @user_id = "user_id_here_" @subscription = recurlySubscription_id: @activeRecurlySubscription.uuid @RecurlyWrapper = @@ -77,23 +77,33 @@ describe "Subscription Handler sanboxed", -> describe "createSubscription", -> - beforeEach (done) -> + beforeEach -> + @callback = sinon.stub() @subscriptionDetails = cvv:"123" number:"12345" @recurly_token_id = "45555666" - @SubscriptionHandler.createSubscription(@user, @subscriptionDetails, @recurly_token_id, done) + @SubscriptionHandler.validateNoSubscriptionInRecurly = sinon.stub().yields(null, true) - it "should create the subscription with the wrapper", (done)-> - @RecurlyWrapper.createSubscription.calledWith(@user, @subscriptionDetails, @recurly_token_id).should.equal true - done() + describe "successfully", -> + beforeEach -> + @SubscriptionHandler.createSubscription(@user, @subscriptionDetails, @recurly_token_id, @callback) - it "should sync the subscription to the user", (done)-> - @SubscriptionUpdater.syncSubscription.calledOnce.should.equal true - @SubscriptionUpdater.syncSubscription.args[0][0].should.deep.equal @activeRecurlySubscription - @SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal @user._id - done() + it "should create the subscription with the wrapper", -> + @RecurlyWrapper.createSubscription.calledWith(@user, @subscriptionDetails, @recurly_token_id).should.equal true + it "should sync the subscription to the user", -> + @SubscriptionUpdater.syncSubscription.calledOnce.should.equal true + @SubscriptionUpdater.syncSubscription.args[0][0].should.deep.equal @activeRecurlySubscription + @SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal @user._id + + describe "when there is already a subscription in Recurly", -> + beforeEach -> + @SubscriptionHandler.validateNoSubscriptionInRecurly = sinon.stub().yields(null, false) + @SubscriptionHandler.createSubscription(@user, @subscriptionDetails, @recurly_token_id, @callback) + + it "should return an error", -> + @callback.calledWith(new Error("user already has subscription in recurly")) describe "updateSubscription", -> describe "with a user with a subscription", -> @@ -145,8 +155,6 @@ describe "Subscription Handler sanboxed", -> updateOptions = @RecurlyWrapper.updateSubscription.args[0][1] updateOptions.plan_code.should.equal @plan_code - - describe "cancelSubscription", -> describe "with a user without a subscription", -> beforeEach (done) -> @@ -210,5 +218,39 @@ describe "Subscription Handler sanboxed", -> @SubscriptionUpdater.syncSubscription.args[0][0].should.deep.equal @activeRecurlySubscription @SubscriptionUpdater.syncSubscription.args[0][1].should.deep.equal @user._id + describe "validateNoSubscriptionInRecurly", -> + beforeEach -> + @subscriptions = [] + @RecurlyWrapper.listAccountActiveSubscriptions = sinon.stub().yields(null, @subscriptions) + @SubscriptionUpdater.syncSubscription = sinon.stub().yields() + @callback = sinon.stub() + describe "with no subscription in recurly", -> + beforeEach -> + @subscriptions.push @subscription = { "mock": "subscription" } + @SubscriptionHandler.validateNoSubscriptionInRecurly @user_id, @callback + it "should call RecurlyWrapper.listAccountActiveSubscriptions with the user id", -> + @RecurlyWrapper.listAccountActiveSubscriptions + .calledWith(@user_id) + .should.equal true + + it "should sync the subscription", -> + @SubscriptionUpdater.syncSubscription + .calledWith(@subscription, @user_id) + .should.equal true + + it "should call the callback with valid == false", -> + @callback.calledWith(null, false).should.equal true + + describe "with a subscription in recurly", -> + beforeEach -> + @SubscriptionHandler.validateNoSubscriptionInRecurly @user_id, @callback + + it "should not sync the subscription", -> + @SubscriptionUpdater.syncSubscription + .called + .should.equal false + + it "should call the callback with valid == true", -> + @callback.calledWith(null, true).should.equal true