Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX release] fix regression where dynamically set id is not serialized in a create… #3701

Merged
merged 1 commit into from
Sep 8, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/ember-data/lib/system/model/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,6 @@ export default function attr(type, options) {
}
},
set: function(key, value) {
Ember.assert("You may not set `id` as an attribute on your model. Please remove any lines that look like: `id: DS.attr('<type>')` from " + this.constructor.toString(), key !== 'id');
var internalModel = this._internalModel;
var oldValue = getValue(internalModel, key);

Expand Down
5 changes: 2 additions & 3 deletions packages/ember-data/lib/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,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 @@ -542,9 +541,9 @@ InternalModel.prototype = {
},

setId: function(id) {
var oldId = this.id;
Ember.assert('A record\'s id cannot be changed once it is in the loaded state', oldId === null || oldId === id || this.isNew());
this.id = id;
//TODO figure out whether maybe we should proxy
set(this.record, 'id', id);
},

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

/**
@property currentState
Expand Down Expand Up @@ -769,6 +768,7 @@ var Model = Ember.Object.extend(Ember.Evented, {
willMergeMixin: function(props) {
var constructor = this.constructor;
Ember.assert('`' + intersection(Object.keys(props), RESERVED_MODEL_PROPS)[0] + '` is a reserved property name on DS.Model objects. Please choose a different property name for ' + constructor.toString(), !intersection(Object.keys(props), RESERVED_MODEL_PROPS)[0]);
Ember.assert("You may not set `id` as an attribute on your model. Please remove any lines that look like: `id: DS.attr('<type>')` from " + constructor.toString(), Object.keys(props).indexOf('id') === -1);
},

attr: function() {
Expand Down Expand Up @@ -840,4 +840,19 @@ Model.reopenClass({
modelName: null
});

Object.defineProperty(Model.prototype, 'id', {
configurable: true,
enumerable: false,
set(id) {
if (this._internalModel) {
this._internalModel.setId(id);
}
},
get() {
if (this._internalModel) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should never be false

return this._internalModel.id;
}
}
});

export default Model;
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.');
});
});