From 00634847195a952978728930ddee06fe4aab9125 Mon Sep 17 00:00:00 2001 From: Nick Anderson Date: Sun, 13 Aug 2017 13:24:09 -0700 Subject: [PATCH] [BUGFIX release] fix relationship merging When relationship information is merged, the newest information should win. Unfortunately prior to this PR, relationship containing only `link` information, would mistakenly purge any existing `data` information on that relationship. 1. Update One ```js { data: { /* existing data */ }, Link: { /* existing data */ }, } ``` 2. Update Two ```js { Link: { /* new data */ }, } ``` Previously resulted in: ```js { link: { /* new data */ }, data: null // oops } ``` But now (correctly) results in: ```js { link: { /* new data */ }, data: { /* existing data */ } } ``` --- .../relationships/relationship-payloads.js | 8 +- .../relationships/nested-relationship-test.js | 148 ++++++++++++++++++ 2 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 tests/integration/relationships/nested-relationship-test.js diff --git a/addon/-private/system/relationships/relationship-payloads.js b/addon/-private/system/relationships/relationship-payloads.js index ee9626c8683..d0815007048 100644 --- a/addon/-private/system/relationships/relationship-payloads.js +++ b/addon/-private/system/relationships/relationship-payloads.js @@ -220,7 +220,13 @@ export default class RelationshipPayloads { // Then we will initially have set user:2 as having helicopter:1, which we // need to remove before adding helicopter:1 to user:4 // - this._removeInverse(id, previousPayload, inverseIdToPayloads); + // only remove relationship information before adding if there is relationshipData.data + // * null is considered new information "empty", and it should win + // * undefined is NOT considered new information, we should keep original state + // * anything else is considered new information, and it should win + if (relationshipData.data !== undefined) { + this._removeInverse(id, previousPayload, inverseIdToPayloads); + } idToPayloads[id] = relationshipData; this._populateInverse(relationshipData, inverseRelationshipData, inverseIdToPayloads, inverseIsMany); } diff --git a/tests/integration/relationships/nested-relationship-test.js b/tests/integration/relationships/nested-relationship-test.js new file mode 100644 index 00000000000..425a48701ec --- /dev/null +++ b/tests/integration/relationships/nested-relationship-test.js @@ -0,0 +1,148 @@ +import setupStore from 'dummy/tests/helpers/store'; +import Ember from 'ember'; + +import {module, test} from 'qunit'; + +import DS from 'ember-data'; + +const { get, run } = Ember; +const { attr, hasMany, belongsTo } = DS; + +let env, store, serializer, Elder, MiddleAger, Kid; + +module('integration/relationships/nested_relationships_test - Nested relationships', { + beforeEach() { + Elder = DS.Model.extend({ + name: attr('string'), + middleAgers: hasMany('middle-ager') + }); + + MiddleAger = DS.Model.extend({ + name: attr('string'), + elder: belongsTo('elder'), + kids: hasMany('kid') + }); + + Kid = DS.Model.extend({ + name: attr('string'), + middleAger: belongsTo('middle-ager') + }); + + env = setupStore({ + elder: Elder, + 'middle-ager': MiddleAger, + kid: Kid, + adapter: DS.JSONAPIAdapter + }); + + store = env.store; + serializer = env.serializer; + }, + + afterEach() { + run(env.container, 'destroy'); + } +}); + +/* + Server loading tests +*/ + +test('Sideloaded nested relationships load correctly', function(assert) { + run(() => { + serializer.pushPayload(store, { + data: [ + { + id: '1', + type: 'kids', + links: { + self: '/kids/1' + }, + attributes: { + name: 'Kid 1' + }, + relationships: { + 'middle-ager': { + links: { + self: '/kids/1/relationships/middle-ager', + related: '/kids/1/middle-ager' + }, + data:{ + type: 'middle-agers', + id: '1' + } + } + } + } + ], + included: [ + { + id: '1', + type: 'middle-agers', + links: { + self: '/middle-ager/1' + }, + attributes: { + name: 'Middle Ager 1' + }, + relationships: { + elder: { + links: { + self: '/middle-agers/1/relationships/elder', + related: '/middle-agers/1/elder' + }, + data: { + type: 'elders', + id: '1' + } + }, + kids: { + links: { + self: '/middle-agers/1/relationships/kids', + related: '/middle-agers/1/kids' + } + } + } + }, + + { + id: '1', + type: 'elders', + links: { + self: '/elders/1' + }, + attributes: { + name: 'Elder 1' + }, + relationships: { + 'middle-agers': { + links: { + self: '/elders/1/relationships/middle-agers', + related: '/elders/1/middle-agers' + } + } + } + } + ] + }); + }); + + return run(() => { + let kid = store.peekRecord('kid', 1); + + return kid.get('middleAger').then(middleAger => { + assert.ok(middleAger, 'MiddleAger relationship was set up correctly'); + + let middleAgerName = get(middleAger, 'name'); + assert.equal(middleAgerName, 'Middle Ager 1', 'MiddleAger name is there'); + assert.ok(middleAger.get('kids').includes(kid)); + + return middleAger.get('elder').then(elder => { + assert.notEqual(elder, null, 'Elder relationship was set up correctly'); + let elderName = get(elder, 'name'); + assert.equal(elderName, 'Elder 1', 'Elder name is there'); + }); + }); + }); +}); +