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

ENH Rename and move Title field #207

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
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');
```
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved

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)'
Copy link
Member Author

@emteknetnz emteknetnz Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unrelated refactoring to add brackets to "File missing" and "Page missing" default titles so that it matches "(No value provided)" for generic links

It's just a little something to make it really obvious that it's not regular link text

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)'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're now always showing a title, this was the default provided for custom links without LinkText - I've updated it to something more sensible since it's more likely the custom link have a "value" rather that a "link (to a relation)"

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']);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These array index changes are required because we're move LinkText to the bottom in getCMSFields()

$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
Loading