jeremyevans / rodauth

Ruby's Most Advanced Authentication Framework
http://rodauth.jeremyevans.net
MIT License
1.65k stars 95 forks source link

Is verify_account_email_sent_redirect misnamed? #416

Closed pboling closed 2 months ago

pboling commented 2 months ago

It appears that it is only used in the scenario where the verify account email request has failed. Is there a clear way I can distinguish which "redirect" settings are used in a failure case, versus the success case?

      r.post do
        if account_from_login(param(login_param)) && allow_resending_verify_account_email?
          if verify_account_email_recently_sent?
            set_redirect_error_flash verify_account_email_recently_sent_error_flash
            redirect verify_account_email_recently_sent_redirect
          end

          before_verify_account_email_resend
          if verify_account_email_resend
            after_verify_account_email_resend
            verify_account_email_sent_response
          end
        end

        set_redirect_error_status(no_matching_login_error_status)
        set_error_reason :no_matching_login
        set_redirect_error_flash verify_account_resend_error_flash
        redirect verify_account_email_sent_redirect # <========= Only used here!
      end

By way of comparison, the verify_account_email_recently_sent_redirect does indicate exactly what use case it accounts for.

I want to get my thinking right, because this confused me tremendously. It "felt" obvious what verify_account_email_sent_redirect was for, so after trying to understand why it had no effect, as I was expecting it to handle the success case, I ended up going to the source, and couldn't understand what I was seeing at first.

jeremyevans commented 2 months ago

verify_account_email_sent_response calls verify_account_email_sent_redirect (https://github.com/jeremyevans/rodauth/blob/master/lib/rodauth.rb#L225). verify_account_email_sent_redirect is called in both the the success and failure cases. I don't think it's misnamed, though if you want a separate redirect method added just for the failure case, that could be added.

pboling commented 2 months ago

I really appreciate being able to understand how you mentally structure this. Cheers!