Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Form] fix "prototype" not required when parent form is not required#18317
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
HeahDude commentedMar 26, 2016
| Q | A |
|---|---|
| Branch? | 2.3+ |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #18311 |
| License | MIT |
| Doc PR | ~ |
7ee5392 toe8e5682Compare| if ($view->parent && !$view->parent->vars['required']) { | ||
| $view->vars['prototype']->vars['required'] =false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
May be a better solution:
$prototype =$form->getConfig()->getAttribute('prototype');$view->vars['prototype'] =$prototype->setParent($form)->createView($view);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed! Thanks@sergeyfedotov
sergeyfedotov commentedMar 26, 2016
HeahDude commentedMar 26, 2016
@sergeyfedotov The assertion in your last test looks wrong, "prototype" should not be required in that case, and the test is passing while it's not as expected. I don't think there is a BC break here. |
sergeyfedotov commentedMar 26, 2016
Ok. I really don't know should it be interpreted as a BC break or not. But currently |
sergeyfedotov commentedMar 26, 2016
Seems like this linehttps://github.com/HeahDude/symfony/blob/bug-prototype_required/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php#L31 is not needed anymore. |
HeahDude commentedMar 26, 2016
@sergeyfedotov Yes it is, if the parent does not override the prototype options, |
e8e5682 to041c804Compare041c804 to7df9ca2CompareHeahDude commentedMar 31, 2016
Status: Ready |
xabbuh commentedApr 5, 2016
👍 Status: Reviewed |
HeahDude commentedApr 5, 2016
Note to the merger: in 2.8 |
fabpot commentedApr 5, 2016
Thank you@HeahDude. |
…t required (HeahDude)This PR was merged into the 2.3 branch.Discussion----------[Form] fix "prototype" not required when parent form is not required| Q | A| ------------- | ---| Branch? | 2.3+| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#18311| License | MIT| Doc PR | ~Commits-------7df9ca2 [Form] fix "prototype" not required when parent form is not required
This PR was merged into the 2.3 branch.Discussion----------[Form] Remove unnecessary option assignment| Q | A| ------------- | ---| Branch? | 2.3| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Option assignment is not required because the prototype inherits this option from the parent form via standard inheritance mechanism.Related pull requests:#16959,#18317Commits-------da8a197 Remove unnecessary option assignment