Return better errors to help downstream clients

* Return 409 Conflict if the role already exists
* Return 422 Unprocessable if there is a validation error.

Author: patch0

memberships/errors.js

@@ -9,7 +9,7 @@ class MembershipExists extends Error {
   constructor() {
     super();
     this.message = 'Membership already has this role';
-    this.status = 400;
+    this.status = 409;
   }
 }
 module.exports = {

test/integration/club.createMembership.spec.js

@@ -45,7 +45,7 @@ describe('integration:club:createMembership', () => {
         userType: 'mentor',
       })
       .set('Accept', 'application/json')
-      .expect(400);
+      .expect(409);
   });
   it('should return 404 on non-existing club', async () => {
     await request(app)

test/integration/lead.list.spec.js

@@ -28,11 +28,11 @@ describe('integration:leads:list', () => {
     expect(res.body.total).to.equal(0);
   });
 
-  it('should return 400 on wrong params', async () => {
+  it('should return 422 on wrong params', async () => {
     await request(app)
       .get('/leads?userId=123')
       .set('Accept', 'application/json')
-      .expect(400);
+      .expect(422);
   });
 });
 

test/integration/membership.update.spec.js

@@ -21,14 +21,14 @@ describe('integration:membership:update', () => {
     expect(res.body.userTypes).to.eql(['parent-guardian', 'mentor']);
     expect(res.body.userPermissions).to.eql([{ title: 'Ticketing Admin', name: 'ticketing-admin' }]);
   });
-  it('should return 400 when the role already exists', async () => {
+  it('should return 409 when the role already exists', async () => {
     await request(app)
       .post('/memberships/463e716d-e2c6-42f6-9809-dc6236f0e480/roles')
       .send({
         userType: 'mentor',
       })
       .set('Accept', 'application/json')
-      .expect(400);
+      .expect(409);
   });
   it('should not have duplicate permissions', async () => {
     const res = await request(app)

test/unit/memberships/controller/create.spec.js

@@ -42,7 +42,7 @@ describe('memberships/controller:create', () => {
         await memberController.create('u1', 'd1', 'champion');
       } catch (e) {
         expect(constr).to.not.have.been.called;
-        expect(e.status).to.equal(400);
+        expect(e.status).to.equal(409);
       }
     });
   });

test/unit/memberships/controller/createRole.spec.js

@@ -42,7 +42,7 @@ describe('memberships/controller:update', () => {
         expect(queryBuilder.findOne).to.have.been.calledOnce.and
           .calledWith({ id: 'u1' });
         expect(queryBuilder.update).to.not.have.been.called;
-        expect(e.status).to.equal(400);
+        expect(e.status).to.equal(409);
       }
     });
   });

util/ValidationHelper.js

@@ -6,7 +6,7 @@ class ValidationHelper {
     const errors = validationResult(req);
     if (!errors.isEmpty()) {
       logger.error(errors.mapped());
-      return res.status(400).json({ errors: errors.mapped() });
+      return res.status(422).json({ errors: errors.mapped() });
     }
     return next();
   }