diff --git a/.travis.yml b/.travis.yml index 1f60706a..060bbb02 100644 --- a/.travis.yml +++ b/.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 diff --git a/code/Report.php b/code/Report.php index 9e29b763..514c8feb 100644 --- a/code/Report.php +++ b/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 - * - * @param string $class - * @return string - */ - protected function sanitiseClassName($class) { - return str_replace('\\', '-', $class); - } + /** + * Sanitise a model class' name for inclusion in a link + * + * @param string $class + * @return string + */ + protected function sanitiseClassName($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) { diff --git a/code/ReportAdmin.php b/code/ReportAdmin.php index c7024cfd..db026cb5 100644 --- a/code/ReportAdmin.php +++ b/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 - * - * @param string $class - * @return string - */ - protected function unsanitiseClassName($class) { - return str_replace('-', '\\', $class); - } + /** + * Unsanitise a model class' name from a URL param + * + * @param string $class + * @return string + */ + protected function unsanitiseClassName($class) + { + return str_replace('-', '\\', $class); + } /** * Determine if we have reports and need diff --git a/code/SideReport.php b/code/SideReport.php index 24e89d90..171ef960 100644 --- a/code/SideReport.php +++ b/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 - //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... + // Cast value 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 diff --git a/composer.json b/composer.json index 31852495..13e18394 100644 --- a/composer.json +++ b/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": { - "phpunit/PHPUnit": "~4.8" + "scripts": { + "lint": "phpcs code/ tests/", + "lint-clean": "phpcbf code/ tests/" } } diff --git a/phpcs.xml.dist b/phpcs.xml.dist new file mode 100644 index 00000000..9276cd5a --- /dev/null +++ b/phpcs.xml.dist @@ -0,0 +1,22 @@ + + + CodeSniffer ruleset for SilverStripe coding conventions. + + + + + + + + + + + + + + + + + + + diff --git a/tests/ReportTest.php b/tests/ReportTest.php index ee4a73bd..b6ac5fc3 100644 --- a/tests/ReportTest.php +++ b/tests/ReportTest.php @@ -1,23 +1,20 @@ 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('ReportTest_FakeTest2', $reportNames, 'ReportTest_FakeTest2 is NOT in reports list'); + $this->assertNotContains(FakeTest::class, $reportNames, 'ReportTest_FakeTest 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 - $this->logInWithPermission('ADMIN'); - $this->assertTrue($report->canView()); - } + // Admin can view + $this->logInWithPermission('ADMIN'); + $this->assertTrue($report->canView()); + } - public function testColumnLink() { - $report = new ReportTest_FakeTest(); - /** @var GridField $gridField */ - $gridField = $report->getReportField(); - /** @var GridFieldDataColumns $columns */ - $columns = $gridField->getConfig()->getComponentByType(GridFieldDataColumns::class); + public function testColumnLink() + { + $report = new ReportTest\FakeTest(); + /** @var GridField $gridField */ + $gridField = $report->getReportField(); + /** @var GridFieldDataColumns $columns */ + $columns = $gridField->getConfig()->getComponentByType(GridFieldDataColumns::class); - $page = new ReportTest_FakeObject(); - $page->Title = 'My Object'; - $page->ID = 959547; + $page = new ReportTest\FakeObject(); + $page->Title = 'My Object'; + $page->ID = 959547; - $titleContent = $columns->getColumnContent($gridField, $page, 'Title'); - $this->assertEquals('My Object', $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; + $titleContent = $columns->getColumnContent($gridField, $page, 'Title'); + $this->assertEquals('My Object', $titleContent); } } diff --git a/tests/ReportTest/FakeObject.php b/tests/ReportTest/FakeObject.php new file mode 100644 index 00000000..9951b172 --- /dev/null +++ b/tests/ReportTest/FakeObject.php @@ -0,0 +1,41 @@ + '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'; + } +} diff --git a/tests/ReportTest/FakeTest.php b/tests/ReportTest/FakeTest.php new file mode 100644 index 00000000..7194ba55 --- /dev/null +++ b/tests/ReportTest/FakeTest.php @@ -0,0 +1,35 @@ + array( + "title" => "Page Title", + "link" => true, + ) + ); + } + + public function sourceRecords($params, $sort, $limit) + { + return new ArrayList(); + } + + public function sort() + { + return 100; + } +} diff --git a/tests/ReportTest/FakeTest2.php b/tests/ReportTest/FakeTest2.php new file mode 100644 index 00000000..736de851 --- /dev/null +++ b/tests/ReportTest/FakeTest2.php @@ -0,0 +1,34 @@ + array( + "title" => "Page Title 2" + ) + ); + } + + public function sourceRecords($params, $sort, $limit) + { + return new ArrayList(); + } + + public function sort() + { + return 98; + } +} diff --git a/tests/ReportTest/FakeTestAbstract.php b/tests/ReportTest/FakeTestAbstract.php new file mode 100644 index 00000000..271cd815 --- /dev/null +++ b/tests/ReportTest/FakeTestAbstract.php @@ -0,0 +1,35 @@ + array( + "title" => "Page Title Abstract" + ) + ); + } + + public function sourceRecords($params, $sort, $limit) + { + return new ArrayList(); + } + + public function sort() + { + return 5; + } +}