Просмотр исходного кода

Merge branch 'master' into pr-new-logo

Paulo Reis 9 лет назад
Родитель
Сommit
4a86ff4b44

+ 1 - 0
services/web/.nvmrc

@@ -0,0 +1 @@
+0.10.22

+ 1 - 0
services/web/app/coffee/Features/Email/EmailSender.coffee

@@ -72,6 +72,7 @@ module.exports =
 			client.sendMail options, (err, res)->
 				if err?
 					logger.err err:err, "error sending message"
+					err = new Error('Cannot send email')
 				else
 					logger.log "Message sent to #{options.to}"
 				callback(err)

+ 2 - 4
services/web/app/coffee/Features/Project/ProjectApiController.coffee

@@ -4,11 +4,9 @@ logger = require("logger-sharelatex")
 
 module.exports = 
 
-	getProjectDetails : (req, res)->
+	getProjectDetails : (req, res, next)->
 		{project_id} = req.params
 		ProjectDetailsHandler.getDetails project_id, (err, projDetails)->
-			if err?
-				logger.log err:err, project_id:project_id, "something went wrong getting project details"
-				return res.sendStatus 500
+			return next(err) if err?
 			res.json(projDetails)
 

+ 2 - 0
services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee

@@ -5,6 +5,7 @@ logger = require("logger-sharelatex")
 tpdsUpdateSender = require '../ThirdPartyDataStore/TpdsUpdateSender'
 _ = require("underscore")
 PublicAccessLevels = require("../Authorization/PublicAccessLevels")
+Errors = require("../Errors/Errors")
 
 module.exports = 
 
@@ -13,6 +14,7 @@ module.exports =
 			if err?
 				logger.err err:err, project_id:project_id, "error getting project"
 				return callback(err)
+			return callback(new Errors.NotFoundError("project not found")) if !project?
 			UserGetter.getUser project.owner_ref, (err, user) ->
 				return callback(err) if err?
 				details =

+ 24 - 18
services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee

@@ -469,33 +469,39 @@ module.exports = RecurlyWrapper =
 				logger.err err:error,  subscriptionId:subscriptionId, daysUntilExpire:daysUntilExpire,  "error exending trial"
 			callback(error)
 		)
-
-	_parseSubscriptionXml: (xml, callback) ->
-		RecurlyWrapper._parseXml xml, (error, data) ->
+	
+	listAccountActiveSubscriptions: (account_id, callback = (error, subscriptions) ->) ->
+		RecurlyWrapper.apiRequest {
+			url: "accounts/#{account_id}/subscriptions"
+			qs:
+				state: "active"
+			expect404: true
+		}, (error, response, body) ->
 			return callback(error) if error?
-			if data? and data.subscription?
-				recurlySubscription = data.subscription
+			if response.statusCode == 404
+				return callback null, []
 			else
-				return callback "I don't understand the response from Recurly"
-			callback null, recurlySubscription
+				RecurlyWrapper._parseSubscriptionsXml body, callback
+
+	_parseSubscriptionsXml: (xml, callback) ->
+		RecurlyWrapper._parseXmlAndGetAttribute xml, "subscriptions", callback
+
+	_parseSubscriptionXml: (xml, callback) ->
+		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) ->

+ 33 - 25
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 ""
 
 
 

+ 16 - 3
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"

+ 2 - 2
services/web/public/js/ace-1.2.5/worker-latex.js

@@ -1931,7 +1931,7 @@ var InterpretTokens = function (TokeniseResult, ErrorReporter) {
                 if (newPos === null) { continue; } else {i = newPos;};
             } else if (seq === "hbox" || seq === "text" || seq === "mbox" || seq === "footnote" || seq === "intertext" || seq === "shortintertext" || seq === "textnormal" || seq === "tag" || seq === "reflectbox" || seq === "textrm") {
                 nextGroupMathMode = false;
-            } else if (seq === "rotatebox" || seq === "scalebox") {
+            } else if (seq === "rotatebox" || seq === "scalebox"  || seq == "feynmandiagram") {
                 newPos = readOptionalGeneric(TokeniseResult, i);
                 if (newPos === null) { /* do nothing */ } else {i = newPos;};
                 newPos = readDefinition(TokeniseResult, i);
@@ -2179,7 +2179,7 @@ var EnvHandler = function (ErrorReporter) {
     this._beginMathMode = function (thisEnv) {
         var currentMathMode = this.getMathMode(); // undefined, null, $, $$, name of mathmode env
         if (currentMathMode) {
-            ErrorFrom(thisEnv, thisEnv.name + " used inside existing math mode " + getName(currentMathMode),
+            ErrorFrom(thisEnv, getName(thisEnv) + " used inside existing math mode " + getName(currentMathMode),
                       {suppressIfEditing:true, errorAtStart: true, mathMode:true});
         };
         thisEnv.mathMode = thisEnv;

+ 5 - 3
services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee

@@ -51,17 +51,19 @@ describe "EmailSender", ->
 		it "should set the properties on the email to send", (done)->
 			@sesClient.sendMail.callsArgWith(1)
 
-			@sender.sendEmail @opts, =>
+			@sender.sendEmail @opts, (err) =>
+				expect(err).to.not.exist
 				args = @sesClient.sendMail.args[0][0]
 				args.html.should.equal @opts.html
 				args.to.should.equal @opts.to
 				args.subject.should.equal @opts.subject
 				done()
 
-		it "should return the error", (done)->
+		it "should return a non-specific error", (done)->
 			@sesClient.sendMail.callsArgWith(1, "error")
 			@sender.sendEmail {}, (err)=>
-				err.should.equal "error"
+				err.should.exist
+				err.toString().should.equal 'Error: Cannot send email'
 				done()
 
 

+ 4 - 5
services/web/test/UnitTests/coffee/Project/ProjectApiControllerTests.coffee

@@ -20,6 +20,7 @@ describe 'Project api controller', ->
 			session:
 				destroy:sinon.stub()
 		@res = {}
+		@next = sinon.stub()
 		@projDetails = {name:"something"}
 
 
@@ -34,9 +35,7 @@ describe 'Project api controller', ->
 			@controller.getProjectDetails @req, @res
 
 
-		it "should send a 500 if there is an error", (done)->
+		it "should send a 500 if there is an error", ()->
 			@ProjectDetailsHandler.getDetails.callsArgWith(1, "error")
-			@res.sendStatus = (resCode)=>
-				resCode.should.equal 500
-				done()
-			@controller.getProjectDetails @req, @res
+			@controller.getProjectDetails @req, @res, @next
+			@next.calledWith("error").should.equal true

+ 8 - 0
services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee

@@ -1,5 +1,6 @@
 should = require('chai').should()
 modulePath = "../../../../app/js/Features/Project/ProjectDetailsHandler"
+Errors = require "../../../../app/js/Features/Errors/Errors"
 SandboxedModule = require('sandboxed-module')
 sinon = require('sinon')
 assert = require("chai").assert
@@ -48,6 +49,13 @@ describe 'ProjectDetailsHandler', ->
 				assert.equal(details.something, undefined)
 				done()
 
+		it "should return an error for a non-existent project", (done)->
+			@ProjectGetter.getProject.callsArg(2, null, null)
+			err = new Errors.NotFoundError("project not found")
+			@handler.getDetails "0123456789012345678901234", (error, details) =>
+				err.should.eql error
+				done()
+
 		it "should return the error", (done)->
 			error = "some error"
 			@ProjectGetter.getProject.callsArgWith(2, error)

+ 32 - 0
services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee

@@ -1030,3 +1030,35 @@ describe "RecurlyWrapper", ->
 					@call (err, result) =>
 						expect(err).to.be.instanceof Error
 						done()
+
+	describe "listAccountActiveSubscriptions", ->
+		beforeEach ->
+			@user_id = "mock-user-id"
+			@callback = sinon.stub()
+			@RecurlyWrapper.apiRequest = sinon.stub().yields(null, @response = {"mock": "response"}, @body = "<mock body/>")
+			@RecurlyWrapper._parseSubscriptionsXml = sinon.stub().yields(null, @subscriptions = ["mock", "subscriptions"])
+			
+		describe "with an account", ->
+			beforeEach ->
+				@RecurlyWrapper.listAccountActiveSubscriptions @user_id, @callback
+			
+			it "should send a request to Recurly", ->
+				@RecurlyWrapper.apiRequest
+					.calledWith({
+						url: "accounts/#{@user_id}/subscriptions"
+						qs:
+							state: "active"
+						expect404: true
+					})
+					.should.equal true
+
+			it "should return the subscriptions", ->
+				@callback.calledWith(null, @subscriptions).should.equal true
+			
+		describe "without an account", ->
+			beforeEach ->
+				@response.statusCode = 404
+				@RecurlyWrapper.listAccountActiveSubscriptions @user_id, @callback
+
+			it "should return an empty array of subscriptions", ->
+				@callback.calledWith(null, []).should.equal true

+ 12 - 2
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) ->

+ 58 - 16
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