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

@@ -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()
     };

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);
+        }
+    }
+};

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;
 

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) {

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();
+        });
+    });
+});