From bcc98e55368817a4d334bea36c9f025f3833e88c Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Tue, 14 Nov 2017 11:09:41 +0000 Subject: [PATCH] Added GET/DELETE /subscribers/email/:email/ endpoints (#9238) no issue - useful for managing subscribers via external systems/API calls where it's likely only the e-mail address will be known - adds `GET /subscribers/email/:email/` - adds `DELETE /subscribers/email/:email/` --- core/server/api/routes.js | 2 + core/server/api/subscribers.js | 28 ++++++++++++- core/server/api/utils.js | 3 +- .../integration/api/api_subscribers_spec.js | 42 +++++++++++++++++-- 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/core/server/api/routes.js b/core/server/api/routes.js index 9cea8b783d..524e99d794 100644 --- a/core/server/api/routes.js +++ b/core/server/api/routes.js @@ -84,9 +84,11 @@ module.exports = function apiRoutes() { api.http(api.subscribers.importCSV) ); apiRouter.get('/subscribers/:id', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.read)); + apiRouter.get('/subscribers/email/:email', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.read)); apiRouter.post('/subscribers', labs.subscribers, mw.authenticatePublic, api.http(api.subscribers.add)); apiRouter.put('/subscribers/:id', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.edit)); apiRouter.del('/subscribers/:id', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.destroy)); + apiRouter.del('/subscribers/email/:email', labs.subscribers, mw.authenticatePrivate, api.http(api.subscribers.destroy)); // ## Roles apiRouter.get('/roles/', mw.authenticatePrivate, api.http(api.roles.browse)); diff --git a/core/server/api/subscribers.js b/core/server/api/subscribers.js index 36998e865f..6f1321c8ed 100644 --- a/core/server/api/subscribers.js +++ b/core/server/api/subscribers.js @@ -53,7 +53,7 @@ subscribers = { * @return {Promise} Subscriber */ read: function read(options) { - var attrs = ['id'], + var attrs = ['id', 'email'], tasks; /** @@ -190,6 +190,29 @@ subscribers = { destroy: function destroy(options) { var tasks; + /** + * ### Delete Subscriber + * If we have an email param, check the subscriber exists + * @type {[type]} + */ + function getSubscriberByEmail(options) { + if (options.email) { + return models.Subscriber.getByEmail(options.email, options) + .then(function (subscriber) { + if (!subscriber) { + return Promise.reject(new errors.NotFoundError({ + message: i18n.t('errors.api.subscribers.subscriberNotFound') + })); + } + + options.id = subscriber.get('id'); + return options; + }); + } + + return options; + } + /** * ### Delete Subscriber * Make the call to the Model layer @@ -201,8 +224,9 @@ subscribers = { // Push all of our tasks into a `tasks` array in the correct order tasks = [ - apiUtils.validate(docName, {opts: apiUtils.idDefaultOptions}), + apiUtils.validate(docName, {opts: ['id', 'email']}), apiUtils.handlePermissions(docName, 'destroy'), + getSubscriberByEmail, doQuery ]; diff --git a/core/server/api/utils.js b/core/server/api/utils.js index a31aaf5ac5..cbdfbfebef 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -118,7 +118,8 @@ utils = { to: {isDate: true}, fields: {matches: /^[\w, ]+$/}, order: {matches: /^[a-z0-9_,\. ]+$/i}, - name: {} + name: {}, + email: {isEmail: true} }, // these values are sanitised/validated separately noValidation = ['data', 'context', 'include', 'filter', 'forUpdate', 'transacting', 'formats'], diff --git a/core/test/integration/api/api_subscribers_spec.js b/core/test/integration/api/api_subscribers_spec.js index 08a08e1e8d..b284d7c66a 100644 --- a/core/test/integration/api/api_subscribers_spec.js +++ b/core/test/integration/api/api_subscribers_spec.js @@ -155,10 +155,10 @@ describe('Subscribers API', function () { }); describe('Destroy', function () { - var firstSubscriber = testUtils.DataGenerator.Content.subscribers[0].id; + var firstSubscriber = testUtils.DataGenerator.Content.subscribers[0]; it('can destroy subscriber as admin', function (done) { - SubscribersAPI.destroy(_.extend({}, testUtils.context.admin, {id: firstSubscriber})) + SubscribersAPI.destroy(_.extend({}, testUtils.context.admin, {id: firstSubscriber.id})) .then(function (results) { should.not.exist(results); @@ -166,8 +166,28 @@ describe('Subscribers API', function () { }).catch(done); }); + it('can destroy subscriber by email', function (done) { + SubscribersAPI.destroy(_.extend({}, testUtils.context.admin, {email: firstSubscriber.email})) + .then(function (results) { + should.not.exist(results); + + done(); + }).catch(done); + }); + + it('returns NotFoundError for unknown email', function (done) { + SubscribersAPI.destroy(_.extend({}, testUtils.context.admin, {email: 'unknown@example.com'})) + .then(function (results) { + done(new Error('Destroy subscriber should not be possible with unknown email.')); + }, function (err) { + should.exist(err); + (err instanceof errors.NotFoundError).should.eql(true); + done(); + }).catch(done); + }); + it('CANNOT destroy subscriber', function (done) { - SubscribersAPI.destroy(_.extend({}, testUtils.context.editor, {id: firstSubscriber})) + SubscribersAPI.destroy(_.extend({}, testUtils.context.editor, {id: firstSubscriber.id})) .then(function () { done(new Error('Destroy subscriber should not be possible as editor.')); }, function (err) { @@ -227,6 +247,22 @@ describe('Subscribers API', function () { }).catch(done); }); + it('with email', function (done) { + SubscribersAPI.browse({context: {user: 1}}).then(function (results) { + should.exist(results); + should.exist(results.subscribers); + results.subscribers.length.should.be.above(0); + + var firstSubscriber = _.find(results.subscribers, {id: testUtils.DataGenerator.Content.subscribers[0].id}); + return SubscribersAPI.read({context: {user: 1}, email: firstSubscriber.email}); + }).then(function (found) { + should.exist(found); + testUtils.API.checkResponse(found.subscribers[0], 'subscriber'); + + done(); + }).catch(done); + }); + it('CANNOT fetch a subscriber which doesn\'t exist', function (done) { SubscribersAPI.read({context: {user: 1}, id: 999}).then(function () { done(new Error('Should not return a result'));