ENH: compatability with other modules

code/compatability/ContentReviewCompatability.php

@@ -0,0 +1,46 @@
+<?php 
+
+/**
+ * This is a helper class which lets us do things with content review data without subsites 
+ * and translatable messing our SQL queries up.
+ * 
+ * Make sure any DataQuery-ies you are building are BOTH created & executed between start() 
+ * and done() because augmentDataQueryCreate and augmentSQL happens there.
+ */
+class ContentReviewCompatability {
+
+	const SUBSITES     = 0;
+	const TRANSLATABLE = 1;
+	
+	/**
+	 * @return array - state before compatability mode started, to be passed into done().
+	 */
+	public static function start(){
+		$compat = array(
+			self::SUBSITES     => null,
+			self::TRANSLATABLE => null
+		);
+		if(ClassInfo::exists('Subsite')){
+			$compat[self::SUBSITES] = Subsite::$disable_subsite_filter;
+			Subsite::disable_subsite_filter(true);
+		}
+		if(ClassInfo::exists('Translatable')){
+			$compat[self::TRANSLATABLE] = Translatable::locale_filter_enabled();
+			Translatable::disable_locale_filter();
+		}
+		return $compat;
+	}
+	
+	/**
+	 * @param array $compat - see start()
+	 */
+	public static function done(array $compat){
+		if(class_exists('Subsite')){
+			Subsite::$disable_subsite_filter = $compat[self::SUBSITES];
+		}
+		if(class_exists('Translatable')){
+			Translatable::enable_locale_filter($compat[self::TRANSLATABLE]);
+		}
+	}
+	
+}

code/extensions/ContentReviewCMSExtension.php

