Skip to content

[Form] Errors now reference the field they were added to and the violation/exception that caused them #9993

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

Merged
merged 1 commit into from
Jan 10, 2014
Merged

[Form] Errors now reference the field they were added to and the violation/exception that caused them #9993

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
29 changes: 24 additions & 5 deletions src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/form.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@
</div>

{% for formName, formData in collector.data.forms %}
{{ form_tree_details(formName, formData) }}
{{ form_tree_details(formName, formData, collector.data.forms_by_hash) }}
{% endfor %}
</div>
{% else %}
Expand Down Expand Up @@ -366,7 +366,7 @@
</li>
{% endmacro %}

{% macro form_tree_details(name, data) %}
{% macro form_tree_details(name, data, forms_by_hash) %}
<div class="tree-details" id="{{ data.id }}-details">
<h2>
{{ name }}
Expand All @@ -386,13 +386,32 @@

<table id="{{ data.id }}-errors">
<tr>
<th width="50%">Message</th>
<th>Message</th>
<th>Origin</th>
<th>Cause</th>
</tr>
{% for error in data.errors %}
<tr>
<td>{{ error.message }}</td>
<td><em>Unknown.</em></td>
<td>
{% if error.origin is empty %}
<em>This form.</em>
{% elseif forms_by_hash[error.origin] is not defined %}
<em>Unknown.</em>
{% else %}
{{ forms_by_hash[error.origin].name }}
{% endif %}
</td>
<td>
{% if error.cause is empty %}
<em>Unknown.</em>
{% elseif error.cause.root is defined %}
<strong>Constraint Violation</strong><br/>
<pre>{{ error.cause.root }}{% if error.cause.path is not empty %}{% if error.cause.path|first != '[' %}.{% endif %}{{ error.cause.path }}{% endif %} = {{ error.cause.value }}</pre>
{% else %}
<pre>{{ error.cause }}</pre>
{% endif %}
</td>
</tr>
{% endfor %}
</table>
Expand Down Expand Up @@ -565,6 +584,6 @@
</div>

{% for childName, childData in data.children %}
{{ _self.form_tree_details(childName, childData) }}
{{ _self.form_tree_details(childName, childData, forms_by_hash) }}
{% endfor %}
{% endmacro %}
2 changes: 2 additions & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ CHANGELOG
------

* added an option for multiple files upload
* form errors now reference their cause (constraint violation, exception, ...)
* form errors now remember which form they were originally added to

2.4.0
-----
Expand Down
17 changes: 11 additions & 6 deletions src/Symfony/Component/Form/Extension/DataCollector/FormDataCollector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public function __construct(FormDataExtractorInterface $dataExtractor)
$this->dataExtractor = $dataExtractor;
$this->data = array(
'forms' => array(),
'forms_by_hash' => array(),
'nb_errors' => 0,
);
}
Expand Down Expand Up @@ -184,7 +185,7 @@ public function buildPreliminaryFormTree(FormInterface $form)
{
$this->data['forms'][$form->getName()] = array();

$this->recursiveBuildPreliminaryFormTree($form, $this->data['forms'][$form->getName()]);
$this->recursiveBuildPreliminaryFormTree($form, $this->data['forms'][$form->getName()], $this->data['forms_by_hash']);
}

/**
Expand All @@ -194,7 +195,7 @@ public function buildFinalFormTree(FormInterface $form, FormView $view)
{
$this->data['forms'][$form->getName()] = array();

$this->recursiveBuildFinalFormTree($form, $view, $this->data['forms'][$form->getName()]);
$this->recursiveBuildFinalFormTree($form, $view, $this->data['forms'][$form->getName()], $this->data['forms_by_hash']);
}

/**
Expand All @@ -213,24 +214,26 @@ public function getData()
return $this->data;
}

private function recursiveBuildPreliminaryFormTree(FormInterface $form, &$output = null)
private function recursiveBuildPreliminaryFormTree(FormInterface $form, &$output = null, array &$outputByHash)
{
$hash = spl_object_hash($form);

$output = isset($this->dataByForm[$hash])
? $this->dataByForm[$hash]
: array();

$outputByHash[$hash] = &$output;

$output['children'] = array();

foreach ($form as $name => $child) {
$output['children'][$name] = array();

$this->recursiveBuildPreliminaryFormTree($child, $output['children'][$name]);
$this->recursiveBuildPreliminaryFormTree($child, $output['children'][$name], $outputByHash);
}
}

private function recursiveBuildFinalFormTree(FormInterface $form = null, FormView $view, &$output = null)
private function recursiveBuildFinalFormTree(FormInterface $form = null, FormView $view, &$output = null, array &$outputByHash)
{
$viewHash = spl_object_hash($view);
$formHash = null;
Expand All @@ -255,6 +258,8 @@ private function recursiveBuildFinalFormTree(FormInterface $form = null, FormVie
? $this->dataByForm[$formHash]
: array()
);

$outputByHash[$formHash] = &$output;
}

$output['children'] = array();
Expand All @@ -268,7 +273,7 @@ private function recursiveBuildFinalFormTree(FormInterface $form = null, FormVie

$output['children'][$name] = array();

$this->recursiveBuildFinalFormTree($childForm, $childView, $output['children'][$name]);
$this->recursiveBuildFinalFormTree($childForm, $childView, $output['children'][$name], $outputByHash);
}
}
}
29 changes: 26 additions & 3 deletions src/Symfony/Component/Form/Extension/DataCollector/FormDataExtractor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormView;
use Symfony\Component\HttpKernel\DataCollector\Util\ValueExporter;
use Symfony\Component\Validator\ConstraintViolationInterface;

/**
* Default implementation of {@link FormDataExtractorInterface}.
Expand Down Expand Up @@ -43,6 +44,7 @@ public function extractConfiguration(FormInterface $form)
{
$data = array(
'id' => $this->buildId($form),
'name' => $form->getName(),
'type' => $form->getConfig()->getType()->getName(),
'type_class' => get_class($form->getConfig()->getType()->getInnerType()),
'synchronized' => $this->valueExporter->exportValue($form->isSynchronized()),
Expand Down Expand Up @@ -108,9 +110,26 @@ public function extractSubmittedData(FormInterface $form)
}

foreach ($form->getErrors() as $error) {
$data['errors'][] = array(
$errorData = array(
'message' => $error->getMessage(),
'origin' => is_object($error->getOrigin())
? spl_object_hash($error->getOrigin())
: null,
);

$cause = $error->getCause();

if ($cause instanceof ConstraintViolationInterface) {
$errorData['cause'] = array(
'root' => $this->valueExporter->exportValue($cause->getRoot()),
'path' => $this->valueExporter->exportValue($cause->getPropertyPath()),
'value' => $this->valueExporter->exportValue($cause->getInvalidValue()),
);
} else {
$errorData['cause'] = null !== $cause ? $this->valueExporter->exportValue($cause) : null;
}

$data['errors'][] = $errorData;
}

$data['synchronized'] = $this->valueExporter->exportValue($form->isSynchronized());
Expand All @@ -127,8 +146,12 @@ public function extractViewVariables(FormView $view)

// Set the ID in case no FormInterface object was collected for this
// view
if (isset($view->vars['id'])) {
$data['id'] = $view->vars['id'];
if (!isset($data['id'])) {
$data['id'] = isset($view->vars['id']) ? $view->vars['id'] : null;
}

if (!isset($data['name'])) {
$data['name'] = isset($view->vars['name']) ? $view->vars['name'] : null;
}

foreach ($view->vars as $varName => $value) {
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
$violation->getMessage(),
$violation->getMessageTemplate(),
$violation->getMessageParameters(),
$violation->getMessagePluralization()
$violation->getMessagePluralization(),
$violation
));
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/Form/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,10 @@ public function bind($submittedData)
public function addError(FormError $error)
{
if ($this->parent && $this->config->getErrorBubbling()) {
if (null === $error->getOrigin()) {
$error->setOrigin($this);
}

$this->parent->addError($error);
} else {
$this->errors[] = $error;
Expand Down
86 changes: 83 additions & 3 deletions src/Symfony/Component/Form/FormError.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@

namespace Symfony\Component\Form;

use Symfony\Component\Form\Exception\BadMethodCallException;

/**
* Wraps errors in forms
*
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class FormError
class FormError implements \Serializable
{
/**
* @var string
Expand All @@ -41,6 +43,18 @@ class FormError
*/
protected $messagePluralization;

/**
* The cause for this error
* @var mixed
*/
private $cause;

/**
* The form that spawned this error
* @var FormInterface
*/
private $origin;

/**
* Constructor
*
Expand All @@ -50,17 +64,19 @@ class FormError
* @param string $message The translated error message
* @param string|null $messageTemplate The template for the error message
* @param array $messageParameters The parameters that should be
* substituted in the message template.
* substituted in the message template
* @param integer|null $messagePluralization The value for error message pluralization
* @param mixed $cause The cause of the error
*
* @see \Symfony\Component\Translation\Translator
*/
public function __construct($message, $messageTemplate = null, array $messageParameters = array(), $messagePluralization = null)
public function __construct($message, $messageTemplate = null, array $messageParameters = array(), $messagePluralization = null, $cause = null)
{
$this->message = $message;
$this->messageTemplate = $messageTemplate ?: $message;
$this->messageParameters = $messageParameters;
$this->messagePluralization = $messagePluralization;
$this->cause = $cause;
}

/**
Expand Down Expand Up @@ -102,4 +118,68 @@ public function getMessagePluralization()
{
return $this->messagePluralization;
}

/**
* Returns the cause of this error.
*
* @return mixed The cause of this error
*/
public function getCause()
{
return $this->cause;
}

/**
* Sets the form that caused this error.
*
* This method must only be called once.
*
* @param FormInterface $origin The form that caused this error
*
* @throws BadMethodCallException If the method is called more than once
*/
public function setOrigin(FormInterface $origin)
{
if (null !== $this->origin) {
throw new BadMethodCallException('setOrigin() must only be called once.');
}

$this->origin = $origin;
}

/**
* Returns the form that caused this error.
*
* @return FormInterface The form that caused this error
*/
public function getOrigin()
{
return $this->origin;
}

/**
* Serializes this error.
*
* @return string The serialized error
*/
public function serialize()
{
return serialize(array(
$this->message,
$this->messageTemplate,
$this->messageParameters,
$this->messagePluralization,
$this->cause
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we enforce that the cause of the error is serializable as well ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think so. The cause is important for historical analysis.

));
}

/**
* Unserializes a serialized error.
*
* @param string $serialized The serialized error
*/
public function unserialize($serialized)
{
list($this->message, $this->messageTemplate, $this->messageParameters, $this->messagePluralization, $this->cause) = unserialize($serialized);
}
}
Loading