Danielss89 / ZfcUserAdmin

An admin interface for ZfcUser
36 stars 45 forks source link

Method for updating Entity not designed for using ManyToMany annotation #30

Open webdevilopers opened 11 years ago

webdevilopers commented 11 years ago

I am extending my User entity with an attribute using a ManyToMany association. In my example I use "costCenters" but you can think of it as the already existing "roles" attribute.

User entity:

    /**
     * @var \Doctrine\Common\Collections\Collection
     * @ORM\ManyToMany(targetEntity="Application\Entity\Role")
     * @ORM\JoinTable(name="user_role_linker",
     *      joinColumns={@ORM\JoinColumn(name="user_id", referencedColumnName="id")},
     *      inverseJoinColumns={@ORM\JoinColumn(name="role_id", referencedColumnName="id")}
     * )
     */
    protected $roles;

    /**
     * @var \Doctrine\Common\Collections\Collection
     * @ORM\ManyToMany(targetEntity="Application\Entity\CostCenter")
     * @ORM\JoinTable(name="user_costCenter",
     *      joinColumns={@ORM\JoinColumn(name="user_id", referencedColumnName="user_id")},
     *      inverseJoinColumns={@ORM\JoinColumn(name="costCenter_id", referencedColumnName="id")}
     * )
     */
    private $costCenters;

    public function getCostCenters() {
        return $this->costCenters;
    }

    public function setCostCenters($costCenters) {
        $this->costCenters = $costCenters;
    }

Extending the form in my _Module.php_:

    public function onBootstrap($e)
    {
        $app = $e->getParam('application');
        // $em is a Zend\EventManager\SharedEventManager
        $em  = $app->getEventManager()->getSharedManager();

        $em->attach('ZfcUserAdmin\Form\EditUser', 'init', function($e) {
            // $form is a ZfcUser\Form\Register
            $form = $e->getTarget();

            $sm = $form->getServiceManager();
            $om = $sm->get('Doctrine\ORM\EntityManager');

            #$form->setHydrator(new \DoctrineORMModule\Stdlib\Hydrator\DoctrineEntity($om, 'Application\Entity\User'));

            $form->add(array(
                'name' => 'costCenters',
                'type' => 'DoctrineModule\Form\Element\ObjectMultiCheckbox',
                'options' => array(
                    'label' => 'Access to costcenters',
                    'object_manager' => $om,
                    'target_class'   => 'Application\Entity\CostCenter',
                    'property'       => 'id',
                ),
            ));
        });
    }

I already inserted some rows in my database and the edit form is populated correctely with these existing costcenters.

When editing and saving I get these warnings / errors: **Warning: spl_object_hash() expects parameter 1 to be object, string given in /vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php on line 1375***

exception 'Doctrine\ORM\ORMException' with message 'Found entity of type on association Application\Entity\User#costCenters, but expecting Application\Entity\CostCenter' in /vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php:783

I think the configuration of entities and form are correct. It looks like the method to update the user entity in Service/User.php l. 78 ff. causes a problem:

        // first, process all form fields
        foreach ($data as $key => $value) {
            if ($key == 'password') continue;

            $setter = $this->getAccessorName($key);
            if (method_exists($user, $setter)) call_user_func(array($user, $setter), $value);
        }

My posted value for costCenters will be an array of strings including the IDs. It looks like the entity manager will ony receive these strings since it doesn't know they have to be "hydrated" to find the related objects / entities.

Normally I would bind the object to Zend_Form and then a hydrator will do this in order to correctely persist the entity, e.g.:

        $id = /*(int)*/ $this->params('id', null);

        $em = $this->getEntityManager();

        $timeDataStatus = $em->find('Application\Entity\TimeDataStatus', $id);

        $formManager = $this->serviceLocator->get('FormElementManager');
        $form    = $formManager->get('CreateTimeDataStatus');
        $form->bind($timeDataStatus);

        $request = $this->getRequest();
        if ($request->isPost()) {
            $form->setData($request->getPost());

            if ($form->isValid()) {
                $em->persist($timeDataStatus);
                $em->flush();
            }
        }

Since saving multiple roles will probably be a use case for future admin forms can we achieve a solution here?

webdevilopers commented 11 years ago

Here is a possible fix for the ZfcUserAdmin\Form\CreateUser:

Remove the Zend\Stdlib\Hydrator\ClassMethods inside createAction of UserAdminController in l. 51:

            #$form->setHydrator(new ClassMethods());
            #$form->setHydrator(new DoctrineHydrator()); // moved to Module.php onBootstrap event listener

Set the hydrator inside the onBootstrap method of Module.php:

            $form->setHydrator(new \DoctrineORMModule\Stdlib\Hydrator\DoctrineEntity($om, 'Application\Entity\User'))
                ->setObject(new \Application\Entity\User());

The User entity must have the add and remove methods:

    public function addCostCenters(Collection $costCenters)
    {
        foreach ($costCenters as $costCenter) {
            $this->costCenters->add($costCenter);
        }
    }

    public function removeCostCenters(Collection $costCenters)
    {
        foreach ($costCenters as $costCenter) {
            $this->costCenters->removeElement($costCenter);
        }
    }

