From 8bde8157a1adfc4566cbfd1b248def90b8dfb84a Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Fri, 27 Jul 2018 15:14:45 -0700 Subject: [PATCH] finish up fix --- addon/-legacy-private/system/model/model.js | 2 +- addon/-legacy-private/system/model/states.js | 2 ++ addon/-record-data-private/system/model/model.js | 6 +++++- addon/-record-data-private/system/model/states.js | 2 ++ tests/integration/records/delete-record-test.js | 14 ++++++++------ 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/addon/-legacy-private/system/model/model.js b/addon/-legacy-private/system/model/model.js index af209a66b96..ef4117610d2 100644 --- a/addon/-legacy-private/system/model/model.js +++ b/addon/-legacy-private/system/model/model.js @@ -1,7 +1,7 @@ import ComputedProperty from '@ember/object/computed'; import { isNone } from '@ember/utils'; import EmberError from '@ember/error'; -import run from '@ember/runloop'; +import { run } from '@ember/runloop'; import Evented from '@ember/object/evented'; import EmberObject, { computed, get } from '@ember/object'; import Map from '../map'; diff --git a/addon/-legacy-private/system/model/states.js b/addon/-legacy-private/system/model/states.js index 7697a1f1fd9..41358f40de5 100644 --- a/addon/-legacy-private/system/model/states.js +++ b/addon/-legacy-private/system/model/states.js @@ -699,6 +699,8 @@ const RootState = { internalModel.triggerLater('didCommit', internalModel); }, + // TODO @runspired should we special case unloadRecord for root.deleted.saved ? + willCommit() {}, didCommit() {}, pushedData() {}, diff --git a/addon/-record-data-private/system/model/model.js b/addon/-record-data-private/system/model/model.js index 539dfc3a645..8f7a8ab0bb9 100644 --- a/addon/-record-data-private/system/model/model.js +++ b/addon/-record-data-private/system/model/model.js @@ -3,6 +3,7 @@ import { isNone } from '@ember/utils'; import EmberError from '@ember/error'; import Evented from '@ember/object/evented'; import EmberObject, { computed, get } from '@ember/object'; +import { run } from '@ember/runloop'; import Map from '../map'; import { DEBUG } from '@glimmer/env'; import { assert, warn } from '@ember/debug'; @@ -616,7 +617,10 @@ const Model = EmberObject.extend(Evented, { destroyRecord(options) { this.deleteRecord(); return this.save(options).then(() => { - this.unloadRecord(); + // the nested runloop here is necessary to ensure that the record is fully + // destroyed prior to the promise resolving. + // run.join is inadequate as the destroy queue would still be flushed after the resolve + run(() => this.unloadRecord()); }); }, diff --git a/addon/-record-data-private/system/model/states.js b/addon/-record-data-private/system/model/states.js index 98cf914167e..dec86d468a3 100644 --- a/addon/-record-data-private/system/model/states.js +++ b/addon/-record-data-private/system/model/states.js @@ -697,6 +697,8 @@ const RootState = { internalModel.triggerLater('didCommit', internalModel); }, + // TODO @runspired should we special case unloadRecord for root.deleted.saved ? + willCommit() {}, didCommit() {}, pushedData() {}, diff --git a/tests/integration/records/delete-record-test.js b/tests/integration/records/delete-record-test.js index ddc61a9b943..780e76da543 100644 --- a/tests/integration/records/delete-record-test.js +++ b/tests/integration/records/delete-record-test.js @@ -252,16 +252,17 @@ module('integration/deletedRecord - Deleting Records', function(hooks) { ); assert.equal(get(store.peekAll('person'), 'length'), 1, 'The new person should be in the store'); - record.deleteRecord(); + let internalModel = record._internalModel; + + await record.destroyRecord(); - assert.equal(get(record, 'currentState.stateName'), 'root.deleted.saved', 'We reached the correct persisted saved state'); + assert.equal(internalModel.currentState.stateName, 'root.deleted.saved', 'We reached the correct persisted saved state'); assert.equal( get(store.peekAll('person'), 'length'), 0, 'The new person should be removed from the store' ); - let internalModel = record._internalModel; let cache = store._identityMap._map.person._models; assert.todo.ok(cache.indexOf(internalModel) === -1, 'The internal model is removed from the cache'); assert.todo.equal(internalModel.isDestroying, true, 'The internal model is destroyed'); @@ -300,18 +301,19 @@ module('integration/deletedRecord - Deleting Records', function(hooks) { 'records should start in the created.invalid state' ); assert.equal(get(store.peekAll('person'), 'length'), 1, 'The new person should be in the store'); + let internalModel = record._internalModel; await record.destroyRecord(); - assert.equal(get(record, 'currentState.stateName'), 'root.deleted.saved', 'We reached the correct persisted saved state'); - debugger; + // it is uncertain that `root.empty` vs `root.deleted.saved` afterwards is correct + // but this is the expected result of `unloadRecord`. We may want a `root.deleted.saved.unloaded` state? + assert.equal(internalModel.currentState.stateName, 'root.empty', 'We reached the correct persisted saved state'); assert.equal( get(store.peekAll('person'), 'length'), 0, 'The new person should be removed from the store' ); - let internalModel = record._internalModel; let cache = store._identityMap._map.person._models; assert.ok(cache.indexOf(internalModel) === -1, 'The internal model is removed from the cache');