Merge pull request #1464 from open-sausages/pulls/4.0/cleanup-navigator

API Changes to CMSPreviewable interface implementors

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);

code/controllers/SilverStripeNavigator.php

@@ -15,22 +15,15 @@
 class SilverStripeNavigator extends ViewableData {
 
 	/**
-	 * @var DataObject
+	 * @var DataObject|CMSPreviewable
 	 */
 	protected $record;
 
 	/**
-	 * @param DataObject $record
+	 * @param DataObject|CMSPreviewable $record
-	 * @throws InvalidArgumentException if record doesn't implement CMSPreviewable
 	 */
-	public function __construct($record) {
+	public function __construct(CMSPreviewable $record) {
-		if(!in_array('CMSPreviewable', class_implements($record))) {
+		parent::__construct();
-			throw new InvalidArgumentException(sprintf(
-				'SilverStripeNavigator: Record of type %s doesn\'t implement CMSPreviewable',
-				get_class($record)
-			));
-		}
-
 		$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
+			/** @var SilverStripeNavigatorItem $item */
-			if($class == 'SilverStripeNavigatorItem') continue;
-
-			$i++;
 			$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
+	 * @param DataObject|CMSPreviewable $record
-	 * @return Array template data
+	 * @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();
 	}
-	}
+}
-

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() {
+	public function Link($action = null) {
-		if($link = $this->redirectionLink()) return $link;
+		$link = $this->redirectionLink();
-		else return $this->regularLink();
+		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);

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);
 	}
 
 

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');