Bugfix: Check Job class and regenerate if mismatched

Author: Chris Penny

phpcs.xml.dist

@@ -34,6 +34,8 @@
         <!-- Class length does not equal complexity. We absolutely don't care about class length -->
         <exclude name="SlevomatCodingStandard.Classes.ClassLength.ClassTooLong"/>
         <exclude name="SlevomatCodingStandard.Files.FileLength.FileTooLong"/>
+        <!-- We'll be the ones to decide "complexity", not PHPCS -->
+        <exclude name="SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh"/>
         <!-- Don't force the use of early exit. We do try to use it though, and you might get PR feedback where -->
         <!-- early exit is appropriate -->
         <exclude name="SlevomatCodingStandard.ControlStructures.EarlyExit.EarlyExitNotUsed"/>

src/Extension/EmbargoExpiryExtension.php

@@ -525,6 +525,9 @@ public function createOrUpdatePublishJob(int $desiredPublishTime): void
         if ($job !== null
             && $job->exists()
             && DBDatetime::create()->setValue($job->StartAfter)->getTimestamp() === $desiredPublishTime
+            // This check is (mostly) to support migrations from Workflow to E&E. If we previously had a Workflow job,
+            // we would want to clear and update this to an E&E job
+            && $job->Implementation === PublishTargetJob::class
         ) {
             // Make sure our PublishOnDate is up to date.
             $this->updatePublishOnDate();
@@ -579,6 +582,9 @@ public function createOrUpdateUnPublishJob(int $desiredUnPublishTime): void
         if ($job !== null
             && $job->exists()
             && DBDatetime::create()->setValue($job->StartAfter)->getTimestamp() === $desiredUnPublishTime
+            // This check is (mostly) to support migrations from Workflow to E&E. If we previously had a Workflow job,
+            // we would want to clear and update this to an E&E job
+            && $job->Implementation === UnPublishTargetJob::class
         ) {
             // Make sure our UnPublishOnDate is up to date.
             $this->updateUnPublishOnDate();

tests/Extension/EmbargoExpiryExtensionTest.php

@@ -4,6 +4,7 @@
 
 use DateTimeImmutable;
 use Exception;
+use Page;
 use SilverStripe\CMS\Model\SiteTree;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\SapphireTest;
@@ -143,6 +144,39 @@ public function testIsNotEditableWithEmbargo(): void
         $this->assertFalse($page->isEditable());
     }
 
+    public function testCreateOrUpdateJobMethods(): void
+    {
+        $embargo = '2014-02-05 12:00:00';
+        $expiry = '2014-02-07 12:00:00';
+
+        /** @var Page|EmbargoExpiryExtension $page */
+        $page = Page::create();
+        $page->Title = 'Test Page';
+        // This won't yet generate Jobs, as the Page first needs to exist
+        $page->write();
+        // Now we can set our dates
+        $page->DesiredPublishDate = $embargo;
+        $page->DesiredUnPublishDate = $expiry;
+        // This should generate Jobs for the two dates above
+        $page->write();
+
+        $this->assertNotNull($page->PublishJob());
+        $this->assertTrue($page->PublishJob()->exists());
+        $this->assertNotNull($page->UnPublishJob());
+        $this->assertTrue($page->UnPublishJob()->exists());
+
+        // Save away the Job IDs for comparison later
+        $publishJobId = $page->PublishJobID;
+        $unPublishJobId = $page->UnPublishJobID;
+
+        // This should NOT generate any new Jobs, they should remain exactly the same
+        $page->createOrUpdatePublishJob(strtotime($embargo));
+        $page->createOrUpdateUnPublishJob(strtotime($expiry));
+
+        $this->assertEquals($publishJobId, $page->PublishJobID);
+        $this->assertEquals($unPublishJobId, $page->UnPublishJobID);
+    }
+
     /**
      * @throws ValidationException
      * @throws Exception