Skip to content

[Form] Make transform methods public #3514

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 1 commit into from

Conversation

Burgov
Copy link
Contributor

@Burgov Burgov commented Mar 6, 2012

Bug fix: No
Feature addition: Yes
Backwards compatibility break: No
Symfony2 tests pass: Yes
Fixes the following tickets: -

Especially when using the PRE_BIND and BIND_CLIENT_DATA listeners, it is very convenient to be use the transformers. I can fetch them using $form->getClientTransformers() and them looping over them, but that's just code duplication of what the form defines in it's private functions.

For example, someone enters country with ID 12, I fetch this in my listener, and want to create a new form element with a list of states in this country. I'd either have to inject the EntityManager into the listener, or use the transformers, to get the Country entity in order to fetch the states from it. The latter, IMO, is the best option, but since the transform methods are private, i have to rewrite them within my listener instead of being able to reuse Component code

@stof
Copy link
Member

stof commented Mar 6, 2012

I don't see why a listener would need to call these methods. If you need to access these methods, it probably means that you are not using the appropriate event as an event is dispatched between the calls to each of these methods

@Burgov
Copy link
Contributor Author

Burgov commented Mar 7, 2012

Once I hit BIND_NORM_DATA, I no longer have access to the Post data, meaning that I can add the State field, but if that happened during an AJAX call earlier, and the form is now bound in a regular way with the State field set to a value, there is no way to bind this particular value, as it was not transformed to norm data since the field didn't exist yet.

If i'm not mistaken, BIND_CLIENT_DATA or PRE_BIND are the only events in which i can generate dependent fields. Look at this example:

public function buildForm(FormBuilder $builder, array $options)
{
    $builder
        ->add('address', null, array('attr' => array('size' => 56)))
        ->add('zipcode', null, array('attr' => array('size' => 10)))
        ->add('city', null, array('attr' => array('size' => 40)))
        ->add('country', null, array('query_builder' => function($er) {
            return $er->createQueryBuilder('c')->orderBy('c.id');
        }, 'ajax_reload'=>'SamsonAddressBookBundle:Company:_addressesForm.html.twig'))
        ->add('distance', 'number', array('required' => false, 'attr' => array('size' => 10)))
        ->add('addressTemplate', 'samson_expandable_textarea', array('required' => false, 'preview'=>false));
    ;

    $factory = $builder->getFormFactory();

    $stateListener = function(DataEvent $e) use ($factory) {
        $data = $e->getData();
        $form = $e->getForm();

        if (null === $data) {
            return;
        }

        $country = null;
        if ($data instanceof Address) {
            $country = $data->getCountry();
        } elseif (is_array($data)) {
            $transformers = $form->get('country')->getClientTransformers();
            $country = $data['country'];
            foreach($transformers as $transformer) {
                $country = $transformer->reverseTransform($country);
            }
        }

        if (null !== $country && count($country->getStates()) > 0) {
            $form->add($factory->createNamed( 'entity', 'state', null, array(
                'class'=>'SamsonAddressBookBundle:State',
                'choices'=>$country->getStates(),
                'empty_value'=>'Choose state'
            ) ));
        } else {
            $form->remove('state');
            if ($e instanceof FilterDataEvent) {
                unset($data['state']);
                $e->setData($data);
            }
        }
    };

    $builder
        ->addEventListener(FormEvents::PRE_SET_DATA, $stateListener)
        ->addEventListener(FormEvents::BIND_CLIENT_DATA, $stateListener)
    ;
}

As you can see, I'm using the transformers to figure out which country was selected, in order to add the State field. IMO this is duplication of code, since the Form class provides the same logic but better tested, only in private methods. $data['state'] could be populated here if the field had been added by an AJAX call earlier and the field is now bound by a normal request. If I want the state field to be bound, I have to add it here. BIND_NORM_DATA would be too late as the value of $data['state'] will be discarded without fields to bind it to.

@mvrhov
Copy link

mvrhov commented Mar 7, 2012

This is my implementation, but it's still based on pre latest form refactoring. So I cannot confirm if it works on master.

Main form:

    public function buildForm(FormBuilder $builder, array $options)
    {
        $builder
            ->add('country', 'country', array(
                'drill_down_to' => 'city',
            ))
        ;

        $factory = $builder->getFormFactory();
        $function = function (DataEvent $event) use ($factory, $options) {
            $data = $event->getData();

            if (null === $data) {
                return;
            }
            /** @var $form \Symfony\Component\Form\Form */
            $form = $event->getForm();
            $form->remove('state');
            $form->remove('city');

            $country = '';
            if ($data instanceof $options['data_class']) {
                $country = $data->getCountry();
            } elseif (is_array($data)) {
                $country = $data['country'];
            }
            $form->add($factory->createNamed('state', 'state', null, array(
                /*'required' => false,*/
                'country' => $country,
            )));

            $state = '';
            if ($data instanceof $options['data_class']) {
                $state = $data->getState();
            } elseif (is_array($data)) {
                $state = $data['state'];
            }
            $form->add($factory->createNamed('city', 'city', null, array(
                /*'required' => false,*/
                'state' => $state,
            )));
        };

        $builder->addEventListener(FormEvents::PRE_SET_DATA, $function);
        $builder->addEventListener(FormEvents::BIND_CLIENT_DATA, $function);
    }

