Require for nested parameters is broken #70

Open
esad opened this Issue Dec 18, 2012 · 6 comments

Projects

None yet

5 participants

@esad
esad commented Dec 18, 2012

Using current API, it's not really possible to do a sane check for a nested param. Consider the following check:

params.require(:user).require(:id)

If you now supply ?user=test as a param, the second require call will be sent to a string instance.

This could be solved if require would support nested syntax, i.e.:

params.require("user.id")

The above syntax would make code more readable for deeper nested params: params.require(:user).require(:book).require(:authorization).require(:token) becomes
params.require('user.book.authorization.token')

Any thoughts?

reggieb commented Dec 21, 2012

Looking at the code, require is best used to identify the root element, and permit to define the permissible children of that element. So for your example you would do:

def user_params
  params.require(:user).permit(:id)
end

or

def user_params
  params.require(:user).permit(:authorization, :token, {book: [:title, :author]})
end
reggieb commented Dec 21, 2012

Looking at this again, this should be a valid input:

params.require(:user).require(:id)

As is evident from this test: action_controller_required_params_test.rb

The problem is Parameters#require:

def require(key)
  self[key].presence || raise(ActionController::ParameterMissing.new(key))
end

presence returns the value of self[key], so it is the child that is being passed along chain and not the root hash. Used this way require will do some unexpected things. For example:

prams = {
  user: {
    name: Fred,
    book: {title: 'A book'}
  }
}

def user_params
  params.require(:user).require(:book)
end

user_params ---->   {title: 'A book'}

That is, the object returned would be the value of the last required item.

So a fix could be to rewrite Parameters#require, so that it returns the root hash (self), and accepts a syntax that allows you to identify which nested parameters are required.

Permit already has much of that functionality. So you could code Parameters#require so that it goes through each element and raises an exception if it doesn't find them in the parent hash, and then passed them onto permit to do its magic.

Then the syntax would become:

def user_params
  params[:user].require(:name, {book: [:title]}).permit(:age, {book: [:author]})
end

This would in effect run permit twice - the first time raising an exception if a parameter is missing. The second time just marking the fields as permitted.

Note that params[:user] is called first, as require is no longer passing on a part of the root hash, but the root hash itself.

There needs to be a way to make sure the nested model parameters are required and then permit those nested values.

rodrei commented May 28, 2013

I'm finding myself in a similar situation. I need to require nested params and the current implementation doesn't provide that functionality.

@reggieb The approach you proposed solves the issue and keeps the syntax clear.

No one has had a need for requiring nested parameters since 2013?

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