Skip to content

Change hard-coded '/login' param to named route #4143

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

Merged
merged 1 commit into from
Feb 17, 2017
Merged

Change hard-coded '/login' param to named route #4143

merged 1 commit into from
Feb 17, 2017

Conversation

kneipp
Copy link
Contributor

@kneipp kneipp commented Feb 17, 2017

Change hard-coded '/login' param to named route 'login'.

Adding a bit of flexibility to work with translations w/o need change Handler class:
Route::get('entrar', 'CustomController@showLoginForm')->name('login');

@taylorotwell taylorotwell merged commit da9760f into laravel:master Feb 17, 2017
@laurencei
Copy link
Contributor

laurencei commented Feb 17, 2017

@taylorotwell @kneipp - there is one small problem with this PR: on a brand new 5.4 install, you cause a "InvalidArgumentException - Route [login] not defined" if you try and hit the default route my.app/api/user which is registered as part of the base install.

In saying that - the old behavior before this PR looks like it was a "404 NotFoundHttpException" (since there is no /login on a base install anyway).

Not sure which is the lesser of two evils? Maybe the default should be to / so the "out of the box" install works with no errors, and let people customize it themselves as they see fit?

Leave it with you...

bkrukowski pushed a commit to bkrukowski/laravel that referenced this pull request Mar 10, 2017
Change hard-coded '/login' param to named route
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants