Merge pull request #2225 from tractorcow/3.1-hasmany-delete-fix

BUG Fixes issue with HasManyList::remove
This commit is contained in:
Sean Harvey 2013-10-31 15:52:07 -07:00
commit 1951fdba0f
3 changed files with 70 additions and 16 deletions

View File

@ -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 @@ class HasManyList extends RelationList {
/**
* 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 @@ class HasManyList extends RelationList {
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();
}
}

View File

@ -251,14 +251,14 @@ class DataObjectTest extends SapphireTest {
* - 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 @@ class DataObjectTest extends SapphireTest {
$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() {

View File

@ -16,5 +16,44 @@ class HasManyListTest extends SapphireTest {
$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);
}
}