From e256db29a580765af51ef6efc2f26e84638fad77 Mon Sep 17 00:00:00 2001
From: ivaosthu <ivar.conradi.osthus@finn.no>
Date: Wed, 2 Jan 2019 12:38:58 +0100
Subject: [PATCH] fix(http-status): Client errors should use 400 status codes

---
 lib/error/name-exists-error.js          | 13 +++++++++++++
 lib/routes/admin-api/feature.test.js    | 11 +++++++----
 lib/routes/admin-api/strategy.test.js   |  2 +-
 lib/routes/admin-api/util.js            |  4 ----
 test/e2e/api/admin/feature.e2e.test.js  |  6 +++---
 test/e2e/api/admin/strategy.e2e.test.js |  2 +-
 6 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/lib/error/name-exists-error.js b/lib/error/name-exists-error.js
index 644e054cef..bfe1ad16cc 100644
--- a/lib/error/name-exists-error.js
+++ b/lib/error/name-exists-error.js
@@ -8,6 +8,19 @@ class NameExistsError extends Error {
         this.name = this.constructor.name;
         this.message = message;
     }
+
+    toJSON() {
+        const obj = {
+            isJoi: true,
+            name: this.constructor.name,
+            details: [
+                {
+                    message: this.message,
+                },
+            ],
+        };
+        return obj;
+    }
 }
 
 module.exports = NameExistsError;
diff --git a/lib/routes/admin-api/feature.test.js b/lib/routes/admin-api/feature.test.js
index 70f4f6d5e6..5f380260fb 100644
--- a/lib/routes/admin-api/feature.test.js
+++ b/lib/routes/admin-api/feature.test.js
@@ -104,9 +104,12 @@ test('should not be allowed to reuse active toggle name', t => {
         .post(`${base}/api/admin/features/validate`)
         .send({ name: 'ts' })
         .set('Content-Type', 'application/json')
-        .expect(403)
+        .expect(400)
         .expect(res => {
-            t.true(res.body[0].msg === 'A toggle with that name already exist');
+            t.true(
+                res.body.details[0].message ===
+                    'A toggle with that name already exist'
+            );
         });
 });
 
@@ -122,10 +125,10 @@ test('should not be allowed to reuse archived toggle name', t => {
         .post(`${base}/api/admin/features/validate`)
         .send({ name: 'ts.archived' })
         .set('Content-Type', 'application/json')
-        .expect(403)
+        .expect(400)
         .expect(res => {
             t.true(
-                res.body[0].msg ===
+                res.body.details[0].message ===
                     'An archived toggle with that name already exist'
             );
         });
diff --git a/lib/routes/admin-api/strategy.test.js b/lib/routes/admin-api/strategy.test.js
index ff9d4c3718..c23996d99a 100644
--- a/lib/routes/admin-api/strategy.test.js
+++ b/lib/routes/admin-api/strategy.test.js
@@ -81,7 +81,7 @@ test('not be possible to override name', t => {
     return request
         .post(`${base}/api/admin/strategies`)
         .send({ name: 'Testing', parameters: [] })
-        .expect(403);
+        .expect(400);
 });
 
 test('update strategy', t => {
diff --git a/lib/routes/admin-api/util.js b/lib/routes/admin-api/util.js
index 0ff2366952..c1a5c31b16 100644
--- a/lib/routes/admin-api/util.js
+++ b/lib/routes/admin-api/util.js
@@ -42,10 +42,6 @@ const handleErrors = (res, error) => {
         case 'NotFoundError':
             return res.status(404).end();
         case 'NameExistsError':
-            return res
-                .status(403)
-                .json([{ msg: error.message }])
-                .end();
         case 'ValidationError':
             return res
                 .status(400)
diff --git a/test/e2e/api/admin/feature.e2e.test.js b/test/e2e/api/admin/feature.e2e.test.js
index c6a5324837..e7d11d0cd5 100644
--- a/test/e2e/api/admin/feature.e2e.test.js
+++ b/test/e2e/api/admin/feature.e2e.test.js
@@ -151,7 +151,7 @@ test.serial('refuses to create a feature with an existing name', async t => {
         .post('/api/admin/features')
         .send({ name: 'featureX' })
         .set('Content-Type', 'application/json')
-        .expect(403)
+        .expect(400)
         .then(destroy);
 });
 
@@ -162,7 +162,7 @@ test.serial('refuses to validate a feature with an existing name', async t => {
         .post('/api/admin/features/validate')
         .send({ name: 'featureX' })
         .set('Content-Type', 'application/json')
-        .expect(403)
+        .expect(400)
         .then(destroy);
 });
 
@@ -201,6 +201,6 @@ test.serial('should not be possible to create archived toggle', async t => {
             strategies: [{ name: 'default' }],
         })
         .set('Content-Type', 'application/json')
-        .expect(403)
+        .expect(400)
         .then(destroy);
 });
diff --git a/test/e2e/api/admin/strategy.e2e.test.js b/test/e2e/api/admin/strategy.e2e.test.js
index dab596e61c..8628ae1d2c 100644
--- a/test/e2e/api/admin/strategy.e2e.test.js
+++ b/test/e2e/api/admin/strategy.e2e.test.js
@@ -72,7 +72,7 @@ test.serial('refuses to create a strategy with an existing name', async t => {
         .post('/api/admin/strategies')
         .send({ name: 'default', parameters: [] })
         .set('Content-Type', 'application/json')
-        .expect(403)
+        .expect(400)
         .then(destroy);
 });