Skip to content

Commit

Permalink
feature #20978 [Form] TransformationFailedException: Support specifyi…
Browse files Browse the repository at this point in the history
…ng message to display (ogizanagi)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Form] TransformationFailedException: Support specifying message to display

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22501
| License       | MIT
| Doc PR        | N/A

Currently, a failed transformation can't display a very accurate message, as it only uses the value of the `invalid_message` option. I suggest to add more flexibility, somehow the same way we did for `CustomUserMessageAuthenticationException`.

A test case in `FormTypeTest` shows a use-case based on @webmozart's [blog post about value/immutable objects in Symfony forms](https://webmozart.io/blog/2015/09/09/value-objects-in-symfony-forms/).

~I named the exception properties and methods the same way as for `CustomUserMessageAuthenticationException`, but do you think the followings would be better:~

- ~`setSafeMessage` ➜ `setInvalidMessage`~
- ~`getMessageKey` ➜ `getInvalidMessageKey`~
- ~`getMessageData` ➜ `getInvalidMessageParameters`~

~in order to echoes `invalid_message` and `invalid_message_parameters` options?~

=> Replaced to use `invalidMessage` & `invalidMessageParameters`

Commits
-------

d11055c [Form] TransformationFailedException: Support specifying message to display
  • Loading branch information
fabpot committed Mar 27, 2019
2 parents 76260e7 + d11055c commit 79aeed6
Show file tree
Hide file tree
Showing 5 changed files with 169 additions and 7 deletions.
Expand Up @@ -18,4 +18,35 @@
*/
class TransformationFailedException extends RuntimeException
{
private $invalidMessage;
private $invalidMessageParameters;

public function __construct(string $message = '', int $code = 0, \Throwable $previous = null, string $invalidMessage = null, array $invalidMessageParameters = [])
{
parent::__construct($message, $code, $previous);

$this->setInvalidMessage($invalidMessage, $invalidMessageParameters);
}

/**
* Sets the message that will be shown to the user.
*
* @param string|null $invalidMessage The message or message key
* @param array $invalidMessageParameters Data to be passed into the translator
*/
public function setInvalidMessage(string $invalidMessage = null, array $invalidMessageParameters = []): void
{
$this->invalidMessage = $invalidMessage;
$this->invalidMessageParameters = $invalidMessageParameters;
}

public function getInvalidMessage(): ?string
{
return $this->invalidMessage;
}

public function getInvalidMessageParameters(): array
{
return $this->invalidMessageParameters;
}
}
Expand Up @@ -118,12 +118,18 @@ public function validate($form, Constraint $formConstraint)
? (string) $form->getViewData()
: \gettype($form->getViewData());

$failure = $form->getTransformationFailure();

$this->context->setConstraint($formConstraint);
$this->context->buildViolation($config->getOption('invalid_message'))
->setParameters(array_replace(['{{ value }}' => $clientDataAsString], $config->getOption('invalid_message_parameters')))
$this->context->buildViolation($failure->getInvalidMessage() ?? $config->getOption('invalid_message'))
->setParameters(array_replace(
['{{ value }}' => $clientDataAsString],
$config->getOption('invalid_message_parameters'),
$failure->getInvalidMessageParameters()
))
->setInvalidValue($form->getViewData())
->setCode(Form::NOT_SYNCHRONIZED_ERROR)
->setCause($form->getTransformationFailure())
->setCause($failure)
->addViolation();
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Component/Form/Form.php
Expand Up @@ -1070,7 +1070,7 @@ private function modelToNorm($value)
$value = $transformer->transform($value);
}
} catch (TransformationFailedException $exception) {
throw new TransformationFailedException('Unable to transform value for property path "'.$this->getPropertyPath().'": '.$exception->getMessage(), $exception->getCode(), $exception);
throw new TransformationFailedException('Unable to transform value for property path "'.$this->getPropertyPath().'": '.$exception->getMessage(), $exception->getCode(), $exception, $exception->getInvalidMessage(), $exception->getInvalidMessageParameters());
}

return $value;
Expand All @@ -1094,7 +1094,7 @@ private function normToModel($value)
$value = $transformers[$i]->reverseTransform($value);
}
} catch (TransformationFailedException $exception) {
throw new TransformationFailedException('Unable to reverse value for property path "'.$this->getPropertyPath().'": '.$exception->getMessage(), $exception->getCode(), $exception);
throw new TransformationFailedException('Unable to reverse value for property path "'.$this->getPropertyPath().'": '.$exception->getMessage(), $exception->getCode(), $exception, $exception->getInvalidMessage(), $exception->getInvalidMessageParameters());
}

return $value;
Expand Down Expand Up @@ -1125,7 +1125,7 @@ private function normToView($value)
$value = $transformer->transform($value);
}
} catch (TransformationFailedException $exception) {
throw new TransformationFailedException('Unable to transform value for property path "'.$this->getPropertyPath().'": '.$exception->getMessage(), $exception->getCode(), $exception);
throw new TransformationFailedException('Unable to transform value for property path "'.$this->getPropertyPath().'": '.$exception->getMessage(), $exception->getCode(), $exception, $exception->getInvalidMessage(), $exception->getInvalidMessageParameters());
}

