Skip to content

[Form] inherit_data and event dispatching ... #8607

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
rande opened this issue Jul 30, 2013 · 6 comments
Closed

[Form] inherit_data and event dispatching ... #8607

rande opened this issue Jul 30, 2013 · 6 comments
Labels

Comments

@rande
Copy link
Contributor

rande commented Jul 30, 2013

@bschussek this was supposed to be a blog post, but never get published as it is too much technical ....

While migrating the FormatterBundle to Symfony2.3, some small details can take a bit longer to fix than expecting. There is one main issue with the Form Component which makes it pretty impossible for a FormType to access the different fields.

Let's explain how we try to avoid a BC break in the SonataFormatterBundle:

The Formatter Bundle provides a easy way to convert raw markup (markdow, textile, ...) into a final state. The visual representation is

In order to make it work a FormatterType is used. The type allows to define a format (mardown, html, etc ...) to be applied on a target field by reading the data from a source field. So if you have a Post entity, the entity will have :

  • a content field : the transformed field
  • a rawContent field : the source field
  • a contentFormatter field: the format field

As < Symfony2.3, it was possible for a FromType to retrieve the parent form builder. However from 2.3, the FormBuilder::getParent is not available anymore, due to the fact that the parent builder can be changed so it is very hard to rely on it (on some very specific edge cases).

The parent builder was useful to attach an event listener on the parent form to apply the transformation once the form is being posted.

As of Symfony2.3, The FormatterType is broken ... let's see the others options we have:

The inherit_data option can be a solution ~ from the PHP doc: "Sets whether the form should read and write the data of its parent.".
But by looking at the Form class, the Form::setData is not called if the field is defined as ‘inherit_data’. This method is responsible for notifying listeners when data is set. So by using the inherit_data, it is not possible to use FormEvents::PRE_SET_DATA event inside the FormatterType::buildForm

public function getData()
{
    if ($this->config->getInheritData()) {
        if (!$this->parent) {
            throw new RuntimeException('The form is configured to inherit its parent\'s data, but does not have a parent.');
        }

        return $this->parent->getData();
    }

    if (!$this->defaultDataSet) {
        $this->setData($this->config->getData());
    }

    return $this->modelData;
}

After digging into the Form Component source code, the internal event dispatcher is set per FormBuilder and while a FormType::buildForm is called, the callee method does not copy the event dispatcher from the FormatterType builder to the parent (when inherit_data is set to true). This can be the expected behavior as inherited_data means "Sets whether the form should read and write the data of its parent.", we can expect the event dispatcher being shared. By looking at the code, this change is not as easy as it might look. While a Form is being built by using the FormBuilder and the FormType the options are not always available.

So in order to find a solution and keep going, we introduce a BC break in the FormatterType.

The solution: pass the parent event dispatcher to the form type. This might not be the best solution, but at least it will work as before and might break on some edge cases (can someone point me to an edge case?)

The BC break and the fix: sonata-project/SonataFormatterBundle@2d1a412

@webda2l
Copy link

webda2l commented Jul 30, 2013

Related to #8253
You can continue to call getParent during the PRE_SET_DATA event (from a not inherit_data field). Ex: https://github.com/a2lix/TranslationFormBundle/blob/master/Form/EventListener/DefaultTranslationsSubscriber.php#L42

@rande
Copy link
Contributor Author

rande commented Jul 30, 2013

@webda2l no, as the pre set data is never call ...

@webmozart
Copy link
Contributor

I would approach your case in a different way: Create a FormattedType which embeds two fields, one with the select box and another one whose type is configurable via an option. Instead of this code in your form:

$builder->add('myField', 'mytype');
$builder->add('myFieldFormatter', 'formatter', array('for' => 'myField'));

you'd do

$builder->add('myField', 'formatted', array(
    'type' => 'mytype',
));

and in this way eliminate the need to access the parent form. Would that work?

@rande
Copy link
Contributor Author

rande commented Jul 30, 2013

What you propose is actually what the formatter type does. The issue is coming from registered listener with is never call if the listener is attached to the one provided by the FormatterType. So the transformation (converting a raw text to the selected format) is never applied.

See https://github.com/sonata-project/SonataFormatterBundle/blob/master/Form/Type/FormatterType.php#L66-L87

@webmozart
Copy link
Contributor

Do you necessarily want to keep the data of the select field in the parent data?

@webmozart
Copy link
Contributor

Closing as duplicate of #8834.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants