From 7a843e222f89b93fb2a601dd99d8916b5cb7923a Mon Sep 17 00:00:00 2001 From: Stig Lindqvist Date: Tue, 18 Feb 2014 15:39:13 +1300 Subject: [PATCH] Improving test coverage --- .../ContentReviewCMSPageEditController.php | 13 +- code/extensions/SiteTreeContentReview.php | 73 ++++++--- code/models/ContentReviewLog.php | 30 ++++ tests/ContentReviewTest.php | 86 +---------- tests/ContentReviewTest.yml | 5 +- tests/SiteTreeContentReviewTest.php | 140 ++++++++++++++++++ 6 files changed, 235 insertions(+), 112 deletions(-) create mode 100644 code/models/ContentReviewLog.php create mode 100644 tests/SiteTreeContentReviewTest.php diff --git a/code/extensions/ContentReviewCMSPageEditController.php b/code/extensions/ContentReviewCMSPageEditController.php index 2c28393..f780849 100644 --- a/code/extensions/ContentReviewCMSPageEditController.php +++ b/code/extensions/ContentReviewCMSPageEditController.php @@ -66,16 +66,17 @@ class ContentReviewCMSPageEditController extends LeftAndMainExtension { throw new SS_HTTPResponse_Exception("No record ID", 404); } $SQL_id = Convert::raw2sql($data['ID']); - $record = SiteTree::get()->byID($SQL_id); - if($record && !$record->canEdit()) { + $page = SiteTree::get()->byID($SQL_id); + if($page && !$page->canEdit()) { return Security::permissionFailure($this); } - if(!$record || !$record->ID) { + if(!$page || !$page->ID) { throw new SS_HTTPResponse_Exception("Bad record ID #$SQL_id", 404); } - $record->ReviewNotes = $data['ReviewNotes']; - $record->write(); - return $this->owner->redirect($this->owner->Link('show/'.$SQL_id)); + $page->addReviewNote(Member::currentUser(), $data['ReviewNotes']); + $page->advanceReviewDate(); + + return $this->owner->redirect($this->owner->Link('show/'.$page->ID)); } } diff --git a/code/extensions/SiteTreeContentReview.php b/code/extensions/SiteTreeContentReview.php index 9016d6b..a217dca 100644 --- a/code/extensions/SiteTreeContentReview.php +++ b/code/extensions/SiteTreeContentReview.php @@ -14,10 +14,17 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider private static $db = array( "ReviewPeriodDays" => "Int", "NextReviewDate" => "Date", - 'ReviewNotes' => 'Text', 'LastEditedByName' => 'Varchar(255)', 'OwnerNames' => 'Varchar(255)' ); + + /** + * + * @var array + */ + private static $has_many = array( + 'ReviewLogs' => 'ContentReviewLog' + ); /** * @@ -34,11 +41,11 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider */ public function getOwnerNames() { $names = array(); - foreach($this->DirectGroups() as $group) { + foreach($this->OwnerGroups() as $group) { $names[] = $group->Title; } - foreach($this->DirectUsers() as $group) { + foreach($this->OwnerUsers() as $group) { $names[] = $group->getName(); } return implode(', ', $names); @@ -63,11 +70,9 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider * @return \ArrayList */ public function ContentReviewOwners() { - $contentReviewOwners = new ArrayList(); - - $toplevelGroups = $this->DirectGroups(); - if($toplevelGroups) { + $toplevelGroups = $this->OwnerGroups(); + if($toplevelGroups->count()) { $groupIDs = array(); foreach($toplevelGroups as $group) { $familyIDs = $group->collateFamilyIDs(); @@ -75,15 +80,15 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider $groupIDs = array_merge($groupIDs, array_values($familyIDs)); } } + array_unique($groupIDs); if(count($groupIDs)) { $groupMembers = DataObject::get('Member')->where("\"Group\".\"ID\" IN (" . implode(",",$groupIDs) . ")") ->leftJoin("Group_Members", "\"Member\".\"ID\" = \"Group_Members\".\"MemberID\"") ->leftJoin("Group", "\"Group_Members\".\"GroupID\" = \"Group\".\"ID\""); $contentReviewOwners->merge($groupMembers); } - } - $contentReviewOwners->merge($this->DirectUsers()); + $contentReviewOwners->merge($this->OwnerUsers()); $contentReviewOwners->removeDuplicates(); return $contentReviewOwners; } @@ -91,14 +96,14 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider /** * @return ManyManyList */ - public function DirectGroups() { + public function OwnerGroups() { return $this->owner->getManyManyComponents('ContentReviewGroups'); } /** * @return ManyManyList */ - public function DirectUsers() { + public function OwnerUsers() { return $this->owner->getManyManyComponents('ContentReviewUsers'); } @@ -120,7 +125,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider } asort($usersMap); - $userField = ListboxField::create('DirectUsers', _t("ContentReview.PAGEOWNERUSERS", "Users")) + $userField = ListboxField::create('OwnerUsers', _t("ContentReview.PAGEOWNERUSERS", "Users")) ->setMultiple(true) ->setSource($usersMap) ->setAttribute('data-placeholder', _t('ContentReview.ADDUSERS', 'Add users')) @@ -132,7 +137,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider $groupsMap[$group->ID] = $group->getBreadcrumbs(' > '); } asort($groupsMap); - $groupField = ListboxField::create('DirectGroups', _t("ContentReview.PAGEOWNERGROUPS", "Groups")) + $groupField = ListboxField::create('OwnerGroups', _t("ContentReview.PAGEOWNERGROUPS", "Groups")) ->setMultiple(true) ->setSource($groupsMap) ->setAttribute('data-placeholder', _t('ContentReview.ADDGROUP', 'Add groups')) @@ -163,7 +168,7 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider ) )->setDescription(_t('ContentReview.REVIEWFREQUENCYDESCRIPTION', 'The review date will be set to this far in the future whenever the page is published')); - $notesField = TextareaField::create('ReviewNotes', 'Review Notes'); + $notesField = GridField::create('ReviewNotes', 'Review Notes', $this->owner->ReviewLogs()); $fields->addFieldsToTab("Root.Review", array( new HeaderField(_t('ContentReview.REVIEWHEADER', "Content review"), 2), @@ -175,6 +180,37 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider )); } + /** + * Creates a ContentReviewLog and connects it to this Page + * + * @param Member $reviewer + * @param string $message + */ + public function addReviewNote(Member $reviewer, $message) { + $reviewLog = ContentReviewLog::create(); + $reviewLog->Note = $message; + $reviewLog->MemberID = $reviewer->ID; + $this->owner->ReviewLogs()->add($reviewLog); + } + + /** + * Advance review date to the next date based on review period or set it to null + * if there is no schedule + * + * @return bool - returns true if date was set and false is content review is 'off' + */ + public function advanceReviewDate() { + $hasNextReview = true; + if($this->owner->ReviewPeriodDays) { + $this->owner->NextReviewDate = date('Y-m-d', strtotime('+' . $this->owner->ReviewPeriodDays . ' days')); + } else { + $hasNextReview = false; + $this->owner->NextReviewDate = null; + } + $this->owner->write(); + return $hasNextReview; + } + /** * * @param \FieldList $actions @@ -201,13 +237,13 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider if($this->owner->obj('NextReviewDate')->InFuture()) { return false; } - if($this->DirectGroups()->count() == 0 && $this->DirectUsers()->count() == 0) { + if($this->OwnerGroups()->count() == 0 && $this->OwnerUsers()->count() == 0) { return false; } - if($member->inGroups($this->DirectGroups())) { + if($member->inGroups($this->OwnerGroups())) { return true; } - if($this->DirectUsers()->find('ID', $member->ID)) { + if($this->OwnerUsers()->find('ID', $member->ID)) { return true; } return false; @@ -217,9 +253,6 @@ class SiteTreeContentReview extends DataExtension implements PermissionProvider * Set the review data from the review period, if set. */ public function onBeforeWrite() { - if($this->owner->ReviewPeriodDays && !$this->owner->NextReviewDate) { - $this->owner->NextReviewDate = date('Y-m-d', strtotime('+' . $this->owner->ReviewPeriodDays . ' days')); - } $this->owner->LastEditedByName=$this->owner->getEditorName(); $this->owner->OwnerNames = $this->owner->getOwnerNames(); } diff --git a/code/models/ContentReviewLog.php b/code/models/ContentReviewLog.php new file mode 100644 index 0000000..9d6d193 --- /dev/null +++ b/code/models/ContentReviewLog.php @@ -0,0 +1,30 @@ + 'Text' + ); + + /** + * + * @var array + */ + private static $has_one = array( + 'Reviewer' => 'Member', + 'SiteTree' => 'SiteTree' + ); + + /** + * + * @var array + */ + private static $summary_fields = array( + 'Note', + 'Created' + ); +} diff --git a/tests/ContentReviewTest.php b/tests/ContentReviewTest.php index 047e604..9c495f9 100644 --- a/tests/ContentReviewTest.php +++ b/tests/ContentReviewTest.php @@ -1,34 +1,9 @@ providePermissions(); - $this->assertTrue(isset($perms['EDIT_CONTENT_REVIEW_FIELDS'])); - } - - public function testUserWithPermissionCanEdit() { - $editor = $this->objFromFixture('Member', 'editor'); - $this->logInAs($editor); - $page = new Page(); - $fields = $page->getCMSFields(); - $this->assertNotNull($fields->fieldByName('Root.Review')); - } - - public function testUserWithoutPermissionCannotEdit() { - $author = $this->objFromFixture('Member', 'author'); - $this->logInAs($author); - $page = new Page(); - $fields = $page->getCMSFields(); - $this->assertNull($fields->fieldByName('Root.Review')); - } - public function testContentReviewEmails() { SS_Datetime::set_mock_now('2010-02-14 12:00:00'); @@ -40,17 +15,6 @@ class ContentReviewTest extends FunctionalTest { SS_Datetime::clear_mock_now(); } - public function testAutomaticallySettingReviewDate() { - $editor = $this->objFromFixture('Member', 'editor'); - $this->logInAs($editor); - - $page = new Page(); - $page->ReviewPeriodDays = 10; - $page->write(); - $this->assertTrue($page->doPublish()); - $this->assertEquals(date('Y-m-d', strtotime('now + 10 days')), $page->NextReviewDate); - } - public function testReportContent() { $editor = $this->objFromFixture('Member', 'editor'); $this->logInAs($editor); @@ -102,52 +66,4 @@ class ContentReviewTest extends FunctionalTest { $this->assertTrue($page->doPublish()); $this->assertEquals('', $page->OwnerNames); } - - public function testCanNotBeReviewBecauseNoReviewDate() { - SS_Datetime::set_mock_now('2010-01-01 12:00:00'); - $author = $this->objFromFixture('Member', 'author'); - $page = $this->objFromFixture('Page', 'no-review'); - // page 'no-review' is owned by author, but there is no review date - $this->assertFalse($page->canBeReviewedBy($author)); - } - - public function testCanNotBeReviewedBecauseInFuture() { - SS_Datetime::set_mock_now('2010-01-01 12:00:00'); - $author = $this->objFromFixture('Member', 'author'); - $page = $this->objFromFixture('Page', 'staff'); - // page 'staff' is owned by author, but the review date is in the future - $this->assertFalse($page->canBeReviewedBy($author)); - } - - public function testCanNotBeReviewedByUser() { - SS_Datetime::set_mock_now('2010-03-01 12:00:00'); - $author = $this->objFromFixture('Member', 'author'); - $page = $this->objFromFixture('Page', 'home'); - // page 'home' doesnt have any owners - $this->assertFalse($page->canBeReviewedBy($author)); - } - - public function testCanBeReviewedByUser() { - SS_Datetime::set_mock_now('2010-03-01 12:00:00'); - $author = $this->objFromFixture('Member', 'author'); - $page = $this->objFromFixture('Page', 'staff'); - // page 'staff' is owned by author - $this->assertTrue($page->canBeReviewedBy($author)); - } - - public function testCanNotBeReviewedByGroup() { - SS_Datetime::set_mock_now('2010-03-01 12:00:00'); - $author = $this->objFromFixture('Member', 'editor'); - $page = $this->objFromFixture('Page', 'contact'); - // page 'contact' is owned by the authorgroup - $this->assertFalse($page->canBeReviewedBy($author)); - } - - public function testCanBeReviewedByGroup() { - SS_Datetime::set_mock_now('2010-03-01 12:00:00'); - $author = $this->objFromFixture('Member', 'author'); - $page = $this->objFromFixture('Page', 'contact'); - // page 'contact' is owned by the authorgroup - $this->assertTrue($page->canBeReviewedBy($author)); - } } diff --git a/tests/ContentReviewTest.yml b/tests/ContentReviewTest.yml index 90e0f48..76675f9 100644 --- a/tests/ContentReviewTest.yml +++ b/tests/ContentReviewTest.yml @@ -49,4 +49,7 @@ Page: ContentReviewGroups: =>Group.authorgroup no-review: Title: Page without review date - ContentReviewUsers: =>Member.author \ No newline at end of file + ContentReviewUsers: =>Member.author + group-owned: + Title: Page owned by group + ContentReviewGroups: =>Group.authorgroup \ No newline at end of file diff --git a/tests/SiteTreeContentReviewTest.php b/tests/SiteTreeContentReviewTest.php new file mode 100644 index 0000000..4ac55c0 --- /dev/null +++ b/tests/SiteTreeContentReviewTest.php @@ -0,0 +1,140 @@ +providePermissions(); + $this->assertTrue(isset($perms['EDIT_CONTENT_REVIEW_FIELDS'])); + } + + public function testUserWithPermissionCanEdit() { + $editor = $this->objFromFixture('Member', 'editor'); + $this->logInAs($editor); + $page = new Page(); + $fields = $page->getCMSFields(); + $this->assertNotNull($fields->fieldByName('Root.Review')); + } + + public function testUserWithoutPermissionCannotEdit() { + $author = $this->objFromFixture('Member', 'author'); + $this->logInAs($author); + $page = new Page(); + $fields = $page->getCMSFields(); + $this->assertNull($fields->fieldByName('Root.Review')); + } + + public function testAutomaticallyToNotSetReviewDate() { + $editor = $this->objFromFixture('Member', 'editor'); + $this->logInAs($editor); + + $page = new Page(); + $page->ReviewPeriodDays = 10; + $page->write(); + $this->assertTrue($page->doPublish()); + $this->assertEquals(null, $page->NextReviewDate); + } + + public function testSetReviewDate10Days() { + $page = new Page(); + $page->ReviewPeriodDays = 10; + $this->assertTrue($page->advanceReviewDate()); + $page->write(); + $this->assertEquals(date('Y-m-d', strtotime('now + 10 days')), $page->NextReviewDate); + } + + public function testSetReviewDateNull() { + $page = new Page(); + $page->ReviewPeriodDays = 0; + $this->assertFalse($page->advanceReviewDate()); + $page->write(); + $this->assertEquals(null, $page->NextReviewDate); + } + + public function testAddReviewNote() { + $author = $this->objFromFixture('Member', 'author'); + $page = $this->objFromFixture('Page', 'home'); + $page->addReviewNote($author, 'This is a message'); + + // Get the page again to make sure it's not only cached in memory + $homepage = $this->objFromFixture('Page', 'home'); + $this->assertEquals(1, $homepage->ReviewLogs()->count()); + $this->assertEquals('This is a message', $homepage->ReviewLogs()->first()->Note); + } + + public function testGetContentReviewOwners() { + $page = $this->objFromFixture('Page', 'group-owned'); + $owners = $page->ContentReviewOwners(); + $this->assertEquals(1, $owners->count()); + $this->assertEquals('author@example.com', $owners->first()->Email); + } + + public function testCanNotBeReviewBecauseNoReviewDate() { + SS_Datetime::set_mock_now('2010-01-01 12:00:00'); + $author = $this->objFromFixture('Member', 'author'); + $page = $this->objFromFixture('Page', 'no-review'); + // page 'no-review' is owned by author, but there is no review date + $this->assertFalse($page->canBeReviewedBy($author)); + } + + public function testCanNotBeReviewedBecauseInFuture() { + SS_Datetime::set_mock_now('2010-01-01 12:00:00'); + $author = $this->objFromFixture('Member', 'author'); + $page = $this->objFromFixture('Page', 'staff'); + // page 'staff' is owned by author, but the review date is in the future + $this->assertFalse($page->canBeReviewedBy($author)); + } + + public function testCanNotBeReviewedByUser() { + SS_Datetime::set_mock_now('2010-03-01 12:00:00'); + $author = $this->objFromFixture('Member', 'author'); + $page = $this->objFromFixture('Page', 'home'); + // page 'home' doesnt have any owners + $this->assertFalse($page->canBeReviewedBy($author)); + } + + public function testCanBeReviewedByUser() { + SS_Datetime::set_mock_now('2010-03-01 12:00:00'); + $author = $this->objFromFixture('Member', 'author'); + $page = $this->objFromFixture('Page', 'staff'); + // page 'staff' is owned by author + $this->assertTrue($page->canBeReviewedBy($author)); + } + + public function testCanNotBeReviewedByGroup() { + SS_Datetime::set_mock_now('2010-03-01 12:00:00'); + $author = $this->objFromFixture('Member', 'editor'); + $page = $this->objFromFixture('Page', 'contact'); + // page 'contact' is owned by the authorgroup + $this->assertFalse($page->canBeReviewedBy($author)); + } + + public function testCanBeReviewedByGroup() { + SS_Datetime::set_mock_now('2010-03-01 12:00:00'); + $author = $this->objFromFixture('Member', 'author'); + $page = $this->objFromFixture('Page', 'contact'); + // page 'contact' is owned by the authorgroup + $this->assertTrue($page->canBeReviewedBy($author)); + } + + public function testReviewActionVisibleForAuthor() { + SS_Datetime::set_mock_now('2020-03-01 12:00:00'); + $page = $this->objFromFixture('Page', 'contact'); + $author = $this->objFromFixture('Member', 'author'); + $this->logInAs($author); + + $fields = $page->getCMSActions(); + $this->assertNotNull($fields->fieldByName('action_reviewed')); + } + + public function testReviewActionNotVisibleForEditor() { + SS_Datetime::set_mock_now('2020-03-01 12:00:00'); + $page = $this->objFromFixture('Page', 'contact'); + $author = $this->objFromFixture('Member', 'editor'); + $this->logInAs($author); + + $fields = $page->getCMSActions(); + $this->assertNull($fields->fieldByName('action_reviewed')); + } +}