diff --git a/_config/model.yml b/_config/model.yml index 9520d393389..cfb60c02aa6 100644 --- a/_config/model.yml +++ b/_config/model.yml @@ -48,6 +48,8 @@ SilverStripe\Core\Injector\Injector: class: SilverStripe\ORM\FieldType\DBPercentage PolymorphicForeignKey: class: SilverStripe\ORM\FieldType\DBPolymorphicForeignKey + PolymorphicRelationAwareForeignKey: + class: SilverStripe\ORM\FieldType\DBPolymorphicRelationAwareForeignKey PrimaryKey: class: SilverStripe\ORM\FieldType\DBPrimaryKey Text: diff --git a/src/Dev/Validation/RelationValidationService.php b/src/Dev/Validation/RelationValidationService.php index 5fc4a75e357..e19742b1276 100644 --- a/src/Dev/Validation/RelationValidationService.php +++ b/src/Dev/Validation/RelationValidationService.php @@ -2,12 +2,14 @@ namespace SilverStripe\Dev\Validation; +use InvalidArgumentException; use ReflectionException; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Resettable; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\DB; /** @@ -291,6 +293,26 @@ protected function validateHasOne(string $class): void $relations = (array) $singleton->config()->uninherited('has_one'); foreach ($relations as $relationName => $relationData) { + if (is_array($relationData)) { + $spec = $relationData; + if (!isset($spec['class'])) { + $this->logError($class, $relationName, 'No class has been defined for this relation.'); + continue; + } + $relationData = $spec['class']; + if (isset($spec[DataObjectSchema::HASONE_MULTIPLE_HASMANY]) + && $spec[DataObjectSchema::HASONE_MULTIPLE_HASMANY] === true + && $relationData !== DataObject::class + ) { + $this->logError( + $class, + $relationName, + 'has_one relation that can handle multiple reciprocal has_many relations must be polymorphic.' + ); + continue; + } + } + if ($this->isIgnored($class, $relationName)) { continue; } @@ -305,6 +327,11 @@ protected function validateHasOne(string $class): void return; } + // Skip checking for back relations when has_one is polymorphic + if ($relationData === DataObject::class) { + continue; + } + if (!is_subclass_of($relationData, DataObject::class)) { $this->logError( $class, @@ -616,7 +643,8 @@ protected function parseManyManyRelation($relationData): ?string return null; } - return $throughRelations[$to]; + $spec = $throughRelations[$to]; + return is_array($spec) ? $spec['class'] ?? null : $spec; } return $relationData; diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php index e3b5b8c3b97..8804c69eea5 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -204,6 +204,14 @@ public function ItemEditForm() $classKey = $list->getForeignClassKey(); $class = $list->getForeignClass(); $this->record->$classKey = $class; + + // If the has_one relation storing the data can handle multiple reciprocal has_many relations, + // make sure we tell it which has_many relation this belongs to. + $relation = $list->getForeignRelation(); + if ($relation) { + $relationKey = $list->getForeignRelationKey(); + $this->record->$relationKey = $relation; + } } } diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 3e978eb05f0..0f6fa5967b0 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -12,6 +12,7 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Resettable; use SilverStripe\Dev\Debug; +use SilverStripe\Dev\Deprecation; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormField; use SilverStripe\Forms\FormScaffolder; @@ -1972,12 +1973,14 @@ public function getComponents($componentName, $id = null) } // Determine type and nature of foreign relation - $joinField = $schema->getRemoteJoinField(static::class, $componentName, 'has_many', $polymorphic); - /** @var HasManyList $result */ - if ($polymorphic) { - $result = PolymorphicHasManyList::create($componentClass, $joinField, static::class); + $details = $schema->getHasManyComponentDetails(static::class, $componentName); + if ($details['polymorphic']) { + $result = PolymorphicHasManyList::create($componentClass, $details['joinField'], static::class); + if ($details['needsRelation']) { + Deprecation::withNoReplacement(fn () =>$result->setForeignRelation($componentName)); + } } else { - $result = HasManyList::create($componentClass, $joinField); + $result = HasManyList::create($componentClass, $details['joinField']); } return $result @@ -1993,16 +1996,21 @@ public function getComponents($componentName, $id = null) */ public function getRelationClass($relationName) { - // Parse many_many - $manyManyComponent = $this->getSchema()->manyManyComponent(static::class, $relationName); + // Parse many_many, which can have an array instead of a class name + $manyManyComponent = static::getSchema()->manyManyComponent(static::class, $relationName); if ($manyManyComponent) { return $manyManyComponent['childClass']; } - // Go through all relationship configuration fields. + // Parse has_one, which can have an array instead of a class name + $hasOneComponent = static::getSchema()->hasOneComponent(static::class, $relationName); + if ($hasOneComponent) { + return $hasOneComponent; + } + + // Go through all remaining relationship configuration fields. $config = $this->config(); $candidates = array_merge( - ($relations = $config->get('has_one')) ? $relations : [], ($relations = $config->get('has_many')) ? $relations : [], ($relations = $config->get('belongs_to')) ? $relations : [] ); @@ -2228,15 +2236,20 @@ public function getManyManyComponents($componentName, $id = null) } /** - * Return the class of a one-to-one component. If $component is null, return all of the one-to-one components and - * their classes. If the selected has_one is a polymorphic field then 'DataObject' will be returned for the type. + * Return the class of a all has_one relations. * - * @return string|array The class of the one-to-one component, or an array of all one-to-one components and - * their classes. + * @return array An array of all has_one components and their classes. */ public function hasOne() { - return (array)$this->config()->get('has_one'); + $hasOne = (array) $this->config()->get('has_one'); + // Boil down has_one spec to just the class name + foreach ($hasOne as $relationName => $spec) { + if (is_array($spec)) { + $hasOne[$relationName] = DataObject::getSchema()->hasOneComponent(static::class, $relationName); + } + } + return $hasOne; } /** diff --git a/src/ORM/DataObjectSchema.php b/src/ORM/DataObjectSchema.php index 09e15149aac..5fc48fb2a56 100644 --- a/src/ORM/DataObjectSchema.php +++ b/src/ORM/DataObjectSchema.php @@ -24,6 +24,11 @@ class DataObjectSchema use Injectable; use Configurable; + /** + * Configuration key for has_one relations that can support multiple reciprocal has_many relations. + */ + public const HASONE_MULTIPLE_HASMANY = 'handles_multiple_has_many'; + /** * Default separate for table namespaces. Can be set to any string for * databases that do not support some characters. @@ -501,7 +506,20 @@ protected function cacheDatabaseFields($class) // Add in all has_ones $hasOne = Config::inst()->get($class, 'has_one', Config::UNINHERITED) ?: []; - foreach ($hasOne as $fieldName => $hasOneClass) { + foreach ($hasOne as $fieldName => $spec) { + if (is_array($spec)) { + if (!isset($spec['class'])) { + throw new LogicException("has_one relation {$class}.{$fieldName} must declare a class"); + } + // Handle has_one which handles multiple reciprocal has_many relations + $hasOneClass = $spec['class']; + if (isset($spec[self::HASONE_MULTIPLE_HASMANY]) && $spec[self::HASONE_MULTIPLE_HASMANY] === true) { + $compositeFields[$fieldName] = 'PolymorphicRelationAwareForeignKey'; + continue; + } + } else { + $hasOneClass = $spec; + } if ($hasOneClass === DataObject::class) { $compositeFields[$fieldName] = 'PolymorphicForeignKey'; } else { @@ -902,12 +920,36 @@ public function hasOneComponent($class, $component) return null; } + $spec = $hasOnes[$component]; + // Validate - $relationClass = $hasOnes[$component]; + if (is_array($spec)) { + $this->checkHasOneArraySpec($class, $component, $spec); + } + $relationClass = is_array($spec) ? $spec['class'] : $spec; $this->checkRelationClass($class, $component, $relationClass, 'has_one'); + return $relationClass; } + /** + * Check if a has_one relation handles multiple reciprocal has_many relations. + * + * @return bool True if the relation exists and handles multiple reciprocal has_many relations. + */ + public function hasOneComponentHandlesMultipleHasMany(string $class, string $component): bool + { + $hasOnes = Config::forClass($class)->get('has_one'); + if (!isset($hasOnes[$component])) { + return false; + } + + $spec = $hasOnes[$component]; + return is_array($spec) + && isset($spec[self::HASONE_MULTIPLE_HASMANY]) + && $spec[self::HASONE_MULTIPLE_HASMANY] === true; + } + /** * Return data for a specific belongs_to component. * @@ -1046,6 +1088,16 @@ protected function getManyManyInverseRelationship($childClass, $parentClass) * @throws Exception */ public function getRemoteJoinField($class, $component, $type = 'has_many', &$polymorphic = false) + { + return $this->getBelongsAndHasManyDetails($class, $component, $type, $polymorphic)['joinField']; + } + + public function getHasManyComponentDetails(string $class, string $component) + { + return $this->getBelongsAndHasManyDetails($class, $component); + } + + private function getBelongsAndHasManyDetails(string $class, string $component, string $type = 'has_many', &$polymorphic = false) { // Extract relation from current object if ($type === 'has_many') { @@ -1071,6 +1123,11 @@ public function getRemoteJoinField($class, $component, $type = 'has_many', &$pol // Reference remote has_one to check against $remoteRelations = Config::inst()->get($remoteClass, 'has_one'); + foreach ($remoteRelations as $key => $value) { + if (is_array($value)) { + $remoteRelations[$key] = $this->hasOneComponent($remoteClass, $key); + } + } // Without an explicit field name, attempt to match the first remote field // with the same type as the current class @@ -1104,14 +1161,23 @@ public function getRemoteJoinField($class, $component, $type = 'has_many', &$pol on class {$class}"); } - // Inspect resulting found relation - if ($remoteRelations[$remoteField] === DataObject::class) { - $polymorphic = true; - return $remoteField; // Composite polymorphic field does not include 'ID' suffix - } else { - $polymorphic = false; - return $remoteField . 'ID'; + $polymorphic = $this->hasOneComponent($remoteClass, $remoteField) === DataObject::class; + $remoteClassField = $polymorphic ? $remoteField . 'Class' : null; + $needsRelation = $type === 'has_many' && $polymorphic && $this->hasOneComponentHandlesMultipleHasMany($remoteClass, $remoteField); + $remoteRelationField = $needsRelation ? $remoteField . 'Relation' : null; + + // This must be after the above assignments, as they rely on the original value. + if (!$polymorphic) { + $remoteField .= 'ID'; } + + return [ + 'joinField' => $remoteField, + 'relationField' => $remoteRelationField, + 'classField' => $remoteClassField, + 'polymorphic' => $polymorphic, + 'needsRelation' => $needsRelation, + ]; } /** @@ -1204,6 +1270,25 @@ protected function checkManyManyJoinClass($parentClass, $component, $specificati return $joinClass; } + protected function checkHasOneArraySpec(string $class, string $component, array $spec): void + { + if (!array_key_exists('class', $spec)) { + throw new InvalidArgumentException( + "has_one relation {$class}.{$component} doesn't define a class for the relation" + ); + } + + if (isset($spec[self::HASONE_MULTIPLE_HASMANY]) + && $spec[self::HASONE_MULTIPLE_HASMANY] === true + && $spec['class'] !== DataObject::class + ) { + throw new InvalidArgumentException( + "has_one relation {$class}.{$component} must be polymorphic, or not support multiple" + . 'reciprocal has_many relations' + ); + } + } + /** * Validate a given class is valid for a relation * diff --git a/src/ORM/DataQuery.php b/src/ORM/DataQuery.php index 327a0c4ea19..05692c3f720 100644 --- a/src/ORM/DataQuery.php +++ b/src/ORM/DataQuery.php @@ -1025,8 +1025,6 @@ public function applyRelation($relation, $linearOnly = false) * Join the given has_many relation to this query. * Also works with belongs_to * - * Doesn't work with polymorphic relationships - * * @param string $localClass Name of class that has the has_many to the joined class * @param string $localField Name of the has_many relationship to join * @param string $foreignClass Class to join @@ -1065,6 +1063,15 @@ protected function joinHasManyRelation( $localClassColumn = $schema->sqlColumnForField($localClass, 'ClassName', $localPrefix); $joinExpression = "{$foreignKeyIDColumn} = {$localIDColumn} AND {$foreignKeyClassColumn} = {$localClassColumn}"; + + // Add relation key if the has_many points to a has_one that could handle multiple reciprocal has_many relations + if ($type === 'has_many') { + $details = $schema->getHasManyComponentDetails($localClass, $localField); + if ($details['needsRelation']) { + $foreignKeyRelationColumn = $schema->sqlColumnForField($foreignClass, "{$foreignKey}Relation", $foreignPrefix); + $joinExpression .= " AND {$foreignKeyRelationColumn} = {$localField}"; + } + } } else { $foreignKeyIDColumn = $schema->sqlColumnForField($foreignClass, $foreignKey, $foreignPrefix); $joinExpression = "{$foreignKeyIDColumn} = {$localIDColumn}"; diff --git a/src/ORM/FieldType/DBPolymorphicRelationAwareForeignKey.php b/src/ORM/FieldType/DBPolymorphicRelationAwareForeignKey.php new file mode 100644 index 00000000000..524d63cbb16 --- /dev/null +++ b/src/ORM/FieldType/DBPolymorphicRelationAwareForeignKey.php @@ -0,0 +1,38 @@ + 'Varchar', + ]; + + /** + * Get the value of the "Relation" this key points to + * + * @return string Name of the has_many relation being stored + */ + public function getRelationValue(): string + { + return $this->getField('Relation'); + } + + /** + * Set the value of the "Relation" this key points to + * + * @param string $value Name of the has_many relation being stored + * @param bool $markChanged Mark this field as changed? + */ + public function setRelationValue(string $value, bool $markChanged = true): static + { + $this->setField('Relation', $value, $markChanged); + return $this; + } +} diff --git a/src/ORM/PolymorphicHasManyList.php b/src/ORM/PolymorphicHasManyList.php index 499d8e00355..46849ef0258 100644 --- a/src/ORM/PolymorphicHasManyList.php +++ b/src/ORM/PolymorphicHasManyList.php @@ -5,9 +5,11 @@ use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Convert; use InvalidArgumentException; +use SilverStripe\Dev\Deprecation; +use Traversable; /** - * Represents a has_many list linked against a polymorphic relationship + * Represents a has_many list linked against a polymorphic relationship. */ class PolymorphicHasManyList extends HasManyList { @@ -20,7 +22,13 @@ class PolymorphicHasManyList extends HasManyList protected $classForeignKey; /** - * Retrieve the name of the class this relation is filtered by + * Name of the foreign key field that references the relation name, for has_one + * relations that can handle multiple reciprocal has_many relations. + */ + protected string $relationForeignKey; + + /** + * Retrieve the name of the class this (has_many) relation is filtered by * * @return string */ @@ -30,19 +38,51 @@ public function getForeignClass() } /** - * Gets the field name which holds the related object class. + * Retrieve the name of the has_many relation this list is filtered by + */ + public function getForeignRelation(): ?string + { + return $this->dataQuery->getQueryParam('Foreign.Relation'); + } + + /** + * Retrieve the name of the has_many relation this list is filtered by + * + * @deprecated 5.2.0 Will be replaced with a parameter in the constructor + */ + public function setForeignRelation(string $relationName): static + { + Deprecation::notice('5.2.0', 'Will be replaced with a parameter in the constructor'); + $this->dataQuery->where(["\"{$this->relationForeignKey}\"" => $relationName]); + $this->dataQuery->setQueryParam('Foreign.Relation', $relationName); + return $this; + } + + /** + * Gets the field name which holds the related (has_many) object class. */ public function getForeignClassKey(): string { return $this->classForeignKey; } + /** + * Gets the field name which holds the has_many relation name. + * + * Note that this will return a value even if the has_one relation + * doesn't support multiple reciprocal has_many relations. + */ + public function getForeignRelationKey(): string + { + return $this->relationForeignKey; + } + /** * Create a new PolymorphicHasManyList relation list. * * @param string $dataClass The class of the DataObjects that this will list. - * @param string $foreignField The name of the composite foreign relation field. Used - * to generate the ID and Class foreign keys. + * @param string $foreignField The name of the composite foreign (has_one) relation field. Used + * to generate the ID, Class, and Relation foreign keys. * @param string $foreignClass Name of the class filter this relation is filtered against */ public function __construct($dataClass, $foreignField, $foreignClass) @@ -50,6 +90,7 @@ public function __construct($dataClass, $foreignField, $foreignClass) // Set both id foreign key (as in HasManyList) and the class foreign key parent::__construct($dataClass, "{$foreignField}ID"); $this->classForeignKey = "{$foreignField}Class"; + $this->relationForeignKey = "{$foreignField}Relation"; // Ensure underlying DataQuery globally references the class filter $this->dataQuery->setQueryParam('Foreign.Class', $foreignClass); @@ -98,11 +139,19 @@ public function add($item) return; } + // set the {$relationName}Class field value $foreignKey = $this->foreignKey; $classForeignKey = $this->classForeignKey; $item->$foreignKey = $foreignID; $item->$classForeignKey = $this->getForeignClass(); + // set the {$relationName}Relation field value if appropriate + $foreignRelation = $this->getForeignRelation(); + if ($foreignRelation) { + $relationForeignKey = $this->getForeignRelationKey(); + $item->$relationForeignKey = $foreignRelation; + } + $item->write(); } @@ -129,6 +178,13 @@ public function remove($item) return; } + // Don't remove item with unrelated relation key + $foreignRelation = $this->getForeignRelation(); + $relationForeignKey = $this->getForeignRelationKey(); + if (!$this->relationMatches($item->$relationForeignKey, $foreignRelation)) { + return; + } + // Don't remove item which doesn't belong to this list $foreignID = $this->getForeignID(); $foreignKey = $this->foreignKey; @@ -137,9 +193,20 @@ public function remove($item) || $foreignID == $item->$foreignKey || (is_array($foreignID) && in_array($item->$foreignKey, $foreignID ?? [])) ) { + // Unset the foreign relation key if appropriate + if ($foreignRelation) { + $item->$relationForeignKey = null; + } + + // Unset the rest of the relation and write the record $item->$foreignKey = null; $item->$classForeignKey = null; $item->write(); } } + + private function relationMatches(?string $actual, ?string $expected) + { + return (empty($actual) && empty($expected)) || $actual === $expected; + } } diff --git a/tests/php/Dev/Validation/Member.php b/tests/php/Dev/Validation/Member.php index 1af4a81f7ce..9c302e0d897 100644 --- a/tests/php/Dev/Validation/Member.php +++ b/tests/php/Dev/Validation/Member.php @@ -43,6 +43,8 @@ class Member extends DataObject implements TestOnly */ private static $has_many = [ 'TemporaryMembers' => Freelancer::class . '.TemporaryMember', + 'ManyTeams' => Team::class . '.SingleMember', + 'ManyMoreTeams' => Team::class . '.SingleMember', ]; /** diff --git a/tests/php/Dev/Validation/RelationValidationTest.php b/tests/php/Dev/Validation/RelationValidationTest.php index adc8022e97a..bb6e6484f87 100644 --- a/tests/php/Dev/Validation/RelationValidationTest.php +++ b/tests/php/Dev/Validation/RelationValidationTest.php @@ -6,6 +6,8 @@ use SilverStripe\Core\Config\Config; use SilverStripe\Dev\SapphireTest; use SilverStripe\Dev\Validation\RelationValidationService; +use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataObjectSchema; class RelationValidationTest extends SapphireTest { @@ -107,6 +109,14 @@ public function validateCasesProvider(): array 'SilverStripe\Dev\Tests\Validation\Member / Hat : Back relation is ambiguous', ], ], + 'polymorphic has one' => [ + Team::class, + 'has_one', + [ + 'SingleMember' => DataObject::class, + ], + [], + ], 'invalid has one' => [ Member::class, 'has_one', @@ -118,6 +128,45 @@ public function validateCasesProvider(): array 'SilverStripe\Dev\Tests\Validation\Member / HomeTeam : Relation SilverStripe\Dev\Tests\Validation\Team.UnnecessaryRelation is not in the expected format (needs class only format).' ], ], + 'has_one missing class in array config' => [ + Team::class, + 'has_one', + [ + 'SingleMember' => [ + DataObjectSchema::HASONE_MULTIPLE_HASMANY => true, + ], + ], + [ + 'SilverStripe\Dev\Tests\Validation\Team / SingleMember : No class has been defined for this relation.' + ], + ], + 'multi-reciprocal has_one should be polymorphic' => [ + Team::class, + 'has_one', + [ + 'SingleMember' => [ + 'class' => Member::class, + DataObjectSchema::HASONE_MULTIPLE_HASMANY => true, + ], + ], + [ + 'SilverStripe\Dev\Tests\Validation\Team / SingleMember : has_one relation that can handle multiple reciprocal has_many relations must be polymorphic.' + ], + ], + 'has_one defines class in array config' => [ + Team::class, + 'has_one', + [ + 'SingleMember' => [ + 'class' => Member::class, + ], + ], + // Note there's no message about the has_one class, which is technically correctly defined. + // The bad thing now is just that we still have multiple has_many relations pointing at it. + [ + 'SilverStripe\Dev\Tests\Validation\Team / SingleMember : Back relation is ambiguous' + ], + ], 'ambiguous has_many - no relation name' => [ Team::class, 'has_many', diff --git a/tests/php/Dev/Validation/Team.php b/tests/php/Dev/Validation/Team.php index 701423380ab..0e30bade0a0 100644 --- a/tests/php/Dev/Validation/Team.php +++ b/tests/php/Dev/Validation/Team.php @@ -4,6 +4,7 @@ use SilverStripe\Dev\TestOnly; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\HasManyList; use SilverStripe\ORM\ManyManyList; use SilverStripe\ORM\ManyManyThroughList; @@ -31,6 +32,13 @@ class Team extends DataObject implements TestOnly 'Title' => 'Varchar(255)', ]; + private static array $has_one = [ + 'SingleMember' => [ + 'class' => DataObject::class, + DataObjectSchema::HASONE_MULTIPLE_HASMANY => true, + ], + ]; + /** * @var array */ diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest.php b/tests/php/Forms/GridField/GridFieldDetailFormTest.php index 8565a6f5d29..e244a78d9d3 100644 --- a/tests/php/Forms/GridField/GridFieldDetailFormTest.php +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest.php @@ -13,6 +13,7 @@ use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\Category; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\CategoryController; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\GroupController; +use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\MultiReciprocalPeopleGroup; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\PeopleGroup; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\Person; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\PolymorphicPeopleGroup; @@ -26,6 +27,7 @@ class GridFieldDetailFormTest extends FunctionalTest Person::class, PeopleGroup::class, PolymorphicPeopleGroup::class, + MultiReciprocalPeopleGroup::class, Category::class, ]; @@ -143,6 +145,31 @@ public function testAddFormWithPolymorphicHasOne() $this->assertEquals($group->ID, $record->PolymorphicGroupID); } + public function testAddFormWithMultiReciprocalHasOne() + { + // Log in for permissions check + $this->logInWithPermission('ADMIN'); + // Prepare gridfield and other objects + $group = new MultiReciprocalPeopleGroup(); + $group->write(); + $gridField = $group->getCMSFields()->dataFieldByName('People'); + $gridField->setForm(new Form()); + $detailForm = $gridField->getConfig()->getComponentByType(GridFieldDetailForm::class); + $record = new Person(); + + // Trigger creation of the item edit form + $reflectionDetailForm = new \ReflectionClass($detailForm); + $reflectionMethod = $reflectionDetailForm->getMethod('getItemRequestHandler'); + $reflectionMethod->setAccessible(true); + $itemrequest = $reflectionMethod->invoke($detailForm, $gridField, $record, new Controller()); + $itemrequest->ItemEditForm(); + + // The polymorphic and multi-reciprocal values should be pre-loaded + $this->assertEquals(MultiReciprocalPeopleGroup::class, $record->MultiReciprocalGroupClass); + $this->assertEquals('People', $record->MultiReciprocalGroupRelation); + $this->assertEquals($group->ID, $record->MultiReciprocalGroupID); + } + public function testViewForm() { $this->logInWithPermission('ADMIN'); diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest/MultiReciprocalPeopleGroup.php b/tests/php/Forms/GridField/GridFieldDetailFormTest/MultiReciprocalPeopleGroup.php new file mode 100644 index 00000000000..6dfb9cfd9af --- /dev/null +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest/MultiReciprocalPeopleGroup.php @@ -0,0 +1,38 @@ + 'Varchar' + ]; + + private static $has_many = [ + 'People' => Person::class . '.MultiReciprocalGroup' + ]; + + private static $default_sort = '"Name"'; + + public function getCMSFields() + { + $fields = parent::getCMSFields(); + $fields->replaceField( + 'People', + GridField::create( + 'People', + 'People', + $this->People(), + GridFieldConfig_RelationEditor::create() + ) + ); + return $fields; + } +} diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest/Person.php b/tests/php/Forms/GridField/GridFieldDetailFormTest/Person.php index 2d4ac82c083..ab3f07ef525 100644 --- a/tests/php/Forms/GridField/GridFieldDetailFormTest/Person.php +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest/Person.php @@ -7,6 +7,7 @@ use SilverStripe\Forms\GridField\GridFieldConfig_RelationEditor; use SilverStripe\Forms\RequiredFields; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataObjectSchema; class Person extends DataObject implements TestOnly { @@ -20,6 +21,10 @@ class Person extends DataObject implements TestOnly private static $has_one = [ 'Group' => PeopleGroup::class, 'PolymorphicGroup' => DataObject::class, + 'MultiReciprocalGroup' => [ + 'class' => DataObject::class, + DataObjectSchema::HASONE_MULTIPLE_HASMANY => true, + ], ]; private static $many_many = [ diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest/PolymorphicPeopleGroup.php b/tests/php/Forms/GridField/GridFieldDetailFormTest/PolymorphicPeopleGroup.php index ca5a5e617b5..fe941201d07 100644 --- a/tests/php/Forms/GridField/GridFieldDetailFormTest/PolymorphicPeopleGroup.php +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest/PolymorphicPeopleGroup.php @@ -16,7 +16,7 @@ class PolymorphicPeopleGroup extends DataObject implements TestOnly ]; private static $has_many = [ - 'People' => Person::class + 'People' => Person::class . '.PolymorphicGroup' ]; private static $default_sort = '"Name"'; diff --git a/tests/php/ORM/DataObjectTest/Player.php b/tests/php/ORM/DataObjectTest/Player.php index 4aa2f69c791..b92c7c85d37 100644 --- a/tests/php/ORM/DataObjectTest/Player.php +++ b/tests/php/ORM/DataObjectTest/Player.php @@ -3,6 +3,8 @@ namespace SilverStripe\ORM\Tests\DataObjectTest; use SilverStripe\Dev\TestOnly; +use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\Tests\DataObjectTest; use SilverStripe\Security\Member; @@ -17,6 +19,10 @@ class Player extends Member implements TestOnly private static $has_one = [ 'FavouriteTeam' => DataObjectTest\Team::class, + 'MultiReciprocal' => [ + 'class' => DataObject::class, + DataObjectSchema::HASONE_MULTIPLE_HASMANY => true, + ], ]; private static $belongs_many_many = [ diff --git a/tests/php/ORM/DataObjectTest/Team.php b/tests/php/ORM/DataObjectTest/Team.php index e50af1cea96..23364bb6476 100644 --- a/tests/php/ORM/DataObjectTest/Team.php +++ b/tests/php/ORM/DataObjectTest/Team.php @@ -44,7 +44,10 @@ class Team extends DataObject implements TestOnly 'SubTeams' => SubTeam::class, 'Comments' => TeamComment::class, 'Fans' => Fan::class . '.Favourite', // Polymorphic - Team fans - 'PlayerFans' => Player::class . '.FavouriteTeam' + 'PlayerFans' => Player::class . '.FavouriteTeam', + // multi-reciprocal relation: + 'ManyPlayers1' => Player::class . '.MultiReciprocal', + 'ManyPlayers2' => Player::class . '.MultiReciprocal', ]; private static $many_many = [ diff --git a/tests/php/ORM/DataQueryTest.php b/tests/php/ORM/DataQueryTest.php index aa2aa6ea092..6d1c1369afc 100644 --- a/tests/php/ORM/DataQueryTest.php +++ b/tests/php/ORM/DataQueryTest.php @@ -27,6 +27,8 @@ class DataQueryTest extends SapphireTest DataQueryTest\ObjectG::class, DataQueryTest\ObjectH::class, DataQueryTest\ObjectI::class, + DataQueryTest\ObjectHasMultiReciprocalHasOne::class, + DataQueryTest\ObjectHasMultiReciprocalHasMany::class, SQLSelectTest\CteRecursiveObject::class, SQLSelectTest\TestObject::class, SQLSelectTest\TestBase::class, @@ -99,6 +101,33 @@ public function testApplyRelation() $this->assertStringContainsString('"testctwo_DataQueryTest_C"."ID" = "DataQueryTest_B"."TestCTwoID"', $dq->sql()); } + public function provideApplyRelationMultiReciprocal() + { + return [ + 'relation1' => [ + 'relation' => 'MultiReciprocal1', + ], + 'relation2' => [ + 'relation' => 'MultiReciprocal2', + ], + ]; + } + + /** + * @dataProvider provideApplyRelationMultiReciprocal + */ + public function testApplyRelationMultiReciprocal(string $relation) + { + $dq = new DataQuery(DataQueryTest\ObjectHasMultiReciprocalHasMany::class); + $dq->applyRelation($relation); + $joinAlias = strtolower($relation) . '_DataQueryTest_ObjectHasMultiReciprocalHasOne'; + $joinAliasWithQuotes = '"' . $joinAlias . '"'; + $this->assertTrue($dq->query()->isJoinedTo($joinAlias)); + $this->assertStringContainsString($joinAliasWithQuotes . '."MultiReciprocalID" = "DataQueryTest_ObjectHasMultiReciprocalHasMany"."ID"', $dq->sql()); + $this->assertStringContainsString($joinAliasWithQuotes . '."MultiReciprocalRelation" = ' . $relation, $dq->sql()); + $this->assertStringContainsString($joinAliasWithQuotes . '."MultiReciprocalClass" = "DataQueryTest_ObjectHasMultiReciprocalHasMany"."ClassName"', $dq->sql()); + } + public function testApplyRelationDeepInheritance() { //test has_one relation diff --git a/tests/php/ORM/DataQueryTest/ObjectHasMultiReciprocalHasMany.php b/tests/php/ORM/DataQueryTest/ObjectHasMultiReciprocalHasMany.php new file mode 100644 index 00000000000..ba49bb6821d --- /dev/null +++ b/tests/php/ORM/DataQueryTest/ObjectHasMultiReciprocalHasMany.php @@ -0,0 +1,21 @@ + 'Varchar', + 'SortOrder' => 'Int', + ]; + + private static array $has_many = [ + 'MultiReciprocal1' => ObjectHasMultiReciprocalHasOne::class . '.MultiReciprocal', + 'MultiReciprocal2' => ObjectHasMultiReciprocalHasOne::class . '.MultiReciprocal', + ]; +} diff --git a/tests/php/ORM/DataQueryTest/ObjectHasMultiReciprocalHasOne.php b/tests/php/ORM/DataQueryTest/ObjectHasMultiReciprocalHasOne.php new file mode 100644 index 00000000000..cda1ace704c --- /dev/null +++ b/tests/php/ORM/DataQueryTest/ObjectHasMultiReciprocalHasOne.php @@ -0,0 +1,24 @@ + 'Varchar', + 'SortOrder' => 'Int', + ]; + + private static array $has_one = [ + 'MultiReciprocal' => [ + 'class' => DataObject::class, + DataObjectSchema::HASONE_MULTIPLE_HASMANY => true, + ], + ]; +} diff --git a/tests/php/ORM/PolymorphicHasManyListTest.php b/tests/php/ORM/PolymorphicHasManyListTest.php index e7211ef4bad..af1b6c74a5f 100644 --- a/tests/php/ORM/PolymorphicHasManyListTest.php +++ b/tests/php/ORM/PolymorphicHasManyListTest.php @@ -15,7 +15,10 @@ class PolymorphicHasManyListTest extends SapphireTest { // Borrow the model from DataObjectTest - protected static $fixture_file = 'DataObjectTest.yml'; + protected static $fixture_file = [ + 'DataObjectTest.yml', + 'PolymorphicHasManyListTest.yml', + ]; public static function getExtraDataObjects() { @@ -32,6 +35,30 @@ public function testRelationshipEmptyOnNewRecords() $this->assertEquals([], $newTeam->Fans()->column('ID')); } + /** + * Validates that multiple has_many relations can point to a single multi-reciprocal + * has_one relation and still be separate + */ + public function testMultiReciprocalRelations() + { + $team = $this->objFromFixture(Team::class, 'multiReciprocalTeam'); + $playersList1 = $team->ManyPlayers1(); + $playersList2 = $team->ManyPlayers2(); + + // Lists are separate + $this->assertSame( + ['MultiReciprocal Player 1', 'MultiReciprocal Player 2', 'MultiReciprocal Player 6'], + $playersList1->sort('FirstName')->column('FirstName') + ); + $this->assertSame( + ['MultiReciprocal Player 3', 'MultiReciprocal Player 4', 'MultiReciprocal Player 5'], + $playersList2->sort('FirstName')->column('FirstName') + ); + // The relation is saved on the has_one side of the relationship + $this->assertSame('ManyPlayers1', $playersList1->first()->MultiReciprocalRelation); + $this->assertSame('ManyPlayers2', $playersList2->first()->MultiReciprocalRelation); + } + /** * Test that DataList::relation works with PolymorphicHasManyList */ @@ -40,7 +67,7 @@ public function testFilterRelation() // Check that expected teams exist $list = Team::get(); $this->assertEquals( - ['Subteam 1', 'Subteam 2', 'Subteam 3', 'Team 1', 'Team 2', 'Team 3'], + ['MultiReciprocal team', 'Subteam 1', 'Subteam 2', 'Subteam 3', 'Team 1', 'Team 2', 'Team 3'], $list->sort('Title')->column('Title') ); @@ -65,16 +92,60 @@ public function testFilterRelation() $this->assertEquals(['Bobby', 'Damian', 'Mindy', 'Mitch', 'Richard'], $fans->sort('Name')->column('Name')); } + /** + * The same as testFilterRelation but for multi-reciprocal relationships + */ + public function testFilterMultiReciprocalRelation() + { + $list = Team::get(); + + $players1 = $list->relation('ManyPlayers1')->sort('FirstName')->column('FirstName'); + $players2 = $list->relation('ManyPlayers2')->sort('FirstName')->column('FirstName'); + // Test that each relation has the expected players + $this->assertSame( + ['MultiReciprocal Player 1', 'MultiReciprocal Player 2', 'MultiReciprocal Player 6'], + $players1 + ); + $this->assertSame( + ['MultiReciprocal Player 3', 'MultiReciprocal Player 4', 'MultiReciprocal Player 5'], + $players2 + ); + + // Modify list of fans + $team = $this->objFromFixture(DataObjectTest\Team::class, 'multiReciprocalTeam'); + $newPlayer1 = new DataObjectTest\Player(['FirstName' => 'New player 1']); + $team->ManyPlayers1()->add($newPlayer1); + $this->assertSame('ManyPlayers1', $newPlayer1->MultiReciprocalRelation); + $this->assertSame(Team::class, $newPlayer1->MultiReciprocalClass); + $this->assertSame($team->ID, $newPlayer1->MultiReciprocalID); + $newPlayer2 = new DataObjectTest\Player(['FirstName' => 'New player 2']); + $team->ManyPlayers2()->add($newPlayer2); + $this->assertSame('ManyPlayers2', $newPlayer2->MultiReciprocalRelation); + $this->assertSame(Team::class, $newPlayer2->MultiReciprocalClass); + $this->assertSame($team->ID, $newPlayer2->MultiReciprocalID); + + // and retest + $players1 = $list->relation('ManyPlayers1')->sort('FirstName')->column('FirstName'); + $players2 = $list->relation('ManyPlayers2')->sort('FirstName')->column('FirstName'); + $this->assertSame( + ['MultiReciprocal Player 1', 'MultiReciprocal Player 2', 'MultiReciprocal Player 6', 'New player 1'], + $players1 + ); + $this->assertSame( + ['MultiReciprocal Player 3', 'MultiReciprocal Player 4', 'MultiReciprocal Player 5', 'New player 2'], + $players2 + ); + } + /** * Test that related objects can be removed from a relation */ public function testRemoveRelation() { - // Check that expected teams exist $list = Team::get(); $this->assertEquals( - ['Subteam 1', 'Subteam 2', 'Subteam 3', 'Team 1', 'Team 2', 'Team 3'], + ['MultiReciprocal team', 'Subteam 1', 'Subteam 2', 'Subteam 3', 'Team 1', 'Team 2', 'Team 3'], $list->sort('Title')->column('Title') ); @@ -109,10 +180,57 @@ public function testRemoveRelation() $this->assertEmpty($subteam1fan->FavouriteClass); } + /** + * The same as testRemoveRelation but for multi-reciprocal relationships + */ + public function testRemoveMultiReciprocalRelation() + { + $team = $this->objFromFixture(DataObjectTest\Team::class, 'multiReciprocalTeam'); + $originalPlayers1 = $team->ManyPlayers1()->sort('FirstName')->column('FirstName'); + $originalPlayers2 = $team->ManyPlayers2()->sort('FirstName')->column('FirstName'); + + // Test that each relation has the expected players as a baseline + $this->assertSame( + ['MultiReciprocal Player 1', 'MultiReciprocal Player 2', 'MultiReciprocal Player 6'], + $originalPlayers1 + ); + $this->assertSame( + ['MultiReciprocal Player 3', 'MultiReciprocal Player 4', 'MultiReciprocal Player 5'], + $originalPlayers2 + ); + + // Test that you can't remove items from relations they're not in + $playerFromGroup1 = $this->objFromFixture(DataObjectTest\Player::class, 'multiReciprocalPlayer2'); + $team->ManyPlayers2()->remove($playerFromGroup1); + $this->assertSame($originalPlayers1, $team->ManyPlayers1()->sort('FirstName')->column('FirstName')); + $this->assertSame($originalPlayers2, $team->ManyPlayers2()->sort('FirstName')->column('FirstName')); + $this->assertSame('ManyPlayers1', $playerFromGroup1->MultiReciprocalRelation); + $this->assertSame(Team::class, $playerFromGroup1->MultiReciprocalClass); + $this->assertSame($team->ID, $playerFromGroup1->MultiReciprocalID); + + // Test that you *can* remove items from relations they *are* in + $team->ManyPlayers1()->remove($playerFromGroup1); + $this->assertSame( + ['MultiReciprocal Player 1', 'MultiReciprocal Player 6'], + $team->ManyPlayers1()->sort('FirstName')->column('FirstName') + ); + $this->assertSame($originalPlayers2, $team->ManyPlayers2()->sort('FirstName')->column('FirstName')); + $this->assertEmpty($playerFromGroup1->MultiReciprocalRelation); + $this->assertEmpty($playerFromGroup1->MultiReciprocalClass); + $this->assertEmpty($playerFromGroup1->MultiReciprocalID); + } + public function testGetForeignClassKey(): void { $team = $this->objFromFixture(DataObjectTest\Team::class, 'team1'); $list = $team->Fans(); $this->assertSame('FavouriteClass', $list->getForeignClassKey()); } + + public function getGetForeignRelationKey(): void + { + $team = $this->objFromFixture(DataObjectTest\Team::class, 'team1'); + $list = $team->Fans(); + $this->assertSame('FavouriteRelation', $list->getForeignRelationKey()); + } } diff --git a/tests/php/ORM/PolymorphicHasManyListTest.yml b/tests/php/ORM/PolymorphicHasManyListTest.yml new file mode 100644 index 00000000000..4be1d291e73 --- /dev/null +++ b/tests/php/ORM/PolymorphicHasManyListTest.yml @@ -0,0 +1,25 @@ +SilverStripe\ORM\Tests\DataObjectTest\Player: + multiReciprocalPlayer1: + FirstName: MultiReciprocal Player 1 + multiReciprocalPlayer2: + FirstName: MultiReciprocal Player 2 + multiReciprocalPlayer3: + FirstName: MultiReciprocal Player 3 + multiReciprocalPlayer4: + FirstName: MultiReciprocal Player 4 + multiReciprocalPlayer5: + FirstName: MultiReciprocal Player 5 + multiReciprocalPlayer6: + FirstName: MultiReciprocal Player 6 + +SilverStripe\ORM\Tests\DataObjectTest\Team: + multiReciprocalTeam: + Title: MultiReciprocal team + ManyPlayers1: + - =>SilverStripe\ORM\Tests\DataObjectTest\Player.multiReciprocalPlayer1 + - =>SilverStripe\ORM\Tests\DataObjectTest\Player.multiReciprocalPlayer2 + - =>SilverStripe\ORM\Tests\DataObjectTest\Player.multiReciprocalPlayer6 + ManyPlayers2: + - =>SilverStripe\ORM\Tests\DataObjectTest\Player.multiReciprocalPlayer3 + - =>SilverStripe\ORM\Tests\DataObjectTest\Player.multiReciprocalPlayer4 + - =>SilverStripe\ORM\Tests\DataObjectTest\Player.multiReciprocalPlayer5