From 0de71d45dbfe506946e9a22713c412f9fbe4633f Mon Sep 17 00:00:00 2001 From: Nathan Glasl Date: Wed, 10 Jun 2015 11:37:27 +1000 Subject: [PATCH 1/6] Enforced the published/draft selection for SS reports, which was being ignored due to previous code refactoring. --- code/reports/BrokenLinksReport.php | 7 +- code/reports/SideReport.php | 27 ++- tests/reports/ReportTest.php | 239 ++++++++++++++++++++++++ tests/reports/SideReportTest.php | 286 +++++++++++++++++++++++++++++ 4 files changed, 540 insertions(+), 19 deletions(-) diff --git a/code/reports/BrokenLinksReport.php b/code/reports/BrokenLinksReport.php index f030bc30..6de26f8d 100644 --- a/code/reports/BrokenLinksReport.php +++ b/code/reports/BrokenLinksReport.php @@ -29,9 +29,8 @@ class BrokenLinksReport extends SS_Report { $sort = ''; } } - if (!isset($_REQUEST['CheckSite']) || $params['CheckSite'] == 'Published') $ret = Versioned::get_by_stage('SiteTree', 'Live', '("SiteTree"."HasBrokenLink" = 1 OR "SiteTree"."HasBrokenFile" = 1)', $sort, $join, $limit); - else $ret = DataObject::get('SiteTree', '("SiteTree"."HasBrokenFile" = 1 OR "HasBrokenLink" = 1)', $sort, $join, $limit); - + $ret = Versioned::get_by_stage('SiteTree', ((!isset($params['CheckSite']) || ($params['CheckSite'] == 'Published')) ? 'Live' : 'Stage'), '("SiteTree"."HasBrokenLink" = 1 OR "SiteTree"."HasBrokenFile" = 1)', $sort, $join, $limit); + $returnSet = new ArrayList(); if ($ret) foreach($ret as $record) { $reason = false; @@ -73,7 +72,7 @@ class BrokenLinksReport extends SS_Report { return $returnSet; } public function columns() { - if(isset($_REQUEST['CheckSite']) && $_REQUEST['CheckSite'] == 'Draft') { + if(isset($_REQUEST['filters']['CheckSite']) && $_REQUEST['filters']['CheckSite'] == 'Draft') { $dateTitle = _t('BrokenLinksReport.ColumnDateLastModified', 'Date last modified'); } else { $dateTitle = _t('BrokenLinksReport.ColumnDateLastPublished', 'Date last published'); diff --git a/code/reports/SideReport.php b/code/reports/SideReport.php index 3e119c65..c1b2fde0 100644 --- a/code/reports/SideReport.php +++ b/code/reports/SideReport.php @@ -199,9 +199,8 @@ class SideReport_BrokenLinks extends SS_Report { // Get class names for page types that are not virtual pages or redirector pages $classes = array_diff(ClassInfo::subclassesFor('SiteTree'), ClassInfo::subclassesFor('VirtualPage'), ClassInfo::subclassesFor('RedirectorPage')); $classNames = "'".join("','", $classes)."'"; - - if (isset($_REQUEST['OnLive'])) $ret = Versioned::get_by_stage('SiteTree', 'Live', "\"ClassName\" IN ($classNames) AND \"HasBrokenLink\" = 1"); - else $ret = DataObject::get('SiteTree', "\"ClassName\" IN ($classNames) AND \"HasBrokenLink\" = 1"); + + $ret = Versioned::get_by_stage('SiteTree', ((isset($params['OnLive']) && $params['OnLive']) ? 'Live' : 'Stage'), "\"ClassName\" IN ($classNames) AND \"HasBrokenLink\" = 1"); return $ret; } public function columns() { @@ -212,7 +211,7 @@ class SideReport_BrokenLinks extends SS_Report { ), ); } - public function getParameterFields() { + public function parameterFields() { return new FieldList( new CheckboxField('OnLive', _t('SideReport.ParameterLiveCheckbox', 'Check live site')) ); @@ -237,9 +236,8 @@ class SideReport_BrokenFiles extends SS_Report { // Get class names for page types that are not virtual pages or redirector pages $classes = array_diff(ClassInfo::subclassesFor('SiteTree'), ClassInfo::subclassesFor('VirtualPage'), ClassInfo::subclassesFor('RedirectorPage')); $classNames = "'".join("','", $classes)."'"; - - if (isset($_REQUEST['OnLive'])) $ret = Versioned::get_by_stage('SiteTree', 'Live', "\"ClassName\" IN ($classNames) AND \"HasBrokenFile\" = 1"); - else $ret = DataObject::get('SiteTree', "\"ClassName\" IN ($classNames) AND \"HasBrokenFile\" = 1"); + + $ret = Versioned::get_by_stage('SiteTree', ((isset($params['OnLive']) && $params['OnLive']) ? 'Live' : 'Stage'), "\"ClassName\" IN ($classNames) AND \"HasBrokenFile\" = 1"); return $ret; } public function columns() { @@ -251,7 +249,7 @@ class SideReport_BrokenFiles extends SS_Report { ); } - public function getParameterFields() { + public function parameterFields() { return new FieldList( new CheckboxField('OnLive', _t('SideReport.ParameterLiveCheckbox', 'Check live site')) ); @@ -271,8 +269,8 @@ class SideReport_BrokenVirtualPages extends SS_Report { } public function sourceRecords($params = null) { $classNames = "'".join("','", ClassInfo::subclassesFor('VirtualPage'))."'"; - if (isset($_REQUEST['OnLive'])) $ret = Versioned::get_by_stage('SiteTree', 'Live', "\"ClassName\" IN ($classNames) AND \"HasBrokenLink\" = 1"); - else $ret = DataObject::get('SiteTree', "\"ClassName\" IN ($classNames) AND \"HasBrokenLink\" = 1"); + + $ret = Versioned::get_by_stage('SiteTree', ((isset($params['OnLive']) && $params['OnLive']) ? 'Live' : 'Stage'), "\"ClassName\" IN ($classNames) AND \"HasBrokenLink\" = 1"); return $ret; } @@ -285,7 +283,7 @@ class SideReport_BrokenVirtualPages extends SS_Report { ); } - public function getParameterFields() { + public function parameterFields() { return new FieldList( new CheckboxField('OnLive', _t('SideReport.ParameterLiveCheckbox', 'Check live site')) ); @@ -305,9 +303,8 @@ class SideReport_BrokenRedirectorPages extends SS_Report { } public function sourceRecords($params = null) { $classNames = "'".join("','", ClassInfo::subclassesFor('RedirectorPage'))."'"; - - if (isset($_REQUEST['OnLive'])) $ret = Versioned::get_by_stage('SiteTree', 'Live', "\"ClassName\" IN ($classNames) AND \"HasBrokenLink\" = 1"); - else $ret = DataObject::get('SiteTree', "\"ClassName\" IN ($classNames) AND \"HasBrokenLink\" = 1"); + + $ret = Versioned::get_by_stage('SiteTree', ((isset($params['OnLive']) && $params['OnLive']) ? 'Live' : 'Stage'), "\"ClassName\" IN ($classNames) AND \"HasBrokenLink\" = 1"); return $ret; } @@ -320,7 +317,7 @@ class SideReport_BrokenRedirectorPages extends SS_Report { ); } - public function getParameterFields() { + public function parameterFields() { return new FieldList( new CheckboxField('OnLive', _t('SideReport.ParameterLiveCheckbox', 'Check live site')) ); diff --git a/tests/reports/ReportTest.php b/tests/reports/ReportTest.php index 67b5de30..80178103 100644 --- a/tests/reports/ReportTest.php +++ b/tests/reports/ReportTest.php @@ -2,6 +2,33 @@ class ReportTest extends SapphireTest { + /** + * ASSERT whether a report is returning the correct results, based on a broken "draft" and/or "published" page, both with and without the "reason". + * + * @parameter ss_report + * @parameter boolean + * @parameter boolean + * @parameter string + */ + + public function isReportBroken($report, $isDraftBroken, $isPublishedBroken, $reason) { + + $class = get_class($report); + $parameters = array(); + + // ASSERT that the "draft" report is returning the correct results, both with and without the "reason". + + $parameters['CheckSite'] = 'Draft'; + $results = (count($report->sourceRecords($parameters, null, null)) > 0) && (count($report->sourceRecords(array_merge($parameters, array('Reason' => $reason)), null, null)) > 0); + $isDraftBroken ? $this->assertTrue($results, "{$class} has NOT returned the correct DRAFT results, as NO pages were found.") : $this->assertFalse($results, "{$class} has NOT returned the correct DRAFT results, as pages were found."); + + // ASSERT that the "published" report is returning the correct results, both with and without the "reason". + + $parameters['CheckSite'] = 'Published'; + $results = (count($report->sourceRecords($parameters, null, null)) > 0) && (count($report->sourceRecords(array_merge($parameters, array('Reason' => $reason)), null, null)) > 0); + $isPublishedBroken ? $this->assertTrue($results, "{$class} has NOT returned the correct PUBLISHED results, as NO pages were found.") : $this->assertFalse($results, "{$class} has NOT returned the correct PUBLISHED results, as pages were found."); + } + public function testGetReports() { $reports = SS_Report::get_reports(); $this->assertNotNull($reports, "Reports returned"); @@ -52,6 +79,218 @@ class ReportTest extends SapphireTest { $reportNames, 'ReportTest_FakeTest_Abstract is NOT in reports list as it is abstract'); } + + /** + * Test the broken links report. + */ + + public function testBrokenLinksReport() { + + // --- + // BROKEN LINKS + // --- + + // Create a "draft" page with a broken link. + + $page = Page::create(); + $page->Content = "This is a broken link."; + $page->writeToStage('Stage'); + + // Retrieve the broken links report. + + $reports = SS_Report::get_reports(); + $brokenLinksReport = null; + foreach($reports as $report) { + if($report instanceof BrokenLinksReport) { + $brokenLinksReport = $report; + break; + } + } + + // Determine that the report exists, otherwise it has been excluded. + + if($brokenLinksReport) { + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link, as the page has not been "published" yet. + + $this->isReportBroken($brokenLinksReport, true, false, 'BROKENLINK'); + + // Make sure the page is now "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link. + + $this->isReportBroken($brokenLinksReport, true, true, 'BROKENLINK'); + + // Correct the "draft" broken link. + + $page->Content = str_replace('987654321', $page->ID, $page->Content); + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link, as the previous content remains "published". + + $this->isReportBroken($brokenLinksReport, false, true, 'BROKENLINK'); + + // Make sure the change has now been "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link. + + $this->isReportBroken($brokenLinksReport, false, false, 'BROKENLINK'); + $page->delete(); + + // --- + // BROKEN FILES + // --- + + // Create a "draft" page with a broken file. + + $page = Page::create(); + $page->Content = "This is a broken file."; + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has detected the page having a broken file. + // ASSERT that the "published" report has NOT detected the page having a broken file, as the page has not been "published" yet. + + $this->isReportBroken($brokenLinksReport, true, false, 'BROKENFILE'); + + // Make sure the page is now "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has detected the page having a broken file. + // ASSERT that the "published" report has detected the page having a broken file. + + $this->isReportBroken($brokenLinksReport, true, true, 'BROKENFILE'); + + // Correct the "draft" broken file. + + $file = File::create(); + $file->Filename = 'name.pdf'; + $file->write(); + $page->Content = str_replace('987654321', $file->ID, $page->Content); + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has NOT detected the page having a broken file. + // ASSERT that the "published" report has detected the page having a broken file, as the previous content remains "published". + + $this->isReportBroken($brokenLinksReport, false, true, 'BROKENFILE'); + + // Make sure the change has now been "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has NOT detected the page having a broken file. + // ASSERT that the "published" report has NOT detected the page having a broken file. + + $this->isReportBroken($brokenLinksReport, false, false, 'BROKENFILE'); + $page->delete(); + + // --- + // BROKEN VIRTUAL PAGES + // --- + + // Create a "draft" virtual page with a broken link. + + $page = VirtualPage::create(); + $page->CopyContentFromID = 987654321; + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link, as the page has not been "published" yet. + + $this->isReportBroken($brokenLinksReport, true, false, 'VPBROKENLINK'); + + // Make sure the page is now "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link. + + $this->isReportBroken($brokenLinksReport, true, true, 'VPBROKENLINK'); + + // Correct the "draft" broken link. + + $contentPage = Page::create(); + $contentPage->Content = 'This is some content.'; + $contentPage->writeToStage('Stage'); + $contentPage->writeToStage('Live'); + $page->CopyContentFromID = $contentPage->ID; + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link, as the previous content remains "published". + + $this->isReportBroken($brokenLinksReport, false, true, 'VPBROKENLINK'); + + // Make sure the change has now been "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link. + + $this->isReportBroken($brokenLinksReport, false, false, 'VPBROKENLINK'); + $contentPage->delete(); + $page->delete(); + + // --- + // BROKEN REDIRECTOR PAGES + // --- + + // Create a "draft" redirector page with a broken link. + + $page = RedirectorPage::create(); + $page->RedirectionType = 'Internal'; + $page->LinkToID = 987654321; + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link, as the page has not been "published" yet. + + $this->isReportBroken($brokenLinksReport, true, false, 'RPBROKENLINK'); + + // Make sure the page is now "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link. + + $this->isReportBroken($brokenLinksReport, true, true, 'RPBROKENLINK'); + + // Correct the "draft" broken link. + + $contentPage = Page::create(); + $contentPage->Content = 'This is some content.'; + $contentPage->writeToStage('Stage'); + $contentPage->writeToStage('Live'); + $page->LinkToID = $contentPage->ID; + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link, as the previous content remains "published". + + $this->isReportBroken($brokenLinksReport, false, true, 'RPBROKENLINK'); + + // Make sure the change has now been "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link. + + $this->isReportBroken($brokenLinksReport, false, false, 'RPBROKENLINK'); + } + } + } class ReportTest_FakeTest extends SS_Report implements TestOnly { diff --git a/tests/reports/SideReportTest.php b/tests/reports/SideReportTest.php index 1bd557dc..3f8e0e68 100644 --- a/tests/reports/SideReportTest.php +++ b/tests/reports/SideReportTest.php @@ -24,6 +24,31 @@ class SideReportTest extends SapphireTest { DB::query("UPDATE \"SiteTree\" SET \"Created\"='2009-01-01 00:00:00', \"LastEdited\"='".date('Y-m-d H:i:s', $beforeThreshold)."' WHERE \"ID\"='".$before->ID."'"); } + /** + * ASSERT whether a report is returning the correct results, based on a broken "draft" and/or "published" page. + * + * @parameter ss_report + * @parameter boolean + * @parameter boolean + */ + + public function isReportBroken($report, $isDraftBroken, $isPublishedBroken) { + + $class = get_class($report); + + // ASSERT that the "draft" report is returning the correct results. + + $results = count($report->sourceRecords(array())) > 0; + $isDraftBroken ? $this->assertTrue($results, "{$class} has NOT returned the correct DRAFT results, as NO pages were found.") : $this->assertFalse($results, "{$class} has NOT returned the correct DRAFT results, as pages were found."); + + // ASSERT that the "published" report is returning the correct results. + + $results = count($report->sourceRecords(array( + 'OnLive' => 1 + ))) > 0; + $isPublishedBroken ? $this->assertTrue($results, "{$class} has NOT returned the correct PUBLISHED results, as NO pages were found.") : $this->assertFalse($results, "{$class} has NOT returned the correct PUBLISHED results, as pages were found."); + } + public function testRecentlyEdited() { SS_Datetime::set_mock_now('31-06-2009 00:00:00'); @@ -39,4 +64,265 @@ class SideReportTest extends SapphireTest { SS_DateTime::clear_mock_now(); } + + /** + * Test the broken links side report. + */ + + public function testBrokenLinks() { + + // Create a "draft" page with a broken link. + + $page = Page::create(); + $page->Content = "This is a broken link."; + $page->writeToStage('Stage'); + + // Retrieve the broken links side report. + + $reports = SS_Report::get_reports(); + $brokenLinksReport = null; + foreach($reports as $report) { + if($report instanceof SideReport_BrokenLinks) { + $brokenLinksReport = $report; + break; + } + } + + // Determine that the report exists, otherwise it has been excluded. + + if($brokenLinksReport) { + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link, as the page has not been "published" yet. + + $this->isReportBroken($brokenLinksReport, true, false); + + // Make sure the page is now "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link. + + $this->isReportBroken($brokenLinksReport, true, true); + + // Correct the "draft" broken link. + + $page->Content = str_replace('987654321', $page->ID, $page->Content); + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link, as the previous content remains "published". + + $this->isReportBroken($brokenLinksReport, false, true); + + // Make sure the change has now been "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link. + + $this->isReportBroken($brokenLinksReport, false, false); + } + } + + /** + * Test the broken files side report. + */ + + public function testBrokenFiles() { + + // Create a "draft" page with a broken file. + + $page = Page::create(); + $page->Content = "This is a broken file."; + $page->writeToStage('Stage'); + + // Retrieve the broken files side report. + + $reports = SS_Report::get_reports(); + $brokenFilesReport = null; + foreach($reports as $report) { + if($report instanceof SideReport_BrokenFiles) { + $brokenFilesReport = $report; + break; + } + } + + // Determine that the report exists, otherwise it has been excluded. + + if($brokenFilesReport) { + + // ASSERT that the "draft" report has detected the page having a broken file. + // ASSERT that the "published" report has NOT detected the page having a broken file, as the page has not been "published" yet. + + $this->isReportBroken($brokenFilesReport, true, false); + + // Make sure the page is now "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has detected the page having a broken file. + // ASSERT that the "published" report has detected the page having a broken file. + + $this->isReportBroken($brokenFilesReport, true, true); + + // Correct the "draft" broken file. + + $file = File::create(); + $file->Filename = 'name.pdf'; + $file->write(); + $page->Content = str_replace('987654321', $file->ID, $page->Content); + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has NOT detected the page having a broken file. + // ASSERT that the "published" report has detected the page having a broken file, as the previous content remains "published". + + $this->isReportBroken($brokenFilesReport, false, true); + + // Make sure the change has now been "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has NOT detected the page having a broken file. + // ASSERT that the "published" report has NOT detected the page having a broken file. + + $this->isReportBroken($brokenFilesReport, false, false); + } + } + + /** + * Test the broken virtual pages side report. + */ + + public function testBrokenVirtualPages() { + + // Create a "draft" virtual page with a broken link. + + $page = VirtualPage::create(); + $page->CopyContentFromID = 987654321; + $page->writeToStage('Stage'); + + // Retrieve the broken virtual pages side report. + + $reports = SS_Report::get_reports(); + $brokenVirtualPagesReport = null; + foreach($reports as $report) { + if($report instanceof SideReport_BrokenVirtualPages) { + $brokenVirtualPagesReport = $report; + break; + } + } + + // Determine that the report exists, otherwise it has been excluded. + + if($brokenVirtualPagesReport) { + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link, as the page has not been "published" yet. + + $this->isReportBroken($brokenVirtualPagesReport, true, false); + + // Make sure the page is now "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link. + + $this->isReportBroken($brokenVirtualPagesReport, true, true); + + // Correct the "draft" broken link. + + $contentPage = Page::create(); + $contentPage->Content = 'This is some content.'; + $contentPage->writeToStage('Stage'); + $contentPage->writeToStage('Live'); + $page->CopyContentFromID = $contentPage->ID; + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link, as the previous content remains "published". + + $this->isReportBroken($brokenVirtualPagesReport, false, true); + + // Make sure the change has now been "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link. + + $this->isReportBroken($brokenVirtualPagesReport, false, false); + } + } + + /** + * Test the broken redirector pages side report. + */ + + public function testBrokenRedirectorPages() { + + // Create a "draft" redirector page with a broken link. + + $page = RedirectorPage::create(); + $page->RedirectionType = 'Internal'; + $page->LinkToID = 987654321; + $page->writeToStage('Stage'); + + // Retrieve the broken redirector pages side report. + + $reports = SS_Report::get_reports(); + $brokenRedirectorPagesReport = null; + foreach($reports as $report) { + if($report instanceof SideReport_BrokenRedirectorPages) { + $brokenRedirectorPagesReport = $report; + break; + } + } + + // Determine that the report exists, otherwise it has been excluded. + + if($brokenRedirectorPagesReport) { + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link, as the page has not been "published" yet. + + $this->isReportBroken($brokenRedirectorPagesReport, true, false); + + // Make sure the page is now "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link. + + $this->isReportBroken($brokenRedirectorPagesReport, true, true); + + // Correct the "draft" broken link. + + $contentPage = Page::create(); + $contentPage->Content = 'This is some content.'; + $contentPage->writeToStage('Stage'); + $contentPage->writeToStage('Live'); + $page->LinkToID = $contentPage->ID; + $page->writeToStage('Stage'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has detected the page having a broken link, as the previous content remains "published". + + $this->isReportBroken($brokenRedirectorPagesReport, false, true); + + // Make sure the change has now been "published". + + $page->writeToStage('Live'); + + // ASSERT that the "draft" report has NOT detected the page having a broken link. + // ASSERT that the "published" report has NOT detected the page having a broken link. + + $this->isReportBroken($brokenRedirectorPagesReport, false, false); + } + } + } From b71a521c21bb15a5dde007088741bb0c567a8553 Mon Sep 17 00:00:00 2001 From: Stephen Shkardoon Date: Thu, 21 May 2015 20:51:07 +1200 Subject: [PATCH 2/6] Fix incorrect permission check on duplicate() Will now properly fall back to the canCreate() on the parent --- code/controllers/CMSMain.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/code/controllers/CMSMain.php b/code/controllers/CMSMain.php index f5225cbd..ad3311f8 100644 --- a/code/controllers/CMSMain.php +++ b/code/controllers/CMSMain.php @@ -1316,7 +1316,9 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr if(($id = $this->urlParams['ID']) && is_numeric($id)) { $page = DataObject::get_by_id("SiteTree", $id); - if($page && (!$page->canEdit() || !$page->canCreate())) return Security::permissionFailure($this); + if($page && (!$page->canEdit() || !$page->canCreate(null, array('Parent' => $page->Parent())))) { + return Security::permissionFailure($this); + } if(!$page || !$page->ID) throw new SS_HTTPResponse_Exception("Bad record ID #$id", 404); $newPage = $page->duplicate(); @@ -1352,7 +1354,9 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr increase_time_limit_to(); if(($id = $this->urlParams['ID']) && is_numeric($id)) { $page = DataObject::get_by_id("SiteTree", $id); - if($page && (!$page->canEdit() || !$page->canCreate())) return Security::permissionFailure($this); + if($page && (!$page->canEdit() || !$page->canCreate(null, array('Parent' => $page->Parent())))) { + return Security::permissionFailure($this); + } if(!$page || !$page->ID) throw new SS_HTTPResponse_Exception("Bad record ID #$id", 404); $newPage = $page->duplicateWithChildren(); From d78d3250736c5d2f48c5cfc1690fba8b98cc222b Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Fri, 26 Jun 2015 10:40:43 +0100 Subject: [PATCH 3/6] FIX: RedirectorPage_Controller shouldn't attempt redirection if the response is finished (fixes #1230) --- code/model/RedirectorPage.php | 3 ++- tests/model/RedirectorPageTest.php | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/code/model/RedirectorPage.php b/code/model/RedirectorPage.php index 55021b23..56fb76f2 100644 --- a/code/model/RedirectorPage.php +++ b/code/model/RedirectorPage.php @@ -161,7 +161,8 @@ class RedirectorPage_Controller extends Page_Controller { public function init() { parent::init(); - if($link = $this->redirectionLink()) { + // Check we don't already have a redirect code set + if(!$this->getResponse()->isFinished() && $link = $this->redirectionLink()) { $this->redirect($link, 301); return; } diff --git a/tests/model/RedirectorPageTest.php b/tests/model/RedirectorPageTest.php index 24afdab8..5bb7f90f 100644 --- a/tests/model/RedirectorPageTest.php +++ b/tests/model/RedirectorPageTest.php @@ -3,6 +3,7 @@ class RedirectorPageTest extends FunctionalTest { protected static $fixture_file = 'RedirectorPageTest.yml'; protected static $use_draft_site = true; + protected $autoFollowRedirection = false; public function testGoodRedirectors() { /* For good redirectors, the final destination URL will be returned */ @@ -50,4 +51,26 @@ class RedirectorPageTest extends FunctionalTest { $page->write(); $this->assertEquals($page->ExternalURL, 'http://google.com', 'onBeforeWrite will not double prefix if written again!'); } + + /** + * Test that we can trigger a redirection before RedirectorPage_Controller::init() is called + */ + public function testRedirectRespectsFinishedResponse() { + $page = $this->objFromFixture('RedirectorPage', 'goodinternal'); + RedirectorPage_Controller::add_extension('RedirectorPageTest_RedirectExtension'); + + $response = $this->get($page->regularLink()); + $this->assertEquals(302, $response->getStatusCode()); + $this->assertEquals('/foo', $response->getHeader('Location')); + + RedirectorPage_Controller::remove_extension('RedirectorPageTest_RedirectExtension'); + } +} + +class RedirectorPageTest_RedirectExtension extends Extension implements TestOnly { + + public function onBeforeInit() { + $this->owner->redirect('/foo'); + } + } From f9cceaada0b9390713a6104c458282f7144500ce Mon Sep 17 00:00:00 2001 From: Fred Condo Date: Wed, 1 Jul 2015 17:42:26 -0700 Subject: [PATCH 4/6] Correct doubly-escaped " marks The links in the Broken Links Report had extraneous " marks that made them unusable. This fixes the links. --- code/reports/BrokenLinksReport.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/reports/BrokenLinksReport.php b/code/reports/BrokenLinksReport.php index 6de26f8d..5cdf46cb 100644 --- a/code/reports/BrokenLinksReport.php +++ b/code/reports/BrokenLinksReport.php @@ -83,7 +83,7 @@ class BrokenLinksReport extends SS_Report { "Title" => array( "title" => _t('BrokenLinksReport.PageName', 'Page name'), 'formatting' => function($value, $item) use ($linkBase) { - return sprintf('%s', + return sprintf('%s', Controller::join_links($linkBase, $item->ID), _t('BrokenLinksReport.HoverTitleEditPage', 'Edit page'), $value From 4c050b727bd6de029b65844b59c197c6cede1747 Mon Sep 17 00:00:00 2001 From: torleif Date: Wed, 15 Jul 2015 09:17:29 +1200 Subject: [PATCH 5/6] Update CONTRIBUTING.md [Fix] dead links for 3.1 --- CONTRIBUTING.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 485fb7c5..94e3290c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,7 +6,7 @@ See our [high level overview](http://silverstripe.org/contributing-to-silverstri Or, for more detailed guidance, read one of the following pages: - * [Sharing your opinion and raising issues](http://doc.silverstripe.org/framework/en/trunk/misc/contributing/issues) - * [Providing code, whether it's creating a feature or fixing a bug](http://doc.silverstripe.org/framework/en/trunk/misc/contributing/code) - * [Writing and translating documentation](http://doc.silverstripe.org/framework/en/trunk/misc/contributing/documentation) - * [Translating user-interface elements](http://doc.silverstripe.org/framework/en/trunk/misc/contributing/translation) \ No newline at end of file + * [Sharing your opinion and raising issues](http://docs.silverstripe.org/en/3.1/contributing/issues_and_bugs/) + * [Providing code, whether it's creating a feature or fixing a bug](http://docs.silverstripe.org/en/3.1/contributing/code/) + * [Writing and translating documentation](http://docs.silverstripe.org/en/3.1/contributing/translations/) + * [Translating user-interface elements](http://docs.silverstripe.org/en/3.1/contributing/translation_process/) From ce8576429219d28d82d9ad4e266a58073818e6ca Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Sat, 18 Jul 2015 15:38:44 +1200 Subject: [PATCH 6/6] Behat tests hotlinking to non-existant image Caused by silverstripe.com redesign. Ideally we'd have a permanent asset hosted there which is clearly marked as a test dependency. Or create a placeholder for images linked served localhost within the Behat tests, in which case we wouldn't require a network connection to execute Behat. --- tests/behat/features/insert-an-image.feature | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/behat/features/insert-an-image.feature b/tests/behat/features/insert-an-image.feature index 7658f2f3..4414cc1a 100644 --- a/tests/behat/features/insert-an-image.feature +++ b/tests/behat/features/insert-an-image.feature @@ -17,12 +17,12 @@ Feature: Insert an image into a page Then I should see "Choose files to upload..." When I press the "From the web" button - And I fill in "RemoteURL" with "http://www.silverstripe.com/themes/sscom/images/silverstripe_logo_web.png" + And I fill in "RemoteURL" with "http://www.silverstripe.org/themes/ssv3/img/ss_logo.png" And I press the "Add url" button - Then I should see "silverstripe_logo_web.png (www.silverstripe.com)" in the ".ss-assetuploadfield span.name" element + Then I should see "ss_logo.png (www.silverstripe.org)" in the ".ss-assetuploadfield span.name" element When I press the "Insert" button - Then the "Content" HTML field should contain "silverstripe_logo_web.png" + Then the "Content" HTML field should contain "ss_logo.png" # Required to avoid "unsaved changed" browser dialog Then I press the "Save draft" button