Added members CSV import to Admin API (#11197)

no issue

- Improved error handling for member creation. We should be returning 422s instead of 500 when possible
- Wrapped `members.add` method with Bluebird promise. Wrapping is needed to be able to use `.reflect()` in CSV export method
- Added proper members CSV fixture

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

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

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

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

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"]

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

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

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)

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)

core/test/utils/fixtures/csv/valid-members-import.csv

@@ -0,0 +1,3 @@
+email,name
+jbloggs@example.com,joe
+test@example.com,test