Permalink
Browse files

Merge pull request #105 from silverstripe-big-o/ss-reports-documentation

BUGFIX: check for abstract classes when automatically registering SS_Report
  • Loading branch information...
2 parents 68a5686 + 8ad93a1 commit 99e9c4864703a826e0b54e805dce8c6ba81e0872 @sminnee sminnee committed Apr 20, 2012
Showing with 35 additions and 11 deletions.
  1. +3 −11 code/reports/Report.php
  2. +32 −0 tests/reports/ReportTest.php
View
14 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 @@ static function get_reports() {
//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;
View
32 tests/reports/ReportTest.php
@@ -41,6 +41,17 @@ function testExcludeReport() {
$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 @@ function sort() {
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;
+ }
+}
+

0 comments on commit 99e9c48

Please sign in to comment.