Merge pull request #251 from sharelatex/fix-group-member-subscription-page

Fix subscription page for members of a group

services/web/app/coffee/Features/Subscription/SubscriptionController.coffee

@@ -97,39 +97,21 @@ module.exports = SubscriptionController =
 				logger.log user: user, "redirecting to plans"
 				res.redirect "/user/subscription/plans"
 			else
-				RecurlyWrapper.getSubscription subscription.recurlySubscription_id,
-					includeAccount: true,
-					(err, usersSubscription)->
-						# always render the page, but skip the recurly link if
-						# we can't get it for some reason
-						if err?
-							logger.err {err, userId: user._id}, "error getting billing details link from recurly, proceeding"
-						hostedLoginToken = usersSubscription?.account?.hosted_login_token
-						recurlySubdomain = Settings?.apis?.recurly?.subdomain
-						if err? || !hostedLoginToken || !recurlySubdomain
-							billingDetailsLink = null
-						else
-							billingDetailsLink = [
-								"https://",
-								recurlySubdomain,
-								".recurly.com/account/billing_info/edit?ht=",
-								hostedLoginToken
-							].join("")
-						SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel user, (error, subscription, groupSubscriptions) ->
-							return next(error) if error?
-							logger.log {user, subscription, hasSubOrIsGroupMember, groupSubscriptions, billingDetailsLink}, "showing subscription dashboard"
-							plans = SubscriptionViewModelBuilder.buildViewModel()
-							res.render "subscriptions/dashboard",
-								title: "your_subscription"
-								recomendedCurrency: subscription?.currency
-								taxRate:subscription?.taxRate
-								plans: plans
-								subscription: subscription || {}
-								groupSubscriptions: groupSubscriptions
-								subscriptionTabActive: true
-								user:user
-								saved_billing_details: req.query.saved_billing_details?
-								billingDetailsLink: billingDetailsLink
+				SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel user, (error, subscription, groupSubscriptions, billingDetailsLink) ->
+					return next(error) if error?
+					logger.log {user, subscription, hasSubOrIsGroupMember, groupSubscriptions, billingDetailsLink}, "showing subscription dashboard"
+					plans = SubscriptionViewModelBuilder.buildViewModel()
+					res.render "subscriptions/dashboard",
+						title: "your_subscription"
+						recomendedCurrency: subscription?.currency
+						taxRate:subscription?.taxRate
+						plans: plans
+						subscription: subscription || {}
+						groupSubscriptions: groupSubscriptions
+						subscriptionTabActive: true
+						user:user
+						saved_billing_details: req.query.saved_billing_details?
+						billingDetailsLink: billingDetailsLink
 
 	userCustomSubscriptionPage: (req, res, next)->
 		user = AuthenticationController.getSessionUser(req)

services/web/app/coffee/Features/Subscription/SubscriptionViewModelBuilder.coffee

@@ -7,22 +7,39 @@ SubscriptionLocator = require("./SubscriptionLocator")
 logger = require('logger-sharelatex')
 _ = require("underscore")
 
-module.exports =
 
-	buildUsersSubscriptionViewModel: (user, callback = (error, subscription, memberSubscriptions) ->) ->
+buildBillingDetails = (recurlySubscription) ->
+	hostedLoginToken = recurlySubscription?.account?.hosted_login_token
+	recurlySubdomain = Settings?.apis?.recurly?.subdomain
+	if hostedLoginToken? && recurlySubdomain?
+		return [
+			"https://",
+			recurlySubdomain,
+			".recurly.com/account/billing_info/edit?ht=",
+			hostedLoginToken
+		].join("")
+
+module.exports =
+	buildUsersSubscriptionViewModel: (user, callback = (error, subscription, memberSubscriptions, billingDetailsLink) ->) ->
 		SubscriptionLocator.getUsersSubscription user, (err, subscription) ->
 			return callback(err) if err?
+
 			SubscriptionLocator.getMemberSubscriptions user, (err, memberSubscriptions = []) ->
 				return callback(err) if err?
+
 				if subscription?
 					return callback(error) if error?
+
 					plan = PlansLocator.findLocalPlanInSettings(subscription.planCode)
+
 					if !plan?
 						err = new Error("No plan found for planCode '#{subscription.planCode}'")
 						logger.error {user_id: user._id, err}, "error getting subscription plan for user"
 						return callback(err)
-					RecurlyWrapper.getSubscription subscription.recurlySubscription_id, (err, recurlySubscription)->
+
+					RecurlyWrapper.getSubscription subscription.recurlySubscription_id, includeAccount: true, (err, recurlySubscription)->
 						tax = recurlySubscription?.tax_in_cents || 0
+
 						callback null, {
 							admin_id:subscription.admin_id
 							name: plan.name
@@ -34,9 +51,10 @@ module.exports =
 							taxRate:parseFloat(recurlySubscription?.tax_rate?._)
 							groupPlan: subscription.groupPlan
 							trial_ends_at:recurlySubscription?.trial_ends_at
-						}, memberSubscriptions
+						}, memberSubscriptions, buildBillingDetails(recurlySubscription)
+
 				else
-					callback null, null, memberSubscriptions
+					callback null, null, memberSubscriptions, null
 
 	buildViewModel : ->
 		plans = Settings.plans

services/web/test/unit/coffee/Subscription/SubscriptionControllerTests.coffee

@@ -42,10 +42,6 @@ describe "SubscriptionController", ->
 			userHasSubscriptionOrIsGroupMember: sinon.stub()
 			userHasSubscription : sinon.stub()
 
-		@RecurlyWrapper =
-			sign: sinon.stub().callsArgWith(1, null, "somthing")
-			getSubscription: sinon.stub().callsArgWith(2, null, {})
-
 		@SubscriptionViewModelBuilder =
 			buildUsersSubscriptionViewModel:sinon.stub().callsArgWith(1, null, @activeRecurlySubscription)
 			buildViewModel: sinon.stub()
@@ -72,7 +68,6 @@ describe "SubscriptionController", ->
 			'./SubscriptionViewModelBuilder': @SubscriptionViewModelBuilder
 			"./LimitationsManager": @LimitationsManager
 			"../../infrastructure/GeoIpLookup":@GeoIpLookup
-			'./RecurlyWrapper': @RecurlyWrapper
 			"logger-sharelatex":
 				log:->
 				warn:->
@@ -182,7 +177,7 @@ describe "SubscriptionController", ->
 					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)
@@ -191,7 +186,7 @@ describe "SubscriptionController", ->
 					url.should.equal "/user/subscription"
 					done()
 				@SubscriptionController.paymentPage(@req, @res)
-			
+
 
 	describe "successful_subscription", ->
 		beforeEach (done) ->
@@ -230,8 +225,6 @@ describe "SubscriptionController", ->
 					@res.callback = done
 					@settings.apis.recurly.subdomain = 'test'
 					@userSub = {account: {hosted_login_token: 'abcd'}}
-					@RecurlyWrapper.getSubscription = sinon.stub()
-						.callsArgWith(2, null, @userSub)
 					@LimitationsManager.userHasSubscriptionOrIsGroupMember
 						.callsArgWith(1, null, true, {})
 					@SubscriptionController.userSubscriptionPage @req, @res

services/web/test/unit/coffee/Subscription/SubscriptionViewModelBuilderTests.coffee

@@ -0,0 +1,66 @@
+SandboxedModule = require('sandboxed-module')
+sinon = require 'sinon'
+should = require("chai").should()
+modulePath = '../../../../app/js/Features/Subscription/SubscriptionViewModelBuilder'
+
+describe 'SubscriptionViewModelBuilder', ->
+	mockSubscription =
+		uuid: "subscription-123-active"
+		plan:
+			name: "Gold"
+			plan_code: "gold"
+		current_period_ends_at: new Date()
+		state: "active"
+		unit_amount_in_cents: 999
+		account:
+			account_code: "user-123"
+
+
+	beforeEach ->
+		@user =
+			email:"tom@yahoo.com",
+			_id: 'one',
+			signUpDate: new Date('2000-10-01')
+
+		@plan =
+			name: "test plan"
+
+		@SubscriptionFormatters =
+			formatDate: sinon.stub().returns("Formatted date")
+			formatPrice: sinon.stub().returns("Formatted price")
+
+		@RecurlyWrapper =
+			sign: sinon.stub().callsArgWith(1, null, "something")
+			getSubscription: sinon.stub().callsArgWith	2, null,
+				account:
+					hosted_login_token: "hosted_login_token"
+
+		@builder = SandboxedModule.require modulePath, requires:
+			"settings-sharelatex": { apis: { recurly: { subdomain: "example.com" }}}
+			"./RecurlyWrapper": @RecurlyWrapper
+			"./PlansLocator": @PlansLocator
+			"./SubscriptionLocator": @SubscriptionLocator
+			"./SubscriptionFormatters": @SubscriptionFormatters
+			"./LimitationsManager": {}
+			"logger-sharelatex":
+				log:->
+				warn:->
+			"underscore": {}
+
+		@PlansLocator.findLocalPlanInSettings = sinon.stub().returns(@plan)
+		@SubscriptionLocator.getUsersSubscription =  sinon.stub().callsArgWith(1, null, mockSubscription)
+		@SubscriptionLocator.getMemberSubscriptions = sinon.stub().callsArgWith(1, null, null)
+
+	it 'builds the user view model', ->
+		callback = (error, subscription, memberSubscriptions, billingDetailsLink) =>
+			@error = error
+			@subscription = subscription
+			@memberSubscriptions = memberSubscriptions
+			@billingDetailsLink = billingDetailsLink
+
+		@builder.buildUsersSubscriptionViewModel(@user, callback)
+
+		@subscription.name.should.eq 'test plan'
+		@subscription.nextPaymentDueAt.should.eq 'Formatted date'
+		@subscription.price.should.eq 'Formatted price'
+		@billingDetailsLink.should.eq "https://example.com.recurly.com/account/billing_info/edit?ht=hosted_login_token"