Skip to content

Commit

Permalink
Clean up logic to only remove records in the 'deleted.saved' state.
Browse files Browse the repository at this point in the history
  • Loading branch information
bmac committed Jul 13, 2015
1 parent 529f959 commit a570eca
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 33 deletions.
7 changes: 4 additions & 3 deletions packages/ember-data/lib/system/model/states.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,6 @@ var DirtyState = {
// EVENTS
deleteRecord: function(internalModel) {
internalModel.transitionTo('deleted.uncommitted');
internalModel.disconnectRelationships();
},

didSetProperty: function(internalModel, context) {
Expand Down Expand Up @@ -399,9 +398,11 @@ var createdState = dirtyState({
});

createdState.invalid.rolledBack = function(internalModel) {
internalModel.disconnectRelationships();
internalModel.transitionTo('deleted.saved');
};
createdState.uncommitted.rolledBack = function(internalModel) {
internalModel.disconnectRelationships();
internalModel.transitionTo('deleted.saved');
};

Expand All @@ -417,6 +418,7 @@ createdState.uncommitted.deleteRecord = function(internalModel) {

createdState.uncommitted.rollback = function(internalModel) {
DirtyState.uncommitted.rollback.apply(this, arguments);
internalModel.disconnectRelationships();
internalModel.transitionTo('deleted.saved');
};

Expand All @@ -435,7 +437,6 @@ updatedState.inFlight.unloadRecord = assertAgainstUnloadRecord;

updatedState.uncommitted.deleteRecord = function(internalModel) {
internalModel.transitionTo('deleted.uncommitted');
internalModel.disconnectRelationships();
};

var RootState = {
Expand Down Expand Up @@ -572,7 +573,6 @@ var RootState = {

deleteRecord: function(internalModel) {
internalModel.transitionTo('deleted.uncommitted');
internalModel.disconnectRelationships();
},

unloadRecord: function(internalModel) {
Expand Down Expand Up @@ -661,6 +661,7 @@ var RootState = {
// TODO: More robust semantics around save-while-in-flight
willCommit: Ember.K,
didCommit: function(internalModel) {
internalModel.disconnectRelationships();
internalModel.transitionTo('saved');

internalModel.send('invokeLifecycleCallbacks');
Expand Down
12 changes: 5 additions & 7 deletions packages/ember-data/lib/system/record-array-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,12 @@ export default Ember.Object.extend({
@method updateRecordArrays
*/
updateRecordArrays: function() {
this.changedRecords.forEach((record) => {
if (record.isDeleted()) {
// TODO: it can be refactored after #2862 && #2859 are closed
// if (get(record, 'isDestroyed') || get(record, 'isDestroying') ||
// (get(record, 'isDeleted') && !get(record, 'isDirty'))) {
this._recordWasDeleted(record);
this.changedRecords.forEach((internalModel) => {
if (get(internalModel, 'record.isDestroyed') || get(internalModel, 'record.isDestroying') ||
(get(internalModel, 'currentState.stateName') === 'root.deleted.saved')) {
this._recordWasDeleted(internalModel);
} else {
this._recordWasChanged(record);
this._recordWasChanged(internalModel);
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ test("records should not be removed from record arrays just after deleting, but
return Ember.RSVP.Promise.resolve();
};

var all;
run(function() {
adam = env.store.push('person', { id: 1, name: "Adam Sunderland" });
dave = env.store.push('person', { id: 2, name: "Dave Sunderland" });
all = env.store.peekAll('person');
});
var all = env.store.all('person');


// pre-condition
equal(all.get('length'), 2, 'expected 2 records');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ module('integration/relationships/many_to_many_test - ManyToMany relationships',
env = setupStore({
user: User,
topic: Topic,
account: Account
account: Account,
adapter: DS.Adapter.extend({
deleteRecord: () => Ember.RSVP.resolve()
})
});

store = env.store;
Expand Down Expand Up @@ -415,7 +418,7 @@ test("Deleting a record that has a hasMany relationship removes it from the othe
});
});

run(topic, 'deleteRecord');
run(topic, 'destroyRecord');
run(function() {
topic.get('users').then(async(function(fetchedUsers) {
equal(fetchedUsers.get('length'), 1, 'Users are still there');
Expand Down Expand Up @@ -458,7 +461,7 @@ test("Deleting a record that has a hasMany relationship removes it from the othe
});
});

run(account, 'deleteRecord');
run(account, 'destroyRecord');
equal(account.get('users.length'), 1, 'Users are still there');
equal(user.get('accounts.length'), 0, 'Acocount got removed from the user');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ module('integration/relationships/one_to_many_test - OneToMany relationships', {
env = setupStore({
user: User,
message: Message,
account: Account
account: Account,
adapter: DS.Adapter.extend({
deleteRecord: () => Ember.RSVP.resolve()
})
});

store = env.store;
Expand Down Expand Up @@ -1197,7 +1200,7 @@ test("When deleting a record that has a belongsTo it is removed from the hasMany
}
});
});
run(message, 'deleteRecord');
run(message, 'destroyRecord');
run(function() {
message.get('user').then(function(fetchedUser) {
equal(fetchedUser, user, 'Message still has the user');
Expand Down Expand Up @@ -1238,7 +1241,7 @@ test("When deleting a record that has a belongsTo it is removed from the hasMany
}
}
});
account.deleteRecord();
account.destroyRecord();
});
equal(user.get('accounts.length'), 0, "User was removed from the accounts");
equal(account.get('user'), user, 'Account still has the user');
Expand Down Expand Up @@ -1274,7 +1277,7 @@ test("When deleting a record that has a hasMany it is removed from the belongsTo
}
});
});
run(user, 'deleteRecord');
run(user, 'destroyRecord');
run(function() {
message.get('user').then(function(fetchedUser) {
equal(fetchedUser, null, 'Message does not have the user anymore');
Expand Down Expand Up @@ -1316,7 +1319,7 @@ test("When deleting a record that has a hasMany it is removed from the belongsTo
});
});
run(function() {
user.deleteRecord();
user.destroyRecord();
});
equal(user.get('accounts.length'), 1, "User still has the accounts");
equal(account.get('user'), null, 'Account no longer has the user');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ module('integration/relationships/one_to_one_test - OneToOne relationships', {

env = setupStore({
user: User,
job: Job
job: Job,
adapter: DS.Adapter.extend({
deleteRecord: () => Ember.RSVP.resolve()
})
});

store = env.store;
Expand Down Expand Up @@ -862,8 +865,8 @@ test("When deleting a record that has a belongsTo relationship, the record is re
});

});
run(stanley, 'destroyRecord');
run(function() {
stanley.deleteRecord();
stanleysFriend.get('bestFriend').then(function(fetchedUser) {
equal(fetchedUser, null, 'Stanley got removed');
});
Expand Down Expand Up @@ -905,7 +908,7 @@ test("When deleting a record that has a belongsTo relationship, the record is re

});
run(function() {
job.deleteRecord();
job.destroyRecord();
});
equal(user.get('job'), null, 'Job got removed from the user');
equal(job.get('user'), user, 'Job still has the user');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ test("a deleted record's attributes can be rollbacked if it fails to save, recor
run(function() {
person.deleteRecord();
});
equal(people.get('length'), 0, "a deleted record does not appear in record array anymore");
equal(people.objectAt(0), null, "a deleted record does not appear in record array anymore");
equal(people.get('length'), 1, "a deleted record appears in record array until it is saved");
equal(people.objectAt(0), person, "a deleted record appears in record array until it is saved");

run(function() {
person.save().then(null, function() {
Expand Down Expand Up @@ -213,8 +213,8 @@ test("deleted record's attributes can be rollbacked", function() {
person.deleteRecord();
});

equal(people.get('length'), 0, "a deleted record does not appear in record array anymore");
equal(people.objectAt(0), null, "a deleted record does not appear in record array anymore");
equal(people.get('length'), 1, "a deleted record appears in the record array until it is saved");
equal(people.objectAt(0), person, "a deleted record appears in the record array until it is saved");

equal(person.get('isDeleted'), true, "must be deleted");

Expand Down
27 changes: 20 additions & 7 deletions packages/ember-data/tests/unit/record-array-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ test("stops updating when destroyed", function() {


test("a loaded record is removed from a record array when it is deleted", function() {
expect(4);
expect(5);

var Tag = DS.Model.extend({
people: DS.hasMany('person', { async: false })
Expand All @@ -92,7 +92,10 @@ test("a loaded record is removed from a record array when it is deleted", functi

var env = setupStore({
tag: Tag,
person: Person
person: Person,
adapter: DS.Adapter.extend({
deleteRecord: () => Ember.RSVP.resolve()
})
});
var store = env.store;

Expand Down Expand Up @@ -123,7 +126,11 @@ test("a loaded record is removed from a record array when it is deleted", functi

scumbag.deleteRecord();

equal(get(recordArray, 'length'), 0, "record is removed from the record array");
equal(get(recordArray, 'length'), 1, "record is still in the record array until it is saved");

Ember.run(scumbag, 'save');

equal(get(recordArray, 'length'), 0, "record is removed from the array when it is saved");
});
});
});
Expand All @@ -135,15 +142,18 @@ test("a loaded record is removed from a record array when it is deleted even if

var env = setupStore({
tag: Tag,
person: Person
person: Person,
adapter: DS.Adapter.extend({
deleteRecord: () => Ember.RSVP.resolve()
})
});
var store = env.store;
var scumbag, tag;

run(function() {
scumbag = store.push('person', { id: 1, name: 'Scumbag Tom' });
tag = store.push('tag', { id: 1, people: [1] });
scumbag.deleteRecord();
scumbag.destroyRecord();
});

equal(tag.get('people.length'), 0, "record is removed from the record array");
Expand All @@ -161,7 +171,10 @@ test("a loaded record is removed both from the record array and from the belongs
var env = setupStore({
tag: Tag,
person: Person,
tool: Tool
tool: Tool,
adapter: DS.Adapter.extend({
deleteRecord: () => Ember.RSVP.resolve()
})
});
var store = env.store;
var scumbag, tag, tool;
Expand All @@ -176,7 +189,7 @@ test("a loaded record is removed both from the record array and from the belongs
equal(tool.get('person'), scumbag, "the tool belongs to the record");

run(function() {
scumbag.deleteRecord();
scumbag.destroyRecord();
});

equal(tag.get('people.length'), 0, "record is removed from the record array");
Expand Down

0 comments on commit a570eca

Please sign in to comment.