Fix the retrieval of the value with property path when using a loader #15623
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CS fixes need to be done, see fabbot |
|
👍 |
|
@fabpot the URL of the Symfony documentation about the PR contribution header should be a link on fabbot report pages, to make it easier to access them. |
5bee5dc to
5df64dc
Compare
|
CS fixes are done |
|
Thank you @stof. |
Tobion
added a commit
that referenced
this pull request
Aug 26, 2015
…ng a loader (stof) This PR was merged into the 2.7 branch. Discussion ---------- Fix the retrieval of the value with property path when using a loader | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a the ChoiceType is transforming the form data to its value in case of non-expanded fields. When creating the choice list from choices, this works fine when using a property path for the choice_value, because it has a safeguard against values which are not object or arrays (a typical case being ``null`` because nothing is selected yet). However, when loading from a ChoiceLoader, the generated closure was missing the same safeguard, breaking the usage of ChoiceLoader with property path. This went unnoticed because the only usage of choice_loader in core is for the doctrine type, and this one uses its own logic to get the choice value I added test on the PropertyAccessDecorator directly, to have fast tests. We could imagine adding tests in the ChoiceTypeTest using a ChoiceLoader too, matching all existing tests for other ways to specify arguments, but I'm not sure it is worth it. Commits ------- 5df64dc Fix the retrieval of the value with property path when using a loader
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
the ChoiceType is transforming the form data to its value in case of non-expanded fields.
When creating the choice list from choices, this works fine when using a property path for the choice_value, because it has a safeguard against values which are not object or arrays (a typical case being
nullbecause nothing is selected yet).However, when loading from a ChoiceLoader, the generated closure was missing the same safeguard, breaking the usage of ChoiceLoader with property path.
This went unnoticed because the only usage of choice_loader in core is for the doctrine type, and this one uses its own logic to get the choice value
I added test on the PropertyAccessDecorator directly, to have fast tests. We could imagine adding tests in the ChoiceTypeTest using a ChoiceLoader too, matching all existing tests for other ways to specify arguments, but I'm not sure it is worth it.