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.8] Update AuthorizesRequests.php #28957

Closed
wants to merge 1 commit into from
Closed

[5.8] Update AuthorizesRequests.php #28957

wants to merge 1 commit into from

Conversation

ZoWeArdeb
Copy link

The viewAny-method in a policy generated with php artisan is not declared in this mapping. Policies don't work anymore after a composer update.

The viewAny-method in a policy generated with php artisan is not declared in this mapping. Policies don't work anymore after a composer update.
@laurencei
Copy link
Contributor

Have you upgraded to the latest version of Laravel? I thought this was fixed last week?

@ZoWeArdeb
Copy link
Author

Have you upgraded to the latest version of Laravel? I thought this was fixed last week?

Yes, my laravel version is v5.8.26 and the composer update deleted my changes in the AuthorizesRequests-trait. I saw there was a revert a couple of days ago.

@GrahamCampbell GrahamCampbell changed the title Update AuthorizesRequests.php [5.8] Update AuthorizesRequests.php Jun 26, 2019
@laurencei
Copy link
Contributor

So to clarify - you were taking advantage of the new feature introduced in #28820 (which was a breaking change) and then the revert in #28865 caused you a different breaking change (since you had adopted the new behaviour.

Am I understanding that correctly?

@ZoWeArdeb
Copy link
Author

So to clarify - you were taking advantage of the new feature introduced in #28820 (which was a breaking change) and then the revert in #28865 caused you a different breaking change (since you had adopted the new behaviour.

Am I understanding that correctly?

I followed the documentation on https://laravel.com/docs/5.8/authorization . viewAny didn't work. So i searched for a solution. I found it by debugging the code. After a composer update i noticed that the treat was changed (normal). So then i searched if i could add it to the laravel framework, because due a failing of tests an other commit #28820 (with the changes in this treat) was reverted #28865. If the edit causes an error for other users you may close this and delete the request. It wasn't my intention to take advante of the new feature or steal any credit.

@laurencei
Copy link
Contributor

Thanks @ZoWeArdeb - I think that's an error in the documentation, which was added after the original PR, but not reverted when we reverted the changes. So I can see how that would cause confusion.

Is there anyway we can support both behaviours with breaking anything?

@taylorotwell
Copy link
Member

Huh? This was sent to 5.9 because adding it is a breaking change.

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