scambra / devise_invitable

An invitation strategy for devise
MIT License
2.66k stars 553 forks source link

Forms do not work with turbo_stream in the navigational_formats config #886

Closed jtFrancisco closed 1 year ago

jtFrancisco commented 1 year ago

When you have the following config: config.navigational_formats = ['*/*', :html, :turbo_stream]

Invitation forms break causing errors. For example these errors happen when trying to submit a new invitation form: undefined method 'users_url' for #<Users::InvitationsController:0x00000000050dc0>

and undefined method 'empty?' for #<User id: nil, ...

The forms will only work when using data: {turbo: false} in the form_for helper like this:

<%= form_for(resource, as: resource_name, url: invitation_path(resource_name), html: { method: :post }, data: {turbo: false}) do |f| %>

scambra commented 1 year ago

I would need the backtrace to see which line is calling users_url, or empty?

If your backtrace doesn't include gem lines, remove backtrace silencers.

jtFrancisco commented 1 year ago

The Rails' error view (that you see in development env) showed the undefined method 'users_url' error message, but when I looked at the "Application error" there is this:

ActionView::MissingTemplate: Missing template users/invitations/new, devise/invitations/new, devise/new, application/new with {:locale=>[:en], :formats=>[:turbo_stream], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby, :jbuilder]}

This error is saying "I couldn't find your 'new' template." After reading about Turbo Stream I learned that you need a new.turbo_stream.erb file with some code that runs a turbo_stream.replace block with your form partial in the block.

I was able to make the invitation form work like this:

In app/views/devise/invitations/new.html.erb: <%= render "form", resource: resource %>

In app/views/devise/invitations/new.turbo_stream.erb:

<%= turbo_stream.replace dom_id(resource) do %>
  <%= render "form", resource: resource %>
<% end %>

In app/views/devise/invitations/_form.html.erb I made the form_for like normal but added an id:

<%= form_for(resource, as: resource_name, url: user_invitation_path, html: { method: :post, id: dom_id(resource) }) do |f| %>

This will prevent the error because Rails finds the new.turbo_stream.erb file and replaced the partial (if the form submission has errors).

I didn't understand all of the steps required to make a turbo stream form work. So, this might not be an issue with the gem but a problem with my particular implementation.

Here is what I was seeing without the app/views/devise/invitations/new.turbo_stream.erb file:

Screen Shot 2023-04-01 at 9 55 01 AM

scambra commented 1 year ago

The latest devise has :turbo_stream in config.navigational_formats, it's supposed to support Hotwire/Turbo, but I don't see any .turbo_stream.erb in devise views.

Have you checked this guide? https://github.com/heartcombo/devise/wiki/How-To:-Upgrade-to-Devise-4.9.0-%5BHotwire-Turbo-integration%5D Are you using responders 3.1.0? I guess so as you have devise 4.9.0, but it would be good to discard it as the cause.

Or you may need to set these lines in devise initializer, if you have upgraded from an older version: config.responder.error_status = :unprocessable_entity config.responder.redirect_status = :see_other

YarikST commented 1 year ago

Devise::InvitationsController has respond_with_navigational(resource) { render :new, status: :unprocessable_entity }

  def respond_with_navigational(*args, &block)
    respond_with(*args) do |format|
      format.any(*navigational_formats, &block)
    end
  end

it means that your browser request turbo response first and HTML response as a fallback, this is working well with respond_with because you render only HTML there and the server render HTML.erb view However with respond_with_navigational we call the render for turbo response and we should have .*.turbo_stream.erb

I have 200 with no content for signing up page, i just override actions and use respond_with instead

    # PUT /resource/invitation
    def update # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity
      raw_invitation_token = update_resource_params[:invitation_token]
      self.resource = accept_resource
      invitation_accepted = resource.errors.empty?

      yield resource if block_given?

      if invitation_accepted
        if resource.class.allow_insecure_sign_in_after_accept
          flash_message = resource.active_for_authentication? ? :updated : :updated_not_active
          set_flash_message :notice, flash_message if is_flashing_format?
          resource.after_database_authentication
          sign_in(resource_name, resource)
          respond_with resource, location: after_accept_path_for(resource)
        else
          set_flash_message :notice, :updated_not_active if is_flashing_format?
          respond_with resource, location: new_session_path(resource_name)
        end
      else
        resource.invitation_token = raw_invitation_token
        respond_with(resource)
      end
    end
scambra commented 1 year ago

Devise is still using respond_with_navigational in some actions, as show action in unlocks and confirmations controllers, with redirect or render: https://github.com/heartcombo/devise/blob/8e2e3f6fda1ceabe2eff08cf34a6e663527c438b/app/controllers/devise/confirmations_controller.rb#L31

Also in destroy action in registrations controller, this one only with redirect: https://github.com/heartcombo/devise/blob/8e2e3f6fda1ceabe2eff08cf34a6e663527c438b/app/controllers/devise/registrations_controller.rb#L70

When you changed respond_with_navigational with respond_with, you removed the render call (render :new or render :edit). How did it work? Does it still work when turbo stream is not used?

YarikST commented 1 year ago

When you changed respond_with_navigational with respond_with, you removed the render call (render :new or render :edit). How did it work? Does it still work when turbo stream is not used?

i change only this call

you are using this to render errors, rails are looking at turbo response which I don’t have, once I used respond_with it renders edit.erb and shows my errors

I don’t understand this:

removed the render call (render :new or render :edit)

respond_with_navigational is wrapping respond_with with navigational format only, isn’t it?

  def respond_with_navigational(*args, &block)
    respond_with(*args) do |format|
      format.any(*navigational_formats, &block)
    end
  end
scambra commented 1 year ago

The line was

respond_with_navigational(resource) { render :edit, status: :unprocessable_entity }

Which renders edit view, and you changed to

respond_with(resource)

with no render. So I was wondering if it would render the same view. I just checked the doc for responders gem, and I saw respond_with uses :new for post requests and :edit for put and patch requests, so it should be ok changing both lines using respond_with_navigational(resource) to respond_with(resource)

scambra commented 1 year ago

Fixed with #887