diff --git a/core/server/api/canary/members.js b/core/server/api/canary/members.js index aceef0fd78..88392606dc 100644 --- a/core/server/api/canary/members.js +++ b/core/server/api/canary/members.js @@ -1,7 +1,9 @@ // NOTE: We must not cache references to membersService.api // as it is a getter and may change during runtime. +const Promise = require('bluebird'); const membersService = require('../../services/members'); const common = require('../../lib/common'); +const fsLib = require('../../lib/fs'); const members = { docName: 'members', @@ -58,13 +60,30 @@ const members = { } }, permissions: true, - async query(frame) { - const member = await membersService.api.members.create(frame.data.members[0], { - sendEmail: frame.options.send_email, - emailType: frame.options.email_type - }); + query(frame) { + // NOTE: Promise.resolve() is here for a reason! Method has to return an instance + // of a Bluebird promise to allow reflection. If decided to be replaced + // with something else, e.g: async/await, CSV export function + // would need a deep rewrite (see failing tests if this line is removed) + return Promise.resolve() + .then(() => { + return membersService.api.members.create(frame.data.members[0], { + sendEmail: frame.options.send_email, + emailType: frame.options.email_type + }); + }) + .then((member) => { + if (member) { + return Promise.resolve(member); + } + }) + .catch((error) => { + if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) { + return Promise.reject(new common.errors.ValidationError({message: common.i18n.t('errors.api.members.memberAlreadyExists')})); + } - return member; + return Promise.reject(error); + }); } }, @@ -107,6 +126,61 @@ const members = { await membersService.api.members.destroy(frame.options); return null; } + }, + + importCSV: { + statusCode: 201, + permissions: { + method: 'add' + }, + async query(frame) { + let filePath = frame.file.path, + fulfilled = 0, + invalid = 0, + duplicates = 0; + + return fsLib.readCSV({ + path: filePath, + columnsToExtract: [{name: 'email', lookup: /email/i}, {name: 'name', lookup: /name/i}] + }).then((result) => { + return Promise.all(result.map((entry) => { + const api = require('./index'); + + return api.members.add.query({ + data: { + members: [{ + email: entry.email, + name: entry.name + }] + }, + options: { + context: frame.options.context, + options: {send_email: false} + } + }).reflect(); + })).each((inspection) => { + if (inspection.isFulfilled()) { + fulfilled = fulfilled + 1; + } else { + if (inspection.reason() instanceof common.errors.ValidationError) { + duplicates = duplicates + 1; + } else { + invalid = invalid + 1; + } + } + }); + }).then(() => { + return { + meta: { + stats: { + imported: fulfilled, + duplicates: duplicates, + invalid: invalid + } + } + }; + }); + } } }; diff --git a/core/server/api/canary/utils/serializers/output/members.js b/core/server/api/canary/utils/serializers/output/members.js index d909cce65d..b455817f97 100644 --- a/core/server/api/canary/utils/serializers/output/members.js +++ b/core/server/api/canary/utils/serializers/output/members.js @@ -36,5 +36,11 @@ module.exports = { frame.response = { members: [data] }; + }, + + importCSV(data, apiConfig, frame) { + debug('importCSV'); + + frame.response = data; } }; diff --git a/core/server/api/v2/members.js b/core/server/api/v2/members.js index aceef0fd78..88392606dc 100644 --- a/core/server/api/v2/members.js +++ b/core/server/api/v2/members.js @@ -1,7 +1,9 @@ // NOTE: We must not cache references to membersService.api // as it is a getter and may change during runtime. +const Promise = require('bluebird'); const membersService = require('../../services/members'); const common = require('../../lib/common'); +const fsLib = require('../../lib/fs'); const members = { docName: 'members', @@ -58,13 +60,30 @@ const members = { } }, permissions: true, - async query(frame) { - const member = await membersService.api.members.create(frame.data.members[0], { - sendEmail: frame.options.send_email, - emailType: frame.options.email_type - }); + query(frame) { + // NOTE: Promise.resolve() is here for a reason! Method has to return an instance + // of a Bluebird promise to allow reflection. If decided to be replaced + // with something else, e.g: async/await, CSV export function + // would need a deep rewrite (see failing tests if this line is removed) + return Promise.resolve() + .then(() => { + return membersService.api.members.create(frame.data.members[0], { + sendEmail: frame.options.send_email, + emailType: frame.options.email_type + }); + }) + .then((member) => { + if (member) { + return Promise.resolve(member); + } + }) + .catch((error) => { + if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) { + return Promise.reject(new common.errors.ValidationError({message: common.i18n.t('errors.api.members.memberAlreadyExists')})); + } - return member; + return Promise.reject(error); + }); } }, @@ -107,6 +126,61 @@ const members = { await membersService.api.members.destroy(frame.options); return null; } + }, + + importCSV: { + statusCode: 201, + permissions: { + method: 'add' + }, + async query(frame) { + let filePath = frame.file.path, + fulfilled = 0, + invalid = 0, + duplicates = 0; + + return fsLib.readCSV({ + path: filePath, + columnsToExtract: [{name: 'email', lookup: /email/i}, {name: 'name', lookup: /name/i}] + }).then((result) => { + return Promise.all(result.map((entry) => { + const api = require('./index'); + + return api.members.add.query({ + data: { + members: [{ + email: entry.email, + name: entry.name + }] + }, + options: { + context: frame.options.context, + options: {send_email: false} + } + }).reflect(); + })).each((inspection) => { + if (inspection.isFulfilled()) { + fulfilled = fulfilled + 1; + } else { + if (inspection.reason() instanceof common.errors.ValidationError) { + duplicates = duplicates + 1; + } else { + invalid = invalid + 1; + } + } + }); + }).then(() => { + return { + meta: { + stats: { + imported: fulfilled, + duplicates: duplicates, + invalid: invalid + } + } + }; + }); + } } }; diff --git a/core/server/api/v2/utils/serializers/output/members.js b/core/server/api/v2/utils/serializers/output/members.js index a919815510..66a8c64ee2 100644 --- a/core/server/api/v2/utils/serializers/output/members.js +++ b/core/server/api/v2/utils/serializers/output/members.js @@ -36,5 +36,11 @@ module.exports = { frame.response = { members: [data] }; + }, + + importCSV(data, apiConfig, frame) { + debug('importCSV'); + + frame.response = data; } }; diff --git a/core/server/config/overrides.json b/core/server/config/overrides.json index bc24abc5e6..fee9a10278 100644 --- a/core/server/config/overrides.json +++ b/core/server/config/overrides.json @@ -32,6 +32,10 @@ "extensions": [".csv"], "contentTypes": ["text/csv", "application/csv", "application/octet-stream"] }, + "members": { + "extensions": [".csv"], + "contentTypes": ["text/csv", "application/csv", "application/octet-stream"] + }, "images": { "extensions": [".jpg", ".jpeg", ".gif", ".png", ".svg", ".svgz", ".ico"], "contentTypes": ["image/jpeg", "image/png", "image/gif", "image/svg+xml", "image/x-icon", "image/vnd.microsoft.icon"] diff --git a/core/server/web/api/canary/admin/routes.js b/core/server/web/api/canary/admin/routes.js index a2baefe843..f7f3738365 100644 --- a/core/server/web/api/canary/admin/routes.js +++ b/core/server/web/api/canary/admin/routes.js @@ -104,6 +104,15 @@ module.exports = function apiRoutes() { // ## Members router.get('/members', shared.middlewares.labs.members, mw.authAdminApi, http(apiCanary.members.browse)); router.post('/members', shared.middlewares.labs.members, mw.authAdminApi, http(apiCanary.members.add)); + + router.post('/members/csv', + shared.middlewares.labs.members, + mw.authAdminApi, + upload.single('membersfile'), + shared.middlewares.validation.upload({type: 'members'}), + http(apiCanary.members.importCSV) + ); + router.get('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiCanary.members.read)); router.put('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiCanary.members.edit)); router.del('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiCanary.members.destroy)); diff --git a/core/server/web/api/v2/admin/routes.js b/core/server/web/api/v2/admin/routes.js index 2b8917c9d8..1c9abf5b01 100644 --- a/core/server/web/api/v2/admin/routes.js +++ b/core/server/web/api/v2/admin/routes.js @@ -104,6 +104,15 @@ module.exports = function apiRoutes() { // ## Members router.get('/members', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.browse)); router.post('/members', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.add)); + + router.post('/members/csv', + shared.middlewares.labs.members, + mw.authAdminApi, + upload.single('membersfile'), + shared.middlewares.validation.upload({type: 'members'}), + http(apiv2.members.importCSV) + ); + router.get('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.read)); router.put('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.edit)); router.del('/members/:id', shared.middlewares.labs.members, mw.authAdminApi, http(apiv2.members.destroy)); diff --git a/core/test/regression/api/canary/admin/members_spec.js b/core/test/regression/api/canary/admin/members_spec.js index 4813e82a7e..9eb1b01b0e 100644 --- a/core/test/regression/api/canary/admin/members_spec.js +++ b/core/test/regression/api/canary/admin/members_spec.js @@ -96,6 +96,15 @@ describe('Members API', function () { jsonResponse.members[0].name.should.equal(member.name); jsonResponse.members[0].email.should.equal(member.email); + }) + .then(() => { + return request + .post(localUtils.API.getApiQuery(`members/`)) + .send({members: [member]}) + .set('Origin', config.get('url')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(422); }); }); @@ -221,10 +230,10 @@ describe('Members API', function () { }); }); - it.skip('Can import CSV', function () { + it('Can import CSV', function () { return request .post(localUtils.API.getApiQuery(`members/csv/`)) - .attach('membersfile', path.join(__dirname, '/../../../../utils/fixtures/csv/single-column-with-header.csv')) + .attach('membersfile', path.join(__dirname, '/../../../../utils/fixtures/csv/valid-members-import.csv')) .set('Origin', config.get('url')) .expect('Content-Type', /json/) .expect('Cache-Control', testUtils.cacheRules.private) diff --git a/core/test/regression/api/v2/admin/members_spec.js b/core/test/regression/api/v2/admin/members_spec.js index 4813e82a7e..9eb1b01b0e 100644 --- a/core/test/regression/api/v2/admin/members_spec.js +++ b/core/test/regression/api/v2/admin/members_spec.js @@ -96,6 +96,15 @@ describe('Members API', function () { jsonResponse.members[0].name.should.equal(member.name); jsonResponse.members[0].email.should.equal(member.email); + }) + .then(() => { + return request + .post(localUtils.API.getApiQuery(`members/`)) + .send({members: [member]}) + .set('Origin', config.get('url')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(422); }); }); @@ -221,10 +230,10 @@ describe('Members API', function () { }); }); - it.skip('Can import CSV', function () { + it('Can import CSV', function () { return request .post(localUtils.API.getApiQuery(`members/csv/`)) - .attach('membersfile', path.join(__dirname, '/../../../../utils/fixtures/csv/single-column-with-header.csv')) + .attach('membersfile', path.join(__dirname, '/../../../../utils/fixtures/csv/valid-members-import.csv')) .set('Origin', config.get('url')) .expect('Content-Type', /json/) .expect('Cache-Control', testUtils.cacheRules.private) diff --git a/core/test/utils/fixtures/csv/valid-members-import.csv b/core/test/utils/fixtures/csv/valid-members-import.csv new file mode 100644 index 0000000000..508da22aac --- /dev/null +++ b/core/test/utils/fixtures/csv/valid-members-import.csv @@ -0,0 +1,3 @@ +email,name +jbloggs@example.com,joe +test@example.com,test