Skip to content

Commit

Permalink
FIX many_many through should allow subclasses (#11230)
Browse files Browse the repository at this point in the history
```php
class HomePage extends Page
{
    private static $many_many = [
        'HeroImages' => [
            'through' => PageImageLink::class,
            'from' => 'Page',
            'to' => 'Image',
        ]
    ];

}
```

```php
class PageImageLink extends DataObject
{
    private static $has_one = [
        'Page' => SiteTree::class,
        'Image' => Image::class,
    ];
}

This fails because the linking object's relation class doesn't exactly match the owner. Sharing the linking objects across various entries in the ancestry should be a supported use case.

Co-authored-by: Aaron Carlino <unclecheese@leftandmain.com>
  • Loading branch information
GuySartorelli and Aaron Carlino authored May 13, 2024
1 parent 0f6d210 commit 50a0018
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 17 deletions.
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',
]
];
}

0 comments on commit 50a0018

Please sign in to comment.