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

Failed user registration redirects to /users which isn't a valid get url #4573

Closed
tae8838 opened this issue Jul 2, 2017 · 14 comments
Closed
Labels

Comments

@tae8838
Copy link

tae8838 commented Jul 2, 2017

After clicking Sign up, the url changes to /users because we post to /resource. If failed, we do respond_with resource without redirecting. If refreshed, No route matches [GET] "/users" appears.

Could we have failed sign up redirect to the same page?

@tegon
Copy link
Member

tegon commented Dec 20, 2017

As far as I know, this is the default of a Rails scaffold application. The problem is that we don't have a index route for the users resource.
I don't see any problem in changing this behavior.
WDYT @feliperenan @rafaelfranca?

@iorme1
Copy link
Contributor

iorme1 commented Apr 13, 2018

I will be working on a PR for this issue within the next few days.

@iorme1
Copy link
Contributor

iorme1 commented Apr 16, 2018

Unfortunately, redirecting to the same page here loses the "devise_error_messages!" method call in the view because the resource.errors is empty after redirect, and the devise_error_messages! call checks to see if 'resource.errors.empty?' before returning the sentence about the errors and error messages. You can redirect to the same page and keep the error messages themselves, but it won't display the sentence/error display like it does currently through the devise_error_messages! call in the view. I can't seem to figure out a way around this. I would really like to address this and make a PR, so any help would be much appreciated!

@iorme1
Copy link
Contributor

iorme1 commented Apr 25, 2018

@tegon , just making sure you see my last message on this issue. Trying to clarify how you want this issue solved exactly.

@lancecarlson
Copy link
Contributor

lancecarlson commented May 1, 2018

@iorme1 It's kind of a crappy hack, but you could POST to /users/sign_up instead of /users. The alternative would be to hack up the way devise/rails supports validation errors and store it in the session. Neither option is great, but the POST method is certainly much easier.

@tegon
Copy link
Member

tegon commented May 8, 2018

@iorme1 Sorry for taking so long, I was on vacation and didn't see this. I don't have a better solution right now, I'll have to think better about it.

@sarunw
Copy link

sarunw commented Aug 28, 2018

What is the solution for this?

@tegon
Copy link
Member

tegon commented Aug 28, 2018

Sorry, I haven't take a look at this yet. This is the default behavior we get when scaffolding a Rails CRUD, so I'm not sure what path to follow here.

@phillipspc
Copy link

phillipspc commented Dec 6, 2018

Just to add on here, this also appears to happen when the password reset email form errors.
users/password/new becomes /users/password, which lacks a route. As a temporary fix, I've just been doing:

# config/routes.rb
devise_scope :user do
  get '/users', to: 'devise/registrations#new'
  get '/users/password', to: 'devise/passwords#new'
end

@connorshea
Copy link
Contributor

I had this problem when using the registrations#edit form in a custom settings view.

I ended up replacing the update path in my registrations controller with, essentially, the code from the devise repo instead of calling super.

Before:

# app/controllers/users/registrations_controller.rb

# PUT /resource
def update
  skip_authorization
  super
end

After:

# app/controllers/users/registrations_controller.rb

# PUT /resource
def update
  skip_authorization
  self.resource = resource_class.to_adapter.get!(send(:"current_#{resource_name}").to_key)
  prev_unconfirmed_email = resource.unconfirmed_email if resource.respond_to?(:unconfirmed_email)

  resource_updated = update_resource(resource, account_update_params)
  yield resource if block_given?
  if resource_updated
    set_flash_message_for_update(resource, prev_unconfirmed_email)
    bypass_sign_in resource, scope: resource_name if sign_in_after_change_password?

    respond_with resource, location: after_update_path_for(resource)
  else
    clean_up_passwords resource
    set_minimum_password_length
    # Redirect to the settings page.
    redirect_to settings_account_path
  end
end

The only changes I made were to add skip_authorization, which is a Pundit thing that's not really relevant, and changing the respond_with resource line to redirect_to settings_account_path so it would redirect to the page I wanted.

This works (better than it redirecting to /users on error, at least), but it doesn't pass the devise error messages. Any ideas on how I'd go about passing the errors to the redirect?

@tegon
Copy link
Member

tegon commented Oct 14, 2019

Hi everyone,

After giving it some thought the only way to achieve the persistence of error messages seems to be to store them in the session, cookies or some of the alike (please let me know if you have any other ideas).
I don't think the complexity of this is worth it adding to the library and since this is the default Rails' resource behavior, we'll leave as it is for now and let each application handle the way it better fits them.

Hope you understand my reasoning. Thanks.

@tegon tegon closed this as completed Oct 14, 2019
bernb added a commit to bernb/dojam that referenced this issue Oct 26, 2020
By default, devise sticks with rails defaults and redirects to /users after failed edit, which we do not want as there is no user index page.

See heartcombo/devise#4573 and heartcombo/devise#4470 for full reasoning
@jjb
Copy link
Contributor

jjb commented Apr 30, 2022

What's the TLDR on why only registrations has this problem and not sign in and other forms? (btw I am not experiencing this on password reset, which this comment says also has the problem, I wonder if a fix was put in for that in the meantime, but not for registrations #4573 (comment) )

@jjb
Copy link
Contributor

jjb commented Apr 30, 2022

This was all I needed to fix my problems, devise 4.8.1. Maybe I'm missing something that I'll discover later.

get '/users', to: redirect('/users/sign_up')

@404UsernameNotFound404
Copy link

There are lots of solution that fix it, but don't keep the error message. Is there any update on this? My version is 4.8.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

9 participants