Route Model Binding Exception

[aerni]

Bug Description

Route Model Binding as described here doesn't work. It throws a Call to a member function id() on null Exception.

The issue is $route->parameter('collection')->id(). The parameter collection does not exist.

cms/src/Providers/RouteServiceProvider.php

Line 61 in c58e7b4

! ($entry = Entry::find($handle)) || $entry->collection()->id() !== $route->parameter('collection')->id(),

How to Reproduce

Add this route and a test view:

Route::statamic('/test/{entry}', 'my_test_view');

Environment

Statamic 3.1.32 Pro
Laravel 8.53.1
PHP 7.4.20

[edalzell]

Slugs are not unique anymore, not sure the best way to handle this

[aerni]

It works when simply removing the part that throws the error. But I'm not sure if it breaks something else.

[jasonvarga]

It assumes the collection is in the route too. Just rename your route parameter to avoid using route model binding.

Route::statamic('/test/{slug}'

[aerni]

But I want to use route model binding. What I'm after is creating a route that gets the full context like a regular collection entry does. Not sure if I'm understanding route model binding correctly.

This works. I see that the entry will be available in it's own variable instead of flattened like the data is:

Route::statamic('/test/{collection}/{entry}', 'my_test_view');

I also tried to use a controller like mentioned in the docs. But this throws an Array to string conversion exception at vendor/statamic/cms/src/View/View.php:194 .

Route::statamic('/test/{collection}/{entry}', [TestController::class, 'show']);

class TestController extends Controller
{
    public function show($collection, $entry)
    {
        dd($collection, $entry);
    }
}

[duncanmcclean]

Not entirely sure if this is totally related to this issue but we're coming up against a couple issues regarding route model binding when installing Statamic into an existing Laravel application. (I'm happy to move this to a fresh issue if that's better)

In our case we have an Eloquent model called Asset because our app is related to physical assets. However, now we've installed Statamic, this breaks as we're using route model binding.

This is the error we're seeing with any of the routes with asset route-model binding.

{
  "message": "Call to a member function handle() on null",
   "exception": "Error",
   "file": "/home/the-project-in-question/vendor/statamic/cms/src/Providers/RouteServiceProvider.php",
   "line": 111,
}

In Statamic's RouteServiceProvider, it binds asset to a Statamic asset which is what's causing the conflict.

https://github.com/statamic/cms/blob/3.1/src/Providers/RouteServiceProvider.php#L108-L120

We can potentially work around this by overriding the route-model binding ourselves and doing a check if the request is from the CP or not. If it is, we use Statamic's stuff, if not we bind to our own Asset model.

$firstPath = explode('/', request()->getPathInfo())[1];

if ($firstPath === 'cp') {
  return $assetId;
}

This obviously isn't ideal - maybe Statamic could prefix it's bindings with statamic, so it's statamicAsset instead which would certainly cause less issues.

Using Statamic alongside existing Laravel applications are our primary use case - in fact, this is the third app that we're doing it on.

[jasonvarga]

Honestly I think we should remove our bindings. As you can see it conflicts with people's applications too much. We already removed one here #3088

[ryanmitchell]

I just bumped into the same issue installing onto an existing Laravel app - {collection} was causing 404 issues.
It would be great if the bindings could be optional, or scoped to Statamic only.