From 5c7207b269243b8518d925c301e97a36b3498a4a Mon Sep 17 00:00:00 2001 From: Aaron Renoir Date: Tue, 13 Oct 2015 19:31:13 -0700 Subject: [PATCH] fix transitioning into invalid state remove events from error class add functionality to become valid if errors are removed --- .../ember-data/lib/system/model/errors.js | 34 ++------------- .../lib/system/model/internal-model.js | 8 ++++ packages/ember-data/lib/system/model/model.js | 12 +----- .../ember-data/lib/system/model/states.js | 8 ++++ .../integration/adapter/store-adapter-test.js | 2 +- .../tests/integration/records/save-test.js | 12 ++++-- .../tests/unit/model/errors-test.js | 43 +++---------------- 7 files changed, 36 insertions(+), 83 deletions(-) diff --git a/packages/ember-data/lib/system/model/errors.js b/packages/ember-data/lib/system/model/errors.js index 2c1ffda2d24..a2fa87f6f1b 100644 --- a/packages/ember-data/lib/system/model/errors.js +++ b/packages/ember-data/lib/system/model/errors.js @@ -91,19 +91,8 @@ var MapWithDefault = Ember.MapWithDefault; @uses Ember.Enumerable @uses Ember.Evented */ -export default Ember.ArrayProxy.extend(Ember.Evented, { - /** - Register with target handler +export default Ember.ArrayProxy.extend({ - @method registerHandlers - @param {Object} target - @param {Function} becameInvalid - @param {Function} becameValid - */ - registerHandlers: function(target, becameInvalid, becameValid) { - this.on('becameInvalid', target, becameInvalid); - this.on('becameValid', target, becameValid); - }, /** @property errorsByAttributeName @@ -192,8 +181,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { isEmpty: Ember.computed.not('length').readOnly(), /** - Adds error messages to a given attribute and sends - `becameInvalid` event to the record. + Adds error messages to a given attribute. Example: @@ -208,17 +196,11 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { @param {(Array|String)} messages */ add: function(attribute, messages) { - var wasEmpty = get(this, 'isEmpty'); - messages = this._findOrCreateMessages(attribute, messages); this.addObjects(messages); get(this, 'errorsByAttributeName').get(attribute).addObjects(messages); this.notifyPropertyChange(attribute); - - if (wasEmpty && !get(this, 'isEmpty')) { - this.trigger('becameInvalid'); - } }, /** @@ -237,8 +219,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { }, /** - Removes all error messages from the given attribute and sends - `becameValid` event to the record if there no more errors left. + Removes all error messages from the given attribute. Example: @@ -278,15 +259,10 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { get(this, 'errorsByAttributeName').delete(attribute); this.notifyPropertyChange(attribute); - - if (get(this, 'isEmpty')) { - this.trigger('becameValid'); - } }, /** - Removes all error messages and sends `becameValid` event - to the record. + Removes all error messages. Example: @@ -321,8 +297,6 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { }, this); this._super(); - - this.trigger('becameValid'); }, /** diff --git a/packages/ember-data/lib/system/model/internal-model.js b/packages/ember-data/lib/system/model/internal-model.js index ab31ff2032f..0e465985ab5 100644 --- a/packages/ember-data/lib/system/model/internal-model.js +++ b/packages/ember-data/lib/system/model/internal-model.js @@ -629,6 +629,13 @@ InternalModel.prototype = { get(record, 'errors').clear(); }, + hasErrors: function() { + var record = this.getRecord(); + var errors = get(record, 'errors'); + + return !Ember.isEmpty(errors); + }, + // FOR USE DURING COMMIT PROCESS /** @@ -644,6 +651,7 @@ InternalModel.prototype = { } } + this.send('becameInvalid'); this._saveWasRejected(); }, diff --git a/packages/ember-data/lib/system/model/model.js b/packages/ember-data/lib/system/model/model.js index 1438005ceb9..ba93f9e1e0d 100644 --- a/packages/ember-data/lib/system/model/model.js +++ b/packages/ember-data/lib/system/model/model.js @@ -352,17 +352,7 @@ var Model = Ember.Object.extend(Ember.Evented, { @type {DS.Errors} */ errors: Ember.computed(function() { - let errors = Errors.create(); - - errors.registerHandlers(this._internalModel, - function() { - this.send('becameInvalid'); - }, - function() { - this.send('becameValid'); - }); - - return errors; + return Errors.create(); }).readOnly(), /** diff --git a/packages/ember-data/lib/system/model/states.js b/packages/ember-data/lib/system/model/states.js index 7bd7ffd278e..e94ac0f3c8c 100644 --- a/packages/ember-data/lib/system/model/states.js +++ b/packages/ember-data/lib/system/model/states.js @@ -328,6 +328,10 @@ var DirtyState = { internalModel.removeErrorMessageFromAttribute(context.name); didSetProperty(internalModel, context); + + if (!internalModel.hasErrors()) { + this.becameValid(internalModel); + } }, becomeDirty: Ember.K, @@ -704,6 +708,10 @@ var RootState = { internalModel.removeErrorMessageFromAttribute(context.name); didSetProperty(internalModel, context); + + if (!internalModel.hasErrors()) { + this.becameValid(internalModel); + } }, deleteRecord: Ember.K, diff --git a/packages/ember-data/tests/integration/adapter/store-adapter-test.js b/packages/ember-data/tests/integration/adapter/store-adapter-test.js index 240a16241c0..203d0db7570 100644 --- a/packages/ember-data/tests/integration/adapter/store-adapter-test.js +++ b/packages/ember-data/tests/integration/adapter/store-adapter-test.js @@ -661,8 +661,8 @@ test("if an updated record is marked as invalid by the server, it enters an erro equal(get(yehuda, 'isValid'), false, "the record is invalid"); set(yehuda, 'updatedAt', true); - equal(get(yehuda, 'isValid'), false, "the record is still invalid"); + equal(get(yehuda, 'isValid'), false, "the record is still invalid"); set(yehuda, 'name', "Brohuda Brokatz"); equal(get(yehuda, 'isValid'), true, "the record is no longer invalid after changing"); equal(get(yehuda, 'hasDirtyAttributes'), true, "the record has outstanding changes"); diff --git a/packages/ember-data/tests/integration/records/save-test.js b/packages/ember-data/tests/integration/records/save-test.js index 10fc17fbd50..fece7fa2b76 100644 --- a/packages/ember-data/tests/integration/records/save-test.js +++ b/packages/ember-data/tests/integration/records/save-test.js @@ -51,7 +51,9 @@ test("Will reject save on error", function() { }); env.adapter.createRecord = function(store, type, snapshot) { - return Ember.RSVP.reject(); + var error = new DS.InvalidError([{ title: 'not valid' }]); + + return Ember.RSVP.reject(error); }; run(function() { @@ -70,8 +72,10 @@ test("Retry is allowed in a failure handler", function() { var count = 0; env.adapter.createRecord = function(store, type, snapshot) { + var error = new DS.InvalidError([{ title: 'not valid' }]); + if (count++ === 0) { - return Ember.RSVP.reject(); + return Ember.RSVP.reject(error); } else { return Ember.RSVP.resolve({ id: 123 }); } @@ -117,7 +121,9 @@ test("Will reject save on invalid", function() { }); env.adapter.createRecord = function(store, type, snapshot) { - return Ember.RSVP.reject({ title: 'invalid' }); + var error = new DS.InvalidError([{ title: 'not valid' }]); + + return Ember.RSVP.reject(error); }; run(function() { diff --git a/packages/ember-data/tests/unit/model/errors-test.js b/packages/ember-data/tests/unit/model/errors-test.js index 4d41a309e82..1f9b28681e1 100644 --- a/packages/ember-data/tests/unit/model/errors-test.js +++ b/packages/ember-data/tests/unit/model/errors-test.js @@ -9,31 +9,9 @@ module("unit/model/errors", { } }); -function becameInvalid(eventName) { - if (eventName === 'becameInvalid') { - ok(true, 'becameInvalid send'); - } else { - ok(false, eventName + ' is send instead of becameInvalid'); - } -} - -function becameValid(eventName) { - if (eventName === 'becameValid') { - ok(true, 'becameValid send'); - } else { - ok(false, eventName + ' is send instead of becameValid'); - } -} - -function unexpectedSend(eventName) { - ok(false, 'unexpected send : ' + eventName); -} - test("add error", function() { - expect(6); - errors.trigger = becameInvalid; + expect(5); errors.add('firstName', 'error'); - errors.trigger = unexpectedSend; ok(errors.has('firstName'), 'it has firstName errors'); equal(errors.get('length'), 1, 'it has 1 error'); errors.add('firstName', ['error1', 'error2']); @@ -45,11 +23,9 @@ test("add error", function() { }); test("get error", function() { - expect(8); + expect(7); ok(errors.get('firstObject') === undefined, 'returns undefined'); - errors.trigger = becameInvalid; errors.add('firstName', 'error'); - errors.trigger = unexpectedSend; ok(errors.get('firstName').length === 1, 'returns errors'); deepEqual(errors.get('firstObject'), { attribute: 'firstName', message: 'error' }); errors.add('firstName', 'error2'); @@ -68,12 +44,9 @@ test("get error", function() { }); test("remove error", function() { - expect(5); - errors.trigger = becameInvalid; + expect(3); errors.add('firstName', 'error'); - errors.trigger = becameValid; errors.remove('firstName'); - errors.trigger = unexpectedSend; ok(!errors.has('firstName'), 'it has no firstName errors'); equal(errors.get('length'), 0, 'it has 0 error'); ok(errors.get('isEmpty'), 'it is empty'); @@ -81,27 +54,21 @@ test("remove error", function() { }); test("remove same errors from different attributes", function() { - expect(5); - errors.trigger = becameInvalid; + expect(3); errors.add('firstName', 'error'); errors.add('lastName', 'error'); - errors.trigger = unexpectedSend; equal(errors.get('length'), 2, 'it has 2 error'); errors.remove('firstName'); equal(errors.get('length'), 1, 'it has 1 error'); - errors.trigger = becameValid; errors.remove('lastName'); ok(errors.get('isEmpty'), 'it is empty'); }); test("clear errors", function() { - expect(5); - errors.trigger = becameInvalid; + expect(3); errors.add('firstName', ['error', 'error1']); equal(errors.get('length'), 2, 'it has 2 errors'); - errors.trigger = becameValid; errors.clear(); - errors.trigger = unexpectedSend; ok(!errors.has('firstName'), 'it has no firstName errors'); equal(errors.get('length'), 0, 'it has 0 error'); errors.clear();