Skip to content

Commit

Permalink
Rebase and test cleanup for @drogus work on keeping records after del…
Browse files Browse the repository at this point in the history
…eteRecord

- Removed disconnectRelationship method calls from model/states.js
- Removed reconnectRelationships from modle/internal-model.js
- Remove one-sided disconnect and reconnect relationship methods
- Removed method definitions in internal-model for:
  - disconnectRelationships
  - reconnectRelationships
- Removed method definitions in relationship for:
  - disconnect
  - reconnect
  - addRecordToInverse
- Fixed incorrect tests
  • Loading branch information
bmac committed Jul 14, 2015
1 parent a570eca commit 89bf57a
Show file tree
Hide file tree
Showing 10 changed files with 19 additions and 396 deletions.
19 changes: 0 additions & 19 deletions packages/ember-data/lib/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ InternalModel.prototype = {
if (this.isDeleted()) {
//TODO: Should probably move this to the state machine somehow
this.becameReady();
this.reconnectRelationships();
}

if (this.isNew()) {
Expand Down Expand Up @@ -449,7 +448,6 @@ InternalModel.prototype = {
this.eachRelationship((name, relationship) => {
if (this._relationships.has(name)) {
var rel = this._relationships.get(name);
//TODO(Igor) figure out whether we want to clear or disconnect
rel.clear();
rel.destroy();
}
Expand All @@ -460,23 +458,6 @@ InternalModel.prototype = {
});
},

disconnectRelationships: function() {
this.eachRelationship((name, relationship) => {
this._relationships.get(name).disconnect();
});
Object.keys(this._implicitRelationships).forEach((key) => {
this._implicitRelationships[key].disconnect();
});
},

reconnectRelationships: function() {
this.eachRelationship((name, relationship) => {
this._relationships.get(name).reconnect();
});
Object.keys(this._implicitRelationships).forEach((key) => this._implicitRelationships[key].reconnect());
},


/**
When a find request is triggered on the store, the user can optionally pass in
attributes and relationships to be preloaded. These are meant to behave as if they
Expand Down
6 changes: 1 addition & 5 deletions packages/ember-data/lib/system/model/states.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,9 @@ 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 @@ -411,14 +409,12 @@ var updatedState = dirtyState({
});

createdState.uncommitted.deleteRecord = function(internalModel) {
internalModel.disconnectRelationships();
internalModel.transitionTo('deleted.saved');
internalModel.send('invokeLifecycleCallbacks');
};

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

Expand Down Expand Up @@ -661,7 +657,6 @@ 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 All @@ -686,6 +681,7 @@ var RootState = {
isDirty: false,

setup: function(internalModel) {
internalModel.clearRelationships();
var store = internalModel.store;
store._dematerializeRecord(internalModel);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,10 @@ ManyRelationship.prototype.findRecords = function() {
//TODO CLEANUP
return this.store.findMany(this.manyArray.toArray().map((rec) => rec._internalModel)).
then(() => {
//Goes away after the manyArray refactor
this.manyArray.set('isLoaded', true);
if (!this.manyArray.get('isDestroyed')) {
//Goes away after the manyArray refactor
this.manyArray.set('isLoaded', true);
}
return this.manyArray;
});
};
Expand Down
14 changes: 0 additions & 14 deletions packages/ember-data/lib/system/relationships/state/relationship.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,6 @@ Relationship.prototype = {
}
},

disconnect: function() {
this.members.forEach((member) => this.removeRecordFromInverse(member));
},

reconnect: function() {
this.members.forEach((member) => this.addRecordToInverse(member));
},

removeRecords: function(records) {
records.forEach((record) => this.removeRecord(record));
},
Expand Down Expand Up @@ -138,12 +130,6 @@ Relationship.prototype = {
}
},

addRecordToInverse: function(record) {
if (this.inverseKey) {
record._relationships.get(this.inverseKey).addRecord(this.record);
}
},

removeRecordFromInverse: function(record) {
var inverseRelationship = record._relationships.get(this.inverseKey);
//Need to check for existence, as the record might unloading at the moment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ test("records should not be removed from record arrays just after deleting, but


// pre-condition
equal(all.get('length'), 2, 'expected 2 records');
equal(all.get('length'), 2, 'pre-condition: 2 records in array');

Ember.run(adam, 'deleteRecord');

equal(all.get('length'), 2, 'expected 2 record');
equal(all.get('length'), 2, '2 records in array after deleteRecord');

Ember.run(adam, 'save');

equal(all.get('length'), 1, 'expected 1 record');
equal(all.get('length'), 1, '1 record in array after deleteRecord and save');
});

test("records can be deleted during record array enumeration", function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,7 @@ test("adding and removing records from hasMany relationship #2666", function() {
return comments.get('lastObject').destroyRecord();
}).then(function() {
var comments = post.get('comments');
equal(comments.get('length'), 3, "Comments count after delete");
equal(comments.get('length'), 3, "Comments count after destroy");

// Add another comment #4
var comment = env.store.createRecord('comment');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,89 +383,6 @@ test("Removing a record from a hasMany reflects on the other hasMany side - sync
equal(account.get('users.length'), 0, 'Users were removed correctly');
});

/*
Deleting tests
*/

test("Deleting a record that has a hasMany relationship removes it from the otherMany array but does not remove the other record from itself - async", function () {
var user, topic;
run(function() {
user = store.push({
data: {
id: '1',
type: 'user',
attributes: {
name: 'Stanley'
},
relationships: {
topics: {
data: [{
id: '2',
type: 'topic'
}]
}
}
}
});
topic = store.push({
data: {
id: '2',
type: 'topic',
attributes: {
title: 'EmberFest was great'
}
}
});
});

run(topic, 'destroyRecord');
run(function() {
topic.get('users').then(async(function(fetchedUsers) {
equal(fetchedUsers.get('length'), 1, 'Users are still there');
}));
user.get('topics').then(async(function(fetchedTopics) {
equal(fetchedTopics.get('length'), 0, 'Topic got removed from the user');
equal(fetchedTopics.objectAt(0), null, "Topic can't be fetched");
}));
});
});

test("Deleting a record that has a hasMany relationship removes it from the otherMany array but does not remove the other record from itself - sync", function () {
var account, user;
run(function() {
account = store.push({
data: {
id: '2',
type: 'account',
attributes: {
state: 'lonely'
}
}
});
user = store.push({
data: {
id: '1',
type: 'user',
attributes: {
name: 'Stanley'
},
relationships: {
accounts: {
data: [{
id: '2',
type: 'account'
}]
}
}
}
});
});

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');
});

/*
Rollback Attributes tests
*/
Expand Down
Loading

0 comments on commit 89bf57a

Please sign in to comment.