From ff723b58efef419bdbca7d8106ffc58284081a89 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 10 Mar 2023 12:21:29 +1300 Subject: [PATCH 1/9] MNT Update development dependencies --- composer.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 78ea2d2..f82b6f2 100644 --- a/composer.json +++ b/composer.json @@ -22,10 +22,10 @@ "require": { "php": "^7.4 || ^8.0", "silverstripe/vendor-plugin": "^1", - "silverstripe/framework": "^4.10", - "silverstripe/cms": "^4.2", - "silverstripe/reports": "^4.2", - "silverstripe/siteconfig": "^4.2" + "silverstripe/framework": "4.13.x-dev", + "silverstripe/cms": "4.13.x-dev", + "silverstripe/reports": "4.13.x-dev", + "silverstripe/siteconfig": "4.13.x-dev" }, "require-dev": { "silverstripe/recipe-testing": "^2", @@ -49,4 +49,4 @@ }, "minimum-stability": "dev", "prefer-stable": true -} +} \ No newline at end of file From 884a84616732ea179d3238690153539dcfef0494 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 10 Mar 2023 16:33:31 +1300 Subject: [PATCH 2/9] MNT Update release dependencies --- composer.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index f82b6f2..8fe2817 100644 --- a/composer.json +++ b/composer.json @@ -22,10 +22,10 @@ "require": { "php": "^7.4 || ^8.0", "silverstripe/vendor-plugin": "^1", - "silverstripe/framework": "4.13.x-dev", - "silverstripe/cms": "4.13.x-dev", - "silverstripe/reports": "4.13.x-dev", - "silverstripe/siteconfig": "4.13.x-dev" + "silverstripe/framework": "4.13.0-beta1", + "silverstripe/cms": "4.13.0-beta1", + "silverstripe/reports": "4.13.0-beta1", + "silverstripe/siteconfig": "4.13.0-beta1" }, "require-dev": { "silverstripe/recipe-testing": "^2", From 47769763dbba9f7dc3a5295cce44bdc817a3008e Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 10 Mar 2023 16:33:34 +1300 Subject: [PATCH 3/9] MNT Update development dependencies --- composer.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 8fe2817..f82b6f2 100644 --- a/composer.json +++ b/composer.json @@ -22,10 +22,10 @@ "require": { "php": "^7.4 || ^8.0", "silverstripe/vendor-plugin": "^1", - "silverstripe/framework": "4.13.0-beta1", - "silverstripe/cms": "4.13.0-beta1", - "silverstripe/reports": "4.13.0-beta1", - "silverstripe/siteconfig": "4.13.0-beta1" + "silverstripe/framework": "4.13.x-dev", + "silverstripe/cms": "4.13.x-dev", + "silverstripe/reports": "4.13.x-dev", + "silverstripe/siteconfig": "4.13.x-dev" }, "require-dev": { "silverstripe/recipe-testing": "^2", From 77bb69f74c5926269d650a9d4b62ad5fc6de1bce Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Tue, 28 Mar 2023 17:06:01 +1300 Subject: [PATCH 4/9] MNT Revert erroneous dependency changes (#191) --- composer.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index f82b6f2..a3273b7 100644 --- a/composer.json +++ b/composer.json @@ -22,10 +22,10 @@ "require": { "php": "^7.4 || ^8.0", "silverstripe/vendor-plugin": "^1", - "silverstripe/framework": "4.13.x-dev", - "silverstripe/cms": "4.13.x-dev", - "silverstripe/reports": "4.13.x-dev", - "silverstripe/siteconfig": "4.13.x-dev" + "silverstripe/framework": "^4.10", + "silverstripe/cms": "^4.2", + "silverstripe/reports": "^4.2", + "silverstripe/siteconfig": "^4.2" }, "require-dev": { "silverstripe/recipe-testing": "^2", From 8e3dd58f784f77936994f66423948dbf0c40876c Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Mon, 3 Apr 2023 14:31:29 +1200 Subject: [PATCH 5/9] MNT Update dev JS --- package.json | 4 ++-- yarn.lock | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 4db29ed..af3b776 100644 --- a/package.json +++ b/package.json @@ -33,8 +33,8 @@ "redux": "^4.2.0" }, "devDependencies": { - "@silverstripe/eslint-config": "^1.0.0-alpha6", - "@silverstripe/webpack-config": "^2.0.0-alpha5", + "@silverstripe/eslint-config": "^1.0.0", + "@silverstripe/webpack-config": "^2.0.0", "webpack": "^5.74.0", "webpack-cli": "^5.0.0" }, diff --git a/yarn.lock b/yarn.lock index ec0795a..b5ad5fa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1261,10 +1261,10 @@ resolved "https://registry.yarnpkg.com/@sect/modernizr-loader/-/modernizr-loader-1.0.3.tgz#7fd8cec372426c53f113f3cfd9344cb29e959825" integrity sha512-47zKwv4/1I0CYptZz8s4aSYSe0awmuyqa+HFKxN89/75h2q8hr6V752TZ9VjhGDhQ4gU0EU7Plew7b+7bf2crg== -"@silverstripe/eslint-config@^1.0.0-alpha6": - version "1.0.0-alpha6" - resolved "https://registry.yarnpkg.com/@silverstripe/eslint-config/-/eslint-config-1.0.0-alpha6.tgz#1f243b003fddf3503a4abea37f35a8a5968cc96e" - integrity sha512-+P7UzhMRSmc7UlRYCiSXwjauLFYU11oBPwHl/bpacJ7xUcFY3Jt3CgcDt6d+XLvAJO8zMRsG9RcOm5MnxsyCsg== +"@silverstripe/eslint-config@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@silverstripe/eslint-config/-/eslint-config-1.0.0.tgz#dcf3f9cf2158bb587d8048a7c2860c0513221d40" + integrity sha512-pcHzB+6KWd8BPStBhHM8achdNY/Yj1p3WSTEs/CSw61VRcfBfg5GZECtvEerTSX/0ZeawAM1ABvstIAYihcfAg== dependencies: eslint "^8.26.0" eslint-config-airbnb "^19.0.4" @@ -1274,10 +1274,10 @@ eslint-plugin-react "^7.31.10" eslint-webpack-plugin "^3.2.0" -"@silverstripe/webpack-config@^2.0.0-alpha5": - version "2.0.0-alpha8" - resolved "https://registry.yarnpkg.com/@silverstripe/webpack-config/-/webpack-config-2.0.0-alpha8.tgz#df97d047ceb0f1726cea7c18864ce15eba1361ce" - integrity sha512-aIwflLBdC2neS7zAq4qN+P2lhkb1vrxTVZSUILI7Gpthp/8tQ7k91W2UdXWWXOUytQYLo+5gBfkh4P60Mys/nQ== +"@silverstripe/webpack-config@^2.0.0": + version "2.0.0" + resolved "https://registry.yarnpkg.com/@silverstripe/webpack-config/-/webpack-config-2.0.0.tgz#278a72a1adbc6fa2362497d60424c78fba58e8e1" + integrity sha512-m1qGRxlsdhWL567cWe7IZNBUCzeyg3T1Y9yY9Y6XClwAqlg1oIO9uLfvfauA4dbtECrzU5n1AkaaU6kMRtN6Aw== dependencies: "@babel/core" "^7.19.6" "@babel/preset-env" "^7.19.4" From 24766e4e5ab91a82ae7f6ed5b8d8d24a552151ba Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Fri, 31 Mar 2023 15:51:10 +1300 Subject: [PATCH 6/9] 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 * From 36d3718581280cfcbbdcc3d35fcb42dd07b2439d Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Wed, 12 Apr 2023 10:45:36 +1200 Subject: [PATCH 7/9] ENH Add Exception with list of invilid emails --- src/Tasks/ContentReviewEmails.php | 37 +++++++++++++++------ tests/php/ContentReviewNotificationTest.php | 1 + 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/Tasks/ContentReviewEmails.php b/src/Tasks/ContentReviewEmails.php index a622c5b..2131796 100644 --- a/src/Tasks/ContentReviewEmails.php +++ b/src/Tasks/ContentReviewEmails.php @@ -3,6 +3,7 @@ namespace SilverStripe\ContentReview\Tasks; use Page; +use RuntimeException; use SilverStripe\ContentReview\Compatibility\ContentReviewCompatability; use SilverStripe\Control\Email\Email; use SilverStripe\Control\HTTPRequest; @@ -21,11 +22,23 @@ use SilverStripe\View\SSViewer; */ class ContentReviewEmails extends BuildTask { + private array $invalid_emails = []; + /** * @param HTTPRequest $request + * @throws RuntimeException */ public function run($request) { + if (!$this->isValidEmail($senderEmail = SiteConfig::current_site_config()->ReviewFrom)) { + throw new RuntimeException( + sprintf( + 'Provided sender email address is invalid: "%s".', + $senderEmail + ) + ); + } + $compatibility = ContentReviewCompatability::start(); // First grab all the pages with a custom setting @@ -41,6 +54,16 @@ class ContentReviewEmails extends BuildTask } ContentReviewCompatability::done($compatibility); + + if (is_array($this->invalid_emails) && count($this->invalid_emails) > 0) { + $plural = count($this->invalid_emails) > 1 ? 's are' : ' is'; + throw new RuntimeException( + sprintf( + 'Provided email' . $plural . ' invalid: "%s".', + implode(', ', $this->invalid_emails) + ) + ); + } } /** @@ -93,8 +116,9 @@ class ContentReviewEmails extends BuildTask $siteConfig = SiteConfig::current_site_config(); $owner = Member::get()->byID($ownerID); - if (!$this->isValidEmail($owner->Email) - || !$this->isValidEmail($siteConfig->ReviewFrom)) { + if (!$this->isValidEmail($owner->Email)) { + $this->invalid_emails[] = $owner->Name . ': ' . $owner->Email; + return; } @@ -170,13 +194,6 @@ class ContentReviewEmails extends BuildTask */ protected function isValidEmail(?string $email): bool { - if (!$email - || empty($email) - || !filter_var($email, FILTER_VALIDATE_EMAIL) - ) { - return false; - } - - return true; + return (bool) filter_var($email, FILTER_VALIDATE_EMAIL); } } diff --git a/tests/php/ContentReviewNotificationTest.php b/tests/php/ContentReviewNotificationTest.php index ca55d7f..241dd9c 100644 --- a/tests/php/ContentReviewNotificationTest.php +++ b/tests/php/ContentReviewNotificationTest.php @@ -150,6 +150,7 @@ class ContentReviewNotificationTest extends SapphireTest $this->assertFalse($method->invokeArgs($task, [null])); $this->assertFalse($method->invokeArgs($task, ['broken.email'])); + $this->assertFalse($method->invokeArgs($task, ['broken@email'])); } /** From c39e58830698b716d336fdd6539fea4de22821dd Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Fri, 14 Apr 2023 12:22:23 +1200 Subject: [PATCH 8/9] FIX Notification job marked as broken --- src/Tasks/ContentReviewEmails.php | 39 ++++++++++++++++++++- tests/php/ContentReviewNotificationTest.php | 21 +++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/Tasks/ContentReviewEmails.php b/src/Tasks/ContentReviewEmails.php index c59bc56..2131796 100644 --- a/src/Tasks/ContentReviewEmails.php +++ b/src/Tasks/ContentReviewEmails.php @@ -3,6 +3,7 @@ namespace SilverStripe\ContentReview\Tasks; use Page; +use RuntimeException; use SilverStripe\ContentReview\Compatibility\ContentReviewCompatability; use SilverStripe\Control\Email\Email; use SilverStripe\Control\HTTPRequest; @@ -15,18 +16,29 @@ 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. */ class ContentReviewEmails extends BuildTask { + private array $invalid_emails = []; + /** * @param HTTPRequest $request + * @throws RuntimeException */ public function run($request) { + if (!$this->isValidEmail($senderEmail = SiteConfig::current_site_config()->ReviewFrom)) { + throw new RuntimeException( + sprintf( + 'Provided sender email address is invalid: "%s".', + $senderEmail + ) + ); + } + $compatibility = ContentReviewCompatability::start(); // First grab all the pages with a custom setting @@ -42,6 +54,16 @@ class ContentReviewEmails extends BuildTask } ContentReviewCompatability::done($compatibility); + + if (is_array($this->invalid_emails) && count($this->invalid_emails) > 0) { + $plural = count($this->invalid_emails) > 1 ? 's are' : ' is'; + throw new RuntimeException( + sprintf( + 'Provided email' . $plural . ' invalid: "%s".', + implode(', ', $this->invalid_emails) + ) + ); + } } /** @@ -93,6 +115,13 @@ class ContentReviewEmails extends BuildTask // Prepare variables $siteConfig = SiteConfig::current_site_config(); $owner = Member::get()->byID($ownerID); + + if (!$this->isValidEmail($owner->Email)) { + $this->invalid_emails[] = $owner->Name . ': ' . $owner->Email; + + return; + } + $templateVariables = $this->getTemplateVariables($owner, $siteConfig, $pages); // Build email @@ -159,4 +188,12 @@ class ContentReviewEmails extends BuildTask 'ToEmail' => $recipient->Email, ]; } + + /** + * Check validity of email + */ + protected function isValidEmail(?string $email): bool + { + return (bool) filter_var($email, FILTER_VALIDATE_EMAIL); + } } diff --git a/tests/php/ContentReviewNotificationTest.php b/tests/php/ContentReviewNotificationTest.php index d2e7aca..241dd9c 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,26 @@ 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'])); + $this->assertFalse($method->invokeArgs($task, ['broken@email'])); + } + /** * Deletes all pages except those passes in to the $ids parameter * From 7a7e930d402c77e60942855578888c41819f4d62 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Wed, 19 Apr 2023 16:14:53 +1200 Subject: [PATCH 9/9] DOC Update README.md for CMS 5 --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 019c121..1371452 100644 --- a/README.md +++ b/README.md @@ -13,11 +13,11 @@ There are two types of roles with this module. * Website owner; (typically assigned to the Administrator group) ensures that a website is accurate and up-to-date, by delegating responsibility to content reviewers. * Content reviewer; responsible for keeping a website or part of a website accurate and up-to-date. -## Requirements +## Installation - * Silverstripe ^4.0 - - **Note:** For Silverstripe 3.x, please use the [3.x release line](https://github.com/silverstripe/silverstripe-contentreview/tree/3). +```sh +composer require silverstripe/contentreview +``` ## Features @@ -36,7 +36,7 @@ There are two types of roles with this module. ## Composer installation ```sh -$ composer require silverstripe/contentreview +composer require silverstripe/contentreview ``` You'll also need to run `dev/build`.