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

Refactoring : Confirmations controller - useless if redirect_url condition #1611

Open
newfylox opened this issue Sep 29, 2023 · 1 comment
Open

Comments

@newfylox
Copy link

Version: 1.2.2

in this section

if redirect_url
redirect_to DeviseTokenAuth::Url.generate(redirect_url, account_confirmation_success: false)
else
raise ActionController::RoutingError, 'Not Found'
end

I was wondering why we are checking if redirect_url because it's necessary of having DeviseTokenAuth.default_confirm_success_url or params[:redirect_url] set.

In other words, imagine having this method returning nil

def redirect_url
params.fetch(
:redirect_url,
DeviseTokenAuth.default_confirm_success_url
)
end

It means that the sign_up flow would failed in this section
redirect_to_link = signed_in_resource.build_auth_url(redirect_url, redirect_headers)
else
redirect_to_link = DeviseTokenAuth::Url.generate(redirect_url, redirect_header_options)
end

and throw an exception of type ArgumentError saying bad argument (expected URI object or URI string).

Maybe I missed something?

@newfylox newfylox changed the title Refactoring : Confirmations controller - useless if condition Refactoring : Confirmations controller - useless if redirect_url condition Sep 29, 2023
@newfylox
Copy link
Author

newfylox commented Sep 29, 2023

Also, it could be nice to say if email is already confirmed. I know it's possible to get the answer by calling this:

@resource.errors.of_kind? :email, :already_confirmed

So it would be nice to specify if account_confirmation_success AND something like already_confirmed in redirect_header_options so that we know if it's because the link is expired or because the email is already confirmed, giving us the chance to show a better message on frontend. There can also have something like link_expired param to tell if it's because the link is expired.

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

No branches or pull requests

1 participant