Skip to content

[Form] fetch all known ChoiceType values at once #50937

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 1 commit into from
Jul 13, 2023

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Jul 11, 2023

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #50898
License MIT
Doc PR

@xabbuh xabbuh requested a review from yceruto as a code owner July 11, 2023 12:49
@carsonbot carsonbot added this to the 5.4 milestone Jul 11, 2023
@carsonbot carsonbot changed the title [Form] fetch all known values at once [Form]  fetch all known values at once Jul 11, 2023
@xabbuh xabbuh changed the title [Form]  fetch all known values at once [Form] fetch all known ChoiceType values at once Jul 11, 2023
@stof
Copy link
Member

stof commented Jul 11, 2023

shouldn't we also keep fetching them at once for expanded choice fields as well ?

Forget that. The loop above for expanded fields does not call the choice list in that loop.

@stof
Copy link
Member

stof commented Jul 11, 2023

Is there a way to write a test ensuring that a submission of a multiple choice type calls getChoicesForValues only once during the processing, to prevent similar regressions ?

@fabpot
Copy link
Member

fabpot commented Jul 13, 2023

Thank you @xabbuh.

@fabpot fabpot merged commit fbd3988 into symfony:5.4 Jul 13, 2023
@xabbuh xabbuh deleted the issue-50898 branch July 13, 2023 13:08
This was referenced Jul 29, 2023
@elementaire
Copy link
Contributor

For record, denied as bug fix for 4.4 #48208, accepted as feature and already merged in 6.3 #48342

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.

5 participants