Merge pull request #300 from sharelatex/pr-subscription-improvements

Pr subscription improvements

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

@@ -29,13 +29,15 @@ module.exports = RecurlyWrapper =
 			RecurlyWrapper.apiRequest({
 				url:    "accounts/#{user._id}"
 				method: "GET"
+				expect404: true
 			}, (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)
+				if response.statusCode == 404  # actually not an error in this case, just no existing account
+					logger.log {user_id: user._id, recurly_token_id}, "user does not currently exist in recurly, proceed"
+					cache.userExists = false
+					return next(null, cache)
 				logger.log {user_id: user._id, recurly_token_id}, "user appears to exist in recurly"
 				RecurlyWrapper._parseAccountXml responseBody, (err, account) ->
 					if err
@@ -236,10 +238,14 @@ module.exports = RecurlyWrapper =
 			"Authorization" : "Basic " + new Buffer(Settings.apis.recurly.apiKey).toString("base64")
 			"Accept"        : "application/xml"
 			"Content-Type"  : "application/xml; charset=utf-8"
+		expect404 = options.expect404
+		delete options.expect404
 		request options, (error, response, body) ->
-			unless error? or response.statusCode == 200 or response.statusCode == 201 or response.statusCode == 204
+			unless error? or response.statusCode == 200 or response.statusCode == 201 or response.statusCode == 204 or (response.statusCode == 404 and expect404)
 				logger.err err:error, body:body, options:options, statusCode:response?.statusCode, "error returned from recurly"
 				error = "Recurly API returned with status code: #{response.statusCode}"
+			if response.statusCode == 404 and expect404
+				logger.log {url: options.url, method: options.method}, "got 404 response from recurly, expected as valid response"
 			callback(error, response, body)
 
 	sign : (parameters, callback) ->

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

@@ -22,6 +22,7 @@ module.exports = SubscriptionController =
 			viewName = "#{viewName}_#{req.query.v}"
 		logger.log viewName:viewName, "showing plans page"
 		GeoIpLookup.getCurrencyCode req.query?.ip || req.ip, (err, recomendedCurrency)->
+			return next(err) if err?
 			res.render viewName,
 				title: "plans_and_pricing"
 				plans: plans
@@ -71,12 +72,13 @@ module.exports = SubscriptionController =
 		AuthenticationController.getLoggedInUser req, (error, user) =>
 			return next(error) if error?
 			LimitationsManager.userHasSubscriptionOrIsGroupMember user, (err, hasSubOrIsGroupMember, subscription)->
+				return next(err) if err?
 				groupLicenceInviteUrl = SubscriptionDomainHandler.getDomainLicencePage(user)
 				if subscription?.customAccount
 					logger.log user: user, "redirecting to custom account page"
 					res.redirect "/user/subscription/custom_account"
 				else if groupLicenceInviteUrl? and !hasSubOrIsGroupMember
-					logger.log user:user, "redirecting to group subscription invite page"		
+					logger.log user:user, "redirecting to group subscription invite page"
 					res.redirect groupLicenceInviteUrl
 				else if !hasSubOrIsGroupMember
 					logger.log user: user, "redirecting to plans"
@@ -99,7 +101,9 @@ module.exports = SubscriptionController =
 
 	userCustomSubscriptionPage: (req, res, next)->
 		AuthenticationController.getLoggedInUser req, (error, user) ->
+			return next(error) if error?
 			LimitationsManager.userHasSubscriptionOrIsGroupMember user, (err, hasSubOrIsGroupMember, subscription)->
+				return next(err) if err?
 				if !subscription?
 					err = new Error("subscription null for custom account, user:#{user?._id}")
 					logger.warn err:err, "subscription is null for custom accounts page"
@@ -113,6 +117,7 @@ module.exports = SubscriptionController =
 		AuthenticationController.getLoggedInUser req, (error, user) ->
 			return next(error) if error?
 			LimitationsManager.userHasSubscription user, (err, hasSubscription)->
+				return next(err) if err?
 				if !hasSubscription
 					res.redirect "/user/subscription"
 				else
@@ -142,54 +147,67 @@ module.exports = SubscriptionController =
 					return res.sendStatus 500
 				res.sendStatus 201
 
-	successful_subscription: (req, res)->
+	successful_subscription: (req, res, next)->
 		AuthenticationController.getLoggedInUser req, (error, user) =>
+			return next(error) if error?
 			SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel user, (error, subscription) ->
+				return next(error) if error?
 				res.render "subscriptions/successful_subscription",
 					title: "thank_you"
 					subscription:subscription
 
 	cancelSubscription: (req, res, next) ->
 		AuthenticationController.getLoggedInUser req, (error, user) ->
-			logger.log user_id:user._id, "canceling subscription"
 			return next(error) if error?
+			logger.log user_id:user._id, "canceling subscription"
 			SubscriptionHandler.cancelSubscription user, (err)->
 				if err?
 					logger.err err:err, user_id:user._id, "something went wrong canceling subscription"
+					return next(err)
 				res.redirect "/user/subscription"
- 
-	updateSubscription: (req, res)->
+
+	updateSubscription: (req, res, next)->
+		_origin = req?.query?.origin || null
 		AuthenticationController.getLoggedInUser req, (error, user) ->
 			return next(error) if error?
 			planCode = req.body.plan_code
+			if !planCode?
+				err = new Error('plan_code is not defined')
+				logger.err {user_id: user._id, err, planCode, origin: _origin, body: req.body}, "[Subscription] error in updateSubscription form"
+				return next(err)
 			logger.log planCode: planCode, user_id:user._id, "updating subscription"
 			SubscriptionHandler.updateSubscription user, planCode, null, (err)->
 				if err?
 					logger.err err:err, user_id:user._id, "something went wrong updating subscription"
+					return next(err)
 				res.redirect "/user/subscription"
 
-	reactivateSubscription: (req, res)->
+	reactivateSubscription: (req, res, next)->
 		AuthenticationController.getLoggedInUser req, (error, user) ->
-			logger.log user_id:user._id, "reactivating subscription"
 			return next(error) if error?
+			logger.log user_id:user._id, "reactivating subscription"
 			SubscriptionHandler.reactivateSubscription user, (err)->
 				if err?
 					logger.err err:err, user_id:user._id, "something went wrong reactivating subscription"
+					return next(err)
 				res.redirect "/user/subscription"
 
-	recurlyCallback: (req, res)->
+	recurlyCallback: (req, res, next)->
 		logger.log data: req.body, "received recurly callback"
 		# we only care if a subscription has exipired
 		if req.body? and req.body["expired_subscription_notification"]?
 			recurlySubscription = req.body["expired_subscription_notification"].subscription
-			SubscriptionHandler.recurlyCallback recurlySubscription, ->
+			SubscriptionHandler.recurlyCallback recurlySubscription, (err)->
+				return next(err) if err?
 				res.sendStatus 200
 		else
 			res.sendStatus 200
 
-	renderUpgradeToAnnualPlanPage: (req, res)->
+	renderUpgradeToAnnualPlanPage: (req, res, next)->
 		AuthenticationController.getLoggedInUser req, (error, user) ->
+			return next(error) if error?
 			LimitationsManager.userHasSubscription user, (err, hasSubscription, subscription)->
+				return next(err) if err?
 				planCode = subscription?.planCode.toLowerCase()
 				if planCode?.indexOf("annual") != -1
 					planName = "annual"
@@ -204,8 +222,9 @@ module.exports = SubscriptionController =
 					title: "Upgrade to annual"
 					planName: planName
 
-	processUpgradeToAnnualPlan: (req, res)->
+	processUpgradeToAnnualPlan: (req, res, next)->
 		AuthenticationController.getLoggedInUser req, (error, user) ->
+			return next(error) if error?
 			{planName} = req.body
 			coupon_code = Settings.coupon_codes.upgradeToAnnualPromo[planName]
 			annualPlanName = "#{planName}-annual"
@@ -213,13 +232,14 @@ module.exports = SubscriptionController =
 			SubscriptionHandler.updateSubscription user, annualPlanName, coupon_code, (err)->
 				if err?
 					logger.err err:err, user_id:user._id, "error updating subscription"
-					res.sendStatus 500
-				else
-					res.sendStatus 200
+					return next(err)
+				res.sendStatus 200
 
-	extendTrial: (req, res)->
+	extendTrial: (req, res, next)->
 		AuthenticationController.getLoggedInUser req, (error, user) ->
+			return next(error) if error?
 			LimitationsManager.userHasSubscription user, (err, hasSubscription, subscription)->
+				return next(err) if err?
 				SubscriptionHandler.extendTrial subscription, 14, (err)->
 					if err?
 						res.send 500

services/web/app/views/subscriptions/edit-billing-details.jade

@@ -19,7 +19,7 @@ block content
 		Recurly.config(!{recurlyConfig})
 		Recurly.buildBillingInfoUpdateForm({
 			target      : "#billingDetailsForm",
-			successURL  : "#{successURL}?_csrf=#{csrfToken}",
+			successURL  : "#{successURL}?_csrf=#{csrfToken}&origin=editBillingDetails",
 			signature   : "!{signature}",
 			accountCode : "#{user.id}"
 		});

services/web/public/coffee/main/subscription-dashboard.coffee

@@ -62,8 +62,7 @@ define [
 
 			$scope.inflight = true
 
-
-			$http.post(SUBSCRIPTION_URL, body)
+			$http.post("#{SUBSCRIPTION_URL}?origin=confirmChangePlan", body)
 				.success ->
 					location.reload()
 				.error ->
@@ -124,7 +123,7 @@ define [
 				plan_code: 'student'
 				_csrf : window.csrfToken
 			$scope.inflight = true
-			$http.post(SUBSCRIPTION_URL, body)
+			$http.post("#{SUBSCRIPTION_URL}?origin=downgradeToStudent", body)
 				.success ->
 					location.reload()
 				.error ->

services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee

@@ -728,7 +728,7 @@ describe "RecurlyWrapper", ->
 			describe 'when the account does not exist', ->
 
 				beforeEach ->
-					@apiRequest.callsArgWith(1, new Error('not found'), {statusCode: 404}, '')
+					@apiRequest.callsArgWith(1, null, {statusCode: 404}, '')
 
 				it 'should not produce an error', (done) ->
 					@call (err, result) =>