From 24766e4e5ab91a82ae7f6ed5b8d8d24a552151ba Mon Sep 17 00:00:00 2001
From: Sabina Talipova <sabina.talipova@silverstripe.com>
Date: Fri, 31 Mar 2023 15:51:10 +1300
Subject: [PATCH] FIX Notification job marked as broken

---
 src/Tasks/ContentReviewEmails.php           | 22 ++++++++++++++++++++-
 tests/php/ContentReviewNotificationTest.php | 20 +++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/Tasks/ContentReviewEmails.php b/src/Tasks/ContentReviewEmails.php
index c59bc56..a622c5b 100644
--- a/src/Tasks/ContentReviewEmails.php
+++ b/src/Tasks/ContentReviewEmails.php
@@ -15,7 +15,6 @@ 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.
@@ -93,6 +92,12 @@ class ContentReviewEmails extends BuildTask
         // Prepare variables
         $siteConfig = SiteConfig::current_site_config();
         $owner = Member::get()->byID($ownerID);
+
+        if (!$this->isValidEmail($owner->Email)
+            || !$this->isValidEmail($siteConfig->ReviewFrom)) {
+            return;
+        }
+
         $templateVariables = $this->getTemplateVariables($owner, $siteConfig, $pages);
 
         // Build email
@@ -159,4 +164,19 @@ class ContentReviewEmails extends BuildTask
             'ToEmail' => $recipient->Email,
         ];
     }
+
+    /**
+     * Check validity of email
+     */
+    protected function isValidEmail(?string $email): bool
+    {
+        if (!$email
+            || empty($email)
+            || !filter_var($email, FILTER_VALIDATE_EMAIL)
+        ) {
+            return false;
+        }
+
+        return true;
+    }
 }
diff --git a/tests/php/ContentReviewNotificationTest.php b/tests/php/ContentReviewNotificationTest.php
index d2e7aca..ca55d7f 100644
--- a/tests/php/ContentReviewNotificationTest.php
+++ b/tests/php/ContentReviewNotificationTest.php
@@ -3,6 +3,7 @@
 namespace SilverStripe\ContentReview\Tests;
 
 use Page;
+use ReflectionClass;
 use SilverStripe\CMS\Model\SiteTree;
 use SilverStripe\CMS\Controllers\CMSPageEditController;
 use SilverStripe\ContentReview\Extensions\ContentReviewCMSExtension;
@@ -132,6 +133,25 @@ class ContentReviewNotificationTest extends SapphireTest
         DBDatetime::clear_mock_now();
     }
 
+    /**
+     * Test that provided email is valid
+     */
+    public function testIsValidEmail()
+    {
+        $class = new ReflectionClass(ContentReviewEmails::class);
+        $method = $class->getMethod('isValidEmail');
+        $method->setAccessible(true);
+
+        $member = $this->objFromFixture(Member::class, 'author');
+        $task = new ContentReviewEmails();
+
+        $this->assertTrue($method->invokeArgs($task, [$member->Email]));
+        $this->assertTrue($method->invokeArgs($task, ['correct.email@example.com']));
+
+        $this->assertFalse($method->invokeArgs($task, [null]));
+        $this->assertFalse($method->invokeArgs($task, ['broken.email']));
+    }
+
     /**
      * Deletes all pages except those passes in to the $ids parameter
      *