lorisleiva / laravel-actions

⚡️ Laravel components that take care of one specific task
https://laravelactions.com
MIT License
2.52k stars 124 forks source link

AsTransactionObject vs ShouldTransact contract #132

Closed cyrillkalita closed 3 years ago

cyrillkalita commented 3 years ago

This is an idea

@lorisleiva as usual, thanks for a great piece of work.

How, recently I caught myself chaining a lot of Actions and using them one inside the other - and making sure that even if one fails, the whole action is "rolled back"... well database transactions, of course!

So I made a trait to tap() and pipe() the actions with closures, while wrapping them with a DB::transaction() and all works well. but there is a lot of repetitive code, that I don't like.

So would it make more sense to introduce AsTransactionObject trait or ShouldTransact contract on the action?

I don't like the trait idea as it sort of forces you. Interface, on the other hand, will be something that can be added and removed without detriment to the rest.

Would you like a PR to the effect of:

namespace Lorisleiva\Actions\Contracts;

interface ShouldTransact
{
}

and

namespace Lorisleiva\Actions\Concerns;

use Illuminate\Support\Facades\DB;
use Lorisleiva\Actions\Contracts\ShouldTransact;

trait AsObject
{
    /**
     * @return static
     */
    public static function make()
    {
        return app(static::class);
    }

    /**
     * @return bool
     */
    public function shouldTransact(): bool
    {
        return $this instanceof ShouldTransact;
    }

    /**
     * @see static::handle()
     * @param mixed ...$arguments
     * @return mixed
     */
    public static function run(...$arguments)
    {
        $action = static::make();

        if ($action->shouldTransact()) {
            return DB::transaction(fn () => $action->handle(...$arguments));
        }

        return $action->handle(...$arguments);
    }
}
lorisleiva commented 3 years ago

Hi there 👋

That's an interesting idea but I think it is not generic enough to be added to the core. The main reason being, different use cases call for different ways to wrap your actions in DB transactions.

I think the best think to do in such project to avoid repetition would be to create a trait that provides a runInTransaction static method. Something like that:

trait AsObjectInTransaction
{
    public static function runInTransaction(...$arguments)
    {
        return DB::transaction(fn () => static::run(...$arguments));
    }
}

class MyAction
{
    use AsAction;
    use AsObjectInTransaction;
}

MyAction::runInTransaction();
Wulfheart commented 3 years ago

@cyrillkalita maybe you could create a package consisting only of this trait and make it available to other people.

lorisleiva commented 3 years ago

Closing this due to inactivity. 🪴