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

Scoped route binding in apiResource routes no longer working #42541

Closed
joe-pritchard opened this issue May 27, 2022 · 7 comments · Fixed by #42571
Closed

Scoped route binding in apiResource routes no longer working #42541

joe-pritchard opened this issue May 27, 2022 · 7 comments · Fixed by #42571
Labels

Comments

@joe-pritchard
Copy link
Contributor

  • Laravel Version: 9.14.1
  • PHP Version: 8.1.2
  • Database Driver & Version:

Description:

I've recently updated Laravel from 9.13.0 to 9.14.1 and I have a series of nested apiResource routes defined like so:
$router->apiResource('/product/{product}/image', ImageController::class)->scoped();

I've found that when I request an image that doesn't belong to the product id specified in the URL, I get a 404 like I'd expect in v9.13.0, but in v9.14.1 the request falls through to the controller incorrectly... I may be doing something wrong but it has been working until today.

Steps To Reproduce:

Define a scoped API resource like the one above.
Request a URL where the child model is not owned by the parent model.

Expected: The app throws a 404
Actual: The app calls the controller

I've made a new laravel project to demonstrate: https://github.com/joe-pritchard/scoped-resource-bug

There's a test in there at Tests\Feature\Models\ChildTest which shows the issue. The test fails, but it will pass if you downgrade the laravel/framework dependency to 9.13.0

@rachediabdenacer
Copy link

i have exactly the same problem,
the problem appeared when I updated the laravel/framework version from 9.4.1 to 9.14.1

@korkoshko
Copy link
Contributor

@ksassnowski ping! Can you watch it? I think the problem is because of this PR

@ksassnowski
Copy link
Contributor

I’ll have a look at this. Seems like that change had quite a few unforeseen knock-on effects. Sorry about that!

@ksassnowski
Copy link
Contributor

OK so the fix itself could get a little tricky, but there's at least a temporary workaround for now. If you explicitly define the binding field when calling scoped on an api resource, it works as expected.

So instead of

Route::apiResource('/product/{product}/image', ImageController::class)->scoped();

do this

Route::apiResource('/product/{product}/image', ImageController::class)->scoped([
    // Or whatever the route key for `Image` should be
    'image' => 'id',
]);

@driesvints
Copy link
Member

@ksassnowski if there's no way we can fix this except through a workaround we probably best revert the PR for now and re-add the changes in Laravel 10.

@driesvints driesvints added the bug label May 30, 2022
@ksassnowski
Copy link
Contributor

I actually have a solution and am finishing up the PR right now

@ksassnowski
Copy link
Contributor

Here's the PR #42571

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

Successfully merging a pull request may close this issue.

5 participants