Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.0 #54

Closed
33 tasks done
ksassnowski opened this issue Jul 11, 2022 · 10 comments
Closed
33 tasks done

4.0 #54

ksassnowski opened this issue Jul 11, 2022 · 10 comments
Assignees

Comments

@ksassnowski
Copy link
Owner

ksassnowski commented Jul 11, 2022

The development of version 4.0 happens in the 4.x branch of this repository.

Todo

  • Implement Plugin System
  • Replace vimeo/psalm with nunomaduro/larastan
  • Define WorkflowStepInterface for workflow steps
  • Add gated jobs (see Add support for user task #32 for reference)
  • Add closure-based jobs (see Ability to add closure as a job #24) (968c263)
  • Add testing helpers to run the then and catch callbacks of a workflow (4223133)
  • Add option to run all jobs of a workflow on a specific connection (804193e)
  • Add plugin to make workflows entity aware
  • Add to workflow definition of existing workflow instance
  • Write migration guide
    • Migrating the database
    • Minimum Laravel Version
    • Steps should use WorkflowStepInterface
    • Replace Workflow::define(...) with $this->define(...)
  • Update documentation
    • Update all examples to the new API
    • Writing Plugins
    • Optional EntityAwareWorkflows plugin
    • Testing the then and catch callbacks of a workflow
    • Define queue connection when starting a workflow
    • Asserting that a workflow was started on a specific connection
    • Adding jobs to an existing workflow instance
    • Adding conditional jobs
    • Depending on conditional jobs
    • The callback passed to assertStarted now takes the job's queue connection as a second parameter
    • Adding a closure to a workflow
    • Gated jobs
    • Staring a workflow synchronously
    • Defining the queue connection for all jobs of a workflow
    • Defining the queue for all jobs of a workflow
    • Update section about testing workflows to use WorkflowTester
    • Update section about inspecting workflow models to use state
    • Customizing model states

BC Breaks

This section is used to keep track of all BC breaks this version introduces. While I will try to keep them to a minimum and provide sensible migration paths, there are a few warts in the current implementation that require a BC break to really fix.

Drop Laravel 8 support

Starting with 4.0, Venture will require Laravel 9.

Jobs need to implement WorkflowStepInterface

Since we can't type hint against traits, we currently have to resort to the object type whenever we're dealing with workflow jobs. Version 4.0 adds a proper interface and extends the existing WorkflowStep trait so it automatically implements the interface.

Since it's necessary to add implements WorkflowStepInterface to all existing workflow job classes, this is a BC break. We should be able to automate this with some sort of artisan command, however (e.g. venture:migrate-jobs). We basically have to look for classes that use the WorkflowStep trait and perform some kind of string replace on the class declaration.

I found a way to make this backwards compatible for now. Instead, adding a job that doesn't implement the interface to a workflow will trigger a deprecation warning in version 4.

Workflow::define requires the workflow instance as the first parameter

Up until now, the Workflow facade's define method only took a $name parameter. In version 4, this method takes an instance of AbstractWorkflow as a parameter.

- Workflow::define('My workflow');
+ Workflow::define($this, 'My workflow');

This is so we can acces the actual workflow class inside plugins, instead of only dealing with the WorkflowDefinition class. This allows users to define extra properties/methods on their workflow classes which can then be used by various plugins.

As a convenience, the AbstractWorkflow class now provides a define method which automatically passes the current workflow instance to the Workflow::define method.

- Workflow::define('My workflow');
+ $this->define('My workflow');

Again, this is something we should be able to automate since it should be a rather straight-forward string replace.

Changes to WorkflowManagerInterface::startWorkflow signature

The WorkflowManagerInterface::startWorkflow method now takes a second parameter $connection which specifies the connection to use for all jobs of the workflow. If $connection is null, it will use whatever default connection has been set on the job.

This is a fairly low-impact change as a I think it's unlikely that people will implement their own workflow manager or extend the default one. It is still a breaking change, however.

StepIdGenerator needs to account for WorkflowStepAdapter

Jobs that don’t implement WorkflowStepInterface directly now get wrapped in a WorkflowStepAdapter class. This means that when implementing a custom StepIdGenerator, you now need to account for this class and potentially unwrap it before generating the id.

@Bariskau
Copy link

I'm looking forward to it using with laravel 9. @ksassnowski when do you plan to release?

@ksassnowski
Copy link
Owner Author

ksassnowski commented Jul 28, 2022

There is already a release candidate available that should be stable. I don't expect the actual code to change much until the release so this should be safe to use. In fact, I'm already using it in production.

The only thing that's still left is writing a few sections in the documentation.

@stevebauman
Copy link
Collaborator

Inside the new default WorkflowState classes, the $workflow property is private, so I'm not able to extend the DefaultWorkflowState and adjust one of the methods. Is this intended?

public function __construct(private Workflow $workflow)

@ksassnowski
Copy link
Owner Author

Ah, you're right. It should probably be protected

@stevebauman
Copy link
Collaborator

Ok! Would you like me to submit a PR?

Going through the upgrade as we speak so I'll post here if I find anything else 👍

@ksassnowski
Copy link
Owner Author

Sounds good. You could maybe also double check if the same is true for the DefaultWorkflowJobState

@stevebauman
Copy link
Collaborator

Workflow::getState() is also private, so I'm not able to access it in my extended Workflow:

class Workflow extends VentureWorkflow
{
    public function someMethod()
    {
        // $this->getState()
    }
}

@stevebauman
Copy link
Collaborator

stevebauman commented Aug 11, 2022

Ah shoot sorry! I thought I was working off my own fork, but I pushed into v4 directly for the protected property changes 🙈

2aacd25

Let me know if you'd like this reverted 👍

@ksassnowski
Copy link
Owner Author

It's fine 👍

@ksassnowski
Copy link
Owner Author

@ksassnowski ksassnowski unpinned this issue Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants