Merge pull request #80 from robbieaverill/pulls/4.0/make-excluded-reports-configurable

API Remove Report::add_excluded_report() and make excluded_reports configurable

README.md

@@ -10,7 +10,7 @@ versions of SilverStripe (2.4 and 3.0).
 
 ## Requirements
 
- * SilverStripe 3.2
+ * SilverStripe 4.0
 
 ## Troubleshooting
 

code/Report.php

@@ -87,13 +87,15 @@ class Report extends ViewableData
 
     /**
      * Reports which should not be collected and returned in get_reports
+     *
+     * @config
      * @var array
      */
-    public static $excluded_reports = array(
+    private static $excluded_reports = [
-        'SilverStripe\\Reports\\Report',
+        self::class,
-        'SilverStripe\\Reports\\ReportWrapper',
+        ReportWrapper::class,
-        'SilverStripe\\Reports\\SideReportWrapper',
+        SideReportWrapper::class,
-    );
+    ];
 
     /**
      * Return the title of this report.
@@ -205,30 +207,15 @@ class Report extends ViewableData
         return $sourceRecords->count();
     }
 
-    /**
-     * Exclude certain reports classes from the list of Reports in the CMS
-     * @param $reportClass string|array A string with the Report classname or an array of Report classnames
-     */
-    public static function add_excluded_reports($reportClass)
-    {
-        if (is_array($reportClass)) {
-            self::$excluded_reports = array_merge(self::$excluded_reports, $reportClass);
-        } else {
-            if (is_string($reportClass)) {
-                //add to the excluded reports, so this report doesn't get used
-                self::$excluded_reports[] = $reportClass;
-            }
-        }
-    }
-
     /**
      * Return an array of excluded reports. That is, reports that will not be included in
      * the list of reports in report admin in the CMS.
+     *
      * @return array
      */
     public static function get_excluded_reports()
     {
-        return self::$excluded_reports;
+        return (array) self::config()->get('excluded_reports');
     }
 
     /**
@@ -239,22 +226,26 @@ class Report extends ViewableData
     {
         $reports = ClassInfo::subclassesFor(get_called_class());
 
-        $reportsArray = array();
+        $reportsArray = [];
         if ($reports && count($reports) > 0) {
-            //collect reports into array with an attribute for 'sort'
+            $excludedReports = static::get_excluded_reports();
+            // Collect reports into array with an attribute for 'sort'
             foreach ($reports as $report) {
-                if (in_array($report, self::$excluded_reports)) {
+                // Don't use the Report superclass, or any excluded report classes
+                if (in_array($report, $excludedReports)) {
                     continue;
-                }   //don't use the SS_Report superclass
+                }
                 $reflectionClass = new ReflectionClass($report);
+                // Don't use abstract classes
                 if ($reflectionClass->isAbstract()) {
                     continue;
-                }   //don't use abstract classes
+                }
 
-                $reportObj = new $report;
+                $reportObj = $report::create();
-                if (method_exists($reportObj, 'sort')) {
+                if ($reportObj->hasMethod('sort')) {
+                    // Use the sort method to specify the sort field
                     $reportObj->sort = $reportObj->sort();
-                }  //use the sort method to specify the sort field
+                }
                 $reportsArray[$report] = $reportObj;
             }
         }

code/ReportAdmin.php

@@ -37,7 +37,7 @@ class ReportAdmin extends LeftAndMain implements PermissionProvider
 
     private static $template_path = null; // defaults to (project)/templates/email
 
-    private static $tree_class = 'SilverStripe\\Reports\\Report';
+    private static $tree_class = Report::class;
 
     private static $url_handlers = array(
         'show/$ReportClass/$Action' => 'handleAction'

tests/ReportTest.php

@@ -2,12 +2,13 @@
 
 namespace SilverStripe\Reports\Tests;
 
-use SilverStripe\Reports\Report;
+use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Forms\GridField\GridFieldDataColumns;
 use SilverStripe\Reports\Tests\ReportTest\FakeObject;
 use SilverStripe\Reports\Tests\ReportTest\FakeTest;
 use SilverStripe\Reports\Tests\ReportTest\FakeTest2;
+use SilverStripe\Reports\Report;
 
 class ReportTest extends SapphireTest
 {
@@ -29,14 +30,14 @@ class ReportTest extends SapphireTest
     public function testExcludeReport()
     {
         $reports = Report::get_reports();
-        $reportNames = array();
+        $reportNames = [];
         foreach ($reports as $report) {
             $reportNames[] = get_class($report);
         }
         $this->assertContains(FakeTest::class, $reportNames, 'ReportTest_FakeTest is in reports list');
 
-        //exclude one report
+        // Exclude one report
-        Report::add_excluded_reports(FakeTest::class);
+        Config::modify()->merge(Report::class, 'excluded_reports', [FakeTest::class]);
 
         $reports = Report::get_reports();
         $reportNames = array();
@@ -45,11 +46,14 @@ class ReportTest extends SapphireTest
         }
         $this->assertNotContains(FakeTest::class, $reportNames, 'ReportTest_FakeTest is NOT in reports list');
 
-        //exclude two reports
+        // Exclude two reports
-        Report::add_excluded_reports(array(FakeTest::class, FakeTest2::class));
+        Config::modify()->merge(Report::class, 'excluded_reports', [
+            FakeTest::class,
+            FakeTest2::class
+        ]);
 
         $reports = Report::get_reports();
-        $reportNames = array();
+        $reportNames = [];
         foreach ($reports as $report) {
             $reportNames[] = get_class($report);
         }
@@ -105,6 +109,9 @@ class ReportTest extends SapphireTest
         $page->ID = 959547;
 
         $titleContent = $columns->getColumnContent($gridField, $page, 'Title');
-        $this->assertEquals('<a class="grid-field__link-block" href="dummy-edit-link/959547" title="My Object">My Object</a>', $titleContent);
+        $this->assertEquals(
+            '<a class="grid-field__link-block" href="dummy-edit-link/959547" title="My Object">My Object</a>',
+            $titleContent
+        );
     }
 }

tests/ReportTest_FakeObject.php

@@ -1,39 +0,0 @@
-<?php
-namespace SilverStripe\Reports\Tests;
-
-use SilverStripe\ORM\DataObject;
-use SilverStripe\ORM\CMSPreviewable;
-use SilverStripe\Dev\TestOnly;
-use SilverStripe\Control\Controller;
-
-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);
-    }
-
-    public function PreviewLink($action = null)
-    {
-        return false;
-    }
-
-    public function getMimeType()
-    {
-        return 'text/html';
-    }
-}

tests/ReportTest_FakeTest.php

@@ -1,36 +0,0 @@
-<?php
-namespace SilverStripe\Reports\Tests;
-
-use SilverStripe\Dev\TestOnly;
-use SilverStripe\ORM\ArrayList;
-use SilverStripe\Reports\Report;
-
-/**
- * @package reports
- * @subpackage tests
- */
-class ReportTest_FakeTest extends Report implements TestOnly
-{
-    public function title()
-    {
-        return 'Report title';
-    }
-    public function columns()
-    {
-        return array(
-            "Title" => array(
-                "title" => "Page Title",
-                "link" => true,
-            )
-        );
-    }
-    public function sourceRecords($params, $sort, $limit)
-    {
-        return new ArrayList();
-    }
-
-    public function sort()
-    {
-        return 100;
-    }
-}

