From a0cbebb2d1d8459284343132340d5180e0b69213 Mon Sep 17 00:00:00 2001 From: Andrew Paxley Date: Wed, 4 Oct 2023 10:19:35 +1300 Subject: [PATCH] ENH allow stacked messages on FormMessage --- src/Forms/Form.php | 5 +- src/Forms/FormMessage.php | 53 +++++++++++++++- tests/php/Forms/FormTest.php | 115 +++++++++++++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 3 deletions(-) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 96b9d1ee454..7f9b2251ff8 100644 --- 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 index 3fd5c56e242..ce18ee153a6 100644 --- 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 ? '
' : 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 index eab888d68be..8b97293443a 100644 --- 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(