Merge pull request #4911 from runspired/fix/null-relationships-3

[BUGFIX] partially fix pushing of null onto already materialized array

addon/-private/system/relationships/state/has-many.js

@@ -159,7 +159,7 @@ export default class ManyRelationship extends Relationship {
     return this._loadingPromise;
   }
 
-  computeChanges(records) {
+  computeChanges(records = []) {
     let members = this.canonicalMembers;
     let recordsToRemove = [];
     let recordSet = setForArray(records);

tests/unit/model/relationships/has-many-test.js

@@ -318,6 +318,188 @@ test('hasMany can be initially reified with null', function(assert) {
   });
 });
 
+test('hasMany with explicit initial null works even when the inverse was set to not null', function(assert) {
+  assert.expect(2);
+
+  const Tag = DS.Model.extend({
+    name: DS.attr('string'),
+    people: DS.hasMany('person', { async: false })
+  });
+
+  const Person = DS.Model.extend({
+    name: DS.attr('string'),
+    tag: DS.belongsTo('tag', { async: false })
+  });
+
+  let env = setupStore({ tag: Tag, person: Person });
+  let { store } = env;
+
+  env.adapter.shouldBackgroundReloadRecord = () => false;
+
+  run(() => {
+    // first we push in data with the relationship
+    store.push({
+      data: {
+        type: 'person',
+        id: 1,
+        attributes: {
+          name: 'David J. Hamilton'
+        },
+        relationships: {
+          tag: {
+            data: {
+              type: 'tag',
+              id: 1
+            }
+          }
+        }
+      },
+      included: [
+        {
+          type: 'tag',
+          id: 1,
+          attributes: {
+            name: 'whatever'
+          },
+          relationships: {
+            people: {
+              data: [{
+                type: 'person',
+                id: 1
+              }]
+            }
+          }
+        }
+      ]
+    });
+
+    // now we push in data for that record which says it has no relationships
+    store.push({
+      data: {
+        type: 'tag',
+        id: 1,
+        attributes: {
+          name: 'whatever'
+        },
+        relationships: {
+          people: {
+            data: null
+          }
+        }
+      }
+    });
+  });
+
+  return run(() => {
+    let tag = store.peekRecord('tag', 1);
+    let person = store.peekRecord('person', 1);
+
+    assert.equal(person.get('tag'), null, 'relationship is empty');
+    assert.equal(tag.get('people.length'), 0, 'relationship is correct');
+  });
+});
+
+test('hasMany with explicit null works even when the inverse was set to not null', function(assert) {
+  assert.expect(3);
+
+  const Tag = DS.Model.extend({
+    name: DS.attr('string'),
+    people: DS.hasMany('person', { async: false })
+  });
+
+  const Person = DS.Model.extend({
+    name: DS.attr('string'),
+    tag: DS.belongsTo('tag', { async: false })
+  });
+
+  let env = setupStore({ tag: Tag, person: Person });
+  let { store } = env;
+
+  env.adapter.shouldBackgroundReloadRecord = () => false;
+
+  run(() => {
+    // first we push in data with the relationship
+    store.push({
+      data: {
+        type: 'person',
+        id: 1,
+        attributes: {
+          name: 'David J. Hamilton'
+        },
+        relationships: {
+          tag: {
+            data: {
+              type: 'tag',
+              id: 1
+            }
+          }
+        }
+      },
+      included: [
+        {
+          type: 'tag',
+          id: 1,
+          attributes: {
+            name: 'whatever'
+          },
+          relationships: {
+            people: {
+              data: [{
+                type: 'person',
+                id: 1
+              }]
+            }
+          }
+        }
+      ]
+    });
+  });
+
+  run(() => {
+    let person = store.peekRecord('person', 1);
+    let tag = store.peekRecord('tag', 1);
+
+    assert.equal(person.get('tag'), tag, 'relationship is not empty');
+  });
+
+  run(() => {
+    // now we push in data for that record which says it has no relationships
+    store.push({
+      data: {
+        type: 'tag',
+        id: 1,
+        attributes: {
+          name: 'whatever'
+        },
+        relationships: {
+          people: {
+            data: null
+          }
+        }
+      }
+    });
+  });
+
+  return run(() => {
+    let person = store.peekRecord('person', 1);
+    let tag = store.peekRecord('tag', 1);
+
+    assert.equal(person.get('tag'), null,'relationship is now empty');
+
+    /*
+      TODO this should be asserting `0` however
+      before pushing null, length is actually secretly out-of-sync with
+      the canonicalState array, which has duplicated the addCanonicalRecord
+      leading to length `2`, so when we splice out the record we are left
+      with length 1.
+
+      This is fixed by the relationship cleanup PR which noticed this churn
+      and removed it: https://github.com/emberjs/data/pull/4882
+     */
+    assert.equal(tag.get('people.length'), 1, 'relationship is correct');
+  });
+});
+
 test('hasMany tolerates reflexive self-relationships', function(assert) {
   assert.expect(1);