Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[WIP] [2.7] [Form] fixempty_data option in expandedChoiceType#17822

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

Merged

Conversation

@HeahDude
Copy link
Contributor

QA
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#17791
LicenseMIT
Doc PR-

It might happen because inForm::submit() the handling ofempty_dataline 597 comes after each child of a compound field has been submittedline 549.

So whenChoiceType isexpanded,compound option is defaulted totrue and it passes its empty submitted data to its children before handling its ownempty_data option.

This PR uses the listener already added inChoiceType only whenexpanded is true to handleempty_data atPRE_SUBMITline 539.

  • Fix FQCN in tests for 2.8
  • Removechoices_as_values in tests for 3.0

@HeahDude
Copy link
ContributorAuthor

This one misses itsform label :)

@TobionTobion added the Form labelFeb 18, 2016
@HeahDudeHeahDude mentioned this pull requestMar 7, 2016
@HeahDude
Copy link
ContributorAuthor

@webmozart Can you have a look at this one please ?

@HeahDudeHeahDudeforce-pushed thefix-empty_data-choice-type_expanded branch fromb4858c7 tod479adfCompareJune 25, 2016 14:30
@HeahDude
Copy link
ContributorAuthor

@symfony/deciders this bug is still present and is very annoying, this PR allows to useempty_data option with expanded choice type (and sub types) whether it's multiple or not.

Could you please review?

@fabpot
Copy link
Member

Thank you@HeahDude.

@fabpotfabpot merged commitd479adf intosymfony:2.7Jun 28, 2016
fabpot added a commit that referenced this pull requestJun 28, 2016
…oiceType` (HeahDude)This PR was merged into the 2.7 branch.Discussion----------[WIP] [2.7] [Form] fix `empty_data` option in expanded `ChoiceType`| Q             | A| ------------- | ---| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#17791| License       | MIT| Doc PR        | -It might happen because in `Form::submit()` the handling of `empty_data` [line 597](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Form.php#L597) comes after each child of a compound field has been submitted [line 549](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Form.php#L549).So when `ChoiceType` is `expanded`, `compound` option is defaulted to `true` and it passes its empty submitted data to its children before handling its own `empty_data` option.This PR uses the listener already added in `ChoiceType` only when `expanded` is true to handle `empty_data` at `PRE_SUBMIT` [line 539](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Form.php#L539).- [ ] Fix FQCN in tests for 2.8- [ ] Remove `choices_as_values` in tests for 3.0Commits-------d479adf [Form] fix `empty_data` option in expanded `ChoiceType`
@HeahDude
Copy link
ContributorAuthor

Thank you@fabpot

@HeahDudeHeahDude deleted the fix-empty_data-choice-type_expanded branchJune 28, 2016 07:20
HeahDude added a commit to HeahDude/symfony that referenced this pull requestJun 29, 2016
nicolas-grekas added a commit that referenced this pull requestJun 29, 2016
This PR was merged into the 3.0 branch.Discussion----------[Form] fixed ChoiceTypeTest after#17822| Q             | A| ------------- | ---| Branch?       | 3.0| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | ~| License       | MIT| Doc PR        | ~Commits-------777c193 [Form] fixed ChoiceTypeTest after#17822
nicolas-grekas added a commit that referenced this pull requestJun 29, 2016
* 3.0:  [Form] fixed ChoiceTypeTest after#17822
nicolas-grekas added a commit that referenced this pull requestJun 29, 2016
* 3.1:  [Form] fixed ChoiceTypeTest after#17822  [DoctrineBridge] fixed DoctrineChoiceLoaderTest by removing deprecated factory  [ci] Upgrade phpunit wrapper deps  [FrameworkBundle] Fix fixtures  [HttpKernel] Inline ValidateRequestListener logic into HttpKernel  fixed HttpKernel dependencies after#18688
This was referencedJun 30, 2016
fabpot added a commit that referenced this pull requestFeb 1, 2018
…e (HeahDude)This PR was merged into the 2.7 branch.Discussion----------[Form] Fixed empty data on expanded ChoiceType and FileType| Q             | A| ------------- | ---| Branch?       | 2.7| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#25896| License       | MIT| Doc PR        | ~Alternative of#25924.I don't think we have to do this by adding overhead in master and getting it properly fixed in 2 years.This is bug I've introduced while fixing another bug#17822. Which then has been replicated in#20418 and#24993.I propose instead to clean up the code for all LTS, since this is a wrong behaviour that has never been documented, and most of all unreliable.The `empty_data` can by anything in the view format as long as the reverse view transformation supports it. Even an object derived from the data class could be invokable.I think we should remain consistent withhttps://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Form/Form.php#L615.Commits-------9722c06 [Form] Fixed empty data on expanded ChoiceType and FileType
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@HeahDude@fabpot@javiereguiluz@Tobion@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp