BUG Fix incorrect url manipulation

Fixes https://github.com/silverstripe/silverstripe-cms/issues/1183

code/Report.php

@@ -1,32 +1,32 @@
 <?php
 /**
  * Base "abstract" class creating reports on your data.
- * 
+ *
  * Creating reports
  * ================
- * 
+ *
  * Creating a new report is a matter overloading a few key methods
- * 
+ *
  *  {@link title()}: Return the title - i18n is your responsibility
  *  {@link description()}: Return the description - i18n is your responsibility
  *  {@link sourceQuery()}: Return a SS_List of the search results
  *  {@link columns()}: Return information about the columns in this report.
  *  {@link parameterFields()}: Return a FieldList of the fields that can be used to filter this
  *  report.
- * 
+ *
  * If you wish to modify the report in more extreme ways, you could overload these methods instead.
- * 
+ *
  * {@link getReportField()}: Return a FormField in the place where your report's TableListField
  * usually appears.
- * {@link getCMSFields()}: Return the FieldList representing the complete right-hand area of the 
+ * {@link getCMSFields()}: Return the FieldList representing the complete right-hand area of the
  * report, including the title, description, parameter fields, and results.
- * 
+ *
  * Showing reports to the user
  * ===========================
- * 
+ *
  * Right now, all subclasses of SS_Report will be shown in the ReportAdmin. In SS3 there is only
  * one place where reports can go, so this class is greatly simplifed from its version in SS2.
- * 
+ *
  * @package reports
  */
 class SS_Report extends ViewableData {
@@ -46,7 +46,7 @@ class SS_Report extends ViewableData {
 	 * @var string
 	 */
 	protected $description = '';
-	
+
 	/**
 	 * The class of object being managed by this report.
 	 * Set by overriding in your subclass.
@@ -77,35 +77,35 @@ class SS_Report extends ViewableData {
 
 	/**
 	 * Return the title of this report.
-	 * 
+	 *
 	 * You have two ways of specifying the description:
-	 *  - overriding description(), which lets you support i18n 
+	 *  - overriding description(), which lets you support i18n
 	 *  - defining the $description property
 	 */
 	public function title() {
 		return $this->title;
 	}
-	
+
 	/**
 	 * Allows access to title as a property
-	 * 
+	 *
 	 * @return string
 	 */
 	public function getTitle() {
 		return $this->title();
 	}
-	
+
 	/**
 	 * Return the description of this report.
-	 * 
+	 *
 	 * You have two ways of specifying the description:
-	 *  - overriding description(), which lets you support i18n 
+	 *  - overriding description(), which lets you support i18n
 	 *  - defining the $description property
 	 */
 	public function description() {
 		return $this->description;
 	}
-	
+
 	/**
 	 * Return the {@link SQLQuery} that provides your report data.
 	 */
@@ -116,7 +116,7 @@ class SS_Report extends ViewableData {
 			user_error("Please override sourceQuery()/sourceRecords() and columns() or, if necessary, override getReportField()", E_USER_ERROR);
 		}
 	}
-	
+
 	/**
 	 * Return a SS_List records for this report.
 	 */
@@ -223,11 +223,11 @@ class SS_Report extends ViewableData {
 		if($title = $this->title()) {
 			$fields->push(new LiteralField('ReportTitle', "<h3>{$title}</h3>"));
 		}
-		
+
 		if($description = $this->description()) {
 			$fields->push(new LiteralField('ReportDescription', "<p>" . $description . "</p>"));
 		}
-			
+
 		// Add search fields is available
 		if($this->hasMethod('parameterFields') && $parameterFields = $this->parameterFields()) {
 			foreach($parameterFields as $field) {
@@ -240,25 +240,25 @@ class SS_Report extends ViewableData {
 			// Add a search button
 			$fields->push(new FormAction('updatereport', _t('GridField.Filter')));
 		}
-		
+
 		$fields->push($this->getReportField());
 
 		$this->extend('updateCMSFields', $fields);
-		
+
 		return $fields;
 	}
-	
+
 	public function getCMSActions() {
 		// getCMSActions() can be extended with updateCMSActions() on a extension
 		$actions = new FieldList();
 		$this->extend('updateCMSActions', $actions);
 		return $actions;
 	}
-	
+
 	/**
 	 * Return a field, such as a {@link GridField} that is
 	 * used to show and manipulate data relating to this report.
-	 * 
+	 *
 	 * Generally, you should override {@link columns()} and {@link records()} to make your report,
 	 * but if they aren't sufficiently flexible, then you can override this method.
 	 *
@@ -287,14 +287,26 @@ class SS_Report extends ViewableData {
 		// Parse the column information
 		foreach($this->columns() as $source => $info) {
 			if(is_string($info)) $info = array('title' => $info);
-			
+
 			if(isset($info['formatting'])) $fieldFormatting[$source] = $info['formatting'];
 			if(isset($info['csvFormatting'])) $csvFieldFormatting[$source] = $info['csvFormatting'];
 			if(isset($info['casting'])) $fieldCasting[$source] = $info['casting'];
 
 			if(isset($info['link']) && $info['link']) {
-				$link = singleton('CMSPageEditController')->Link('show');
+				$fieldFormatting[$source] = function($value, $item) {
-				$fieldFormatting[$source] = '<a href=\"' . $link . '/$ID\">$value</a>';
+					$title = Convert::raw2xml($value);
+
+					// If this item is previewable, decorate with link
+					if ($item instanceof CMSPreviewable) {
+						return sprintf(
+							'<a href="%s" title="%s">%s</a>',
+							$item->CMSEditLink(), $title, $title
+						);
+					}
+
+					// Fall back to basic title
+					return $title;
+				};
 			}
 
 			$displayFields[$source] = isset($info['title']) ? $info['title'] : $source;
@@ -305,7 +317,7 @@ class SS_Report extends ViewableData {
 
 		return $gridField;
 	}
-	
+
 	/**
 	 * @param Member $member
 	 * @return boolean
@@ -347,7 +359,7 @@ class SS_Report extends ViewableData {
 		}
 		return null;
 	}
-	
+
 
 	/**
 	 * Return the name of this report, which
@@ -365,17 +377,17 @@ class SS_Report extends ViewableData {
 
 /**
  * SS_ReportWrapper is a base class for creating report wappers.
- * 
+ *
  * Wrappers encapsulate an existing report to alter their behaviour - they are implementations of
  * the standard GoF decorator pattern.
- * 
+ *
  * This base class ensure that, by default, wrappers behave in the same way as the report that is
  * being wrapped.  You should override any methods that need to behave differently in your subclass
  * of SS_ReportWrapper.
- * 
+ *
  * It also makes calls to 2 empty methods that you can override {@link beforeQuery()} and
  * {@link afterQuery()}
- * 
+ *
  * @package reports
  */
 abstract class SS_ReportWrapper extends SS_Report {
@@ -450,11 +462,11 @@ abstract class SS_ReportWrapper extends SS_Report {
 	public function title() {
 		return $this->baseReport->title();
 	}
-	
+
 	public function group() {
 		return $this->baseReport->hasMethod('group') ? $this->baseReport->group() : 'Group';
 	}
-	
+
 	public function sort() {
 		return $this->baseReport->hasMethod('sort') ? $this->baseReport->sort() : 0;
 	}
@@ -465,5 +477,5 @@ abstract class SS_ReportWrapper extends SS_Report {
 
 	public function canView($member = null) {
 		return $this->baseReport->canView($member);
-	}	
+	}
 }

tests/ReportTest.php

@@ -6,6 +6,8 @@
  */
 class ReportTest extends SapphireTest {
 
+	protected $usesDatabase = true;
+
 	public function testGetReports() {
 		$reports = SS_Report::get_reports();
 		$this->assertNotNull($reports, "Reports returned");
@@ -76,6 +78,42 @@ class ReportTest extends SapphireTest {
 		$this->logInWithPermission('ADMIN');
 		$this->assertTrue($report->canView());
 	}
+
+	public function testColumnLink() {
+		$report = new ReportTest_FakeTest();
+		/** @var GridField $gridField */
+		$gridField = $report->getReportField();
+		/** @var GridFieldDataColumns $columns */
+		$columns = $gridField->getConfig()->getComponentByType('GridFieldDataColumns');
+
+		$page = new ReportTest_FakeObject();
+		$page->Title = 'My Object';
+		$page->ID = 959547;
+
+		$titleContent = $columns->getColumnContent($gridField, $page, 'Title');
+		$this->assertEquals('<a href="dummy-edit-link/959547" title="My Object">My Object</a>', $titleContent);
+	}
+}
+
+class ReportTest_FakeObject extends DataObject implements CMSPreviewable, TestOnly {
+
+	private static $db = array(
+		'Title' => 'Varchar'
+	);
+
+	/**
+	 * @return String Absolute URL to the end-user view for this record.
+	 * Example: http://mysite.com/my-record
+	 */
+	public function Link()
+	{
+		return Controller::join_links('dummy-link', $this->ID);
+	}
+
+	public function CMSEditLink()
+	{
+		return Controller::join_links('dummy-edit-link', $this->ID);
+	}
 }
 
 /**
@@ -89,7 +127,8 @@ class ReportTest_FakeTest extends SS_Report implements TestOnly {
 	public function columns() {
 		return array(
 			"Title" => array(
-				"title" => "Page Title"
+				"title" => "Page Title",
+				"link" => true,
 			)
 		);
 	}
@@ -131,7 +170,7 @@ class ReportTest_FakeTest2 extends SS_Report implements TestOnly {
  * @subpackage tests
  */
 abstract class ReportTest_FakeTest_Abstract extends SS_Report implements TestOnly {
-	
+
 	public function title() {
 		return 'Report title Abstract';
 	}