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

[11.x] Expand Gate::allows & Gate::denies signature #49500

Conversation

antonkomarev
Copy link
Contributor

@antonkomarev antonkomarev commented Dec 27, 2023

Gate::check method signature was changed in Laravel 5.5:

In reality since then Gate::allows & Gate::denies accepts iterable abilities as well (because they are calling Gate::check under the hood).

public function allows($ability, $arguments = [])
{
    return $this->check($ability, $arguments);
}

public function denies($ability, $arguments = [])
{
    return ! $this->allows($ability, $arguments);
}

public function check($abilities, $arguments = [])
{
    return collect($abilities)->every(
        fn ($ability) => $this->inspect($ability, $arguments)->allowed()
    );
}

I found in many repositories that ability arrays passed to these methods and decided to fix this undocumented inconsistency by allowing arrays as first argument.

Targeted to master because of the contract change.

@driesvints
Copy link
Member

@antonkomarev I don't see a contract change here in the native method signature here so you could just send it in to 10.x 👍

@antonkomarev antonkomarev changed the base branch from master to 10.x December 27, 2023 17:05
@antonkomarev antonkomarev changed the base branch from 10.x to master December 27, 2023 17:05
@antonkomarev
Copy link
Contributor Author

antonkomarev commented Dec 27, 2023

Created separate MR to 10.x branch as @driesvints proposed:

@antonkomarev antonkomarev deleted the extend-gate-methods-signature branch December 28, 2023 17:18
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.

3 participants