Bugfix: more informative name validation errors

Will know tell the user if the toggle name is already
in use by an active feature toggle or an archived toggle.

Also brings lates unleash-frontend fix, which prevents an
invalid form from submitting.

closes: #290, #291

lib/db/feature-toggle-store.js

@@ -51,12 +51,20 @@ class FeatureToggleStore {
             .then(this.rowToFeature);
     }
 
-    hasFeatureName(name) {
+    hasFeature(name) {
         return this.db
-            .first('name')
+            .first('name', 'archived')
             .from(TABLE)
             .where({ name })
-            .then(n => !!n);
+            .then(row => {
+                if (!row) {
+                    throw new NotFoundError('No feature toggle found');
+                }
+                return {
+                    name: row.name,
+                    archived: row.archived === 1,
+                };
+            });
     }
 
     getArchivedFeatures() {

lib/routes/admin-api/feature.js

@@ -23,12 +23,7 @@ const handleErrors = (req, res, error) => {
         case NameExistsError:
             return res
                 .status(403)
-                .json([
-                    {
-                        msg:
-                            'A feature with this name already exists. Try re-activating it from the archive.',
-                    },
-                ])
+                .json([{ msg: error.message }])
                 .end();
         case ValidationError:
             return res
@@ -97,13 +92,15 @@ module.exports.router = function(config) {
 
     function validateUniqueName(req) {
         return new Promise((resolve, reject) => {
-            featureToggleStore.hasFeatureName(req.body.name).then(hasName => {
-                if (hasName) {
-                    reject(new NameExistsError('Feature name already exist'));
-                } else {
-                    resolve(req);
-                }
-            });
+            featureToggleStore
+                .hasFeature(req.body.name)
+                .then(definition => {
+                    const msg = definition.archived
+                        ? 'An archived toggle with that name already exist'
+                        : 'A toggle with that name already exist';
+                    reject(new NameExistsError(msg));
+                })
+                .catch(() => resolve(req));
         });
     }
 

lib/routes/admin-api/feature.test.js

@@ -81,6 +81,56 @@ test('should require at least one strategy when creating a feature toggle', t =>
         .expect(400);
 });
 
+test('should be allowed to use new toggle name', t => {
+    t.plan(0);
+    const { request, base } = getSetup();
+
+    return request
+        .post(`${base}/api/admin/features/validate`)
+        .send({ name: 'new.name' })
+        .set('Content-Type', 'application/json')
+        .expect(201);
+});
+
+test('should not be allowed to reuse active toggle name', t => {
+    t.plan(1);
+    const { request, featureToggleStore, base } = getSetup();
+    featureToggleStore.addFeature({
+        name: 'ts',
+        strategies: [{ name: 'default' }],
+    });
+
+    return request
+        .post(`${base}/api/admin/features/validate`)
+        .send({ name: 'ts' })
+        .set('Content-Type', 'application/json')
+        .expect(403)
+        .expect(res => {
+            t.true(res.body[0].msg === 'A toggle with that name already exist');
+        });
+});
+
+test('should not be allowed to reuse archived toggle name', t => {
+    t.plan(1);
+    const { request, featureToggleStore, base } = getSetup();
+    featureToggleStore.addArchivedFeature({
+        name: 'ts.archived',
+        strategies: [{ name: 'default' }],
+    });
+
+    return request
+        .post(`${base}/api/admin/features/validate`)
+        .send({ name: 'ts.archived' })
+        .set('Content-Type', 'application/json')
+        .expect(403)
+        .expect(res => {
+            t.true(
+                res.body[0].msg ===
+                    'An archived toggle with that name already exist'
+            );
+        });
+});
+
 test('should require at least one strategy when updating a feature toggle', t => {
     t.plan(0);
     const { request, featureToggleStore, base } = getSetup();

test/fixtures/fake-feature-toggle-store.js

@@ -12,8 +12,18 @@ module.exports = () => {
                 return Promise.reject();
             }
         },
+        hasFeature: name => {
+            const toggle = _features.find(f => f.name === name);
+            const archived = _archive.find(f => f.name === name);
+            if (toggle) {
+                return Promise.resolve({ name, archived: false });
+            } else if (archived) {
+                return Promise.resolve({ name, archived: true });
+            } else {
+                return Promise.reject();
+            }
+        },
         getFeatures: () => Promise.resolve(_features),
-        hasFeatureName: () => Promise.resolve(false),
         addFeature: feature => _features.push(feature),
         getArchivedFeatures: () => Promise.resolve(_archive),
         addArchivedFeature: feature => _archive.push(feature),