Issue #3276009 by jonathan1055: UnpublishStateConstraint passes when...

Issue #3276009 by jonathan1055: UnpublishStateConstraint passes when publish-on state is entered with no date

src/Plugin/Validation/Constraint/UnPublishStateConstraintValidator.php

@@ -31,7 +31,9 @@ class UnPublishStateConstraintValidator extends ConstraintValidatorBase {
       return;
     }
 
-    $publish_state = $entity->publish_state->value === '_none' ? NULL : $entity->publish_state->value;
+    // If there is no publish_on date then any publish_state value should be
+    // ignored and the validation will run as if no publish_state was selected.
+    $publish_state = ($entity->publish_state->value === '_none' || empty($entity->publish_on->value)) ? NULL : $entity->publish_state->value;
     $unpublish_state = $entity->unpublish_state->value;
     $moderation_state = $entity->moderation_state->value;
 

tests/src/Functional/UnpublishedStateConstraintTest.php

@@ -33,7 +33,7 @@ class UnpublishedStateConstraintTest extends SchedulerContentModerationBrowserTe
     ]);
     // Assert that the publish and unpublish states pass validation.
     $violations = $entity->validate();
-    $this->assertCount(0, $violations);
+    $this->assertCount(0, $violations, 'Both transitions should pass validation');
   }
 
   /**
@@ -49,17 +49,22 @@ class UnpublishedStateConstraintTest extends SchedulerContentModerationBrowserTe
   public function testInvalidUnPublishStateTransition($entityTypeId, $bundle) {
     $this->drupalLogin($this->schedulerUser);
 
-    $entity = $this->drupalCreateNode([
-      'type' => $bundle,
-      'moderation_state' => 'draft',
-      'unpublish_on' => strtotime('+3 days'),
-      'unpublish_state' => 'archived',
-    ]);
-    // Assert that the change from draft to archived fails validation.
-    $violations = $entity->validate();
-    $this->assertCount(1, $violations);
-    $message = (count($violations) > 0) ? $violations->get(0)->getMessage() : 'No violation message found';
-    $this->assertEquals('The scheduled un-publishing state of <em class="placeholder">archived</em> is not a valid transition from the current moderation state of <em class="placeholder">draft</em> for this content.', $message);
+    // Check cases when a publish_state has been selected and not selected.
+    // No publish_on date been entered, so they should fail validation.
+    foreach (['', '_none', 'published'] as $publish_state) {
+      $entity = $this->drupalCreateNode([
+        'type' => $bundle,
+        'moderation_state' => 'draft',
+        'publish_state' => $publish_state,
+        'unpublish_on' => strtotime('+3 days'),
+        'unpublish_state' => 'archived',
+      ]);
+      // Assert that the change from draft to archived fails validation.
+      $violations = $entity->validate();
+      $this->assertCount(1, $violations, "The transition from draft to archived with publish_state='{$publish_state}' should fail validation");
+      $message = (count($violations) > 0) ? $violations->get(0)->getMessage() : 'No violation message found';
+      $this->assertEquals('The scheduled un-publishing state of archived is not a valid transition from the current moderation state of draft for this content.', strip_tags($message));
+    }
   }
 
   /**
@@ -77,6 +82,8 @@ class UnpublishedStateConstraintTest extends SchedulerContentModerationBrowserTe
     // id 1 which will have permission to use the new state created below.
     $this->drupalLogin($this->rootUser);
 
+    // Add a second published state, and a transition to it from draft, but no
+    // transition from it to archived.
     $this->workflow->getTypePlugin()
       ->addState('published_2', 'Published 2')
       ->addTransition('published_2', 'Published 2', ['draft'], 'published_2');
@@ -95,10 +102,12 @@ class UnpublishedStateConstraintTest extends SchedulerContentModerationBrowserTe
       'unpublish_on' => strtotime('+2 days'),
       'unpublish_state' => 'archived',
     ]);
+    // Check that the attempted scheduled transition from the new published_2
+    // state to archived fails validation.
     $violations = $entity->validate();
-    $this->assertCount(1, $violations);
+    $this->assertCount(1, $violations, 'The transition from published 2 to archived should fail validation');
     $message = (count($violations) > 0) ? $violations->get(0)->getMessage() : 'No violation message found';
-    $this->assertEquals('The scheduled un-publishing state of <em class="placeholder">archived</em> is not a valid transition from the scheduled publishing state of <em class="placeholder">published_2</em>.', $message);
+    $this->assertEquals('The scheduled un-publishing state of archived is not a valid transition from the scheduled publishing state of published_2.', strip_tags($message));
   }
 
 }

tests/src/Kernel/UnPublishedStateConstraintTest.php

@@ -44,7 +44,7 @@ class UnPublishedStateConstraintTest extends SchedulerContentModerationTestBase
     ]);
 
     $violations = $node->validate();
-    $this->assertCount(0, $violations);
+    $this->assertCount(0, $violations, 'Both transitions should pass validation');
   }
 
   /**
@@ -56,18 +56,23 @@ class UnPublishedStateConstraintTest extends SchedulerContentModerationTestBase
    * @cover ::validate
    */
   public function testInvalidUnPublishStateTransition() {
-    $node = Node::create([
-      'type' => 'example',
-      'title' => 'Test title',
-      'moderation_state' => 'draft',
-      'unpublish_on' => strtotime('tomorrow'),
-      'unpublish_state' => 'archived',
-    ]);
-
-    $violations = $node->validate();
-
-    $this->assertCount(1, $violations);
-    $this->assertEquals('The scheduled un-publishing state of <em class="placeholder">archived</em> is not a valid transition from the current moderation state of <em class="placeholder">draft</em> for this content.', $violations->get(0)->getMessage());
+    // Check cases when a publish_state has been selected and not selected.
+    // No publish_on date been entered, so they should fail validation.
+    foreach (['', '_none', 'published'] as $publish_state) {
+      $node = Node::create([
+        'type' => 'example',
+        'title' => 'Test title',
+        'moderation_state' => 'draft',
+        'publish_state' => $publish_state,
+        'unpublish_on' => strtotime('tomorrow'),
+        'unpublish_state' => 'archived',
+      ]);
+
+      // Assert that the change from draft to archived fails validation.
+      $violations = $node->validate();
+      $message = (count($violations) > 0) ? $violations->get(0)->getMessage() : 'No violation message found';
+      $this->assertEquals('The scheduled un-publishing state of archived is not a valid transition from the current moderation state of draft for this content.', strip_tags($message));
+    }
   }
 
   /**
@@ -79,6 +84,8 @@ class UnPublishedStateConstraintTest extends SchedulerContentModerationTestBase
    * @covers ::validate
    */
   public function testInvalidPublishStateToUnPublishStateTransition() {
+    // Add a second published state, and a transition to it from draft, but no
+    // transition from it to archived.
     $this->workflow->getTypePlugin()
       ->addState('published_2', 'Published 2')
       ->addTransition('published_2', 'Published 2', ['draft'], 'published_2');
@@ -100,10 +107,12 @@ class UnPublishedStateConstraintTest extends SchedulerContentModerationTestBase
       'publish_state' => 'published_2',
     ]);
 
+    // Check that the attempted scheduled transition from the new published_2
+    // state to archived fails validation.
     $violations = $node->validate();
-
-    $this->assertCount(1, $violations);
-    $this->assertEquals('The scheduled un-publishing state of <em class="placeholder">archived</em> is not a valid transition from the scheduled publishing state of <em class="placeholder">published_2</em>.', $violations->get(0)->getMessage());
+    $this->assertCount(1, $violations, 'The transition from published 2 to archived should fail validation');
+    $message = (count($violations) > 0) ? $violations->get(0)->getMessage() : 'No violation message found';
+    $this->assertEquals('The scheduled un-publishing state of archived is not a valid transition from the scheduled publishing state of published_2.', strip_tags($message));
   }
 
 }