diff --git a/ghost/members-api/lib/controllers/RouterController.js b/ghost/members-api/lib/controllers/RouterController.js index a06457f30d..3be126c547 100644 --- a/ghost/members-api/lib/controllers/RouterController.js +++ b/ghost/members-api/lib/controllers/RouterController.js @@ -17,7 +17,8 @@ const messages = { memberNotFoundSignUp: 'No member exists with this e-mail address. Please sign up first.', invalidType: 'Invalid checkout type.', notConfigured: 'This site is not accepting payments at the moment.', - invalidNewsletterId: 'Cannot subscribe to invalid newsletter {ids}' + invalidNewsletters: 'Cannot subscribe to invalid newsletters {newsletters}', + archivedNewsletters: 'Cannot subscribe to archived newsletters {newsletters}' }; module.exports = class RouterController { @@ -483,19 +484,30 @@ module.exports = class RouterController { // Validate requested newsletters let {newsletters: requestedNewsletters} = req.body; - if (requestedNewsletters && requestedNewsletters.length > 0) { - const newsletterIds = requestedNewsletters.map(newsletter => newsletter.id); + if (requestedNewsletters && requestedNewsletters.length > 0 && requestedNewsletters.every(newsletter => newsletter.name !== undefined)) { + const newsletterNames = requestedNewsletters.map(newsletter => newsletter.name); + const newsletterNamesFilter = newsletterNames.map(newsletter => `'${newsletter.replace(/("|')/g, '\\$1')}'`); const newsletters = await this._newslettersService.browse({ - filter: `id:[${newsletterIds}]`, - columns: ['id','status'] + filter: `name:[${newsletterNamesFilter}]`, + columns: ['id','name','status'] }); - if (newsletters.length !== newsletterIds.length) { - const validNewsletterIds = newsletters.map(newsletter => newsletter.id); - const invalidNewsletterIds = newsletterIds.filter(id => !validNewsletterIds.includes(id)); + if (newsletters.length !== newsletterNames.length) { //check for invalid newsletters + const validNewsletters = newsletters.map(newsletter => newsletter.name); + const invalidNewsletters = newsletterNames.filter(newsletter => !validNewsletters.includes(newsletter)); throw new errors.BadRequestError({ - message: tpl(messages.invalidNewsletterId, {ids: invalidNewsletterIds}) + message: tpl(messages.invalidNewsletters, {newsletters: invalidNewsletters}) + }); + } + + //validation for archived newsletters + const archivedNewsletters = newsletters + .filter(newsletter => newsletter.status === 'archived') + .map(newsletter => newsletter.name); + if (archivedNewsletters && archivedNewsletters.length > 0) { + throw new errors.BadRequestError({ + message: tpl(messages.archivedNewsletters, {newsletters: archivedNewsletters}) }); } diff --git a/ghost/members-api/test/unit/lib/controllers/router.test.js b/ghost/members-api/test/unit/lib/controllers/router.test.js index fa28b43794..f3b23aa0f8 100644 --- a/ghost/members-api/test/unit/lib/controllers/router.test.js +++ b/ghost/members-api/test/unit/lib/controllers/router.test.js @@ -126,29 +126,33 @@ describe('RouterController', function () { const newsletters = [ { id: 'abc123', + name: 'Newsletter 1', status: 'active' }, { id: 'def456', + name: 'Newsletter 2', status: 'active' }, { id: 'ghi789', + name: 'Newsletter 3', status: 'active' } ]; - req.body.newsletters = newsletters.map(newsletter => ({id: newsletter.id})); + req.body.newsletters = newsletters.map(newsletter => ({name: newsletter.name})); - const newsletterIds = newsletters.map(newsletter => newsletter.id); + const newsletterNames = newsletters.map(newsletter => newsletter.name); + const newsletterNamesFilter = newsletterNames.map(newsletter => `'${newsletter.replace(/("|')/g, '\\$1')}'`); const newslettersServiceStub = { browse: sinon.stub() }; newslettersServiceStub.browse .withArgs({ - filter: `id:[${newsletterIds}]`, - columns: ['id', 'status'] + filter: `name:[${newsletterNamesFilter}]`, + columns: ['id','name','status'] }) .resolves(newsletters); @@ -170,10 +174,10 @@ describe('RouterController', function () { }); it('validates specified newsletters', async function () { - const INVALID_NEWSLETTER_ID = 'abc123'; + const INVALID_NEWSLETTER_NAME = 'abc123'; req.body.newsletters = [ - {id: INVALID_NEWSLETTER_ID} + {name: INVALID_NEWSLETTER_NAME} ]; const newslettersServiceStub = { @@ -182,8 +186,8 @@ describe('RouterController', function () { newslettersServiceStub.browse .withArgs({ - filter: `id:[${INVALID_NEWSLETTER_ID}]`, - columns: ['id', 'status'] + filter: `name:['${INVALID_NEWSLETTER_NAME}']`, + columns: ['id','name','status'] }) .resolves([]); @@ -191,36 +195,39 @@ describe('RouterController', function () { newslettersService: newslettersServiceStub }); - await controller.sendMagicLink(req, res).should.be.rejectedWith(`Cannot subscribe to invalid newsletter ${INVALID_NEWSLETTER_ID}`); + await controller.sendMagicLink(req, res).should.be.rejectedWith(`Cannot subscribe to invalid newsletters ${INVALID_NEWSLETTER_NAME}`); }); - it('does not add specified newsletters to the tokenData if they are archived', async function () { + it('validates archived newsletters', async function () { const newsletters = [ { id: 'abc123', + name: 'Newsletter 1', status: 'active' }, { id: 'def456', + name: 'Newsletter 2', status: 'archived' }, { id: 'ghi789', + name: 'Newsletter 3', status: 'active' } ]; - req.body.newsletters = newsletters.map(newsletter => ({id: newsletter.id})); + req.body.newsletters = newsletters.map(newsletter => ({name: newsletter.name})); - const newsletterIds = newsletters.map(newsletter => newsletter.id); + const newsletterNames = newsletters.map(newsletter => `'${newsletter.name}'`); const newslettersServiceStub = { browse: sinon.stub() }; newslettersServiceStub.browse .withArgs({ - filter: `id:[${newsletterIds}]`, - columns: ['id', 'status'] + filter: `name:[${newsletterNames}]`, + columns: ['id', 'name','status'] }) .resolves(newsletters); @@ -228,16 +235,7 @@ describe('RouterController', function () { newslettersService: newslettersServiceStub }); - await controller.sendMagicLink(req, res); - - res.writeHead.calledOnceWith(201).should.be.true(); - res.end.calledOnceWith('Created.').should.be.true(); - - sendEmailWithMagicLinkStub.calledOnce.should.be.true(); - sendEmailWithMagicLinkStub.args[0][0].tokenData.newsletters.should.eql([ - {id: newsletters[0].id}, - {id: newsletters[2].id} - ]); + await controller.sendMagicLink(req, res).should.be.rejectedWith(`Cannot subscribe to archived newsletters Newsletter 2`); }); }); });