validate nav items when clicking the + button, ignoring last item if blank

core/client/app/controllers/settings/navigation.js

@@ -22,7 +22,15 @@ export const NavItem = Ember.Object.extend(ValidationEngine, {
     validationType: 'navItem',
 
     isComplete: computed('label', 'url', function () {
-        return !(isBlank(this.get('label').trim()) || isBlank(this.get('url')));
+        let {label, url} = this.getProperties('label', 'url');
+
+        return !isBlank(label) && !isBlank(url);
+    }),
+
+    isBlank: computed('label', 'url', function () {
+        let {label, url} = this.getProperties('label', 'url');
+
+        return isBlank(label) && isBlank(url);
     }),
 
     init() {
@@ -83,6 +91,10 @@ export default Controller.extend(SettingsSaveMixin, {
             validationPromises;
 
         validationPromises = navItems.map((item) => {
+            if (item.get('last') && item.get('isBlank')) {
+                return;
+            }
+
             return item.validate();
         });
 
@@ -117,9 +129,11 @@ export default Controller.extend(SettingsSaveMixin, {
             let navItems = this.get('navigationItems');
             let lastItem = navItems.get('lastObject');
 
-            if (lastItem && lastItem.get('isComplete')) {
+            if (lastItem) {
+                lastItem.validate().then(() => {
                     // Add new blank navItem
                     navItems.addObject(NavItem.create({last: true}));
+                });
             }
         },
 

core/client/app/validators/nav-item.js

@@ -7,10 +7,6 @@ export default BaseValidator.create({
         let label = model.get('label');
         let hasValidated = model.get('hasValidated');
 
-        if (this.canBeIgnored(model)) {
-            return;
-        }
-
         if (validator.empty(label)) {
             model.get('errors').add('label', 'You must specify a label');
             this.invalidate();
@@ -27,10 +23,6 @@ export default BaseValidator.create({
         /* jscs:enable requireCamelCaseOrUpperCaseIdentifiers */
         let urlRegex = new RegExp(/^(\/|#|[a-zA-Z0-9\-]+:)/);
 
-        if (this.canBeIgnored(model)) {
-            return;
-        }
-
         if (validator.empty(url)) {
             model.get('errors').add('url', 'You must specify a URL or relative path');
             this.invalidate();
@@ -40,19 +32,5 @@ export default BaseValidator.create({
         }
 
         hasValidated.addObject('url');
-    },
-
-    canBeIgnored(model) {
-        let label = model.get('label');
-        let url = model.get('url');
-        let isLast = model.get('last');
-
-        // if nav item is last and completely blank, mark it valid and skip
-        if (isLast && (validator.empty(url) || url === '/') && validator.empty(label)) {
-            model.get('errors').clear();
-            return true;
-        }
-
-        return false;
     }
 });

core/client/tests/unit/controllers/settings/navigation-test.js

@@ -108,6 +108,28 @@ describeModule(
             });
         });
 
+        it('save: ignores blank last item when saving', function (done) {
+            let ctrl = this.subject();
+
+            run(() => {
+                ctrl.set('model', Ember.Object.create({navigation: `[
+                    {"label":"First",   "url":"/"},
+                    {"label":"",        "url":""}
+                ]`}));
+
+                expect(ctrl.get('navigationItems.length')).to.equal(2);
+
+                ctrl.save().then(function passedValidation() {
+                    assert(false, 'navigationItems weren\'t validated on save');
+                    done();
+                }).catch(function failedValidation() {
+                    let navItems = ctrl.get('navigationItems');
+                    expect(navItems[0].get('errors').toArray()).to.be.empty;
+                    done();
+                });
+            });
+        });
+
         it('save: generates new navigation JSON', function (done) {
             let ctrl = this.subject();
             let model = Ember.Object.create({navigation: {}});
@@ -142,15 +164,20 @@ describeModule(
 
             run(() => {
                 ctrl.set('navigationItems', [NavItem.create({label: 'First', url: '/first', last: true})]);
+            });
+
             expect(ctrl.get('navigationItems.length')).to.equal(1);
+
+            run(() => {
                 ctrl.send('addItem');
+            });
+
             expect(ctrl.get('navigationItems.length')).to.equal(2);
             expect(ctrl.get('navigationItems.firstObject.last')).to.be.false;
             expect(ctrl.get('navigationItems.lastObject.label')).to.equal('');
             expect(ctrl.get('navigationItems.lastObject.url')).to.equal('');
             expect(ctrl.get('navigationItems.lastObject.last')).to.be.true;
         });
-        });
 
         it('action - addItem: doesn\'t insert new item if last object is incomplete', function () {
             let ctrl = this.subject();

core/client/tests/unit/validators/nav-item-test.js

@@ -43,14 +43,6 @@ describe('Unit: Validator: nav-item', function () {
         expect(navItem.get('hasValidated')).to.include('label');
     });
 
-    it('doesn\'t validate label if empty and last', function () {
-        let navItem = NavItem.create({last: true});
-
-        validator.check(navItem, 'label');
-
-        expect(validator.get('passed')).to.be.true;
-    });
-
     it('requires url presence', function () {
         let navItem = NavItem.create();
 
@@ -97,14 +89,6 @@ describe('Unit: Validator: nav-item', function () {
         });
     });
 
-    it('doesn\'t validate url if empty and last', function () {
-        let navItem = NavItem.create({last: true});
-
-        validator.check(navItem, 'url');
-
-        expect(validator.get('passed')).to.be.true;
-    });
-
     it('validates url and label by default', function () {
         let navItem = NavItem.create();