-
-
Notifications
You must be signed in to change notification settings - Fork 546
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
Remove route model binding for users #3088
Remove route model binding for users #3088
Conversation
sorry styleci
Instead of removing route model binding entirely, maybe we could just rename the route model binds to being Statamic specific, like (thanks @edalzell for that idea) |
To be functionally the same as route model binding you need to do |
Ah, good point! The only issue is... there's no |
Also see here: #3055 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
404s need to be thrown when the user in the url doesn't exist. You can copy the logic that you removed from the service provider.
Added the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either remove the messages from the exceptions, or make sure you're passing the handle in from the route argument.
Right now since you're using $user = User::find($user)
before new-ing up the exception, the message will be User [] not found.
but it should be User [whatever] not found.
Since you don't ever see the exception messages, it's probably fine to just remove them.
I've removed the exception messages. |
This pull request resolves #2316, where issues crop up in existing Laravel apps if people are using route model binding to bind to their
User
model.This PR just straight out removes the route model binding and replaces it with a lookup in the controllers where it is referenced with
Entry::find($id)
.While working on this PR, I've also found posts on the Discord by some developers complaining that Statamic's route model binding is a bit generic and can cause issues if used side by side on an existing Laravel app. I'm happy to make any changes to any of the other route model binding stuff if that's something you'd like me to do.
Let me know if you have anything you'd like me to change.