Skip to content
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

Form with radio buttons fails after Symfony 2.7 upgrade #14877

Closed
XWB opened this issue Jun 5, 2015 · 25 comments
Closed

Form with radio buttons fails after Symfony 2.7 upgrade #14877

XWB opened this issue Jun 5, 2015 · 25 comments

Comments

@XWB
Copy link
Contributor

XWB commented Jun 5, 2015

Imagine this form:

public function buildForm(FormBuilderInterface $builder, array $options)
{
        $builder->add('transportType', 'entity',
            array(
                'class' => 'MyBundle:TransportType',
                'property' => 'language.name',
                'expanded' => true,
                'query_builder' => function (EntityRepository $er) {
                    return $er->createQueryBuilder('e')
                        ->join('e.language', 'l')
                        ->orderBy('l.name', 'DESC');
                },
            )
        );
}

public function setDefaultOptions(OptionsResolverInterface $resolver)
{
        $resolver->setDefaults(array(
            'data_class' => 'BLA\MyBundle\Form\Model\Shipping',
        ));
}

After upgrading to Symfony 2.7, the form throws the following exception:

An exception has been thrown during the rendering of a template ("The form's view data is expected to be of type scalar, array or an instance of \ArrayAccess, but is an instance of class Proxies\__CG__\BLA\MyBundle\Entity\TransportType. You can avoid this error by setting the "data_class" option to "Proxies\__CG__\BLA\MyBundle\Entity\TransportType" or by adding a view transformer that transforms an instance of class Proxies\__CG__\BLA\MyBundle\Entity\TransportType to scalar, array or an instance of \ArrayAccess.") in MyBundle:Shipping:form.html.twig at line 8.

Setting the data_class seems to work but I wonder why this has not been documented in the upgrade file?

@jakzal jakzal added the Form label Jun 6, 2015
@webmozart webmozart added the Bug label Jun 11, 2015
@webmozart
Copy link
Contributor

Hi @XWB, thank you for reporting this issue! I'm not sure I understand your problem correctly. Could you please upload a fork of symfony-standard that reproduces your problem?

@isabellebruchet
Copy link

Hi,
Same problem here. It's seems to be like that : when you have an entity type with the option "expanded" to true, after upgrading to 2.7 you have an exception (the same as in the previous message). Adding the data_class option fix the problem, but this option is not required when you look in the documentation.

@XWB
Copy link
Contributor Author

XWB commented Jul 6, 2015

@isabellebruchet Fully correct
@webmozart Private project, I cannot upload a fork

@jakzal
Copy link
Contributor

jakzal commented Jul 6, 2015

@XWB we don't need your private project. What we need is your problem reproduced on a fresh Standard Edition project. While preparing it you'll be able to roll out problems specific to your project or configuration, and might be you'll even find the cause of the problem. Surely, it'll be much easier for us to debug the issue.

@rvanlaak
Copy link
Contributor

rvanlaak commented Jul 7, 2015

Since we've updated to 2.7.1 the EntityType with expanded: true does not accept an entity as a default value anymore. A LogicException occurs when the user goes back to edit the entity, where defaultData is added to the builder:

The form's view data is expected to be of type scalar, array or an instance of \ArrayAccess, but is an instance of class Proxies__CG__\AppBundle\Entity\Order\Subject. You can avoid this error by setting the "data_class" option to "Proxies__CG__\AppBundle\Entity\Order\Subject" or by adding a view transformer that transforms an instance of class Proxies__CG__\AppBundle\Entity\Order\Subject to scalar, array or an instance of \ArrayAccess.

$defaultData = array();
if ($order->getSubject() instanceof Subject) {
    $defaultData['subject'] = $order->getSubject();
}

$form = $this->createFormBuilder($defaultData)
    ->add('subject', 'entity', array(
        'class'      => 'AppBundle:Order\Subject',
        'expanded'   => true,
        'property'   => 'name',
    ))
    ->getForm()
;

