8000 [2.3][Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted by webmozart · Pull Request #8981 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[2.3][Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted #8981

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

Merged
merged 5 commits into from
Sep 10, 2013

Conversation

webmozart
Copy link
Contributor

Same as #7940, rebased onto 2.3.

fabpot added a commit that referenced this pull request Sep 10, 2013
This PR was merged into the 2.3 branch.

Discussion
----------

[2.3][Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted

Same as #7940, rebased onto 2.3.

Commits
-------

7879f07 [DoctrineBridge] Improved test coverage of EntityChoiceList
58e7c10 [Form] Improved test coverage of ChoiceList classes
9542d72 [Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted
72b8807 [Form] Fixed ChoiceList::get*By*() methods to preserve order and array keys
e1bf07f [Form] Removed usage of the ChoiceList::getIndicesFor*() methods where they don't offer any performance benefit
@fabpot fabpot merged commit 7879f07 into symfony:2.3 Sep 10, 2013
@yshyshkin
Copy link

It look's like this pull request created an issue in EntityChoiceList::getChoicesForValues - it doesn't check case with empty array, so it can cause as exception when there is no entities and it generates SQL with condition "id in ()".

@webmozart
Copy link
Contributor Author

Thanks for reporting @yshyshkin. I added a test case before removing the appropriate code, which didn't fail after removing. Going to investigate...

@webmozart
Copy link
Contributor Author

@yshyshkin I can't reproduce the issue. Could it be that updating Doctrine fixes this bug?

@webmozart
Copy link
Contributor Author

@yshyshkin Ah in fact I can reproduce it, it just didn't fail with an exception as I expected.

@webmozart
Copy link
Contributor Author

Fixed in #9001

@yshyshkin
Copy link

@bschussek Thank you, it should help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0