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

confirmation should redirect to default_confirm_success_url by default #1091

Merged
merged 1 commit into from
Sep 8, 2018

Conversation

maysam
Copy link
Contributor

@maysam maysam commented Feb 14, 2018

@dks17
Copy link
Contributor

dks17 commented Feb 14, 2018

It needs some tests covering the case (then redirect_url is absent).
Is controller suitable place to put your code? What if I need to get redirect_url in other controller of an application?

@maysam
Copy link
Contributor Author

maysam commented Feb 14, 2018

this is already following the pattern in registrations_controller.rb#L20-L23

@dks17
Copy link
Contributor

dks17 commented Feb 14, 2018

@maysam you are right.
I think that will be better to put your code and the pattern in one place.
And anyway tests are needed.

@MaicolBen
Copy link
Collaborator

Merging as the author didn't show up anymore and it isn't mandatory in this case the tests as it's pretty simple.

@MaicolBen MaicolBen merged commit 0b28131 into lynndylanhurley:master Sep 8, 2018
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