Merge remote-tracking branch 'origin/3'

# Conflicts:
#	code/Report.php
#	composer.json
#	tests/ReportTest.php

.travis.yml

@@ -1,24 +1,30 @@
 # See https://github.com/silverstripe-labs/silverstripe-travis-support for setup details
 
-language: php 
-php: 
- - 5.3
-
 sudo: false
 
+language: php
+
+php:
+  - 5.5
+  - 5.6
+  - 7.0
+
 env:
   - DB=MYSQL CORE_RELEASE=master
- - DB=PGSQL CORE_RELEASE=master
 
 matrix:
   include:
-    - php: 5.4
-      env: DB=MYSQL CORE_RELEASE=master
+    - php: 5.6
+      env: DB=PGSQL CORE_RELEASE=master
+  allow_failures:
+    - php: 7.0
 
 before_script:
+  - composer self-update || true
   - git clone git://github.com/silverstripe-labs/silverstripe-travis-support.git ~/travis-support
   - php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss
   - cd ~/builds/ss
+  - composer install
 
 script:
- - phpunit reports/tests/
+  - vendor/bin/phpunit reports/tests

code/Report.php

@@ -67,7 +67,13 @@ class SS_Report extends ViewableData
 	public static $excluded_reports = array(
 		'SS_Report',
 		'SS_ReportWrapper',
-        'SideReportWrapper'
+		'SideReportWrapper',
+		'SideReport_RecentlyEdited', // @deprecated 3.2..4.0
+		'SideReport_EmptyPages', // @deprecated 3.2..4.0
+		'SideReport_BrokenVirtualPages', // @deprecated 3.2..4.0
+		'SideReport_BrokenRedirectorPages', // @deprecated 3.2..4.0
+		'SideReport_BrokenLinks', // @deprecated 3.2..4.0
+		'SideReport_BrokenFiles' // @deprecated 3.2..4.0
 	);
 	
 	/**
@@ -109,8 +115,8 @@ class SS_Report extends ViewableData
 	 */
     public function sourceQuery($params)
     {
-        if ($this->hasMethod('sourceRecords')) {
-            return $this->sourceRecords()->dataQuery();
+		if($this->hasMethod('sourceRecords')) {
+			return $this->sourceRecords($params, null, null)->dataQuery();
 		} else {
 			user_error("Please override sourceQuery()/sourceRecords() and columns() or, if necessary, override getReportField()", E_USER_ERROR);
 		}
@@ -121,12 +127,12 @@ class SS_Report extends ViewableData
 	 */
     public function records($params)
     {
-        if ($this->hasMethod('sourceRecords')) {
+		if($this->hasMethod('sourceRecords')) {
 			return $this->sourceRecords($params, null, null);
 		} else {
 			$query = $this->sourceQuery();
 			$results = new ArrayList();
-            foreach ($query->execute() as $data) {
+			foreach($query->execute() as $data) {
 				$class = $this->dataClass();
 				$result = new $class($data);
 				$results->push($result);
@@ -207,7 +213,7 @@ class SS_Report extends ViewableData
 		$reportsArray = array();
 		if ($reports && count($reports) > 0) {
 			//collect reports into array with an attribute for 'sort'
-            foreach ($reports as $report) {
+			foreach($reports as $report) {
                 if (in_array($report, self::$excluded_reports)) {
                     continue;
                 }   //don't use the SS_Report superclass
@@ -224,7 +230,7 @@ class SS_Report extends ViewableData
 			}
 		}
 
-        uasort($reportsArray, function ($a, $b) {
+		uasort($reportsArray, function($a, $b) {
             if ($a->sort == $b->sort) {
                 return 0;
             } else {
@@ -252,17 +258,17 @@ class SS_Report extends ViewableData
     {
 		$fields = new FieldList();
 
-        if ($title = $this->title()) {
+		if($title = $this->title()) {
 			$fields->push(new LiteralField('ReportTitle', "<h3>{$title}</h3>"));
 		}
 		
-        if ($description = $this->description()) {
+		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) {
+		if($this->hasMethod('parameterFields') && $parameterFields = $this->parameterFields()) {
+			foreach($parameterFields as $field) {
 				// Namespace fields for easier handling in form submissions
 				$field->setName(sprintf('filters[%s]', $field->getName()));
 				$field->addExtraClass('no-change-track'); // ignore in changetracker
@@ -312,14 +318,14 @@ class SS_Report extends ViewableData
 			new GridFieldPrintButton('buttons-after-left'),
 			new GridFieldExportButton('buttons-after-left')
 		);
-        $gridField = new GridField('Report', $this->title(), $items, $gridFieldConfig);
+		$gridField = new GridField('Report',$this->title(), $items, $gridFieldConfig);
 		$columns = $gridField->getConfig()->getComponentByType('GridFieldDataColumns');
 		$displayFields = array();
 		$fieldCasting = array();
 		$fieldFormatting = array();
 
 		// Parse the column information
-        foreach ($this->columns() as $source => $info) {
+		foreach($this->columns() as $source => $info) {
             if (is_string($info)) {
                 $info = array('title' => $info);
             }
@@ -334,7 +340,7 @@ class SS_Report extends ViewableData
                 $fieldCasting[$source] = $info['casting'];
             }
 
-            if (isset($info['link']) && $info['link']) {
+			if(isset($info['link']) && $info['link']) {
 				$link = singleton('CMSPageEditController')->Link('show');
 				$fieldFormatting[$source] = '<a href=\"' . $link . '/$ID\">$value</a>';
 			}
@@ -358,9 +364,39 @@ class SS_Report extends ViewableData
 			$member = Member::currentUser();
 		}
 		
+		$extended = $this->extendedCan('canView', $member);
+		if($extended !== null) {
+			return $extended;
+		}
+
+		if($member && Permission::checkMember($member, array('CMS_ACCESS_LeftAndMain', 'CMS_ACCESS_ReportAdmin'))) {
 		return true;
 	}
 	
+		return false;
+	}
+
+	/**
+	 * Helper to assist with permission extension
+	 *
+	 * {@see DataObject::extendedCan()}
+	 *
+	 * @param string $methodName Method on the same object, e.g. {@link canEdit()}
+	 * @param Member|int $member
+	 * @return boolean|null
+	 */
+	public function extendedCan($methodName, $member) {
+		$results = $this->extend($methodName, $member);
+		if($results && is_array($results)) {
+			// Remove NULLs
+			$results = array_filter($results, function($v) {return !is_null($v);});
+			// If there are any non-NULL responses, then return the lowest one of them.
+			// If any explicitly deny the permission, then we don't get access
+			if($results) return min($results);
+		}
+		return null;
+	}
+	
 
 	/**
 	 * Return the name of this report, which
@@ -442,10 +478,10 @@ abstract class SS_ReportWrapper extends SS_Report
 
     public function sourceQuery($params)
     {
-        if ($this->baseReport->hasMethod('sourceRecords')) {
+		if($this->baseReport->hasMethod('sourceRecords')) {
 			// The default implementation will create a fake query from our sourceRecords() method
 			return parent::sourceQuery($params);
-        } elseif ($this->baseReport->hasMethod('sourceQuery')) {
+		} else if($this->baseReport->hasMethod('sourceQuery')) {
 			$this->beforeQuery($params);
 			$query = $this->baseReport->sourceQuery($params);
 			$this->afterQuery();

composer.json

@@ -1,23 +1,30 @@
 {
 	"name": "silverstripe/reports",
 	"type": "silverstripe-module",
+	"description": "Reports module for SilverStripe CMS",
 	"homepage": "http://silverstripe.org",
 	"license": "BSD-3-Clause",
 	"keywords": ["silverstripe", "cms", "reports"],
-	"authors": [{
+	"authors": [
+		{
 		"name": "SilverStripe",
 		"homepage": "http://silverstripe.com"
-	}, {
+		},
+		{
 		"name": "The SilverStripe Community",
 		"homepage": "http://silverstripe.org"
-	}],
+		}
+	],
 	"require": {
 		"php": ">=5.3.3",
-		"silverstripe/framework": ">=3.1.x-dev"
+		"silverstripe/framework": "~3.3"
 	},
 	"extra": {
 		"branch-alias": {
 			"dev-master": "4.0.x-dev"
 		}
+	},
+	"require-dev": {
+		"phpunit/PHPUnit": "~3.7"
 	}
 }

tests/ReportTest.php

@@ -12,7 +12,7 @@ class ReportTest extends SapphireTest
 		$reports = SS_Report::get_reports();
 		$this->assertNotNull($reports, "Reports returned");
 		$previousSort = 0;
-        foreach ($reports as $report) {
+		foreach($reports as $report) {
 			$this->assertGreaterThanOrEqual($previousSort, $report->sort, "Reports are in correct sort order");
 			$previousSort = $report->sort;
 		}
@@ -22,44 +22,64 @@ class ReportTest extends SapphireTest
     {
 		$reports = SS_Report::get_reports();
 		$reportNames = array();
-        foreach ($reports as $report) {
+		foreach($reports as $report) {
 			$reportNames[] = $report->class;
 		}
-        $this->assertContains('ReportTest_FakeTest', $reportNames, 'ReportTest_FakeTest is in reports list');
+		$this->assertContains('ReportTest_FakeTest',$reportNames,'ReportTest_FakeTest is in reports list');
 
 		//exclude one report
 		SS_Report::add_excluded_reports('ReportTest_FakeTest');
 
 		$reports = SS_Report::get_reports();
 		$reportNames = array();
-        foreach ($reports as $report) {
+		foreach($reports as $report) {
 			$reportNames[] = $report->class;
 		}
-        $this->assertNotContains('ReportTest_FakeTest', $reportNames, 'ReportTest_FakeTest is NOT in reports list');
+		$this->assertNotContains('ReportTest_FakeTest',$reportNames,'ReportTest_FakeTest is NOT in reports list');
 
 		//exclude two reports
-        SS_Report::add_excluded_reports(array('ReportTest_FakeTest', 'ReportTest_FakeTest2'));
+		SS_Report::add_excluded_reports(array('ReportTest_FakeTest','ReportTest_FakeTest2'));
 
 		$reports = SS_Report::get_reports();
 		$reportNames = array();
-        foreach ($reports as $report) {
+		foreach($reports as $report) {
 			$reportNames[] = $report->class;
 		}
-        $this->assertNotContains('ReportTest_FakeTest', $reportNames, 'ReportTest_FakeTest is NOT in reports list');
-        $this->assertNotContains('ReportTest_FakeTest2', $reportNames, 'ReportTest_FakeTest2 is NOT in reports list');
+		$this->assertNotContains('ReportTest_FakeTest',$reportNames,'ReportTest_FakeTest is NOT in reports list');
+		$this->assertNotContains('ReportTest_FakeTest2',$reportNames,'ReportTest_FakeTest2 is NOT in reports list');
 	}
 
     public function testAbstractClassesAreExcluded()
     {
 		$reports = SS_Report::get_reports();
 		$reportNames = array();
-        foreach ($reports as $report) {
+		foreach($reports as $report) {
 			$reportNames[] = $report->class;
 		}
 		$this->assertNotContains('ReportTest_FakeTest_Abstract',
 			$reportNames,
 			'ReportTest_FakeTest_Abstract is NOT in reports list as it is abstract');
 	}
+
+	public function testPermissions() {
+		$report = new ReportTest_FakeTest2();
+
+		// Visitor cannot view
+		Session::clear("loggedInAs");
+		$this->assertFalse($report->canView());
+
+		// Logged in user that cannot view reports
+		$this->logInWithPermission('SITETREE_REORGANISE');
+		$this->assertFalse($report->canView());
+
+		// Logged in with report permissions
+		$this->logInWithPermission('CMS_ACCESS_ReportAdmin');
+		$this->assertTrue($report->canView());
+
+		// Admin can view
+		$this->logInWithPermission('ADMIN');
+		$this->assertTrue($report->canView());
+	}
 }
 
 /**