diff --git a/src/ORM/ManyManyList.php b/src/ORM/ManyManyList.php index 5558ba076..9ba18e5ff 100644 --- a/src/ORM/ManyManyList.php +++ b/src/ORM/ManyManyList.php @@ -389,7 +389,6 @@ class ManyManyList extends RelationList */ public function removeAll() { - // Remove the join to the join table to avoid MySQL row locking issues. $query = $this->dataQuery(); $foreignFilter = $query->getQueryParam('Foreign.Filter'); diff --git a/src/ORM/ManyManyThroughList.php b/src/ORM/ManyManyThroughList.php index 3dbfd3f58..6bd5050a0 100644 --- a/src/ORM/ManyManyThroughList.php +++ b/src/ORM/ManyManyThroughList.php @@ -130,27 +130,37 @@ class ManyManyThroughList extends RelationList // Find has_many row with a local key matching the given id $hasManyList = $this->manipulator->getParentRelationship($this->dataQuery()); $records = $hasManyList->filter($this->manipulator->getLocalKey(), $itemID); - $affectedIds = []; // Rather than simple un-associating the record (as in has_many list) // Delete the actual mapping row as many_many deletions behave. /** @var DataObject $record */ foreach ($records as $record) { - $affectedIds[] = $record->ID; $record->delete(); } - if ($this->removeCallbacks && $affectedIds) { - $this->removeCallbacks->call($this, $affectedIds); + if ($this->removeCallbacks && $itemID) { + $this->removeCallbacks->call($this, [$itemID]); } } public function removeAll() { - // Empty has_many table matching the current foreign key - $hasManyList = $this->manipulator->getParentRelationship($this->dataQuery()); - $affectedIds = $hasManyList->column('ID'); - $hasManyList->removeAll(); + // Get the IDs of records in the current list + $affectedIds = $this->limit(null)->column('ID'); + if (empty($affectedIds)) { + return; + } + + // Get the join records that apply for the current list + $records = $this->manipulator->getJoinClass()::get()->filter([ + $this->manipulator->getForeignIDKey() => $this->getForeignID(), + $this->manipulator->getLocalKey() => $affectedIds, + ]); + + /** @var DataObject $record */ + foreach ($records as $record) { + $record->delete(); + } if ($this->removeCallbacks && $affectedIds) { $this->removeCallbacks->call($this, $affectedIds); diff --git a/src/ORM/ManyManyThroughQueryManipulator.php b/src/ORM/ManyManyThroughQueryManipulator.php index e44d2dc9d..06a422f12 100644 --- a/src/ORM/ManyManyThroughQueryManipulator.php +++ b/src/ORM/ManyManyThroughQueryManipulator.php @@ -3,7 +3,6 @@ namespace SilverStripe\ORM; -use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\Deprecation; diff --git a/tests/php/ORM/ManyManyThroughListTest.php b/tests/php/ORM/ManyManyThroughListTest.php index 905cc814b..2d9cb8908 100644 --- a/tests/php/ORM/ManyManyThroughListTest.php +++ b/tests/php/ORM/ManyManyThroughListTest.php @@ -203,6 +203,64 @@ class ManyManyThroughListTest extends SapphireTest ); } + public function testRemoveAll() + { + $first = $this->objFromFixture(ManyManyThroughListTest\TestObject::class, 'parent1'); + $first->Items()->add($this->objFromFixture(ManyManyThroughListTest\Item::class, 'child0')); + $second = $this->objFromFixture(ManyManyThroughListTest\TestObject::class, 'parent2'); + + $firstItems = $first->Items(); + $secondItems = $second->Items(); + $initialJoins = ManyManyThroughListTest\JoinObject::get()->count(); + $initialItems = ManyManyThroughListTest\Item::get()->count(); + $initialRelations = $firstItems->count(); + $initialSecondListRelations = $secondItems->count(); + + $firstItems->removeAll(); + + // Validate all items were removed from the first list, but none were removed from the second list + $this->assertEquals(0, count($firstItems)); + $this->assertEquals($initialSecondListRelations, count($secondItems)); + + // Validate that the JoinObjects were actually removed from the database + $this->assertEquals($initialJoins - $initialRelations, ManyManyThroughListTest\JoinObject::get()->count()); + + // Confirm Item objects were not removed from the database + $this->assertEquals($initialItems, ManyManyThroughListTest\Item::get()->count()); + } + + public function testRemoveAllIgnoresLimit() + { + $parent = $this->objFromFixture(ManyManyThroughListTest\TestObject::class, 'parent1'); + $parent->Items()->add($this->objFromFixture(ManyManyThroughListTest\Item::class, 'child0')); + $initialJoins = ManyManyThroughListTest\JoinObject::get()->count(); + // Validate there are enough items in the relation for this test + $this->assertTrue($initialJoins > 1); + + $items = $parent->Items()->Limit(1); + $items->removeAll(); + + // Validate all items were removed from the list - not only one + $this->assertEquals(0, count($items)); + } + + public function testFilteredRemoveAll() + { + $parent = $this->objFromFixture(ManyManyThroughListTest\TestObject::class, 'parent1'); + $parent->Items()->add($this->objFromFixture(ManyManyThroughListTest\Item::class, 'child0')); + $items = $parent->Items(); + $initialJoins = ManyManyThroughListTest\JoinObject::get()->count(); + $initialRelations = $items->count(); + + $items->filter('Title:not', 'not filtered')->removeAll(); + + // Validate only the filtered items were removed + $this->assertEquals(1, $items->count()); + + // Validate the list only contains the correct remaining item + $this->assertEquals(['not filtered'], $items->column('Title')); + } + /** * Test validation */ diff --git a/tests/php/ORM/ManyManyThroughListTest.yml b/tests/php/ORM/ManyManyThroughListTest.yml index c7c3c3c2a..f1549fe89 100644 --- a/tests/php/ORM/ManyManyThroughListTest.yml +++ b/tests/php/ORM/ManyManyThroughListTest.yml @@ -1,7 +1,12 @@ SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObject: parent1: Title: 'my object' + parent2: + Title: 'my object2' SilverStripe\ORM\Tests\ManyManyThroughListTest\Item: + # Having this one first means the IDs of records aren't the same as the IDs of the join objects. + child0: + Title: 'not filtered' child1: Title: 'item 1' child2: @@ -17,6 +22,14 @@ SilverStripe\ORM\Tests\ManyManyThroughListTest\JoinObject: Sort: 2 Parent: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObject.parent1 Child: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Item.child2 + join3: + Title: 'join 3' + Parent: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObject.parent2 + Child: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Item.child1 + join4: + Title: 'join 4' + Parent: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObject.parent2 + Child: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Item.child2 SilverStripe\ORM\Tests\ManyManyThroughListTest\PolyObjectA: obja1: Title: 'object A1' @@ -75,4 +88,4 @@ SilverStripe\ORM\Tests\ManyManyThroughListTest\FallbackLocale: mexico_argentina: Sort: 1 Parent: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Locale.mexico - Locale: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Locale.argentina \ No newline at end of file + Locale: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Locale.argentina