BC break for collection type's prototype_data feature #15707
Comments
|
@backbone87 Could you provide a failing test case? |
|
@backbone87 ping |
|
Sorry, i am really busy atm. public function testLegacyPrototypeData() {
$form = $this->factory->create('collection', array(), array(
'type' => 'text',
'allow_add' => true,
'prototype' => true,
//'prototype_data' => 'foo', // in a legacy form this is not present
'options' => array(
'data' => 'bar',
),
));
// this will fail, but is expected legacy behavior
$this->assertSame('bar', $form->createView()->vars['prototype']->vars['value']);
}I didnt actually run this test case, but it should be pretty obvious that it fails. |
|
Found same BC break in our project too. After update to 2.8 value attributes just disappeared in the prototype. Anyone can suggest a fast workaround for this without downgrading? I can work on a fix and PR but not too soon. |
|
@backbone87 @memphys here's a possible fix : // Symfony/Component/Form/Extension/Core/CollectionType.php
public function buildForm(FormBuilderInterface $builder, array $options)
{
if ($options['allow_add'] && $options['prototype']) {
$prototype = $builder->create($options['prototype_name'], $options['entry_type'], array_replace(array(
'label' => $options['prototype_name'].'label__',
), $options['entry_options'], ($options['prototype_data'] ? array(
'data' => $options['prototype_data'],
) : array())));
$builder->setAttribute('prototype', $prototype->getForm());
}
... |
|
@HeahDude thanks, but I already found PR with the original introduction of |
|
@xabbuh @jakzal @backbone87 made a PR but have some problems with the test. Tested the code in our project and it fixes the BC break. If you can help with some directions on the test, I'll try to finish it. Thanks. |
|
Made new PR to correct branch. Sorry for inconvenience. |
|
@memphys you should replace |
|
I suggest this implementation with a comment for depreciation : // Symfony/Component/Form/Extension/Core/CollectionType.php
public function buildForm(FormBuilderInterface $builder, array $options)
{
if ($options['allow_add'] && $options['prototype']) {
$prototype = $builder->create($options['prototype_name'], $options['entry_type'], array_replace(array(
'label' => $options['prototype_name'].'label__',
// check "$options['prototype_data'] for BC. Condition to be removed in 3.0.
), $options['entry_options'], ($options['prototype_data'] ? array(
'data' => $options['prototype_data'],
) : array())));
$builder->setAttribute('prototype', $prototype->getForm());
}
... |
|
@HeahDude so in 3.0 this BC is intended? I mean if no prototype_data is set, then it won't be filled from form data? |
|
@memphys I just suppose it does, considering |
|
@HeahDude ok, understood, thanks. But it sounds strange for me) |
…(memphys) This PR was squashed before being merged into the 2.8 branch (closes #17044). Discussion ---------- [Form] fix BC break introduced with prototype_data option | Q | A | ------------- | --- | Bug fix? | [yes] | New feature? | [no] | BC breaks? | [no] | Deprecations? | [no] | Tests pass? | [no] | Fixed tickets | [#15707] | License | MIT | Doc PR | [] This fixes the BC break introduced with prototype_data option in collection type. At the moment whether option is set or not it overwrites prototype data but it has different behaviour before and prototype data was taken from the mapped form data/entity. - [x] make the test work (can't figure yet how to test that prototype without prototype_data option has default values) Commits ------- d73485a [Form] fix BC break introduced with prototype_data option
related PR: #12314
related commit: 54bd6f7
the way its currently implemented in this commit is a BC break since the prototype_data will always be used, even if legacy code not using prototype_data relies on the old behavior where the data option is used for the prototype form. i recommend revisiting this implementation and using code similar to the one i showed in my comment in the PR #12314 (comment)
The text was updated successfully, but these errors were encountered: