-
Notifications
You must be signed in to change notification settings - Fork 43
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
Impossible to change route model binding? #282
Comments
Hi. Have you set a pattern on the |
Thanks, I haven't actually. After trying it out - looks like this hasn't affected anything. After some debugging, it appears that The routes: $server->resource('posts', JsonApiController::class)
->parameter('post:slug')
->only('show'); The Schema: use LaravelJsonApi\Eloquent\Contracts\Paginator;
use LaravelJsonApi\Eloquent\Fields\ArrayHash;
use LaravelJsonApi\Eloquent\Fields\DateTime;
use LaravelJsonApi\Eloquent\Fields\ID;
use LaravelJsonApi\Eloquent\Fields\Str;
use LaravelJsonApi\Eloquent\Filters\WhereIdIn;
use LaravelJsonApi\Eloquent\Pagination\PagePagination;
use LaravelJsonApi\Eloquent\Schema;
class PostSchema extends Schema
{
public static string $model = Post::class;
public function fields(): array
{
return [
ID::make('slug')->matchAs('[A-Z_]+'),
Str::make('slug'),
DateTime::make('created_at')->sortable()->readOnly(),
DateTime::make('updated_at')->sortable()->readOnly(),
];
}
public function filters(): array
{
return [
WhereIdIn::make($this),
];
}
public function pagination(): ?Paginator
{
return PagePagination::make();
}
} P.S. it doesn't matter if I change the Model's Are you able to replicate the issue? Maybe it's just me? |
Ok so what's wrong here is you're messing with the routing implementation, to try and solve a problem that's caused by you not setting a correct ID matching pattern. The fix is to set a matching pattern on the id that works with the slug. Fix that, then remove any changes you've made to the routing to try and fix the problem - as they are now the problem. This does all work. It's 100% tested, and used in production applications. |
This bit:
is not in any of the Laravel JSON:API routing docs, so is the bit you need to remove (I believe from glancing over the code). |
You're right, thanks so much, using the So I was definitely misusing the feature. Would it make sense to mention in the docs that route ID binding is different from Laravel's? I might be the exception here, though. |
According to the docs of this package (here and here) and this issue here, it seems like the package should have the ability to use an attribute as an ID for routes.
My goal is to provide an API endpoint for "blog posts" which can be created and viewed. Instead of using the
id
in the URL I want to access myposts
by theslug
attribute.Unfortunately, I'm unable to get it working and I'm not sure if it's a bug or if am I misunderstanding something. The docs mention this customization here:
Here's how I've tried to make it work
ID::make('slug')
. When sending a request toapi/v1/posts/test-post
I got the 404 error with the messageThe route api/v1/posts/test-post could not be found.
parameter()
method should be used in the routes:$server->resource('posts', JsonApiController::class)->parameter('post:slug')
. The parameterpost:slug
is defined according to Laravel's routing key customization. After this change, sending a GET request toapi/v1/posts/test-post
results in aLogicException
with a messageNo JSON API resource id set on route.
.The exception is thrown, because the method modelOrResourceId() isn't able to get a parameter with the
$name
ofpost:slug
, because only these parameters exist:It appears as if the method is looking for a parameter that doesn't even exist in the route.
Secondly, even if this didn't throw an exception, the request still returns a 404 because in substituteBindings() the method uses the repository to
find()
a model by the resource ID. Unfortunately, this resource ID is no longer the primary key, but an attributeslug
.Am I missing something here or is this feature not working as expected?
EDIT: I've also tried using a custom Controller that uses route-model binding, which also failed due to the
substituteBindings()
methodThe text was updated successfully, but these errors were encountered: