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

{BACKPORT RELEASE} [BUGFIX release] Don't notify relationships with links during initial… #5121

Closed
wants to merge 1 commit 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
7 changes: 5 additions & 2 deletions addon/-private/system/relationships/state/relationship.js
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ export default class Relationship {
this.store._updateRelationshipState(this);
}

updateLink(link) {
updateLink(link, initial) {
heimdall.increment(updateLink);
warn(`You pushed a record of type '${this.internalModel.modelName}' with a relationship '${this.key}' configured as 'async: false'. You've included a link but no primary data, this may be an error in your payload.`, this.isAsync || this.hasData , {
id: 'ds.store.push-link-for-sync-relationship'
Expand All @@ -355,7 +355,10 @@ export default class Relationship {

this.link = link;
this.linkPromise = null;
this.internalModel.notifyPropertyChange(this.key);

if (!initial) {
this.internalModel.notifyPropertyChange(this.key);
}
}

findLink() {
Expand Down
2 changes: 1 addition & 1 deletion addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -2881,7 +2881,7 @@ function setupRelationships(store, internalModel, data, modelNameToInverseMap) {

if (relationshipRequiresNotification) {
let relationshipData = data.relationships[relationshipName];
relationships.get(relationshipName).push(relationshipData);
relationships.get(relationshipName).push(relationshipData, false);
}

// in debug, assert payload validity eagerly
Expand Down
35 changes: 35 additions & 0 deletions tests/integration/relationships/belongs-to-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1406,3 +1406,38 @@ testInDebug("A belongsTo relationship warns if malformatted data is pushed into
});
}, /expected the data for the book relationship on a <chapter:1> to be in a JSON API format/);
});

test("belongsTo relationship with links doesn't trigger extra change notifications - #4942", function(assert) {
Chapter.reopen({
book: DS.belongsTo({ async: true })
});

run(() => {
env.store.push({
data: {
type: 'chapter',
id: '1',
relationships: {
book: {
data: { type: 'book', id: '1' },
links: { related: '/chapter/1/book' }
}
}
},
included: [{ type: 'book', id: '1' }]
});
});

let chapter = env.store.peekRecord('chapter', '1');
let count = 0;

chapter.addObserver('book', () => {
count++;
});

run(() => {
chapter.get('book');
});

assert.equal(count, 0);
});
31 changes: 31 additions & 0 deletions tests/integration/relationships/has-many-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3057,3 +3057,34 @@ test("deleted records should stay deleted", function(assert) {
);
});
});

test("hasMany relationship with links doesn't trigger extra change notifications - #4942", function(assert) {
run(() => {
env.store.push({
data: {
type: 'book',
id: '1',
relationships: {
chapters: {
data: [{ type: 'chapter', id: '1' }],
links: { related: '/book/1/chapters' }
}
}
},
included: [{ type: 'chapter', id: '1' }]
});
});

let book = env.store.peekRecord('book', '1');
let count = 0;

book.addObserver('chapters', () => {
count++;
});

run(() => {
book.get('chapters');
});

assert.equal(count, 0);
});