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.4] Add the method methodsWithoutModels in AuthorizesRequests Trait. #18916

Merged

Conversation

mathieutu
Copy link
Contributor

@mathieutu mathieutu commented Apr 24, 2017

Hey guys!

In this trait we have the resourceAbilityMap() which allows us to add directly in controller some methods to check in the authorize* methods.

Actually we can't add methods which doesn't need any parameter, because the setting of $modelName is done with a hard coded array.

I added this method to fix that.

Example of use :

// Untouched, already usable and overridable in controller. 
protected function resourceAbilityMap()
{
    return [
        'index'      => 'list',
        'datatables' => 'list', // Here, a new resource method.
        'show'       => 'view',
        'create'     => 'create',
        'store'      => 'create',
        'edit'       => 'update',
        'update'     => 'update',
        'destroy'    => 'delete',
    ];
}
// New, have to add it to allow devs to override this array.
protected function methodsWithoutModels()
{
   // This new resource method doesn't need any parameter, so I can add it here.
    return ['index', 'datatables', 'create', 'store'];
}

What do you think about that ?

Thx.

@m1guelpf
Copy link
Contributor

@mathieutu Could you add some tests for this?

@tillkruss tillkruss changed the title Add the method methodsWithoutModels in AuthorizesRequests Trait. [5.4] Add the method methodsWithoutModels in AuthorizesRequests Trait. Apr 24, 2017
@mathieutu
Copy link
Contributor Author

mathieutu commented Apr 24, 2017

Actually, this is already tested in \Illuminate\Tests\Auth\AuthorizesResourcesTest, because I've just pulled out the array in an other method.

Something I could do is to test that the method exists in the controller :

public function testMethodsExists()
{
    $this->assertTrue(method_exists(new AuthorizesResourcesController(), 'methodsWithoutModels'));
}

but there is no test like that for the resourceAbilityMap() method.

I also eventually might override the methods in the test controller, add a new entry like in the previous example and check if a new middleware is correctly added at the end, but this is just testing how PHP overriding works, as this is already tested for all other standard resource methods.

In fact this is principally about code refactoring here, nothing changed, it's just to allow developers to add easily new methods to their resources (no need to override the autorizeResource() method)...

@taylorotwell taylorotwell merged commit cbe1d69 into laravel:5.4 Apr 26, 2017
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