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

Support for custom Eloquent route keys #308

Closed
arondeparon opened this issue Jul 13, 2020 · 1 comment · Fixed by #315
Closed

Support for custom Eloquent route keys #308

arondeparon opened this issue Jul 13, 2020 · 1 comment · Fixed by #315

Comments

@arondeparon
Copy link

Description

Laravel has the ability to return a value other than id as the route key name using the getRouteKeyName method on an Eloquent model. (See https://laravel.com/docs/7.x/routing#implicit-binding)

This is useful, if you would like to refer to models by their UUID instead of id, for example.

Ziggy currently has the ability to feed an object to the route() helper, which will automatically access object.id if it is present so that the route will autotically receive this argument as well.

Suggestion

It would be great if there were some way to check the return value of getRouteKeyName() to determine the route key that is needed to fulfil the route expectations, instead of referring to .id.

@arondeparon arondeparon changed the title Support for custom Eloquent route keys? Support for custom Eloquent route keys Jul 13, 2020
@bakerkretzmar
Copy link
Collaborator

bakerkretzmar commented Jul 20, 2020

This is a really cool idea, feels like an extension of #307. One thing I'm hesitant about is that right now Ziggy doesn't actually know when something is a 'model', it's just naively checking for an id key on the object. I would want to make sure that if the route key name is something pretty generic like slug, it doesn't accidentally apply to other routes using different model bindings if we pass an object that also has that key.

Also, I suspect it might be hard to get the route key name info, I'm not certain when it's resolved but if it's not available until the request is being handled, that's probably too late for Ziggy.

Would love to see a PR for this! And otherwise I'll take a stab at it when I have time 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants