[Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted #7940
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
|
#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:
|
fabpot
added a commit
that referenced
this pull request
Sep 10, 2013
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 submitted Same as #7940, rebased onto 2.2. Commits ------- 4d2dc55 [DoctrineBridge] Improved test coverage of EntityChoiceList 9d3628c [Form] Improved test coverage of ChoiceList classes ed83752 [Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted 30aa1de [Form] Fixed ChoiceList::get*By*() methods to preserve order and array keys 53f292a [Form] Removed usage of the ChoiceList::getIndicesFor*() methods where they don't offer any performance benefit
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
added a commit
that referenced
this pull request
Sep 10, 2013
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*() methods Commits ------- 9efdb8e [Form] Deprecated ChoiceList::getIndicesFor*() methods 67ba131 [DoctrineBridge] Improved test coverage of EntityChoiceList 31e5ce5 [Form] Improved test coverage of ChoiceList classes 6283b0e [Form] Fixed expanded choice field to be marked invalid when unknown choices are submitted 79a214f [Form] Fixed ChoiceList::get*By*() methods to preserve order and array keys 62fbed6 [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. |
jakzal
added a commit
to jakzal/symfony
that referenced
this pull request
Sep 24, 2013
Broken in symfony#7940.
fabpot
added a commit
that referenced
this pull request
Sep 24, 2013
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.
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.
#7939 must be merged before this PR is merged.
TODO: