lorisleiva/laravel-actions

⚡️ Laravel components that take care of one specific task

laravel-actions's Issues

Additional method for creation

When creating an action I'd like to use the autocompletion of my IDE. However, when using an array I can't use the autocompletion. Any plans to update it?

Im thinking of something similar to a build method.

$action = PublishANewArticle::build(title: 'My blog post', body: 'Lorem ipsum.');

Actions as Laravel Livewire Components

Would you be open to this?

[Advice] Different `handle()` method arguments in create/update actions

Hey!

Opening a new issue as @lorisleiva adviced in #46 because still struggling to find an answer.

The case: I have two action classes - CreateApplication and UpdateApplication. They both have identical rules() and messages() methods, but the latter class has typed $application argument in handle() method, while the former has the same method with no arguments at all. What would be the best way to implement these actions so they share the same rules, but have different amount of arguments in handle() method?

I have few options how this problem can be solved, like moving repeating logic into a trait or instantiating one action class in another and use its logic, but they seem more like an overkill or "bad-practices" options for me. I also don't really like the idea of overriding authorize() method as was suggested in the mentioned issue because it can also have a different number of arguments in both actions.

Thanks in advance for an advice! 😃

How to mock when running the action?

Hi,

Is there anyway to mock an action when using it like this:

$other = ActionName::run([
            'country' => 'US'
        ]);

(This creates an instance of the action, instead of a mocked version)

It works if I do this:

$test = app(ActionName::class);

This is where I setup the mock:

  $this->instance(ActionName::class, Mockery::mock(ActionName::class, function ($mock) {
	   $mock->shouldReceive('run')->once()->with(['country' => 'US'])->andReturn('123456');
   }));

Update route binding resolution for custom and nested bindings

[Self-note]

https://github.com/laravel/framework/blob/7.x/src/Illuminate/Routing/ImplicitRouteBinding.php#L37-L47

https://github.com/laravel/framework/blob/d5d0dd7202f0e2204c6763650959ee5adcfbf15a/src/Illuminate/Database/Eloquent/Model.php#L1567-L1590

Annotation request

This request from @deviouspk started in another issue. In order to not lose focus on the original issue, I'm moving the discussion here. Here is the description of the feature request:

---

