From 1aa54924108a877e40fb3048eb87919b64028ac0 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 27 Apr 2016 19:47:25 +1200 Subject: [PATCH] API Cleanup SilverStripeNavigator and CMSPreview --- code/controllers/CMSMain.php | 2 +- code/controllers/SilverStripeNavigator.php | 57 +++++++++---------- code/model/RedirectorPage.php | 16 +++++- code/model/SiteTree.php | 17 +++++- .../controller/SilverStripeNavigatorTest.php | 15 +++++ 5 files changed, 69 insertions(+), 38 deletions(-) diff --git a/code/controllers/CMSMain.php b/code/controllers/CMSMain.php index 2d304e71..a3915692 100644 --- a/code/controllers/CMSMain.php +++ b/code/controllers/CMSMain.php @@ -599,7 +599,7 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr } // Added in-line to the form, but plucked into different view by LeftAndMain.Preview.js upon load - if(in_array('CMSPreviewable', class_implements($record)) && !$fields->fieldByName('SilverStripeNavigator')) { + if($record instanceof CMSPreviewable && !$fields->fieldByName('SilverStripeNavigator')) { $navField = new LiteralField('SilverStripeNavigator', $this->getSilverStripeNavigator()); $navField->setAllowHTML(true); $fields->push($navField); diff --git a/code/controllers/SilverStripeNavigator.php b/code/controllers/SilverStripeNavigator.php index 472bd18e..297ea23c 100644 --- a/code/controllers/SilverStripeNavigator.php +++ b/code/controllers/SilverStripeNavigator.php @@ -15,22 +15,15 @@ class SilverStripeNavigator extends ViewableData { /** - * @var DataObject + * @var DataObject|CMSPreviewable */ protected $record; /** - * @param DataObject $record - * @throws InvalidArgumentException if record doesn't implement CMSPreviewable + * @param DataObject|CMSPreviewable $record */ - public function __construct($record) { - if(!in_array('CMSPreviewable', class_implements($record))) { - throw new InvalidArgumentException(sprintf( - 'SilverStripeNavigator: Record of type %s doesn\'t implement CMSPreviewable', - get_class($record) - )); - } - + public function __construct(CMSPreviewable $record) { + parent::__construct(); $this->record = $record; } @@ -41,23 +34,23 @@ class SilverStripeNavigator extends ViewableData { $items = array(); $classes = ClassInfo::subclassesFor('SilverStripeNavigatorItem'); - array_shift($classes); + unset($classes['SilverStripeNavigatorItem']); // Sort menu items according to priority - $i = 0; foreach($classes as $class) { - // Skip base class - if($class == 'SilverStripeNavigatorItem') continue; - - $i++; + /** @var SilverStripeNavigatorItem $item */ $item = new $class($this->record); - if(!$item->canView()) continue; + if(!$item->canView()) { + continue; + } // This funny litle formula ensures that the first item added with the same priority will be left-most. $priority = $item->getPriority() * 100 - 1; // Ensure that we can have duplicates with the same (default) priority - while(isset($items[$priority])) $priority++; + while(isset($items[$priority])) { + $priority++; + } $items[$priority] = $item; } @@ -68,15 +61,15 @@ class SilverStripeNavigator extends ViewableData { } /** - * @return DataObject + * @return DataObject|CMSPreviewable */ public function getRecord() { return $this->record; } /** - * @param DataObject $record - * @return Array template data + * @param DataObject|CMSPreviewable $record + * @return array template data */ static public function get_for_record($record) { $html = ''; @@ -105,17 +98,18 @@ class SilverStripeNavigator extends ViewableData { * @package cms * @subpackage content */ -class SilverStripeNavigatorItem extends ViewableData { +abstract class SilverStripeNavigatorItem extends ViewableData { /** - * @param DataObject + * @param DataObject|CMSPreviewable */ protected $record; /** - * @param DataObject + * @param DataObject|CMSPreviewable $record */ - public function __construct($record) { + public function __construct(CMSPreviewable $record) { + parent::__construct(); $this->record = $record; } @@ -123,16 +117,18 @@ class SilverStripeNavigatorItem extends ViewableData { * @return string HTML, mostly a link - but can be more complex as well. * For example, a "future state" item might show a date selector. */ - public function getHTML() {} + abstract public function getHTML(); /** * @return string * Get the Title of an item */ - public function getTitle() {} + abstract public function getTitle(); /** * Machine-friendly name. + * + * @return string */ public function getName() { return substr(get_class($this), strpos(get_class($this), '_')+1); @@ -180,7 +176,7 @@ class SilverStripeNavigatorItem extends ViewableData { * Filters items based on member permissions or other criteria, * such as if a state is generally available for the current record. * - * @param Member + * @param Member $member * @return Boolean */ public function canView($member = null) { @@ -403,5 +399,4 @@ class SilverStripeNavigatorItem_ArchiveLink extends SilverStripeNavigatorItem { public function isActive() { return $this->isArchived(); } - } - +} diff --git a/code/model/RedirectorPage.php b/code/model/RedirectorPage.php index 6498c098..0e5777c5 100644 --- a/code/model/RedirectorPage.php +++ b/code/model/RedirectorPage.php @@ -42,15 +42,25 @@ class RedirectorPage extends Page { * If the redirectorpage has been appropriately configured, then it will return the redirection * destination, to prevent unnecessary 30x redirections. However, if it's misconfigured, then * it will return a link to itself, which will then display an error message. + * + * @param string $action + * @return string */ - public function Link() { - if($link = $this->redirectionLink()) return $link; - else return $this->regularLink(); + public function Link($action = null) { + $link = $this->redirectionLink(); + if($link) { + return $link; + } else { + return $this->regularLink($action); + } } /** * Return the normal link directly to this page. Once you visit this link, a 30x redirection * will take you to your final destination. + * + * @param string $action + * @return string */ public function regularLink($action = null) { return parent::Link($action); diff --git a/code/model/SiteTree.php b/code/model/SiteTree.php index 108db8c8..c2e79033 100755 --- a/code/model/SiteTree.php +++ b/code/model/SiteTree.php @@ -424,10 +424,17 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid */ public function PreviewLink($action = null) { if($this->hasMethod('alternatePreviewLink')) { + Deprecation::notice('5.0', 'Use updatePreviewLink or override PreviewLink method'); return $this->alternatePreviewLink($action); - } else { - return $this->AbsoluteLink($action); } + + $link = $this->AbsoluteLink($action); + $this->extend('updatePreviewLink', $link, $action); + return $link; + } + + public function getMimeType() { + return 'text/html'; } /** @@ -497,7 +504,11 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid * @return string */ public function CMSEditLink() { - return Controller::join_links(singleton('CMSPageEditController')->Link('show'), $this->ID); + $link = Controller::join_links( + singleton('CMSPageEditController')->Link('show'), + $this->ID + ); + return Director::absoluteURL($link); } diff --git a/tests/controller/SilverStripeNavigatorTest.php b/tests/controller/SilverStripeNavigatorTest.php index 5e95f293..091c4b63 100644 --- a/tests/controller/SilverStripeNavigatorTest.php +++ b/tests/controller/SilverStripeNavigatorTest.php @@ -44,9 +44,24 @@ class SilverStripeNavigatorTest extends SapphireTest { } class SilverStripeNavigatorTest_TestItem extends SilverStripeNavigatorItem implements TestOnly { + public function getTitle() { + return __CLASS__; + } + public function getHTML() { + return null; + } } class SilverStripeNavigatorTest_ProtectedTestItem extends SilverStripeNavigatorItem implements TestOnly { + + public function getTitle() { + return __CLASS__; + } + + public function getHTML() { + return null; + } + public function canView($member = null) { if(!$member) $member = Member::currentUser(); return Permission::checkMember($member, 'ADMIN');