diff --git a/README.md b/README.md index 7182a1da..abf5e9f3 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ class Page extends SiteTree ]; private static $has_many = [ - 'HasManyLinks' => Link::class + 'HasManyLinks' => Link::class . '.Owner', ]; public function getCMSFields() @@ -63,11 +63,9 @@ class Page extends SiteTree } ``` -Note that you also need to add a `has_one` relation on the `Link` model to match your `has_many` here. See [official docs about `has_many`](https://docs.silverstripe.org/en/developer_guides/model/relations/#has-many) - ## Default title for each link type -By default, if the title for the link has not been set, then the default title will be used instead according to the type of link that is used. Default link is not stored in the database as link title. This value is used only when rendering page content. +By default, if the title for the link has not been set, then the default title will be used instead according to the type of link that is used. Default link is not stored in the database as link title. This value is used only when rendering page content. The developer also can set his own default title value using an extension by using `updateDefaultLinkTitle` method for each link type class. diff --git a/src/Form/LinkField.php b/src/Form/LinkField.php index e31eab16..e02a3d98 100644 --- a/src/Form/LinkField.php +++ b/src/Form/LinkField.php @@ -44,6 +44,16 @@ public function saveInto(DataObjectInterface $record) $dbColumn = $fieldname . 'ID'; $record->$dbColumn = $linkID; + // Store the record as the owner of the link. + // Required for permission checks, etc. + $link = Link::get()->byID($linkID); + if ($link) { + $link->OwnerID = $record->ID; + $link->OwnerClass = $record->ClassName; + $link->OwnerRelation = $fieldname; + $link->write(); + } + return $this; } diff --git a/src/Models/FileLink.php b/src/Models/FileLink.php index 483dd6ee..21b8d832 100644 --- a/src/Models/FileLink.php +++ b/src/Models/FileLink.php @@ -5,6 +5,12 @@ use SilverStripe\Assets\File; use SilverStripe\Forms\FieldList; +/** + * A link to a File in the CMS + * + * @property int $FileID + * @method File File() + */ class FileLink extends Link { private static string $table_name = 'LinkField_FileLink'; diff --git a/src/Models/Link.php b/src/Models/Link.php index 315e08b6..f7823d2e 100644 --- a/src/Models/Link.php +++ b/src/Models/Link.php @@ -13,11 +13,15 @@ use SilverStripe\Forms\RequiredFields; use SilverStripe\LinkField\Type\Registry; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\FieldType\DBHTMLText; /** - * A Link Data Object. This class should be a subclass, and you should never directly interact with a plain Link - * instance + * A Link Data Object. This class should be treated as abstract. You should never directly interact with a plain Link + * instance. + * + * Note that links should be added via a has_one or has_many relation, NEVER a many_many relation. This is because + * some functionality such as the can* methods rely on having a single Owner. * * @property string $Title * @property bool $OpenInNew @@ -31,6 +35,17 @@ class Link extends DataObject 'OpenInNew' => 'Boolean', ]; + private static array $has_one = [ + // Note that this handles one-to-many relations AND one-to-one relations. + // Any has_one pointing at Link will be intentionally double handled - this allows us to use the owner + // for permission checks and to link back to the owner from reports, etc. + // See also the Owner method. + 'Owner' => [ + 'class' => DataObject::class, + DataObjectSchema::HASONE_IS_MULTIRECIPROCAL => true, + ], + ]; + /** * In-memory only property used to change link type * This case is relevant for CMS edit form which doesn't use React driven UI @@ -277,6 +292,33 @@ public function getURL(): string return ''; } + /** + * Get the owner of this link, if there is one. + * + * Returns null if the reciprocal relation is a has_one which no longer contains this link + * or if there simply is no actual owner record in the db. + */ + public function Owner(): ?DataObject + { + $owner = $this->getComponent('Owner'); + + // Since the has_one is being stored in two places, double check the owner + // actually still owns this record. If not, return null. + if ($this->OwnerRelation && $owner->getRelationType($this->OwnerRelation) === 'has_one') { + $idField = "{$this->OwnerRelation}ID"; + if ($owner->$idField !== $this->ID) { + return null; + } + } + + // Return null if there simply is no owner + if (!$owner || !$owner->isInDB()) { + return null; + } + + return $owner; + } + /** * Get all link types except the generic one * @@ -304,7 +346,7 @@ public function getDisplayTitle(): string if ($this->Title) { return $this->Title; } - + $defaultLinkTitle = $this->getDefaultTitle(); $this->extend('updateDefaultLinkTitle', $defaultLinkTitle); diff --git a/tests/php/Form/LinkFieldTest.php b/tests/php/Form/LinkFieldTest.php new file mode 100644 index 00000000..4fb94aec --- /dev/null +++ b/tests/php/Form/LinkFieldTest.php @@ -0,0 +1,43 @@ +write(); + $owner = new LinkOwner(); + $owner->write(); + + // Save link into owner + $field->setValue($link->ID); + $field->saveInto($owner); + // Get the link again - the new values are in the DB. + $link = Link::get()->byID($link->ID); + + // Validate + $this->assertSame($link->ID, $owner->LinkID); + $this->assertSame($owner->ID, $link->OwnerID); + $this->assertSame($owner->ClassName, $link->OwnerClass); + $this->assertSame('Link', $link->OwnerRelation); + } +} diff --git a/tests/php/Models/LinkTest.php b/tests/php/Models/LinkTest.php index ad02ef3d..44f505a3 100644 --- a/tests/php/Models/LinkTest.php +++ b/tests/php/Models/LinkTest.php @@ -21,6 +21,7 @@ use SilverStripe\ORM\ValidationException; use SilverStripe\Versioned\Versioned; use SilverStripe\LinkField\Tests\Extensions\ExternalLinkExtension; +use SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner; class LinkTest extends SapphireTest { @@ -35,6 +36,10 @@ class LinkTest extends SapphireTest ], ]; + protected static $extra_dataobjects = [ + LinkOwner::class, + ]; + protected function setUp(): void { parent::setUp(); @@ -407,4 +412,91 @@ public function testDefaultLinkTitle(string $identifier, string $class, string $ $this->assertEquals($expected, $link->getDisplayTitle()); } + + public function provideOwner() + { + return [ + 'null because there is no owner' => [ + 'class' => EmailLink::class, + 'fixture' => 'email-link-with-email', + 'expected' => null, + ], + 'null because the has_one is only stored on the owner' => [ + 'class' => SiteTreeLink::class, + 'fixture' => 'page-link-1', + // The owner has_one link, but the relationship wasn't saved in the link's Owner has_one. + // See the LinkOwner.owns-has-one fixture. + 'expected' => null, + ], + 'has_many owner always works' => [ + 'class' => SiteTreeLink::class, + 'fixture' => 'page-link-page-only', + 'expected' => [ + 'class' => LinkOwner::class, + 'fixture' => 'owns-has-many', + ], + ], + ]; + } + + /** + * Test the functionality of the overridden Owner method. + * Note this is NOT explicitly testing multi-reciprocal has_many relations pointing at the has_one since that's + * a framework functionality, not a linkfield one. + * + * @dataProvider provideOwner + */ + public function testOwner(string $class, string $fixture, ?array $expected) + { + $link = $this->objFromFixture($class, $fixture); + if (is_array($expected)) { + $expected = $this->idFromFixture($expected['class'], $expected['fixture']); + } + + $this->assertSame($expected, $link->Owner()?->ID); + } + + /** + * Testing a scenario where a has_one to has_one is stored on the link. + * Note we can't easily use providers here because of all the necessary logic to set this all up. + */ + public function testOwnerHasOne() + { + $link = new Link(); + $link->write(); + $owner = new LinkOwner(); + $owner->write(); + + // Add the owner relation on the link - without the relation + $link->update([ + 'OwnerID' => $owner->ID, + 'OwnerClass' => $owner->ClassName, + ]); + $link->write(); + + // Clear out any previous-fetches of the owner component. We'll do this each time we check the owner. + $link->flushCache(false); + // The link tells us who the owner is - it doesn't have any way to tell that + // the owner doesn't have a reciprocal relationship yet. + $this->assertSame($owner->ID, $link->Owner()?->ID); + + // LinkField adds the relation name to the link, so this is what we'll normally see + $link->OwnerRelation = 'Link'; + $link->write(); + + // The actual has_one component is the LinkOwner record + $link->flushCache(false); + $this->assertSame($owner->ID, $link->getComponent('Owner')?->ID); + // Owner returns null, because there is no reciprocal relationship from the LinkOwner record + $link->flushCache(false); + $this->assertSame(null, $link->Owner()); + + // Add the link relation on the owner + $owner->LinkID = $link->ID; + $owner->write(); + + // The link is now happy to declare its owner to us + $link->flushCache(false); + $this->assertSame($owner->ID, $link->Owner()?->ID); + } } diff --git a/tests/php/Models/LinkTest.yml b/tests/php/Models/LinkTest.yml index 89229344..7f2bdb0b 100644 --- a/tests/php/Models/LinkTest.yml +++ b/tests/php/Models/LinkTest.yml @@ -79,3 +79,10 @@ SilverStripe\LinkField\Models\FileLink: OpenInNew: true file-link-with-default-title: File: =>SilverStripe\Assets\Image.image-1 + +SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner: + owns-has-one: + Link: =>SilverStripe\LinkField\Models\SiteTreeLink.page-link-1 + owns-has-many: + LinkList: + - =>SilverStripe\LinkField\Models\SiteTreeLink.page-link-page-only diff --git a/tests/php/Models/LinkTest/LinkOwner.php b/tests/php/Models/LinkTest/LinkOwner.php new file mode 100644 index 00000000..a9b9e02e --- /dev/null +++ b/tests/php/Models/LinkTest/LinkOwner.php @@ -0,0 +1,18 @@ + Link::class, + ]; + + private static array $has_many = [ + 'LinkList' => Link::class . '.Owner', + ]; +}