Skip to content

Commit

Permalink
[BUGFIX release] createRecord should only setup relationships it has … (
Browse files Browse the repository at this point in the history
emberjs#5048)

* [BUGFIX release] createRecord should only setup relationships it has data for

* update deps
  • Loading branch information
stefanpenner authored and bmac committed Jul 11, 2017
1 parent e1699cb commit 3eb760b
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 37 deletions.
4 changes: 3 additions & 1 deletion addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,9 @@ Store = Service.extend({

// TODO @runspired this should also be coalesced into some form of internalModel.setState()
internalModel.eachRelationship((key, descriptor) => {
internalModel._relationships.get(key).setHasData(true);
if (properties[key] !== undefined) {
internalModel._relationships.get(key).setHasData(true);
}
});

return record;
Expand Down
2 changes: 1 addition & 1 deletion addon/serializers/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const { camelize } = Ember.String;
@namespace DS
@extends DS.JSONSerializer
*/
let RESTSerializer = JSONSerializer.extend({
const RESTSerializer = JSONSerializer.extend({

/**
`keyForPolymorphicType` can be used to define a custom key when
Expand Down
6 changes: 5 additions & 1 deletion tests/helpers/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,13 @@ export default function setupStore(options) {
registry.register('adapter:-rest', DS.RESTAdapter);
registry.register('adapter:-json-api', DS.JSONAPIAdapter);

env.restSerializer = container.lookup('serializer:-rest');
registry.injection('serializer', 'store', 'service:store');

env.store = container.lookup('service:store');
env.restSerializer = container.lookup('serializer:-rest');
env.restSerializer.store = env.store;
env.serializer = env.store.serializerFor('-default');
env.serializer.store = env.store;
env.adapter = env.store.get('defaultAdapter');

return env;
Expand Down
31 changes: 27 additions & 4 deletions tests/integration/relationships/belongs-to-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -955,26 +955,49 @@ test("belongsTo hasData sync not loaded", function(assert) {
});
});

test("belongsTo hasData async created", function(assert) {
assert.expect(1);
test("belongsTo hasData NOT created", function(assert) {
assert.expect(2);

Book.reopen({
author: belongsTo('author', { async: true })
});

run(() => {
let author = store.createRecord('author');
let book = store.createRecord('book', { name: 'The Greatest Book' });
let relationship = book._internalModel._relationships.get('author');

assert.equal(relationship.hasData, false, 'relationship does not have data');

book = store.createRecord('book', {
name: 'The Greatest Book',
author
});

relationship = book._internalModel._relationships.get('author');

assert.equal(relationship.hasData, true, 'relationship has data');
});
});

test("belongsTo hasData sync created", function(assert) {
assert.expect(1);
assert.expect(2);

run(() => {
let book = store.createRecord('book', { name: 'The Greatest Book' });
let author = store.createRecord('author');
let book = store.createRecord('book', {
name: 'The Greatest Book'
});

let relationship = book._internalModel._relationships.get('author');
assert.equal(relationship.hasData, false, 'relationship does not have data');

book = store.createRecord('book', {
name: 'The Greatest Book',
author
});

relationship = book._internalModel._relationships.get('author');
assert.equal(relationship.hasData, true, 'relationship has data');
});
});
Expand Down
23 changes: 21 additions & 2 deletions tests/integration/relationships/has-many-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2642,25 +2642,44 @@ test("hasMany hasData sync not loaded", function(assert) {
});

test("hasMany hasData async created", function(assert) {
assert.expect(1);
assert.expect(2);

Chapter.reopen({
pages: hasMany('pages', { async: true })
});

run(() => {
let chapter = store.createRecord('chapter', { title: 'The Story Begins' });
let page = store.createRecord('page');

let relationship = chapter._internalModel._relationships.get('pages');
assert.equal(relationship.hasData, false, 'relationship does not have data');

chapter = store.createRecord('chapter', {
title: 'The Story Begins',
pages: [page]
});

relationship = chapter._internalModel._relationships.get('pages');
assert.equal(relationship.hasData, true, 'relationship has data');
});
});

test("hasMany hasData sync created", function(assert) {
assert.expect(1);
assert.expect(2);

run(() => {
let chapter = store.createRecord('chapter', { title: 'The Story Begins' });
let relationship = chapter._internalModel._relationships.get('pages');

assert.equal(relationship.hasData, false, 'relationship does not have data');

chapter = store.createRecord('chapter', {
title: 'The Story Begins',
pages: [store.createRecord('page')]
});
relationship = chapter._internalModel._relationships.get('pages');

assert.equal(relationship.hasData, true, 'relationship has data');
});
});
Expand Down
13 changes: 7 additions & 6 deletions tests/integration/relationships/inverse-relationships-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -600,16 +600,17 @@ test("inverseFor short-circuits when inverse is null", function(assert) {
});
});

testInDebug("Inverse null relationships with models that don't exist throw a nice error", function(assert) {
testInDebug("Inverse null relationships with models that don't exist throw a nice error if trying to use that relationship", function(assert) {
User = DS.Model.extend({
post: DS.belongsTo('post', { inverse: null })
});

var env = setupStore({ user: User });
let env = setupStore({ user: User });

assert.throws(function() {
run(function() {
env.store.createRecord('user');
});
assert.throws(() => {
run(() => env.store.createRecord('user', { post: {}}));
}, /No model was found for 'post'/);

// but don't error if the relationship is not used
run(() => env.store.createRecord('user', {}));
});
49 changes: 28 additions & 21 deletions tests/integration/serializers/json-serializer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,37 @@ module("integration/serializer/json - JSONSerializer", {
});

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

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

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

test("serialize includes id when includeId is true", function(assert) {
run(function() {

test("serialize doesn't include relationship if not aware of one", function(assert) {
run(() => {
post = env.store.createRecord('post', { title: 'Rails is omakase' });
});

let json = serializer.serialize(post._createSnapshot());

assert.deepEqual(json, {
title: "Rails is omakase"
});
});

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

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

assert.deepEqual(json, {
id: 'test',
Expand All @@ -71,12 +81,12 @@ test("serialize includes id when includeId is true", function(assert) {

if (isEnabled("ds-serialize-id")) {
test("serializeId", function(assert) {
run(function() {
run(() => {
post = env.store.createRecord('post');
post.set('id', 'test');
});
var json = {};

let json = {};
serializer.serializeId(post._createSnapshot(), json, 'id');

assert.deepEqual(json, {
Expand All @@ -102,8 +112,7 @@ if (isEnabled("ds-serialize-id")) {

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

Expand All @@ -122,8 +131,7 @@ if (isEnabled("ds-serialize-id")) {

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

Expand Down Expand Up @@ -289,12 +297,11 @@ if (isEnabled("ds-check-should-serialize-relationships")) {
}

test("serializeIntoHash", function(assert) {
run(function() {
post = env.store.createRecord('post', { title: "Rails is omakase" });
run(() => {
post = env.store.createRecord('post', { title: "Rails is omakase", comments: [] });
});

var json = {};

let json = {};
serializer.serializeIntoHash(json, Post, post._createSnapshot());

assert.deepEqual(json, {
Expand All @@ -308,8 +315,8 @@ test("serializePolymorphicType sync", function(assert) {

env.registry.register('serializer:comment', DS.JSONSerializer.extend({
serializePolymorphicType(record, json, relationship) {
var key = relationship.key;
var belongsTo = record.belongsTo(key);
let key = relationship.key;
let belongsTo = record.belongsTo(key);
json[relationship.key + "TYPE"] = belongsTo.modelName;

assert.ok(true, 'serializePolymorphicType is called when serialize a polymorphic belongsTo');
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/serializers/rest-serializer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,9 @@ test('serializeIntoHash uses payloadKeyFromModelName to normalize the payload ro
}
}));

env.container.lookup('serializer:home-planet').serializeIntoHash(json, HomePlanet, league._createSnapshot());
let serializer = env.store.serializerFor('home-planet');

serializer.serializeIntoHash(json, HomePlanet, league._createSnapshot());

assert.deepEqual(json, {
'home-planet': {
Expand Down
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5168,6 +5168,7 @@ right-align@^0.1.1:
dependencies:
align-text "^0.1.1"


rimraf@2.5.2, rimraf@^2.2.8, rimraf@^2.3.4, rimraf@^2.4.3, rimraf@^2.4.4:
version "2.5.2"
resolved "https://registry.npmjs.org/rimraf/-/rimraf-2.5.2.tgz#62ba947fa4c0b4363839aefecd4f0fbad6059726"
Expand Down

0 comments on commit 3eb760b

Please sign in to comment.