From 450283bd1beda3e676425a7810036ae33cd1802b Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Thu, 15 Feb 2018 12:06:50 +1300 Subject: [PATCH] FIX Apply PSR-2 linting ruleset and phpcs configuration --- phpcs.xml.dist | 10 +++ src/Control/GoogleSitemapController.php | 10 ++- src/Extensions/GoogleSitemapExtension.php | 5 +- .../GoogleSitemapSiteTreeExtension.php | 22 ++++-- src/GoogleSitemap.php | 32 +++++---- tests/GoogleSitemapTest.php | 69 ++++++++++++++----- tests/Model/TestDataObject.php | 1 - 7 files changed, 106 insertions(+), 43 deletions(-) create mode 100644 phpcs.xml.dist diff --git a/phpcs.xml.dist b/phpcs.xml.dist new file mode 100644 index 0000000..d834f9a --- /dev/null +++ b/phpcs.xml.dist @@ -0,0 +1,10 @@ + + + CodeSniffer ruleset for SilverStripe coding conventions. + + + + + + + diff --git a/src/Control/GoogleSitemapController.php b/src/Control/GoogleSitemapController.php index 5403074..31932b7 100644 --- a/src/Control/GoogleSitemapController.php +++ b/src/Control/GoogleSitemapController.php @@ -68,7 +68,11 @@ class GoogleSitemapController extends Controller $class = $this->unsanitiseClassName($this->request->param('ID')); $page = $this->request->param('OtherID'); - if (GoogleSitemap::enabled() && $class && $page && ($class == SiteTree::class || $class == 'GoogleSitemapRoute' || GoogleSitemap::is_registered($class))) { + if (GoogleSitemap::enabled() + && $class + && $page + && ($class == SiteTree::class || $class == 'GoogleSitemapRoute' || GoogleSitemap::is_registered($class)) + ) { $this->getResponse()->addHeader('Content-Type', 'application/xml; charset="utf-8"'); $this->getResponse()->addHeader('X-Robots-Tag', 'noindex'); @@ -78,9 +82,9 @@ class GoogleSitemapController extends Controller return array( 'Items' => $items ); - } else { - return new HTTPResponse('Page not found', 404); } + + return new HTTPResponse('Page not found', 404); } /** diff --git a/src/Extensions/GoogleSitemapExtension.php b/src/Extensions/GoogleSitemapExtension.php index 0f8ed29..0df7c24 100644 --- a/src/Extensions/GoogleSitemapExtension.php +++ b/src/Extensions/GoogleSitemapExtension.php @@ -42,8 +42,9 @@ class GoogleSitemapExtension extends DataExtension return false; } - // Allow override. In this case, since this can return multiple results, we'll use an "and" based policy. That is - // if any value is false then the current value will be false. Only only if all are true will we then return true. + // Allow override. In this case, since this can return multiple results, we'll use an "and" based policy. + // That is if any value is false then the current value will be false. Only only if all are true will we + // then return true. $override = $this->owner->invokeWithExtensions('alterCanIncludeInGoogleSitemap', $can); if ($override) { diff --git a/src/Extensions/GoogleSitemapSiteTreeExtension.php b/src/Extensions/GoogleSitemapSiteTreeExtension.php index 2a6ea29..e2d6e54 100644 --- a/src/Extensions/GoogleSitemapSiteTreeExtension.php +++ b/src/Extensions/GoogleSitemapSiteTreeExtension.php @@ -39,14 +39,26 @@ class GoogleSitemapSiteTreeExtension extends GoogleSitemapExtension $tabset = $fields->findOrMakeTab('Root.Settings'); $message = "

"; - $message .= sprintf(_t('GoogleSitemaps.METANOTEPRIORITY', "Manually specify a Google Sitemaps priority for this page (%s)"), - '?' + $message .= sprintf( + _t( + 'GoogleSitemaps.METANOTEPRIORITY', + "Manually specify a Google Sitemaps priority for this page (%s)" + ), + '?' ); $message .= "

"; - $tabset->push(new Tab('GoogleSitemap', _t('GoogleSitemaps.TABGOOGLESITEMAP', 'Google Sitemap'), - new LiteralField("GoogleSitemapIntro", $message), - $priority = new DropdownField("Priority", $this->owner->fieldLabel('Priority'), $prorities, $this->owner->Priority) + $tabset->push(new Tab( + 'GoogleSitemap', + _t('GoogleSitemaps.TABGOOGLESITEMAP', 'Google Sitemap'), + LiteralField::create("GoogleSitemapIntro", $message), + $priority = DropdownField::create( + "Priority", + $this->owner->fieldLabel('Priority'), + $prorities, + $this->owner->Priority + ) )); $priority->setEmptyString(_t('GoogleSitemaps.PRIORITYAUTOSET', 'Auto-set based on page depth')); diff --git a/src/GoogleSitemap.php b/src/GoogleSitemap.php index a1aa8ed..eee29ea 100644 --- a/src/GoogleSitemap.php +++ b/src/GoogleSitemap.php @@ -42,15 +42,15 @@ use ReflectionException; * e.g mysite/_config/googlesitemaps.yml * * - * --- - * Name: customgooglesitemaps - * After: googlesitemaps - * --- - * Wilr\GoogleSitemaps\GoogleSitemap: - * enabled: true - * objects_per_sitemap: 1000 - * google_notification_enabled: true - * use_show_in_search: true + * --- + * Name: customgooglesitemaps + * After: googlesitemaps + * --- + * Wilr\GoogleSitemaps\GoogleSitemap: + * enabled: true + * objects_per_sitemap: 1000 + * google_notification_enabled: true + * use_show_in_search: true * * * @see http://www.google.com/support/webmasters/bin/answer.py?hl=en&answer=34609 @@ -249,11 +249,11 @@ class GoogleSitemap if ($class == 'SilverStripe\CMS\Model\SiteTree') { $instances = Versioned::get_by_stage('SilverStripe\CMS\Model\SiteTree', 'Live'); - if($filter) { + if ($filter) { $instances = $instances->filter('ShowInSearch', 1); } - if($redirector) { + if ($redirector) { foreach (ClassInfo::subclassesFor('SilverStripe\\CMS\\Model\\RedirectorPage') as $redirectorClass) { $instances = $instances->exclude('ClassName', $redirectorClass); } @@ -473,15 +473,19 @@ class GoogleSitemap )); $googleResponse = self::send_ping( - "www.google.com", "/webmasters/sitemaps/ping", sprintf("sitemap=%s", $location) + "www.google.com", + "/webmasters/sitemaps/ping", + sprintf("sitemap=%s", $location) ); // bing $bing = Config::inst()->get(__CLASS__, 'bing_notification_enabled'); - if($bing) { + if ($bing) { $bingResponse = self::send_ping( - "www.bing.com", "/ping", sprintf("sitemap=%s", $location) + "www.bing.com", + "/ping", + sprintf("sitemap=%s", $location) ); } diff --git a/tests/GoogleSitemapTest.php b/tests/GoogleSitemapTest.php index 1adae6a..56f022b 100644 --- a/tests/GoogleSitemapTest.php +++ b/tests/GoogleSitemapTest.php @@ -2,22 +2,21 @@ namespace Wilr\GoogleSitemaps\Tests; +use Exception; +use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Director; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\FunctionalTest; -use SilverStripe\Dev\TestOnly; use SilverStripe\Forms\DropdownField; use SilverStripe\Forms\LiteralField; use SilverStripe\Forms\Tab; -use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\Versioned\Versioned; -use Wilr\GoogleSitemaps\GoogleSitemap; use Wilr\GoogleSitemaps\Extensions\GoogleSitemapExtension; -use Wilr\GoogleSitemaps\Tests\Model\TestDataObject; +use Wilr\GoogleSitemaps\GoogleSitemap; use Wilr\GoogleSitemaps\Tests\Model\OtherDataObject; +use Wilr\GoogleSitemaps\Tests\Model\TestDataObject; use Wilr\GoogleSitemaps\Tests\Model\UnviewableDataObject; -use Exception; class GoogleSitemapTest extends FunctionalTest { @@ -117,14 +116,32 @@ class GoogleSitemapTest extends FunctionalTest // the sitemap should contain to both those files and not the other // dataobject as it hasn't been registered - $expected = "". Director::absoluteURL("sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-TestDataObject/1") .""; - $this->assertEquals(1, substr_count($body, $expected), 'A link to GoogleSitemapTest_DataObject exists'); + $expected = "". Director::absoluteURL( + "sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-TestDataObject/1" + ) .""; + $this->assertEquals( + 1, + substr_count($body, $expected), + 'A link to GoogleSitemapTest_DataObject exists' + ); - $expected = "". Director::absoluteURL("sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-OtherDataObject/1") .""; - $this->assertEquals(1, substr_count($body, $expected), 'A link to GoogleSitemapTest_OtherDataObject exists'); + $expected = "". Director::absoluteURL( + "sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-OtherDataObject/1" + ) .""; + $this->assertEquals( + 1, + substr_count($body, $expected), + 'A link to GoogleSitemapTest_OtherDataObject exists' + ); - $expected = "". Director::absoluteURL("sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-UnviewableDataObject/2") .""; - $this->assertEquals(0, substr_count($body, $expected), 'A link to a GoogleSitemapTest_UnviewableDataObject does not exist'); + $expected = "". Director::absoluteURL( + "sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-UnviewableDataObject/2" + ) .""; + $this->assertEquals( + 0, + substr_count($body, $expected), + 'A link to a GoogleSitemapTest_UnviewableDataObject does not exist' + ); } public function testLastModifiedDateOnRootXML() @@ -151,7 +168,11 @@ class GoogleSitemapTest extends FunctionalTest $expected = '2014-03-14'; - $this->assertEquals(1, substr_count($body, $expected), 'The last mod date should use most recent LastEdited date'); + $this->assertEquals( + 1, + substr_count($body, $expected), + 'The last mod date should use most recent LastEdited date' + ); } public function testIndexFilePaginatedSitemapFiles() @@ -162,11 +183,23 @@ class GoogleSitemapTest extends FunctionalTest $response = $this->get('sitemap.xml'); $body = $response->getBody(); - $expected = "". Director::absoluteURL("sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-TestDataObject/1") .""; - $this->assertEquals(1, substr_count($body, $expected), 'A link to the first page of GoogleSitemapTest_DataObject exists'); + $expected = "". Director::absoluteURL( + "sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-TestDataObject/1" + ) .""; + $this->assertEquals( + 1, + substr_count($body, $expected), + 'A link to the first page of GoogleSitemapTest_DataObject exists' + ); - $expected = "". Director::absoluteURL("sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-TestDataObject/2") .""; - $this->assertEquals(1, substr_count($body, $expected), 'A link to the second page GoogleSitemapTest_DataObject exists'); + $expected = "". Director::absoluteURL( + "sitemap.xml/sitemap/Wilr-GoogleSitemaps-Tests-Model-TestDataObject/2" + ) .""; + $this->assertEquals( + 1, + substr_count($body, $expected), + 'A link to the second page GoogleSitemapTest_DataObject exists' + ); Config::inst()->update(GoogleSitemap::class, 'objects_per_sitemap', $original); } @@ -218,7 +251,7 @@ class GoogleSitemapTest extends FunctionalTest $this->assertDOSContains(array( array('Title' => 'Testpage1'), array('Title' => 'Testpage2') - ), GoogleSitemap::get_items('\SilverStripe\CMS\Model\SiteTree'), "There should be 2 pages in the sitemap after publishing"); + ), GoogleSitemap::inst()->getItems(SiteTree::class), "There should be 2 pages in the sitemap after publishing"); // check if we make a page readonly that it is hidden $page2->CanViewType = 'LoggedInUsers'; @@ -229,7 +262,7 @@ class GoogleSitemapTest extends FunctionalTest $this->assertDOSEquals(array( array('Title' => 'Testpage1') - ), GoogleSitemap::get_items('\SilverStripe\CMS\Model\SiteTree'), "There should be only 1 page, other is logged in only"); + ), GoogleSitemap::inst()->getItems(SiteTree::class), "There should be only 1 page, other is logged in only"); } public function testAccess() diff --git a/tests/Model/TestDataObject.php b/tests/Model/TestDataObject.php index a0f104a..cf28780 100644 --- a/tests/Model/TestDataObject.php +++ b/tests/Model/TestDataObject.php @@ -6,7 +6,6 @@ use SilverStripe\ORM\DataObject; use SilverStripe\Dev\TestOnly; use SilverStripe\Control\Director; - class TestDataObject extends DataObject implements TestOnly {