Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH allow stacked messages on FormMessage #10966

Merged
merged 1 commit into from
Oct 9, 2023
Merged

ENH allow stacked messages on FormMessage #10966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Forms/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand Down
53 changes: 52 additions & 1 deletion src/Forms/FormMessage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ trait FormMessage
*/
protected $messageCast = null;


/**
* Returns the field message, used by form validation.
*
Expand Down Expand Up @@ -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)) {
andrewandante marked this conversation as resolved.
Show resolved Hide resolved
return $this->setMessage($message, $messageType, $messageCast);
}

$canBeMerged = ($messageType === $this->getMessageType() && $messageCast === $this->getMessageCast());
andrewandante marked this conversation as resolved.
Show resolved Hide resolved

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;
}
andrewandante marked this conversation as resolved.
Show resolved Hide resolved

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
*
Expand Down
115 changes: 115 additions & 0 deletions tests/php/Forms/FormTest.php
View file
Open in desktop
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -1030,6 +1097,54 @@ public function testFieldMessageEscapeHtml()
);
}

/**
* @dataProvider formMessageDataProvider
*/
public function testFieldMessageAppend($messages)
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
{
$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(
Expand Down