Skip to content

Commit

Permalink
finish up fix
Browse files Browse the repository at this point in the history
  • Loading branch information
runspired committed Jul 27, 2018
1 parent 2c40216 commit 8bde815
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 8 deletions.
2 changes: 1 addition & 1 deletion addon/-legacy-private/system/model/model.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
2 changes: 2 additions & 0 deletions addon/-legacy-private/system/model/states.js
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,8 @@ const RootState = {
internalModel.triggerLater('didCommit', internalModel);
},

// TODO @runspired should we special case unloadRecord for root.deleted.saved ?

willCommit() {},
didCommit() {},
pushedData() {},
Expand Down
6 changes: 5 additions & 1 deletion addon/-record-data-private/system/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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());
});
},

Expand Down
2 changes: 2 additions & 0 deletions addon/-record-data-private/system/model/states.js
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,8 @@ const RootState = {
internalModel.triggerLater('didCommit', internalModel);
},

// TODO @runspired should we special case unloadRecord for root.deleted.saved ?

willCommit() {},
didCommit() {},
pushedData() {},
Expand Down
14 changes: 8 additions & 6 deletions tests/integration/records/delete-record-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit 8bde815

Please sign in to comment.