Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Form] Fix choices defined as Traversable#16796
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedDec 2, 2015
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #16792 |
| License | MIT |
| Doc PR | - |
stof commentedDec 2, 2015
@nicolas-grekas this still does not fix the case of |
piogrek commentedDec 2, 2015
@stof thanks for a hint I think that's the simplest way and EntityType already have it. Just found PR from last night (but it needs more thought)symfony/propel1-bridge#1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@stof here isnull handling. But should we deprecate accepting thenull value? Otherwise, 3.0 misses a "choices" normalizer also, isn't it?
https://github.com/symfony/symfony/pull/16796/files#diff-11ee91a32c601e8e1d509117556d53b3R400
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@nicolas-grekas some sub-types are usingnull as default value of the option to distinguish between passing choices explicitly and using other ways of building choices (using the query builder). This change would break them (the PropelType for instance).
The Doctrine entity type is not affected because it changes thechoices_as_values default and so is not triggered this line of code.
IMO, ifnull is given as input, you should just return it as is here (and letting the validation happening later checking this case in case the subtype does not handle it properly)
mbabker commentedDec 3, 2015
FWIW I ran into the same error condition in#16792 using a combination of the Form component 2.8 and Doctrine bridge 3.0 with the EntityType and this PR fixed the error for that case. |
stof commentedDec 5, 2015
👍 |
1 similar comment
fabpot commentedDec 5, 2015
👍 |
stof commentedDec 5, 2015
Thanks for fixing this bug@nicolas-grekas. |
This PR was merged into the 2.8 branch.Discussion----------[Form] Fix choices defined as Traversable| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#16792| License | MIT| Doc PR | -Commits-------021d93a [Form] Fix choices defined as Traversable
Tobion commentedDec 29, 2015
Shouldn't this have been merged in 2.7? See#17163 |