From 736111b58f3d091c8eae95af4602c2a5bacc2656 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 4 Feb 2016 15:33:33 +0000 Subject: [PATCH] validate nav items when clicking the `+` button, ignoring last item if blank --- .../app/controllers/settings/navigation.js | 22 ++++++++-- core/client/app/validators/nav-item.js | 22 ---------- .../controllers/settings/navigation-test.js | 41 +++++++++++++++---- .../tests/unit/validators/nav-item-test.js | 16 -------- 4 files changed, 52 insertions(+), 49 deletions(-) diff --git a/core/client/app/controllers/settings/navigation.js b/core/client/app/controllers/settings/navigation.js index 271d7bf05e..0e7a477f30 100644 --- a/core/client/app/controllers/settings/navigation.js +++ b/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')) { - // Add new blank navItem - navItems.addObject(NavItem.create({last: true})); + if (lastItem) { + lastItem.validate().then(() => { + // Add new blank navItem + navItems.addObject(NavItem.create({last: true})); + }); } }, diff --git a/core/client/app/validators/nav-item.js b/core/client/app/validators/nav-item.js index f8201459fe..081ab53ba1 100644 --- a/core/client/app/validators/nav-item.js +++ b/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; } }); diff --git a/core/client/tests/unit/controllers/settings/navigation-test.js b/core/client/tests/unit/controllers/settings/navigation-test.js index 49e91515b7..62bc90814e 100644 --- a/core/client/tests/unit/controllers/settings/navigation-test.js +++ b/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,14 +164,19 @@ describeModule( run(() => { ctrl.set('navigationItems', [NavItem.create({label: 'First', url: '/first', last: true})]); - expect(ctrl.get('navigationItems.length')).to.equal(1); - 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; }); + + 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 () { diff --git a/core/client/tests/unit/validators/nav-item-test.js b/core/client/tests/unit/validators/nav-item-test.js index 1e05a4c239..8ec664c2f0 100644 --- a/core/client/tests/unit/validators/nav-item-test.js +++ b/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();