Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Injects route parameters into action authorize #308

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CWAscend
Copy link

@CWAscend CWAscend commented Feb 10, 2025

💪 Motivation

Currently, when running the authorize method on an Action, the only variable you can access is the ActionRequest by resolving it from the container via dependency injection. This can sometimes lead to the authorize method becoming messy if we need to access a route-model bound model, as an example, and authorize that a certain action can run based on the state of the particular model.

This example might explain it a bit better:

// Current implementation
class ShowOrder
{
    use AsController;

    public function authorize(ActionRequest $request): bool
    {
        /** @var Order $order */
        $order = $request->order;

        return $order->status->is(OrderStatus::COMPLETE);
    }

    public function handle(Order $order): Response
    {
        // ...
    }
}

// New implementation
class ShowOrder
{
    use AsController;

    public function authorize(Order $order): bool
    {
        return $order->status->is(OrderStatus::COMPLETE);
    }

    public function handle(Order $order): Response
    {
        // ...
    }
}

The ActionRequest can still be resolved via dependency injection, so this isn't a breaking change.

🛠 Changes

  • Pass route parameters into the authorize method, when authorizing the incoming request
  • This allows us to tidy our authorize method, avoiding the need to pluck parameters from the request

🧪 Testing

  • A test has been added to tests/AsControllerWithAuthorizeAndRulesTest.php

🧙 Reminders (Author)

  • Have you read through your own diff? 👀
  • Are all tests passing? 🧪

@CWAscend
Copy link
Author

Might be worth doing the same here as well, but will leave that open for discussion

https://github.com/lorisleiva/laravel-actions/blob/main/src/Concerns/ValidateActions.php#L85-L90

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