[Form] choices_as_value not working properly with associative arrays

[soullivaneuh]

By following the upgrade to 2.7 documentation, I'm changing choice options.

With the given form:

->add('environment', 'choice', [
    'choices'       => [
        'production'    => 'Production',
        'beta'          => 'Beta',
        'development'   => 'Development',
        'binpkg'        => 'BinPkg',
    ],
])

We get this output:

<select>
    <option value=""></option>
    <option value="production">Production</option>
    <option value="beta">Beta</option>
    <option value="development">Development</option>
    <option value="binpkg">BinPkg</option>
</select>

If I try to reverse choices and use choices_as_value option:

->add('environment', 'choice', [
    'choices'       => [
        'Production'    => 'production',
        'Beta'          => 'beta',
        'Development'   => 'development',
        'BinPkg'        => 'binpkg',
    ],
    'choices_as_values' => true,
])

I got this:

<select>
    <option value=""></option>
    <option value="0">Production</option>
    <option value="1">Beta</option>
    <option value="2">Development</option>
    <option value="3">BinPkg</option>
</select>

As you can see, my associated array keys are simply ignored and replaced by numeric values.

[soullivaneuh]

Possibly related to #14350 but not sure.

[Tobion]

If you submit this form I assume the, the correct value, e.g. production is written to the model? Does that work? If so it's just the form values that changed, not the actual data on the model.
Btw, you can also try to use choice_value option.

[soullivaneuh]

@Tobion indeed, the model data is not impacted.

Sorry for that, I was just in trouble with the new render. ;)

Thanks! 👍

[Tobion]

I guess for people that rely on the values in javascript for example, it's still problematic.

[soullivaneuh]

@Tobion it's possible, didn't think about that. Do you want me to reopen this issue?

[soullivaneuh]

I reopen it. This not work with array fields on model.

According to this roles field ORM mapping: https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/Resources/config/doctrine-mapping/User.orm.xml#L35

And the following security roles type:

class SecurityRolesType extends AbstractType
{
    protected $pool;
    protected $authorizationChecker;

    public function __construct(Pool $pool, AuthorizationCheckerInterface $authorizationChecker)
    {
        $this->pool = $pool;
        $this->authorizationChecker = $authorizationChecker;
    }

    public function configureOptions(OptionsResolver $resolver)
    {
        $roles = [];

        // Define the Sonata roles by group
        $adminGroups = $this->pool->getAdminGroups();
        foreach ($adminGroups as $adminGroup) {
            foreach ($adminGroup['items'] as $adminItem) {
                try {
                    $admin = $this->pool->getInstance($adminItem['admin']);
                } catch (\Exception $e) {
                    continue;
                }

                $isMaster = $admin->isGranted('MASTER');
                $securityHandler = $admin->getSecurityHandler();
                $baseRole = $securityHandler->getBaseRole($admin);

                $subRoles = [];
                foreach ($admin->getSecurityInformation() as $role => $permissions) {
                    $completeRole = sprintf($baseRole, $role);

                    if ($isMaster) {
                        $subRoles[$role] = $completeRole;
                    }
                }

                $roles['SONATA ' . $admin->getLabel()] = $subRoles;
            }
        }

        // Define the Symfony roles
        $subRoles = [];
        foreach ($this->pool->getContainer()->getParameter('security.role_hierarchy.roles') as $name => $rolesHierarchy) {
            if ($this->authorizationChecker->isGranted($name)) {
                $subRoles[$name] = $name;

                foreach ($rolesHierarchy as $role) {
                    if (!isset($subRoles[$role])) {
                        $subRoles[$role] = $role;
                    }
                }
            }
        }
        $roles['SYMFONY'] = $subRoles;

        $resolver->setDefaults([
            'choices'       => function (Options $options, $parentChoices) use ($roles) {
                return empty($parentChoices) ? $roles : [];
            },
            'choices_as_values' => true, // TODO: Remove it on SF 3.0, default value.
            'expanded'      => false,
            'multiple'      => true,
            'required'      => false,
            'data_class'    => null
        ]);
    }

    public function getParent()
    {
        return 'choice';
    }

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

This will give numeric again value but we also affect the data model as it's an array.

[soullivaneuh]

@webmozart can we have your opinion about this?

Thanks.

[soullivaneuh]

This not works with complex encapsulated forms, like this twig form override:

{% trans_default_domain 'front' %}
<legend>{{ form.vars.label | trans | ucfirst }}</legend>

{% for choiceFormView in form.type %}
    <div class="form-group {% if form[choiceFormView.vars.value].vars.errors | length %}has-error{% endif %}"
        {% if not form[choiceFormView.vars.value].vars.errors | length %}style="margin-bottom: 35px"{% endif %}>
        <div class="input-group">
            <span class="input-group-addon">
                {{ form_widget(choiceFormView) }}
                <label for="{{ choiceFormView.vars.id }}">
                    {{ choiceFormView.vars.label }}
                </label>
            </span>
            {{ form_widget(form[choiceFormView.vars.value]) }}
            {% if form[choiceFormView.vars.value].vars.label %}
                <span class="input-group-addon">
                    {{ form[choiceFormView.vars.value].vars.label | trans }}
                </span>
            {% endif %}
        </div>
        {{ form_errors(form[choiceFormView.vars.value]) }}
    </div>
{% endfor %}

{{ form_rest(form) }}

form[choiceFormView.vars.value] doesn't work anymore cause value is not an integer and not the real value.

I think it's BC break because if not, I shouldn't have to change my code to get it working, isn't it?

[maximecolin]

I think it's a BC break too, many of my choice type don't work anymore since this update.

[ghost]

I just noticed this also thanks to a commenter on the blog post announcing this feature.

[soullivaneuh]

Any new for this issue? Would be great to solve it before 2.7 stable release.

ping @webmozart @fabpot

[stof]

@soullivaneuh if you want to access the model data, you should use the data var though rather than the value one. The choice value can be different from the choice data even in older versions of Symfony depending on the ChoiceList implementation.

[ninsky]

Same question here: http://symfony.help/viewtopic.php?f=8&t=43

@stof I'm not sure if this issue (described by Soullivaneuh at the very beginning) is considered as a bug in your last post. When there's no option to prevent the values get lost, I would consider it to be one. (In my case, I need the values for JavaScript handling and I don't see a practical workaround at the moment)

[althaus]

Still an issue with 2.7 released. If it's not a BC at the model end, it's still a BC for everything else. Most of the cases, I've got some kind of ID-to-entity-map in the choice fields and I find it hard to read/debug/code if the array just runs with a zero based integer index.

In addition the handling isn't consistent, when you're putting an integer based array into the form as it's then keeping the given keys. 😞

[weaverryan]

@webmozart thoughts? Sounds like there may be a BC break, and some people are having issues with the upgrade.

[weaverryan]

Hi guys!

A few things:

1. From what I can see, the HTML rendering is exactly the same if you simply upgrade, but don't make any other changes (i.e. don't do any of the "flip" refactoring or setting of choices_as_values true). If I'm right, it means that if you're having issues, just leave your code as it was for now. If I'm mistaken (i.e. if simply upgrading is causing some BC changes), please let us know :).

2. If you do refactor by flipping your arrays and setting choices_as_values to true, then yes, the value attributes on your choices does seem to change. But, you can get the old functionality back with the choice_value option. For example:

// Before:
$builder->add('status', 'choice', array(
    'choices' => array(
        'published' => 'Published',
        'unpublished' => 'Unpublished',
        'draft' => 'Draft',
    ),
    'mapped' => false
));

// After:
$builder->add('status', 'choice', array(
    'choices' => array(
        'Published' => 'published',
        'Unpublished' => 'unpublished',
        'Draft' => 'draft',
    ),
    'choices_as_values' => true,
    'choice_value' => function ($choice) {
        return $choice;
    },
));

For me, these both render identical HTML markup. Please let me know what you guys are seeing so we can help make sure others don't hit issues :).

Thanks!

[althaus]

Yes, not changing the form type at all doesn't introduce any BC as the rendered markup doesn't change. That's due to the fact that choices_as_values defaults to false until 3.0. Being a good boy I try to do most of the upgrades asap.

What troubles me: Why do I need to configure a boolean and a closure the get the simplest results you'd expect? That doesn't feel very Symfony like.

In addition the blogpost by @webmozart states the choice_value closure gets two arguments, which seems to be incorrect?

    'choice_value' => function ($allChoices, $currentChoiceKey) {
        if (null === $currentChoiceKey) {
            return 'null';
        }

        if (true === $currentChoiceKey) {
            return 'true';
        }

        return 'false';
    },

Using this results in an exception:

Warning: Missing argument 2 for Acme\DemoBundle\Form\Type\MyType::Acme\DemoBundle\Form\Type\{closure}()

Using your approach with only one argument works flawlessly.

I must admit that I'm working with a more complex test case based using the entity type, but I'd expect that this doesn't matter. I tried the correct choice_value approach (with closure and also with a simple literal id), but then I encountered issues where internal handling differs between integer and string IDs ( 😥 ) and the form children are indexed differently. At the end the only out-of-the-box solution for me is this:

        'choice_name' => function ($allChoices, $currentChoiceKey) {
            return $currentChoiceKey;
        },

No clue why, but this works. It changes the choices' values and the form children are simply 0-based. At least with my form element using multiple and expanded to render checkboxes.

[althaus]

Another observation:

// After:
$builder->add('status', 'choice', array(
    'choices' => array(
        'Published' => 'published',
        'Unpublished' => 'unpublished',
        'Draft' => 'draft',
    ),
    'choices_as_values' => true,
    'choice_value' => function ($choice) {
        return $choice;
    },
));

Without the choice_value closure the form doesn't set the selected value correctly.

[soullivaneuh]

Case of SecurityRolesType was because I made mistake.

Replace $subRoles[$role] = $completeRole; by $subRoles[$completeRole] = $completeRole; solve the problem.

@stof

@soullivaneuh if you want to access the model data, you should use the data var though rather than the value one. The choice value can be different from the choice data even in older versions of Symfony depending on the ChoiceList implementation.

Just replaced, .value by .data on code shown in #14377 (comment), change nothing. Same error. The needed key working on SF 2.6 is not here.

Can you please explain a little bit more?

[althaus]

Any update on this? @webmozart @fabpot

This is somehow a major showstopper for our product release. Especially with #14607 unresolved.

[dimarick]

Default behavior can produce strange bugs, when choice list changes between form get and form post
f.e. you have choice cities, sorted by popularity.
On form get form builds with:

[
  ...
  'choices' => [
    'Moscow' => 123,
    'St. Peterburg' => 124
  ]
  ...
]

If on form post cities order will be changed like

[
  ...
  'choices' => [
    'St. Peterburg' => 124,
    'Moscow' => 123
  ]
  ...
]

User can user can very surprised and frustrated, because he is select Moscow, but factically will be selected St. Peterburg. But QA never known about it, if user do not write to support.

Default behavior never produce undetectable potential bugs. If it is not, then it is very bad.

[craigh]

This ticket isn't solved by documentation, IMO.

[docteurklein]

I think this belongs here, so here is a reproducible test case : https://gist.github.com/anonymous/8d8dd618a7185c24d203

[MichaelMackus]

I agree with @craigh and feel this is a big BC break. I've been running my head into a wall for a day debugging this, and find the proposed "fix" ridiculous (even though it does indeed seem to work)...

       // important if you rely on your option value attribute (e.g. for JavaScript)
       // this will keep the same functionality as before
       'choice_value' => function ($choice) {
           return $choice;
       }, 

From my experience, this doesn't just affect the view layer. I was unable to set the default "data" value of my choice forms to anything other than a "string" and have the option set as checked.

[stof]

@MichaelMackus your proposal does not always work. It depends what $choice is

[MichaelMackus]

@stof not sure what you mean by that, I guess I just find it confusing that this worked in SF ~2.6 but seems broken in >2.7.

For example, if I have: (Symfony >2.7)

[
  'choices' => array_combine(array('Object 1', 'Object 2', 'Object 3'), array(ValueObject1, ValueObject2, ValueObject3)),
  'choices_as_values' => true,
]

Without setting choice_value to the closure in the comment above, if I set the form's data to any of the value objects in the choices array the option does not show as checked in the form.

I'm not sure if this is a separate issue, but seems related from my research.

Curiously, the option stays checked upon form submission, and the correct ValueObject choice gets returned from $form->getData(). It just seems like the initial $form->setData() isn't working properly (maybe something to do with the casting to string in the ChoiceType view transformer, but I'm not familiar enough with the form framework to pinpoint the exact issue).

[webmozart]

@MichaelMackus This should definitely work. Could you please open a new ticket with a reproducible test case?

[webmozart]

By the way, if you pass options that can be cast to strings without generating duplicates, those strings should also be printed in the "value" attributes in the HTML. If that doesn't work for you, please also open a ticket. Thanks!

[MichaelMackus]

@webmozart thanks I've submitted the issue #17939