Skip to content

Commit

Permalink
fix transitioning into invalid state
Browse files Browse the repository at this point in the history
remove events from error class
  • Loading branch information
arenoir committed Oct 14, 2015
1 parent 5e6820d commit 1b48190
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 89 deletions.
34 changes: 4 additions & 30 deletions packages/ember-data/lib/system/model/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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');
}
},

/**
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -321,8 +297,6 @@ export default Ember.ArrayProxy.extend(Ember.Evented, {
}, this);

this._super();

this.trigger('becameValid');
},

/**
Expand Down
1 change: 1 addition & 0 deletions packages/ember-data/lib/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,7 @@ InternalModel.prototype = {
}
}

this.send('becameInvalid');
this._saveWasRejected();
},

Expand Down
12 changes: 1 addition & 11 deletions packages/ember-data/lib/system/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,15 +470,13 @@ test("if a created record is marked as invalid by the server, it enters an error
yehuda.save().then(null, async(function(error) {
equal(get(yehuda, 'isValid'), false, "the record is invalid");
ok(get(yehuda, 'errors.name'), "The errors.name property exists");

set(yehuda, 'updatedAt', true);
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");
//shouldn't we use rollbackAttributes. or perhaps a rollbackAttribute method is needed.
//keeping track of changes this is fragile.
//equal(get(yehuda, 'isValid'), true, "the record is no longer invalid after changing");
equal(get(yehuda, 'hasDirtyAttributes'), true, "the record has outstanding changes");

equal(get(yehuda, 'isNew'), true, "precond - record is still new");

return yehuda.save();
Expand Down Expand Up @@ -662,9 +660,11 @@ test("if an updated record is marked as invalid by the server, it enters an erro

set(yehuda, 'updatedAt', true);
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");

//shouldn't we use rollbackAttributes. or perhaps a rollbackAttribute method is needed.
//keeping track of changes this is fragile.
//equal(get(yehuda, 'isValid'), true, "the record is no longer invalid after changing");
equal(get(yehuda, 'hasDirtyAttributes'), true, "the record has outstanding changes");

return yehuda.save();
Expand Down
12 changes: 9 additions & 3 deletions packages/ember-data/tests/integration/records/save-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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 });
}
Expand Down Expand Up @@ -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() {
Expand Down
43 changes: 5 additions & 38 deletions packages/ember-data/tests/unit/model/errors-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand All @@ -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');
Expand All @@ -68,40 +44,31 @@ 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');
errors.remove('firstName');
});

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();
Expand Down

0 comments on commit 1b48190

Please sign in to comment.