This fix allows storing the ManyToMany entities.

Unfortunately this will not work for the ZfcUserAdmin\Form\EditUser because there is no hydrator passed inside the editAction.

webdevilopers commented 11 years ago

Here is a fix for saving ManyToMany entities using the ZfcUserAdmin\Form\EditUser:

Service/User.php l. 78 ff.

    public function edit(Form $form, array $data, UserInterface $user)
    {
        // first, process all form fields
//         foreach ($data as $key => $value) {
//             if ($key == 'password') continue;

//             $setter = $this->getAccessorName($key);
//             if (method_exists($user, $setter)) call_user_func(array($user, $setter), $value);
//         }
        $user = $form->getData(); // same procedure as create()

Unfortunately the additional data from form is no longer added. We need a new setter process here.

webdevilopers commented 11 years ago

Correcting myself: The user data IS set, except the displayName - but this possibly is an issue with my config. I will check that tomorrow.

webdevilopers commented 11 years ago

Summary: Both workarounds above work to enable Doctrine hydration for custom ManyToMany relationships.

Except: Both workarounds using hydration exclude the getAccessorName methods. This causes non-setting values on form elements that DO NOT MATCH the user entities properties f.e. "display_name" should be "displayName".

There is an issue about these naming conventions in ZfcUser which would solve this problem: https://github.com/ZF-Commons/ZfcUser/issues/261

If these naming conventions are accepted then extending the forms should follow them: https://github.com/ZF-Commons/ZfcUser/issues/233

webdevilopers commented 11 years ago

Listed below are some issues that may relate or help to solve this topic.

Custom hydration for User entity requested here: https://github.com/ZF-Commons/ZfcUser/pull/174

Using hydrators: https://github.com/ZF-Commons/ZfcUser/issues/252

Using different hydrators than ClassMethods: https://github.com/ZF-Commons/ZfcUser/issues/157

webdevilopers commented 11 years ago

Are there any plans for refactoring the update process for an edit form to make it work?

webdevilopers commented 10 years ago

If the ZfcAdmin module will merge the _zfcuser_userhydrator option should we implement this feature to ZfcUserAdmin?

Adapting the hydrator on the CreateUser and EditUser form should fix this and other issues, @Danielss89 Issue https://github.com/Danielss89/ZfcUserAdmin/issues/31 Adding custom form elements with Doctrine mapping / relationships fail

tad3j commented 10 years ago

Was wondering, does this mean that ManyToMany relationship won't work with current version? I have some problems where hydration doesn't seem to work and the ArrayCollection returned to entity is always empty, although the data is set inside form and form is valid. Is this related?

Edit:Just tried out and I'm getting similar error inside UnitOfWork.php for Edit action.

webdevilopers commented 10 years ago

Hi @tad3j , regarding my issues I think nothing has changed. I believe @Danielss89 is very busy. Some of my issues are about a year old. But since a lot of things happened with ZfcUser in the last months it is hard to maintain this module and keep up with the changes.

At least for some of my issues there seems to be a fix using the zfcuser_user_hydrator option and change some lines of code as you can see in my latest PR https://github.com/Danielss89/ZfcUserAdmin/pull/50/files .

Please tell me if this works for you.

tad3j commented 10 years ago

Hey @webdevilopers, thanks for your reply. I've noticed that this issue is old, and yes, was thinking the same. I think they are re-factoring ZfcUSer, so that might be the cause this module isn't moving forward.

I've tried the code but I'm still experiencing the same problems (Hydrator doesn't hydrate ManyToMany select element and I get an error when submitting the edit form). So unfortunately it doesn't. :/ Btw, I believe that your code also populates password field with the encrypted password on edit form.

webdevilopers commented 10 years ago

Guess it's best to wait for the next versions.

BTW: what error did you get?

tad3j commented 10 years ago

I think I'm getting the same error as you (on edit form):

Warning: spl_object_hash() expects parameter 1 to be object, string given in Doctrine\ORM\UnitOfWork

it's just that it's on line 1388.

Edit: I've found out why this is happening...In my case ObjectSelect gets hydrated as a number(string) that's why error is thrown when we try to flush that. Temporary solution is to manually find references to those ids and re-add those custom objects/entites.

tad3j commented 10 years ago

@webdevilopers, have you tried to use DoctrineModule\Stdlib\Hydrator\DoctrineObject as DoctrineHydrator instead of ClassMethods one? I think this should solve the problem where you pass string/integer to your enitity, since this one does hydration properly. I've just tested this on Create form and it works great; before that I had to manually "connect" all the related data but Doctrine's hydrator does the job for me now. That's why I'm starting to think that hydrator should be configurable inside this module. Could anyone confirm? Edit: I've tried to do something similar on EditForm, but it's a bit more complicated there, so I had no success. :( Edit2: After taking one step back and trying again I was able to get ManyToMany working without editing the source code of this module. Maybe it's not the cleanest way but I think it's one of a few options (I'm adding relations manually, where they are not available I use doctrines $entityManager->getReference(...)).