From cf35e056b6461d6c5932c5ef5a195064508d2652 Mon Sep 17 00:00:00 2001
From: Massey Isa'ako <lasiisaako@gmail.com>
Date: Tue, 14 Aug 2018 15:43:10 +1200
Subject: [PATCH] BUGFIX: Remove pages from content review email list, if a
 review log submitted after review date (#98)

---
 src/Tasks/ContentReviewEmails.php           | 11 ++++
 tests/php/ContentReviewNotificationTest.php | 67 +++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/src/Tasks/ContentReviewEmails.php b/src/Tasks/ContentReviewEmails.php
index 28d58f9..2a6e9ee 100644
--- a/src/Tasks/ContentReviewEmails.php
+++ b/src/Tasks/ContentReviewEmails.php
@@ -15,6 +15,7 @@ use SilverStripe\Security\Member;
 use SilverStripe\SiteConfig\SiteConfig;
 use SilverStripe\View\ArrayData;
 use SilverStripe\View\SSViewer;
+use SilverStripe\ContentReview\Models\ContentReviewLog;
 
 /**
  * Daily task to send emails to the owners of content items when the review date rolls around.
@@ -57,6 +58,16 @@ class ContentReviewEmails extends BuildTask
                 continue;
             }
 
+            // get most recent review log of current [age]
+            $contentReviewLog = $page->ReviewLogs()->sort('Created DESC')->first();
+
+            // check log date vs NextReviewDate. If someone has left a content review
+            // after the review date, then we don't need to notify anybody
+            if ($contentReviewLog && $contentReviewLog->Created >= $page->NextReviewDate) {
+                $page->advanceReviewDate();
+                continue;
+            }
+
             $option = $page->getOptions();
 
             foreach ($option->ContentReviewOwners() as $owner) {
diff --git a/tests/php/ContentReviewNotificationTest.php b/tests/php/ContentReviewNotificationTest.php
index 09d32d7..019fa4a 100644
--- a/tests/php/ContentReviewNotificationTest.php
+++ b/tests/php/ContentReviewNotificationTest.php
@@ -16,6 +16,7 @@ use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\Security\Group;
 use SilverStripe\Security\Member;
 use SilverStripe\SiteConfig\SiteConfig;
+use SilverStripe\ContentReview\Models\ContentReviewLog;
 
 /**
  * @mixin PHPUnit_Framework_TestCase
@@ -81,4 +82,70 @@ class ContentReviewNotificationTest extends SapphireTest
 
         DBDatetime::clear_mock_now();
     }
+
+    /**
+     * When a content review is left after a review date, we want to ensure that
+     * overdue notifications aren't sent.
+     */
+    public function testContentReviewNeeded()
+    {
+        DBDatetime::set_mock_now('2018-08-10 12:00:00');
+
+        /** @var Page|SiteTreeContentReview $childParentPage */
+        $childParentPage = $this->objFromFixture(Page::class, 'no-review');
+        $childParentPage->NextReviewDate = '2018-08-10';
+        $childParentPage->write();
+
+        // we need to ensure only our test page is being ran. If we don't do this
+        // then it may notify for other pages which fails our test
+        $this->deleteAllPagesExcept([$childParentPage->ID]);
+
+        // Grabbing the 'author' from member class
+        $member = $this->objFromFixture(Member::class, 'author');
+
+        // Assigning member as contentreviewer to page
+        $childParentPage->ContentReviewUsers()->add($member);
+
+        // Assert that only one reviewer is assigned to page
+        $this->assertCount(1, $childParentPage->ContentReviewUsers());
+
+        // Create new log
+        $log = new ContentReviewLog();
+
+        // Assign log reviewer as current member
+        $log->ReviewerID = $member->ID;
+
+        // Assign log ID to page ID
+        $log->SiteTreeID = $childParentPage->ID;
+
+        // Write to DB
+        $log->write();
+
+        // assert that log was created day of review
+        $this->assertEquals('2018-08-10 12:00:00', $log->Created);
+        $this->assertCount(1, $childParentPage->ReviewLogs());
+
+        $task = new ContentReviewEmails();
+        $task->run(new HTTPRequest('GET', '/dev/tasks/ContentReviewEmails'));
+
+        // Expecting to not send the email as content review for page is done
+        $email = $this->findEmail($member->Email);
+        $this->assertNull($email);
+
+        DBDatetime::clear_mock_now();
+    }
+
+    /**
+     * Deletes all pages except those passes in to the $ids parameter
+     *
+     * @param  array  $ids Page IDs which will NOT be deleted
+     */
+    private function deleteAllPagesExcept(array $ids)
+    {
+        $pages = SiteTree::get()->exclude('ID', $ids);
+
+        foreach ($pages as $page) {
+            $page->delete();
+        }
+    }
 }