tests/ReportTest_FakeTest2.php

@@ -1,35 +0,0 @@
-<?php
-namespace SilverStripe\Reports\Tests;
-
-use SilverStripe\Dev\TestOnly;
-use SilverStripe\ORM\ArrayList;
-use SilverStripe\Reports\Report;
-
-/**
- * @package reports
- * @subpackage tests
- */
-class ReportTest_FakeTest2 extends Report implements TestOnly
-{
-    public function title()
-    {
-        return 'Report title 2';
-    }
-    public function columns()
-    {
-        return array(
-            "Title" => array(
-                "title" => "Page Title 2"
-            )
-        );
-    }
-    public function sourceRecords($params, $sort, $limit)
-    {
-        return new ArrayList();
-    }
-
-    public function sort()
-    {
-        return 98;
-    }
-}

tests/ReportTest_FakeTest_Abstract.php

@@ -1,37 +0,0 @@
-<?php
-namespace SilverStripe\Reports\Tests;
-
-use SilverStripe\Dev\TestOnly;
-use SilverStripe\ORM\ArrayList;
-use SilverStripe\Reports\Report;
-
-/**
- * @package reports
- * @subpackage tests
- */
-abstract class ReportTest_FakeTest_Abstract extends Report implements TestOnly
-{
-
-    public function title()
-    {
-        return 'Report title Abstract';
-    }
-
-    public function columns()
-    {
-        return array(
-            "Title" => array(
-                "title" => "Page Title Abstract"
-            )
-        );
-    }
-    public function sourceRecords($params, $sort, $limit)
-    {
-        return new ArrayList();
-    }
-
-    public function sort()
-    {
-        return 5;
-    }
-}