From 88b37e2bfdf749987aeca96636a203216c3d7ebf Mon Sep 17 00:00:00 2001 From: Braymon Date: Mon, 15 Apr 2019 08:14:38 +1200 Subject: [PATCH] [BUGFIX unloadRecord] Fix unloadRecord() when unloading a previous parent of a child record Fixes an issue where changing the parent of a child record then unloading the original parent incorrectly resets the child association to null. See PR #5571. --- .../integration/records/edit-record-test.js | 83 +++++++++++++++++++ .../-private/system/model/record-data.ts | 2 +- .../relationships/state/relationship.ts | 17 +++- 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/packages/-ember-data/tests/integration/records/edit-record-test.js b/packages/-ember-data/tests/integration/records/edit-record-test.js index 8df3f7b793b..d96554685b1 100644 --- a/packages/-ember-data/tests/integration/records/edit-record-test.js +++ b/packages/-ember-data/tests/integration/records/edit-record-test.js @@ -195,6 +195,89 @@ module('Editing a Record', function(hooks) { .map(pet => pet.get('name')); assert.deepEqual(pets, ['Shen'], 'Chris has Shen as a pet'); }); + + test('Change parent relationship and unload original parent', async function(assert) { + let chris = store.push({ + data: { + id: '1', + type: 'person', + attributes: { name: 'Chris' }, + relationships: { + pets: { + data: [{ type: 'pet', id: '3' }, { type: 'pet', id: '4' }], + }, + }, + }, + }); + + let john = store.push({ + data: { + id: '2', + type: 'person', + attributes: { name: 'John' }, + relationships: { + pets: { + data: [], + }, + }, + }, + }); + + let shen = store.push({ + data: { + id: '3', + type: 'pet', + attributes: { name: 'Shen' }, + relationships: { + owner: { + data: { + type: 'person', + id: '1', + }, + }, + }, + }, + }); + + let rocky = store.push({ + data: { + id: '4', + type: 'pet', + attributes: { name: 'Rocky' }, + relationships: { + owner: { + data: { + type: 'person', + id: '1', + }, + }, + }, + }, + }); + + assert.ok(shen.get('owner') === chris, 'Precondition: Chris is the current owner'); + assert.ok(rocky.get('owner') === chris, 'Precondition: Chris is the current owner'); + + let pets = chris.pets.toArray().map(pet => pet.name); + assert.deepEqual(pets, ['Shen', 'Rocky'], 'Precondition: Chris has Shen and Rocky as pets'); + + shen.set('owner', john); + assert.ok(shen.get('owner') === john, 'Precondition: John is the new owner of Shen'); + + pets = chris.pets.toArray().map(pet => pet.name); + assert.deepEqual(pets, ['Rocky'], 'Precondition: Chris has Rocky as a pet'); + + pets = john.pets.toArray().map(pet => pet.name); + assert.deepEqual(pets, ['Shen'], 'Precondition: John has Shen as a pet'); + + chris.unloadRecord(); + + assert.ok(rocky.get('owner') === null, 'Rocky has no owner'); + assert.ok(shen.get('owner') === john, 'John should still be the owner of Shen'); + + pets = john.pets.toArray().map(pet => pet.name); + assert.deepEqual(pets, ['Shen'], 'John still has Shen as a pet'); + }); }); module('Adding an async belongsTo relationship to a record', function() { diff --git a/packages/store/addon/-private/system/model/record-data.ts b/packages/store/addon/-private/system/model/record-data.ts index 48e16f44977..9796ec9be4b 100644 --- a/packages/store/addon/-private/system/model/record-data.ts +++ b/packages/store/addon/-private/system/model/record-data.ts @@ -766,7 +766,7 @@ function assertRelationshipData(store, recordData, data, meta) { } // Handle dematerialization for relationship `rel`. In all cases, notify the -// relatinoship of the dematerialization: this is done so the relationship can +// relationship of the dematerialization: this is done so the relationship can // notify its inverse which needs to update state // // If the inverse is sync, unloading this record is treated as a client-side diff --git a/packages/store/addon/-private/system/relationships/state/relationship.ts b/packages/store/addon/-private/system/relationships/state/relationship.ts index ffce4f60a91..19d2ef5d461 100644 --- a/packages/store/addon/-private/system/relationships/state/relationship.ts +++ b/packages/store/addon/-private/system/relationships/state/relationship.ts @@ -214,7 +214,8 @@ export default class Relationship { } recordDataDidDematerialize() { - if (!this.inverseKey) { + const inverseKey = this.inverseKey + if (!inverseKey) { return; } // TODO @runspired fairly sure we need to become stale here @@ -226,8 +227,18 @@ export default class Relationship { if (!this._hasSupportForRelationships(inverseRecordData)) { return; } - let relationship = relationshipStateFor(inverseRecordData, this.inverseKey); - relationship.inverseDidDematerialize(this.recordData); + let relationship = relationshipStateFor(inverseRecordData, inverseKey); + let belongsToRelationship = inverseRecordData.getBelongsTo(inverseKey)._relationship; + + // For canonical members, it is possible that inverseRecordData has already been associated to + // to another record. For such cases, do not dematerialize the inverseRecordData + if ( + !belongsToRelationship || + !belongsToRelationship.inverseRecordData || + this.recordData === belongsToRelationship.inverseRecordData + ) { + relationship.inverseDidDematerialize(this.recordData); + } }); }