return $value;
Expand Down Expand Up @@ -1153,7 +1153,7 @@ private function viewToNorm($value)
$value = $transformers[$i]->reverseTransform($value);
}
} catch (TransformationFailedException $exception) {
throw new TransformationFailedException('Unable to reverse value for property path "'.$this->getPropertyPath().'": '.$exception->getMessage(), $exception->getCode(), $exception);
throw new TransformationFailedException('Unable to reverse value for property path "'.$this->getPropertyPath().'": '.$exception->getMessage(), $exception->getCode(), $exception, $exception->getInvalidMessage(), $exception->getInvalidMessageParameters());
}

return $value;
Expand Down
Expand Up @@ -12,10 +12,18 @@
namespace Symfony\Component\Form\Tests\Extension\Core\Type;

use Symfony\Component\Form\CallbackTransformer;
use Symfony\Component\Form\DataMapperInterface;
use Symfony\Component\Form\Exception\TransformationFailedException;
use Symfony\Component\Form\Extension\Core\Type\CurrencyType;
use Symfony\Component\Form\Extension\Core\Type\FormType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\Extension\Validator\ValidatorExtension;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\Forms;
use Symfony\Component\Form\Tests\Fixtures\Author;
use Symfony\Component\Form\Tests\Fixtures\FixedDataTransformer;
use Symfony\Component\PropertyAccess\PropertyPath;
use Symfony\Component\Validator\Validation;

class FormTest_AuthorWithoutRefSetter
{
Expand Down Expand Up @@ -624,6 +632,32 @@ public function testNormDataIsPassedToView()
$this->assertSame('baz', $view->vars['value']);
}

public function testDataMapperTransformationFailedExceptionInvalidMessageIsUsed()
{
$money = new Money(20.5, 'EUR');
$factory = Forms::createFormFactoryBuilder()
->addExtensions([new ValidatorExtension(Validation::createValidator())])
->getFormFactory()
;

$builder = $factory
->createBuilder(FormType::class, $money, ['invalid_message' => 'not the one to display'])
->add('amount', TextType::class)
->add('currency', CurrencyType::class)
;
$builder->setDataMapper(new MoneyDataMapper());
$form = $builder->getForm();

$form->submit(['amount' => 'invalid_amount', 'currency' => 'USD']);

$this->assertFalse($form->isValid());
$this->assertNull($form->getData());
$this->assertCount(1, $form->getErrors());
$this->assertSame('Expected numeric value', $form->getTransformationFailure()->getMessage());
$error = $form->getErrors()[0];
$this->assertSame('Money amount should be numeric. "invalid_amount" is invalid.', $error->getMessage());
}

// https://github.com/symfony/symfony/issues/6862
public function testPassZeroLabelToView()
{
Expand Down Expand Up @@ -700,3 +734,53 @@ public function testPreferOwnHelpTranslationParameters()
$this->assertEquals(['%parent_param%' => 'parent_value', '%override_param%' => 'child_value'], $view['child']->vars['help_translation_parameters']);
}
}

class Money
{
private $amount;
private $currency;

public function __construct($amount, $currency)
{
$this->amount = $amount;
$this->currency = $currency;
}

public function getAmount()
{
return $this->amount;
}

public function getCurrency()
{
return $this->currency;
}
}

class MoneyDataMapper implements DataMapperInterface
{
public function mapDataToForms($data, $forms)
{
$forms = iterator_to_array($forms);
$forms['amount']->setData($data ? $data->getAmount() : 0);
$forms['currency']->setData($data ? $data->getCurrency() : 'EUR');
}

public function mapFormsToData($forms, &$data)
{
$forms = iterator_to_array($forms);

$amount = $forms['amount']->getData();
if (!is_numeric($amount)) {
$failure = new TransformationFailedException('Expected numeric value');
$failure->setInvalidMessage('Money amount should be numeric. {{ amount }} is invalid.', ['{{ amount }}' => json_encode($amount)]);

throw $failure;
}

$data = new Money(
$forms['amount']->getData(),
$forms['currency']->getData()
);
}
}
Expand Up @@ -343,6 +343,47 @@ function () { throw new TransformationFailedException(); }
->assertRaised();
}

public function testTransformationFailedExceptionInvalidMessageIsUsed()
{
$object = $this->createMock('\stdClass');

$form = $this
->getBuilder('name', '\stdClass', [
'invalid_message' => 'invalid_message_key',
'invalid_message_parameters' => ['{{ foo }}' => 'foo'],
])
->setData($object)
->addViewTransformer(new CallbackTransformer(
function ($data) { return $data; },
function () {
$failure = new TransformationFailedException();
$failure->setInvalidMessage('safe message to be used', ['{{ bar }}' => 'bar']);

throw $failure;
}
))
->getForm()
;

$form->submit('value');

$this->expectNoValidate();

$this->validator->validate($form, new Form());

$this->buildViolation('safe message to be used')
->setParameters([
'{{ value }}' => 'value',
'{{ foo }}' => 'foo',
'{{ bar }}' => 'bar',
])
->setInvalidValue('value')
->setCode(Form::NOT_SYNCHRONIZED_ERROR)
->setCause($form->getTransformationFailure())
->assertRaised()
;
}

// https://github.com/symfony/symfony/issues/4359
public function testDontMarkInvalidIfAnyChildIsNotSynchronized()
{
Expand Down

0 comments on commit 79aeed6

Please sign in to comment.