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

fix notifyPropertyChange error causing backtracking re-render error #5042

Closed
wants to merge 3 commits into from
Closed
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
8 changes: 5 additions & 3 deletions addon/-private/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -724,25 +724,27 @@ export default class InternalModel {

notifyHasManyAdded(key, record, idx) {
if (this.hasRecord) {
this._record.notifyHasManyAdded(key, record, idx);
Ember.run.join(() => this._record.notifyHasManyAdded(key, record, idx));
}
}

notifyHasManyRemoved(key, record, idx) {
if (this.hasRecord) {
this._record.notifyHasManyRemoved(key, record, idx);
Ember.run.join(() => this._record.notifyHasManyRemoved(key, record, idx));
}
}

notifyBelongsToChanged(key, record) {
if (this.hasRecord) {
this._record.notifyBelongsToChanged(key, record);
Ember.run.join(() => this._record.notifyBelongsToChanged(key, record));
}
}

notifyPropertyChange(key) {
if (this.hasRecord) {
Ember.beginPropertyChanges();
this._record.notifyPropertyChange(key);
Ember.endPropertyChanges();
}
}

Expand Down
4 changes: 4 additions & 0 deletions addon/-private/system/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,9 @@ const Model = Ember.Object.extend(Ember.Evented, {
},

notifyBelongsToChanged(key) {
Ember.beginPropertyChanges();
this.notifyPropertyChange(key);
Ember.endPropertyChanges();
},
/**
Given a callback, iterates over each of the relationships in the model,
Expand Down Expand Up @@ -1112,7 +1114,9 @@ const Model = Ember.Object.extend(Ember.Evented, {
//TODO(Igor): Consider whether we could do this only if the record state is unloaded

//Goes away once hasMany is double promisified
Ember.beginPropertyChanges();
this.notifyPropertyChange(key);
Ember.endPropertyChanges();
},

eachAttribute(callback, binding) {
Expand Down
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