ENH Use symfony/validation logic

README.md

@@ -39,10 +39,7 @@ composer require silverstripe/contentreview
 composer require silverstripe/contentreview
 ```
 
-You'll also need to run `dev/build`.
-
-Run dev/build either via the web server by opening the URL `http://<your-host>/dev/build?flush` or
-by running the dev/build via a CLI: `sake dev/build flush=1`
+You'll also need to build the database either via the web server by opening the URL `http://<your-host>/dev/build?flush` or via a CLI: `sake db:build --flush`
 
 ## Documentation
 

docs/en/index.md

@@ -11,7 +11,7 @@ The module is set up in the `Settings` section of the CMS, see the [User guide](
 In order for the contentreview module to send emails, you need to *either*:
 
  * Setup the `ContentReviewEmails` script to run daily via a system cron job.
- * Install the [queuedjobs](https://github.com/symbiote/silverstripe-queuedjobs) module and follow the configuration steps to create a cron job for that module. Once installed, you can just run `dev/build` to have a job created, which will run at 9am every day by default.
+ * Install the [queuedjobs](https://github.com/symbiote/silverstripe-queuedjobs) module and follow the configuration steps to create a cron job for that module. Once installed, you can just run `sake db:build` to have a job created, which will run at 9am every day by default.
 
 ## Using
 

src/Jobs/ContentReviewNotificationJob.php

@@ -5,9 +5,11 @@ namespace SilverStripe\ContentReview\Jobs;
 use SilverStripe\ContentReview\Tasks\ContentReviewEmails;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Core\Config\Config;
+use SilverStripe\PolyExecution\PolyOutput;
 use Symbiote\QueuedJobs\Services\AbstractQueuedJob;
 use Symbiote\QueuedJobs\Services\QueuedJob;
 use Symbiote\QueuedJobs\Services\QueuedJobService;
+use Symfony\Component\Console\Input\ArrayInput;
 
 if (!class_exists(AbstractQueuedJob::class)) {
     return;
@@ -94,7 +96,10 @@ class ContentReviewNotificationJob extends AbstractQueuedJob implements QueuedJo
         $this->queueNextRun();
 
         $task = ContentReviewEmails::create();
-        $task->run(new HTTPRequest("GET", "/dev/tasks/ContentReviewEmails"));
+        $output = PolyOutput::create(PolyOutput::FORMAT_ANSI);
+        $input = new ArrayInput([]);
+        $input->setInteractive(false);
+        $task->run($input, $output);
 
         $this->currentStep = 1;
         $this->isComplete = true;

src/Tasks/ContentReviewEmails.php

@@ -3,13 +3,11 @@
 namespace SilverStripe\ContentReview\Tasks;
 
 use Page;
-use RuntimeException;
 use SilverStripe\ContentReview\Compatibility\ContentReviewCompatability;
 use SilverStripe\Control\Email\Email;
-use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Dev\BuildTask;
+use SilverStripe\PolyExecution\PolyOutput;
 use SilverStripe\Model\List\ArrayList;
-use SilverStripe\Dev\Deprecation;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\Model\List\SS_List;
@@ -17,6 +15,8 @@ use SilverStripe\Security\Member;
 use SilverStripe\SiteConfig\SiteConfig;
 use SilverStripe\Model\ArrayData;
 use SilverStripe\View\SSViewer;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
 
 /**
  * Daily task to send emails to the owners of content items when the review date rolls around.
@@ -25,20 +25,22 @@ class ContentReviewEmails extends BuildTask
 {
     private array $invalid_emails = [];
 
-    /**
-     * @param HTTPRequest $request
-     * @throws RuntimeException
-     */
-    public function run($request)
+    protected static string $commandName = 'content-review-emails';
+
+    protected static string $description = 'Daily task to send emails to the owners of content items when the review'
+        . ' date rolls around';
+
+    protected function execute(InputInterface $input, PolyOutput $output): int
     {
         $senderEmail = SiteConfig::current_site_config()->ReviewFrom;
-        if (!Deprecation::withSuppressedNotice(fn() => $this->isValidEmail($senderEmail))) {
-            throw new RuntimeException(
+        if (!Email::is_valid_address($senderEmail)) {
+            $output->writeln(
                 sprintf(
-                    'Provided sender email address is invalid: "%s".',
+                    '<error>Provided sender email address is invalid: "%s".</>',
                     $senderEmail
                 )
             );
+            return Command::FAILURE;
         }
 
         $compatibility = ContentReviewCompatability::start();
@@ -59,13 +61,16 @@ class ContentReviewEmails extends BuildTask
 
         if (is_array($this->invalid_emails) && count($this->invalid_emails) > 0) {
             $plural = count($this->invalid_emails) > 1 ? 's are' : ' is';
-            throw new RuntimeException(
+            $output->writeln(
                 sprintf(
-                    'Provided email' . $plural . ' invalid: "%s".',
+                    '<error>Provided email' . $plural . ' invalid: "%s".</>',
                     implode(', ', $this->invalid_emails)
                 )
             );
+            return Command::FAILURE;
         }
+
+        return Command::SUCCESS;
     }
 
     /**
@@ -118,7 +123,7 @@ class ContentReviewEmails extends BuildTask
         $siteConfig = SiteConfig::current_site_config();
         $owner = Member::get()->byID($ownerID);
 
-        if (!Deprecation::withSuppressedNotice(fn() => $this->isValidEmail($owner->Email))) {
+        if (!Email::is_valid_address($owner->Email)) {
             $this->invalid_emails[] = $owner->Name . ': ' . $owner->Email;
 
             return;
@@ -190,14 +195,4 @@ class ContentReviewEmails extends BuildTask
             'ToEmail' => $recipient->Email,
         ];
     }
-
-    /**
-     * Check validity of email
-     * @deprecated 5.4.0 Use SilverStripe\Control\Email\Email::is_valid_address() instead.
-     */
-    protected function isValidEmail(?string $email): bool
-    {
-        Deprecation::notice('5.4.0', 'Use ' . Email::class . '::is_valid_address() instead.');
-        return (bool) filter_var($email, FILTER_VALIDATE_EMAIL);
-    }
 }

src/Tasks/ContentReviewOwnerMigrationTask.php

@@ -1,47 +0,0 @@
-<?php
-
-namespace SilverStripe\ContentReview\Tasks;
-
-use SilverStripe\Control\HTTPRequest;
-use SilverStripe\Dev\BuildTask;
-use SilverStripe\Dev\Deprecation;
-use SilverStripe\ORM\DB;
-
-/**
- * Task which migrates the ContentReview Module's SiteTree->OwnerID column to a new column name.
- * @deprecated 5.4.0 Will be removed without equivalent functionality to replace it
- */
-class ContentReviewOwnerMigrationTask extends BuildTask
-{
-    public function __construct()
-    {
-        parent::__construct();
-        Deprecation::withSuppressedNotice(function () {
-            Deprecation::notice(
-                '5.4.0',
-                'Will be removed without equivalent functionality to replace it',
-                Deprecation::SCOPE_CLASS
-            );
-        });
-    }
-
-    /**
-     * @param HTTPRequest $request
-     */
-    public function run($request)
-    {
-        $results = DB::query("SHOW columns from \"SiteTree\" WHERE \"field\" = 'OwnerID'");
-
-        if ($results->numRecords() == 0) {
-            echo "<h1>No need to run task. SiteTree->OwnerID doesn't exist</h1>";
-        } else {
-            DB::query("UPDATE \"SiteTree\" SET \"ContentReviewOwnerID\" = \"OwnerID\"");
-            DB::query("UPDATE \"SiteTree_Live\" SET \"ContentReviewOwnerID\" = \"OwnerID\"");
-            DB::query("UPDATE \"SiteTree_versions\" SET \"ContentReviewOwnerID\" = \"OwnerID\"");
-            DB::query("ALTER TABLE \"SiteTree\" DROP COLUMN \"OwnerID\"");
-            DB::query("ALTER TABLE \"SiteTree_Live\" DROP COLUMN \"OwnerID\"");
-            DB::query("ALTER TABLE \"SiteTree_Versions\" DROP COLUMN \"OwnerID\"");
-            echo "<h1>Migrated 3 tables. Dropped obsolete OwnerID column</h1>";
-        }
-    }
-}

tests/php/ContentReviewNotificationTest.php

@@ -12,13 +12,15 @@ use SilverStripe\ContentReview\Extensions\ContentReviewOwner;
 use SilverStripe\ContentReview\Extensions\SiteTreeContentReview;
 use SilverStripe\ContentReview\Tasks\ContentReviewEmails;
 use SilverStripe\Control\HTTPRequest;
-use SilverStripe\Dev\Deprecation;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\Security\Group;
 use SilverStripe\Security\Member;
 use SilverStripe\SiteConfig\SiteConfig;
 use SilverStripe\ContentReview\Models\ContentReviewLog;
+use SilverStripe\PolyExecution\PolyOutput;
+use Symfony\Component\Console\Input\ArrayInput;
+use Symfony\Component\Console\Output\BufferedOutput;
 
 class ContentReviewNotificationTest extends SapphireTest
 {
@@ -59,7 +61,11 @@ class ContentReviewNotificationTest extends SapphireTest
         $childParentPage->write();
 
         $task = new ContentReviewEmails();
-        $task->run(new HTTPRequest('GET', '/dev/tasks/ContentReviewEmails'));
+        $buffer = new BufferedOutput();
+        $output = new PolyOutput(PolyOutput::FORMAT_ANSI, wrappedOutput: $buffer);
+        $input = new ArrayInput([]);
+        $input->setInteractive(false);
+        $task->run($input, $output);
 
         // Set template variables (as per variable case)
         $ToEmail = 'author@example.com';
@@ -125,7 +131,11 @@ class ContentReviewNotificationTest extends SapphireTest
         $this->assertCount(1, $childParentPage->ReviewLogs());
 
         $task = new ContentReviewEmails();
-        $task->run(new HTTPRequest('GET', '/dev/tasks/ContentReviewEmails'));
+        $buffer = new BufferedOutput();
+        $output = new PolyOutput(PolyOutput::FORMAT_ANSI, wrappedOutput: $buffer);
+        $input = new ArrayInput([]);
+        $input->setInteractive(false);
+        $task->run($input, $output);
 
         // Expecting to not send the email as content review for page is done
         $email = $this->findEmail($member->Email);
@@ -134,28 +144,6 @@ 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();
-
-        Deprecation::withSuppressedNotice(function () use ($method, $task, $member) {
-            $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
      *