From 295f2cce963689aaf5253a68d6de10ee2ffae5be Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Tue, 31 Jan 2017 19:27:11 +0000 Subject: [PATCH] fix double-render issues with validation-state mixin --- ghost/admin/app/mixins/validation-state.js | 29 +++++++++++----- .../integration/components/gh-navitem-test.js | 13 +++++--- .../components/gh-tag-settings-form-test.js | 31 +++++++++-------- .../gh-validation-status-container-test.js | 33 +++++++++++++------ 4 files changed, 68 insertions(+), 38 deletions(-) diff --git a/ghost/admin/app/mixins/validation-state.js b/ghost/admin/app/mixins/validation-state.js index 6ac885dded..508ff97f62 100644 --- a/ghost/admin/app/mixins/validation-state.js +++ b/ghost/admin/app/mixins/validation-state.js @@ -1,7 +1,8 @@ import Mixin from 'ember-metal/mixin'; -import computed from 'ember-computed'; import {isEmpty} from 'ember-utils'; import {A as emberA} from 'ember-array/utils'; +import observer from 'ember-metal/observer'; +import run from 'ember-runloop'; export default Mixin.create({ @@ -9,26 +10,36 @@ export default Mixin.create({ property: '', hasValidated: emberA(), - hasError: computed('errors.[]', 'property', 'hasValidated.[]', function () { + hasError: false, + + setHasError() { let property = this.get('property'); let errors = this.get('errors'); let hasValidated = this.get('hasValidated'); // if we aren't looking at a specific property we always want an error class - if (!property && !isEmpty(errors)) { - return true; + if (!property && errors && !errors.get('isEmpty')) { + this.set('hasError', true); + return; } // If we haven't yet validated this field, there is no validation class needed if (!hasValidated || !hasValidated.includes(property)) { - return false; + this.set('hasError', false); + return; } - if (errors) { - return errors.get(property); + if (errors && !isEmpty(errors.errorsFor(property))) { + this.set('hasError', true); + return; } - return false; - }) + this.set('hasError', false); + }, + + hasErrorObserver: observer('errors.[]', 'property', 'hasValidated.[]', function () { + run.once(this, 'setHasError'); + // this.setHasError(); + }).on('init') }); diff --git a/ghost/admin/tests/integration/components/gh-navitem-test.js b/ghost/admin/tests/integration/components/gh-navitem-test.js index 6b945c4841..74c5ac2d94 100644 --- a/ghost/admin/tests/integration/components/gh-navitem-test.js +++ b/ghost/admin/tests/integration/components/gh-navitem-test.js @@ -4,6 +4,7 @@ import {describe, it} from 'mocha'; import {setupComponentTest} from 'ember-mocha'; import hbs from 'htmlbars-inline-precompile'; import NavItem from 'ghost-admin/models/navigation-item'; +import wait from 'ember-test-helpers/wait'; describe('Integration: Component: gh-navitem', function () { setupComponentTest('gh-navitem', { @@ -100,10 +101,12 @@ describe('Integration: Component: gh-navitem', function () { this.render(hbs`{{gh-navitem navItem=navItem baseUrl=baseUrl}}`); let $item = this.$('.gh-blognav-item'); - expect($item.hasClass('gh-blognav-item--error')).to.be.true; - expect($item.find('.gh-blognav-label').hasClass('error')).to.be.true; - expect($item.find('.gh-blognav-label .response').text().trim()).to.equal('You must specify a label'); - expect($item.find('.gh-blognav-url').hasClass('error')).to.be.true; - expect($item.find('.gh-blognav-url .response').text().trim()).to.equal('You must specify a URL or relative path'); + return wait().then(() => { + expect($item.hasClass('gh-blognav-item--error')).to.be.true; + expect($item.find('.gh-blognav-label').hasClass('error')).to.be.true; + expect($item.find('.gh-blognav-label .response').text().trim()).to.equal('You must specify a label'); + expect($item.find('.gh-blognav-url').hasClass('error')).to.be.true; + expect($item.find('.gh-blognav-url .response').text().trim()).to.equal('You must specify a URL or relative path'); + }); }); }); diff --git a/ghost/admin/tests/integration/components/gh-tag-settings-form-test.js b/ghost/admin/tests/integration/components/gh-tag-settings-form-test.js index 50ca6643e8..14c4a4c3ec 100644 --- a/ghost/admin/tests/integration/components/gh-tag-settings-form-test.js +++ b/ghost/admin/tests/integration/components/gh-tag-settings-form-test.js @@ -7,6 +7,7 @@ import Service from 'ember-service'; import EmberObject from 'ember-object'; import run from 'ember-runloop'; import DS from 'ember-data'; +import wait from 'ember-test-helpers/wait'; const {Errors} = DS; @@ -195,24 +196,26 @@ describe('Integration: Component: gh-tag-settings-form', function () { {{gh-tag-settings-form tag=tag setProperty=(action 'setProperty')}} `); - let nameFormGroup = this.$('input[name="name"]').closest('.form-group'); - expect(nameFormGroup.hasClass('error'), 'name form group has error state').to.be.true; - expect(nameFormGroup.find('.response').length, 'name form group has error message').to.equal(1); + return wait().then(() => { + let nameFormGroup = this.$('input[name="name"]').closest('.form-group'); + expect(nameFormGroup.hasClass('error'), 'name form group has error state').to.be.true; + expect(nameFormGroup.find('.response').length, 'name form group has error message').to.equal(1); - let slugFormGroup = this.$('input[name="slug"]').closest('.form-group'); - expect(slugFormGroup.hasClass('error'), 'slug form group has error state').to.be.true; - expect(slugFormGroup.find('.response').length, 'slug form group has error message').to.equal(1); + let slugFormGroup = this.$('input[name="slug"]').closest('.form-group'); + expect(slugFormGroup.hasClass('error'), 'slug form group has error state').to.be.true; + expect(slugFormGroup.find('.response').length, 'slug form group has error message').to.equal(1); - let descriptionFormGroup = this.$('textarea[name="description"]').closest('.form-group'); - expect(descriptionFormGroup.hasClass('error'), 'description form group has error state').to.be.true; + let descriptionFormGroup = this.$('textarea[name="description"]').closest('.form-group'); + expect(descriptionFormGroup.hasClass('error'), 'description form group has error state').to.be.true; - let metaTitleFormGroup = this.$('input[name="metaTitle"]').closest('.form-group'); - expect(metaTitleFormGroup.hasClass('error'), 'metaTitle form group has error state').to.be.true; - expect(metaTitleFormGroup.find('.response').length, 'metaTitle form group has error message').to.equal(1); + let metaTitleFormGroup = this.$('input[name="metaTitle"]').closest('.form-group'); + expect(metaTitleFormGroup.hasClass('error'), 'metaTitle form group has error state').to.be.true; + expect(metaTitleFormGroup.find('.response').length, 'metaTitle form group has error message').to.equal(1); - let metaDescriptionFormGroup = this.$('textarea[name="metaDescription"]').closest('.form-group'); - expect(metaDescriptionFormGroup.hasClass('error'), 'metaDescription form group has error state').to.be.true; - expect(metaDescriptionFormGroup.find('.response').length, 'metaDescription form group has error message').to.equal(1); + let metaDescriptionFormGroup = this.$('textarea[name="metaDescription"]').closest('.form-group'); + expect(metaDescriptionFormGroup.hasClass('error'), 'metaDescription form group has error state').to.be.true; + expect(metaDescriptionFormGroup.find('.response').length, 'metaDescription form group has error message').to.equal(1); + }); }); it('displays char count for text fields', function () { diff --git a/ghost/admin/tests/integration/components/gh-validation-status-container-test.js b/ghost/admin/tests/integration/components/gh-validation-status-container-test.js index 8b5f8e0c9b..d2c7442348 100644 --- a/ghost/admin/tests/integration/components/gh-validation-status-container-test.js +++ b/ghost/admin/tests/integration/components/gh-validation-status-container-test.js @@ -5,6 +5,7 @@ import {setupComponentTest} from 'ember-mocha'; import hbs from 'htmlbars-inline-precompile'; import EmberObject from 'ember-object'; import DS from 'ember-data'; +import wait from 'ember-test-helpers/wait'; const {Errors} = DS; @@ -27,9 +28,12 @@ describe('Integration: Component: gh-validation-status-container', function () { {{#gh-validation-status-container class="gh-test" property="name" errors=testObject.errors hasValidated=testObject.hasValidated}} {{/gh-validation-status-container}} `); - expect(this.$('.gh-test')).to.have.length(1); - expect(this.$('.gh-test').hasClass('success')).to.be.false; - expect(this.$('.gh-test').hasClass('error')).to.be.false; + + return wait().then(() => { + expect(this.$('.gh-test')).to.have.length(1); + expect(this.$('.gh-test').hasClass('success')).to.be.false; + expect(this.$('.gh-test').hasClass('error')).to.be.false; + }); }); it('has success class when valid', function () { @@ -39,9 +43,12 @@ describe('Integration: Component: gh-validation-status-container', function () { {{#gh-validation-status-container class="gh-test" property="name" errors=testObject.errors hasValidated=testObject.hasValidated}} {{/gh-validation-status-container}} `); - expect(this.$('.gh-test')).to.have.length(1); - expect(this.$('.gh-test').hasClass('success')).to.be.true; - expect(this.$('.gh-test').hasClass('error')).to.be.false; + + return wait().then(() => { + expect(this.$('.gh-test')).to.have.length(1); + expect(this.$('.gh-test').hasClass('success')).to.be.true; + expect(this.$('.gh-test').hasClass('error')).to.be.false; + }); }); it('has error class when invalid', function () { @@ -52,9 +59,12 @@ describe('Integration: Component: gh-validation-status-container', function () { {{#gh-validation-status-container class="gh-test" property="name" errors=testObject.errors hasValidated=testObject.hasValidated}} {{/gh-validation-status-container}} `); - expect(this.$('.gh-test')).to.have.length(1); - expect(this.$('.gh-test').hasClass('success')).to.be.false; - expect(this.$('.gh-test').hasClass('error')).to.be.true; + + return wait().then(() => { + expect(this.$('.gh-test')).to.have.length(1); + expect(this.$('.gh-test').hasClass('success')).to.be.false; + expect(this.$('.gh-test').hasClass('error')).to.be.true; + }); }); it('still renders if hasValidated is undefined', function () { @@ -64,6 +74,9 @@ describe('Integration: Component: gh-validation-status-container', function () { {{#gh-validation-status-container class="gh-test" property="name" errors=testObject.errors hasValidated=testObject.hasValidated}} {{/gh-validation-status-container}} `); - expect(this.$('.gh-test')).to.have.length(1); + + return wait().then(() => { + expect(this.$('.gh-test')).to.have.length(1); + }); }); });