From 7aa4c0c030d994272f17bbd09d82c0583d64dab3 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 27 Mar 2017 12:07:43 +0100 Subject: [PATCH 01/10] Check Recurly for subscription as well before creating subscription --- .../Subscription/RecurlyWrapper.coffee | 38 +++++----- .../SubscriptionController.coffee | 58 ++++++++------- .../Subscription/SubscriptionHandler.coffee | 19 ++++- .../SubscriptionControllerTests.coffee | 14 +++- .../SubscriptionHandlerTests.coffee | 70 +++++++++++++++---- 5 files changed, 137 insertions(+), 62 deletions(-) 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 From 30c5bbfdfc832951607f2d3e0b818801809c083c Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Mar 2017 14:45:34 +0100 Subject: [PATCH 02/10] Add a .nvmrc file --- services/web/.nvmrc | 1 + 1 file changed, 1 insertion(+) create mode 100644 services/web/.nvmrc diff --git a/services/web/.nvmrc b/services/web/.nvmrc new file mode 100644 index 0000000000..994fe99096 --- /dev/null +++ b/services/web/.nvmrc @@ -0,0 +1 @@ +0.10.22 \ No newline at end of file From 69b9b308d4c929cfa133e12d0d827dd1b800c0ce Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Mar 2017 16:17:38 +0100 Subject: [PATCH 03/10] If sending email fails, return a generic error. This prevents us from leaking juicy details of our aws/ses setup via the password-reset form. --- services/web/app/coffee/Features/Email/EmailSender.coffee | 2 +- .../web/test/UnitTests/coffee/Email/EmailSenderTests.coffee | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/Email/EmailSender.coffee b/services/web/app/coffee/Features/Email/EmailSender.coffee index 69574c8276..009d24e182 100644 --- a/services/web/app/coffee/Features/Email/EmailSender.coffee +++ b/services/web/app/coffee/Features/Email/EmailSender.coffee @@ -74,4 +74,4 @@ module.exports = logger.err err:err, "error sending message" else logger.log "Message sent to #{options.to}" - callback(err) + callback(new Error('Cannot send email')) diff --git a/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee b/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee index beba91fcb3..bdc2a44811 100644 --- a/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee +++ b/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee @@ -58,10 +58,11 @@ describe "EmailSender", -> 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() From d2e1efe4a9098bc6713301061d5f923f255034f5 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 27 Mar 2017 17:45:19 +0100 Subject: [PATCH 04/10] fix a daft mistake --- services/web/app/coffee/Features/Email/EmailSender.coffee | 3 ++- .../web/test/UnitTests/coffee/Email/EmailSenderTests.coffee | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Email/EmailSender.coffee b/services/web/app/coffee/Features/Email/EmailSender.coffee index 009d24e182..04b4d39132 100644 --- a/services/web/app/coffee/Features/Email/EmailSender.coffee +++ b/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(new Error('Cannot send email')) + callback(err) diff --git a/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee b/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee index bdc2a44811..dc504907bf 100644 --- a/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee +++ b/services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee @@ -51,7 +51,8 @@ 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 From 4e66b045e38cb1b8bf354e3936994b48588e1c6e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 28 Mar 2017 09:44:50 +0100 Subject: [PATCH 05/10] fix unhandled exception in ProjectDetailsHandler --- .../coffee/Features/Project/ProjectDetailsHandler.coffee | 2 +- .../coffee/Project/ProjectDetailsHandlerTests.coffee | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee index 7bd1d33561..a067a600d1 100644 --- a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee @@ -10,7 +10,7 @@ module.exports = getDetails: (project_id, callback)-> ProjectGetter.getProject project_id, {name:true, description:true, compiler:true, features:true, owner_ref:true}, (err, project)-> - if err? + if err? or !project? logger.err err:err, project_id:project_id, "error getting project" return callback(err) UserGetter.getUser project.owner_ref, (err, user) -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee index b9e8ca27c6..228319abf2 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee @@ -48,6 +48,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) + @handler.getDetails "0123456789012345678901234", (err, details)=> + assert.equal(err, undefined) + assert.equal(details, undefined) + done() + it "should return the error", (done)-> error = "some error" @ProjectGetter.getProject.callsArgWith(2, error) From f433510e619d116190d8f7b30fea3343fdf8cf5f Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 28 Mar 2017 10:12:52 +0100 Subject: [PATCH 06/10] return NotFound error in ProjectDetailsHandler --- .../coffee/Features/Project/ProjectDetailsHandler.coffee | 4 +++- .../coffee/Project/ProjectDetailsHandlerTests.coffee | 7 ++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee index a067a600d1..9c823c9f17 100644 --- a/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectDetailsHandler.coffee @@ -5,14 +5,16 @@ logger = require("logger-sharelatex") tpdsUpdateSender = require '../ThirdPartyDataStore/TpdsUpdateSender' _ = require("underscore") PublicAccessLevels = require("../Authorization/PublicAccessLevels") +Errors = require("../Errors/Errors") module.exports = getDetails: (project_id, callback)-> ProjectGetter.getProject project_id, {name:true, description:true, compiler:true, features:true, owner_ref:true}, (err, project)-> - if err? or !project? + 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 = diff --git a/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee index 228319abf2..501e48f1a5 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectDetailsHandlerTests.coffee +++ b/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 @@ -50,9 +51,9 @@ describe 'ProjectDetailsHandler', -> it "should return an error for a non-existent project", (done)-> @ProjectGetter.getProject.callsArg(2, null, null) - @handler.getDetails "0123456789012345678901234", (err, details)=> - assert.equal(err, undefined) - assert.equal(details, undefined) + err = new Errors.NotFoundError("project not found") + @handler.getDetails "0123456789012345678901234", (error, details) => + err.should.eql error done() it "should return the error", (done)-> From 6002fdbad60b2dd3d42d8e6ce0537d6f20079a40 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 28 Mar 2017 10:30:53 +0100 Subject: [PATCH 07/10] return 404 on project details not found --- .../coffee/Features/Project/ProjectApiController.coffee | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectApiController.coffee b/services/web/app/coffee/Features/Project/ProjectApiController.coffee index b16991ac62..715ccf19ae 100644 --- a/services/web/app/coffee/Features/Project/ProjectApiController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectApiController.coffee @@ -1,4 +1,5 @@ ProjectDetailsHandler = require("./ProjectDetailsHandler") +Errors = require("../Errors/Errors") logger = require("logger-sharelatex") @@ -7,8 +8,11 @@ module.exports = getProjectDetails : (req, res)-> {project_id} = req.params ProjectDetailsHandler.getDetails project_id, (err, projDetails)-> - if err? + if err? and err instanceof Errors.NotFoundError + return res.sendStatus 404 + else if err? logger.log err:err, project_id:project_id, "something went wrong getting project details" return res.sendStatus 500 - res.json(projDetails) + else + res.json(projDetails) From 835d8d618d21f0bd9098953c0570f589654915f9 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 28 Mar 2017 11:33:37 +0100 Subject: [PATCH 08/10] use error handler --- .../Features/Project/ProjectApiController.coffee | 12 +++--------- .../coffee/Project/ProjectApiControllerTests.coffee | 9 ++++----- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectApiController.coffee b/services/web/app/coffee/Features/Project/ProjectApiController.coffee index 715ccf19ae..f832a75b54 100644 --- a/services/web/app/coffee/Features/Project/ProjectApiController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectApiController.coffee @@ -1,18 +1,12 @@ ProjectDetailsHandler = require("./ProjectDetailsHandler") -Errors = require("../Errors/Errors") 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? and err instanceof Errors.NotFoundError - return res.sendStatus 404 - else if err? - logger.log err:err, project_id:project_id, "something went wrong getting project details" - return res.sendStatus 500 - else - res.json(projDetails) + return next(err) if err? + res.json(projDetails) diff --git a/services/web/test/UnitTests/coffee/Project/ProjectApiControllerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectApiControllerTests.coffee index 9476a80019..ddf23c0bac 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectApiControllerTests.coffee +++ b/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 From 08699d7aa265ad0622a0ef82cb00ec0ac892aeba Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 28 Mar 2017 15:46:58 +0100 Subject: [PATCH 09/10] Handle a 404 from Recurly if account doesn't exist --- .../Subscription/RecurlyWrapper.coffee | 6 +++- .../Subscription/RecurlyWrapperTests.coffee | 32 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee index ce40847a27..fddd4f3567 100644 --- a/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee +++ b/services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee @@ -475,9 +475,13 @@ module.exports = RecurlyWrapper = url: "accounts/#{account_id}/subscriptions" qs: state: "active" + expect404: true }, (error, response, body) -> return callback(error) if error? - RecurlyWrapper._parseSubscriptionsXml body, callback + if response.statusCode == 404 + return callback null, [] + else + RecurlyWrapper._parseSubscriptionsXml body, callback _parseSubscriptionsXml: (xml, callback) -> RecurlyWrapper._parseXmlAndGetAttribute xml, "subscriptions", callback diff --git a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee b/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee index c55efdb3c5..d951653310 100644 --- a/services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee +++ b/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 = "") + @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 \ No newline at end of file From 13492c7fc4e99af7790feb21c116c9c502a40c19 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 29 Mar 2017 15:27:10 +0100 Subject: [PATCH 10/10] handle the \feynmandiagram command in code check pulled from our ace repository https://github.com/sharelatex/ace commit baeb9aff561d048b8a839683261ffdf149ecd4ef --- services/web/public/js/ace-1.2.5/worker-latex.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/public/js/ace-1.2.5/worker-latex.js b/services/web/public/js/ace-1.2.5/worker-latex.js index 3fa9bceb07..ccd7e2da9b 100644 --- a/services/web/public/js/ace-1.2.5/worker-latex.js +++ b/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;