From affee4bb85e334d99d6a60cecccb8e55aba2fb80 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 17 Nov 2015 15:12:20 +1300 Subject: [PATCH] BUG Show inherited date even when inheriting settings --- code/extensions/SiteTreeContentReview.php | 35 +++-- javascript/contentreview.js | 11 +- tests/ContentReviewSettingsTest.php | 48 +++--- tests/ContentReviewSettingsTest.yml | 176 +++++++++++----------- 4 files changed, 132 insertions(+), 138 deletions(-) diff --git a/code/extensions/SiteTreeContentReview.php b/code/extensions/SiteTreeContentReview.php index 81d358e..b990543 100644 --- a/code/extensions/SiteTreeContentReview.php +++ b/code/extensions/SiteTreeContentReview.php @@ -352,6 +352,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider $userField = ListboxField::create("OwnerUsers", _t("ContentReview.PAGEOWNERUSERS", "Users"), $usersMap) ->setMultiple(true) + ->addExtraClass('custom-setting') ->setAttribute("data-placeholder", _t("ContentReview.ADDUSERS", "Add users")) ->setDescription(_t('ContentReview.OWNERUSERSDESCRIPTION', 'Page owners that are responsible for reviews')); @@ -364,6 +365,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider $groupField = ListboxField::create("OwnerGroups", _t("ContentReview.PAGEOWNERGROUPS", "Groups"), $groupsMap) ->setMultiple(true) + ->addExtraClass('custom-setting') ->setAttribute("data-placeholder", _t("ContentReview.ADDGROUP", "Add groups")) ->setDescription(_t("ContentReview.OWNERGROUPSDESCRIPTION", "Page owners that are responsible for reviews")); @@ -373,7 +375,12 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider ->setConfig("datavalueformat", "yyyy-MM-dd") ->setDescription(_t("ContentReview.NEXTREVIEWDATADESCRIPTION", "Leave blank for no review")); - $reviewFrequency = DropdownField::create("ReviewPeriodDays", _t("ContentReview.REVIEWFREQUENCY", "Review frequency"), self::get_schedule()) + $reviewFrequency = DropdownField::create( + "ReviewPeriodDays", + _t("ContentReview.REVIEWFREQUENCY", "Review frequency"), + self::get_schedule() + ) + ->addExtraClass('custom-setting') ->setDescription(_t("ContentReview.REVIEWFREQUENCYDESCRIPTION", "The review date will be set to this far in the future whenever the page is published")); $notesField = GridField::create("ReviewNotes", "Review Notes", $this->owner->ReviewLogs(), GridFieldConfig_RecordEditor::create()); @@ -386,9 +393,8 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider $groupField, $reviewDate, $reviewFrequency - )->addExtraClass("custom-settings"), + )->addExtraClass("review-settings"), ReadonlyField::create("ROContentOwners", _t("ContentReview.CONTENTOWNERS", "Content Owners"), $this->getOwnerNames()), - ReadonlyField::create("RONextReviewDate", _t("ContentReview.NEXTREVIEWDATE", "Next review date"), $this->owner->NextReviewDate), $notesField, )); } @@ -504,19 +510,6 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider $nextReviewUnixSec = strtotime(" + " . $this->owner->ReviewPeriodDays . " days", SS_Datetime::now()->format("U")); $this->owner->NextReviewDate = date("Y-m-d", $nextReviewUnixSec); } - - // - if ($this->owner->isChanged("NextReviewDate", 2)) { - $children = $this->owner->stageChildren(true)->filter("ContentReviewType", "Inherit"); - $compatibility = ContentReviewCompatability::start(); - - foreach ($children as $child) { - $child->NextReviewDate = $this->owner->NextReviewDate; - $child->write(); - } - - ContentReviewCompatability::done($compatibility); - } } private function setDefaultReviewDateForDisabled() @@ -526,6 +519,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider protected function setDefaultReviewDateForCustom() { + // Don't overwrite existing value if ($this->owner->NextReviewDate) { return; } @@ -542,11 +536,16 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider protected function setDefaultReviewDateForInherited() { + // Don't overwrite existing value + if($this->owner->NextReviewDate) { + return; + } + $options = $this->getOptions(); $nextDate = null; - if ($options && $this->owner->parent()->exists()) { - $nextDate = $this->getReviewDate($this->owner->parent()); + if ($options instanceof SiteTree) { + $nextDate = $this->getReviewDate($options); } elseif ($options instanceof SiteConfig) { $nextDate = $this->getReviewDate(); } diff --git a/javascript/contentreview.js b/javascript/contentreview.js index da0646a..b27db16 100644 --- a/javascript/contentreview.js +++ b/javascript/contentreview.js @@ -40,16 +40,15 @@ jQuery(function($) { }, _custom: function() { - $('.custom-settings').show(); - $('.inherited-settings').hide(); + $('.review-settings').show(); + $('.field.custom-setting').show(); }, _inherited: function() { - $('.inherited-settings').show(); - $('.custom-settings').hide(); + $('.review-settings').show(); + $('.field.custom-setting').hide(); }, _disabled: function() { - $('.inherited-settings').hide(); - $('.custom-settings').hide(); + $('.review-settings').hide(); } }); diff --git a/tests/ContentReviewSettingsTest.php b/tests/ContentReviewSettingsTest.php index be447ab..6d5e55c 100644 --- a/tests/ContentReviewSettingsTest.php +++ b/tests/ContentReviewSettingsTest.php @@ -68,17 +68,19 @@ class ContentReviewSettingsTest extends SapphireTest public function testAdvanceReviewFromInheritedSettings() { - /** @var Page|SiteTreeContentReview $page */ - $page = $this->objFromFixture("Page", "page-1-1"); - - /** @var Page|SiteTreeContentReview $parentPage */ + // When a parent page is advanced, the next review date of the child is not automatically advanced $parentPage = $this->objFromFixture("Page", "page-1"); + $this->assertTrue($parentPage->advanceReviewDate()); + $parentPage->write(); + + $page = $this->objFromFixture("Page", "page-1-1"); + $this->assertEquals(date("Y-m-d", strtotime("now + 5 days")), $parentPage->NextReviewDate); + $this->assertEquals('2011-04-12', $page->NextReviewDate); + // When a sub page is advanced, the next review date is advanced by the number of days in the parent $this->assertTrue($page->advanceReviewDate()); - $page->write(); - - $this->assertEquals(date("Y-m-d", strtotime("now + " . $parentPage->ReviewPeriodDays . " days")), $page->NextReviewDate); + $this->assertEquals(date("Y-m-d", strtotime("now + 5 days")), $page->NextReviewDate); } public function testAdvanceReviewFromInheritedSiteConfigSettings() @@ -183,19 +185,11 @@ class ContentReviewSettingsTest extends SapphireTest public function testGetNextReviewDateFromPageInheritedSetting() { - /** @var Page|SiteTreeContentReview $page */ + // Although page-1-1 inherits from page-1, it has an independent review date $page = $this->objFromFixture("Page", "page-1-1"); - $nextReviewDate = $page->getReviewDate(); - $this->assertInstanceOf("Date", $nextReviewDate); - - /** @var Page|SiteTreeContentReview $nextPage */ - $nextPage = $this->objFromFixture("Page", "page-1"); - - $expected = $nextPage->NextReviewDate; - - $this->assertEquals($expected, $nextReviewDate->format("Y-m-d")); + $this->assertEquals('2011-04-12', $nextReviewDate->format("Y-m-d")); } public function testUpdateNextReviewDateFromCustomToDisabled() @@ -248,13 +242,10 @@ class ContentReviewSettingsTest extends SapphireTest /** @var Page|SiteTreeContentReview $childPage */ $childPage = $this->objFromFixture("Page", "page-1-1"); - // BEFORE: parent page have a period of five days, so childPage should have a - // review date LastEdited + 5 days - $this->assertEquals($parentPage->NextReviewDate, $childPage->NextReviewDate); + // Parent and child pages have different review dates + $this->assertNotEquals($parentPage->NextReviewDate, $childPage->NextReviewDate); - $oldChildDate = $childPage->NextReviewDate; - // But if we change the parent page ReviewPeriodDays to 10, the childs should - // change as well + // But if we change the parent page ReviewPeriodDays to 10, the childs stays the same $parentPage->ReviewPeriodDays = 10; $parentPage->write(); @@ -267,11 +258,14 @@ class ContentReviewSettingsTest extends SapphireTest /** @var Page|SiteTreeContentReview $page */ $childPage = $this->objFromFixture("Page", "page-1-1"); - // AFTER: parent page have a period of 10 days, so childPage should have - // a review date now + 10 days. - $this->assertNotEquals($oldChildDate, $childPage->NextReviewDate); + // The parent page's date advances, but not the child's + $this->assertEquals('2011-04-12', $childPage->NextReviewDate); + $this->assertEquals($this->addDaysToDate(date("Y-m-d"), 10), $parentPage->NextReviewDate); + + // Reviewing the child page should, however, advance its review by 10 days + $childPage->advanceReviewDate(); + $childPage->write(); $this->assertEquals($this->addDaysToDate(date("Y-m-d"), 10), $childPage->NextReviewDate); - $this->assertEquals($parentPage->NextReviewDate, $childPage->NextReviewDate); } /** diff --git a/tests/ContentReviewSettingsTest.yml b/tests/ContentReviewSettingsTest.yml index 579cc84..270ac6a 100644 --- a/tests/ContentReviewSettingsTest.yml +++ b/tests/ContentReviewSettingsTest.yml @@ -1,94 +1,96 @@ Permission: - cmsmain1: - Code: CMS_ACCESS_CMSMain - cmsmain2: - Code: CMS_ACCESS_CMSMain - setreviewdates: - Code: EDIT_CONTENT_REVIEW_FIELDS - workflowadmin1: - Code: IS_WORKFLOW_ADMIN - workflowadmin2: - Code: IS_WORKFLOW_ADMIN + cmsmain1: + Code: CMS_ACCESS_CMSMain + cmsmain2: + Code: CMS_ACCESS_CMSMain + setreviewdates: + Code: EDIT_CONTENT_REVIEW_FIELDS + workflowadmin1: + Code: IS_WORKFLOW_ADMIN + workflowadmin2: + Code: IS_WORKFLOW_ADMIN Group: - webmastergroup: - Title: Edit existing pages - Code: editorgroup - Permissions: =>Permission.cmsmain1,=>Permission.workflowadmin1,=>Permission.setreviewdates - editorgroup: - Title: Edit existing pages - Code: editorgroup - Permissions: =>Permission.cmsmain1,=>Permission.workflowadmin1,=>Permission.setreviewdates - authorgroup: - Title: Author existing pages - Code: authorgroup - Permissions: =>Permission.cmsmain2,=>Permission.workflowadmin2 + webmastergroup: + Title: Edit existing pages + Code: editorgroup + Permissions: =>Permission.cmsmain1,=>Permission.workflowadmin1,=>Permission.setreviewdates + editorgroup: + Title: Edit existing pages + Code: editorgroup + Permissions: =>Permission.cmsmain1,=>Permission.workflowadmin1,=>Permission.setreviewdates + authorgroup: + Title: Author existing pages + Code: authorgroup + Permissions: =>Permission.cmsmain2,=>Permission.workflowadmin2 Member: - webmaster: - FirstName: Web - Surname: Master - Email: webmaster@example.com - Groups: =>Group.webmastergroup - author: - FirstName: Test - Surname: Author - Email: author@example.com - Groups: =>Group.authorgroup - editor: - FirstName: Test - Surname: Editor - Groups: =>Group.editorgroup + webmaster: + FirstName: Web + Surname: Master + Email: webmaster@example.com + Groups: =>Group.webmastergroup + author: + FirstName: Test + Surname: Author + Email: author@example.com + Groups: =>Group.authorgroup + editor: + FirstName: Test + Surname: Editor + Groups: =>Group.editorgroup SiteConfig: - default: - ContentReviewUsers: =>Member.webmaster - ContentReviewGroups: =>Group.webmastergroup - ReviewPeriodDays: 30 + default: + ContentReviewUsers: =>Member.webmaster + ContentReviewGroups: =>Group.webmastergroup + ReviewPeriodDays: 30 Page: - custom: - Title: custom - ContentReviewType: Custom - NextReviewDate: 2010-02-01 - ContentReviewUsers: =>Member.editor - ReviewPeriodDays: 10 - disabled: - Title: disabled - ContentReviewType: Disabled - inherit: - Title: inherit - ContentReviewType: Inherit - page-1: - Title: page 1 - ContentReviewType: Custom - ReviewPeriodDays: 5 - NextReviewDate: 2010-02-01 - page-1-1: - Title: page 1 1 - ContentReviewType: Inherit - ParentID: =>Page.page-1 - page-2: - Title: page 2 - ContentReviewType: Inherit - page-2-1: - Title: page 2 1 - ContentReviewType: Disabled - ParentID: =>Page.page-2 - page-2-1-1: - Title: page 2 1 1 - ContentReviewType: Inherit - ParentID: =>Page.page-2-1 - page-3: - Title: page 3 - ContentReviewType: Inherit - page-3-1: - Title: page 3 1 - ContentReviewType: Inherit - ParentID: =>Page.page-3 - page-3-1-1: - Title: page 3 1 1 - ContentReviewType: Inherit - ParentID: =>Page.page-3-1 - page-3-1-1-1: - Title: page 3 1 1 1 - ContentReviewType: Inherit - ParentID: =>Page.page-3-1-1 \ No newline at end of file + custom: + Title: custom + ContentReviewType: Custom + NextReviewDate: 2010-02-01 + ContentReviewUsers: =>Member.editor + ReviewPeriodDays: 10 + disabled: + Title: disabled + ContentReviewType: Disabled + inherit: + Title: inherit + ContentReviewType: Inherit + page-1: + Title: page 1 + ContentReviewType: Custom + ReviewPeriodDays: 5 + NextReviewDate: 2010-02-01 + page-1-1: + Title: page 1 1 + ContentReviewType: Inherit + ReviewPeriodDays: 1 + NextReviewDate: 2011-04-12 + ParentID: =>Page.page-1 + page-2: + Title: page 2 + ContentReviewType: Inherit + page-2-1: + Title: page 2 1 + ContentReviewType: Disabled + ParentID: =>Page.page-2 + page-2-1-1: + Title: page 2 1 1 + ContentReviewType: Inherit + ParentID: =>Page.page-2-1 + page-3: + Title: page 3 + ContentReviewType: Inherit + page-3-1: + Title: page 3 1 + ContentReviewType: Inherit + ParentID: =>Page.page-3 + page-3-1-1: + Title: page 3 1 1 + ContentReviewType: Inherit + ParentID: =>Page.page-3-1 + page-3-1-1-1: + Title: page 3 1 1 1 + ContentReviewType: Inherit + ParentID: =>Page.page-3-1-1