-
-
Notifications
You must be signed in to change notification settings - Fork 549
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 { collection } route binding #5774
Remove { collection } route binding #5774
Conversation
I appreciate this PR and think it's a good change to have, but I think it might be a potential breaking change for addons that may be relying on these bindings in the control panel. Could we instead just provide the bindings to control panel routes? Like we do for the API: cms/src/Providers/RouteServiceProvider.php Lines 89 to 91 in 38f1116
Have one of these: if (! $this->isCpRoute($route)) {
return $handle;
} |
if (! $collection) { | ||
throw new NotFoundHttpException("Collection [$collectionHandle] not found."); | ||
} | ||
|
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.
throw_unless($collection = Collection::findByHandle($collection))
?
if (! $collection) { | ||
throw new NotFoundHttpException("Collection [$collectionHandle] not found."); | ||
} | ||
|
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.
same as above?
@jasonvarga well that would have been a lot less work 🤣 new PR incoming |
Sorry 😬 |
I'm having issues with the
{ collection }
route binding when using Statamic alongside another package.So as suggested here I've removed the collection binding from all routes that use it.
I'm happy to do the leg work on any changes you want to this - so just direct me.