Skip to content

[Form] fix BC break introduced with prototype_data option #17044

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 4 commits into from
Closed

[Form] fix BC break introduced with prototype_data option #17044

wants to merge 4 commits into from

Conversation

memphys
Copy link
Contributor

@memphys memphys commented Dec 17, 2015

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.

  • make the test work (can't figure yet how to test that prototype without prototype_data option has default values)

$form = $this->factory->create('Symfony\Component\Form\Extension\Core\Type\CollectionType', array(), array(
'allow_add' => true,
'prototype' => true,
'entry_type' => 'Symfony\Component\Form\Extension\Core\Type\TextType',
Copy link
Contributor

Choose a reason for hiding this comment

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

should be type

@memphys
Copy link
Contributor Author

memphys commented Dec 17, 2015

@HeahDude thanks! Made new commit, now test is passing.

@memphys memphys changed the title [WIP] [Form] fix BC break introduced with prototype_data option [Form] fix BC break introduced with prototype_data option Dec 18, 2015
)));
), $options['options']);

if ($options['prototype_data'] !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Symfony CS prefers to use Yoda conditions: if (null !== $options['prototype_data']) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sstok thanks! Fixed

@nicolas-grekas
Copy link
Member

👍

@fabpot
Copy link
Member

fabpot commented Dec 18, 2015

Thank you @memphys.

fabpot added a commit that referenced this pull request Dec 18, 2015
…(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
@fabpot fabpot closed this Dec 18, 2015
@memphys
Copy link
Contributor Author

memphys commented Dec 18, 2015

@fabpot with pleasure!

@memphys memphys deleted the 15707-fix-prototype-data-bc-break branch December 21, 2015 16:19
This was referenced Dec 26, 2015
nicolas-grekas added a commit that referenced this pull request Dec 29, 2015
This PR was merged into the 2.8 branch.

Discussion
----------

[Form] Fix regression on Collection type

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

A regression was introduced in #17044.

Commits
-------

bd686cd [Form] Fixed regression on Collection type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants