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

Offer different user_params method for users#create and users#new #542

Open
derekprior opened this issue Mar 19, 2015 · 10 comments
Open

Offer different user_params method for users#create and users#new #542

derekprior opened this issue Mar 19, 2015 · 10 comments
Milestone

Comments

@derekprior
Copy link
Contributor

Right now both of these actions call user_from_params, which calls user_params. I think this makes life difficult for people using strong parameters as something they attempt to do with the params for create might cause an error in the new action.

I wonder if there's something quick and backward compatible we can do here...

@pedrosmmoreira
Copy link
Contributor

I'd like to help with this, but I'm not sure if I understand the full extent of the issue.

Would it suffice to make user_params check the action key and then call out to different methods? This would at least be quick and backward compatible.

@derekprior
Copy link
Contributor Author

@pedrosmmoreira This came out of this Stack Overflow question: http://stackoverflow.com/questions/29136246/rails-4-and-thoughtbot-clearance-adding-fields-to-the-user-model/29146029

Consider a case where you expect to have different parameter rules for the user you construct for the new action and the user you construct in create. Both of these actions call user_from_params with no arugments. That method gets the params from user_params, again with no params. So there's no obvious way for user_params to know if it should be using params for the new action or params for the create action.

Using the action key is one way to do it, but I don't think I want to do that. Feels wrong. Not sure what right is though... any other ideas?

@pedrosmmoreira
Copy link
Contributor

Ah, I see. Whenever I've had to deal with this, I've used the same mechanism that is in place: deleting the param from the hash and assigning it to a local var that can get used in the block. For adding a couple of attrs, that doesn't get too bad.

If I am customizing beyond a couple of attrs, I resort to a form object, since at that point it is quite likely I am not pushing everything onto the user anyway.

I do agree that checking the key is "smelly", but other than some kind of 'Registration' object, I'd go with documenting that 'user_from_params' is the place to extend behaviour. I could also see the initializer as a place for whitelisting, in order to keep compatibility.

@j-dexx
Copy link
Contributor

j-dexx commented Dec 24, 2015

@derekprior Using rails 5 this breaks completely for me with a forbidden attributes error.

@derekprior
Copy link
Contributor Author

@j-dexx What breaks completely? Sign up?

@j-dexx
Copy link
Contributor

j-dexx commented Jan 10, 2016

@derekprior Sorry, that was a pretty unhelpful comment. I can't find where I had an issue and having created a simple app to try and replicate it I can't.

@DavidVII
Copy link

DavidVII commented Feb 8, 2016

@j-dexx I just ran into the forbidden attributes error as well. Happened when I tried to add a name field to the users/form partial. I got around it by adding a user_params method in my own users_controller. I kept getting the error until I remembered to update the routes file to use my own users controller rather than the clearance/users_controller.

@derekprior derekprior added this to the 2.0 milestone Apr 29, 2016
@krsyoung
Copy link

@DavidVII (one year later) I'm hitting the same thing. Any chance you can (remember and) share the code you used? I take it you subclassed your users_controller and inherited from clearance/users_controller? Then made sure to not clobber the new and create methods but did define a custom user_params that makes use of strong parameters?

@DavidVII
Copy link

Ya, you pretty much summed up my approach. I overwrote the user params and the create action (I needed to do a few extra things). Here's some code.

class UsersController < Clearance::UsersController

  def create
    @user = user_from_params

    if @user.save
      sign_in @user

      # unique tasks I needed done went in here

      redirect_to new_order_path
    else
      render template: "users/new"
    end
  end

  private

  def user_params
    params[:user].permit(:email, :password, :name, :phone_number)
  end
end

@krsyoung
Copy link

Beautiful, that's the winner. Thanks so much @DavidVII! 👍

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

No branches or pull requests

6 participants