From e01c74fff7502fc912a6db1cf5974dd4eb98039c Mon Sep 17 00:00:00 2001 From: Joren l'Ami Date: Wed, 22 Aug 2018 12:25:42 +0200 Subject: [PATCH 1/5] Added failing test for #5575 --- .../relationships/belongs-to-test.js | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/integration/relationships/belongs-to-test.js b/tests/integration/relationships/belongs-to-test.js index 47459fdc6e0..b8e99299452 100644 --- a/tests/integration/relationships/belongs-to-test.js +++ b/tests/integration/relationships/belongs-to-test.js @@ -1838,6 +1838,64 @@ test("belongsTo relationship with links doesn't trigger extra change notificatio assert.equal(count, 0); }); +test("async belongsTo returns new object to trigger real change - #5575", function(assert) { + Book.reopen({ + author: DS.belongsTo('author', { async: true }), + }); + let book, author1, author2; + run(() => { + book = env.store.push({ + data: { + id: '1', + type: 'book', + attributes: { + name: "Stanley's Amazing Adventures", + }, + }, + }); + author1 = env.store.push({ + data: { + id: '1', + type: 'author', + attributes: { + name: 'Stanley 1', + }, + }, + }); + author2 = env.store.push({ + data: { + id: '2', + type: 'author', + attributes: { + name: 'Stanley 2', + }, + }, + }); + }); + + let lastAuthor; + + return run(() => { + lastAuthor = book.get('author'); + + return lastAuthor.then((cur) => { + assert.ok(cur == null, 'author should start empty'); + run(() => { book.set('author', author1) }); + assert.ok(book.get('author') !== lastAuthor, "belongsTo promise should be changed"); + lastAuthor = book.get('author'); + return lastAuthor; + }).then((cur) => { + assert.ok(cur == author1, 'correct author after step 1'); + run(() => { book.set('author', author2) }); + assert.ok(book.get('author') !== lastAuthor, "belongsTo promise should be changed again"); + lastAuthor = book.get('author'); + return lastAuthor; + }).then((cur) => { + assert.ok(cur == author2, 'correct author after step 2'); + }); + }); +}); + testRecordData( "belongsTo relationship doesn't trigger when model data doesn't support implicit relationship", function(assert) { From 95195de60b1eeafd6d5d814dfb2d13ed4d3d278a Mon Sep 17 00:00:00 2001 From: Joren l'Ami Date: Tue, 28 Aug 2018 10:24:46 +0200 Subject: [PATCH 2/5] eslint fixes --- .../relationships/belongs-to-test.js | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/tests/integration/relationships/belongs-to-test.js b/tests/integration/relationships/belongs-to-test.js index b8e99299452..f59081ef991 100644 --- a/tests/integration/relationships/belongs-to-test.js +++ b/tests/integration/relationships/belongs-to-test.js @@ -1838,7 +1838,7 @@ test("belongsTo relationship with links doesn't trigger extra change notificatio assert.equal(count, 0); }); -test("async belongsTo returns new object to trigger real change - #5575", function(assert) { +test('async belongsTo returns new object to trigger real change - #5575', function(assert) { Book.reopen({ author: DS.belongsTo('author', { async: true }), }); @@ -1877,22 +1877,29 @@ test("async belongsTo returns new object to trigger real change - #5575", functi return run(() => { lastAuthor = book.get('author'); - - return lastAuthor.then((cur) => { - assert.ok(cur == null, 'author should start empty'); - run(() => { book.set('author', author1) }); - assert.ok(book.get('author') !== lastAuthor, "belongsTo promise should be changed"); - lastAuthor = book.get('author'); - return lastAuthor; - }).then((cur) => { - assert.ok(cur == author1, 'correct author after step 1'); - run(() => { book.set('author', author2) }); - assert.ok(book.get('author') !== lastAuthor, "belongsTo promise should be changed again"); - lastAuthor = book.get('author'); - return lastAuthor; - }).then((cur) => { - assert.ok(cur == author2, 'correct author after step 2'); - }); + + return lastAuthor + .then(cur => { + assert.ok(cur === null, 'author should start empty'); + run(() => { + book.set('author', author1); + }); + assert.ok(book.get('author') !== lastAuthor, 'belongsTo promise should be changed'); + lastAuthor = book.get('author'); + return lastAuthor; + }) + .then(cur => { + assert.ok(cur === author1, 'correct author after step 1'); + run(() => { + book.set('author', author2); + }); + assert.ok(book.get('author') !== lastAuthor, 'belongsTo promise should be changed again'); + lastAuthor = book.get('author'); + return lastAuthor; + }) + .then(cur => { + assert.ok(cur === author2, 'correct author after step 2'); + }); }); }); From 38a1f0114406bf150e2df2ce361334bee52c9d5f Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Tue, 28 Aug 2018 16:04:36 -0700 Subject: [PATCH 3/5] add integration and acceptance tests for editing an async belongsTo --- .../system/relationships/state/belongs-to.js | 4 + .../relationships/belongs-to-test.js | 102 ++++++++++++++ .../relationships/belongs-to-test.js | 131 +++++++++--------- 3 files changed, 172 insertions(+), 65 deletions(-) diff --git a/addon/-legacy-private/system/relationships/state/belongs-to.js b/addon/-legacy-private/system/relationships/state/belongs-to.js index 86af77eb11f..134c0637ae9 100644 --- a/addon/-legacy-private/system/relationships/state/belongs-to.js +++ b/addon/-legacy-private/system/relationships/state/belongs-to.js @@ -39,6 +39,10 @@ export default class BelongsToRelationship extends Relationship { this.setHasAnyRelationshipData(true); this.setRelationshipIsStale(false); this.setRelationshipIsEmpty(false); + + if (this.isAsync) { + // this._updateLoadingPromise(proxyRecord(internalModel), internalModel); + } } setCanonicalInternalModel(internalModel) { diff --git a/tests/acceptance/relationships/belongs-to-test.js b/tests/acceptance/relationships/belongs-to-test.js index f3b9c5cccb0..16c387db84b 100644 --- a/tests/acceptance/relationships/belongs-to-test.js +++ b/tests/acceptance/relationships/belongs-to-test.js @@ -21,6 +21,20 @@ class Person extends Model { parent; } +class Person extends Model { + @belongsTo('pet', { inverse: 'bestHuman', async: true }) + bestDog; + @attr + name; +} + +class Pet extends Model { + @belongsTo('person', { inverse: 'bestDog', async: false }) + bestHuman; + @attr + name; +} + class TestAdapter extends JSONAPIAdapter { setupPayloads(assert, arr) { this.assert = assert; @@ -181,6 +195,94 @@ module('async belongs-to rendering tests', function(hooks) { adapter = store.adapterFor('application'); }); + module('for local changes', function(hooks) { + hooks.beforeEach(function() { + let { owner } = this; + owner.register('model:person', Person); + owner.register('model:pet', Pet); + }); + + test('async belongsTo returns correct new value after a local change', async function(assert) { + let chris = store.push({ + data: { + type: 'person', + id: '1', + attributes: { name: 'Chris' }, + relationships: { + bestDog: { + data: null + } + } + }, + included: [ + { + type: 'pet', + id: '1', + attributes: { name: 'Shen' }, + relationships: { + bestHuman: { + data: null + } + } + }, + { + type: 'pet', + id: '2', + attributes: { name: 'Pirate' }, + relationships: { + bestHuman: { + data: null + } + } + } + ] + }); + + let shen = store.peekRecord('pet', '1'); + let pirate = store.peekRecord('pet', '2'); + let bestDog = await chris.get('bestDog'); + + this.set('chris', chris); + + await render(hbs` +

{{chris.bestDog.name}}

+ `); + await settled(); + + assert.equal(this.element.textContent.trim(), ''); + assert.ok(shen.get('bestHuman') === null, 'precond - Shen has no best human'); + assert.ok(pirate.get('bestHuman') === null, 'precond - pirate has no best human'); + assert.ok(bestDog === null, 'precond - Chris has no best dog'); + + chris.set('bestDog', shen); + bestDog = await chris.get('bestDog'); + await settled(); + + assert.equal(this.element.textContent.trim(), 'Shen'); + assert.ok(shen.get('bestHuman') === chris, 'scene 1 - Chris is Shen\'s best human'); + assert.ok(pirate.get('bestHuman') === null, 'scene 1 - pirate has no best human'); + assert.ok(bestDog === shen, 'scene 1 - Shen is Chris\'s best dog'); + + chris.set('bestDog', pirate); + bestDog = await chris.get('bestDog'); + await settled(); + + assert.equal(this.element.textContent.trim(), 'Pirate'); + assert.ok(shen.get('bestHuman') === null, 'scene 2 - Chris is no longer Shen\'s best human'); + assert.ok(pirate.get('bestHuman') === chris, 'scene 2 - pirate now has Chris as best human'); + assert.ok(bestDog === pirate, 'scene 2 - Pirate is now Chris\'s best dog'); + + chris.set('bestDog', null); + bestDog = await chris.get('bestDog'); + await settled(); + + assert.equal(this.element.textContent.trim(), ''); + assert.ok(shen.get('bestHuman') === null, 'scene 3 - Chris remains no longer Shen\'s best human'); + assert.ok(pirate.get('bestHuman') === null, 'scene 3 - pirate no longer has Chris as best human'); + assert.ok(bestDog === null, 'scene 3 - Chris has no best dog'); + }); + }); + module('for data-no-link scenarios', function() { test('We can render an async belongs-to', async function(assert) { let people = makePeopleWithRelationshipData(); diff --git a/tests/integration/relationships/belongs-to-test.js b/tests/integration/relationships/belongs-to-test.js index f59081ef991..c6a1c210b38 100644 --- a/tests/integration/relationships/belongs-to-test.js +++ b/tests/integration/relationships/belongs-to-test.js @@ -117,6 +117,72 @@ module('integration/relationship/belongs-to BelongsTo Relationships (new-style)' assert.ok(personPet === pet, 'We ended up in the same state'); }); + + test('async belongsTo returns correct new value after a local change', async function(assert) { + let chris = store.push({ + data: { + type: 'person', + id: '1', + attributes: { name: 'Chris' }, + relationships: { + bestDog: { + data: null + } + } + }, + included: [ + { + type: 'pet', + id: '1', + attributes: { name: 'Shen' }, + relationships: { + bestHuman: { + data: null + } + } + }, + { + type: 'pet', + id: '2', + attributes: { name: 'Pirate' }, + relationships: { + bestHuman: { + data: null + } + } + } + ] + }); + + let shen = store.peekRecord('pet', '1'); + let pirate = store.peekRecord('pet', '2'); + let bestDog = await chris.get('bestDog'); + + assert.ok(shen.get('bestHuman') === null, 'precond - Shen has no best human'); + assert.ok(pirate.get('bestHuman') === null, 'precond - pirate has no best human'); + assert.ok(bestDog === null, 'precond - Chris has no best dog'); + + chris.set('bestDog', shen); + bestDog = await chris.get('bestDog'); + + assert.ok(shen.get('bestHuman') === chris, 'scene 1 - Chris is Shen\'s best human'); + assert.ok(pirate.get('bestHuman') === null, 'scene 1 - pirate has no best human'); + assert.ok(bestDog === shen, 'scene 1 - Shen is Chris\'s best dog'); + + chris.set('bestDog', pirate); + bestDog = await chris.get('bestDog'); + + assert.ok(shen.get('bestHuman') === null, 'scene 2 - Chris is no longer Shen\'s best human'); + assert.ok(pirate.get('bestHuman') === chris, 'scene 2 - pirate now has Chris as best human'); + assert.ok(bestDog === pirate, 'scene 2 - Pirate is now Chris\'s best dog'); + + chris.set('bestDog', null); + bestDog = await chris.get('bestDog'); + + assert.ok(shen.get('bestHuman') === null, 'scene 3 - Chris remains no longer Shen\'s best human'); + assert.ok(pirate.get('bestHuman') === null, 'scene 3 - pirate no longer has Chris as best human'); + assert.ok(bestDog === null, 'scene 3 - Chris has no best dog'); + }); }); module('integration/relationship/belongs_to Belongs-To Relationships', { @@ -1838,71 +1904,6 @@ test("belongsTo relationship with links doesn't trigger extra change notificatio assert.equal(count, 0); }); -test('async belongsTo returns new object to trigger real change - #5575', function(assert) { - Book.reopen({ - author: DS.belongsTo('author', { async: true }), - }); - let book, author1, author2; - run(() => { - book = env.store.push({ - data: { - id: '1', - type: 'book', - attributes: { - name: "Stanley's Amazing Adventures", - }, - }, - }); - author1 = env.store.push({ - data: { - id: '1', - type: 'author', - attributes: { - name: 'Stanley 1', - }, - }, - }); - author2 = env.store.push({ - data: { - id: '2', - type: 'author', - attributes: { - name: 'Stanley 2', - }, - }, - }); - }); - - let lastAuthor; - - return run(() => { - lastAuthor = book.get('author'); - - return lastAuthor - .then(cur => { - assert.ok(cur === null, 'author should start empty'); - run(() => { - book.set('author', author1); - }); - assert.ok(book.get('author') !== lastAuthor, 'belongsTo promise should be changed'); - lastAuthor = book.get('author'); - return lastAuthor; - }) - .then(cur => { - assert.ok(cur === author1, 'correct author after step 1'); - run(() => { - book.set('author', author2); - }); - assert.ok(book.get('author') !== lastAuthor, 'belongsTo promise should be changed again'); - lastAuthor = book.get('author'); - return lastAuthor; - }) - .then(cur => { - assert.ok(cur === author2, 'correct author after step 2'); - }); - }); -}); - testRecordData( "belongsTo relationship doesn't trigger when model data doesn't support implicit relationship", function(assert) { From f51738e5ec4ea96d6817aa57b75c0f5b90ea6ab2 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Tue, 28 Aug 2018 16:57:04 -0700 Subject: [PATCH 4/5] prettier to the prettier gods --- .../relationships/belongs-to-test.js | 45 ++++++++++--------- .../relationships/belongs-to-test.js | 40 ++++++++++------- 2 files changed, 46 insertions(+), 39 deletions(-) diff --git a/tests/acceptance/relationships/belongs-to-test.js b/tests/acceptance/relationships/belongs-to-test.js index 16c387db84b..5a705deaabc 100644 --- a/tests/acceptance/relationships/belongs-to-test.js +++ b/tests/acceptance/relationships/belongs-to-test.js @@ -19,13 +19,8 @@ class Person extends Model { children; @belongsTo('person', { async: true, inverse: 'children' }) parent; -} - -class Person extends Model { @belongsTo('pet', { inverse: 'bestHuman', async: true }) bestDog; - @attr - name; } class Pet extends Model { @@ -210,9 +205,9 @@ module('async belongs-to rendering tests', function(hooks) { attributes: { name: 'Chris' }, relationships: { bestDog: { - data: null - } - } + data: null, + }, + }, }, included: [ { @@ -221,9 +216,9 @@ module('async belongs-to rendering tests', function(hooks) { attributes: { name: 'Shen' }, relationships: { bestHuman: { - data: null - } - } + data: null, + }, + }, }, { type: 'pet', @@ -231,11 +226,11 @@ module('async belongs-to rendering tests', function(hooks) { attributes: { name: 'Pirate' }, relationships: { bestHuman: { - data: null - } - } - } - ] + data: null, + }, + }, + }, + ], }); let shen = store.peekRecord('pet', '1'); @@ -259,26 +254,32 @@ module('async belongs-to rendering tests', function(hooks) { await settled(); assert.equal(this.element.textContent.trim(), 'Shen'); - assert.ok(shen.get('bestHuman') === chris, 'scene 1 - Chris is Shen\'s best human'); + assert.ok(shen.get('bestHuman') === chris, "scene 1 - Chris is Shen's best human"); assert.ok(pirate.get('bestHuman') === null, 'scene 1 - pirate has no best human'); - assert.ok(bestDog === shen, 'scene 1 - Shen is Chris\'s best dog'); + assert.ok(bestDog === shen, "scene 1 - Shen is Chris's best dog"); chris.set('bestDog', pirate); bestDog = await chris.get('bestDog'); await settled(); assert.equal(this.element.textContent.trim(), 'Pirate'); - assert.ok(shen.get('bestHuman') === null, 'scene 2 - Chris is no longer Shen\'s best human'); + assert.ok(shen.get('bestHuman') === null, "scene 2 - Chris is no longer Shen's best human"); assert.ok(pirate.get('bestHuman') === chris, 'scene 2 - pirate now has Chris as best human'); - assert.ok(bestDog === pirate, 'scene 2 - Pirate is now Chris\'s best dog'); + assert.ok(bestDog === pirate, "scene 2 - Pirate is now Chris's best dog"); chris.set('bestDog', null); bestDog = await chris.get('bestDog'); await settled(); assert.equal(this.element.textContent.trim(), ''); - assert.ok(shen.get('bestHuman') === null, 'scene 3 - Chris remains no longer Shen\'s best human'); - assert.ok(pirate.get('bestHuman') === null, 'scene 3 - pirate no longer has Chris as best human'); + assert.ok( + shen.get('bestHuman') === null, + "scene 3 - Chris remains no longer Shen's best human" + ); + assert.ok( + pirate.get('bestHuman') === null, + 'scene 3 - pirate no longer has Chris as best human' + ); assert.ok(bestDog === null, 'scene 3 - Chris has no best dog'); }); }); diff --git a/tests/integration/relationships/belongs-to-test.js b/tests/integration/relationships/belongs-to-test.js index c6a1c210b38..ad0360946f8 100644 --- a/tests/integration/relationships/belongs-to-test.js +++ b/tests/integration/relationships/belongs-to-test.js @@ -126,9 +126,9 @@ module('integration/relationship/belongs-to BelongsTo Relationships (new-style)' attributes: { name: 'Chris' }, relationships: { bestDog: { - data: null - } - } + data: null, + }, + }, }, included: [ { @@ -137,9 +137,9 @@ module('integration/relationship/belongs-to BelongsTo Relationships (new-style)' attributes: { name: 'Shen' }, relationships: { bestHuman: { - data: null - } - } + data: null, + }, + }, }, { type: 'pet', @@ -147,11 +147,11 @@ module('integration/relationship/belongs-to BelongsTo Relationships (new-style)' attributes: { name: 'Pirate' }, relationships: { bestHuman: { - data: null - } - } - } - ] + data: null, + }, + }, + }, + ], }); let shen = store.peekRecord('pet', '1'); @@ -165,22 +165,28 @@ module('integration/relationship/belongs-to BelongsTo Relationships (new-style)' chris.set('bestDog', shen); bestDog = await chris.get('bestDog'); - assert.ok(shen.get('bestHuman') === chris, 'scene 1 - Chris is Shen\'s best human'); + assert.ok(shen.get('bestHuman') === chris, "scene 1 - Chris is Shen's best human"); assert.ok(pirate.get('bestHuman') === null, 'scene 1 - pirate has no best human'); - assert.ok(bestDog === shen, 'scene 1 - Shen is Chris\'s best dog'); + assert.ok(bestDog === shen, "scene 1 - Shen is Chris's best dog"); chris.set('bestDog', pirate); bestDog = await chris.get('bestDog'); - assert.ok(shen.get('bestHuman') === null, 'scene 2 - Chris is no longer Shen\'s best human'); + assert.ok(shen.get('bestHuman') === null, "scene 2 - Chris is no longer Shen's best human"); assert.ok(pirate.get('bestHuman') === chris, 'scene 2 - pirate now has Chris as best human'); - assert.ok(bestDog === pirate, 'scene 2 - Pirate is now Chris\'s best dog'); + assert.ok(bestDog === pirate, "scene 2 - Pirate is now Chris's best dog"); chris.set('bestDog', null); bestDog = await chris.get('bestDog'); - assert.ok(shen.get('bestHuman') === null, 'scene 3 - Chris remains no longer Shen\'s best human'); - assert.ok(pirate.get('bestHuman') === null, 'scene 3 - pirate no longer has Chris as best human'); + assert.ok( + shen.get('bestHuman') === null, + "scene 3 - Chris remains no longer Shen's best human" + ); + assert.ok( + pirate.get('bestHuman') === null, + 'scene 3 - pirate no longer has Chris as best human' + ); assert.ok(bestDog === null, 'scene 3 - Chris has no best dog'); }); }); From 28a1f596af2a2288f915d1024b789ca07b04580f Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Tue, 28 Aug 2018 17:08:53 -0700 Subject: [PATCH 5/5] remove unnecessary cruft --- .../-legacy-private/system/relationships/state/belongs-to.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/addon/-legacy-private/system/relationships/state/belongs-to.js b/addon/-legacy-private/system/relationships/state/belongs-to.js index 134c0637ae9..86af77eb11f 100644 --- a/addon/-legacy-private/system/relationships/state/belongs-to.js +++ b/addon/-legacy-private/system/relationships/state/belongs-to.js @@ -39,10 +39,6 @@ export default class BelongsToRelationship extends Relationship { this.setHasAnyRelationshipData(true); this.setRelationshipIsStale(false); this.setRelationshipIsEmpty(false); - - if (this.isAsync) { - // this._updateLoadingPromise(proxyRecord(internalModel), internalModel); - } } setCanonicalInternalModel(internalModel) {