-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] Make bind() method bind while fields are being added #4548
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
Closed
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
const | ||
POST_SET_DATA = 1, | ||
POST_BIND = 2 | ||
; |
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.
this does not follow the Symfony CS. You should use a const
for each constant
Thank you for the PR! This won't make it into 2.1 anymore, but I'll look at your solution after that. |
@bschussek ping |
I think this should be solved in a more elaborate fashion (see #5807). |
This was referenced Aug 22, 2013
fabpot
added a commit
that referenced
this pull request
Aug 23, 2013
This PR was merged into the 2.2 branch. Discussion ---------- [Form][2.2] Fixed Form::submit() to react to dynamic form modifications | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - ref #3767, #3768, #4548, #8748 cc @Burgov This PR ensures that fields that are added during the submission process of a form are submitted as well. For example: ```php $builder = $this->createFormBuilder() ->add('country', 'choice', ...) ->getForm(); $builder->get('country')->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event) { $form = $event->getForm()->getParent(); $country = $event->getForm()->getData(); $form->add('province', 'choice', /* ... something with $country ... */); }); ``` Currently, the field "province" will not be submitted, because the submission loop uses `foreach`, which ignores changes in the underyling array. Additionally, this PR ensures that `setData()` is called on *every* element of a form (except those inheriting their parent data) when `setData()` is called on the root element (i.e., during initialization). Currently, when some of the intermediate nodes (e.g. embedded forms) are submitted with an empty value, `setData()` won't be called on the nodes below (i.e. the fields of the embedded form) until `get*Data()` is called on them. If `getData()` is *not* called before `createView()`, any effects of `*_DATA` event listeners attached to those fields will not be visible in the view. Commits ------- cd27e1f [Form] Extracted ReferencingArrayIterator out of VirtualFormAwareIterator ccaaedf [Form] PropertyPathMapper::mapDataToForms() *always* calls setData() on every child to ensure that all *_DATA events were fired when the initialization phase is over (except for virtual forms) 19b483f [Form] Removed superfluous reset() call 00bc270 [Form] Fixed: submit() reacts to dynamic modifications of the form children
fabpot
added a commit
that referenced
this pull request
Aug 23, 2013
This PR was merged into the 2.3 branch. Discussion ---------- [Form][2.3] Fixed Form::submit() and Form::setData() to react to dynamic form modifications | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - ref #3767, #3768, #4548, #8748 cc @Burgov This PR ensures that fields that are added during the submission process of a form are submitted as well. For example: ```php $builder = $this->createFormBuilder() ->add('country', 'choice', ...) ->getForm(); $builder->get('country')->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event) { $form = $event->getForm()->getParent(); $country = $event->getForm()->getData(); $form->add('province', 'choice', /* ... something with $country ... */); }); ``` Currently, the field "province" will not be submitted, because the submission loop uses `foreach`, which ignores changes in the underyling array. Contrary to #8827 (for 2.2), this PR also allows dynamic modifications during `setData()`: ```php $builder = $this->createFormBuilder() ->add('country', 'choice', ...) ->getForm(); $builder->get('country')->addEventListener(FormEvents::POST_SET_DATA, function (FormEvent $event) { $form = $event->getForm()->getParent(); $country = $event->getData(); $form->add('province', 'choice', /* ... something with $country ... */); }); ``` For 2.2, this fix is not possible, because it would require changing the signature `DataMapperInterface::mapDataToForms($data, array $forms)` to `DataMapperInterface::mapDataToForms($data, array &$forms)`, which would be a BC break. For 2.3, this change is not necessary (instead of an array we now pass an iterator, which is passed by reference anyway). Additionally, this PR ensures that `setData()` is called on *every* element of a form (except those inheriting their parent data) when `setData()` is called on the root element (i.e., during initialization). Currently, when some of the intermediate nodes (e.g. embedded forms) are submitted with an empty value, `setData()` won't be called on the nodes below (i.e. the fields of the embedded form) until `get*Data()` is called on them. If `getData()` is *not* called before `createView()`, any effects of `*_DATA` event listeners attached to those fields will not be visible in the view. Commits ------- 7a34d96 Merge branch 'form-submit-2.2' into form-submit-2.3 cd27e1f [Form] Extracted ReferencingArrayIterator out of VirtualFormAwareIterator 3cb8a80 [Form] Added a test that ensures that setData() reacts to dynamic modifications of a form's children 07d14e5 [Form] Removed exception in Button::setData(): setData() is now always called for all elements in the form tree during the initialization of the tree b9a3770 [Form] Removed call to deprecated method 878e27c Merge branch 'form-submit-2.2' into form-submit-2.3 ccaaedf [Form] PropertyPathMapper::mapDataToForms() *always* calls setData() on every child to ensure that all *_DATA events were fired when the initialization phase is over (except for virtual forms) 19b483f [Form] Removed superfluous reset() call 5d60a4f Merge branch 'form-submit-2.2' into form-submit-2.3 00bc270 [Form] Fixed: submit() reacts to dynamic modifications of the form children
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.
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3767
Todo: See Cons
This PR fixes #3767 and could probably replace #3768
The method used in this PR has it's pros and it's cons compared to #3768:
Pro:
Con: