8000 bug #15251 [DoctrineBridge][Form] Fix IdReader when indexing by prima… · symfony/symfony@d3a1cb1 · GitHub
[go: up one dir, main page]
Skip to content

Commit d3a1cb1

Browse files
fabpotfabpot
committed
bug #15251 [DoctrineBridge][Form] Fix IdReader when indexing by primary foreign key (giosh94mhz)
This PR was merged into the 2.7 branch. Discussion ---------- [DoctrineBridge][Form] Fix IdReader when indexing by primary foreign key | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Port to Symfony 2.7 of PR #14372 . The `IdReader` class can now resolve association and determine the real id value. There is still room for improvement, though. Since I've added a `new IdReader` in the constructor, it is better to declare `IdReader` as `final` just to be safe. PS: sorry to keep you waiting, @webmozart . When merging both commit don't forget to add the `@deprecated` annotation, and that `SingleAssociationToIntIdEntity.php` is duplicated. Commits ------- 799d0e0 [DoctrineBridge][Form] Fix IdReader when indexing by primary foreign key
2 parents e894867 + 799d0e0 commit d3a1cb1

File tree

2 files changed

+134
-1
lines changed

2 files changed

+134
-1
lines changed

‎src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* A utility for reading object IDs.
2020
*
2121
* @since 1.0
22+
*
2223
* @author Bernhard Schussek <bschussek@gmail.com>
2324
*
2425
* @internal This class is meant for internal use only.
@@ -50,6 +51,11 @@ class IdReader
5051
*/
5152
private $idField;
5253

54+
/**
55+
* @var IdReader|null
56+
*/
57+
private $associationIdReader;
58+
5359
public function __construct(ObjectManager $om, ClassMetadata $classMetadata)
5460
{
5561
$ids = $classMetadata->getIdentifierFieldNames();
@@ -60,6 +66,16 @@ public function __construct(ObjectManager $om, ClassMetadata $classMetadata)
6066
$this->singleId = 1 === count($ids);
6167
$this->intId = $this->singleId && in_array($idType, array('integer', 'smallint', 'bigint'));
6268
$this->idField = current($ids);
69+
70+
// single field association are resolved, since the schema column could be an int
71+
if ($this->singleId && $classMetadata->hasAssociation($this->idField)) {
72+
$this->associationIdReader = new self($om, $om->getClassMetadata(
73+
$classMetadata->getAssociationTargetClass($this->idField)
74+
));
75+
76+
$this->singleId = $this->associationIdReader->isSingleId();
77+
$this->intId = $this->associationIdReader->isIntId();
78+
}
6379
}
6480

