From 506c81c2831fc1dec2db4a7e5fdc13989e4feaf5 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Thu, 8 Feb 2024 12:00:08 +1300 Subject: [PATCH] ENH Rename and move Title field --- README.md | 16 ++++++ lang/en.yml | 6 +-- src/Form/Traits/AllowedLinkClassesTrait.php | 2 +- src/Models/Link.php | 18 +++---- src/Models/SiteTreeLink.php | 6 +-- src/Tasks/LinkableMigrationTask.php | 2 +- .../Controllers/LinkFieldControllerTest.php | 42 +++++++-------- .../Controllers/LinkFieldControllerTest.yml | 10 ++-- tests/php/Models/LinkTest.php | 52 +++++++++---------- tests/php/Models/LinkTest.yml | 32 ++++++------ .../Traits/AllowedLinkClassesTraitTest.php | 20 +++---- 11 files changed, 111 insertions(+), 95 deletions(-) diff --git a/README.md b/README.md index 1e2ade6d..26e85e65 100644 --- a/README.md +++ b/README.md @@ -168,6 +168,22 @@ class MyCustomLink extends Link Custom links can have validation set using standard [model validation](https://docs.silverstripe.org/en/5/developer_guides/forms/validation/#model-validation). +## Migration form LinkField v3 to v4 + +The `Title` DB field has been renamed to `LinkText` + +You can manually rename this column in your database with the following: + +```php +// app/_config.php + +use SilverStripe\ORM\DB; + +// Only run this once +// This will rename the `Title` field to `LinkText` in all relevant tables +DB::get_conn()->getSchemaManager()->renameField('LinkField_Link', 'Title', 'LinkText'); +``` + ## Migrating from Shae Dawson's Linkable module https://github.com/sheadawson/silverstripe-linkable diff --git a/lang/en.yml b/lang/en.yml index 2fb42f45..f41ed2e0 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -38,8 +38,8 @@ en: SINGULARNAME: 'File Link' has_one_File: File SilverStripe\LinkField\Models\Link: - LINK_FIELD_TITLE: Title - LINK_FIELD_TITLE_DESCRIPTION: 'If left blank, an appropriate default title will be used on the front-end' + LINK_TEXT_TITLE: 'Link text' + LINK_TEXT_TEXT_DESCRIPTION: 'If left blank, an appropriate default will be used on the front-end' LINK_TYPE_TITLE: 'Link Type' MISSING_DEFAULT_TITLE: 'No link provided' OPEN_IN_NEW_TITLE: 'Open in new window?' @@ -49,7 +49,7 @@ en: other: '{count} Links' SINGULARNAME: Link db_OpenInNew: 'Open in new' - db_Title: Title + db_LinkText: 'Link text' db_Version: Version has_one_Owner: Owner SilverStripe\LinkField\Models\PhoneLink: diff --git a/src/Form/Traits/AllowedLinkClassesTrait.php b/src/Form/Traits/AllowedLinkClassesTrait.php index 52cf31ee..069abc7c 100644 --- a/src/Form/Traits/AllowedLinkClassesTrait.php +++ b/src/Form/Traits/AllowedLinkClassesTrait.php @@ -92,7 +92,7 @@ public function getTypesProps(): string $allowed = array_key_exists($key, $typeDefinitions) && $type->canCreate(); $typesList[$key] = [ 'key' => $key, - 'title' => $type->getMenuTitle(), + 'linkText' => $type->getMenuTitle(), 'handlerName' => $type->LinkTypeHandlerName(), 'priority' => $class::config()->get('menu_priority'), 'icon' => $class::config()->get('icon'), diff --git a/src/Models/Link.php b/src/Models/Link.php index 4a0cfed6..c5fee3d4 100644 --- a/src/Models/Link.php +++ b/src/Models/Link.php @@ -31,7 +31,7 @@ class Link extends DataObject private static $table_name = 'LinkField_Link'; private static array $db = [ - 'Title' => 'Varchar', + 'LinkText' => 'Varchar', 'OpenInNew' => 'Boolean', 'Sort' => 'Int', ]; @@ -94,11 +94,11 @@ public function getCMSFields(): FieldList $this->beforeUpdateCMSFields(function (FieldList $fields) { $linkTypes = $this->getLinkTypes(); - $titleField = $fields->dataFieldByName('Title'); - $titleField->setTitle(_t(__CLASS__ . '.LINK_FIELD_TITLE', 'Title')); - $titleField->setDescription(_t( - self::class . '.LINK_FIELD_TITLE_DESCRIPTION', - 'If left blank, an appropriate default title will be used on the front-end', + $linkTextField = $fields->dataFieldByName('LinkText'); + $linkTextField->setTitle(_t(__CLASS__ . '.LINK_TEXT_TITLE', 'Link text')); + $linkTextField->setDescription(_t( + self::class . '.LINK_TEXT_TEXT_DESCRIPTION', + 'If left blank, an appropriate default will be used on the front-end', )); $fields->removeByName('Sort'); @@ -117,7 +117,7 @@ public function getCMSFields(): FieldList $linkTypes ), ], - 'Title' + 'LinkText' ); $linkTypeField->setEmptyString('-- select type --'); @@ -455,8 +455,8 @@ private function getLinkTypes(): array public function getDisplayTitle(): string { // If we have a title, we can just bail out without any changes - if ($this->Title) { - return $this->Title; + if ($this->LinkText) { + return $this->LinkText; } $defaultLinkTitle = $this->getDefaultTitle(); diff --git a/src/Models/SiteTreeLink.php b/src/Models/SiteTreeLink.php index aed10875..cedef079 100644 --- a/src/Models/SiteTreeLink.php +++ b/src/Models/SiteTreeLink.php @@ -61,8 +61,8 @@ public function getCMSFields(): FieldList 'QueryString', ]); - $titleField = $fields->dataFieldByName('Title'); - $titleField?->setDescription( + $linkTextField = $fields->dataFieldByName('LinkText'); + $linkTextField?->setDescription( _t( __CLASS__ . '.TITLE_DESCRIPTION', 'Auto generated from Page title if left blank', @@ -70,7 +70,7 @@ public function getCMSFields(): FieldList ); $fields->insertAfter( - 'Title', + 'LinkText', TreeDropdownField::create( 'PageID', _t(__CLASS__ . '.PAGE_FIELD_TITLE', 'Page'), diff --git a/src/Tasks/LinkableMigrationTask.php b/src/Tasks/LinkableMigrationTask.php index b63a4a4e..758beeb5 100644 --- a/src/Tasks/LinkableMigrationTask.php +++ b/src/Tasks/LinkableMigrationTask.php @@ -90,7 +90,7 @@ class LinkableMigrationTask extends BuildTask 'ID' => 'ID', 'LastEdited' => 'LastEdited', 'Created' => 'Created', - 'Title' => 'Title', + 'Title' => 'LinkText', 'OpenInNewWindow' => 'OpenInNew', ]; diff --git a/tests/php/Controllers/LinkFieldControllerTest.php b/tests/php/Controllers/LinkFieldControllerTest.php index 755fc324..e6b33077 100644 --- a/tests/php/Controllers/LinkFieldControllerTest.php +++ b/tests/php/Controllers/LinkFieldControllerTest.php @@ -548,18 +548,18 @@ public function provideLinkDelete(): array * @dataProvider provideLinkSort */ public function testLinkSort( - array $newTitleOrder, + array $newLinkTextOrder, string $fail, int $expectedCode, - array $expectedTitles + array $expectedLinkTexts ): void { TestPhoneLink::$fail = $fail; $url = "/admin/linkfield/sort"; $newLinkIDs = []; $links = TestPhoneLink::get(); - foreach ($newTitleOrder as $num) { + foreach ($newLinkTextOrder as $num) { foreach ($links as $link) { - if ($link->Title === "My phone link 0$num") { + if ($link->LinkText === "My phone link 0$num") { $newLinkIDs[] = $link->ID; } } @@ -575,8 +575,8 @@ public function testLinkSort( $response = $this->post($url, null, $headers, null, $body); $this->assertSame($expectedCode, $response->getStatusCode()); $this->assertSame( - $this->getExpectedTitles($expectedTitles), - TestPhoneLink::get()->filter(['OwnerRelation' => 'LinkList'])->column('Title') + $this->getExpectedLinkTexts($expectedLinkTexts), + TestPhoneLink::get()->filter(['OwnerRelation' => 'LinkList'])->column('LinkText') ); } @@ -584,51 +584,51 @@ public function provideLinkSort(): array { return [ 'Success' => [ - 'newTitleOrder' => [4, 2, 3], + 'newLinkTextOrder' => [4, 2, 3], 'fail' => '', 'expectedCode' => 204, - 'expectedTitles' => [4, 2, 3], + 'expectedLinkTexts' => [4, 2, 3], ], 'Emtpy data' => [ - 'newTitleOrder' => [], + 'newLinkTextOrder' => [], 'fail' => '', 'expectedCode' => 400, - 'expectedTitles' => [2, 3, 4], + 'expectedLinkTexts' => [2, 3, 4], ], 'Fail can edit' => [ - 'newTitleOrder' => [4, 2, 3], + 'newLinkTextOrder' => [4, 2, 3], 'fail' => 'can-edit', 'expectedCode' => 403, - 'expectedTitles' => [2, 3, 4], + 'expectedLinkTexts' => [2, 3, 4], ], 'Fail object data' => [ - 'newTitleOrder' => [], + 'newLinkTextOrder' => [], 'fail' => 'object-data', 'expectedCode' => 400, - 'expectedTitles' => [2, 3, 4], + 'expectedLinkTexts' => [2, 3, 4], ], 'Fail csrf token' => [ - 'newTitleOrder' => [4, 2, 3], + 'newLinkTextOrder' => [4, 2, 3], 'fail' => 'csrf-token', 'expectedCode' => 400, - 'expectedTitles' => [2, 3, 4], + 'expectedLinkTexts' => [2, 3, 4], ], 'Mismatched owner' => [ - 'newTitleOrder' => [4, 1, 2], + 'newLinkTextOrder' => [4, 1, 2], 'fail' => '', 'expectedCode' => 400, - 'expectedTitles' => [2, 3, 4], + 'expectedLinkTexts' => [2, 3, 4], ], 'Mismatched owner relation' => [ - 'newTitleOrder' => [4, 5, 2], + 'newLinkTextOrder' => [4, 5, 2], 'fail' => '', 'expectedCode' => 400, - 'expectedTitles' => [2, 3, 4], + 'expectedLinkTexts' => [2, 3, 4], ], ]; } - private function getExpectedTitles(array $expected): array + private function getExpectedLinkTexts(array $expected): array { return array_map(function ($num) { return "My phone link 0$num"; diff --git a/tests/php/Controllers/LinkFieldControllerTest.yml b/tests/php/Controllers/LinkFieldControllerTest.yml index c5b4ac1a..1f132ff3 100644 --- a/tests/php/Controllers/LinkFieldControllerTest.yml +++ b/tests/php/Controllers/LinkFieldControllerTest.yml @@ -3,30 +3,30 @@ SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner: TestLinkOwner02: SilverStripe\LinkField\Tests\Controllers\LinkFieldControllerTest\TestPhoneLink: TestPhoneLink01: - Title: My phone link 01 + LinkText: My phone link 01 Phone: 0123456789 Sort: 1 # Link relation is manually joined in LinkFieldControllerTest::setup() TestPhoneLink02: - Title: My phone link 02 + LinkText: My phone link 02 Phone: 111222333 Sort: 1 Owner: =>SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner.TestLinkOwner02 OwnerRelation: LinkList TestPhoneLink03: - Title: My phone link 03 + LinkText: My phone link 03 Phone: 321321321 Sort: 2 Owner: =>SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner.TestLinkOwner02 OwnerRelation: LinkList TestPhoneLink04: - Title: My phone link 04 + LinkText: My phone link 04 Phone: 444444444 Sort: 3 Owner: =>SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner.TestLinkOwner02 OwnerRelation: LinkList TestPhoneLink05: - Title: My phone link 05 + LinkText: My phone link 05 Phone: 555555555 Sort: 1 Owner: =>SilverStripe\LinkField\Tests\Models\LinkTest\LinkOwner.TestLinkOwner02 diff --git a/tests/php/Models/LinkTest.php b/tests/php/Models/LinkTest.php index a740f2eb..b2231bfc 100644 --- a/tests/php/Models/LinkTest.php +++ b/tests/php/Models/LinkTest.php @@ -81,24 +81,24 @@ public function testSiteTreeLinkTitleFallback(): void /** @var SiteTreeLink $model */ $model = $this->objFromFixture(SiteTreeLink::class, 'page-link-1'); - $this->assertEquals('PageLink1', $model->Title, 'We expect to get a default Link title'); + $this->assertEquals('PageLink1', $model->LinkText, 'We expect to get a default Link title'); /** @var SiteTree $page */ $page = $this->objFromFixture(SiteTree::class, 'page-1'); $model->PageID = $page->ID; - $model->Title = null; + $model->LinkText = null; $model->write(); // The actual Database Title field should still be null - $this->assertNull($model->getField('Title')); - $this->assertEquals(null, $model->Title, 'We expect that link does not have a title'); + $this->assertNull($model->getField('LinkText')); + $this->assertEquals(null, $model->LinkText, 'We expect that link does not have a title'); $customTitle = 'My custom title'; - $model->Title = $customTitle; + $model->LinkText = $customTitle; $model->write(); - $this->assertEquals($customTitle, $model->Title, 'We expect to get the custom title not page title'); + $this->assertEquals($customTitle, $model->LinkText, 'We expect to get the custom title not page title'); } /** @@ -133,39 +133,25 @@ public function linkTypeProvider(): array public function testGetVersionedState(): void { // Versioned Link - $link = Link::create(['Title' => 'abc']); + $link = Link::create(['LinkText' => 'abc']); $this->assertTrue(Link::has_extension(Versioned::class)); $this->assertEquals('unsaved', $link->getVersionedState()); $link->write(); $this->assertEquals('draft', $link->getVersionedState()); $link->publishSingle(); $this->assertEquals('published', $link->getVersionedState()); - $link->Title = 'def'; + $link->LinkText = 'def'; $link->write(); $this->assertEquals('modified', $link->getVersionedState()); // Unversioned Link Link::remove_extension(Versioned::class); - $link = Link::create(['Title' => '123']); + $link = Link::create(['LinkText' => '123']); $this->assertEquals('unsaved', $link->getVersionedState()); $link->write(); $this->assertEquals('unversioned', $link->getVersionedState()); } - /** - * @param string $identifier - * @param string $class - * @param string $expected - * @return void - * @dataProvider linkUrlCasesDataProvider - */ - public function testGetUrl(string $identifier, string $class, string $expected): void - { - /** @var Link $link */ - $link = $this->objFromFixture($class, $identifier); - $this->assertSame($expected, $link->getURL(), 'We expect specific URL value'); - } - - public function linkUrlCasesDataProvider(): array + public function provideGetUrl(): array { return [ 'internal link / page only' => [ @@ -241,7 +227,21 @@ public function linkUrlCasesDataProvider(): array ]; } - function linkDefaultTitleDataProvider(): array + /** + * @param string $identifier + * @param string $class + * @param string $expected + * @return void + * @dataProvider provideGetUrl + */ + public function testGetUrl(string $identifier, string $class, string $expected): void + { + /** @var Link $link */ + $link = $this->objFromFixture($class, $identifier); + $this->assertSame($expected, $link->getURL(), 'We expect specific URL value'); + } + + function provideDefaultLinkTitle(): array { return [ 'page link' => [ @@ -303,7 +303,7 @@ function linkDefaultTitleDataProvider(): array } /** - * @dataProvider linkDefaultTitleDataProvider + * @dataProvider provideDefaultLinkTitle */ public function testDefaultLinkTitle(string $identifier, string $class, string $expected): void { diff --git a/tests/php/Models/LinkTest.yml b/tests/php/Models/LinkTest.yml index 7f2bdb0b..ba28b2e8 100644 --- a/tests/php/Models/LinkTest.yml +++ b/tests/php/Models/LinkTest.yml @@ -10,30 +10,30 @@ SilverStripe\Assets\Image: SilverStripe\LinkField\Models\Link: link-1: - Title: 'Link1' + LinkText: 'Link1' SilverStripe\LinkField\Models\SiteTreeLink: page-link-1: - Title: 'PageLink1' + LinkText: 'PageLink1' page-link-page-only: - Title: 'PageLinkPageOnly' + LinkText: 'PageLinkPageOnly' Page: =>SilverStripe\CMS\Model\SiteTree.page-1 page-link-anchor-only: - Title: 'PageLinkAnchorOnly' + LinkText: 'PageLinkAnchorOnly' Anchor: 'my-anchor' page-link-query-string-only: - Title: 'PageLinkQueryStringOnly' + LinkText: 'PageLinkQueryStringOnly' QueryString: 'param1=value1¶m2=option2' page-link-with-anchor: - Title: 'PageLinkWithAnchor' + LinkText: 'PageLinkWithAnchor' Anchor: 'my-anchor' Page: =>SilverStripe\CMS\Model\SiteTree.page-1 page-link-with-query-string: - Title: 'PageLinkWithQueryString' + LinkText: 'PageLinkWithQueryString' QueryString: 'param1=value1¶m2=option2' Page: =>SilverStripe\CMS\Model\SiteTree.page-1 page-link-with-query-string-and-anchor: - Title: 'PageLinkWithQueryStringAndAnchor' + LinkText: 'PageLinkWithQueryStringAndAnchor' QueryString: 'param1=value1¶m2=option2' Anchor: 'my-anchor' Page: =>SilverStripe\CMS\Model\SiteTree.page-1 @@ -44,37 +44,37 @@ SilverStripe\LinkField\Models\SiteTreeLink: SilverStripe\LinkField\Models\EmailLink: email-link-with-email: - Title: 'EmailLinkWithEmail' + LinkText: 'EmailLinkWithEmail' Email: 'maxime@silverstripe.com' email-link-no-email: - Title: 'EmailLinkNoEmail' + LinkText: 'EmailLinkNoEmail' email-link-with-default-title: Email: 'maxime@silverstripe.com' SilverStripe\LinkField\Models\ExternalLink: external-link-with-url: - Title: 'ExternalLinkWithUrl' + LinkText: 'ExternalLinkWithUrl' ExternalUrl: 'https://google.com' external-link-no-url: - Title: 'ExternalLinkNoUrl' + LinkText: 'ExternalLinkNoUrl' external-link-with-default-title: ExternalUrl: 'https://google.com' SilverStripe\LinkField\Models\PhoneLink: phone-link-with-phone: - Title: 'PhoneLinkWithPhone' + LinkText: 'PhoneLinkWithPhone' Phone: '+64 4 978 7330' phone-link-no-phone: - Title: 'PhoneLinkNoPhone' + LinkText: 'PhoneLinkNoPhone' phone-link-with-default-title: Phone: '+64 4 978 7330' SilverStripe\LinkField\Models\FileLink: file-link-with-image: - Title: 'FileLinkWithImage' + LinkText: 'FileLinkWithImage' File: =>SilverStripe\Assets\Image.image-1 file-link-no-image: - Title: null + LinkText: null File: =>SilverStripe\Assets\Image.image-2 OpenInNew: true file-link-with-default-title: diff --git a/tests/php/Traits/AllowedLinkClassesTraitTest.php b/tests/php/Traits/AllowedLinkClassesTraitTest.php index 511a63f8..983fd918 100644 --- a/tests/php/Traits/AllowedLinkClassesTraitTest.php +++ b/tests/php/Traits/AllowedLinkClassesTraitTest.php @@ -199,13 +199,13 @@ public function testGetTypesPropsCanCreate(): void $this->assertFalse($json['testphone']['allowed']); } - public function typePropsDataProvider() : array + public function provideGetTypesProps() : array { return [ 'SiteTreeLink props' => [ 'class' => SiteTreeLink::class, 'key' => 'sitetree', - 'title' => 'Page on this site', + 'linkText' => 'Page on this site', 'priority' => 0, 'icon' => 'font-icon-page', 'allowed' => true, @@ -213,7 +213,7 @@ public function typePropsDataProvider() : array 'EmailLink props' => [ 'class' => EmailLink::class, 'key' => 'email', - 'title' => 'Link to email address', + 'linkText' => 'Link to email address', 'priority' => 30, 'icon' => 'font-icon-p-mail', 'allowed' => false, @@ -221,7 +221,7 @@ public function typePropsDataProvider() : array 'ExternalLink props' => [ 'class' => ExternalLink::class, 'key' => 'external', - 'title' => 'Link to external URL', + 'linkText' => 'Link to external URL', 'priority' => 20, 'icon' => 'font-icon-external-link', 'allowed' => false, @@ -229,7 +229,7 @@ public function typePropsDataProvider() : array 'FileLink props' => [ 'class' => FileLink::class, 'key' => 'file', - 'title' => 'Link to a file', + 'linkText' => 'Link to a file', 'priority' => 10, 'icon' => 'font-icon-image', 'allowed' => true, @@ -237,7 +237,7 @@ public function typePropsDataProvider() : array 'PhoneLink props' => [ 'class' => PhoneLink::class, 'key' => 'phone', - 'title' => 'Phone number', + 'linkText' => 'Phone number', 'priority' => 40, 'icon' => 'font-icon-mobile', 'allowed' => true, @@ -245,7 +245,7 @@ public function typePropsDataProvider() : array 'TestPhoneLink props' => [ 'class' => TestPhoneLink::class, 'key' => 'testphone', - 'title' => 'Test Phone Link', + 'linkText' => 'Test Phone Link', 'priority' => 100, 'icon' => 'font-icon-link', 'allowed' => false, @@ -254,12 +254,12 @@ public function typePropsDataProvider() : array } /** - * @dataProvider typePropsDataProvider + * @dataProvider provideGetTypesProps */ public function testGetTypesProps( string $class, string $key, - string $title, + string $linkText, int $priority, string $icon, bool $allowed @@ -273,7 +273,7 @@ public function testGetTypesProps( } $json = json_decode($linkField->getTypesProps(), true); $this->assertEquals($key, $json[$key]['key']); - $this->assertEquals($title, $json[$key]['title']); + $this->assertEquals($linkText, $json[$key]['linkText']); $this->assertEquals($priority, $json[$key]['priority']); $this->assertEquals($icon, $json[$key]['icon']); $this->assertEquals($allowed, $json[$key]['allowed']);