Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

BUG Fixes issue where items could be deleted from a has_many relation…

… by an entirely unrelated HasManyList calling delete on that item.
  • Loading branch information...
commit c74f7e764003f7b9cc2087d49f63dd73ca40ac4f 1 parent c547e42
Damian Mooyman tractorcow authored
21 model/HasManyList.php
View
@@ -16,9 +16,8 @@ class HasManyList extends RelationList {
* {@link DataList} methods. Addition arguments are used to support {@@link add()}
* and {@link remove()} methods.
*
- * @param $dataClass The class of the DataObjects that this will list.
- * @param $relationFilters A map of key => value filters that define which records
- * in the $dataClass table actually belong to this relationship.
+ * @param string $dataClass The class of the DataObjects that this will list.
+ * @param string $foreignKey The name of the foreign key field to set the ID filter against.
*/
public function __construct($dataClass, $foreignKey) {
parent::__construct($dataClass);
@@ -96,6 +95,7 @@ public function removeByID($itemID) {
/**
* Remove an item from this relation.
* Doesn't actually remove the item, it just clears the foreign key value.
+ *
* @param $item The DataObject to be removed
* @todo Maybe we should delete the object instead?
*/
@@ -105,10 +105,17 @@ public function remove($item) {
E_USER_ERROR);
}
- $fk = $this->foreignKey;
- $item->$fk = null;
+ // Don't remove item which doesn't belong to this list
+ $foreignID = $this->getForeignID();
+ $foreignKey = $this->getForeignKey();
+
+ if( empty($foreignID)
+ || (is_array($foreignID) && in_array($item->$foreignKey, $foreignID))
+ || $foreignID == $item->$foreignKey
+ ) {
+ $item->$foreignKey = null;
+ $item->write();
+ }
- $item->write();
}
-
}
26 tests/model/DataObjectTest.php
View
@@ -251,14 +251,14 @@ public function testWritePropertyWithoutDBField() {
* - Test the IDs on the DataObjects are set correctly
*/
public function testHasManyRelationships() {
- $team = $this->objFromFixture('DataObjectTest_Team', 'team1');
+ $team1 = $this->objFromFixture('DataObjectTest_Team', 'team1');
// Test getComponents() gets the ComponentSet of the other side of the relation
- $this->assertTrue($team->Comments()->Count() == 2);
+ $this->assertTrue($team1->Comments()->Count() == 2);
// Test the IDs on the DataObjects are set correctly
- foreach($team->Comments() as $comment) {
- $this->assertEquals($team->ID, $comment->TeamID);
+ foreach($team1->Comments() as $comment) {
+ $this->assertEquals($team1->ID, $comment->TeamID);
}
// Test that we can add and remove items that already exist in the database
@@ -266,15 +266,23 @@ public function testHasManyRelationships() {
$newComment->Name = "Automated commenter";
$newComment->Comment = "This is a new comment";
$newComment->write();
- $team->Comments()->add($newComment);
- $this->assertEquals($team->ID, $newComment->TeamID);
+ $team1->Comments()->add($newComment);
+ $this->assertEquals($team1->ID, $newComment->TeamID);
$comment1 = $this->objFromFixture('DataObjectTest_TeamComment', 'comment1');
$comment2 = $this->objFromFixture('DataObjectTest_TeamComment', 'comment2');
- $team->Comments()->remove($comment2);
+ $team1->Comments()->remove($comment2);
- $commentIDs = $team->Comments()->sort('ID')->column('ID');
- $this->assertEquals(array($comment1->ID, $newComment->ID), $commentIDs);
+ $team1CommentIDs = $team1->Comments()->sort('ID')->column('ID');
+ $this->assertEquals(array($comment1->ID, $newComment->ID), $team1CommentIDs);
+
+ // Test that removing an item from a list doesn't remove it from the same
+ // relation belonging to a different object
+ $team1 = $this->objFromFixture('DataObjectTest_Team', 'team1');
+ $team2 = $this->objFromFixture('DataObjectTest_Team', 'team2');
+ $team2->Comments()->remove($comment1);
+ $team1CommentIDs = $team1->Comments()->sort('ID')->column('ID');
+ $this->assertEquals(array($comment1->ID, $newComment->ID), $team1CommentIDs);
}
public function testHasOneRelationship() {
39 tests/model/HasManyListTest.php
View
@@ -16,5 +16,44 @@ public function testRelationshipEmptyOnNewRecords() {
$newTeam = new DataObjectTest_Team(); // has_many Comments
$this->assertEquals(array(), $newTeam->Comments()->column('ID'));
}
+
+ /**
+ * Test that related objects can be removed from a relation
+ */
+ public function testRemoveRelation() {
+
+ // Check that expected teams exist
+ $list = DataObjectTest_Team::get();
+ $this->assertEquals(
+ array('Subteam 1', 'Subteam 2', 'Subteam 3', 'Team 1', 'Team 2', 'Team 3'),
+ $list->sort('Title')->column('Title')
+ );
+
+ // Test that each team has the correct fans
+ $team1 = $this->objFromFixture('DataObjectTest_Team', 'team1');
+ $team2 = $this->objFromFixture('DataObjectTest_Team', 'team2');
+ $this->assertEquals(array('Bob', 'Joe'), $team1->Comments()->sort('Name')->column('Name'));
+ $this->assertEquals(array('Phil'), $team2->Comments()->sort('Name')->column('Name'));
+
+ // Test that removing comments from unrelated team has no effect
+ $team1comment = $this->objFromFixture('DataObjectTest_TeamComment', 'comment1');
+ $team2comment = $this->objFromFixture('DataObjectTest_TeamComment', 'comment3');
+ $team1->Comments()->remove($team2comment);
+ $team2->Comments()->remove($team1comment);
+ $this->assertEquals(array('Bob', 'Joe'), $team1->Comments()->sort('Name')->column('Name'));
+ $this->assertEquals(array('Phil'), $team2->Comments()->sort('Name')->column('Name'));
+ $this->assertEquals($team1->ID, $team1comment->TeamID);
+ $this->assertEquals($team2->ID, $team2comment->TeamID);
+
+ // Test that removing items from the related team resets the has_one relations on the fan
+ $team1comment = $this->objFromFixture('DataObjectTest_TeamComment', 'comment1');
+ $team2comment = $this->objFromFixture('DataObjectTest_TeamComment', 'comment3');
+ $team1->Comments()->remove($team1comment);
+ $team2->Comments()->remove($team2comment);
+ $this->assertEquals(array('Bob'), $team1->Comments()->sort('Name')->column('Name'));
+ $this->assertEquals(array(), $team2->Comments()->sort('Name')->column('Name'));
+ $this->assertEmpty($team1comment->TeamID);
+ $this->assertEmpty($team2comment->TeamID);
+ }
}
Please sign in to comment.
Something went wrong with that request. Please try again.