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

API Add new Owner relation for handling permissions #127

Merged
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,36 +40,42 @@ class Page extends SiteTree
];

private static $has_many = [
'HasManyLinks' => Link::class,
// Multiple has_many relations on the same class should point at the same has_one on Link.
'HasManyLinksOne' => Link::class . '.Owner',
'HasManyLinksTwo' => Link::class . '.Owner',
emteknetnz marked this conversation as resolved.
Show resolved Hide resolved
];

private static array $owns = [
'HasOneLink',
'HasManyLinks',
'HasManyLinksOne',
'HasManyLinksTwo',
];

private static array $cascade_deletes = [
'HasOneLink',
'HasManyLinks',
'HasManyLinksOne',
'HasManyLinksTwo',
];

private static array $cascade_duplicates = [
'HasOneLink',
'HasManyLinks',
'HasManyLinksOne',
'HasManyLinksTwo',
];

public function getCMSFields()
{
$fields = parent::getCMSFields();

// Don't forget to remove the auto-scaffolded fields!
$fields->removeByName(['HasOneLinkID', 'Links']);
$fields->removeByName(['HasOneLinkID', 'HasManyLinksOne', 'HasManyLinksTwo']);

$fields->addFieldsToTab(
'Root.Main',
[
LinkField::create('HasOneLink'),
MultiLinkField::create('HasManyLinks'),
MultiLinkField::create('HasManyLinksOne'),
MultiLinkField::create('HasManyLinksTwo'),
],
);

Expand All @@ -78,13 +84,11 @@ 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)

Adding the relationship(s) to the `$owns`, `$cascade_deletes`, and `$cascade_duplicates` config properties is required for versioning (publishing) to work correctly.

## 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 default title value can be updated using an `Extension` with an `updateDefaultLinkTitle()` method and applying that extension to a subclass of `Link`.

Expand Down
10 changes: 10 additions & 0 deletions src/Form/LinkField.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Models/FileLink.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function getDefaultTitle(): string
'File missing',
);
}

return (string) $this->getDescription();
}
}
44 changes: 43 additions & 1 deletion src/Models/Link.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@
use SilverStripe\Forms\RequiredFields;
use SilverStripe\LinkField\Type\Registry;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DataObjectSchema;
use SilverStripe\ORM\FieldType\DBHTMLText;
use SilverStripe\Versioned\Versioned;

/**
* 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
*/
Expand All @@ -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::HAS_ONE_MULTI_RELATIONAL => true,
],
];

private static array $extensions = [
Versioned::class,
];
Expand Down Expand Up @@ -300,6 +315,33 @@ public function getVersionedState(): string
return 'published';
}

/**
* 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
*
Expand Down Expand Up @@ -327,7 +369,7 @@ public function getDisplayTitle(): string
if ($this->Title) {
return $this->Title;
}

$defaultLinkTitle = $this->getDefaultTitle();

$this->extend('updateDefaultLinkTitle', $defaultLinkTitle);
Expand Down
43 changes: 43 additions & 0 deletions tests/php/Form/LinkFieldTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

namespace SilverStripe\LinkField\Tests\Form;

use SilverStripe\Dev\SapphireTest;
use SilverStripe\LinkField\Form\LinkField;
use SilverStripe\LinkField\Models\Link;
use SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner;

class LinkFieldTest extends SapphireTest
{
protected $usesDatabase = true;

protected static $extra_dataobjects = [
LinkOwner::class,
];

/**
* When we save a link into the has_one of a record, we also need to save
* the Owner has_one on the link itself.
*/
public function testSaveInto()
{
// Prepare fixtures (need new records for this)
$field = new LinkField('Link');
$link = new Link();
$link->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);
}
}
92 changes: 92 additions & 0 deletions tests/php/Models/LinkTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -35,6 +36,10 @@ class LinkTest extends SapphireTest
],
];

protected static $extra_dataobjects = [
LinkOwner::class,
];

protected function setUp(): void
{
parent::setUp();
Expand Down Expand Up @@ -423,4 +428,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-relational 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);
}
}
7 changes: 7 additions & 0 deletions tests/php/Models/LinkTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
18 changes: 18 additions & 0 deletions tests/php/Models/LinkTest/LinkOwner.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace SilverStripe\LinkField\Tests\Models\LinkTest;

use SilverStripe\Dev\TestOnly;
use SilverStripe\LinkField\Models\Link;
use SilverStripe\ORM\DataObject;

class LinkOwner extends DataObject implements TestOnly
{
private static array $has_one = [
'Link' => Link::class,
];

private static array $has_many = [
'LinkList' => Link::class . '.Owner',
];
}