From 3ed10ecdf6722e192fb2f65101d6f56e73905611 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Thu, 25 Mar 2021 12:19:01 +0000 Subject: [PATCH] Added tests for updateSubscription refs https://github.com/TryGhost/Team/issues/530 The RouterController was a grab bag of all controller methods, making it difficult to mock & test. This adds a MemberController with a smaller API - making it easier to test. --- ghost/members-api/index.js | 10 +- .../lib/controllers/member/index.js | 104 ++++++++++++++++++ .../lib/controllers/router/index.js | 96 ---------------- .../lib/repositories/member/index.js | 38 ++++--- .../unit/lib/controllers/member/index.test.js | 94 ++++++++++++++++ 5 files changed, 232 insertions(+), 110 deletions(-) create mode 100644 ghost/members-api/lib/controllers/member/index.js create mode 100644 ghost/members-api/test/unit/lib/controllers/member/index.test.js diff --git a/ghost/members-api/index.js b/ghost/members-api/index.js index 5f3e7d3ef9..fe851ffae3 100644 --- a/ghost/members-api/index.js +++ b/ghost/members-api/index.js @@ -11,6 +11,7 @@ const GeolocationSerice = require('./lib/services/geolocation'); const MemberRepository = require('./lib/repositories/member'); const EventRepository = require('./lib/repositories/event'); const RouterController = require('./lib/controllers/router'); +const MemberController = require('./lib/controllers/member'); module.exports = function MembersApi({ tokenConfig: { @@ -113,6 +114,13 @@ module.exports = function MembersApi({ getSubject }); + const memberController = new MemberController({ + memberRepository, + stripeAPIService, + stripePlansService, + tokenService + }); + const routerController = new RouterController({ memberRepository, allowSelfSignup, @@ -282,7 +290,7 @@ module.exports = function MembersApi({ ), updateSubscription: Router({mergeParams: true}).use( body.json(), - (req, res) => routerController.updateSubscription(req, res) + (req, res) => memberController.updateSubscription(req, res) ), handleStripeWebhook: Router() }; diff --git a/ghost/members-api/lib/controllers/member/index.js b/ghost/members-api/lib/controllers/member/index.js new file mode 100644 index 0000000000..0756613920 --- /dev/null +++ b/ghost/members-api/lib/controllers/member/index.js @@ -0,0 +1,104 @@ +const errors = require('ghost-ignition').errors; + +/** + * MemberController + * + * @param {object} deps + * @param {any} deps.memberRepository + * @param {any} deps.stripePlansService + * @param {any} deps.tokenService + */ +module.exports = class MemberController { + constructor({ + memberRepository, + stripePlansService, + tokenService + }) { + this._memberRepository = memberRepository; + this._stripePlansService = stripePlansService; + this._tokenService = tokenService; + } + + async updateSubscription(req, res) { + try { + const identity = req.body.identity; + const subscriptionId = req.params.id; + const cancelAtPeriodEnd = req.body.cancel_at_period_end; + const cancellationReason = req.body.cancellation_reason; + const planName = req.body.planName; + + if (cancelAtPeriodEnd === undefined && planName === undefined) { + throw new errors.BadRequestError({ + message: 'Updating subscription failed!', + help: 'Request should contain "cancel_at_period_end" or "planName" field.' + }); + } + + if ((cancelAtPeriodEnd === undefined || cancelAtPeriodEnd === false) && cancellationReason !== undefined) { + throw new errors.BadRequestError({ + message: 'Updating subscription failed!', + help: '"cancellation_reason" field requires the "cancel_at_period_end" field to be true.' + }); + } + + if (cancellationReason && cancellationReason.length > 500) { + throw new errors.BadRequestError({ + message: 'Updating subscription failed!', + help: '"cancellation_reason" field can be a maximum of 500 characters.' + }); + } + + let email; + try { + if (!identity) { + throw new errors.BadRequestError({ + message: 'Updating subscription failed! Could not find member' + }); + } + + const claims = await this._tokenService.decodeToken(identity); + email = claims && claims.sub; + } catch (err) { + res.writeHead(401); + return res.end('Unauthorized'); + } + + if (!email) { + throw new errors.BadRequestError({ + message: 'Invalid token' + }); + } + + if (planName !== undefined) { + const plan = this._stripePlansService.getPlan(planName); + if (!plan) { + throw new errors.BadRequestError({ + message: 'Updating subscription failed! Could not find plan' + }); + } + await this._memberRepository.updateSubscription({ + email, + subscription: { + subscription_id: subscriptionId, + plan: plan.id + } + }); + } else if (cancelAtPeriodEnd !== undefined) { + await this._memberRepository.updateSubscription({ + email, + subscription: { + subscription_id: subscriptionId, + cancel_at_period_end: cancelAtPeriodEnd, + cancellationReason + } + }); + } + + res.writeHead(204); + res.end(); + } catch (err) { + res.writeHead(err.statusCode || 500); + res.end(err.message); + } + } +}; diff --git a/ghost/members-api/lib/controllers/router/index.js b/ghost/members-api/lib/controllers/router/index.js index b8f682a3f1..5ca1228d80 100644 --- a/ghost/members-api/lib/controllers/router/index.js +++ b/ghost/members-api/lib/controllers/router/index.js @@ -49,102 +49,6 @@ module.exports = class RouterController { } } - async updateSubscription(req, res) { - try { - const identity = req.body.identity; - const subscriptionId = req.params.id; - const cancelAtPeriodEnd = req.body.cancel_at_period_end; - const cancellationReason = req.body.cancellation_reason; - const planName = req.body.planName; - - if (cancelAtPeriodEnd === undefined && planName === undefined) { - throw new errors.BadRequestError({ - message: 'Updating subscription failed!', - help: 'Request should contain "cancel_at_period_end" or "planName" field.' - }); - } - - if ((cancelAtPeriodEnd === undefined || cancelAtPeriodEnd === false) && cancellationReason !== undefined) { - throw new errors.BadRequestError({ - message: 'Updating subscription failed!', - help: '"cancellation_reason" field requires the "cancel_at_period_end" field to be true.' - }); - } - - if (cancellationReason && cancellationReason.length > 500) { - throw new errors.BadRequestError({ - message: 'Updating subscription failed!', - help: '"cancellation_reason" field can be a maximum of 500 characters.' - }); - } - - let email; - try { - if (!identity) { - throw new errors.BadRequestError({ - message: 'Updating subscription failed! Could not find member' - }); - } - - const claims = await this._tokenService.decodeToken(identity); - email = claims && claims.sub; - } catch (err) { - res.writeHead(401); - return res.end('Unauthorized'); - } - - const member = email ? await this._memberRepository.get({email}, {withRelated: ['stripeSubscriptions']}) : null; - - if (!member) { - throw new errors.BadRequestError({ - message: 'Updating subscription failed! Could not find member' - }); - } - - // Don't allow removing subscriptions that don't belong to the member - const subscription = member.related('stripeSubscriptions').models.find( - subscription => subscription.get('subscription_id') === subscriptionId - ); - if (!subscription) { - res.writeHead(403); - return res.end('No permission'); - } - - let updatedSubscription; - if (planName !== undefined) { - const plan = this._stripePlansService.getPlans().find(plan => plan.nickname === planName); - if (!plan) { - throw new errors.BadRequestError({ - message: 'Updating subscription failed! Could not find plan' - }); - } - updatedSubscription = await this._stripeAPIService.changeSubscriptionPlan(subscriptionId, plan.id); - } else if (cancelAtPeriodEnd !== undefined) { - if (cancelAtPeriodEnd) { - updatedSubscription = await this._stripeAPIService.cancelSubscriptionAtPeriodEnd( - subscriptionId, cancellationReason - ); - } else { - updatedSubscription = await this._stripeAPIService.continueSubscriptionAtPeriodEnd( - subscriptionId - ); - } - } - if (updatedSubscription) { - await this._memberRepository.linkSubscription({ - id: member.id, - subscription: updatedSubscription - }); - } - - res.writeHead(204); - res.end(); - } catch (err) { - res.writeHead(err.statusCode || 500); - res.end(err.message); - } - } - async createCheckoutSetupSession(req, res) { const identity = req.body.identity; diff --git a/ghost/members-api/lib/repositories/member/index.js b/ghost/members-api/lib/repositories/member/index.js index 9bda5b9ffc..c31f83b0b8 100644 --- a/ghost/members-api/lib/repositories/member/index.js +++ b/ghost/members-api/lib/repositories/member/index.js @@ -374,8 +374,9 @@ module.exports = class MemberRepository { if (!this._stripeAPIService.configured) { throw new Error('Cannot update Stripe Subscription with no Stripe Connection'); } + const member = await this._Member.findOne({ - id: data.id + email: data.email }); const subscription = await member.related('stripeSubscriptions').query({ @@ -388,22 +389,33 @@ module.exports = class MemberRepository { throw new Error('Subscription not found'); } - if (data.subscription.cancel_at_period_end === undefined) { - throw new Error('Incorrect usage'); + let updatedSubscription; + if (data.subscription.plan) { + updatedSubscription = await this._stripeAPIService.changeSubscriptionPlan( + data.subscription.subscription_id, + data.subscription.plan + ); } - if (data.subscription.cancel_at_period_end) { - await this._stripeAPIService.cancelSubscriptionAtPeriodEnd(data.subscription.subscription_id); - } else { - await this._stripeAPIService.continueSubscriptionAtPeriodEnd(data.subscription.subscription_id); + if (data.subscription.cancel_at_period_end !== undefined) { + if (data.subscription.cancel_at_period_end) { + updatedSubscription = await this._stripeAPIService.cancelSubscriptionAtPeriodEnd( + data.subscription.subscription_id, + data.subscription.cancellationReason + ); + } else { + updatedSubscription = await this._stripeAPIService.continueSubscriptionAtPeriodEnd( + data.subscription.subscription_id + ); + } } - await this._StripeCustomerSubscription.edit({ - subscription_id: data.subscription.subscription_id, - cancel_at_period_end: data.subscription.cancel_at_period_end - }, { - id: subscription.id - }); + if (updatedSubscription) { + await this.linkSubscription({ + id: member.id, + subscription: updatedSubscription + }); + } } async setComplimentarySubscription(data, options) { diff --git a/ghost/members-api/test/unit/lib/controllers/member/index.test.js b/ghost/members-api/test/unit/lib/controllers/member/index.test.js new file mode 100644 index 0000000000..cde35117fc --- /dev/null +++ b/ghost/members-api/test/unit/lib/controllers/member/index.test.js @@ -0,0 +1,94 @@ +const sinon = require('sinon'); +const MemberController = require('../../../../../lib/controllers/member'); + +describe('MemberController', function () { + describe('updateSubscription', function () { + it('Updates a subscriptions plan via the member repository', async function () { + const tokenService = { + decodeToken: sinon.fake.resolves({sub: 'fake@email.com'}) + }; + const stripePlansService = { + getPlan: sinon.fake.returns({id: 'plan_id'}) + }; + + const memberRepository = { + updateSubscription: sinon.mock('updateSubscription').once().withArgs({ + email: 'fake@email.com', + subscription: { + subscription_id: 'subscription_id', + plan: 'plan_id' + } + }) + }; + + const controller = new MemberController({ + memberRepository, + stripePlansService, + tokenService + }); + + const req = { + body: { + identity: 'token', + planName: 'plan_name' + }, + params: { + id: 'subscription_id' + } + }; + const res = { + writeHead() {}, + end() {} + }; + + await controller.updateSubscription(req, res); + + memberRepository.updateSubscription.verify(); + }); + + it('Updates a subscriptions cancel_at_period_end via the member repository', async function () { + const tokenService = { + decodeToken: sinon.fake.resolves({sub: 'fake@email.com'}) + }; + const stripePlansService = { + getPlan: sinon.fake.returns({id: 'plan_id'}) + }; + + const memberRepository = { + updateSubscription: sinon.mock('updateSubscription').once().withArgs({ + email: 'fake@email.com', + subscription: { + subscription_id: 'subscription_id', + cancellationReason: 'For reasonable reasons', + cancel_at_period_end: true + } + }) + }; + + const controller = new MemberController({ + memberRepository, + stripePlansService, + tokenService + }); + + const req = { + body: { + identity: 'token', + cancel_at_period_end: true, + cancellation_reason: 'For reasonable reasons' + }, + params: { + id: 'subscription_id' + } + }; + const res = { + writeHead() {}, + end() {} + }; + + await controller.updateSubscription(req, res); + + memberRepository.updateSubscription.verify(); + }); + }); +});