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 Add versioning to Link #90

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
32 changes: 31 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,22 @@ class Page extends SiteTree
];

private static $has_many = [
'HasManyLinks' => Link::class
'HasManyLinks' => Link::class,
];

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

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

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

public function getCMSFields()
Expand All @@ -65,6 +80,8 @@ 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.
Expand All @@ -88,6 +105,19 @@ class ExternalLinkExtension extends Extension

```

## Unversioned links

The `Link` model has the `Versioned` extension applied to it by default. If you wish for links to not be versioned, then remove the extension from the `Link` model in the projects `app/_config.php` file.

```php
// app/_config.php

use SilverStripe\LinkField\Models\Link;
use SilverStripe\Versioned\Versioned;

Link::remove_extension(Versioned::class);
```

## Migrating from Shae Dawson's Linkable module

https://github.com/sheadawson/silverstripe-linkable
Expand Down
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/dist/styles/bundle.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion client/lang/src/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,9 @@
"LinkField.CONFIRM_DELETE": "Deleted link",
"LinkField.DELETE_ERROR": "Failed to delete link",
"LinkField.ADD_LINK": "Add Link",
"LinkField.CLEAR": "Clear"
"LinkField.CLEAR": "Clear",
"LinkField.LINK_DRAFT_TITLE": "Link has draft changes",
"LinkField.LINK_DRAFT_LABEL": "Draft",
"LinkField.LINK_MODIFIED_TITLE": "Link has unpublished changes",
"LinkField.LINK_MODIFIED_LABEL": "Modified"
}
1 change: 1 addition & 0 deletions client/src/components/LinkField/LinkField.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ const LinkField = ({ value = null, onChange, types, actions, isMulti = false })
id={linkID}
title={data[linkID]?.Title}
description={data[linkID]?.description}
versionState={data[linkID]?.versionState}
typeTitle={type.title || ''}
onClear={onClear}
onClick={() => { setEditingID(linkID); }}
Expand Down
40 changes: 40 additions & 0 deletions client/src/components/LinkPicker/LinkPicker.scss
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
text-align: left;
margin-right: 0;
justify-content: space-between;
position: relative;

&:not(:last-child) {
border-bottom: 0;
Expand All @@ -63,6 +64,35 @@
text-decoration: none;
color: inherit;
}

// version-state icon
&::before {
top: 29px;
left: 32px;
content: ' ';
position: absolute;
border: 1px solid $state-draft;
border-radius: 100%;
bottom: 6px;
box-shadow: 0 0 1px .5px $white;
display: block;
height: 8px;
width: 8px;
z-index: 1;
}

&--draft::before {
background-color: $state-draft-bg;;
}

&--modified::before {
background-color: $state-modified-bg;
}

&--unsaved::before,
&--published::before {
display: none;
}
}

.link-picker__button {
Expand Down Expand Up @@ -92,3 +122,13 @@
.link-picker__url {
color: $link-color;
}

.link-picker__title-text {
margin-right: 5px;
}

.link-picker__title .badge {
color: #cf3f00;
background-color: #fff2ea;
padding: 2px 3px 2px 4px;
}
40 changes: 36 additions & 4 deletions client/src/components/LinkPicker/LinkPickerTitle.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable */
import classnames from 'classnames';
import i18n from 'i18n';
import React from 'react';
import PropTypes from 'prop-types';
Expand All @@ -12,11 +13,41 @@ const stopPropagation = (fn) => (e) => {
fn && fn();
}

const LinkPickerTitle = ({ id, title, description, typeTitle, onClear, onClick }) => (
<div className={classnames('link-picker__link', 'form-control')}>
const getVersionedBadge = (versionState) => {
let title = '';
let label = ''
if (versionState === 'draft') {
title = i18n._t('LinkField.LINK_DRAFT_TITLE', 'Link has draft changes');
label = i18n._t('LinkField.LINK_DRAFT_LABEL', 'Draft');
} else if (versionState === 'modified') {
title = i18n._t('LinkField.LINK_MODIFIED_TITLE', 'Link has unpublished changes');
label = i18n._t('LinkField.LINK_MODIFIED_LABEL', 'Modified');
} else {
return null;
}
const className = classnames('badge', `status-${versionState}`);
return <span className={className} title={title}>{label}</span>;
};

const LinkPickerTitle = ({ id, title, description, versionState, typeTitle, onClear, onClick }) => {
const classes = {
'link-picker__link': true,
'form-control': true,
};
if (versionState) {
classes[` link-picker__link--${versionState}`] = true;
}
if (title && title.length > 25) {
title = title.substring(0, 25) + '...';
}
const className = classnames(classes);
return <div className={className}>
<Button className="link-picker__button font-icon-link" color="secondary" onClick={stopPropagation(onClick)}>
<div className="link-picker__link-detail">
<div className="link-picker__title">{title}</div>
<div className="link-picker__title">
<span className="link-picker__title-text">{title}</span>
{getVersionedBadge(versionState)}
</div>
<small className="link-picker__type">
{typeTitle}:&nbsp;
<span className="link-picker__url">{description}</span>
Expand All @@ -25,12 +56,13 @@ const LinkPickerTitle = ({ id, title, description, typeTitle, onClear, onClick }
</Button>
<Button className="link-picker__clear" color="link" onClick={stopPropagation(() => onClear(id))}>{i18n._t('LinkField.CLEAR', 'Clear')}</Button>
</div>
);
};

LinkPickerTitle.propTypes = {
id: PropTypes.number.isRequired,
title: PropTypes.string,
description: PropTypes.string,
versionState: PropTypes.string,
typeTitle: PropTypes.string.isRequired,
onClear: PropTypes.func.isRequired,
onClick: PropTypes.func.isRequired,
Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"type": "silverstripe-vendormodule",
"require": {
"php": "^8.1",
"silverstripe/cms": "^5"
"silverstripe/cms": "^5",
"silverstripe/versioned": "^2"
},
"require-dev": {
"silverstripe/recipe-testing": "^3",
Expand Down
1 change: 1 addition & 0 deletions src/Controllers/LinkFieldController.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ private function getLinkData(Link $link): array
}
$data = $link->jsonSerialize();
$data['description'] = $link->getDescription();
$data['versionState'] = $link->getVersionedState();
return $data;
}

Expand Down
25 changes: 24 additions & 1 deletion src/Models/Link.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use InvalidArgumentException;
use ReflectionException;
use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Forms\CompositeValidator;
use SilverStripe\Forms\DropdownField;
Expand All @@ -14,6 +13,7 @@
use SilverStripe\LinkField\Type\Registry;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\FieldType\DBHTMLText;
use SilverStripe\Versioned\Versioned;

/**
* A Link Data Object. This class should be a subclass, and you should never directly interact with a plain Link
Expand All @@ -31,6 +31,10 @@ class Link extends DataObject
'OpenInNew' => 'Boolean',
];

private static array $extensions = [
Versioned::class,
];

/**
* In-memory only property used to change link type
* This case is relevant for CMS edit form which doesn't use React driven UI
Expand Down Expand Up @@ -277,6 +281,25 @@ public function getURL(): string
return '';
}

public function getVersionedState(): string
{
if (!$this->exists()) {
return 'unsaved';
}
if ($this->hasExtension(Versioned::class)) {
if ($this->isPublished()) {
if ($this->isModifiedOnDraft()) {
return 'modified';
}
return 'published';
}
return 'draft';
}
// Unversioned - links are saved in the modal so there is no 'dirty state' and
// when undversioned saved is the same thing as published
return 'published';
}

/**
* Get all link types except the generic one
*
Expand Down
2 changes: 2 additions & 0 deletions tests/php/Controllers/LinkFieldControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,8 @@ public function testLinkData(
$data = json_decode($response->getBody(), true);
$this->assertSame($id, $data['ID']);
$this->assertSame('0123456789', $data['Phone']);
$link = $this->getFixtureLink();
$this->assertSame($link->getVersionedState(), $data['versionState']);
}
}

Expand Down
32 changes: 24 additions & 8 deletions tests/php/Models/LinkTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,27 @@ public function linkTypeEnabledProvider(): array
];
}

public function testGetVersionedState(): void
{
// Versioned Link
$link = Link::create(['Title' => '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->write();
$this->assertEquals('modified', $link->getVersionedState());
// Unversioned Link
Link::remove_extension(Versioned::class);
$link = Link::create(['Title' => '123']);
$this->assertEquals('unsaved', $link->getVersionedState());
$link->write();
$this->assertEquals('published', $link->getVersionedState());
}

/**
* @param string $identifier
* @param string $class
Expand All @@ -250,14 +271,9 @@ public function linkTypeEnabledProvider(): array
*/
public function testGetUrl(string $identifier, string $class, string $expected): void
{
Versioned::withVersionedMode(function () use ($identifier, $class, $expected): void {
Versioned::set_stage(Versioned::LIVE);
Copy link
Member Author

@emteknetnz emteknetnz Dec 5, 2023

Choose a reason for hiding this comment

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

Versioning doesn't have any impact on this test (it's testing getURL()), and setting things to LIVE means we'd need to publish the Links before making assertions


/** @var Link $link */
$link = $this->objFromFixture($class, $identifier);

$this->assertSame($expected, $link->getURL(), 'We expect specific URL value');
});
/** @var Link $link */
$link = $this->objFromFixture($class, $identifier);
$this->assertSame($expected, $link->getURL(), 'We expect specific URL value');
}

public function linkUrlCasesDataProvider(): array
Expand Down