The exception can be solved in two ways, by setting expanded to false or by defining data_class:

// ...

$form = $this->createFormBuilder($defaultData)
    ->add('subject', 'entity', array(
        'class'      => 'AppBundle:Order\Subject',
        'data_class' => 'AppBundle\Entity\Order\Subject',
        'expanded'   => true,
        'property'   => 'name',
    ))
    ->getForm()
;

Looks like a BC break, think EntityType should resolve the data_class based on the class right?

@jakzal jakzal removed the Unconfirmed label Jul 7, 2015
@pascallapointe
Copy link

You can install the 2.7 branch of my symfony-standard fork:
https://github.com/pascallapointe/symfony-standard.git

Once it's done, add manually some companies and create some employees.

The exception will occur in the 'edit' view once you have linked an employee with a company.
Unlinked entities will not throw any exception.

Hope it will help, thank you in advance for taking a look at it!

@jakzal
Copy link
Contributor

jakzal commented Jul 14, 2015

@pascallapointe thanks for your work. I confirmed your fork reproduces the issue.

@superdav42
Copy link
Contributor

I looked into this a little and found the $viewData set before the exception
is an int when the expanded option is not used but is an object when it is used. I believe this bug was introduced in Refactored choice lists commit
It's a little over my head by probably it should return consistent view data regardless if the 'expanded' option is used. However simply commenting out the LogicExeption allows the code to run with no problems so it's hard to say what the real problem is.

I have had success using this workaround to avoid adding data_class everywhere in my code:

// src/Acme/DemoBundle/Form/Type
namespace Acme\DemoBundle\Form\Type;

use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\OptionsResolver\Options;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Bridge\Doctrine\Form\Type\EntityType;

class AcmeEntityType extends EntityType
{
    public function configureOptions(OptionsResolver $resolver) 
    {
        parent::configureOptions($resolver);

        if(Kernel::MAJOR_VERSION == '2' &&
           Kernel::MINOR_VERSION == '7'  &&
           (int) Kernel::RELEASE_VERSION <= 2) {

            $dataClassNormalizer = function (Options $options, $dataClass) {
                if($dataClass || $options['multiple'] || !$options['expanded']) {
                    return $dataClass;
                }
                return $options['em']->getClassMetadata($options['class'])->getName();
            };

            $resolver->setNormalizer('data_class', $dataClassNormalizer);
        }
    }
}

and add a service definition like:

# src/Acme/DemoBundle/Resources/config/services.yml
    form.type.entity: 
        class: Phoenix\CmsBundle\Form\Type\PheonixEntityType
        tags: [{ name: form.type, alias: entity}]
        arguments: [@doctrine]

This workaround is a rather ugly hack and assumes this will be fixed in 2.7.3.

@adsc
Copy link

adsc commented Aug 11, 2015

Can confirm this issue, setting "expanded" to false removes the error, but is not a solution when you need expanded rendering. The workaround with data_class only helps for fields with multiple = false, otherwise you get the error:

The form's view data is expected to be an instance of class Blabla, but is a(n) array. You can avoid this error by setting the "data_class" option to null or by adding a view transformer that transforms a(n) array to an instance of Blabla

@sustmi
Copy link
Contributor

sustmi commented Aug 31, 2015

Here is a test for the original @XWB's issue: sustmi@57b297e .
Prior to commit 03efce1 ChoiceType used ChoicesToBooleanArrayTransformer view transformer to transform modelData to string.
Since v2.7.0, I guess RadioListMapper is supposed to replace the ChoicesToBooleanArrayTransformer.
The problem is that without the transformer, viewData is filled with an object instead of string. Form::setData() then throws LogicException because it expects:

scalar, array or an instance of \ArrayAccess

as viewData.

@sustmi
Copy link
Contributor

sustmi commented Aug 31, 2015

I can confirm, that you can workaround this issue by setting the data_class option of your choice form to the appropriate class name.

@XWB
Copy link
Contributor Author

XWB commented Sep 2, 2015

Thanks for the test @sustmi

Ping @webmozart

@stof
Copy link
Member

stof commented Sep 2, 2015

but in 2.6, it was possible to have mixed choices (strings and objects), which is not possible anymore because of this data mapper usage.

@XWB
Copy link
Contributor Author

XWB commented Nov 10, 2015

Any updates on this issue? @webmozart

@dmaicher
Copy link
Contributor

Also ran into this issue today after upgrading from 2.6 to 2.7.

Any news? :)

@Einenlum
Copy link
Contributor

Same, any news?

@webmozart
Copy link
Contributor

As far as I understand, the real issue here are that data_class isn't set to class for the entity type. But @stof mentioned an even bigger issues: Choice fields cannot contain mixed objects and scalars anymore.

IMO, the possible good solutions are:

  1. to add a way to specify data_class such that a field can either have that class or a scalar/array
  2. to disable validation of the view data against data_class altogether

@webmozart
Copy link
Contributor

Fixed in #16679.

@XWB
Copy link
Contributor Author

XWB commented Nov 26, 2015

👍

@rvanlaak
Copy link
Contributor

Will it solve setting an entity as default data when expanded is set to true? See #14877 (comment)

@webmozart
Copy link
Contributor

@rvanlaak yes, it should

@rvanlaak
Copy link
Contributor

👍

fabpot added a commit that referenced this issue Nov 26, 2015
…t to null (webmozart)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Disabled view data validation if "data_class" is set to null

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #14877
| License       | MIT
| Doc PR        | -

After this PR, Form::setData() does not validate the view data anymore when "data_class" is set to `null`. This way it is possible to create fields with dynamic view data types (see #14877).

Commits
-------

f495410 [Form] Disabled view data validation if "data_class" is set to null
@fabpot fabpot closed this as completed Nov 26, 2015
@adampastusiak
Copy link

This issue is closed, but I don't know if it really should be because problem @adsc mentioned still exists.
I'm on Symfony 2.7.9 and I have form like this:

$form = $this->get('form.factory')->createNamedBuilder('', 'form')
            ->add('entities', 'entity', [
                'class' => 'Namespace\SomeBundle\Entity\SomeEntity',
                'choice_label' => 'name',
                'label' => 'Label',
                'multiple' => true,
                'expanded' => true,
                'read_only' => true,
                'query_builder' => function (EntityRepository $repository) use ($ids) {
                    return $repository->createQueryBuilder('e')
                        ->where('e.id IN(:ids)')
                        ->setParameter(':ids', array_values($ids))
                        ->orderBy('e.name', 'ASC');
                }
            ])
            ->getForm();

When I try to create view from that form I got 2 exeptions in stack:

TransformationFailedException: Can not read the choices from the choice list.

in
vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/DataMapper/CheckboxListMapper.php at line 55

        try {
            $valueMap = array_flip($this->choiceList->getValuesForChoices($choices));
        } catch (\Exception $e) {
            throw new TransformationFailedException(
                'Can not read the choices from the choice list.',
                $e->getCode(),
                $e

and

ContextErrorException: Warning: spl_object_hash() expects parameter 1 to be object, string given

in
vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php at line 1243

     */
    public function isScheduledForInsert($entity)
    {
        return isset($this->entityInsertions[spl_object_hash($entity)]);
    }
    /**

Of course when you'll try to set data_class you got

The form's view data is expected to be an instance of class Namespace\SomeBundle\Entity\SomeEntity, but is a(n) array.

exactly like @adsc said.

I didn't found workaround for that yet. This is really pain in the ass when you have widget where user can pick specific entities in form (with checkboxes, not multiple select).

Any ideas @webmozart ?

@xabbuh
Copy link
Member

xabbuh commented Feb 9, 2016

@adampastusiak Can you create a new issue if you think that we still have a bug here?

@adampastusiak
Copy link

@xabbuh Yes of course, I just did: #17736

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

No branches or pull requests