Skip to content

Commit

Permalink
Merge pull request #207 from creative-commoners/pulls/4/title-field
Browse files Browse the repository at this point in the history
ENH Rename and move Title field
  • Loading branch information
GuySartorelli authored Feb 8, 2024
2 parents 533f9ed + 4e43d54 commit 3786a07
Show file tree
Hide file tree
Showing 13 changed files with 138 additions and 117 deletions.
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,26 @@ 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 from 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 code:

```php
// app/_config.php
use SilverStripe\LinkField\Models\Link;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DB;

// Only run this once
// This will rename the `Title` database column to `LinkText` in all relevant tables
$linkTable = DataObject::getSchema()->baseDataTable(Link::class);
DB::get_conn()->getSchemaManager()->renameField($linkTable, 'Title', 'LinkText');
```

It's recommended to put this code in a `BuildTask` so that you can run it exactly once, and then remove that code in a future deployment.

## Migrating from Shae Dawson's Linkable module

https://github.com/sheadawson/silverstripe-linkable
Expand Down
12 changes: 6 additions & 6 deletions lang/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,26 @@ en:
FILE_DOES_NOT_EXIST: 'File does not exist'
FILE_FIELD: File
LINKLABEL: 'Link to a file'
MISSING_DEFAULT_TITLE: 'File missing'
MISSING_DEFAULT_TITLE: '(File missing)'
PLURALNAME: 'File Links'
PLURALS:
one: 'A File Link'
other: '{count} File Links'
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'
MISSING_DEFAULT_TITLE: '(No value provided)'
OPEN_IN_NEW_TITLE: 'Open in new window?'
PLURALNAME: Links
PLURALS:
one: 'A Link'
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:
Expand All @@ -66,7 +66,7 @@ en:
ANCHOR_FIELD_TITLE: Anchor
CANNOT_VIEW_PAGE: 'Cannot view page'
LINKLABEL: 'Page on this site'
MISSING_DEFAULT_TITLE: 'Page missing'
MISSING_DEFAULT_TITLE: '(Page missing)'
PAGE_DOES_NOT_EXIST: 'Page does not exist'
PAGE_FIELD_TITLE: Page
PLURALNAME: 'Site Tree Links'
Expand Down
2 changes: 1 addition & 1 deletion src/Models/FileLink.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function getDefaultTitle(): string
{
$file = $this->File();
if (!$file->exists()) {
return _t(__CLASS__ . '.MISSING_DEFAULT_TITLE', 'File missing');
return _t(__CLASS__ . '.MISSING_DEFAULT_TITLE', '(File missing)');
}

return (string) $this->getDescription();
Expand Down
41 changes: 23 additions & 18 deletions src/Models/Link.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use SilverStripe\ORM\DataObjectSchema;
use SilverStripe\ORM\FieldType\DBHTMLText;
use SilverStripe\Versioned\Versioned;
use SilverStripe\Forms\Tip;

/**
* A Link Data Object. This class should be treated as abstract. You should never directly interact with a plain Link
Expand All @@ -31,7 +32,7 @@ class Link extends DataObject
private static $table_name = 'LinkField_Link';

private static array $db = [
'Title' => 'Varchar',
'LinkText' => 'Varchar',
'OpenInNew' => 'Boolean',
'Sort' => 'Int',
];
Expand Down Expand Up @@ -94,12 +95,12 @@ 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->setTitleTip(new Tip(_t(
self::class . '.LINK_TEXT_TEXT_DESCRIPTION',
'If left blank, an appropriate default will be used on the front-end',
)));

$fields->removeByName('Sort');

Expand All @@ -117,19 +118,21 @@ public function getCMSFields(): FieldList
$linkTypes
),
],
'Title'
'LinkText'
);

$linkTypeField->setEmptyString('-- select type --');
}
});
$this->afterUpdateCMSFields(function (FieldList $fields) {
// Move the OpenInNew field to the bottom of the form if it hasn't been removed in
// Move the LinkText and OpenInNew fields to the bottom of the form if it hasn't been removed in
// a subclasses getCMSFields() method
$openInNewField = $fields->dataFieldByName('OpenInNew');
if ($openInNewField) {
$fields->removeByName('OpenInNew');
$fields->addFieldToTab('Root.Main', $openInNewField);
foreach (['LinkText', 'OpenInNew'] as $name) {
$field = $fields->dataFieldByName($name);
if ($field) {
$fields->removeByName($name);
$fields->addFieldToTab('Root.Main', $field);
}
}
});

Expand Down Expand Up @@ -285,6 +288,8 @@ public function jsonSerialize(): mixed
unset($data['ClassName']);
unset($data['RecordClassName']);

$data['Title'] = $this->getTitle();

return $data;
}

Expand Down Expand Up @@ -452,11 +457,11 @@ private function getLinkTypes(): array
return $types;
}

public function getDisplayTitle(): string
public function getTitle(): string
{
// If we have a title, we can just bail out without any changes
if ($this->Title) {
return $this->Title;
// If we have link text, we can just bail out without any changes
if ($this->LinkText) {
return $this->LinkText;
}

$defaultLinkTitle = $this->getDefaultTitle();
Expand All @@ -470,7 +475,7 @@ public function getDefaultTitle(): string
{
$default = $this->getDescription() ?: $this->getURL();
if (!$default) {
$default = _t(static::class . '.MISSING_DEFAULT_TITLE', 'No link provided');
$default = _t(static::class . '.MISSING_DEFAULT_TITLE', '(No value provided)');
}
return $default;
}
Expand Down
20 changes: 8 additions & 12 deletions src/Models/SiteTreeLink.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use SilverStripe\Forms\TreeDropdownField;
use SilverStripe\Forms\CompositeValidator;
use SilverStripe\Forms\RequiredFields;
use SilverStripe\Forms\Tip;

/**
* A link to a Page in the CMS
Expand Down Expand Up @@ -61,16 +62,14 @@ public function getCMSFields(): FieldList
'QueryString',
]);

$titleField = $fields->dataFieldByName('Title');
$titleField?->setDescription(
_t(
__CLASS__ . '.TITLE_DESCRIPTION',
'Auto generated from Page title if left blank',
),
);
$linkTextField = $fields->dataFieldByName('LinkText');
$linkTextField?->setTitleTip(new Tip(_t(
__CLASS__ . '.TITLE_DESCRIPTION',
'Auto generated from Page title if left blank',
)));

$fields->insertAfter(
'Title',
'LinkText',
TreeDropdownField::create(
'PageID',
_t(__CLASS__ . '.PAGE_FIELD_TITLE', 'Page'),
Expand Down Expand Up @@ -130,10 +129,7 @@ public function getDefaultTitle(): string
{
$page = $this->Page();
if (!$page->exists()) {
return _t(
static::class . '.MISSING_DEFAULT_TITLE',
'Page missing',
);
return _t(static::class . '.MISSING_DEFAULT_TITLE', '(Page missing)');
}
if (!$page->canView()) {
return '';
Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/LinkableMigrationTask.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class LinkableMigrationTask extends BuildTask
'ID' => 'ID',
'LastEdited' => 'LastEdited',
'Created' => 'Created',
'Title' => 'Title',
'Title' => 'LinkText',
'OpenInNewWindow' => 'OpenInNew',
];

Expand Down
2 changes: 1 addition & 1 deletion templates/SilverStripe/LinkField/Models/Link.ss
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<% if $exists %>
<a href="$URL" <% if $OpenInNew %>target="_blank" rel="noopener noreferrer"<% end_if %>>$DisplayTitle</a>
<a href="$URL" <% if $OpenInNew %>target="_blank" rel="noopener noreferrer"<% end_if %>>$Title</a>
<% end_if %>
50 changes: 25 additions & 25 deletions tests/php/Controllers/LinkFieldControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ public function testLinkFormGetSchema(
. "&ownerRelation=$ownerRelation";
$this->assertSame($expectedAction, $formSchema['schema']['action']);
// schema is nested and retains 'Root' and 'Main' tab hierarchy
$this->assertSame('Phone', $formSchema['schema']['fields'][0]['children'][0]['children'][1]['name']);
$this->assertSame('Phone', $formSchema['schema']['fields'][0]['children'][0]['children'][0]['name']);
$this->assertSame('action_save', $formSchema['schema']['actions'][0]['name']);
// state node is flattened, unlike schema node
$this->assertSame($expectedValue, $formSchema['state']['fields'][3]['value']);
$this->assertSame($expectedValue, $formSchema['state']['fields'][2]['value']);
$this->assertFalse(array_key_exists('errors', $formSchema));
if ($idType === 'new-record') {
$this->assertSame('OwnerID', $formSchema['state']['fields'][6]['name']);
Expand Down Expand Up @@ -224,10 +224,10 @@ public function testLinkFormPost(
. "&ownerRelation=$ownerRelation";
$this->assertSame($expectedUrl, $formSchema['schema']['action']);
// schema is nested and retains 'Root' and 'Main' tab hierarchy
$this->assertSame('Phone', $formSchema['schema']['fields'][0]['children'][0]['children'][1]['name']);
$this->assertSame('Phone', $formSchema['schema']['fields'][0]['children'][0]['children'][0]['name']);
$this->assertSame('action_save', $formSchema['schema']['actions'][0]['name']);
// state node is flattened, unlike schema node
$this->assertSame('9876543210', $formSchema['state']['fields'][3]['value']);
$this->assertSame('9876543210', $formSchema['state']['fields'][2]['value']);
if ($fail) {
// Phone was note updated on PhoneLink dataobject
$link = TestPhoneLink::get()->byID($newID);
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -575,60 +575,60 @@ 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')
);
}

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";
Expand Down
10 changes: 5 additions & 5 deletions tests/php/Controllers/LinkFieldControllerTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 3786a07

Please sign in to comment.