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] Bad mapping from Controller's Destroy action to Ability / Policy action #13716

Merged
merged 2 commits into from
May 26, 2016

Conversation

silverqx
Copy link
Contributor

Laravel's Resource Controller doesn't have delete action, when I generate Controller Resource with php artisan make:controller --resource TestController, so there is only destroy action.

Laravel's Resource Controller doesn't have delete action, when I generate Controller Resource with `php artisan make:controller --resource TestController`, so there is only destroy action.
@GrahamCampbell GrahamCampbell changed the title Bad mapping from Controller's Destroy action to Ability / Policy action [5.2] Bad mapping from Controller's Destroy action to Ability / Policy action May 26, 2016
@silverqx
Copy link
Contributor Author

silverqx commented May 26, 2016

I think, that would be good to leave 'delete' => 'delete' mapping in this array too, even if there isn't any delete action in a resources, Laravel would be more intelligent.
Of course phpunit tests need update too. :)

@silverqx
Copy link
Contributor Author

silverqx commented May 26, 2016

I have upgraded phpunit tests too, now everything passes.

What do you think about leaving 'delete' => 'delete' mapping in this ability / policy array map? @JosephSilber

@JosephSilber
Copy link
Member

That was an oversight on my part. Makes no sense to keep it in there. Just keep the change the way you did it here.

Don't forget to change the name of the test method.

@taylorotwell taylorotwell merged commit 3482d50 into laravel:5.2 May 26, 2016
This was referenced May 26, 2016
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