-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[2.2] [Form] Added form type "entity_identifier" (#1946) #1951
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.
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
Changes from 1 commit
05a064a
662691a
4a62fc3
6f11075
0802d83
295bd6d
38713e2
fc400f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Bridge\Doctrine\Form\DataTransformer; | ||
|
||
use Symfony\Component\Form\FormInterface; | ||
use Symfony\Component\Form\DataTransformerInterface; | ||
use Symfony\Component\Form\FormError; | ||
use Symfony\Component\Form\Exception\TransformationFailedException; | ||
use Symfony\Component\Form\Exception\UnexpectedTypeException; | ||
|
||
use Doctrine\ORM\EntityManager; | ||
use Doctrine\ORM\NoResultException; | ||
|
||
class OneEntityToIdTransformer implements DataTransformerInterface | ||
{ | ||
private $em; | ||
private $class; | ||
private $queryBuilder; | ||
|
||
public function __construct(EntityManager $em, $class, $queryBuilder) | ||
{ | ||
if (!(null === $queryBuilder || $queryBuilder instanceof \Closure)) { | ||
throw new UnexpectedTypeException($queryBuilder, 'Doctrine\ORM\QueryBuilder or \Closure'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is wrong. The check forbids giving a QueryBuilder instance so the error message should not say it is allowed |
||
} | ||
|
||
if (null == $class) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should use |
||
throw new UnexpectedTypeException($class, 'string'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need to add curly braces |
||
|
||
$this->em = $em; | ||
$this->class = $class; | ||
$this->queryBuilder = $queryBuilder; | ||
} | ||
|
||
/** | ||
* Fetch the id of the entity to populate the form | ||
*/ | ||
public function transform($data) | ||
{ | ||
if (null === $data) | ||
return null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need to add curly braces here too |
||
|
||
$meta = $this->em->getClassMetadata($this->class); | ||
|
||
if (!$meta->getReflectionClass()->isInstance($data)) | ||
throw new TransformationFailedException('Invalid data, must be an instance of '.$this->class); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need to add the curly braces here. |
||
|
||
$identifierField = $meta->getSingleIdentifierFieldName(); | ||
$id = $meta->getReflectionProperty($identifierField)->getValue($data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using Reflection here is actually a problem because we lose the ability of the Proxy to load the missing properties of the entity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I use "Form\Util\PropertyPath" to get the id value then ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I sent you a PR on your Bundle which is inspired of the original EntityToIdTransformer. |
||
|
||
return $id; | ||
} | ||
|
||
/** | ||
* Try to fetch the entity from its id in the database | ||
*/ | ||
public function reverseTransform($data) | ||
{ | ||
if (!$data) { | ||
return null; | ||
} | ||
|
||
$em = $this->em; | ||
$class = $this->class; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why doing this ? |
||
$repository = $em->getRepository($class); | ||
|
||
if ($qb = $this->queryBuilder) { | ||
// If a closure was passed, call id with the repository and the id | ||
if ($qb instanceof \Closure) { | ||
$qb = $qb($repository, $data); | ||
} | ||
|
||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use getOneOrNullResult? |
||
$result = $qb->getQuery()->getSingleResult(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this does not make sense in the case where a QueryBuilder is passed directly as it does not receive the data at all There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah sorry, looking at the constructor, the |
||
} catch (NoResultException $e) { | ||
$result = null; | ||
} | ||
} else { | ||
// Defaults to find() | ||
$result = $repository->find($data); | ||
} | ||
|
||
if (!$result) | ||
throw new TransformationFailedException('Can not find entity'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. curly braces are missing here too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TransformationFailedException are turned into errors by the Form class. This is the way to add errors in transformers (see the doc of the interface) |
||
|
||
return $result; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Bridge\Doctrine\Form\Type; | ||
|
||
use Symfony\Component\Form\FormBuilder; | ||
use Symfony\Bridge\Doctrine\RegistryInterface; | ||
use Symfony\Bridge\Doctrine\Form\DataTransformer\OneEntityToIdTransformer; | ||
use Symfony\Component\Form\AbstractType; | ||
|
||
class EntityIdType extends AbstractType | ||
{ | ||
protected $em; | ||
|
||
public function __construct(RegistryInterface $registry) | ||
{ | ||
$this->em = $registry->getEntityManager(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is wrong. You should keep the reference to the registry instead and then retrieve the good entity manager according to the name passed as option, like in the EntityType. |
||
} | ||
|
||
public function buildForm(FormBuilder $builder, array $options) | ||
{ | ||
$em = $options['em'] ?: $this->em; | ||
|
||
$builder->prependClientTransformer(new OneEntityToIdTransformer($em, $options['class'], $options['query_builder'])); | ||
} | ||
|
||
public function getDefaultOptions(array $options) | ||
{ | ||
$defaultOptions = array( | ||
'required' => true, | ||
'em' => null, | ||
'class' => null, | ||
'query_builder' => null, | ||
'hidden' => true | ||
); | ||
|
||
$options = array_replace($defaultOptions, $options); | ||
|
||
return $defaultOptions; | ||
} | ||
|
||
public function getParent(array $options) | ||
{ | ||
return $options['hidden'] ? 'hidden' : 'field'; | ||
} | ||
|
||
public function getName() | ||
{ | ||
return 'entity_id'; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should write it
if (null !== $queryBuilder && ! $queryBuilder instanceof \Closure)
instead.Also, what about allowing any callable instead of forcing to use a closure ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is almost the same as in "../ChoiceList/EntityChoiceList.php" which is written so and just accepts closure, I must confess I don't know what it would not be any callable