mikker / passwordless

🗝 Authentication for your Rails app without the icky-ness of passwords
MIT License
1.28k stars 88 forks source link

add controller support for Turbo #116

Closed kylekeesling closed 2 years ago

kylekeesling commented 2 years ago

Hi there mikker - thanks for a great gem! I noticed a small little tweak was needed to support the way Turbo/Hotwire expects responses from controller actions.

I hope this is something you'll find useful and please let me know if you need any other tweaks or additions to this PR.

mikker commented 2 years ago

Looks great! Thank you for fixing this. It's a bit unorthodox to render in a create, I think that's why I explicitly added the call to render.

💙💚💛💜❤️

kylekeesling commented 2 years ago

Happy to help, thanks for a great gem

nickhammond commented 2 years ago

Loving this gem, great work.

It looks like this might still be an issue, I'm getting Error: Form responses must redirect to another location when the session is successfully created because of the render instead of the redirect.

What about utilizing just the new view file and rendering the content from create in the flash? This would be a breaking change I realize for anyone that's already using this though but a fairly easy update for most.

    def create
      @resource = find_authenticatable
      session = build_passwordless_session(@resource)

      if session.save
        if Passwordless.after_session_save.arity == 2
          Passwordless.after_session_save.call(session, request)
        else
          Passwordless.after_session_save.call(session)
        end
      end

      flash[:notice] = I18n.t('passwordless.sessions.create.email_sent_if_record_found')
      redirect_to(sign_in_path)
    end
mikker commented 2 years ago

PR welcome!

nickhammond commented 2 years ago

@mikker Sounds good, I can take a look next week!

nickhammond commented 2 years ago

@mikker After looking at this a bit more and referencing how Slack and Magic.link utilize this flow I think it makes more sense to keep rendering the create.html.erb template instead of utilizing the flash. There's most likely going to be a few additional things that people will want to do that restricting that to flash might be more difficult to work with.