Skip to content

Commit

Permalink
fix regression where dynamically set id is not serialized in a create…
Browse files Browse the repository at this point in the history
… save request

closes #3694
- uses es5 getters and setters to be able to set the internalModel id
- add assertion that model is new before setting id
  • Loading branch information
acburdine committed Aug 27, 2015
1 parent 9485564 commit cc84676
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 4 deletions.
4 changes: 1 addition & 3 deletions packages/ember-data/lib/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ InternalModel.prototype = {
// lookupFactory should really return an object that creates
// instances with the injections applied
this.record = this.type._create({
id: this.id,
store: this.store,
container: this.container,
_internalModel: this,
Expand Down Expand Up @@ -541,9 +540,8 @@ InternalModel.prototype = {
},

setId: function(id) {
Ember.assert('Object is a newly created model instance', this.isNew);
this.id = id;
//TODO figure out whether maybe we should proxy
set(this.record, 'id', id);
},

didError: function(error) {
Expand Down
8 changes: 7 additions & 1 deletion packages/ember-data/lib/system/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,13 @@ var Model = Ember.Object.extend(Ember.Evented, {
@property id
@type {String}
*/
id: null,
get id () {
return this._internalModel.id;
},

set id (id) {
this._internalModel.setId(id);
},

/**
@property currentState
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,36 @@ module("integration/serializer/json - JSONSerializer", {
}
});

test("serialize doesn't include ID when includeId is false", function() {
run(function() {
post = env.store.createRecord('post', { title: 'Rails is omakase' });
});
var json = {};

json = env.serializer.serialize(post._createSnapshot(), { includeId: false });

deepEqual(json, {
title: "Rails is omakase",
comments: []
});
});

test("serialize includes id when includeId is true", function() {
run(function() {
post = env.store.createRecord('post', { title: 'Rails is omakase' });
post.set('id', 'test');
});
var json = {};

json = env.serializer.serialize(post._createSnapshot(), { includeId: true });

deepEqual(json, {
id: 'test',
title: 'Rails is omakase',
comments: []
});
});

test("serializeAttribute", function() {
run(function() {
post = env.store.createRecord('post', { title: "Rails is omakase" });
Expand Down
22 changes: 22 additions & 0 deletions packages/ember-data/tests/unit/model-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1116,3 +1116,25 @@ test('accessing attributes in the initializer should not throw an error', functi

run(() => store.createRecord('person'));
});

test('setting the id after model creation should correctly update the id', function () {
expect(2);
var Person = DS.Model.extend({
name: DS.attr('string')
});

var env = setupStore({
person: Person
});
var store = env.store;

run(function () {
var person = store.createRecord('person');

equal(person.get('id'), null, 'initial created model id should be null');

person.set('id', 'john');

equal(person.get('id'), 'john', 'new id should be correctly set.');
});
});

0 comments on commit cc84676

Please sign in to comment.