Skip to content
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

issue 4356 adds is_navigational_format? check to after_sign_up_path_for #4833

Merged
merged 1 commit into from
Aug 2, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/devise/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def sign_up(resource_name, resource)
# The path used after sign up. You need to overwrite this method
# in your own RegistrationsController.
def after_sign_up_path_for(resource)
after_sign_in_path_for(resource)
after_sign_in_path_for(resource) if is_navigational_format?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we cover this change with tests?

Copy link
Contributor Author

@iorme1 iorme1 Apr 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked up all the current methods in Devise that use 'is_navigational_format?' checks and I couldn't seem to find anything in those tests that specifically incorporate a test for that navigational check. In my previous pull requests for Devise, I wrote my tests based off similar structured methods and their corresponding tests. However, since I cannot find how the other methods using 'is_navigational_format?' are testing it, I am not sure how to
incorporate additional tests for the method I added it to. Based on the issue conversation, i was under the impression that the method I changed used to have a 'is_navigational_format?' check and that it somehow got deleted by accident from a git reset. So I simply added it back to the method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we don't have tests for any of these so I think it's ok for now.

end

# The path used after sign up for inactive accounts. You need to overwrite
Expand Down