@@ -70,7 +70,7 @@ class ContentReviewCMSExtension extends LeftAndMainExtension {
 		$SQL_id = Convert::raw2sql($data['ID']);
 		$page = SiteTree::get()->byID($SQL_id);
 		if($page && !$page->canEdit()) {
-			return Security::permissionFailure($this);
+			return Security::permissionFailure();
 		}
 		if(!$page || !$page->ID) {
 			throw new SS_HTTPResponse_Exception("Bad record ID #$SQL_id", 404);

code/extensions/SiteTreeContentReview.php

@@ -443,11 +443,13 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
 		
 		//
 		if($this->owner->isChanged('NextReviewDate', 2)) {
+			$c = ContentReviewCompatability::start();
 			$children = $this->owner->stageChildren(true)->filter('ContentReviewType', 'Inherit');
 			foreach($children as $child) {
 				$child->NextReviewDate = $this->owner->NextReviewDate;
 				$child->write();
 			}
+			ContentReviewCompatability::done($c);
 		}
 	}
 	

code/reports/PagesDueForReviewReport.php

@@ -114,12 +114,11 @@ class PagesDueForReviewReport extends SS_Report {
 	/**
 	 * 
 	 * @param array $params
-	 * @param string $sort
-	 * @param array $limit
 	 * @return SS_List
 	 */
-	public function sourceRecords($params, $sort, $limit) {
+	public function sourceRecords($params = array()) {
 		Versioned::reading_stage('Stage');
+		$c = ContentReviewCompatability::start();
 		$records = SiteTree::get();
 
 		if(empty($params['ReviewDateBefore']) && empty($params['ReviewDateAfter'])) {
@@ -159,18 +158,8 @@ class PagesDueForReviewReport extends SS_Report {
 			$records = $records->filter('OwnerNames:PartialMatch', $ownerNames);
 		}
 		
-		// Make sure we get results from all subsites, because only the main site has the Reports section
-		$subsiteFilterBefore = null;
-		if(class_exists('Subsite')){
-			$subsiteFilterBefore = Subsite::disable_subsite_filter(true);
-		}
-		
 		$r = new ArrayList($records->sort('NextReviewDate', 'DESC')->toArray());
-		
+		ContentReviewCompatability::done($c);
-		if(class_exists('Subsite')){
-			Subsite::disable_subsite_filter($subsiteFilterBefore);
-		}
-		
 		return $r;
 	}
 }

code/reports/PagesWithoutReviewScheduleReport.php

@@ -88,12 +88,11 @@ class PagesWithoutReviewScheduleReport extends SS_Report {
 	/**
 	 * 
 	 * @param array $params
-	 * @param string $sort
-	 * @param array $limit
 	 * @return SS_List
 	 */
-	public function sourceRecords($params, $sort, $limit) {
+	public function sourceRecords($params = array()) {
 		Versioned::reading_stage('Stage');
+		$c = ContentReviewCompatability::start();
 		$records = SiteTree::get();
 
 		// If there's no review dates set, default to all pages due for review now
@@ -108,22 +107,9 @@ class PagesWithoutReviewScheduleReport extends SS_Report {
 			));
 		}
 		
-		
 		$records->sort('ParentID');
-		
-		// Make sure we get results from all subsites, because only the main site has the Reports section
-		$subsiteFilterBefore = null;
-		if(class_exists('Subsite')){
-			$subsiteFilterBefore = Subsite::disable_subsite_filter(true);
-		}
-		
 		$records = $records->toArray();
 		
-		if(class_exists('Subsite')){
-			Subsite::disable_subsite_filter($subsiteFilterBefore);
-		}
-		
-		
 		// Trim out calculated values
 		$list = new ArrayList();
 		foreach($records as $record) {
@@ -131,6 +117,8 @@ class PagesWithoutReviewScheduleReport extends SS_Report {
 				$list->push($record);
 			}
 		}
+		
+		ContentReviewCompatability::done($c);
 		return $list;
 	}
 	

code/tasks/ContentReviewEmails.php

@@ -22,31 +22,21 @@ class ContentReviewEmails extends BuildTask {
 	 * @param SS_HTTPRequest $request
 	 */
 	public function run($request) {
-		// Disable subsite filter (if installed)
+		$c = ContentReviewCompatability::start();
-		if (ClassInfo::exists('Subsite')) {
-			$oldSubsiteState = Subsite::$disable_subsite_filter;
-			Subsite::$disable_subsite_filter = true;
-		}
-		
-		$overduePages = array();
 		
 		$now = class_exists('SS_Datetime') ? SS_Datetime::now()->URLDate() : SSDatetime::now()->URLDate();
 		
 		// First grab all the pages with a custom setting
 		$pages = Page::get('Page')->where('"SiteTree"."NextReviewDate" <= \''.$now.'\'');
 		
-		$this->getOverduePagesForOwners($pages, $overduePages);
+		$overduePages = $this->getOverduePagesForOwners($pages);
-		
 		
 		// Lets send one email to one owner with all the pages in there instead of no of pages of emails
 		foreach($overduePages as $memberID => $pages) {
 			$this->notifyOwner($memberID, $pages);
 		}
 		
-		// Revert subsite filter (if installed)
+		ContentReviewCompatability::done($c);
-		if(ClassInfo::exists('Subsite')) {
-			Subsite::$disable_subsite_filter = $oldSubsiteState;
-		}
 	}
 	
 	/**
@@ -55,7 +45,8 @@ class ContentReviewEmails extends BuildTask {
 	 * @param array &$pages
 	 * @return array
 	 */
-	protected function getOverduePagesForOwners(SS_list $pages, array &$overduePages) {
+	protected function getOverduePagesForOwners(SS_list $pages) {
+		$overduePages = array();
 		
 		foreach($pages as $page) {
 			if(!$page->canBeReviewedBy()) {

tests/ContentReviewBaseTest.php

@@ -0,0 +1,36 @@
+<?php 
+
+/**
+ * Extend this class when writing unit tests which are compatable with other modules. All
+ * compatability code goes here.
+ */
+abstract class ContentReviewBaseTest extends FunctionalTest {
+	
+	protected $translatableEnabledBefore;
+	
+	public function setUp(){
+		parent::setUp();
+		/*
+		 *  We set the locale for pages explicitly, because if we don't, then we get into a situation
+		 *  where the page takes on the tester's (your) locale, and any calls to simulate subsequent requests
+		 *  (e.g. $this->post()) do not seem to get passed the tester's locale, but instead fallback to the default locale.
+		 *
+		 *  So we set the pages locale to be the default locale, which will then match any subsequent requests.
+		 *  
+		 *  If creating pages in your unit tests (rather than reading from the fixtures file), you must explicitly call
+		 *  self::compat() on the page, for the same reasons as above.
+		*/
+		if(class_exists('Translatable')){
+			fwrite(STDOUT, 'TRANSLATABLE DISABLED FFS');
+			$this->translatableEnabledBefore = Translatable::is_enabled();
+			Translatable::disable();
+		}
+	}
+	
+	public function tearDown(){
+		if(class_exists('Translatable')){
+			if($this->translatableEnabledBefore) Translatable::enable();
+		}
+	}
+	
+}

tests/ContentReviewCMSPageEditControllerTest.php

@@ -1,6 +1,6 @@
 <?php
 
-class ContentReviewCMSPageEditControllerTest extends FunctionalTest {
+class ContentReviewCMSPageEditControllerTest extends ContentReviewBaseTest {
 	
 	public static $fixture_file = 'contentreview/tests/ContentReviewTest.yml';
 	
@@ -43,7 +43,7 @@ class ContentReviewCMSPageEditControllerTest extends FunctionalTest {
 	
 	public function testSaveReview() {
 		$author = $this->objFromFixture('Member', 'author');
-		$this->loginAs($author);		
+		$this->loginAs($author);	
 		$page = $this->objFromFixture('Page', 'home');
 		
 		$data = array(

tests/ContentReviewReportTest.php

@@ -12,7 +12,7 @@ class ContentReviewReportTest extends FunctionalTest {
 		"SiteConfig" => array("ContentReviewDefaultSettings"),
 	);
 	
-	public function testReportContent() {
+	public function testPagesDueForReviewReport() {
 		$editor = $this->objFromFixture('Member', 'editor');
 		$this->logInAs($editor);
 		$report = new PagesDueForReviewReport();
@@ -24,8 +24,8 @@ class ContentReviewReportTest extends FunctionalTest {
 		$results = $report->sourceRecords(array(
 			'ReviewDateAfter' => '01/01/2010',
 			'ReviewDateBefore' => '12/12/2010'
-		), 'NextReviewDate ASC', false);
+		));
-		
+
 		$this->assertEquals(array(
 			'Contact Us',
 			'Contact Us Child',
@@ -35,8 +35,7 @@ class ContentReviewReportTest extends FunctionalTest {
 		), $results->column('Title'));
 		
 		SS_Datetime::set_mock_now('2010-02-13 00:00:00');
-		$results = $report->sourceRecords(array(
+		$results = $report->sourceRecords(array());
-		), 'NextReviewDate ASC', false);
 		$this->assertEquals(array(
 			'About Us',
 			'Home'
@@ -45,5 +44,24 @@ class ContentReviewReportTest extends FunctionalTest {
 		SS_Datetime::clear_mock_now();
 	}
 	
+	public function testPagesWithoutReviewScheduleReport() {
+		$editor = $this->objFromFixture('Member', 'editor');
+		$this->logInAs($editor);
+		$report = new PagesWithoutReviewScheduleReport();
+	
+		$report->parameterFields();
+		$report->columns();
+		$report->title();
+	
+		$results = $report->sourceRecords();
+	
+		$this->assertEquals(array(
+			'Home',
+			'About Us',
+			'Page without review date',
+			'Page owned by group',
+		), $results->column('Title'));
+	}
+	
 }
 

tests/ContentReviewSettingsTest.php

@@ -82,8 +82,10 @@ class ContentReviewSettingsTest extends SapphireTest {
 	
 	public function testGetSettingsObjectFromInheritPage() {
 		$page = $this->objFromFixture('Page', 'page-1-1');
+		$parentPage = $this->objFromFixture('Page', 'page-1');
 		$this->assertEquals('Inherit', $page->ContentReviewType);
-		$this->assertEquals($this->objFromFixture('Page', 'page-1'), $page->getOptions());
+		$this->assertEquals(get_class($parentPage), get_class($page->getOptions()));
+		$this->assertEquals($parentPage->ID, $page->getOptions()->ID);
 	}
 
 	public function testGetSettingsObjectFromInheritedRootPage() {

tests/SiteTreeContentReviewTest.php

@@ -1,6 +1,6 @@
 <?php
 
-class SiteTreeContentReviewTest extends FunctionalTest {
+class SiteTreeContentReviewTest extends ContentReviewBaseTest {
 	
 	public static $fixture_file = 'contentreview/tests/ContentReviewTest.yml';
 	
@@ -22,6 +22,7 @@ class SiteTreeContentReviewTest extends FunctionalTest {
 		$page->ContentReviewUsers()->push($editor);
 		$page->write();
 
+		$this->assertTrue($page->canPublish());
 		$this->assertTrue($page->doPublish());
 		$this->assertEquals($page->OwnerNames, "Test Editor", 'Test Editor should be the owner');
 		
@@ -29,6 +30,7 @@ class SiteTreeContentReviewTest extends FunctionalTest {
 		$page->OwnerUsers()->removeAll();
 		$page->write();
 		
+		$this->assertTrue($page->canPublish());
 		$this->assertTrue($page->doPublish());
 		$this->assertEquals('', $page->OwnerNames);
 	}
@@ -171,4 +173,5 @@ class SiteTreeContentReviewTest extends FunctionalTest {
 		$this->assertNull($fields->fieldByName('action_reviewed'));
 		SS_Datetime::clear_mock_now();
 	}
+	
 }