Skip to content

Conversation

lam0819
Copy link
Contributor

@lam0819 lam0819 commented May 29, 2025

No description provided.

lam0819 and others added 6 commits May 29, 2025 01:53
…flow-engine-laravel into feature/core

* 'feature/core' of https://github.com/solutionforest/workflow-engine-laravel:
  Fix styling

# Conflicts:
#	packages/workflow-engine-core/src/Events/WorkflowCancelled.php
#	packages/workflow-engine-core/src/Events/WorkflowCompletedEvent.php
#	packages/workflow-engine-core/src/Events/WorkflowFailedEvent.php
@lam0819 lam0819 requested a review from Copilot May 29, 2025 11:11
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR renames and migrates the Laravel integration from the old workflow-mastery package to the new workflow-engine-laravel package, updates namespaces, removes legacy action classes, and adjusts documentation and composer metadata accordingly.

  • Renamed package identifiers, namespaces, and composer configuration to workflow-engine-laravel.
  • Removed legacy SolutionForest\WorkflowMastery\Actions classes.
  • Updated core event classes under SolutionForest\WorkflowEngine\Events and refreshed docs and baseline settings.

Reviewed Changes

Copilot reviewed 82 out of 82 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Adapters/LaravelEventDispatcher.php Added new adapter to bridge Laravel’s dispatcher with the core EventDispatcher.
composer.json Renamed package metadata, updated dependencies, autoload paths, and provider alias.
config/workflow-mastery.php Removed old config file for workflow-mastery (needs replacement under new name).
packages/workflow-engine-core/src/Events/WorkflowFailedEvent.php Updated namespace and removed Laravel traits; event class lacks a constructor.
packages/workflow-engine-core/src/Events/WorkflowCompletedEvent.php Added new event class with proper constructor for completion events.
packages/workflow-engine-core/src/Events/WorkflowCancelled.php Updated namespace but class name lacks consistent Event suffix.
phpstan-baseline.neon Cleaned up outdated baseline entries.
docs/migration.md Updated composer update command to new package name.
docs/getting-started.md Updated composer require command to new package name.
docs/best-practices.md Adjusted event listener references to WorkflowFailedEvent.
docs/advanced-features.md Updated listener imports for completed and failed event classes.
docs/README.md Refreshed quick links and badges to point at the new repository and package.
README.md Updated install command and badges for the renamed package.
CHANGELOG.md Updated project name reference in the changelog header.
.github/ISSUE_TEMPLATE/config.yml Updated discussion and security URLs to the new repo paths.
Comments suppressed due to low confidence (3)

packages/workflow-engine-core/src/Events/WorkflowFailedEvent.php:7

  • The WorkflowFailedEvent class has no constructor to initialize $instance and $exception. Consider adding a constructor with property promotion, e.g. public function __construct(public WorkflowInstance $instance, public \Exception $exception) {}.
public WorkflowInstance $instance;

packages/workflow-engine-core/src/Events/WorkflowCancelled.php:5

  • Rename this class to WorkflowCancelledEvent to maintain consistency with other event class names.
class WorkflowCancelled

config/workflow-mastery.php:1

  • The old configuration file was removed but no new config file has been added under the renamed package. Add and publish a new workflow-engine-laravel.php config or adjust the publish tag.
<?php

@lam0819 lam0819 requested a review from Copilot May 29, 2025 11:15
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR renames and restructures the Laravel integration package from workflow-mastery to workflow-engine-laravel, aligns event namespaces, removes legacy action classes in favor of core implementation, and adds a new Laravel adapter for dispatching events.

  • Renamed package coordinates and updated composer.json, autoloads, and badges in documentation
  • Changed event namespaces (SolutionForest\WorkflowMasterySolutionForest\WorkflowEngine) and added a new LaravelEventDispatcher
  • Removed the old action classes (LogAction, HttpAction, etc.) and cleaned up phpstan baseline entries

Reviewed Changes

Copilot reviewed 82 out of 82 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Adapters/LaravelEventDispatcher.php Adds an adapter bridging the core EventDispatcher to Laravel
packages/workflow-engine-core/src/Events/WorkflowFailedEvent.php Updated namespace and stripped traits—but missing constructor
packages/workflow-engine-core/src/Events/WorkflowCompletedEvent.php New event class with constructor
packages/workflow-engine-core/src/Events/WorkflowCancelled.php Renamed namespace and traits removed
composer.json Renamed package, updated autoload and dependencies
docs/* Updated docs for new package name, namespaces, and links
phpstan-baseline.neon Cleaned up outdated baseline entries
Comments suppressed due to low confidence (4)

packages/workflow-engine-core/src/Events/WorkflowFailedEvent.php:7

  • WorkflowFailedEvent lacks a constructor to initialize $instance and $exception, which will leave those properties uninitialized. Add a public constructor that accepts a WorkflowInstance and an Exception and assigns them to the properties.
class WorkflowFailedEvent

packages/workflow-engine-core/src/Events/WorkflowCancelled.php:5

  • [nitpick] The class name 'WorkflowCancelled' is inconsistent with the other events that use the 'Event' suffix. Consider renaming it to 'WorkflowCancelledEvent' to maintain a consistent naming convention.
class WorkflowCancelled

docs/best-practices.md:354

  • The example references $event->workflowName, but WorkflowFailedEvent exposes the WorkflowInstance on $event->instance. Update this to reference $event->instance->workflowName (or the correct property on your WorkflowInstance).
if (in_array($event->workflowName, ['payment-processing', 'order-fulfillment'])) {

src/Adapters/LaravelEventDispatcher.php:22

  • [nitpick] The new LaravelEventDispatcher adapter isn’t covered by unit tests. Consider adding a test that mocks the underlying Dispatcher to verify that dispatch() is called with the correct event.
public function dispatch(object $event): void

@lam0819 lam0819 closed this May 29, 2025
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

Successfully merging this pull request may close these issues.

1 participant