Merge pull request #74 from open-sausages/pulls/4.0/app-object

[WIP] Upgrade module for app object refactor

.travis.yml

@@ -4,16 +4,21 @@ sudo: false
 
 language: php
 
+env:
+  global:
+    - CORE_RELEASE=master
+
 matrix:
   include:
     - php: 5.6
-      env: DB=MYSQL CORE_RELEASE=master
+      env: DB=MYSQL PHPCS_TEST=1
     - php: 7.0
-      env: DB=PGSQL CORE_RELEASE=master
+      env: DB=PGSQL
     - php: 7.1.2
-      env: DB=MYSQL CORE_RELEASE=master
+      env: DB=MYSQL
 
 before_script:
+  - composer validate
   - 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
@@ -22,3 +27,4 @@ before_script:
 
 script:
   - vendor/bin/phpunit reports/tests
+  - if [[ $PHPCS_TEST ]]; then (cd reports && composer run-script lint); fi

code/Report.php

@@ -2,29 +2,29 @@
 
 namespace SilverStripe\Reports;
 
+use ReflectionClass;
+use SilverStripe\Control\Controller;
+use SilverStripe\Core\ClassInfo;
+use SilverStripe\Core\Convert;
+use SilverStripe\Forms\FieldList;
+use SilverStripe\Forms\FormAction;
+use SilverStripe\Forms\GridField\GridField;
+use SilverStripe\Forms\GridField\GridFieldButtonRow;
+use SilverStripe\Forms\GridField\GridFieldConfig;
+use SilverStripe\Forms\GridField\GridFieldDataColumns;
+use SilverStripe\Forms\GridField\GridFieldExportButton;
+use SilverStripe\Forms\GridField\GridFieldPaginator;
+use SilverStripe\Forms\GridField\GridFieldPrintButton;
+use SilverStripe\Forms\GridField\GridFieldSortableHeader;
+use SilverStripe\Forms\GridField\GridFieldToolbarHeader;
+use SilverStripe\Forms\LiteralField;
 use SilverStripe\ORM\ArrayList;
+use SilverStripe\ORM\CMSPreviewable;
 use SilverStripe\ORM\SS_List;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\Permission;
-use SilverStripe\Control\Controller;
-use SilverStripe\Core\ClassInfo;
-use SilverStripe\Forms\FieldList;
-use SilverStripe\Forms\LiteralField;
-use SilverStripe\Forms\FormAction;
-use SilverStripe\Forms\GridField\GridFieldConfig;
-use SilverStripe\Forms\GridField\GridFieldButtonRow;
-use SilverStripe\Forms\GridField\GridFieldPrintButton;
-use SilverStripe\Forms\GridField\GridFieldExportButton;
-use SilverStripe\Forms\GridField\GridFieldToolbarHeader;
-use SilverStripe\Forms\GridField\GridFieldSortableHeader;
-use SilverStripe\Forms\GridField\GridFieldDataColumns;
-use SilverStripe\Forms\GridField\GridFieldPaginator;
-use SilverStripe\Forms\GridField\GridField;
-use SilverStripe\Core\Convert;
 use SilverStripe\Security\Security;
 use SilverStripe\View\ViewableData;
-use ReflectionClass;
-use SilverStripe\ORM\CMSPreviewable ;
 
 /**
  * Base "abstract" class creating reports on your data.
@@ -178,15 +178,16 @@ class Report extends ViewableData
         );
     }
 
-	/**
+    /**
-	 * Sanitise a model class' name for inclusion in a link
+     * Sanitise a model class' name for inclusion in a link
-	 *
+     *
-	 * @param string $class
+     * @param string $class
-	 * @return string
+     * @return string
-	 */
+     */
-	protected function sanitiseClassName($class) {
+    protected function sanitiseClassName($class)
-		return str_replace('\\', '-', $class);
+    {
-	}
+        return str_replace('\\', '-', $class);
+    }
 
 
     /**
@@ -198,7 +199,6 @@ class Report extends ViewableData
     {
         $sourceRecords = $this->sourceRecords($params, null, null);
         if (!$sourceRecords instanceof SS_List) {
-
             user_error(static::class . "::sourceRecords does not return an SS_List", E_USER_NOTICE);
             return "-1";
         }
@@ -370,7 +370,7 @@ class Report extends ViewableData
             }
 
             if (isset($info['link']) && $info['link']) {
-                $fieldFormatting[$source] = function($value, $item) {
+                $fieldFormatting[$source] = function ($value, $item) {
                     if ($item instanceof CMSPreviewable) {
                         /** @var CMSPreviewable $item */
                         return sprintf(
@@ -381,7 +381,7 @@ class Report extends ViewableData
                         );
                     }
                     return $value;
-				};
+                };
             }
 
             $displayFields[$source] = isset($info['title']) ? $info['title'] : $source;
@@ -429,7 +429,9 @@ class Report extends ViewableData
         $results = $this->extend($methodName, $member);
         if ($results && is_array($results)) {
             // Remove NULLs
-            $results = array_filter($results, function ($v) {return !is_null($v);});
+            $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) {

code/ReportAdmin.php

@@ -6,12 +6,11 @@ use SilverStripe\Admin\LeftAndMain;
 use SilverStripe\Control\Controller;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\Form;
+use SilverStripe\Forms\GridField\GridField;
 use SilverStripe\Forms\GridField\GridFieldConfig;
-use SilverStripe\Forms\GridField\GridFieldToolbarHeader;
-use SilverStripe\Forms\GridField\GridFieldSortableHeader;
 use SilverStripe\Forms\GridField\GridFieldDataColumns;
 use SilverStripe\Forms\GridField\GridFieldFooter;
-use SilverStripe\Forms\GridField\GridField;
+use SilverStripe\Forms\GridField\GridFieldSortableHeader;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\SS_List;
@@ -132,15 +131,16 @@ class ReportAdmin extends LeftAndMain implements PermissionProvider
         return parent::handleAction($request, $action);
     }
 
-	/**
+    /**
-	 * Unsanitise a model class' name from a URL param
+     * Unsanitise a model class' name from a URL param
-	 *
+     *
-	 * @param string $class
+     * @param string $class
-	 * @return string
+     * @return string
-	 */
+     */
-	protected function unsanitiseClassName($class) {
+    protected function unsanitiseClassName($class)
-		return str_replace('-', '\\', $class);
+    {
-	}
+        return str_replace('-', '\\', $class);
+    }
 
     /**
      * Determine if we have reports and need

code/SideReport.php

@@ -2,8 +2,8 @@
 
 namespace SilverStripe\Reports;
 
-use TableListField;
 use SilverStripe\Core\Convert;
+use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\View\ViewableData;
 
 /**
@@ -70,17 +70,11 @@ class SideReportView extends ViewableData
 
     protected function formatValue($record, $source, $info)
     {
-        // Field sources
+        // Cast value
-        //if(is_string($source)) {
-            $val = Convert::raw2xml($record->$source);
-        //} else {
-        //	$val = $record->val($source[0], $source[1]);
-        //}
-
-        // Casting, a la TableListField.  We're deep-calling a helper method on TableListField that
-        // should probably be pushed elsewhere...
         if (!empty($info['casting'])) {
-            $val = TableListField::getCastedValue($val, $info['casting']);
+            $val = DBField::create_field($info['casting'], $record->source)->forTemplate();
+        } else {
+            $val = Convert::raw2xml($record->$source);
         }
 
         // Formatting, a la TableListField

composer.json

@@ -19,6 +19,9 @@
     "silverstripe/framework": "^4.0@dev",
     "silverstripe/admin": "^1.0@dev"
   },
+  "require-dev": {
+    "phpunit/phpunit": "^5.7"
+  },
   "extra": {
     "branch-alias": {
       "dev-master": "4.0.x-dev"
@@ -26,10 +29,12 @@
   },
   "autoload": {
     "psr-4": {
-      "SilverStripe\\Reports\\": "code/"
+      "SilverStripe\\Reports\\": "code/",
+      "SilverStripe\\Reports\\Tests\\": "tests/"
     }
   },
-  "require-dev": {
+  "scripts": {
-    "phpunit/PHPUnit": "~4.8"
+    "lint": "phpcs code/ tests/",
+    "lint-clean": "phpcbf code/ tests/"
   }
 }

phpcs.xml.dist

@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset name="SilverStripe">
+	<description>CodeSniffer ruleset for SilverStripe coding conventions.</description>
+
+	<!-- base rules are PSR-2 -->
+	<rule ref="PSR2" >
+		<!-- Current exclusions -->
+		<exclude name="PSR1.Methods.CamelCapsMethodName" />
+		<exclude name="PSR1.Files.SideEffects.FoundWithSymbols" />
+		<exclude name="PSR2.Classes.PropertyDeclaration" />
+		<exclude name="PSR2.ControlStructures.SwitchDeclaration" /> <!-- causes php notice while linting -->
+		<exclude name="PSR2.ControlStructures.SwitchDeclaration.WrongOpenercase" />
+		<exclude name="PSR2.ControlStructures.SwitchDeclaration.WrongOpenerdefault" />
+		<exclude name="PSR2.ControlStructures.SwitchDeclaration.TerminatingComment" />
+		<exclude name="PSR2.Methods.MethodDeclaration.Underscore" />
+		<exclude name="Squiz.Scope.MethodScope" />
+		<exclude name="Squiz.Classes.ValidClassName.NotCamelCaps" />
+		<exclude name="Generic.Files.LineLength.TooLong" />
+		<exclude name="PEAR.Functions.ValidDefaultValue.NotAtEnd" />
+	</rule>
+</ruleset>
+

tests/ReportTest.php

@@ -1,23 +1,20 @@
 <?php
 
-use SilverStripe\ORM\ArrayList;
+namespace SilverStripe\Reports\Tests;
-use SilverStripe\ORM\DataObject;
+
-use SilverStripe\Reports\Report;
-use SilverStripe\Control\Controller;
-use SilverStripe\Control\Session;
 use SilverStripe\Dev\SapphireTest;
-use SilverStripe\Dev\TestOnly;
+use SilverStripe\Forms\GridField\GridField;
-use SilverStripe\ORM\CMSPreviewable ;
 use SilverStripe\Forms\GridField\GridFieldDataColumns;
+use SilverStripe\Reports\Report;
+use SilverStripe\Reports\Tests\ReportTest\FakeObject;
+use SilverStripe\Reports\Tests\ReportTest\FakeTest;
+use SilverStripe\Reports\Tests\ReportTest\FakeTest2;
 
-
-/**
- * @package reports
- * @subpackage tests
- */
 class ReportTest extends SapphireTest
 {
-    protected $usesDatabase = true;
+    protected static $extra_dataobjects = [
+        FakeObject::class,
+    ];
 
     public function testGetReports()
     {
@@ -37,28 +34,28 @@ class ReportTest extends SapphireTest
         foreach ($reports as $report) {
             $reportNames[] = get_class($report);
         }
-        $this->assertContains('ReportTest_FakeTest', $reportNames, 'ReportTest_FakeTest is in reports list');
+        $this->assertContains(FakeTest::class, $reportNames, 'ReportTest_FakeTest is in reports list');
 
         //exclude one report
-        Report::add_excluded_reports('ReportTest_FakeTest');
+        Report::add_excluded_reports(FakeTest::class);
 
         $reports = Report::get_reports();
         $reportNames = array();
         foreach ($reports as $report) {
             $reportNames[] = get_class($report);
         }
-        $this->assertNotContains('ReportTest_FakeTest', $reportNames, 'ReportTest_FakeTest is NOT in reports list');
+        $this->assertNotContains(FakeTest::class, $reportNames, 'ReportTest_FakeTest is NOT in reports list');
 
         //exclude two reports
-        Report::add_excluded_reports(array('ReportTest_FakeTest', 'ReportTest_FakeTest2'));
+        Report::add_excluded_reports(array(FakeTest::class, FakeTest2::class));
 
         $reports = Report::get_reports();
         $reportNames = array();
         foreach ($reports as $report) {
             $reportNames[] = get_class($report);
         }
-        $this->assertNotContains('ReportTest_FakeTest', $reportNames, 'ReportTest_FakeTest is NOT in reports list');
+        $this->assertNotContains(FakeTest::class, $reportNames, 'ReportTest_FakeTest is NOT in reports list');
-        $this->assertNotContains('ReportTest_FakeTest2', $reportNames, 'ReportTest_FakeTest2 is NOT in reports list');
+        $this->assertNotContains(FakeTest2::class, $reportNames, 'ReportTest_FakeTest2 is NOT in reports list');
     }
 
     public function testAbstractClassesAreExcluded()
@@ -68,17 +65,19 @@ class ReportTest extends SapphireTest
         foreach ($reports as $report) {
             $reportNames[] = get_class($report);
         }
-        $this->assertNotContains('ReportTest_FakeTest_Abstract',
+        $this->assertNotContains(
+            'ReportTest_FakeTest_Abstract',
             $reportNames,
-            'ReportTest_FakeTest_Abstract is NOT in reports list as it is abstract');
+            'ReportTest_FakeTest_Abstract is NOT in reports list as it is abstract'
+        );
     }
 
     public function testPermissions()
     {
-        $report = new ReportTest_FakeTest2();
+        $report = new ReportTest\FakeTest2();
 
         // Visitor cannot view
-        Session::clear("loggedInAs");
+        $this->logOut();
         $this->assertFalse($report->canView());
 
         // Logged in user that cannot view reports
@@ -89,142 +88,24 @@ class ReportTest extends SapphireTest
         $this->logInWithPermission('CMS_ACCESS_ReportAdmin');
         $this->assertTrue($report->canView());
 
-		// Admin can view
+        // Admin can view
-		$this->logInWithPermission('ADMIN');
+        $this->logInWithPermission('ADMIN');
-		$this->assertTrue($report->canView());
+        $this->assertTrue($report->canView());
-	}
+    }
 
