Merge pull request #10966 from andrewandante/ENH/better_pwd_validatio…

…n_msgs ENH allow stacked messages on FormMessage
silverstripe · Oct 9, 2023 · 87958e7 · 87958e7
2 parents 44b1700 + a0cbebb
commit 87958e7
Show file tree

Hide file tree

Showing 3 changed files with 170 additions and 3 deletions.
diff --git a/src/Forms/Form.php b/src/Forms/Form.php
@@ -477,7 +477,8 @@ public function clearMessage()
 
     /**
      * Populate this form with messages from the given ValidationResult.
-     * Note: This will not clear any pre-existing messages
+     * Note: This will try not to clear any pre-existing messages, but will clear them
+     * if a new message has a different message type or cast than the existing ones.
      *
      * @param ValidationResult $result
      * @return $this
@@ -494,7 +495,7 @@ public function loadMessagesFrom($result)
                 $owner = $this;
             }
 
-            $owner->setMessage($message['message'], $message['messageType'], $message['messageCast']);
+            $owner->appendMessage($message['message'], $message['messageType'], $message['messageCast'], true);
         }
         return $this;
     }

diff --git a/src/Forms/FormMessage.php b/src/Forms/FormMessage.php
@@ -31,7 +31,6 @@ trait FormMessage
      */
     protected $messageCast = null;
 
-
     /**
      * Returns the field message, used by form validation.
      *
@@ -92,6 +91,58 @@ public function setMessage(
         return $this;
     }
 
+    /**
+     * Appends a message to the existing message if the types and casts match.
+     * If either is different, the $force argument determines the behaviour.
+     *
+     * Note: to prevent duplicates, we check for the $message string in the existing message.
+     * If the existing message contains $message as a substring, it won't be added.
+     *
+     * @param bool $force if true, and the new message cannot be appended to the existing one, the existing message will be overridden.
+     * @throws InvalidArgumentException if $force is false and the messages can't be merged because of a mismatched type or cast.
+     */
+    public function appendMessage(
+        string $message,
+        string $messageType = ValidationResult::TYPE_ERROR,
+        string $messageCast = ValidationResult::CAST_TEXT,
+        bool $force = false,
+    ): static {
+        if (empty($message)) {
+            return $this;
+        }
+
+        if (empty($this->message)) {
+            return $this->setMessage($message, $messageType, $messageCast);
+        }
+
+        $canBeMerged = ($messageType === $this->getMessageType() && $messageCast === $this->getMessageCast());
+
+        if (!$canBeMerged && !$force) {
+            throw new InvalidArgumentException(
+                sprintf(
+                    "Couldn't append message of type %s and cast %s to existing message of type %s and cast %s",
+                    $messageType,
+                    $messageCast,
+                    $this->getMessageType(),
+                    $this->getMessageCast(),
+                )
+            );
+        }
+
+        // Checks that the exact message string is not already contained before appending
+        $messageContainsString = strpos($this->message, $message) !== false;
+        if ($canBeMerged && $messageContainsString) {
+            return $this;
+        }
+
+        if ($canBeMerged) {
+            $separator = $messageCast === ValidationResult::CAST_HTML ? '<br />' : PHP_EOL;
+            $message = $this->message . $separator . $message;
+        }
+
+        return $this->setMessage($message, $messageType, $messageCast);
+    }
+
     /**
      * Get casting helper for message cast, or null if not known
      *

diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php
@@ -76,6 +76,73 @@ public function boolDataProvider()
         ];
     }
 
+    public function formMessageDataProvider()
+    {
+        return [
+            [
+                [
+                    'Just a string',
+                ],
+            ],
+            [
+                [
+                    'Just a string',
+                    'Certainly different',
+                ],
+            ],
+            [
+                [
+                    'Just a string',
+                    'Certainly different',
+                    'Just a string',
+                ],
+            ],
+        ];
+    }
+
+    public function formMessageExceptionsDataProvider()
+    {
+        return [
+            [
+                'message_1' => [
+                    'val' => 'Just a string',
+                    'type' => ValidationResult::TYPE_ERROR,
+                    'cast' => ValidationResult::CAST_TEXT,
+                ],
+                'message_2' => [
+                    'val' => 'This is a good message',
+                    'type' => ValidationResult::TYPE_GOOD,
+                    'cast' => ValidationResult::CAST_TEXT,
+                ],
+            ],
+            [
+                'message_1' => [
+                    'val' => 'This is a good message',
+                    'type' => ValidationResult::TYPE_GOOD,
+                    'cast' => ValidationResult::CAST_TEXT,
+                ],
+                'message_2' => [
+                    'val' => 'HTML is the future of the web',
+                    'type' => ValidationResult::TYPE_GOOD,
+                    'cast' => ValidationResult::CAST_HTML,
+                ],
+            ],
+            [
+                'message_1' => [
+                    'val' => 'This is a good message',
+                    'type' => ValidationResult::TYPE_GOOD,
+                    'cast' => ValidationResult::CAST_TEXT,
+                ],
+                'message_2' => [
+                    'val' => 'HTML is the future of the web',
+                    'type' => ValidationResult::TYPE_GOOD,
+                    'cast' => ValidationResult::CAST_HTML,
+                ],
+                'force' => true,
+            ],
+        ];
+    }
+
     public function testLoadDataFromRequest()
     {
         $form = new Form(
@@ -1030,6 +1097,54 @@ public function testFieldMessageEscapeHtml()
         );
     }
 
+    /**
+     * @dataProvider formMessageDataProvider
+     */
+    public function testFieldMessageAppend($messages)
+    {
+        $form = $this->getStubForm();
+        foreach ($messages as $message) {
+            $form->appendMessage($message);
+        }
+        $parser = new CSSContentParser($form->forTemplate());
+        $messageEls = $parser->getBySelector('.message');
+
+        foreach ($messages as $message) {
+            $this->assertStringContainsString($message, $messageEls[0]->asXML());
+            $this->assertEquals(1, substr_count($messageEls[0]->asXML(), $message), 'Should not append if already present');
+        }
+    }
+
+    /**
+     * @dataProvider formMessageExceptionsDataProvider
+     */
+    public function testFieldMessageAppendExceptions(array $message1, array $message2, bool $force = false)
+    {
+        $form = $this->getStubForm();
+        $form->appendMessage($message1['val'], $message1['type'], $message1['cast']);
+        if (!$force) {
+            $this->expectException(\InvalidArgumentException::class);
+            $this->expectExceptionMessage(
+                sprintf(
+                    "Couldn't append message of type %s and cast %s to existing message of type %s and cast %s",
+                    $message2['type'],
+                    $message2['cast'],
+                    $message1['type'],
+                    $message1['cast'],
+                )
+            );
+        }
+
+        $form->appendMessage($message2['val'], $message2['type'], $message2['cast'], $force);
+
+        if ($force) {
+            $parser = new CSSContentParser($form->forTemplate());
+            $messageEls = $parser->getBySelector('.message');
+            $this->assertStringContainsString($message2['val'], $messageEls[0]->asXML());
+            $this->assertStringNotContainsString($message1['val'], $messageEls[0]->asXML());
+        }
+    }
+
     public function testGetExtraFields()
     {
         $form = new FormTest\ExtraFieldsForm(