-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] collection type is broken #3539
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
Comments
Hmmm, do you have some prevention of duplicates in your entity ? I.e.: <?php
class User {
public function addGroup($group) {
if (!$this->groups->contains($group)) {
$this->groups[] = $group;
}
}
} |
No, I don't. But at least for now I could implement this as a workaround. |
Along with the code above you need another loop before persisting and calling whatever code your add* method calls like setUser($group) |
@mvrhov why another loop ? if your collection is the inversed side, simply add the needed code to update the owning side in your adder instead of just adding it in the collection. and btw, for the inversed side, duplicates are not really an issue as they won't be persisted by Doctrine (it is only an issue if you display your object directly but you generally redirect after handling a form anyway). Note that I'm talking about symfony 2.1 here. the 2.0 code does not support inversed side of relations properly. |
I'm also talking about master or 2.1 here. And as I said I have to do some post processing on the collection and later on some data from that collection and post processing is put into the flash message. |
well, then keep the contains check in the inversed side as well. But this does not forbid you to update the owning side |
Can you check please whether this is fixed in #3819? Or has this issue resolved itself? |
I can't verify before Tuesday/Wednesday. |
I've updated to latest master and this still doesn't work. as I said the problem is that by the time the add* gets called the details are already in the collection but those details are not added via add but somehow bound inside form code. |
@mvrhov Can you provide more information in order to reproduce this issue? Even better, a test case? |
Ah.. PR#3789 was merged today and I have trouble adapting the testcase. use Symfony\Component\Form\Extension\Core\CoreExtension;
use Symfony\Component\Form\FormFactory;
use Symfony\Component\Form\AbstractType;
use Doctrine\Common\Collections\ArrayCollection;
class MergeCollectionListenerTest_CarAxes
{
private $axes;
public function addAxis(MergeCollectionListenerTest_CarAxis $axis)
{
//we are not checking if it's already in collection on purpose
//if (!$this->getAxes()->contains($axis)) {
$this->getAxes()->add($axis);
$axis->setCar($this);
//}
}
public function removeAxis(MergeCollectionListenerTest_CarAxis $axis)
{
$this->getAxes()->removeElement($axis);
}
public function getAxes()
{
return $this->axes ?: $this->axes = new ArrayCollection();
}
}
class MergeCollectionListenerTest_CarAxis
{
private $car;
public $prop1;
public $prop2;
public function setCar($car)
{
$this->car = $car;
}
public function getCar()
{
return $this->car;
}
}
class MergeCollectionListenerTest_CarAxisType extends AbstractType
{
public function buildForm(FormBuilder $builder, array $options)
{
$builder
->add('prop1', 'text')
->add('prop2', 'text')
;
}
public function getName()
{
return 'axis';
}
public function getDefaultOptions(array $options)
{
return array(
'data_class' => __NAMESPACE__ .'\\MergeCollectionListenerTest_CarAxis',
);
}
}
public function testDoubles()
{
$ff = new FormFactory(array(new CoreExtension()));
$axes = new MergeCollectionListenerTest_CarAxes();
$builder = $ff->createBuilder('form', $axes);
$builder
->add('axes', 'collection', array(
'type' => new MergeCollectionListenerTest_CarAxisType(),
'allow_add' => true,
'options' => array( //FIXME we need the line below because of bug https://github.com/symfony/symfony/issues/3354
'data_class' => 'Symfony\Component\Form\Tests\Extension\Core\EventListener\MergeCollectionListenerTest_CarAxis',
)
)
)
;
$builder
->getForm()
->bind(
array('axes' => array(
array('prop1' => '01', 'prop2' => '02'),
array('prop1' => '11', 'prop2' => '12'),
)
)
);
$newAxis = function($prop1, $prop2) {
$ret = new MergeCollectionListenerTest_CarAxis();
$ret->prop1 = $prop1;
$ret->prop2 = $prop2;
return $ret;
};
$cmpAxes = new MergeCollectionListenerTest_CarAxes();
$cmpAxes->addAxis($newAxis('01', '02'));
$cmpAxes->addAxis($newAxis('11', '12'));
$this->assertEquals($cmpAxes->getAxes()->count(), $axes->getAxes()->count());
$this->assertEquals($cmpAxes, $axes);
} |
I think I've adapted the testcase successfully, but the first assert doesn't fail because after the PR #3789 got merged the calculated diff is empty. Failing testcase is https://github.com/mvrhov/symfony/tree/3539_testcase |
You forgot to set "by_reference" in the collection form to false. See #3839 and symfony/symfony-docs#1057. |
…alling stop() (jochenvdv) This PR was merged into the 2.5-dev branch. Discussion ---------- [Stopwatch] Allow getting duration of events without calling stop() | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | [#10175](#10175) | License | MIT | Doc PR | [#3539](symfony/symfony-docs#3539) Commits ------- 2efe461 Allow retrieving unstopped stopwatch events d3d097d Include running periods in duration
This one is a bit hard to explain, but I'll try my best.
PR#3239 brought proper handling of collections, but it doesn't work as expected. I've traced this only for blank collection, but this is probably the same or similar when you go and edit existing one.
My collection object is having duplicates. And if I take a look at the memory addresses of those duplicates with the debugger, I can see that the same objects are added twice as they reside on the same address.
It seems, that the data is bind to the collection twice, first in the form's bind() method and then once again in MergeCollectionListener.
Looking at the $originalData when onBindNormData is called in
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Core/EventListener/MergeCollectionListener.php#L142
I can see, that $originalData already contains the whole collection that came in through the request.
Later down the code calls add* method and adds the same objects.
Now the problem is not visible if all you do with the data is just call $em->persist($form->getData()
as doctrine will silently handle this, but if you need to traverse the collection and do some calculations
on it before persisting, then you have the problem as you'll do the whatever you want to do twice.
The text was updated successfully, but these errors were encountered: