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

[5.2] If ability has more than one arguments, callbacks receive only first #12305

Merged
merged 1 commit into from
Feb 16, 2016

Conversation

dlnsk
Copy link

@dlnsk dlnsk commented Feb 15, 2016

If ability takes array as argument, before and after callbacks receive only first element throught $arguments

@dlnsk dlnsk changed the title If arguments more than one returns only first [5.2] If ability has more than one arguments, callbacks receive only first Feb 15, 2016
@taylorotwell
Copy link
Member

Can you provide a code sample so I can recreate the issue?

@dlnsk
Copy link
Author

dlnsk commented Feb 16, 2016

In AuthServiceProvider:

    public function boot(GateContract $gate)
    {
        $this->registerPolicies($gate);

        $gate->before(function ($user, $ability, $arguments) {
            \Debugbar::info($arguments);
        });
    }

in PostController:

    public function show(\App\Post $post)
    {
        if (\Gate::allows('update-post', ['one', 'two'])) {
            $dosomething = true;
        }
        return view('post.view', compact(['post']));
    }

@dlnsk
Copy link
Author

dlnsk commented Feb 16, 2016

Debugbar shows one instead of [ 0 => "one", 1 => "two" ]

taylorotwell added a commit that referenced this pull request Feb 16, 2016
[5.2] If ability has more than one arguments, callbacks receive only first
@taylorotwell taylorotwell merged commit 3493475 into laravel:5.2 Feb 16, 2016
@dlnsk dlnsk deleted the patch-1 branch February 16, 2016 17:44
sebdesign added a commit to sebdesign/framework that referenced this pull request Apr 1, 2019
This commit removes some legacy code that was laying around since laravel#12305 was merged.

Before that PR, the arguments were spread out before being passed to the callbacks,
so the user and the ability were merged with the other arguments, before being passed to the callback.

Since then, the arguments are being passed as an array,
so instead of merging the user and the ability with the array of arguments, which is superfluous,
we can directly pass the user, the ability, and the arguments directly to the callback.

This commit doesn't change the existing functionality, and it keeps backwards compatibility.

I have also added some assertions to the tests, since the arguments in the before callbacks were never tested.
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