Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted#7940
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
#7939 has been merged now. |
@bschussek Is there any help we could bring for this to get merged ? |
@bscussek - anything more that can be done to get this one merged? |
…e they don't offer any performance benefit
…choices are submitted
I finished this PR now. I did not improve the test coverage of ModelChoiceList since currently none of our Propel tests uses a temporary database, which is needed here, but I don't know how to set it up. I propose to merge this PR as is and improve the ModelChoiceList test suite later on in cooperation with the propel guys. /cc@willdurand |
If this is a but fix with no BC beaks, this PR should be merged into 2.2, right? |
I'll check whether this is possible |
Ok, here are three versions now:
|
This PR was merged into the 2.2 branch.Discussion----------[2.2][Form] Fixed expanded choice field to be marked invalid when unknown choices are submittedSame as#7940, rebased onto 2.2.Commits-------4d2dc55 [DoctrineBridge] Improved test coverage of EntityChoiceList9d3628c [Form] Improved test coverage of ChoiceList classesed83752 [Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted30aa1de [Form] Fixed ChoiceList::get*By*() methods to preserve order and array keys53f292a [Form] Removed usage of the ChoiceList::getIndicesFor*() methods where they don't offer any performance benefit
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 submittedSame as#7940, rebased onto 2.3.Commits-------7879f07 [DoctrineBridge] Improved test coverage of EntityChoiceList58e7c10 [Form] Improved test coverage of ChoiceList classes9542d72 [Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted72b8807 [Form] Fixed ChoiceList::get*By*() methods to preserve order and array keyse1bf07f [Form] Removed usage of the ChoiceList::getIndicesFor*() methods where they don't offer any performance benefit
This PR was merged into the master branch.Discussion----------[Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted#7939 must be merged before this PR is merged.| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#5113,#5190| License | MIT| Doc PR | -TODO:- [x] test EntityChoiceList for stricter rules- [ ] test ModelChoiceList for stricter rules- [x] remove/deprecate the ChoiceList::getIndicesFor*() methodsCommits-------9efdb8e [Form] Deprecated ChoiceList::getIndicesFor*() methods67ba131 [DoctrineBridge] Improved test coverage of EntityChoiceList31e5ce5 [Form] Improved test coverage of ChoiceList classes6283b0e [Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted79a214f [Form] Fixed ChoiceList::get*By*() methods to preserve order and array keys62fbed6 [Form] Removed usage of the ChoiceList::getIndicesFor*() methods where they don't offer any performance benefit
ok, I thought I was able to write some tests for Propel, but I must admit I need some more pointers. What should I test? |
Basically all the _EntityChoiceList_Test.php tests should be ported 1:1 to |
Alright, I'll try to come up with something. |
This PR was merged into the 2.2 branch.Discussion----------Fixed an entity class name in a benchmark test| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Broken in#7940 (67ba131). Since benchmark tests are not run on travis, it didn't complain.Commits-------50ff35a Fixed an entity class name.
#7939 must be merged before this PR is merged.
TODO: