Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

entity type with multiple = true should accept an array as well as a Collection #6640

Closed
ianfp opened this Issue Jan 9, 2013 · 6 comments

Comments

Projects
None yet
4 participants
Contributor

ianfp commented Jan 9, 2013

It would be very useful if the entity form type, with "multiple" enabled, could accept an array from the bound object in addition to a Collection. Consider my User class, which is used in the
authentication system:

class User
implements UserInterface, ...
{
    /** @var ArrayCollection<Role> */
    private $roles;

    public function getRoles()
    {
        return $this->roles->toArray();
    }

    public function addRole(Role $role)
    {
        $this->roles[] = $role;
    }

    public function removeRole(Role $role)
    {
        $this->roles->removeElement($role);
    }
}

And my form type for editing the user:

class UserType
extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $builder->add('name', 'text')
            // other fields ...
            ->add('roles', 'entity', array(
                'class' => 'MySecurityBundle:Role',
                'multiple' => true,
                'expanded' => true,
            ))
    }
}

If getRoles() returns an array (as shown above), the form component
throws this exception when I try to change the user's roles:

Expected argument of type "Doctrine\Common\Collections\Collection", "array" given

If I change getRoles to return a Collection, then the authentication
system throws this exception:

Catchable Fatal Error: Argument 4 passed to
Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken::__construct()
must be an array, object given

In addition to consistency with other parts of the system (like the authentication system), another reason for my request is that returning an array instead of a collection is a convenient way to perform a defensive copy, so that clients of the User class do not have access to its internal state.

I have this very same problem, and also rather curious as to how to fix or circumvent it. Anyone out there care to help us out?

UPDATE:
Just found a rather old (2 years!) issue that exactly described this problem: it seems you should make a separate method in your UserEntity like 'getRolesCollection'. This method should be made to only return the ArrayCollection itself. The method will be called if you rename your fieldname in the formbuilder accordingly (in this case 'rolesCollection')

Check out the original issue first; #1909

This fixed the issue for me, though I agree with the original poster; I'm surprised how this is not obvious or made clear somewhere at all, it seems such a common thing to do...

Contributor

gnutix commented Jun 30, 2013

@sarcasme Do you think additional documentation in the security chapter would allow to close this issue ? I'll gladly try to create a PR for it.

That would be great for getting this cleaned up. If you need me to
summarize it for use in the docs let me know, happy tot help when I find
some spare time.
Op 30 jun. 2013 23:07 schreef "Dorian Villet" notifications@github.com
het volgende:

@sarcasme https://github.com/sarcasme Do you think additional
documentation in the security chapter would allow to close this issue ?
I'll gladly try to create a PR for it.


Reply to this email directly or view it on GitHubhttps://github.com/symfony/symfony/issues/6640#issuecomment-20255443
.

Contributor

cordoval commented Dec 3, 2013

@gnutix did you create the PR? could you please reference it back to this ticket?

ping what is the status of this

Contributor

gnutix commented Dec 3, 2013

@cordoval Unfortunately, it's been months since I have post-its on my desk for OSS stuff (including this) and had not found anytime for it. I hope to catch up a little bit during december (will be off work for a while).

But if you want to handle this one, feel free to do so. Otherwise, I'll give my best to close it before Christmas.

Contributor

ianfp commented Dec 3, 2013

This was fixed here:
#9308

@ianfp ianfp closed this Dec 3, 2013

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