6581
/**
@@ -108,7 +124,13 @@ public function getIdValue($object)
108124

109125
$this->om->initializeObject($object);
110126

111-
return current($this->classMetadata->getIdentifierValues($object));
127+
$idValue = current($this->classMetadata->getIdentifierValues($object));
128+
129+
if ($this->associationIdReader) {
130+
$idValue = $this->associationIdReader->getIdValue($idValue);
131+
}
132+
133+
return $idValue;
112134
}
113135

114136
/**

‎src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,16 @@
3030
use Symfony\Component\Form\Forms;
3131
use Symfony\Component\Form\Test\TypeTestCase;
3232
use Symfony\Component\PropertyAccess\PropertyAccess;
33+
use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity;
34+
use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity;
3335

3436
class EntityTypeTest extends TypeTestCase
3537
{
3638
const ITEM_GROUP_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\GroupableEntity';
3739
const SINGLE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity';
40+
const SINGLE_IDENT_NO_TO_STRING_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity';
3841
const SINGLE_STRING_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdEntity';
42+
const SINGLE_ASSOC_I 6D47 DENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity';
3943
const COMPOSITE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeIntIdEntity';
4044
const COMPOSITE_STRING_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeStringIdEntity';
4145

@@ -60,7 +64,9 @@ protected function setUp()
6064
$classes = array(
6165
$this->em->getClassMetadata(self::ITEM_GROUP_CLASS),
6266
$this->em->getClassMetadata(self::SINGLE_IDENT_CLASS),
67+
$this->em->getClassMetadata(self::SINGLE_IDENT_NO_TO_STRING_CLASS),
6368
$this->em->getClassMetadata(self::SINGLE_STRING_IDENT_CLASS),
69+
$this->em->getClassMetadata(self::SINGLE_ASSOC_IDENT_CLASS),
6470
$this->em->getClassMetadata(self::COMPOSITE_IDENT_CLASS),
6571
$this->em->getClassMetadata(self::COMPOSITE_STRING_IDENT_CLASS),
6672
);
@@ -304,6 +310,31 @@ public function testSubmitSingleNonExpandedSingleIdentifier()
304310
$this->assertSame('2', $field->getViewData());
305311
}
306312

313+
public function testSubmitSingleNonExpandedSingleAssocIdentifier()
314+
{
315+
$innerEntity1 = new SingleIntIdNoToStringEntity(1, 'InFoo');
316+
$innerEntity2 = new SingleIntIdNoToStringEntity(2, 'InBar');
317+
318+
$entity1 = new SingleAssociationToIntIdEntity($innerEntity1, 'Foo');
319+
$entity2 = new SingleAssociationToIntIdEntity($innerEntity2, 'Bar');
320+
321+
$this->persist(array($innerEntity1, $innerEntity2, $entity1, $entity2));
322+
323+
$field = $this->factory->createNamed('name', 'entity', null, array(
324+
'multiple' => false,
325+
'expanded' => false,
326+
'em' => 'default',
327+
'class' => self::SINGLE_ASSOC_IDENT_CLASS,
328+
'choice_label' => 'name',
329+
));
330+
331+
$field->submit('2');
332+
333+
$this->assertTrue($field->isSynchronized());
334+
$this->assertSame($entity2, $field->getData());
335+
$this->assertSame('2', $field->getViewData());
336+
}
337+
307338
public function testSubmitSingleNonExpandedCompositeIdentifier()
308339
{
309340
$entity1 = new CompositeIntIdEntity(10, 20, 'Foo');
@@ -352,6 +383,35 @@ public function testSubmitMultipleNonExpandedSingleIdentifier()
352383
$this->assertSame(array('1', '3'), $field->getViewData());
353384
}
354385

386+
public function testSubmitMultipleNonExpandedSingleAssocIdentifier()
387+
{
388+
$innerEntity1 = new SingleIntIdNoToStringEntity(1, 'InFoo');
389+
$innerEntity2 = new SingleIntIdNoToStringEntity(2, 'InBar');
390+
$innerEntity3 = new SingleIntIdNoToStringEntity(3, 'InBaz');
391+
392+
$entity1 = new SingleAssociationToIntIdEntity($innerEntity1, 'Foo');
393+
$entity2 = new SingleAssociationToIntIdEntity($innerEntity2, 'Bar');
394+
$entity3 = new SingleAssociationToIntIdEntity($innerEntity3, 'Baz');
395+
396+
$this->persist(array($innerEntity1, $innerEntity2, $innerEntity3, $entity1, $entity2, $entity3));
397+
398+
$field = $this->factory->createNamed('name', 'entity', null, array(
399+
'multiple' => true,
400+
'expanded' => false,
401+
'em' => 'default',
402+
'class' => self::SINGLE_ASSOC_IDENT_CLASS,
403+
'choice_label' => 'name',
404+
));
405+
406+
$field->submit(array('1', '3'));
407+
408+
$expected = new ArrayCollection(array($entity1, $entity3));
409+
410+
$this->assertTrue($field->isSynchronized());
411+
$this->assertEquals($expected, $field->getData());
412+
$this->assertSame(array('1', '3'), $field->getViewData());
413+
}
414+
355415
public function testSubmitMultipleNonExpandedSingleIdentifierForExistingData()
356416
{
357417
$entity1 = new SingleIntIdEntity(1, 'Foo');
@@ -636,6 +696,29 @@ public function testDisallowChoicesThatAreNotIncludedChoicesSingleIdentifier()
636696
$this->assertNull($field->getData());
637697
}
638698

699+
public function testDisallowChoicesThatAreNotIncludedChoicesSingleAssocIdentifier()
700+
{
701+
$innerEntity1 = new SingleIntIdNoToStringEntity(1, 'InFoo');
702+
$innerEntity2 = new SingleIntIdNoToStringEntity(2, 'InBar');
703+
704+
$entity1 = new SingleAssociationToIntIdEntity($innerEntity1, 'Foo');
705+
$entity2 = new SingleAssociationToIntIdEntity($innerEntity2, 'Bar');
706+
707+
$this->persist(array($innerEntity1, $innerEntity2, $entity1, $entity2));
708+
709+
$field = $this->factory->createNamed('name', 'entity', null, array(
710+
'em' => 'default',
711+
'class' => self::SINGLE_ASSOC_IDENT_CLASS,
712+
'choices' => array($entity1, $entity2),
713+
'choice_label' => 'name',
714+
));
715+
716+
$field->submit('3');
717+
718+
$this->assertFalse($field->isSynchronized());
719+
$this->assertNull($field->getData());
720+
}
721+
639722
public function testDisallowChoicesThatAreNotIncludedChoicesCompositeIdentifier()
640723
{
641724
$entity1 = new CompositeIntIdEntity(10, 20, 'Foo');
@@ -681,6 +764,34 @@ public function testDisallowChoicesThatAreNotIncludedQueryBuilderSingleIdentifie
681764
$this->assertNull($field->getData());
682765
}
683766

767+
public function testDisallowChoicesThatAreNotIncludedQueryBuilderSingleAssocIdentifier()
768+
{
769+
$innerEntity1 = new SingleIntIdNoToStringEntity(1, 'InFoo');
770+
$innerEntity2 = new SingleIntIdNoToStringEntity(2, 'InBar');
771+
$innerEntity3 = new SingleIntIdNoToStringEntity(3, 'InBaz');
772+
773+
$entity1 = new SingleAssociationToIntIdEntity($innerEntity1, 'Foo');
774+
$entity2 = new SingleAssociationToIntIdEntity($innerEntity2, 'Bar');
775+
$entity3 = new SingleAssociationToIntIdEntity($innerEntity3, 'Baz');
776+
777+
$this->persist(array($innerEntity1, $innerEntity2, $innerEntity3, $entity1, $entity2, $entity3));
778+
779+
$repository = $this->em->getRepository(self::SINGLE_ASSOC_IDENT_CLASS);
780+
781+
$field = $this->factory->createNamed('name', 'entity', null, array(
782+
'em' => 'default',
783+
'class' => self::SINGLE_ASSOC_IDENT_CLASS,
784+
'query_builder' => $repository->createQueryBuilder('e')
785+
->where('e.entity IN (1, 2)'),
786+
'choice_label' => 'name',
787+
));
788+
789+
$field->submit('3');
790+
791+
$this->assertFalse($field->isSynchronized());
792+
$this->assertNull($field->getData());
793+
}
794+
684795
public function testDisallowChoicesThatAreNotIncludedQueryBuilderAsClosureSingleIdentifier()
685796
{
686797
$entity1 = new SingleIntIdEntity(1, 'Foo');

0 commit comments

Comments
 (0)
0