symfony-cmf / create-bundle

UNMAINTAINED - Integrates create.js and createphp into Symfony2
http://cmf.symfony.com/
29 stars 31 forks source link

Custom workflows do not work, there is no subject. #128

Closed UFTimmy closed 9 years ago

UFTimmy commented 9 years ago

Howdy,

I was able to piece together how to make custom workflows by following the the example one provided, but I am getting an exception from DoctrinePhpcrOdmMapper that there is no subject passed in.

This is where the exception gets thrown: https://github.com/flack/createphp/blob/master/src/Midgard/CreatePHP/Mapper/DoctrinePhpcrOdmMapper.php#L98

The issue, I believe, comes from the interaction of the RestService and RestController. For custom workflows, CreatePHP's RestService expects you to pass in the $subject as a parameter. If you don't, it tries to find the subject in the URL.

https://github.com/flack/createphp/blob/master/src/Midgard/CreatePHP/RestService.php#L130-L138

However, the RestController does not pass in the $subject, even though it has the variable, instead it passes in null:

https://github.com/symfony-cmf/CreateBundle/blob/master/Controller/RestController.php#L128

As a workaround, I have had to put the subject as a query parameter in all of my URLs that I define in the workflows, which I don't believe is the intended way.

/**
 * {@inheritdoc}
 */
public function getToolbarConfig($subject)
{
    if ($subject instanceof Page) {
        return [
            'name'   => 'create',
            'label'  => 'Create Page',
            'action' => [
                'http' => ['type' => 'CREATE'],
                'type' => 'http',
                'url'  => $this->getActionUrl($subject),
            ],
            'type'   => 'button',
        ];
    }
}
public function getActionUrl($subject)
{
    $url = $this->router->generate('cmf_create_put_document', ['subject' => $subject->getId()]);
    $url .= '?subject=' . urlencode($subject->getId());

    return $url;
}

Should the $subject be passed from the RestController to the RestService, or am I tackling the creation of workflows in the wrong way (#119)?

If the subject should be passed in, I would be happy to make the change, but I have absolutely zero git experience (svn shop) but I am happy to learn.

Thanks!

dbu commented 9 years ago

i fear i broke this in https://github.com/symfony-cmf/CreateBundle/pull/117/files - looking at the previous deleteAction for example, we seem to have passed the $subject along previously. the only case where we don't is in the post action (which makes sense, when creating a new document there is no subject yet, as subject is the system id).

handleUpdate seems to read the subject from the $data if its not present. i have no idea why the workflow tries to read from $_GET instead. i propose you ask @flack about that - probably it would make sense to have the same code as on L191 also when triggering the workflow.

and if you can do a pull request on the bundle to fix the controller updateDocumentAction to pass on the $subject, that would be great!

regarding git: the cheap way: just browse to the file here on github and click on edit. github will ask you if you want to fork, which you confirm. then you can edit in the browser, save and click the green thing at the top to say "create pull request". if you have more complicated things that you want to try out locally, you can use the Fork button to copy this repository into your namespace on github. then you

git clone ... 
git checkout -b fix-controller-subject
# edit the files and test
git add <modified files>
git commit -m "fixing subject handling in rest controller"
git push origin HEAD -u

and then go to the github website of your fork and you should see a green button to create a pull request - or go into the pull request tab to create the new pr. if you get stuck, just ask for help - i am always keen to help people who want to help with the cmf ;-)

there seems to be some uncertainty in the code what handles what. we have to already load the model to determine the type, but then the RestHandler does not expect the model and fetches it again... but fixing that would be a major refactor involving the library, which seems not that urgent. at least with doctrine, the model will be cached so the operation is very cheap.

UFTimmy commented 9 years ago

Thank you for the very detailed instructions, I have submitted a pull request.

dbu commented 9 years ago

fixed in #129

flack commented 9 years ago

@dbu The $_GET thing is in workflows mainly because of delete which has to be implemented as a workflow in Create.js, and the delete HTTP requests generated by Create.js/backbone do not have a body. So the only way to transmit the subject to be deleted is in the URL. Put requests on the other hand do have a body, so the handleUpdate method does not look for $_GET['subject'], because Create.js will send the correct body anyways. I know it's not the most intuitive setup, but it's ndetermined by JS, and we can't really change that on the PHP side of things