-
Notifications
You must be signed in to change notification settings - Fork 4.1k
use dynamic route name #3147
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
use dynamic route name #3147
Conversation
app/controllers/users_controller.rb
Outdated
@@ -6,7 +6,7 @@ class UsersController < ApplicationController | |||
def edit | |||
unless current_user | |||
skip_authorization | |||
return redirect_to "/enter" | |||
return redirect_to new_user_registration_path_path |
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.
can we also change the alias into new_user_registration_path
in routes? If it too much effort, let's stick with this one.
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.
new_user_registration_path
has already been declared by devise. What else do you reckon? :)
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.
okay, keep it then
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.
Thank you for this PR!.
new_user_registration_path_path
is a bit long. Let's change that path to sign_up_path
.
that is a good suggestion |
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.
LGTM!
What type of PR is this? (check all applicable)
Description
redirect_to
inusers_controller.rb
was using a hardcoded route path, so I just changed it to using a dynamic route name pathAdded to documentation?