Skip to content

Commit

Permalink
Log an assertion if the response from createRecord does not have an i…
Browse files Browse the repository at this point in the history
…d and the id was not already provided by the client

Closes emberjs#4346
  • Loading branch information
bmac authored and runspired committed Oct 17, 2016
1 parent f3ab488 commit 7d7017c
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 8 deletions.
2 changes: 1 addition & 1 deletion addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -1756,7 +1756,7 @@ Store = Service.extend({
this._backburner.schedule('normalizeRelationships', this, '_setupRelationships', internalModel, data);
this.updateId(internalModel, data);
}

assert(`Your ${internalModel.type.modelName} record was saved but it does not have an id. Please make the server provides an id in the createRecord response or you are setting the on the client side before saving the record.`, internalModel.id !== null);
//We first make sure the primary data has been updated
//TODO try to move notification to the user to the end of the runloop
internalModel.adapterDidCommit(data);
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/adapter/rest-adapter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,14 @@ test("createRecord - a serializer's primary key and attributes are consulted whe
test("createRecord - a serializer's attributes are consulted when building the payload if no id is pre-defined", function(assert) {
var post;
env.registry.register('serializer:post', DS.RESTSerializer.extend({
primarykey: '_id_',

attrs: {
name: '_name_'
}
}));

ajaxResponse();
ajaxResponse({
post: { '_name_': "The Parley Letter", id: '1' }
});

run(function() {
post = store.createRecord('post', { name: "The Parley Letter" });
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/adapter/store-adapter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,7 @@ test("createRecord receives a snapshot", function(assert) {
var person;

run(function() {
person = store.createRecord('person', { name: "Tom Dale" });
person = store.createRecord('person', { name: "Tom Dale", id: 1 });
person.save();
});
});
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/client-id-generation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ test("empty string and undefined ids should coerce to null", function(assert) {

env.adapter.createRecord = function(store, type, record) {
assert.equal(typeof get(record, 'id'), 'object', 'correct type');
return Ember.RSVP.resolve();
return Ember.RSVP.resolve({ id: 1 });
};

run(function() {
Expand Down
5 changes: 4 additions & 1 deletion tests/integration/relationships/has-many-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ test("A hasMany updated link should not remove new children", function(assert) {

env.adapter.createRecord = function(store, snapshot, link, relationship) {
return Ember.RSVP.resolve({
id: 1,
links: {
comments: '/some/link'
}
Expand Down Expand Up @@ -382,6 +383,7 @@ test("A hasMany updated link should not remove new children when the parent reco

env.adapter.createRecord = function(store, snapshot, link, relationship) {
return Ember.RSVP.resolve({
id: 1,
links: {
comments: '/some/link'
}
Expand Down Expand Up @@ -2207,6 +2209,7 @@ test("adding and removing records from hasMany relationship #2666", function(ass
})
});

var commentId = 4;
env.registry.register('adapter:comment', DS.RESTAdapter.extend({
deleteRecord(record) {
return Ember.RSVP.resolve();
Expand All @@ -2215,7 +2218,7 @@ test("adding and removing records from hasMany relationship #2666", function(ass
return Ember.RSVP.resolve();
},
createRecord() {
return Ember.RSVP.resolve();
return Ember.RSVP.resolve({ comments: { id: commentId++ }});
}
}));

Expand Down
15 changes: 15 additions & 0 deletions tests/integration/store-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,19 @@ testInDebug('store#findRecord that returns an array should assert', assert => {
}, /expected the primary data returned from a `findRecord` response to be an object but instead it found an array/);
});

testInDebug('store#didSaveRecord should assert when the response to a save does not include the id', function(assert) {
env.adapter.createRecord = function() {
return {};
};

assert.expectAssertion(function() {
run(function() {
var car = store.createRecord('car');
car.save();
});
}, /record was saved but it does not have an id. Please make the server provides an id in the createRecord/);
});

module("integration/store - queryRecord", {
beforeEach() {
initializeStore(DS.Adapter.extend());
Expand All @@ -898,3 +911,5 @@ testInDebug('store#queryRecord should assert when normalized payload of adapter
});
}, /Expected the primary data returned by the serializer for a `queryRecord` response to be a single object or null but instead it was an array./);
});


2 changes: 1 addition & 1 deletion tests/unit/model-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ test("changedAttributes() works while the record is being saved", function(asser
assert.deepEqual(toObj(cat.changedAttributes()), {
name: [undefined, 'Argon'],
likes: [undefined, 'Cheese'] });
return {};
return { id: 1 };
}
});
var Mascot = DS.Model.extend({
Expand Down

0 comments on commit 7d7017c

Please sign in to comment.