Skip to content

[Form] added ChildDataEvent #3768

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
wants to merge 3 commits into from

Conversation

Burgov
Copy link
Contributor

@Burgov Burgov commented Apr 3, 2012

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3767

This new event is fired every time a forms child is bound, allowing listeners to add or remove field accordingly, based on the appData of the child form. See the ticket mentioned above for a use case scenario

/**
* Gets the data of the field that was just set
*/
public function getData()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method is totally useless as it does not change anything from the parent method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, hence my comment, but you beat me to it ;) The only reason I added it was for the docblock

@Burgov
Copy link
Contributor Author

Burgov commented Apr 3, 2012

I couldn't come up with a better name for the event... Any ideas?

And perhaps another argument should be added: $clientData, rather than $data, whereas $data can then become the $extraData array? (not in the least to avoid the current naming confusion)

In that case, optionally we could make this event extend the filter data event so $extraData can be updated, but I'm not sure if this is useful. Can't really think of a use case

}
}
} while(count($extraData) && $lastCount != ($lastCount = count($this)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the second part of the check is wrong IMO. It will always return true as the assignment is done before the condition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely to be an infinite loop when a form is submitted with extra data

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I've added a commit to fix it. I decided to actually check the old array of children to the new one, because it could also be that a field was added and another removed, leaving the count unchanged

@Burgov
Copy link
Contributor Author

Burgov commented Apr 19, 2012

@bschussek is this similar to what you had in mind?

unset($extraData[$name]);

// every time we bind a child, we dispatch an event to allow
// listeners to add or remove field based on the result value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: a field or fields

@stof
Copy link
Member

stof commented Oct 13, 2012

@bschussek what do you think about it ?

@webmozart
Copy link
Contributor

I think this should be solved in a more elaborate fashion (see #5807).

@webmozart webmozart closed this Oct 23, 2012
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants