[Form] Add flexibility for EntityType

[raziel057]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR

Sometimes it can be usefull to return null rather than a QueryBuilder from the closure attached to the query_builder attribute of an EntityType.

For example, if I have the following method in a "DelegateRepository", which can return a QueryBuilder or null:

public function getQbForOfficers()
{
    $permanentMeeting = $this->getEntityManager()->getRepository('MyBundle:PermanentMeeting')->findOneBy(array());

    if ($permanentMeeting === null || $permanentMeeting->getMeeting() === null) {
        return null;
    }

    $qb = $this->getEntityManager()->createQueryBuilder();
    $qb->select('p')
        ->from('MyBundle:Delegate', 'p')
        ->andWhere('p.meeting = :meetingId')
        ->setParameter('meetingId', $permanentMeeting->getMeeting()->getId());

    return $qb;
}

To be able to present a list without entries when creating an entity field like this:

$event->getForm()->add('officers', 'entity', array(
    'class' => 'MyBundle:Delegate',
    'property' => 'fullName',
    'query_builder' => function(EntityRepository $er) use ($meeting, $delegationId) {
        return $er->getQbForOfficers();
    }
)));

Rather than using the "choices" attributes (more verbose and which requires the injection of the entityManager):

$qb = $this->entityManager->getRepository('PTCNoventoBundle:Delegate')->getQbForOfficers();

$officers = ($qb !== null) ? $qb->getQuery()->getResult() : array();

$event->getForm()->add('officers', 'entity', array(
    'class' => 'PTCNoventoBundle:Delegate',
    'property' => 'fullName',
    'choices' => $officers,
));

[fabpot]

This one is not needed AFAIR.

[raziel057]

If I remove this condition, I will get the UnexpectedTypeException if the Closure returns null. So we need to let null as supported value here.

[fabpot]

👍

[raziel057]

Is it ok for merge? I added a unit test to cover this case.

[Tobion]

phpdoc for ORMQueryBuilderLoader::queryBuilder must be changed to @var QueryBuilder|null

[webmozart]

What do you think about this @beberlei? Should we support null or should we return a query builder that returns no entities instead?

[fabpot]

ping @symfony/deciders @beberlei Any decisions on this one?

[xabbuh]

👍 looks like a simple but effective solution

[fabpot]

Thank you @raziel057.

[HeahDude]

The implementation of this feature was reverted by eb08baa, and was never meant to work because when the query_builder option is set to null (which is the default), whether or not returned by a callable, all entities are loaded.

The only feature it adds is that null can be set conditionally via the callable.

The test is wrong as no entities are persisted, it should be:

public function testConfigureQueryBuilderWithClosureReturningNull()
{
    $entity1 = new SingleIntIdEntity(1, 'Foo');
    $entity2 = new SingleIntIdEntity(2, 'Bar');

    $this->persist(array($entity1, $entity2));

    $field = $this->factory->createNamed('name', 'Symfony\Bridge\Doctrine\Form\Type\EntityType', null, array(
        'em' => 'default',
        'class' => self::SINGLE_IDENT_CLASS,
        'query_builder' => function () {
            return;
        },
    ));

    $this->assertSame(array(), $field->createView()->vars['choices']);
}

Then it fails.

The only way I see to support such use case is to create a custom type that handle its own configuration but the easiest way would be to set choices option to an empty array.