Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[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

Closed
wants to merge 8 commits into from
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
<?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 Symfony\Component\Form\Exception\FormException;
use Symfony\Component\Form\Util\PropertyPath;

use Doctrine\ORM\EntityManager;
use Doctrine\ORM\NoResultException;

class OneEntityToIdTransformer implements DataTransformerInterface
{
private $em;
private $class;
private $property;
private $queryBuilder;

private $unitOfWork;

public function __construct(EntityManager $em, $class, $property, $queryBuilder)
{
if (null !== $queryBuilder && ! $queryBuilder instanceof \Closure) {
throw new UnexpectedTypeException($queryBuilder, '\Closure');
}

if (null === $class) {
throw new UnexpectedTypeException($class, 'string');
Copy link
Member

Choose a reason for hiding this comment

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

you need to add curly braces

}

$this->em = $em;
$this->unitOfWork = $em->getUnitOfWork();
$this->class = $class;
$this->queryBuilder = $queryBuilder;

if ($property) {
$this->property = $property;
}
}

/**
* Fetch the id of the entity to populate the form
*/
public function transform($data)
{
if (null === $data) {
return null;
Copy link
Member

Choose a reason for hiding this comment

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

you need to add curly braces here too

}
if (!$this->unitOfWork->isInIdentityMap($data)) {
throw new FormException('Entities passed to the choice field must be managed');
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the entity have to be managed if a property is provided? In my case, I'm providing an unserialized entity from the session that's not managed by the entity manager. It has an ID, so there's no reason for it not to work.

Can we move this check after if ($this->property) {}?

}

if ($this->property) {
$propertyPath = new PropertyPath($this->property);
return $propertyPath->getValue($data);
}

return current($this->unitOfWork->getEntityIdentifier($data));
Copy link
Member

Choose a reason for hiding this comment

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

this is wrong IMO as you are loosing part of the data in case of a composite key without saying anything to the user

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would you handle that ?
Throwing an exception if the array contains is bigger that 1 element ?

}

/**
* Try to fetch the entity from its id in the database
*/
public function reverseTransform($data)
{
if (!$data) {
return null;
}

$em = $this->em;
$repository = $em->getRepository($this->class);

if ($qb = $this->queryBuilder) {
// Call the closure with the repository and the id
$qb = $qb($repository, $data);

try {

Choose a reason for hiding this comment

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

Why not use getOneOrNullResult?

$result = $qb->getQuery()->getSingleResult();
Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

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

ah sorry, looking at the constructor, the queryBuilder property is always a closure (the naming is bad then) which means that the previous check is useless and that a check is missing to be sure you receive a query builder

} catch (NoResultException $e) {
$result = null;
}
} else {
// Defaults to find()
if ($this->property) {
$result = $repository->findOneBy(array($this->property => $data));
} else {
$result = $repository->find($data);
}
}

if (!$result) {
throw new TransformationFailedException('Can not find entity');
Copy link
Member

Choose a reason for hiding this comment

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

curly braces are missing here too

Copy link
Member

Choose a reason for hiding this comment

The 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;
}
}

68 changes: 68 additions & 0 deletions 68 src/Symfony/Bridge/Doctrine/Form/Type/EntityIdentifierType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?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;
use Symfony\Component\Form\Exception\FormException;

class EntityIdentifierType extends AbstractType
{
protected $registry;

public function __construct(RegistryInterface $registry)
{
$this->registry = $registry;
}

public function buildForm(FormBuilder $builder, array $options)
{
$builder->prependClientTransformer(new OneEntityToIdTransformer(
$this->registry->getEntityManager($options['em']),
$options['class'],
$options['property'],
$options['query_builder']
));
}

public function getDefaultOptions(array $options)
Copy link
Contributor

Choose a reason for hiding this comment

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

The method is deprecated.

Choose a reason for hiding this comment

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

You should have a look to https://github.com/Gregwar/FormBundle/blob/master/Type/EntityIdType.php#L40
The component is up to date there ! :)

{
$defaultOptions = array(
'required' => true,
'em' => null,
'class' => null,
'query_builder' => null,
'property' => null,
'hidden' => true
);

$options = array_replace($defaultOptions, $options);

if (null === $options['class']) {
throw new FormException('You must provide a class option for the entity_identifier field');
}

return $defaultOptions;
}

public function getParent(array $options)
Copy link
Contributor

Choose a reason for hiding this comment

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

the method is not compatible with AbstractType :)

{
return $options['hidden'] ? 'hidden' : 'field';
}

public function getName()
{
return 'entity_identifier';
}
}
5 changes: 5 additions & 0 deletions 5 src/Symfony/Bundle/DoctrineBundle/Resources/config/orm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@
<argument type="service" id="doctrine" />
</service>

<service id="form.type.entity_identifier" class="Symfony\Bridge\Doctrine\Form\Type\EntityIdentifierType">
<tag name="form.type" alias="entity_identifier" />
<argument type="service" id="doctrine" />
</service>

<service id="doctrine.orm.configuration" class="%doctrine.orm.configuration.class%" abstract="true" public="false" />

<service id="doctrine.orm.entity_manager.abstract" class="%doctrine.orm.entity_manager.class%" factory-class="%doctrine.orm.entity_manager.class%" factory-method="create" abstract="true" />
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.