ksassnowski/venture

Cant get nested Workflows to work?

Closed this issue · 11 comments

I wanted to test out the nested workflows, so I did the following.
Its a typical scenario of document signing.

I have a bunch of dummy jobs that does nothing else than write a line in the log. just to see that it works.

return Workflow::define('Sign a document')
    ->addJob(new PrepareDocumentForSigning($this->document))
    ->addWorkflow(new FirstOrderSigning($this->document), [
        PrepareDocumentForSigning::class,
    ])
    ->addWorkflow(new SecondOrderSigning($this->document), [
        FirstOrderSigning::class,
    ])
    ->addJob(new ExecuteDocument($this->document), [
        SecondOrderSigning::class,
    ])
    ->addJob(new SendExecutedDocument($this->document), [
        ExecuteDocument::class,
    ])
    ->catch(function (\Sassnowski\Venture\Models\Workflow $workflow, $job, Throwable $e) {
        info("Workflow cancelled " .  $e);
        $workflow->cancel();
    });

FirstOrderSigning and SecondOrderSigning are identical and its content is:

class FirstOrderSigning extends AbstractWorkflow
{
    private Document $document;

    public function __construct(Document $document)
    {
        $this->document = $document;
    }

    public function definition(): WorkflowDefinition
    {
        return Workflow::define('Sign a document')
            ->addJob(new SendSigningEmailInvite($this->document))
            ->addJob(new CaptureSignature($this->document), [
                SendSigningEmailInvite::class,
            ]);
    }
}

I run

SignDocumentWorkflow::start($document);

but Horizon outputs only 1 job executed: (the one before the nested workflow)

image

However, if I remove those workflows and only use addJobs - then add 3 jobs get executed.

return Workflow::define('Sign a document')
    ->addJob(new PrepareDocumentForSigning($this->document))
    ->addJob(new ExecuteDocument($this->document), [
        PrepareDocumentForSigning::class,
    ])
    ->addJob(new SendExecutedDocument($this->document), [
        ExecuteDocument::class,
    ])
    ->catch(function (\Sassnowski\Venture\Models\Workflow $workflow, $job, Throwable $e) {
        info("Workflow cancelled " .  $e);
        $workflow->cancel();
    });

image

What am I missing here?

Hm, that's definitely strange, your configuration looks correct. I'll have to try and reproduce this locally.

I'm unable to reproduce this issue locally. I have a hunch about what could be going on, however. Do FirstOrderSigning and SecondOrderJoining have the same jobs, or at least some shared jobs? Can you try and only add the FirstOrderSigning Workflow and see if it works this way?

yes they do.

class FirstOrderSigning extends AbstractWorkflow
{
    private Document $document;

    public function __construct(Document $document)
    {
        $this->document = $document;
    }

    public function definition(): WorkflowDefinition
    {
        return Workflow::define('Sign a document')
            ->addJob(new SendSigningEmailInvite($this->document))
            ->addJob(new CaptureSignature($this->document), [
                SendSigningEmailInvite::class,
            ]);
    }
}
class SecondOrderSigning extends AbstractWorkflow
{
    private Document $document;

    public function __construct(Document $document)
    {
        $this->document = $document;
    }

    public function definition(): WorkflowDefinition
    {
        return Workflow::define('Sign a document')
            ->addJob(new SendSigningEmailInvite($this->document))
            ->addJob(new CaptureSignature($this->document), [
                SendSigningEmailInvite::class,
            ]);
    }
}

Ok, this is the case. If I do not use SecondOrderSigning in the workflow, then everything works.

But this means we cannot use jobs in different scenarios. Im super interested in getting #14 merged in now :D Otherwise it will be super hard to build up more complex scenarios.

Secondly, maybe it could be improved that it does not fail silently. There were no exceptions or any indication what was not working as excepted.

Can you check out the 3.0.0-RC1 tag and tell me if it solves your problem? You will probably have to change your code slightly. I tried to document the changes in the release description on Github. Thanks!

@ksassnowski it works well.

Interesting.. The below also works. So I can add the same nested workflow (FirstOrderSigning) multiple times and give each an unique identifier.

Workflow::define('Sign a document Workflow')
    ->addJob(new PrepareDocumentForSigning($this->document))
    ->addWorkflow(new FirstOrderSigning($this->document), [
        PrepareDocumentForSigning::class,
    ], "batch1")
    ->addWorkflow(new FirstOrderSigning($this->document), [
        "batch1", 
    ], "batch2")
    ->addJob(new ExecuteDocument($this->document), [
        "batch2",
    ])
    ->addJob(new SendExecutedDocument($this->document), [
        ExecuteDocument::class,
    ]);

But how unique is this identifier? If I run it many times, which each WorkflowDefinition knows which "batch1" belongs to which?

Not sure if I understand your question. The id just needs to be unique within the definition of the Workflow. It doesn't matter how many times you then start the workflow. So let's say your code lives in a class like this.

class DocumentSigningWorkflow extends AbstractWorkflow
{
    public function definition(): WorkflowDefinition
    {
        return Workflow::define('Sign a document Workflow')
            ->addJob(new PrepareDocumentForSigning($this->document))
            ->addWorkflow(new FirstOrderSigning($this->document), [
                PrepareDocumentForSigning::class,
            ], "batch1")
            ->addWorkflow(new FirstOrderSigning($this->document), [
                "batch1", 
            ], "batch2")
            ->addJob(new ExecuteDocument($this->document), [
                "batch2",
            ])
            ->addJob(new SendExecutedDocument($this->document), [
                ExecuteDocument::class,
            ]);
    }
}

This would work fine and you can start as many DocumentSigningWorkflows as you want. The ids don't leak outside the workflow instance.

Does this answer your question?

it does. thats great.

Great. I'll leave this issue open until I've tagged a stable version 3.

Version 3 just got released so I'm going to close this issue.