From 8ad93a197e7797859cbf410b907bca898df20432 Mon Sep 17 00:00:00 2001
From: Julian Seidenberg <julian@silverstripe.com>
Date: Fri, 20 Apr 2012 16:32:39 +1200
Subject: [PATCH] BUGFIX: check for abstract classes when automatically
 registering SS_Report classes. Abstract classes are now no longer included in
 the list of Reports. Includes unit test for this scenario.

---
 code/reports/Report.php      | 14 +++-----------
 tests/reports/ReportTest.php | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/code/reports/Report.php b/code/reports/Report.php
index 2226396f..fb0ad216 100644
--- a/code/reports/Report.php
+++ b/code/reports/Report.php
@@ -14,17 +14,6 @@
  *  {@link parameterFields()}: Return a FieldList of the fields that can be used to filter this
  *  report.
  * 
- * If you can't express your report as a query, you can implement the this method instead:
- * 
- *     // Return an array of fields that can be used to sort the data
- *     public function sourceRecords($params, $sort, $limit) { ... }
- * 
- * The $sort value will be set to the corresponding key of the columns() array.  If you wish to
- * make only a subset of the columns sortable, then you can override `sortColumns()` to return a
- * subset of the array keys.
- * 
- * Note that this implementation is less efficient and should only be used when necessary.
- * 
  * 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
@@ -204,6 +193,9 @@ class SS_Report extends ViewableData {
 			//collect reports into array with an attribute for 'sort'
 			foreach($reports as $report) {
 				if (in_array($report, self::$excluded_reports)) continue;   //don't use the SS_Report superclass
+				$reflectionClass = new ReflectionClass($report);
+				if ($reflectionClass->isAbstract()) continue;   //don't use abstract classes
+
 				$reportObj = new $report;
 				if (method_exists($reportObj,'sort')) $reportObj->sort = $reportObj->sort();  //use the sort method to specify the sort field
 				$reportsArray[$report] = $reportObj;
diff --git a/tests/reports/ReportTest.php b/tests/reports/ReportTest.php
index ea1435b2..1d4ae2f5 100644
--- a/tests/reports/ReportTest.php
+++ b/tests/reports/ReportTest.php
@@ -41,6 +41,17 @@ class ReportTest extends SapphireTest {
 		$this->assertNotContains('ReportTest_FakeTest',$reportNames,'ReportTest_FakeTest is NOT in reports list');
 		$this->assertNotContains('ReportTest_FakeTest2',$reportNames,'ReportTest_FakeTest2 is NOT in reports list');
 	}
+
+	function testAbstractClassesAreExcluded() {
+		$reports = SS_Report::get_reports();
+		$reportNames = array();
+		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');
+	}
 }
 
 class ReportTest_FakeTest extends SS_Report implements TestOnly {
@@ -83,3 +94,24 @@ class ReportTest_FakeTest2 extends SS_Report implements TestOnly {
 		return 98;
 	}
 }
+
+abstract class ReportTest_FakeTest_Abstract extends SS_Report implements TestOnly {
+	function title() {
+		return 'Report title Abstract';
+	}
+	function columns() {
+		return array(
+			"Title" => array(
+				"title" => "Page Title Abstract"
+			)
+		);
+	}
+	function sourceRecords($params, $sort, $limit) {
+		return new ArrayList();
+	}
+
+	function sort() {
+		return 5;
+	}
+}
+