Skip to content

Commit

Permalink
[BUGFIX release] fix relationship merging
Browse files Browse the repository at this point in the history
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 */ }
}
```
  • Loading branch information
nbrookie authored and stefanpenner committed Aug 24, 2017
1 parent e23ded9 commit 0063484
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 1 deletion.
8 changes: 7 additions & 1 deletion addon/-private/system/relationships/relationship-payloads.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
148 changes: 148 additions & 0 deletions tests/integration/relationships/nested-relationship-test.js
Original file line number Diff line number Diff line change
@@ -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');
});
});
});
});

0 comments on commit 0063484

Please sign in to comment.