Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX many_many through should allow subclasses #11230

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ORM/DataObjectSchema.php
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,7 @@ protected function checkManyManyFieldClass($parentClass, $component, $joinClass,
}

// Validate bad types on parent relation
if ($key === 'from' && $relationClass !== $parentClass) {
if ($key === 'from' && $relationClass !== $parentClass && !is_subclass_of($parentClass, $relationClass)) {
throw new InvalidArgumentException(
"many_many through relation {$parentClass}.{$component} {$key} references a field name "
. "{$joinClass}::{$relation} of type {$relationClass}; {$parentClass} expected"
Expand Down
70 changes: 54 additions & 16 deletions tests/php/ORM/ManyManyThroughListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ class ManyManyThroughListTest extends SapphireTest
ManyManyThroughListTest\Item::class,
ManyManyThroughListTest\JoinObject::class,
ManyManyThroughListTest\TestObject::class,
ManyManyThroughListTest\TestObjectSubclass::class,
ManyManyThroughListTest\PolyItem::class,
ManyManyThroughListTest\PolyJoinObject::class,
ManyManyThroughListTest\PolyObjectA::class,
ManyManyThroughListTest\PolyObjectB::class,
ManyManyThroughListTest\PseudoPolyJoinObject::class,
ManyManyThroughListTest\Locale::class,
ManyManyThroughListTest\FallbackLocale::class,
];
Expand Down Expand Up @@ -161,46 +163,82 @@ public function sortingProvider()
];
}

public function testAdd()
public function provideAdd(): array
{
/** @var ManyManyThroughListTest\TestObject $parent */
$parent = $this->objFromFixture(ManyManyThroughListTest\TestObject::class, 'parent1');
return [
[
'parentClass' => ManyManyThroughListTest\TestObject::class,
'joinClass' => ManyManyThroughListTest\JoinObject::class,
'joinProperty' => 'ManyManyThroughListTest_JoinObject',
'relation' => 'Items',
],
[
'parentClass' => ManyManyThroughListTest\TestObjectSubclass::class,
'joinClass' => ManyManyThroughListTest\PseudoPolyJoinObject::class,
'joinProperty' => 'ManyManyThroughListTest_PseudoPolyJoinObject',
'relation' => 'MoreItems',
],
];
}

/**
* @dataProvider provideAdd
*/
public function testAdd(string $parentClass, string $joinClass, string $joinProperty, string $relation)
{
$parent = $this->objFromFixture($parentClass, 'parent1');
$newItem = new ManyManyThroughListTest\Item();
$newItem->Title = 'my new item';
$newItem->write();
$parent->Items()->add($newItem, ['Title' => 'new join record']);
$parent->$relation()->add($newItem, ['Title' => 'new join record']);

// Check select
$newItem = $parent->Items()->filter(['Title' => 'my new item'])->first();
$newItem = $parent->$relation()->filter(['Title' => 'my new item'])->first();
$this->assertNotNull($newItem);
$this->assertEquals('my new item', $newItem->Title);
$this->assertInstanceOf(
ManyManyThroughListTest\JoinObject::class,
$joinClass,
$newItem->getJoin()
);
$this->assertInstanceOf(
ManyManyThroughListTest\JoinObject::class,
$newItem->ManyManyThroughListTest_JoinObject
$joinClass,
$newItem->$joinProperty
);
$this->assertEquals('new join record', $newItem->ManyManyThroughListTest_JoinObject->Title);
$this->assertEquals('new join record', $newItem->$joinProperty->Title);
}

public function testRemove()
public function provideRemove(): array
{
/** @var ManyManyThroughListTest\TestObject $parent */
$parent = $this->objFromFixture(ManyManyThroughListTest\TestObject::class, 'parent1');
return [
[
'parentClass' => ManyManyThroughListTest\TestObject::class,
'relation' => 'Items',
],
[
'parentClass' => ManyManyThroughListTest\TestObjectSubclass::class,
'relation' => 'MoreItems',
],
];
}

/**
* @dataProvider provideRemove
*/
public function testRemove(string $parentClass, string $relation)
{
$parent = $this->objFromFixture($parentClass, 'parent1');
$this->assertListEquals(
[
['Title' => 'item 1'],
['Title' => 'item 2']
],
$parent->Items()
$parent->$relation()
);
$item1 = $parent->Items()->filter(['Title' => 'item 1'])->first();
$parent->Items()->remove($item1);
$item1 = $parent->$relation()->filter(['Title' => 'item 1'])->first();
$parent->$relation()->remove($item1);
$this->assertListEquals(
[['Title' => 'item 2']],
$parent->Items()
$parent->$relation()
);
}

Expand Down
24 changes: 24 additions & 0 deletions tests/php/ORM/ManyManyThroughListTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObject:
Title: 'my object'
parent2:
Title: 'my object2'
SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObjectSubclass:
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:
Expand Down Expand Up @@ -30,6 +35,25 @@ SilverStripe\ORM\Tests\ManyManyThroughListTest\JoinObject:
Title: 'join 4'
Parent: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObject.parent2
Child: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Item.child2
SilverStripe\ORM\Tests\ManyManyThroughListTest\PseudoPolyJoinObject:
join1:
Title: 'join 1'
Sort: 4
Parent: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObjectSubclass.parent1
Child: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Item.child1
join2:
Title: 'join 2'
Sort: 2
Parent: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObjectSubclass.parent1
Child: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Item.child2
join3:
Title: 'join 3'
Parent: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObjectSubclass.parent2
Child: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Item.child1
join4:
Title: 'join 4'
Parent: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\TestObjectSubclass.parent2
Child: =>SilverStripe\ORM\Tests\ManyManyThroughListTest\Item.child2
SilverStripe\ORM\Tests\ManyManyThroughListTest\PolyObjectA:
obja1:
Title: 'object A1'
Expand Down
28 changes: 28 additions & 0 deletions tests/php/ORM/ManyManyThroughListTest/PseudoPolyJoinObject.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

namespace SilverStripe\ORM\Tests\ManyManyThroughListTest;

use SilverStripe\Dev\TestOnly;
use SilverStripe\ORM\DataObject;

/**
* @property string $Title
* @method TestObject Parent()
* @method Item Child()
*/
class PseudoPolyJoinObject extends DataObject implements TestOnly
{
private static $table_name = 'ManyManyThroughListTest_PseudoPolyJoinObject';

private static $db = [
'Title' => 'Varchar',
'Sort' => 'Int',
];

private static $has_one = [
'Parent' => TestObject::class,
'Child' => Item::class,
];

private static $default_sort = '"Sort" ASC';
}
30 changes: 30 additions & 0 deletions tests/php/ORM/ManyManyThroughListTest/TestObjectSubclass.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

namespace SilverStripe\ORM\Tests\ManyManyThroughListTest;

use SilverStripe\Dev\TestOnly;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\ManyManyThroughList;

/**
* Basic parent object
*
* @property string $Title
* @method ManyManyThroughList Items()
*/
class TestObjectSubclass extends TestObject implements TestOnly
{
private static $table_name = 'ManyManyThroughListTest_TestObjectSubclass';

private static $db = [
'Title' => 'Varchar'
];

private static $many_many = [
'MoreItems' => [
'through' => PseudoPolyJoinObject::class,
'from' => 'Parent',
'to' => 'Child',
]
];
}