StateType

class StateType extends AbstractType
{

    /**
     * @var TranslatorInterface
     */
    private $translator;

    public function __construct(TranslatorInterface $translator)
    {
        $this->translator = $translator;
    }

    public function getParent(array $options)
    {
        return 'entity';
    }

    public function getDefaultOptions(array $options)
    {
        $routeName = isset($options['route']) ? $options['route'] : 'oa_ajax_state';
        $emptyValue = isset($options['empty_value']) ? $options['empty_value'] : /** @Desc("Please select state")*/ $this->translator->trans('select.state');

        $options = array_merge(array(
                'multiple'  => false,
                'drill_down_child' => 'city',
                'drill_down_to' => 'state',
                'class'         => 'XBundle:State',
                'property'      => 'state',
                'property_path' => 'state',
                'empty_value'   => $emptyValue,
            ), $options
        );

        //do we need to drill down?
        if (!empty($options['drill_down_to'])) {
            $options['attr'] = array('data-dd-type' => 'state', 'data-dd-route' => $routeName, 'data-dd-empty-value' => $emptyValue, 'data-dd-child' => $options['drill_down_child']);
        }

        $country = $options['country'];
        if (null == $country) {
            return array_merge($options, array(
                'choices' => array(),
            ));
        }

        return array_merge($options, array(
            'query_builder' => function(EntityRepository $er) use ($country) {
                return $er->createQueryBuilder('s')
                    ->where('s.country = :country')
                    ->orderBy('s.state', 'ASC')
                    ->setParameter('country', $country)
                    ;
            },
        ));
    }

    public function getName()
    {
        return 'state';
    }
}

@Burgov
Copy link
Contributor Author

Burgov commented Mar 30, 2012

@mvrhov Your example uses the ID of the country in the query in the BindClientData, rather than the actual Country object as in the PreSetData event, which is OK in this case, but if you do want to use the actual Country object you'd either have to pass the $em to the FormType and then to the listener, in order to fetch the object, or, as I showed in my example, use the Form child's Data Transformers. While this works fine, it'd be nice to just be able to call the Form child's clientToNorm() method.

Here is another example:

public function buildForm(FormBuilder $builder, array $options)
{
    $choices = array('contract_field', 'value', 'none');
    $choices = array_combine($choices, $choices);
    $builder->add('hour_multiplier_type', 'choice', array('choices' => $choices, 'empty_value' => 'Kies type', 'ajax_reload' => true));

    $formType = $this;
    $factory = $builder->getFormFactory();
    $builder->addEventListener(FormEvents::BIND_CLIENT_DATA, function(FilterDataEvent $e) use ($formType, $factory) {
        $data = $e->getData();
        $transformers = $e->getForm()->get('hour_multiplier_type')->getClientTransformers();
        $type = isset($data['hour_multiplier_type']) ? $data['hour_multiplier_type'] : null;
        foreach ($transformers as $transformer) {
            $type = $transformer->reverseTransform($type);
        }
        $formType->multiplierTypeListener($e, $type, $factory);
    })->addEventListener(FormEvents::SET_DATA, function(FilterDataEvent $e) use ($formType, $factory) {
        $data = $e->getData();
        $formType->multiplierTypeListener($e, isset($data['hour_multiplier_type']) ? $data['hour_multiplier_type'] : null, $factory);
    });
}

Here I need to get the actual value from the ChoiceList, not the keyed index generated by the ChoiceList. I'd either have to pass the ChoiceList object into the listener, or fetch it as attribute from the Form object, to be able to get the actual value, while simply being able to call the Form's clientToNorm method is much more convenient:

public function buildForm(FormBuilder $builder, array $options)
{
    $choices = array('contract_field', 'value', 'none');
    $choices = array_combine($choices, $choices);
    $builder->add('hour_multiplier_type', 'choice', array('choices' => $choices, 'empty_value' => 'Kies type', 'ajax_reload' => true));

    $formType = $this;
    $factory = $builder->getFormFactory();
    $builder->addEventListener(FormEvents::BIND_CLIENT_DATA, function(FilterDataEvent $e) use ($formType, $factory) {
        $data = $e->getData();
        $type = $e->getForm()->get('hour_multiplier_type')->clientToNorm(isset($data['hour_multiplier_type']) ? $data['hour_multiplier_type'] : null);
        $formType->multiplierTypeListener($e, $type, $factory);
    })->addEventListener(FormEvents::SET_DATA, function(FilterDataEvent $e) use ($formType, $factory) {
        $data = $e->getData();
        $formType->multiplierTypeListener($e, isset($data['hour_multiplier_type']) ? $data['hour_multiplier_type'] : null, $factory);
    });
}

@webmozart
Copy link
Contributor

The internal behaviour of data transformation should not be made public. I created a new issue (linked above) that summarizes the essence of this PR in a feature request.

@webmozart webmozart closed this Apr 3, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants