From 66998e2a008c414efc36ec2c33b5a994377e0c86 Mon Sep 17 00:00:00 2001 From: M Fahru Date: Wed, 20 Jul 2022 07:05:57 -0400 Subject: [PATCH] Refactor createSubscription in SubscriptionController to use async/await (#8938) GitOrigin-RevId: 47fd56f35ac1d5727c7e53eeecedf200738c4e11 --- .../Subscription/SubscriptionController.js | 74 +++++++++---------- .../SubscriptionControllerTests.js | 26 ++++--- 2 files changed, 48 insertions(+), 52 deletions(-) diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index d72d1e2e7f..29d3b4a1b1 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -261,7 +261,7 @@ async function interstitialPaymentPage(req, res) { } } -function createSubscription(req, res, next) { +async function createSubscription(req, res) { const user = SessionManager.getSessionUser(req.session) const recurlyTokenIds = { billing: req.body.recurly_token_id, @@ -270,47 +270,41 @@ function createSubscription(req, res, next) { } const { subscriptionDetails } = req.body - LimitationsManager.userHasV1OrV2Subscription( - user, - function (err, hasSubscription) { - if (err) { - return next(err) - } - if (hasSubscription) { - logger.warn({ user_id: user._id }, 'user already has subscription') - return res.sendStatus(409) // conflict - } - return SubscriptionHandler.createSubscription( - user, - subscriptionDetails, - recurlyTokenIds, - function (err) { - if (!err) { - return res.sendStatus(201) - } + const hasSubscription = + await LimitationsManager.promises.userHasV1OrV2Subscription(user) - if ( - err instanceof SubscriptionErrors.RecurlyTransactionError || - err instanceof Errors.InvalidError - ) { - logger.error({ err }, 'recurly transaction error, potential 422') - HttpErrorHandler.unprocessableEntity( - req, - res, - err.message, - OError.getFullInfo(err).public - ) - } else { - logger.warn( - { err, user_id: user._id }, - 'something went wrong creating subscription' - ) - next(err) - } - } + if (hasSubscription) { + logger.warn({ user_id: user._id }, 'user already has subscription') + return res.sendStatus(409) // conflict + } + + try { + await SubscriptionHandler.promises.createSubscription( + user, + subscriptionDetails, + recurlyTokenIds + ) + + res.sendStatus(201) + } catch (err) { + if ( + err instanceof SubscriptionErrors.RecurlyTransactionError || + err instanceof Errors.InvalidError + ) { + logger.error({ err }, 'recurly transaction error, potential 422') + HttpErrorHandler.unprocessableEntity( + req, + res, + err.message, + OError.getFullInfo(err).public + ) + } else { + logger.warn( + { err, user_id: user._id }, + 'something went wrong creating subscription' ) } - ) + } } async function successfulSubscription(req, res) { @@ -605,7 +599,7 @@ module.exports = { paymentPage: expressify(paymentPage), userSubscriptionPage: expressify(userSubscriptionPage), interstitialPaymentPage: expressify(interstitialPaymentPage), - createSubscription, + createSubscription: expressify(createSubscription), successfulSubscription: expressify(successfulSubscription), cancelSubscription, canceledSubscription, diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js index 5122d83415..ffc771ee5a 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js @@ -492,7 +492,7 @@ describe('SubscriptionController', function () { }) it('should send the user and subscriptionId to the handler', function (done) { - this.SubscriptionHandler.createSubscription + this.SubscriptionHandler.promises.createSubscription .calledWithMatch( this.user, this.subscriptionDetails, @@ -502,7 +502,7 @@ describe('SubscriptionController', function () { done() }) - it('should redurect to the subscription page', function (done) { + it('should redirect to the subscription page', function (done) { this.res.sendStatus.calledWith(201).should.equal(true) done() }) @@ -510,11 +510,13 @@ describe('SubscriptionController', function () { describe('createSubscription with errors', function () { it('should handle users with subscription', function (done) { - this.LimitationsManager.userHasV1OrV2Subscription.yields(null, true) + this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(true) this.SubscriptionController.createSubscription(this.req, { sendStatus: status => { expect(status).to.equal(409) - this.SubscriptionHandler.createSubscription.called.should.equal(false) + this.SubscriptionHandler.promises.createSubscription.called.should.equal( + false + ) done() }, @@ -523,8 +525,8 @@ describe('SubscriptionController', function () { it('should handle 3DSecure errors', function (done) { this.next = sinon.stub() - this.LimitationsManager.userHasV1OrV2Subscription.yields(null, false) - this.SubscriptionHandler.createSubscription.yields( + this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(false) + this.SubscriptionHandler.promises.createSubscription.rejects( new SubscriptionErrors.RecurlyTransactionError({}) ) this.HttpErrorHandler.unprocessableEntity = sinon.spy( @@ -540,8 +542,8 @@ describe('SubscriptionController', function () { it('should handle validation errors', function (done) { this.next = sinon.stub() - this.LimitationsManager.userHasV1OrV2Subscription.yields(null, false) - this.SubscriptionHandler.createSubscription.yields( + this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(false) + this.SubscriptionHandler.promises.createSubscription.rejects( new Errors.InvalidError('invalid error test') ) this.HttpErrorHandler.unprocessableEntity = sinon.spy( @@ -556,8 +558,8 @@ describe('SubscriptionController', function () { }) it('should handle recurly errors', function (done) { - this.LimitationsManager.userHasV1OrV2Subscription.yields(null, false) - this.SubscriptionHandler.createSubscription.yields( + this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(false) + this.SubscriptionHandler.promises.createSubscription.rejects( new SubscriptionErrors.RecurlyTransactionError({}) ) @@ -574,8 +576,8 @@ describe('SubscriptionController', function () { }) it('should handle invalid error', function (done) { - this.LimitationsManager.userHasV1OrV2Subscription.yields(null, false) - this.SubscriptionHandler.createSubscription.yields( + this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(false) + this.SubscriptionHandler.promises.createSubscription.rejects( new Errors.InvalidError({}) )