-	public function testColumnLink() {
+    public function testColumnLink()
-		$report = new ReportTest_FakeTest();
+    {
-		/** @var GridField $gridField */
+        $report = new ReportTest\FakeTest();
-		$gridField = $report->getReportField();
+        /** @var GridField $gridField */
-		/** @var GridFieldDataColumns $columns */
+        $gridField = $report->getReportField();
-		$columns = $gridField->getConfig()->getComponentByType(GridFieldDataColumns::class);
+        /** @var GridFieldDataColumns $columns */
+        $columns = $gridField->getConfig()->getComponentByType(GridFieldDataColumns::class);
 
-		$page = new ReportTest_FakeObject();
+        $page = new ReportTest\FakeObject();
-		$page->Title = 'My Object';
+        $page->Title = 'My Object';
-		$page->ID = 959547;
+        $page->ID = 959547;
 
-		$titleContent = $columns->getColumnContent($gridField, $page, 'Title');
+        $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);
-	}
-}
-
-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';
-    }
-}
-
-/**
- * @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;
-    }
-}
-
-/**
- * @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;
-    }
-}
-
-/**
- * @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;
     }
 }

tests/ReportTest/FakeObject.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace SilverStripe\Reports\Tests\ReportTest;
+
+use SilverStripe\Control\Controller;
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\CMSPreviewable;
+use SilverStripe\ORM\DataObject;
+
+class FakeObject extends DataObject implements CMSPreviewable, TestOnly
+{
+    private static $table_name = 'ReportTest_FakeObject';
+
+    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

@@ -0,0 +1,35 @@
+<?php
+
+namespace SilverStripe\Reports\Tests\ReportTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\ArrayList;
+use SilverStripe\Reports\Report;
+
+class 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

@@ -0,0 +1,34 @@
+<?php
+
+namespace SilverStripe\Reports\Tests\ReportTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\ArrayList;
+use SilverStripe\Reports\Report;
+
+class 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/FakeTestAbstract.php

@@ -0,0 +1,35 @@
+<?php
+
+namespace SilverStripe\Reports\Tests\ReportTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\ArrayList;
+use SilverStripe\Reports\Report;
+
+abstract class FakeTestAbstract 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;
+    }
+}