What i would love though as extra feature would be something like this. This can be accomplished with doctrine/annotations (which is present in laravel by default if i'm not mistaken). Tell me what you think ;)

Not the best example :p but you get the point. The rules are generated from the attributes. And you have the flexibility to typehint the necessary properties on the class. It will also work great with actions that are used as listeners.

Originally posted by @deviouspk in #3 (comment)

How can I add additional values ​​to the validation data?

In Laravel you could do something like this:

protected function prepareForValidation()
{
        $this->merge([
            'aditional_value_1' => 'Value One',
            'aditional_value_2_using_inputs' => 'Value Two ' . $this->input('field_in_request'),
            'replace_value' => 'New value for replace_value field ' . $this->input('replace_value'),
        ]);
}

Make `handle` method optional

Currently, the handle() method is required on an action - if it is not present, it will cause a BadMethodCallException when trying to use it.

My use case for making this optional is quite simple; it allows actions to be used as a complete replacement for controllers, even if you don't have any logic to handle in the action itself. An action could instead just define a response() method that returns a view or resource.

While the handle() method is required, there are generally 2 options to go, both of which make sense but aren't desirable when keeping things:

• Use a controller (or Route::view() if there is no extra data/validation/authorisation/etc. going into rendering it) to render it, which puts it in a different namespace and structure as the rest of your code
• Define an empty handle() method on the action, which works but is redundant

Cant Access Nova After Installation

After I install this package I can no longer access Laravel Nova. Just returns 404 with no error message.

But everything works fine when I remove library.

[2.0] Inherit auth policy method response message by default

Hey this package is great!

For https://laravelactions.com/authorisation.html, I've noticed custom policy response messages are not inherited e.g. Response::deny('custom denied message').

I had a quick look into this & it seemed like to get them to be inherited by default would introduce a breaking change by introducing an extra parameter to failedAuthorization().

Is this something that could be considered for V2?

Implement injectable ADR responders

Thanks for a great package.

I feel an Action from the package already has many responsibilities (maybe too many), and it would be beneficial to avoid having responses return directly from the Action. In my opinion the Action shouldn't create a response if following SRP. If responses could be implemented as injectable ADR-style Responders, they could be composed and it would be a great addition to the simplification of Action implementation in projects.

Is there any chance that we will see this in a release any time soon?
Or do you have any recommendations on how these could currently be injected and bound to the routes on where the Action is called?

Conflict: must be an instance of Illuminate\Events\Dispatcher, instance of LorisLeiva\Actions\EventDispatcherDecorator

In combination with https://github.com/musonza/chat after update to v0.1.3 this exception is thrown. Seems to be like a conflict with the new pushed decorator?

How could you configure or fix this class problem?

Argument 1 passed to Musonza\Chat\Eventing\EventDispatcher::__construct() must be an instance of Illuminate\Events\Dispatcher, instance of LorisLeiva\Actions\EventDispatcherDecorator given {"userId":298,"exception":"[object] (Symfony\\Component\\Debug\\Exception\\FatalThrowableError(code: 0): Argument 1 passed to Musonza\\Chat\\Eventing\\EventDispatcher::__construct() must be an instance of Illuminate\\Events\\Dispatcher, instance of LorisLeiva\\Actions\\EventDispatcherDecorator given at /data/www/vendor/musonza/chat/src/Eventing/EventDispatcher.php:12)

d

Actions as listeners are run as 'object'

When actions are registered as event listeners, the runAsListener() method is never called, and the action runs as 'object'. This also means that attributes are not collected from the event.
Apparently Laravel's EventDispatcher resolves the Action and calls the handle() method directly.

Not sure if this is a bug, or I'm misunderstanding something.

Define available methods on base Action class

A lot of optional methods don't exist on the base Action, and are only called if a method_exists('methodNameAsMagicString') check returns true.

This makes it impossible for an IDE like phpStorm to know that these methods are available, so they're not listed when searching for methods, or using the Override Methods tool.

If it's possible, I suggest defining all available methods on the Action class, preferably with PHPDocs describing argument types and return type.

I won't mind creating a pull request if you're interested.

[Vote] Register routes directly in Actions

Add a field on each action where routes can be defined and so everything can be setup into the action and no need to create routes or controllers its just create Activity define everything and good to go!

Method ReflectionParameter::getClass() is deprecated

Hello,

I received the "Method ReflectionParameter::getClass() is deprecated" error while working PHP 8.

I'm not sure about what I supposed to do. The answer described here: https://php.watch/versions/8.0/deprecated-reflectionparameter-methods#:~:text=Deprecated%3A%20Function%20ReflectionParameter%3A%3AgetClass,are%20in%20a%20single%20Union.

execute validation before authorization

Hi,

I think it makes more sense to execute the method

$this->resolveValidation();

before the

$this->resolveAuthorization();

Since you often need to rely on the input to do an authorization.

Can we switch these methods? like this:

    public function run(array $attributes = [])
    {
        $this->fill($attributes);
        $this->resolveBeforeHook();
        $this->resolveValidation();
        $this->resolveAuthorization();

        return $this->resolveAndCall($this, 'handle');
    }

Consider using expectsJson instead of wantsJson when action runs as controller

https://laravelactions.com/actions-as-controllers.html#returning-http-responses

Since wantsJson only checks for Accept header but expectsJson does support ajax request and most likely clients expect JSON format on ajax request as well

PHP 8.0 Compatibility Issue

Method ReflectionParameter::getClass() is deprecated

https://php.watch/versions/8.0/deprecated-reflectionparameter-methods#:~:text=Deprecated%3A%20Function%20ReflectionParameter%3A%3AgetClass,are%20in%20a%20single%20Union.

vendor/lorisleiva/laravel-actions/src/Concerns/ResolvesMethodDependencies.php:38

protected function resolveDependency(ReflectionParameter $parameter, $extras = [])

    {

        list($key, $value) = $this->findAttributeFromParameter($parameter->name, $extras);

        $class = $parameter->getClass();

onSuccess & onFail methods

Hi!
I would like to have optional onFail & onSuccess methods on the action class.

This could be very useful to fire events. or rollback certain things.
I was thinking about something like this:

    public function run(array $attributes = [])
    {
        $this->fill($attributes);
        $this->resolveBeforeHook();
        $this->resolveAuthorization();
        $this->resolveValidation();

       return $this->execute();
    }

    protected function execute()
    {
        try {
            $value = $this->resolveAndCall($this, 'handle');
        } catch (\Throwable $exception) {
            if (method_exists($this, 'onFail')) {
                $this->onFail();
            }
        }
        return tap($value, function ($value) {
            if (method_exists($this, 'onSuccess')) {
                $parameters = (new \ReflectionMethod($this, 'onSuccess'))->getParameters();
                empty($parameters) ? $this->onSuccess() : $this->onSuccess($value);
            }
        });
    }

Class hash does not exist

I'm getting this Class hash does not exist when running composer update.

Laravel 5.8: Failing
Laravel 5.7: Successful

Found out it was this package with:

"extra": {
    "laravel": {
      "dont-discover": [
        "lorisleiva/laravel-actions"
      ]
    }
  },

Specs:
Laravel Framework 5.8.19

Feature Request: Force JSON responses for all actions (configurable).

I think maybe something like

protected function allowedResponses(){
  return config('actions.response_types', ['html', 'json']);
}

which if the config defaults to only json for example then failedValidation would look something like:

protected function failedValidation()
    {
       ...
       if(
           count($this->allowedResponses() === 1) 
          && in_array('json', $this->allowedResponses()
       ){
            $errors = (new ValidationException($this->validator))->errors();
            throw new HttpResponseException(
                response()->json([
                        'errors' => $errors
                    ], 
                   JsonResponse::HTTP_UNPROCESSABLE_ENTITY
               )
           );
        }
       ...

I've implemented this myself for an API by just overwriting the failedValidation, so it doesn't redirect, and extending the Action class, but it would be neater if it was just baked into the original class. There could be a better way of doing this as well, and there's also the option of forcing the middleware to change headers to add 'accept: application/json' but I felt that was a bit messy.

isset($this->{parameter}) returns false

Maybe __isset() is not defined?

Attributes dynamically resolved from handle method do not use default value

Consider the following action.

class MyAction extends Action
{
    protected $getAttributesFromConstructor = true;

    public function handle($myAttributeWithDefaultValue = true)
    {
        return $myAttributeWithDefaultValue;
    }
}

Current behaviour.

MyAction::run();
// => null

Expected behaviour.

MyAction::run();
// => true

Root cause: default value is hardcoded to null here.

laravel-actions/src/Action.php

Lines 77 to 79 in ef190a2

	foreach (Arr::wrap($attributes) as $index => $name) {
	$this->set($name, Arr::get($arguments, $index, null));
	}

laravel 8 support

Add method errors in action class

First of all congratulations for the awesome package! It's very useful!

I would like to suggest adding an errors() method in Action class. Some times we would like to manipulate validation errors manually instead of throw a ValidationException.

For example:

public function errors(): MessageBag
{
    return $this->getValidatorInstance()->errors();
}

I've created a child Action class that extends from yours and added this method. It worked just fine.
With this method we could do something like this:

$action = new MyAction(['someting' => 'invalid']);

if ($action->errors()->any()) {
    $user->notify(new VeryBadErrorOccurred($action));

    throw new MyCustomException($action->errors()->messages());
}

What do you think? Do you think this is something simple and nice to implement? I would appreciate it.

Thank you!

Unintuitive dependency injection behavior in the authorize method

The authorize method support dependency injection of models resolved through route model binding.
Since this is a feature that's usually used in Controllers, I expected it to behave in a similar way, but it doesn't.

Example:

public function authorize(Request $request, Customer $customer)
{
    /*
     * If the "customer" attribute isn't provided, the injected $customer parameter
     * is an empty instance of Customer (no id or any other attributes).
     */
}

public function rules()
{
    return [
        'customer' => 'required|exists:customers,id',
    ];
}

The missing customer attribute isn't caught by the validation rule, since that is checked after authorization.

The behavior I expected was that since (1) the $customer parameter is mandatory and (2) a Customer couldn't be resolved, an exception should have been thrown. Had the $customer parameter been optional, I would have expected a null value, and not an empty model instance.

[Command] make:action directory/filename

Hi 👋🏻
Thank you for your amazing work on this repo 🤙🏻

I notice that the command make:action (name) support only the filename and there isn't a way to pass also the namespace.

Example:

php artisan make:action User\UserProfile

Result -> App\Actions\UserUserProfile

Is there a way to implement this functionality? 😀

Possible lumen support

Is the package compatible with lumen? Have you ever thought about support for this framework?
Incidentally very good work ;)

Use stubs to generate Actions through the make command

Just opening an issue to make sure I don't forget to use Laravel 7's new stub feature for generated Actions.

Method to transform attributes before validation

I think this is something nice to add for extra optimisation and keeping things DRY, I would like to create a PR if it's something you want to add too.

I often find myself needing to transform some user input before and after validation. After validation can be perfectly done in the handle method or some other method you can create yourself.

But I'm struggling a bit with the before validation input, because when you have rules setup for validation, you don't reach the handle method. Now I can validate the data myself inside the handle method, but then I find myself creating a validatingInput method which calls the validate function to begin with. It's not really dirty, but it's also not clean - what seems to be the aim of this package.

I would suggest to have a transform method which can transform the specified attributes within an action, before validation. For example, I want to add a slug attribute and check this value against the database if it already exists or not.

public function transform()
{
    return [
        'slug' => str_slug($this->name),
    ];
}

public function rules()
{
    return [
        'slug' => 'required|unique:posts,slug',
   ];
}

I hope I explained it well enough :)

Route Model Binding returns new Model instance instead of null

I am using the latest version of this awesome package and have a question if the Route Model Binding can return new Model instance instead of null.

The reason because when we type-hinting the Model we expect to retrieve that Model even if it does not exist. Let's think of the NullObject pattern. By doing this inside the action logic we don't need to check whether the Model is null.

Currently, I am doing the following:

public function handle(User $user, ?Role $role): User
{
        $user->update([
                'role_id' => optional($role)->id
                // could be more checks for other attributes/models
        ]);

        // Potentially check more afterwards

        return $user;
}

I am happy to submit the simple pull request if you guys consider this question.

Thanks again for this great package!

Calling 'run' multiple times on the same instance will ignore arguments

when calling run for the second time on the same Action instance, ex: $action->run($params) , it will be executed with the same arguments as when called for the first time, even if we are passing totally different values.

Job serialization for models

When queuing an action as a job the actions data is not serialized like we would see by default in a Laravel job, I believe this is due to attributes being an array and Laravel's SerializesModels trait only looks at the top level property and not at the values of an array.

To achieve this I believe we could loop through the attributes array during the Laravel Actions SerializesModels __sleep method and apply the same logic Laravel does for each of those properties, same applies for the__wakeup method to, for rehydrating this data.

How to inject dependency on the action before the handle method?

Great work, love working with it. I recently came across this hurdle and I am not sure how to solve it. Is there a way to inject a dependency before the handle method is invoked? I added the withValidator method to the action and it requires an instance of class to perform validation. I went through the docs and I could find a way to inject a dependency that is accessible at the time of validation. Could you please let me know if it is possible and how to do it?

Delegating an action doesn't keep the `actingAs` user

As the title suggests, when you delete an action within an action after using actingAs the user isn't passed through.

(new TopLevelAction)->actingAs($user)->run();

class TopLevelAction extends Action 
{
 public function authorise()
 {
   $this->user(); // User object
   $result = $this->delegateTo(DelegatedAction::class);
 }
}

class TopLevelAction extends Action 
{
 public function authorise()
 {
   $this->user(); // null
 }
}

From a quick look at the code, perhaps something like this can be added to the extended Action class

    /**
     * @param Action $action
     * @return static
     */
    public static function createFrom(Action $action)
    {
        return (new static)->fill($action->all())->actingAs($action->user());
    }

Happy to do a PR if you think this worth adding in, otherwise I can just override in my app :)

Best practice - using constructor to pass typed objects or not?

I really like the approach I can take with this package. Thank's for your work!

However, whenever possible, I would like to pass (typed) objects to my actions instead of an array of attributes. It just makes development - especially with the IDE - easier. I don't have to look into the action calls to know what is needed.

So, what is the suggested way to pass objects to the Action? Using a constructor with a docblock for the the run action - something like this:

/**
 * @method static run(Person $person)
 */
class PersonAction extends Action
{

    /**
     * @var Person
     */
    public $person;

    public function __construct(Person $person)
    {
        $this->person = $person;

        parent::__construct();
    }
    
    public function handle()
    {
        $person = $this->person;
        
        [...]

    }
}

? Or is there a better way?

Events are not fired

I installed this package, and my listeners stopped working. After removal of package everything returned back normal.

PhpStorm shows unsuppressable error for $commandSignature

This isn't really an issue with the package, but I though it was worth mentioning that PhpStorm has suddenly started complaining about Action subclasses that override the static $commandSignature property from the base Action class (via the RunsAsCommand trait).

I'm not exactly sure where the error comes from. I've tried disabling all inspections, and it's still there.

I'll submit a bug report to Jetbrains, but before I do that, I'd like it if someone else could confirm that this is an issue, and that it's not illegal to override static properties from an inherited trait?

I'm running PhpStorm 2020.3 on a Mac. The project is PHP 7.4, and the php binary in my $PATH is also 7.4. I do have php 8.0 installed though, but like I said, it's not in the $PATH.

[Advice] Update action

Hi there,

Currently I am considering this approach. Any side effects? Any better approach?

I am targetting better readbilitty when calling a action, by passing parameters through constructor, instead just setting a "magic" $action->tournament = $tournament

Controller

 public function update(Request $request, Tournament $tournament)
    {
        $action = new UpdateTournamentAction($tournament, $request->all());

        $action->run();

        return redirect()->to("/tournaments/{$tournament->id}/participants");
    }

UpdateAction

  

class UpdateTournamentAction extends Action
{
    private Tournament $tournament;

    public function __construct(Tournament $tournament, array $attributes = [])
    {
        $this->tournament = $tournament;
        $this->fill($attributes);
    }

    public function authorize()
    {
        return $this->can('update', $this->tournament);
    }

    public function rules()
    {
       // complex rule here
    }


    public function handle()
    {
        $this->tournament->fill($this->validated());
        $this->tournament->save();

        return $this->tournament;
    }
}

User update validation rules problem

He, what I'll mention is not an "issue" I must say. If the message not proper just delete it. When I want to update a user with LaravelActions, I want to use some classic validation rules like email and username that must be unique except for the user's own email and username. But model binding not there while in rules() function. If I use middleware to get the user before validation then I have two same DB queries. What should I do?

$timeout not respected when running Actions as Jobs

When running actions as jobs, public $timeout defined inside the class, doesn't seem to be respected by the worker

Ignore `null` for command output

When using an action as a command, the default behaviour, without the consoleOutput method, is to output the return value of the handle method.

In a lot of cases, at least in my uses, there is no return value and then the command just outputs null without formatting causing line inconsistencies, it'd be great if the output was ignored when null, as it's a nothing value anyway, and nothing output to the console.

Class App\Listeners\EventListener does not exist

Not sure why this is popping up. Fatal error appears after running composer require.... Rest of composer.json below:

"php": ">=7.1.3",
"laravel/framework": "5.7.*",
"ramsey/uuid": "^3.0.0",
"doctrine/dbal": "^2.5",
"league/fractal": "^0.17.0",
"laravel/tinker": "^1.0",
"fideloper/proxy": "^4.0"

[Suggestion] Add custom messages function

I had a situation where I needed to set custom messages for my validations, and I wish there was an easier way to set them (even though the way I did it was pretty easy).

This is what I did:

public function rules()
{
    return [
        'name' => 'required|unique:subdomains,name|min:4|alpha_dash',
        'token' => 'required'
    ];
}

public function withValidator($validator)
{
    $validator->setCustomMessages([
        'name.unique' => 'This subdomain already exists.',
    ]);
}

This is what I would propose:

public function rules()
{
    return [
        'name' => 'required|unique:subdomains,name|min:4|alpha_dash',
        'token' => 'required'
    ];
}

public function customMessages()
{
    return [
        'name.unique' => 'This subdomain already exists.',
    ];
}

An alias (serializeFromBaseSerializesModels) does not exist

When using versions 1.2 and higher on Laravel 5.8, I am getting the error
An alias (serializeFromBaseSerializesModels) was defined for method __serialize(), but this method does not exist in the file src/Concerns/SerializesModels.php line 7.

I was however able to get things to work by using 1.1.8.
I looked over the docs to make sure I had what was needed, so not too sure if the Laravel version is outdated for 1.2 and higher or just an error with the package.

Actions to use repository pattern

Such a great package. Really helps. I just wanted to ask if its possible for the action to use repository pattern.

How to respond with ajax when validation fails?

I'm trying to catch validation exceptions, and return them as ajax responses. But I can't find a way to do it.

Is it possible?

[2.0] Register routes directly in Actions

Hello & happy new year!

I noticed that v2 doesn't yet have the ability to define routes inside the action; #3.

Your routes(Router $route) hook implementation was spot on IMO.

For context I'm a heavy user of this... to the point where routes/web.php is empty!

I was interested to hear your stance on this being included in v2?