Implicit route model binding in validation methods

[lorisleiva]

Thanks for the super quick reply! I think that solves my issue, but there might be a secondary one.

In every case I've tested in my application, the $event parameter passed to the authorize function is always null. If I check the parameter in the handle function, it is correctly present. Just not in the authorize function!

Full code sample is below, and I am truly puzzled 🙂

class ShowEvent
{
    use AsAction;

    public function authorize(ActionRequest $request, Event $event): bool
    {
        dd($event->id);
        return Gate::check('view', $event);
    }

    public function handle(ActionRequest $request, Event $event): Response
    {
        return Inertia::render('Events/Show', [
            'event' => $event,
        ]);
    }

    public function asController(ActionRequest $request, Event $event): Response
    {
        return $this->handle($request, $event);
    }
}

In this case, dd just shows null.

Originally posted by @Celant in https://github.com/lorisleiva/laravel-actions/issues/137#issuecomment-939339628

---

@lorisleiva I suspect the issue here is that inspectAuthorization() in the ValidateActions concern uses $this->resolveAndCallMethod('authorize'), whereas the ControllerDecorator uses $this->resolveFromRouteAndCall().

The former uses the service container to resolve dependencies and then call the method, whereas the latter does the same but also includes $this->route->parametersWithoutNulls(), meaning handle gets the Event object from the route correctly but authorize doesn't.

I don't know enough about Laravel and this library to know whether it's as simple as just including the parametersWithoutNulls in the arguments passed to resolveAndCallMethod, or if some wider change is needed to support this.

If it is the former, I am happy to try and PR that. Otherwise, I'll need your help on this one. Additionally, It looks like the tests cover controllers with implicit route bindings, as well as authorization and validation, but not both. Maybe this is an edge case that requires a test case? 🙂

[lorisleiva]

Hey 👋 Sorry about the late reply.

Yeah I am aware of this limitation and Laravel Actions currently does not support implicit route model binding in the validation methods such as authorize and rules — hence the lack of test for it. I do realise now that in my previous example, I gave the wrong impression that this would work so sorry about that haha.

Currently, instead of using dependency injection to retrieve your event, you'd need to access it via the Request using the route method like so:

public function authorize(ActionRequest $request): bool
{
    return Gate::check('view', $request->route('event'));
}

Note that for the return value of that method to be an instance of App\Event, it needs to be dependency injected into the handle or asController method. Otherwise, you'll end up with the identifier of the event. Note that regular controllers and form requests behave exactly the same.

Now, I do realise this is not as elegant and intuitive as a direct dependency injection, and it even got me a few times. There are a few reasons that lead me to make that decision:

• In regular controllers, implicit model binding happen only once per request making it easier to understand and debug. When using form requests, the authorize and rules method will not resolve it again for you.
• If we had multiple resolutions of implicit model binding and each injected a different model, these models will be resolved in a certain order which could confuse users.
• I want to make sure Laravel Actions delegates to the same native implicit route model binding to be less intrusive and ensure that this feature is available if and only if they use the SubstituteBindings middleware.

Having said that, I haven't given up on this feature and if you can find a way to go around them, I would happily accept a PR for it. 😊

---

P.S.: I've taken the liberty to extract this in a new issue so it's more searchable for other users.

[Celant]

No worries!

Honestly, that's what I've been using as a workaround, but using $request->route()->parameter('event'); instead of the route method directly, but same-same really.

I think the reasoning for it not doing implicit binding makes total sense, and unfortunately I'm not proficient enough with Laravel to offer an alternative solution! I suspect at the very least, it may be worth some mention in the docs even just as a "gotcha" point, to help anyone else who might run into this as I have. Especially as the docs at a few points do have models passed into authorize via dependency injection, which is what threw me off!

[lorisleiva]

Oh thanks for mentioning it, I hadn't noticed! I'll fix the documentation.

I wouldn't want to just discard this feature though because I do believe it would improve the dev experience. I think a good middle ground would be to simply inject more data when resolving the validation methods from the container but there are two little issues with that:

• A. It won't work if the model you're injecting into the authorize method was not injected in the handle or asController method which can create confusion and force unused variables to be present in the main method.
• B. The logic will have to be different when using the WithAttributes traits because validation at this point could either come from a request or a set of attributes.

I'll just leave this issue open anyway as there might be a better solution out there and this issue is not closed in my mind. 😊