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

Devise 4.8.1 + Rails 7.0.0 | Undefined method 'user_url' #5439

Closed
ghost opened this issue Dec 18, 2021 · 25 comments · Fixed by #5548
Closed

Devise 4.8.1 + Rails 7.0.0 | Undefined method 'user_url' #5439

ghost opened this issue Dec 18, 2021 · 25 comments · Fixed by #5548

Comments

@ghost
Copy link

ghost commented Dec 18, 2021

Pre-check

  • Created a new rails-app with rails new app-name -a propshaft -j importmap -c tailwind
  • Added gem 'devise', '~> 4.8', '>= 4.8.1' to Gemfile and followed installation instructions

Environment

  • Ruby 3.0.3p157
  • Rails 7.0.0
  • Devise 4.8.1

Current behavior

  • Receiving NoMethodError (undefined method 'user_url' for #<Devise::RegistrationsController:0x0000000000d638>) upon successful user registration.
  • User gets persisted in database

Demo app to reproduce the error is available here

Expected behavior

  • Redirect to root_path without error
@nickrivadeneira
Copy link

Ran into this at the same time as you 😆

Quick fix, tl;dr

Add :turbo_stream as a navigational format. This line goes in config/initializers/devise.rb. I haven't tested this extensively, but I think it should be fine.

config.navigational_formats = ['*/*', :html, :turbo_stream]

More detail

A few lines from a trace starting with the offending line in registrations controller:

respond_with resource, location: after_sign_up_path_for(resource)

def after_sign_up_path_for(resource)
after_sign_in_path_for(resource) if is_navigational_format?
end

def is_navigational_format?
Devise.navigational_formats.include?(request_format)
end

devise/lib/devise.rb

Lines 218 to 220 in 025b1c8

# Which formats should be treated as navigational.
mattr_accessor :navigational_formats
@@navigational_formats = ["*/*", :html]

Basically, requests in Rails 7 may come with format :turbo_stream rather than simply :html. This causes is_navigational_format? to be falsey and thus after_sign_up_path_for returns nil. With location: nil for responds_with, the redirect path resolves from the resource - in our case the user.

I'm happy to open a PR for this, though someone with more familiarity with turbo should sanity check it.

nickrivadeneira added a commit to nickrivadeneira/devise that referenced this issue Dec 18, 2021
Fixes heartcombo#5439
Rails 7 returns :turbo_stream as a request format out-of-the-box.
@nortonandreev
Copy link

nortonandreev commented Dec 20, 2021

@erik-brueggemann my understanding is that method: :delete won't work with a link_to when not using UJS (which is not a dependency by default in Rails 7). So the sign_out method is going to be called as a GET, which is the fallback, resulting in an error.

You can try changing link_to to button_to (which I don't like), or add data-turbo-method="delete", like so:

<%= link_to "Log out", destroy_user_session_path,
data: {
 "turbo-method": :delete
}, class: "link-secondary smooth" %>

A problem I am trying to find a solution for with this approach is that upon singing out it won't redirect to the root, but to the page the user was on before signing out, which, of course, most probably requires authorisation, so it is not the best experience.

The approach above works without the @nickrivadeneira fix (however, you might be experiencing a different issue). I'm not sure what is it changes exactly in the behaviour. I was hoping it will be a fix for my issue, but it isn't... any help will be appreciated.

@ghost
Copy link
Author

ghost commented Dec 20, 2021

Hi @nortonandreev, the problem you're describing is yet another one that resulted from the Rails 7 / turbo-rails aftermath as far as I can tell.

This particular issue here wasn't meant to address the log out mechanism, but rather user registration only. For that, the fix suggested by @nickrivadeneira makes sense to me - nevertheless you've got a point! It's probably a good idea to open a separate issue to find a permanent solution for that?

But since we've already brought it up, instead of changing from link_to to button_to and instead of adding the :delete data-tag, you might try editing your config/devise.rb to sign out via GET instead of DELETE

  # The default HTTP method used to sign out a resource. Default is :delete.
  config.sign_out_via = :get # <= change this from :delete to :get and remove the `method:` in your `link_to` helper

Happy to open up another issue if ya'll think it's two separate topics.

@modosc
Copy link

modosc commented Dec 21, 2021

But since we've already brought it up, instead of changing from link_to to button_to and instead of adding the :delete data-tag, you might try editing your config/devise.rb to sign out via GET instead of DELETE

just curious - wouldn't that open one up to csrf attacks?

@nortonandreev
Copy link

Hi @nortonandreev, the problem you're describing is yet another one that resulted from the Rails 7 / turbo-rails aftermath as far as I can tell.

This particular issue here wasn't meant to address the log out mechanism, but rather user registration only. For that, the fix suggested by @nickrivadeneira makes sense to me - nevertheless you've got a point! It's probably a good idea to open a separate issue to find a permanent solution for that?

But since we've already brought it up, instead of changing from link_to to button_to and instead of adding the :delete data-tag, you might try editing your config/devise.rb to sign out via GET instead of DELETE

  # The default HTTP method used to sign out a resource. Default is :delete.
  config.sign_out_via = :get # <= change this from :delete to :get and remove the `method:` in your `link_to` helper

Happy to open up another issue if ya'll think it's two separate topics.

The changelog says that "Please note that Turbo integration is not fully supported by Devise yet.". It would be nice if someone familiar with the matter updates us on what does this mean exactly and if there is some ETA of when full support will be added. I don't really want to waste hours implementing hacky solutions which might make my app vulnerable. On the other hand, I don't want to be waiting forever, if devise won't be fully updated to support Rails 7.

@edusurf10
Copy link

method: :delete in my link_to dont working, only work this format >
<%= link_to "Deletar", admin_admin_path(admin), data: { "turbo-method": :delete, confirm: "Certeza que deseja excluir essa conta?" }, class: "btn btn-danger btn-sm" %>

@carlosantoniodasilva
Copy link
Member

For now you have to disable turbo on devise links, i.e. data: { turbo: false }. I should have some time to look into more official turbo integration soon.

@edusurf10
Copy link

For now you have to disable turbo on devise links, i.e. data: { turbo: false }. I should have some time to look into more official turbo integration soon.

all right, thanks.

@ahowardm
Copy link

ahowardm commented Jan 28, 2022

As noted by @nortonandreev this worked for me in an app with Rails 7 and importmap

<%= link_to "Log out", destroy_user_session_path,
data: {
 "turbo-method": :delete
} %>

@joshuaneedham
Copy link

joshuaneedham commented Feb 5, 2022

#5439 (comment)

This solved my issue to help me close out the initial setup of my new application that I'm working on, thank you!

@john-999
Copy link

john-999 commented Mar 3, 2022

The suggestion by @nortonandreev works for me, but adds an additional, failing and unnecessary (DELETE) HTTP request:

Started DELETE "/users/sign_out" for 127.0.0.1 at 2022-03-03 17:33:09 +0100
Processing by Users::SessionsController#destroy as TURBO_STREAM
  [...]
Redirected to http://localhost:3000/
Completed 302 Found in 5ms (ActiveRecord: 0.3ms | Allocations: 3175)


Started DELETE "/" for 127.0.0.1 at 2022-03-03 17:33:10 +0100
  
ActionController::RoutingError (No route matches [DELETE] "/")

@5minpause
Copy link

I added , data: { turbo: false } to all link_to and form_for helpers on the Devise views.

@omokehinde
Copy link

I am on Rails 7.0.2.3 and I didn't have this issue. Maybe upgrading to this version of rails or higher might fix it,

@b-sep
Copy link

b-sep commented May 1, 2022

im having this same problem in a app with bootstrap, link_to perfoms GET method instead DELETE even if i use data: {turbo_method: :delete}
using button_to i manage to delete things, but confirmations dont work. Im guessing is something with bootstrap cuz in other project without it, destroy works just fine in link and button_to ..

@pargara
Copy link

pargara commented May 17, 2022

I am on Rails 7.0.2.3 and I didn't have this issue. Maybe upgrading to this version of rails or higher might fix it,

@omokehinde dont work im using the rails 7.0.2.3 and i have the same issue but when i register a user

@sergiy-koshoviy
Copy link

It works for me
<%= button_to 'Logout', destroy_user_session_path, method: :delete, form: {turbolink: false} %>

ChillerDragon added a commit to BingeBounty/BingeBounty that referenced this issue Aug 25, 2022
My app worked fine until suddenly I got these errors:
NoMethodError (undefined method 'user_url' for #<Devise::RegistrationsController:0x0000000000d638>)

Thanks to @nickrivadeneira for posting a fix here
heartcombo/devise#5439 (comment)
jackparsons93 added a commit to jackparsons93/rails_7_devise_example that referenced this issue Sep 25, 2022
This is a step from github issue 5439 It get error. Receiving NoMethodError (undefined method 'user_url' for #<Devise::RegistrationsController:0x0000000000d638>) upon successful user registration.
User gets persisted in database
heartcombo/devise#5439
marcel-strzalka added a commit to marcel-strzalka/monte-cinema that referenced this issue Sep 30, 2022
Add turbo_stream as navigational format
in order to fix redirect after user registration.

heartcombo/devise#5439 (comment)
tvararu added a commit to design-history/design-history that referenced this issue Oct 25, 2022
tvararu added a commit to design-history/design-history that referenced this issue Oct 25, 2022
@HelloOjasMutreja
Copy link

Hi everyone,
This message is from Dec, 2022,
I am currently making a rails app in which I do use devise, but certainly all of it works fine from registering a new user session to destroying the same.
I am just a beginner at Ruby on Rails but this app needed nothing more.

This is what I did

<%= link_to "Log in", new_user_session_path %>

<%= link_to "Sign Up", new_user_registration_path %>

<%= link_to "Sign Out", destroy_user_session_path, 'data-turbo-method': :delete %>

I am attaching the repository
You can check the application code to verify and let's close the issue finally.

@iambenkis
Copy link

Ran into this at the same time as you laughing

Quick fix, tl;dr

Add :turbo_stream as a navigational format. This line goes in config/initializers/devise.rb. I haven't tested this extensively, but I think it should be fine.

config.navigational_formats = ['*/*', :html, :turbo_stream]

More detail

A few lines from a trace starting with the offending line in registrations controller:

respond_with resource, location: after_sign_up_path_for(resource)

def after_sign_up_path_for(resource)
after_sign_in_path_for(resource) if is_navigational_format?
end

def is_navigational_format?
Devise.navigational_formats.include?(request_format)
end

devise/lib/devise.rb

Lines 218 to 220 in 025b1c8

# Which formats should be treated as navigational.
mattr_accessor :navigational_formats
@@navigational_formats = ["*/*", :html]

Basically, requests in Rails 7 may come with format :turbo_stream rather than simply :html. This causes is_navigational_format? to be falsey and thus after_sign_up_path_for returns nil. With location: nil for responds_with, the redirect path resolves from the resource - in our case the user.

I'm happy to open a PR for this, though someone with more familiarity with turbo should sanity check it.

This solution worked for me too!

@JunichiIto
Copy link
Contributor

My sample app is working fine with the changes I described here:
How to customize Devise for Rails 7.0 and Turbo - DEV Community

I tested these behaviors:

  • Sign up
  • Sign in and sign out
  • Edit account
  • Reset password
  • Cancel account

carlosantoniodasilva added a commit that referenced this issue Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like
HTML, so Devise/responders can work properly.

Allow configuring the `error_status` and `redirect_status` using the
latest responders features, via a new custom Devise responder, so we can
customize the both responses to match Hotwire/Turbo behavior, for
example with `422 Unprocessable Entity` and `303 See Other`,
respectively. The defaults aren't changing in Devise itself (yet), so it
still responds on errors cases with `200 OK`, and redirects on non-GET
requests with `302 Found`, but new apps are generated with the new
statuses and existing apps can opt-in. Please note that these defaults
might change in a future release of Devise.

PRs/Issues references:

#5545
#5529
#5516
#5499
#5487
#5467
#5440
#5410
#5340

#5542
#5530
#5519
#5513
#5478
#5468
#5463
#5458
#5448
#5446
#5439
carlosantoniodasilva added a commit that referenced this issue Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like
HTML, so Devise/responders can work properly.

Allow configuring the `error_status` and `redirect_status` using the
latest responders features, via a new custom Devise responder, so we can
customize the both responses to match Hotwire/Turbo behavior, for
example with `422 Unprocessable Entity` and `303 See Other`,
respectively. The defaults aren't changing in Devise itself (yet), so it
still responds on errors cases with `200 OK`, and redirects on non-GET
requests with `302 Found`, but new apps are generated with the new
statuses and existing apps can opt-in. Please note that these defaults
might change in a future release of Devise.

PRs/Issues references:

#5545
#5529
#5516
#5499
#5487
#5467
#5440
#5410
#5340

#5542
#5530
#5519
#5513
#5478
#5468
#5463
#5458
#5448
#5446
#5439
carlosantoniodasilva added a commit that referenced this issue Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like
HTML, so Devise/responders can work properly.

Allow configuring the `error_status` and `redirect_status` using the
latest responders features, via a new custom Devise responder, so we can
customize the both responses to match Hotwire/Turbo behavior, for
example with `422 Unprocessable Entity` and `303 See Other`,
respectively. The defaults aren't changing in Devise itself (yet), so it
still responds on errors cases with `200 OK`, and redirects on non-GET
requests with `302 Found`, but new apps are generated with the new
statuses and existing apps can opt-in. Please note that these defaults
might change in a future release of Devise.

PRs/Issues references:

#5545
#5529
#5516
#5499
#5487
#5467
#5440
#5410
#5340

#5542
#5530
#5519
#5513
#5478
#5468
#5463
#5458
#5448
#5446
#5439
carlosantoniodasilva added a commit that referenced this issue Jan 31, 2023
Treat `:turbo_stream` request format as a navigational format, much like
HTML, so Devise/responders can work properly.

Allow configuring the `error_status` and `redirect_status` using the
latest responders features, via a new custom Devise responder, so we can
customize the both responses to match Hotwire/Turbo behavior, for
example with `422 Unprocessable Entity` and `303 See Other`,
respectively. The defaults aren't changing in Devise itself (yet), so it
still responds on errors cases with `200 OK`, and redirects on non-GET
requests with `302 Found`, but new apps are generated with the new
statuses and existing apps can opt-in. Please note that these defaults
might change in a future release of Devise.

PRs/Issues references:

#5545
#5529
#5516
#5499
#5487
#5467
#5440
#5410
#5340

#5542
#5530
#5519
#5513
#5478
#5468
#5463
#5458
#5448
#5446
#5439
@johnpitchko
Copy link

Wanted to confirm that the article that @JunichiIto posted/wrote worked successfully for me. Thank you!

@carlosantoniodasilva
Copy link
Member

The main branch should contain all that's necessary for fully working with Turbo now, which should fix this. A new version will be released soon, but feel free to test it out from the main branch in the meantime, and report back on any issues. Thanks.

@alex223125
Copy link

alex223125 commented Feb 24, 2023

I have same issue after installing gem 'devise-security', '~> 0.17.0' Not sure what causes it, but after I removed that gem everything works fine. I had problem in new form (simple form gem, devise registrations controller).

@carlosantoniodasilva
Copy link
Member

@alex223125 I don't know much about that gem to help, sorry. They may need some tweak to work with Rails 7 and / or this latest Devise version.

@botsarenthuman
Copy link

botsarenthuman commented Apr 4, 2023

What about the case where the object is not persisted? I'm getting the same error on this line

https://github.com/heartcombo/devise/blob/main/app/controllers/devise/registrations_controller.rb#L34

      clean_up_passwords resource
      set_minimum_password_length
      respond_with resource

@carlosantoniodasilva
Copy link
Member

@botsarenthuman my first wild guess is that you're hitting something like this: #5565 (comment), where the object does not contain any errors associated with it (even if it's not persisted), so responders will try to redirect (it uses the presence of errors to determine how to respond), causing the error.

MartinSugasti added a commit to MartinSugasti/simplest-resumes that referenced this issue Aug 25, 2023
… the invitations limit..

I added data: { turbo: false } following the suggestion heartcombo/devise#5439
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet