-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
[8.x] Give a more meaningul message when route parameters are missing #35706
[8.x] Give a more meaningul message when route parameters are missing #35706
Conversation
Hey Beau, would you mind sending this to 8.x? |
The message for `UrlGenerationException` when parameters are missing is a nice start, but knowing which parameters are missing can be a huge help in figuring out where something is broken. Especially if a route has multiple possible parameters. Which parameter is actually missing? Are more than one missing? Now we'll know!
d387f35
to
a24ccc2
Compare
@taylorotwell No problem. I fixed a styleci issue, rebased, and changed the base branch on GitHub. Looks like the tests are all queued again. |
* @return static | ||
*/ | ||
public static function forMissingParameters($route) | ||
public static function forMissingParameters(Route $route, array $parameters = []) |
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.
We'll have to be aware that this is technically a breaking change for people extending this exception and method.
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.
We could just re-target this PR at master. It's not long till another release anyway (indeed, it never really is, with the 6 month release cycle).
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.
I'm happy with whatever. :)
Co-authored-by: Michel Bardelmeijer <[email protected]>
The message for
UrlGenerationException
when parameters are missing is a nice start, but knowing which parameters are missing can be a huge help in figuring out where something is broken. Especially if a route has multiple possible parameters. Which parameter is actually missing? Are more than one missing? Now we'll know!For example, say a developer is presented with the following exception message:
It would lead them to their controller which might look something like this:
... it is not immediately obvious where to start. Is it that the
$connectionRequest
doesn't have aclient
? Or is the$serviceIntake
set tonull
for some reason? Or are they both having issues?With this PR, the exception message would change to one of the following:
This feedback will save the developer a lot of time trying to figure out which parameters are missing so they can get right to fixing the issue instead of digging more to figure out which parameter is actually causing the problem.
Originally targeted
8.x
with this PR, but realized it can go all the way back to6.x
, too. Let me know if you'd like me to retarget another branch or if there is anything I can do to help ensure this makes it into the most versions possible if it is accepted. :)