[BUGFIX] destroyRecord should also unload, to avoid unload use delete…

…Record + save

addon/-legacy-private/system/model/internal-model.js

@@ -1197,12 +1197,13 @@ export default class InternalModel {
     @method adapterDidCommit
   */
   adapterDidCommit(data) {
+    let store = this.store;
+
     if (data) {
-      this.store._internalModelDidReceiveRelationshipData(
-        this.modelName,
-        this.id,
-        data.relationships
-      );
+      // normalize relationship IDs into records
+      store.updateId(this, data);
+      store._setupRelationshipsForModel(this, data);
+      store._internalModelDidReceiveRelationshipData(this.modelName, this.id, data.relationships);
 
       data = data.attributes;
     }

addon/-legacy-private/system/store.js

@@ -1926,20 +1926,14 @@ Store = Service.extend({
     if (dataArg) {
       data = dataArg.data;
     }
-    if (data) {
-      // normalize relationship IDs into records
-      this.updateId(internalModel, data);
-      this._setupRelationshipsForModel(internalModel, data);
-    } else {
-      assert(
-        `Your ${
-          internalModel.modelName
-        } record was saved to the server, but the response does not have an id and no id has been set client side. Records must have ids. Please update the server response to provide an id in the response or generate the id on the client side either before saving the record or while normalizing the response.`,
-        internalModel.id
-      );
-    }
 
-    //We first make sure the primary data has been updated
+    assert(
+      `Your ${
+        internalModel.modelName
+      } record was saved to the server, but the response does not have an id and no id has been set client side. Records must have ids. Please update the server response to provide an id in the response or generate the id on the client side either before saving the record or while normalizing the response.`,
+      internalModel.id || (data && data.id)
+    );
+
     //TODO try to move notification to the user to the end of the runloop
     internalModel.adapterDidCommit(data);
   },

addon/-record-data-private/system/model/model.js

@@ -615,7 +615,9 @@ const Model = EmberObject.extend(Evented, {
   */
   destroyRecord(options) {
     this.deleteRecord();
-    return this.save(options);
+    return this.save(options).then(() => {
+      this.unloadRecord();
+    });
   },
 
   /**

tests/integration/records/delete-record-test.js

@@ -133,27 +133,32 @@ test('deleting a record that is part of a hasMany removes it from the hasMany re
   assert.equal(person.get('name'), 'Adam Sunderland', 'expected related records to be loaded');
 
   run(function() {
-    person.destroyRecord();
+    person.deleteRecord();
+    person.save();
   });
 
   assert.equal(group.get('people.length'), 1, 'expected 1 related records after delete');
 });
 
-test('deleting a record that is part of a hasMany removes it from the hasMany recordArray when adapter deleteRecord function returns a payload', function(assert) {
+test('We properly unload a record when destroyRecord is called', function(assert) {
   let group;
-  let person;
   const Group = DS.Model.extend({
-    people: DS.hasMany('person', { inverse: null, async: false })
+    name: attr(),
   });
-  Group.toString = () => { return 'Group'; }
+  Group.toString = () => {
+    return 'Group';
+  };
 
+  env.adapter.shouldBackgroundReloadRecord = () => false;
   env.adapter.deleteRecord = function() {
     return EmberPromise.resolve({
       data: {
         id: '1',
         type: 'group',
-        attributes: {}
-      }
+        attributes: {
+          name: 'Deleted Checkers',
+        },
+      },
     });
   };
 
@@ -164,49 +169,23 @@ test('deleting a record that is part of a hasMany removes it from the hasMany re
       data: {
         type: 'group',
         id: '1',
-        relationships: {
-          people: {
-            data: [
-              { type: 'person', id: '1' },
-              { type: 'person', id: '2' }
-            ]
-          }
-        }
-      },
-      included: [
-        {
-          type: 'person',
-          id: '1',
-          attributes: {
-            name: 'Adam Sunderland'
-          }
+        attributes: {
+          name: 'Checkers',
         },
-        {
-          type: 'person',
-          id: '2',
-          attributes: {
-            name: 'Dave Sunderland'
-          }
-        }
-      ]
+      },
     });
 
     group = env.store.peekRecord('group', '1');
-    person = env.store.peekRecord('person', '1');
   });
 
-  // Sanity Check we are in the correct state.
-  assert.equal(group.get('people.length'), 2, 'expected 2 related records before delete');
-  assert.equal(person.get('name'), 'Adam Sunderland', 'expected related records to be loaded');
+  assert.equal(group.get('name'), 'Checkers', 'We have the right group');
 
   const promise = run(() => group.destroyRecord());
 
   return promise.then(() => {
-    // qunit complains if i pass the record to QUnit due to some assertion in ember
-    // iterating over the properties.
-    const hasGroup = env.store.peekRecord('group', '1') ? true : false;
-    assert.equal(hasGroup, false);
-    assert.equal(person.get('group'), null, 'expected relationship to be null');
+    const deletedGroup = env.store.peekRecord('group', '1');
+
+    assert.equal(!!deletedGroup, false, 'expected to no longer have group 1');
   });
 });
 
@@ -246,7 +225,8 @@ test('records can be deleted during record array enumeration', function(assert)
 
   run(function() {
     all.forEach(function(record) {
-      record.destroyRecord();
+      record.deleteRecord();
+      record.save();
     });
   });
 

tests/integration/relationships/has-many-test.js

@@ -3626,30 +3626,15 @@ test('deleteRecord + unloadRecord fun', function(assert) {
     ]);
 
     return run(() => {
-      return env.store
-        .peekRecord('post', 'post-2')
-        .destroyRecord()
-        .then(record => {
-          return env.store.unloadRecord(record);
-        });
+      return env.store.peekRecord('post', 'post-2').destroyRecord();
     })
       .then(() => {
         assert.deepEqual(posts.map(x => x.get('id')), ['post-1', 'post-3', 'post-4', 'post-5']);
-        return env.store
-          .peekRecord('post', 'post-3')
-          .destroyRecord()
-          .then(record => {
-            return env.store.unloadRecord(record);
-          });
+        return env.store.peekRecord('post', 'post-3').destroyRecord();
       })
       .then(() => {
         assert.deepEqual(posts.map(x => x.get('id')), ['post-1', 'post-4', 'post-5']);
-        return env.store
-          .peekRecord('post', 'post-4')
-          .destroyRecord()
-          .then(record => {
-            return env.store.unloadRecord(record);
-          });
+        return env.store.peekRecord('post', 'post-4').destroyRecord();
       })
       .then(() => {
         assert.deepEqual(posts.map(x => x.get('id')), ['post-1', 'post-5']);

tests/unit/model-test.js

@@ -688,7 +688,8 @@ test('supports canonical updates via pushedData in root.deleted.saved', function
   );
 
   run(() => {
-    record.destroyRecord().then(() => {
+    record.deleteRecord();
+    record.save().then(() => {
       let currentState = record._internalModel.currentState;
 
       assert.ok(
@@ -749,7 +750,8 @@ test('Does not support dirtying in root.deleted.saved', function(assert) {
   );
 
   run(() => {
-    record.destroyRecord().then(() => {
+    record.deleteRecord();
+    record.save().then(() => {
       let currentState = record._internalModel.currentState;
 
       assert.ok(