Wrong argument type when calling GuardEvent about laravel-workflow HOT 17 CLOSED

Hmmm, yeah, that was originally part of the design to always get the Laravel-style events first and to get the symfony-style events from those. I think it may be better if we change the ZeroDaHero\GuardEvent to extend the SymfonyEvent?

Would that address the issue?

from laravel-workflow.

I've read the code a few times and TBH can't find reason to fire your own events, except for having the option to configure the subscriber using them:

 $events->listen(
            'ZeroDaHero\LaravelWorkflow\Events\EnteredEvent',
            'App\Listeners\BlogPostWorkflowSubscriber@onEntered'
        );

But that actually seems not very useful, because when some is making a guard he needs to know for what specific transition this guard is for, for example. Or when we have leaving event, we need to handle it for the specific workflow. So it kinda seems redundant to me to have all those your own events instead of Symfony original ones.

I suggest if you insist on having them, to keep them along with Symfony ones for dot style confiration.
It will be very easy hack in DispatcherAdapter::EVENT_MAP.

My rationale is that we are using Symfony workflow and would be great to stick with its events instead of taking them over with your own namespaced events, especially when they do not add something on top. Or maybe add configuration option to choose which event should be fired?...

from laravel-workflow.

Moreover, we can't use Symfony's AuditTrail because:

  Symfony\Component\Workflow\EventListener\AuditTrailListener::onLeave(): Argument #1 ($event) must be of type Symfony\Component\Workflow\Event\Event, ZeroDaHero\LaravelWorkflow\Events\LeaveEvent given, called in /Users/sag/Documents/code/zz_api/vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php on line 424

from laravel-workflow.

@klimenttoshkov, you're absolutely right. I'll see if we can remove them altogether, they don't really provide much benefit--especially since the adapter layer is still pretty clumsy.

The only thing I can think of is that the base event has some unique serialization behavior which allows these events to be queued (see #44). I'm not sure if that's still necessary or not with Symfony 6.

from laravel-workflow.

from laravel-workflow.

It doesn't look like the Symfony code has changed much in that regard (the Symfony event has the workflow instance which has transitions which can be closures), so I think we may be partially stuck with both sets of models.

One change we can make though, is to let this packages events extend Symfony's base event--that should pass the type check for the AuditTrail. For the type-hinted subscriber, it's an option in PHP 8 to use a union type-hint in the subscriber, but I would imagine that might mess up the event/listener generation?

from laravel-workflow.

I've pushed up some changes that I think should address this--at least partially. If you get a chance to try out the new branch: https://github.com/zerodahero/laravel-workflow/tree/3.4

from laravel-workflow.

from laravel-workflow.

It is still using the old event:

  App\Listeners\WorkflowTestSubscriber::guard_t2(): Argument #1 ($event) must be of type Symfony\Component\Workflow\Event\GuardEvent, ZeroDaHero\LaravelWorkflow\Events\GuardEvent given, called in /Users/sag/Documents/code/zz_api/vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php on line 424

use Symfony\Component\Workflow\Event\GuardEvent;

...

    public function guard_t2(GuardEvent $event)
    {
        // print "\tGuard T1\n";

        $subject = $event->getSubject();
        // $event->setBlocked(true, 'test blocking reason');
    }

    public function subscribe($events)
    {
        return [
            // 'workflow.straight.*' => 'test',
            // 'workflow.straight.guard.*' => 'guard_t1',
            'workflow.straight.guard.t2' => 'guard_t2',
        ];
    }

In regard to AuditTrail how do you suggest to use it? We also need configuration option to enable it for specific workflow.

from laravel-workflow.

from laravel-workflow.

Hi Zack,

I suggest to heavily modify DispatcherAdapter to the point to just call Symfony events, they have them all:

from laravel-workflow.

@klimenttoshkov We can't dispatch the Symfony events directly since they are not serializeable due to the closures in the Transitions objects--it would break anything dispatching to the queue.

I think we'll need to keep the split for now. I've just merged in #63 and made some more changes from the 3.4 branch. Everything is in develop now, can you give that a try?

A quick summary of the issues here and progress:

• Wrong argument calling GuardEvent: we aren't able to solve this cleanly like the other events since extending the Symfony Guard event directly isn't possible due to the final declaration. The GuardEvent in this package has the same functionality now and extends the Symfony base event (like the others). Unfortunately this means in your listener, you'll either have to use union type hints (on PHP 8.1) or omit the type hint to be able to listen to both events. You could type hint the base event, but as the guard is explicitly what you're looking for, that might be misleading.
• Audit trail: this should be resolved now that all events extend that base event.

Anything else I'm missing from this thread?

from laravel-workflow.

from laravel-workflow.

That's what Symfony said

from laravel-workflow.

Hi,

I know it may be not very relevant. Testing the dev-development, if you call Guard Listener that is Queued (I know this doesn't make sense), the following happens:

[2022-02-21 15:29:58] local.ERROR: Typed property Symfony\Component\Workflow\Event\Event::$subject must not be accessed before initialization {"exception":"[object] (Error(code: 0): Typed property Symfony\\Component\\Workflow\\Event\\Event::$subject must not be accessed before initialization at /Users/sag/Documents/code/zz_api/vendor/symfony/workflow/Event/Event.php:48)
[stacktrace]

Logicwise it is useless to call guard in queue, but IMHO at least it should not be getting fatal error.

from laravel-workflow.

Happy to report that I was able to use AuditTrailListener in the following way:

Defined new subscriber AuditSubscriber:

<?php

namespace App\Listeners;

use Symfony\Component\Workflow\EventListener\AuditTrailListener;

class AuditSubscriber extends AuditTrailListener
{
    public function subscribe($events)
    {
        return parent::getSubscribedEvents();
    }
}

Then subscribed it in EventServiceProvider with:

    protected $subscribe = [
        AuditSubscriber::class,
    ];

from laravel-workflow.

This was addressed in v4.0.0, please let me know if there's still any issues remaining for this.

from laravel-workflow.