diff --git a/_config/config.yml b/_config/config.yml index 2875c0b8..553ede50 100644 --- a/_config/config.yml +++ b/_config/config.yml @@ -17,3 +17,7 @@ SilverStripe\Forms\TreeDropdownField: SilverStripe\CMS\Forms\AnchorSelectorField: extensions: - SilverStripe\LinkField\Extensions\AjaxField + +SilverStripe\LinkField\Form\FormFactory: + extensions: + - SilverStripe\LinkField\Extensions\FormFactoryExtension diff --git a/src/Extensions/FormFactoryExtension.php b/src/Extensions/FormFactoryExtension.php new file mode 100644 index 00000000..1491f924 --- /dev/null +++ b/src/Extensions/FormFactoryExtension.php @@ -0,0 +1,55 @@ +exists()) { + // This is a new link, so we don't need to to any further customisation + return; + } + + /** @var FormAction $insertAction */ + $insertAction = $actions->fieldByName('action_insert'); + + if (!$insertAction) { + // We couldn't find the insert action + return; + } + + // Update the title of the action to reflect the link model data state + $insertActionTitle = _t('Admin.EDIT_LINK', 'Edit link'); + $insertAction->setTitle($insertActionTitle); + } +} diff --git a/src/Extensions/ModalController.php b/src/Extensions/ModalController.php index d19b79a4..4df18011 100644 --- a/src/Extensions/ModalController.php +++ b/src/Extensions/ModalController.php @@ -6,9 +6,12 @@ use SilverStripe\Admin\ModalController as OwnerController; use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Core\Extension; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\Form; use SilverStripe\LinkField\Form\FormFactory; +use SilverStripe\LinkField\Models\Link; use SilverStripe\LinkField\Type\Registry; +use SilverStripe\ORM\DataObject; /** * Extensions to apply to ModalController so it knows how to handle the DynamicLink action. @@ -69,10 +72,20 @@ private function getContext(): array throw new HTTPResponse_Exception(sprintf('%s is not a valid link type', $type), 400); } + $data = $this->getData(); + + // Hydrate current model in case data is available, so more options are available for CMS fields customsation + // This allows model-level form customisation + if ($data && array_key_exists('ID', $data) && $data['ID']) { + /** @var Link $type */ + $type = Injector::inst()->create($type->ClassName, $data, DataObject::CREATE_HYDRATED); + } + return [ - 'LinkData' => $this->getData(), + 'LinkData' => $data, 'LinkType' => $type, 'LinkTypeKey' => $linkTypeKey, + // TODO this is likely a legacy field, use form validator instead 'RequireLinkText' => false ]; } diff --git a/src/Form/FormFactory.php b/src/Form/FormFactory.php index 70070777..e9f57f5c 100644 --- a/src/Form/FormFactory.php +++ b/src/Form/FormFactory.php @@ -6,6 +6,7 @@ use SilverStripe\Admin\Forms\LinkFormFactory; use SilverStripe\Forms\HiddenField; use SilverStripe\LinkField\Type\Type; +use SilverStripe\ORM\DataObject; /** * Create Form schema for the LinkField based on a key provided by the request. @@ -21,7 +22,11 @@ protected function getFormFields($controller, $name, $context) throw new LogicException(sprintf('%s: LinkType must be provided and must be an instance of Type', static::class)); } - $fields = $type->scaffoldLinkFields([]); + // Pass on any available link data + $linkData = array_key_exists('LinkData', $context) + ? $context['LinkData'] + : []; + $fields = $type->scaffoldLinkFields($linkData); $fields->push(HiddenField::create('typeKey')->setValue($context['LinkTypeKey'])); $this->extend('updateFormFields', $fields, $controller, $name, $context); @@ -30,6 +35,13 @@ protected function getFormFields($controller, $name, $context) protected function getValidator($controller, $name, $context) { - return null; + if (!array_key_exists('LinkType', $context)) { + return null; + } + + /** @var DataObject|Type $type */ + $type = $context['LinkType']; + + return $type->getCMSCompositeValidator(); } } diff --git a/src/Form/JsonField.php b/src/Form/JsonField.php index 172a317c..4f7ad0b6 100644 --- a/src/Form/JsonField.php +++ b/src/Form/JsonField.php @@ -27,7 +27,7 @@ public function setValue($value, $data = null) return parent::setValue($value, $data); } - /** + /** * @param DataObject|DataObjectInterface $record * @return $this */ @@ -51,16 +51,22 @@ public function saveInto(DataObjectInterface $record) if ($jsonDataObjectID && $jsonDataObject = $record->$fieldname) { if ($value) { $jsonDataObject = $jsonDataObject->setData($value); + $this->extend('onBeforeLinkEdit', $jsonDataObject, $record); $jsonDataObject->write(); + $this->extend('onAfterLinkEdit', $jsonDataObject, $record); } else { + $this->extend('onBeforeLinkDelete', $jsonDataObject, $record); $jsonDataObject->delete(); $record->{"{$fieldname}ID"} = 0; + $this->extend('onAfterLinkDelete', $jsonDataObject, $record); } } elseif ($value) { $jsonDataObject = new $class(); $jsonDataObject = $jsonDataObject->setData($value); + $this->extend('onBeforeLinkCreate', $jsonDataObject, $record); $jsonDataObject->write(); $record->{"{$fieldname}ID"} = $jsonDataObject->ID; + $this->extend('onAfterLinkCreate', $jsonDataObject, $record); } } elseif ((DataObject::getSchema()->databaseField(get_class($record), $fieldname))) { $record->{$fieldname} = $value; diff --git a/src/Models/EmailLink.php b/src/Models/EmailLink.php index f21db6be..410455db 100644 --- a/src/Models/EmailLink.php +++ b/src/Models/EmailLink.php @@ -25,11 +25,11 @@ public function generateLinkDescription(array $data): string public function getCMSFields(): FieldList { - $fields = parent::getCMSFields(); + $this->beforeUpdateCMSFields(static function (FieldList $fields) { + $fields->replaceField('Email', EmailField::create('Email')); + }); - $fields->replaceField('Email', EmailField::create('Email')); - - return $fields; + return parent::getCMSFields(); } public function getURL(): string diff --git a/src/Models/FileLink.php b/src/Models/FileLink.php index 770124dd..0d40b933 100644 --- a/src/Models/FileLink.php +++ b/src/Models/FileLink.php @@ -7,8 +7,8 @@ /** * A link to a File track in asset-admin * - * @property File $File * @property int $FileID + * @method File File() */ class FileLink extends Link { @@ -38,6 +38,8 @@ public function LinkTypeHandlerName(): string public function getURL(): string { - return $this->File?->getURL() ?? ''; + $file = $this->File(); + + return $file->exists() ? (string) $file->getURL() : ''; } } diff --git a/src/Models/Link.php b/src/Models/Link.php index 1e582695..87d2de0f 100644 --- a/src/Models/Link.php +++ b/src/Models/Link.php @@ -73,23 +73,24 @@ public function scaffoldLinkFields(array $data): FieldList */ public function getCMSFields(): FieldList { - $fields = parent::getCMSFields(); - $linkTypes = $this->getLinkTypes(); - - if (static::class === self::class) { - // Add a link type selection field for generic links - $fields->addFieldsToTab( - 'Root.Main', - [ - $linkTypeField = DropdownField::create('LinkType', 'Link Type', $linkTypes), - ], - 'Title' - ); - - $linkTypeField->setEmptyString('-- select type --'); - } + $this->beforeUpdateCMSFields(function (FieldList $fields) { + $linkTypes = $this->getLinkTypes(); + + if (static::class === self::class) { + // Add a link type selection field for generic links + $fields->addFieldsToTab( + 'Root.Main', + [ + $linkTypeField = DropdownField::create('LinkType', 'Link Type', $linkTypes), + ], + 'Title' + ); + + $linkTypeField->setEmptyString('-- select type --'); + } + }); - return $fields; + return parent::getCMSFields(); } /** diff --git a/src/Models/SiteTreeLink.php b/src/Models/SiteTreeLink.php index 1312b4f2..f3f611d8 100644 --- a/src/Models/SiteTreeLink.php +++ b/src/Models/SiteTreeLink.php @@ -4,7 +4,9 @@ use SilverStripe\CMS\Forms\AnchorSelectorField; use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\Control\Controller; use SilverStripe\Forms\FieldList; +use SilverStripe\Forms\TextField; use SilverStripe\Forms\TreeDropdownField; /** @@ -12,6 +14,7 @@ * * @property int $PageID * @property string $Anchor + * @property string $QueryString * @method SiteTree Page() */ class SiteTreeLink extends Link @@ -20,6 +23,7 @@ class SiteTreeLink extends Link private static array $db = [ 'Anchor' => 'Varchar', + 'QueryString' => 'Varchar', ]; private static array $has_one = [ @@ -46,42 +50,58 @@ public function generateLinkDescription(array $data): string public function getCMSFields(): FieldList { - $fields = parent::getCMSFields(); + $this->beforeUpdateCMSFields(static function (FieldList $fields) { + // Remove scaffolded fields to we don't have field name conflicts which would prevent field customisation + $fields->removeByName([ + 'PageID', + 'Anchor', + 'QueryString', + ]); + + $titleField = $fields->dataFieldByName('Title'); + $titleField?->setDescription('Auto generated from Page title if left blank'); + + $fields->insertAfter( + 'Title', + TreeDropdownField::create( + 'PageID', + 'Page', + SiteTree::class, + 'ID', + 'TreeTitle' + ) + ); + + $fields->insertAfter( + 'PageID', + $queryStringField = TextField::create('QueryString') + ); - $titleField = $fields->dataFieldByName('Title'); - $titleField->setDescription('Auto generated from Page title if left blank'); + $queryStringField->setDescription('Do not prepend "?". EG: "option1=value&option2=value2"'); - $fields->insertAfter( - 'Title', - TreeDropdownField::create( - 'PageID', - 'Page', - SiteTree::class, - 'ID', - 'TreeTitle' - ) - ); - - $fields->insertAfter( - 'PageID', - AnchorSelectorField::create('Anchor') - ->setDescription('Do not prepend "#". EG: "option1=value&option2=value2"') - ); - - return $fields; + $fields->insertAfter( + 'QueryString', + $anchorField = AnchorSelectorField::create('Anchor') + ); + + $anchorField->setDescription( + 'Do not prepend "#". Anchor suggestions will be displayed once the linked page is attached.' + ); + }); + + return parent::getCMSFields(); } public function getURL(): string { - $url = $this->Page() ? $this->Page()->Link() : ''; + $page = $this->Page(); + $url = $page->exists() ? $page->Link() : ''; + $anchorSegment = $this->Anchor ? '#' . $this->Anchor : ''; + $queryStringSegment = $this->QueryString ? '?' . $this->QueryString : ''; $this->extend('updateGetURLBeforeAnchor', $url); - if ($this->Anchor) { - $url .= '#' . $this->Anchor; - } - - return $url; + return Controller::join_links($url, $anchorSegment, $queryStringSegment); } /** diff --git a/src/Tasks/LinkableMigrationTask.php b/src/Tasks/LinkableMigrationTask.php index 2a08d432..a3914992 100644 --- a/src/Tasks/LinkableMigrationTask.php +++ b/src/Tasks/LinkableMigrationTask.php @@ -81,6 +81,10 @@ class LinkableMigrationTask extends BuildTask /** * LinkableLink field => LinkField_Link field + * Note: If Link is versioned on your project add Version field to the mapping + * + * @config + * @var string[] */ private static array $link_mapping = [ 'ID' => 'ID', @@ -190,7 +194,8 @@ public function run($request): void foreach ($tables as $table) { // Grab any/all records from the desired table (base, live, versions) - $linkableResults = SQLSelect::create('*', $table)->execute(); + $query = $this->getDataSelect($table); + $linkableResults = $query->execute(); // Nothing to see here if ($linkableResults->numRecords() === 0) { @@ -269,18 +274,41 @@ protected function truncateLinkFieldTables(): void $isVersioned = Link::singleton()->hasExtension(Versioned::class); foreach ($tables as $table) { - DB::get_conn()->clearTable($table); + $this->clearTable($table); if (!$isVersioned) { continue; } foreach ($versioned as $tableSuffix) { - DB::get_conn()->clearTable($table . $tableSuffix); + $this->clearTable($table . $tableSuffix); } } } + /** + * Fetch link data that needs to undergo migration + * + * @param string $tableName + * @return SQLSelect + */ + protected function getDataSelect(string $tableName): SQLSelect + { + return SQLSelect::create('*', sprintf('"%s"', $tableName)); + } + + /** + * Called before migration to delete existing data from the target tables, so we have a clean state + * to migrate into + * + * @param string $tableName + * @return void + */ + protected function clearTable(string $tableName): void + { + DB::get_conn()->clearTable($tableName); + } + /** * Create assignments from the old field values to the new fields based on provided configuration * @@ -323,7 +351,7 @@ protected function insertLink(string $className, array $linkableData, string $or // If we're processing the _Versions table, then we need to add all the Version table field assignments that are // specifically for the base record (such as all the "WasPublished", "WasDraft", etc fields) if ($originTable === self::TABLE_VERSIONS) { - $config += $this->config()->get('versions_mapping_global'); + $config += $this->config()->get('versions_mapping_base_only'); } // These assignments are based on our config diff --git a/src/Type/Registry.php b/src/Type/Registry.php index 2c2ee36d..152138ea 100644 --- a/src/Type/Registry.php +++ b/src/Type/Registry.php @@ -52,6 +52,11 @@ public function list(): array $typeDefinitions = self::config()->get('types'); foreach ($typeDefinitions as $key => $def) { + // This link type is disabled, so we can skip it + if (!array_key_exists('enabled', $def) || !$def['enabled']) { + continue; + } + $types[$key] = $this->definitionToType($def); } diff --git a/tests/php/Models/LinkTest.php b/tests/php/Models/LinkTest.php index 1ce642fb..396f676e 100644 --- a/tests/php/Models/LinkTest.php +++ b/tests/php/Models/LinkTest.php @@ -3,7 +3,12 @@ namespace SilverStripe\LinkField\Tests\Models; use ReflectionException; +use SilverStripe\Assets\Dev\TestAssetStore; +use SilverStripe\Assets\Image; use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\Config\Collections\MutableConfigCollectionInterface; +use SilverStripe\Control\Director; +use SilverStripe\Core\Config\Config; use SilverStripe\Dev\SapphireTest; use SilverStripe\LinkField\Models\EmailLink; use SilverStripe\LinkField\Models\ExternalLink; @@ -11,8 +16,11 @@ use SilverStripe\LinkField\Models\Link; use SilverStripe\LinkField\Models\PhoneLink; use SilverStripe\LinkField\Models\SiteTreeLink; +use SilverStripe\LinkField\Type\Registry; +use SilverStripe\LinkField\Type\Type; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\ValidationException; +use SilverStripe\Versioned\Versioned; class LinkTest extends SapphireTest { @@ -21,6 +29,30 @@ class LinkTest extends SapphireTest */ protected static $fixture_file = 'LinkTest.yml'; + protected function setUp(): void + { + parent::setUp(); + + TestAssetStore::activate('ImageTest'); + + /** @var Image $image */ + $image = $this->objFromFixture(Image::class, 'image-1'); + $image->setFromLocalFile(Director::baseFolder() . '/tests/resources/600x400.png'); + $image->write(); + $image->publishSingle(); + + /** @var SiteTree $page */ + $page = $this->objFromFixture(SiteTree::class, 'page-1'); + $page->publishSingle(); + } + + protected function tearDown(): void + { + TestAssetStore::reset(); + + parent::tearDown(); + } + public function testLinkModel(): void { $model = $this->objFromFixture(Link::class, 'link-1'); @@ -85,4 +117,217 @@ public function linkTypeProvider(): array [Link::class, true], ]; } + + /** + * @param array $types + * @param array $expected + * @return void + * @dataProvider linkTypeEnabledProvider + */ + public function testLinkTypeEnabled(array $types, array $expected): void + { + Config::withConfig(function (MutableConfigCollectionInterface $config) use ($types, $expected): void { + $config->set(Registry::class, 'types', $types); + + $enabledTypes = Registry::singleton()->list(); + $enabledTypes = array_map(static function (Type $type): string { + return $type->LinkTypeTile(); + }, $enabledTypes); + $enabledTypes = array_values($enabledTypes); + sort($enabledTypes, SORT_STRING); + + $this->assertSame($expected, $enabledTypes, 'We expect specific enabled link types'); + }); + } + + public function linkTypeEnabledProvider(): array + { + return [ + 'all types enabled' => [ + [ + 'cms' => [ + 'classname' => SiteTreeLink::class, + 'enabled' => true, + ], + 'external' => [ + 'classname' => ExternalLink::class, + 'enabled' => true, + ], + 'file' => [ + 'classname' => FileLink::class, + 'enabled' => true, + ], + 'email' => [ + 'classname' => EmailLink::class, + 'enabled' => true, + ], + 'phone' => [ + 'classname' => PhoneLink::class, + 'enabled' => true, + ], + ], + [ + 'Email Link', + 'External Link', + 'File Link', + 'Phone Link', + 'Site Tree Link', + ], + ], + 'file type disabled' => [ + [ + 'cms' => [ + 'classname' => SiteTreeLink::class, + 'enabled' => true, + ], + 'external' => [ + 'classname' => ExternalLink::class, + 'enabled' => true, + ], + 'file' => [ + 'classname' => FileLink::class, + 'enabled' => false, + ], + 'email' => [ + 'classname' => EmailLink::class, + 'enabled' => true, + ], + 'phone' => [ + 'classname' => PhoneLink::class, + 'enabled' => true, + ], + ], + [ + 'Email Link', + 'External Link', + 'Phone Link', + 'Site Tree Link', + ], + ], + 'phone and email types disabled' => [ + [ + 'cms' => [ + 'classname' => SiteTreeLink::class, + 'enabled' => true, + ], + 'external' => [ + 'classname' => ExternalLink::class, + 'enabled' => true, + ], + 'file' => [ + 'classname' => FileLink::class, + 'enabled' => true, + ], + 'email' => [ + 'classname' => EmailLink::class, + 'enabled' => false, + ], + 'phone' => [ + 'classname' => PhoneLink::class, + 'enabled' => false, + ], + ], + [ + 'External Link', + 'File Link', + 'Site Tree Link', + ], + ], + ]; + } + + /** + * @param string $identifier + * @param string $class + * @param string $expected + * @return void + * @dataProvider linkUrlCasesDataProvider + */ + public function testGetUrl(string $identifier, string $class, string $expected): void + { + Versioned::withVersionedMode(function () use ($identifier, $class, $expected): void { + Versioned::set_stage(Versioned::LIVE); + + /** @var Link $link */ + $link = $this->objFromFixture($class, $identifier); + + $this->assertSame($expected, $link->getURL(), 'We expect specific URL value'); + }); + } + + public function linkUrlCasesDataProvider(): array + { + return [ + 'internal link / page only' => [ + 'page-link-page-only', + SiteTreeLink::class, + '/page-1', + ], + 'internal link / anchor only' => [ + 'page-link-anchor-only', + SiteTreeLink::class, + '/#my-anchor', + ], + 'internal link / query string only' => [ + 'page-link-query-string-only', + SiteTreeLink::class, + '/?param1=value1¶m2=option2', + ], + 'internal link / with anchor' => [ + 'page-link-with-anchor', + SiteTreeLink::class, + '/page-1#my-anchor', + ], + 'internal link / with query string' => [ + 'page-link-with-query-string', + SiteTreeLink::class, + '/page-1?param1=value1¶m2=option2', + ], + 'internal link / with query string and anchor' => [ + 'page-link-with-query-string-and-anchor', + SiteTreeLink::class, + '/page-1?param1=value1¶m2=option2#my-anchor', + ], + 'email link / with email' => [ + 'email-link-with-email', + EmailLink::class, + 'mailto:maxime@silverstripe.com', + ], + 'email link / no email' => [ + 'email-link-no-email', + EmailLink::class, + '', + ], + 'external link / with URL' => [ + 'external-link-with-url', + ExternalLink::class, + 'https://google.com', + ], + 'external link / no URL' => [ + 'external-link-no-url', + ExternalLink::class, + '', + ], + 'phone link / with phone' => [ + 'phone-link-with-phone', + PhoneLink::class, + 'tel:+64 4 978 7330', + ], + 'phone link / no phone' => [ + 'phone-link-no-phone', + PhoneLink::class, + '', + ], + 'file link / with image' => [ + 'file-link-with-image', + FileLink::class, + '/assets/ImageTest/600x400.png', + ], + 'file link / no image' => [ + 'file-link-no-image', + FileLink::class, + '', + ], + ]; + } } diff --git a/tests/php/Models/LinkTest.yml b/tests/php/Models/LinkTest.yml index 92e0f883..7faac9d9 100644 --- a/tests/php/Models/LinkTest.yml +++ b/tests/php/Models/LinkTest.yml @@ -1,11 +1,66 @@ +SilverStripe\CMS\Model\SiteTree: + page-1: + Title: 'Page1' + URLSegment: 'page-1' + +SilverStripe\Assets\Image: + image-1: + Title: 'Image1' + SilverStripe\LinkField\Models\Link: link-1: - Title: Link1 + Title: 'Link1' SilverStripe\LinkField\Models\SiteTreeLink: page-link-1: - Title: PageLink1 + Title: 'PageLink1' + page-link-page-only: + Title: 'PageLinkPageOnly' + Page: =>SilverStripe\CMS\Model\SiteTree.page-1 + page-link-anchor-only: + Title: 'PageLinkAnchorOnly' + Anchor: 'my-anchor' + page-link-query-string-only: + Title: 'PageLinkQueryStringOnly' + QueryString: 'param1=value1¶m2=option2' + page-link-with-anchor: + Title: 'PageLinkWithAnchor' + Anchor: 'my-anchor' + Page: =>SilverStripe\CMS\Model\SiteTree.page-1 + page-link-with-query-string: + Title: 'PageLinkWithQueryString' + QueryString: 'param1=value1¶m2=option2' + Page: =>SilverStripe\CMS\Model\SiteTree.page-1 + page-link-with-query-string-and-anchor: + Title: 'PageLinkWithQueryStringAndAnchor' + QueryString: 'param1=value1¶m2=option2' + Anchor: 'my-anchor' + Page: =>SilverStripe\CMS\Model\SiteTree.page-1 -SilverStripe\CMS\Model\SiteTree: - page-1: - Title: Page1 +SilverStripe\LinkField\Models\EmailLink: + email-link-with-email: + Title: 'EmailLinkWithEmail' + Email: 'maxime@silverstripe.com' + email-link-no-email: + Title: 'EmailLinkNoEmail' + +SilverStripe\LinkField\Models\ExternalLink: + external-link-with-url: + Title: 'ExternalLinkWithUrl' + ExternalUrl: 'https://google.com' + external-link-no-url: + Title: 'ExternalLinkNoUrl' + +SilverStripe\LinkField\Models\PhoneLink: + phone-link-with-phone: + Title: 'PhoneLinkWithPhone' + Phone: '+64 4 978 7330' + phone-link-no-phone: + Title: 'PhoneLinkNoPhone' + +SilverStripe\LinkField\Models\FileLink: + file-link-with-image: + Title: 'FileLinkWithImage' + File: =>SilverStripe\Assets\Image.image-1 + file-link-no-image: + Title: 'FileLinkNoImage' diff --git a/tests/resources/600x400.png b/tests/resources/600x400.png new file mode 100644 index 00000000..4d4a4dc2 Binary files /dev/null and b/tests/resources/600x400.png differ