Refactorings

This commit is contained in:
Stig Lindqvist 2014-02-24 21:10:10 +13:00
parent 1057e5823a
commit 638d2fc331
4 changed files with 34 additions and 75 deletions

View File

@ -72,16 +72,12 @@ class ContentReviewEmails extends BuildTask {
*/ */
protected function getOverduePagesForOwners(SS_list $pages, array &$overduePages) { protected function getOverduePagesForOwners(SS_list $pages, array &$overduePages) {
foreach($pages as $page) { foreach($pages as $page) {
// Update the NextReviewDate cache for this page
//$page->updateNextReviewDate($forceWrite = true);
if(!$page->isContentReviewOverdue()) { if(!$page->isContentReviewOverdue()) {
continue; continue;
} }
$settings = SiteTreeContentReview::get_options($page); $option = $page->getOptions();
foreach($settings->ContentReviewOwners() as $owner) { foreach($option->ContentReviewOwners() as $owner) {
if(!isset(self::$member_cache[$owner->ID])) { if(!isset(self::$member_cache[$owner->ID])) {
self::$member_cache[$owner->ID] = $owner; self::$member_cache[$owner->ID] = $owner;
} }

View File

@ -62,24 +62,22 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
/** /**
* *
* @param DataObject $setting * @param DataObject $options
* @param SiteTree $page * @param SiteTree $page
* @return Date | false - returns false if the content review have disabled * @return Date | false - returns false if the content review have disabled
*/ */
public static function get_next_review_date(DataObject $setting, SiteTree $page) { public function getReviewDate(DataObject $options, SiteTree $page) {
if($page->obj('NextReviewDate')->exists()) { if($page->obj('NextReviewDate')->exists()) {
return $page->obj('NextReviewDate'); return $page->obj('NextReviewDate');
} }
if(!$options) {
if(!$setting) {
return false; return false;
} }
if(!$options->ReviewPeriodDays) {
if(!$setting->ReviewPeriodDays) {
return false; return false;
} }
// Failover to check on ReviewPeriodDays + LastEdited // Failover to check on ReviewPeriodDays + LastEdited
$nextReviewUnixSec = strtotime(' + '.$setting->ReviewPeriodDays . ' days', SS_Datetime::now()->format('U')); $nextReviewUnixSec = strtotime(' + '.$options->ReviewPeriodDays . ' days', SS_Datetime::now()->format('U'));
$date = Date::create('NextReviewDate'); $date = Date::create('NextReviewDate');
$date->setValue(date('Y-m-d H:i:s', $nextReviewUnixSec)); $date->setValue(date('Y-m-d H:i:s', $nextReviewUnixSec));
return $date; return $date;
@ -87,22 +85,25 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
/** /**
* Get the object that have the information about the content * Get the object that have the information about the content
* review settings * review settings. Either
* - a SiteTreeContentReview decorated object
* - the default SiteTree config
* - false if this page have it's content review disabled
* *
* Will go through parents and root pages will use the siteconfig * Will go through parents and root pages will use the siteconfig
* if their setting is Inherit. * if their setting is Inherit.
* *
* @param SiteTree $page
* @return DataObject or false if no settings found * @return DataObject or false if no settings found
*/ */
public static function get_options($page) { public function getOptions() {
if($page->ContentReviewType == 'Custom') { if($this->owner->ContentReviewType == 'Custom') {
return $page; return $this->owner;
} }
if($page->ContentReviewType == 'Disabled') { if($this->owner->ContentReviewType == 'Disabled') {
return false; return false;
} }
$page = $this->owner;
// $page is inheriting it's settings from it's parent, find // $page is inheriting it's settings from it's parent, find
// the first valid parent with a valid setting // the first valid parent with a valid setting
while($parent = $page->Parent()) { while($parent = $page->Parent()) {
@ -142,10 +143,11 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
* @return string * @return string
*/ */
public function getEditorName() { public function getEditorName() {
if($member = Member::currentUser()) { $member = Member::currentUser();
if($member) {
return $member->FirstName .' '. $member->Surname; return $member->FirstName .' '. $member->Surname;
} }
return NULL; return null;
} }
/** /**
@ -316,32 +318,6 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
return $hasNextReview; return $hasNextReview;
} }
/**
* Update the NextReviewDate and save the dataobject,
* this is typically done after the ContentReviewType has changed
*
* @param DataObject $settings
* @param bool $forceWrite
*/
public function updateNextReviewDate($forceWrite = false) {
$settings = self::get_options($this->owner);
if(!$settings && $this->owner->NextReviewDate) {
$this->owner->NextReviewDate = null;
}
if($settings) {
$nextDate = self::get_next_review_date($settings, $this->owner);
if($nextDate && $nextDate) {
$this->owner->NextReviewDate = $nextDate->getValue();
}
}
if($forceWrite) {
$this->owner->write();
}
}
/** /**
* *
* @param \FieldList $actions * @param \FieldList $actions
@ -395,7 +371,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
// This contains the DataObject that have the content review settings // This contains the DataObject that have the content review settings
// for this object, it might be this page, one of its parent or SiteConfig // for this object, it might be this page, one of its parent or SiteConfig
$settings = self::get_options($this->owner); $settings = $this->owner->getOptions();
// If the user changed the Type, we need to recalculate the // If the user changed the Type, we need to recalculate the
// Next review date // Next review date
@ -409,10 +385,10 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
$this->owner->NextReviewDate = null; $this->owner->NextReviewDate = null;
// Take from Parent page // Take from Parent page
if($settings && $this->owner->parent()->exists()) { if($settings && $this->owner->parent()->exists()) {
$nextDate = self::get_next_review_date($settings, $this->owner->parent()); $nextDate = $this->getReviewDate($settings, $this->owner->parent());
// Inherit from siteconfig // Inherit from siteconfig
} elseif($settings instanceof SiteConfig) { } elseif($settings instanceof SiteConfig) {
$nextDate = self::get_next_review_date($settings, $this->owner); $nextDate = $this->getReviewDate($settings, $this->owner);
// No setting, parent disabled // No setting, parent disabled
} else { } else {
$nextDate = null; $nextDate = null;
@ -424,7 +400,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider
// No review date provided, use today + period // No review date provided, use today + period
} else { } else {
$this->owner->NextReviewDate = null; $this->owner->NextReviewDate = null;
$nextDate = self::get_next_review_date($settings, $this->owner); $nextDate = $this->getReviewDate($settings, $this->owner);
} }
} }
if(is_object($nextDate)) { if(is_object($nextDate)) {

View File

@ -101,7 +101,7 @@ class PagesDueForReviewReport extends SS_Report {
return 'disabled'; return 'disabled';
} }
if($item->ContentReviewType == 'Inherit') { if($item->ContentReviewType == 'Inherit') {
$setting = $item->getContentReviewSetting($item); $setting = SiteTreeContentReview::getOptions($item);
if(!$setting) { if(!$setting) {
return 'disabled'; return 'disabled';
} }
@ -117,7 +117,7 @@ class PagesDueForReviewReport extends SS_Report {
return 'disabled'; return 'disabled';
} }
if($item->ContentReviewType == 'Inherit') { if($item->ContentReviewType == 'Inherit') {
$setting = $item->getContentReviewSetting($item); $setting = SiteTreeContentReview::getOptions($item);
if(!$setting) { if(!$setting) {
return 'disabled'; return 'disabled';
} }
@ -158,9 +158,7 @@ class PagesDueForReviewReport extends SS_Report {
// If there's no review dates set, default to all pages due for review now // If there's no review dates set, default to all pages due for review now
$reviewDate = new Zend_Date(SS_Datetime::now()->Format('U')); $reviewDate = new Zend_Date(SS_Datetime::now()->Format('U'));
$reviewDate->add(1, Zend_Date::DAY); $reviewDate->add(1, Zend_Date::DAY);
$records = $records->where('"ContentReviewType" != \'Disabled\''); $records = $records->where(sprintf('"NextReviewDate" < \'%s\'', $reviewDate->toString('YYYY-MM-dd')));
//$records = $records->where(sprintf('"NextReviewDate" < \'%s\'', $reviewDate->toString('YYYY-MM-dd')));
} else { } else {
// Review date before // Review date before
if(!empty($params['ReviewDateBefore'])) { if(!empty($params['ReviewDateBefore'])) {

View File

@ -4,10 +4,9 @@
* This class tests that settings are inherited correctly based on the inherited, custom or disabled settings * This class tests that settings are inherited correctly based on the inherited, custom or disabled settings
*/ */
class ContentReviewSettingsTest extends SapphireTest { class ContentReviewSettingsTest extends SapphireTest {
public static $fixture_file = 'contentreview/tests/ContentReviewSettingsTest.yml'; public static $fixture_file = 'contentreview/tests/ContentReviewSettingsTest.yml';
public function testAdvanceReviewFromCustomSettings() { public function testAdvanceReviewFromCustomSettings() {
$page = $this->objFromFixture('Page', 'custom'); $page = $this->objFromFixture('Page', 'custom');
$this->assertTrue($page->advanceReviewDate()); $this->assertTrue($page->advanceReviewDate());
@ -34,42 +33,36 @@ class ContentReviewSettingsTest extends SapphireTest {
public function testGetSettingsObjectFromCustom() { public function testGetSettingsObjectFromCustom() {
$page = $this->objFromFixture('Page', 'custom'); $page = $this->objFromFixture('Page', 'custom');
$this->assertEquals('Custom', $page->ContentReviewType); $this->assertEquals('Custom', $page->ContentReviewType);
$setting = SiteTreeContentReview::get_options($page); $this->assertEquals($page, $page->getOptions());
$this->assertEquals($page, $setting);
} }
public function testGetSettingsObjectFromDisabled() { public function testGetSettingsObjectFromDisabled() {
$page = $this->objFromFixture('Page', 'disabled'); $page = $this->objFromFixture('Page', 'disabled');
$this->assertEquals('Disabled', $page->ContentReviewType); $this->assertEquals('Disabled', $page->ContentReviewType);
$setting = SiteTreeContentReview::get_options($page); $this->assertFalse($page->getOptions());
$this->assertFalse($setting);
} }
public function testGetSettingsObjectFromInheritPage() { public function testGetSettingsObjectFromInheritPage() {
$page = $this->objFromFixture('Page', 'page-1-1'); $page = $this->objFromFixture('Page', 'page-1-1');
$this->assertEquals('Inherit', $page->ContentReviewType); $this->assertEquals('Inherit', $page->ContentReviewType);
$settings = SiteTreeContentReview::get_options($page); $this->assertEquals($this->objFromFixture('Page', 'page-1'), $page->getOptions());
$this->assertEquals($this->objFromFixture('Page', 'page-1'), $settings);
} }
public function testGetSettingsObjectFromInheritedRootPage() { public function testGetSettingsObjectFromInheritedRootPage() {
$page = $this->objFromFixture('Page', 'inherit'); $page = $this->objFromFixture('Page', 'inherit');
$this->assertEquals('Inherit', $page->ContentReviewType); $this->assertEquals('Inherit', $page->ContentReviewType);
$settings = SiteTreeContentReview::get_options($page); $this->assertEquals($this->objFromFixture('SiteConfig', 'default'), $page->getOptions());
$this->assertEquals($this->objFromFixture('SiteConfig', 'default'), $settings);
} }
public function testGetNextReviewDateFromCustomSettings() { public function testGetNextReviewDateFromCustomSettings() {
$page = $this->objFromFixture('Page', 'custom'); $page = $this->objFromFixture('Page', 'custom');
$settings = SiteTreeContentReview::get_options($page); $date = $page->getReviewDate($page->getOptions(), $page);
$date = SiteTreeContentReview::get_next_review_date($settings, $page);
$this->assertEquals('2010-02-01', $date->format('Y-m-d')); $this->assertEquals('2010-02-01', $date->format('Y-m-d'));
} }
public function testGetNextReviewDateFromSiteConfigInheritedSetting() { public function testGetNextReviewDateFromSiteConfigInheritedSetting() {
$page = $this->objFromFixture('Page', 'inherit'); $page = $this->objFromFixture('Page', 'inherit');
$settings = SiteTreeContentReview::get_options($page); $nextReviewDate = $page->getReviewDate($page->getOptions(), $page);
$nextReviewDate = SiteTreeContentReview::get_next_review_date($settings, $page);
$this->assertInstanceOf('Date', $nextReviewDate); $this->assertInstanceOf('Date', $nextReviewDate);
$expected = $this->addDaysToDate(SS_Datetime::now(), $this->objFromFixture('SiteConfig', 'default')->ReviewPeriodDays); $expected = $this->addDaysToDate(SS_Datetime::now(), $this->objFromFixture('SiteConfig', 'default')->ReviewPeriodDays);
@ -78,9 +71,8 @@ class ContentReviewSettingsTest extends SapphireTest {
public function testGetNextReviewDateFromPageInheritedSetting() { public function testGetNextReviewDateFromPageInheritedSetting() {
$page = $this->objFromFixture('Page', 'page-1-1'); $page = $this->objFromFixture('Page', 'page-1-1');
$settings = SiteTreeContentReview::get_options($page); $nextReviewDate = $page->getReviewDate($page->getOptions(), $page);
$nextReviewDate = SiteTreeContentReview::get_next_review_date($settings, $page);
$this->assertInstanceOf('Date', $nextReviewDate); $this->assertInstanceOf('Date', $nextReviewDate);
// It should be the same as the parents reviewdate // It should be the same as the parents reviewdate
$expected = $this->objFromFixture('Page', 'page-1')->NextReviewDate; $expected = $this->objFromFixture('Page', 'page-1')->NextReviewDate;
@ -133,7 +125,6 @@ class ContentReviewSettingsTest extends SapphireTest {
$expected = $this->addDaysToDate($childPage->obj('LastEdited'), $parentPage->ReviewPeriodDays); $expected = $this->addDaysToDate($childPage->obj('LastEdited'), $parentPage->ReviewPeriodDays);
$this->assertEquals($parentPage->NextReviewDate, $childPage->NextReviewDate); $this->assertEquals($parentPage->NextReviewDate, $childPage->NextReviewDate);
$oldChildDate = $childPage->NextReviewDate; $oldChildDate = $childPage->NextReviewDate;
// But if we change the parent page ReviewPeriodDays to 10, the childs should // But if we change the parent page ReviewPeriodDays to 10, the childs should
// change as well // change as well
@ -149,7 +140,6 @@ class ContentReviewSettingsTest extends SapphireTest {
// AFTER: parent page have a period of five days, so childPage should have a // AFTER: parent page have a period of five days, so childPage should have a
// review date LastEdited + 5 days // review date LastEdited + 5 days
$this->assertNotEquals($oldChildDate, $childPage->NextReviewDate); $this->assertNotEquals($oldChildDate, $childPage->NextReviewDate);
$this->assertEquals($parentPage->NextReviewDate, $childPage->NextReviewDate); $this->assertEquals($parentPage->NextReviewDate, $childPage->NextReviewDate);
} }
@ -158,5 +148,4 @@ class ContentReviewSettingsTest extends SapphireTest {
$sec = strtotime('+ '. $days .' days', $date->format('U')); $sec = strtotime('+ '. $days .' days', $date->format('U'));
return date($format, $sec); return date($format, $sec);
} }
} }