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

permit_all! to permit all attributes in a given key. #14317

Closed
wants to merge 1 commit into from

Conversation

alvarezloaiciga
Copy link

No description provided.

@arthurnn
Copy link
Member

arthurnn commented Mar 7, 2014

Not sure about the syntax. Maybe we should try to come up with a better API.

@alvarezloaiciga
Copy link
Author

what can you recommend? I was also thinking in something like ActionController::AnyParam to avoid problems with keys

@senny
Copy link
Member

senny commented Mar 8, 2014

/cc @fxn

@carlosantoniodasilva
Copy link
Member

Based on the tests, how's this different than:

params.require(:book).permit!

@alvarezloaiciga
Copy link
Author

@lanej
Copy link

lanej commented Mar 18, 2014

I had a similar conversation at the tail end of #9454.

I personally just need something works. There is no solution that I know of whose aesthetics pleases everyone.

@ss2k
Copy link

ss2k commented Mar 18, 2014

I am in favor of this.

@hertling
Copy link

hertling commented Apr 3, 2014

We need this!

@Draiken
Copy link
Contributor

Draiken commented Apr 4, 2014

This is really needed for custom data in form of hashes

@NikoRoberts
Copy link

+1 for reviewing this use case and coming up with a strong solution

e.g. translated fields seem to be a big pain either a loop is needed for allowing each locale variation of the field, or an explicit .permit! statement for each translated field and passing in the fields as a hash

cc @guy-silva

@guilleiguaran
Copy link
Member

As @carlosantoniodasilva and @rafaelfranca said this is possible to do using permit!

@Draiken
Copy link
Contributor

Draiken commented Apr 24, 2014

For example, I want to send a message with complete custom content

{ 
  "message" : { 
    "user_id" : 1
    "content" : { 
      "data" : "Hello world",
      "custom_field" : "custom data"
    } 
}

In this case I want to make sure message is required and with a user id and a content.

params.require(:message).permit(:user_id, :content)

This does not work, because inside content are other keys that are not whitelisted. But I want to permit any attribute only for the :content key, not every key. And I can't use a predefined whitelist of something that's meant to be completely open.

Either I'm missing something or this is not possible with permit!

Edit: better phrasing

@lanej
Copy link

lanej commented Apr 24, 2014

@Draiken as @rafaelfranca has suggested the solution to your example would be:

params.require(:message) # verify presence
params[:message][:content].permit! # this is the key piece
params.require(:message).permit(:user_id, :content)

This above solution is not great.

I personally love the solution that @alvarezloaiciga has put forth in this PR but none of the rails devs are biting.

@Draiken
Copy link
Contributor

Draiken commented Apr 24, 2014

Oh well, at least I can make it work, as awkward as it ends up looking. Thanks @lanej :)

@alvarezloaiciga
Copy link
Author

Yeah I had the same issue, the current version of it, is not scalable. The deeper the hash is, the harder it is to permit all params. @lanej do you have an idea on how can get rails devs in here?

@AlexCppns
Copy link

@senny I just tried what's in the documentation with little success. My current fix is to not use permit use permit! ... I wish there were a way to arbitrary json/hash in a model attribute, postgresql supports json as a data format after all.

@senny
Copy link
Member

senny commented Mar 9, 2015

@AlexCppns sorry but you need to provide more context. What is not working with:

params.require(:product).permit(:name).tap do |whitelisted|
  whitelisted[:data] = params[:product][:data]
end

@AlexCppns
Copy link

@senny What I saw in the documentation link you provided was:

def product_params
  params.require(:product).permit(:name, data: params[:product][:data].try(:keys))
end

I will try your solution, what is the syntax when you have several attributes, half are json and the others are regular attributes?

@senny
Copy link
Member

senny commented Mar 9, 2015

@AlexCppns the one in the docs should work for flat json documents. If you have arbitrary json you can use the snippet I provided above. There is no special syntax for these attributes, it's just regular ruby, effectively by-passing the strong-parameters api by directly assigning the attributes:

JSON_ATTRS = [:payload, :data]

params.require(:product).permit(:name).tap do |whitelisted|
  JSON_ATTRS.each do |attr|
    whitelisted[attr] = params[attr]
  end
end

@AlexCppns
Copy link

Oh I see, sorry I was misreading it.

@joelpresence
Copy link

Can we reopen this issue please? I'm trying to pass in a serialized Hash with arbitrary keys and the current implementation of strong parameters just doesn't work in this case ... This is a pretty common use case actually. :-)

Thanks!

@senny
Copy link
Member

senny commented Jun 11, 2015

@joelpresence I think the exact scenario you describe is listed in our guides. See http://guides.rubyonrails.org/action_controller_overview.html#outside-the-scope-of-strong-parameters

@rhymes
Copy link

rhymes commented Nov 11, 2015

@senny that works only with flat JSON, if you have hashes nested in hashes for now the only way to bypass is:

params.require(:device).permit(:name).tap do |whitelisted|
  whitelisted[:data] = params[:device][:data] if params[:device][:data]
end

@andersodt
Copy link

@senny @rhymes this whitelist approach works in Rails 4, but upon upgrading to the Rails 5 beta, I seem to lose this functionality.

@senny
Copy link
Member

senny commented Feb 8, 2016

@andersodt could you be a bit more specific. What is no longer working?

irb(main):006:0> Rails.version
=> "5.0.0.beta2"
irb(main):007:0> params.require(:person).permit(:first_name, payload: params[:person][:payload].try(:keys))
=> {"first_name"=>"Bob", "payload"=>{"one"=>"One", "two"=>"Two"}}

@andersodt
Copy link

I have a Question model with a JSON type that stores different information about its possible answers. Depending on the question_type, a different structure might get thrown into the data column. Multiple choice, for instance, would look something like this:

{
    "question" => {
        "data" => {
            "0" => {
                "question_data_text" => "abc"
            }, "1" => {
                "question_data_text" => "one two three"
            }
        }, "prompt" => "question_prompt", "question_type" => "multiple_choice", "folder_id" => "1"
    }, "id" => ""
}

Neither of these approaches saves the "data"

params.require(:question).permit(:questoin_type, :folder_id, :prompt).tap do |whitelisted|
  whitelisted[:data] = params[:question][:data] if params[:question][:data]
end
params.require(:question).permit(:question_type, :folder_id, :prompt, params[:question][:data].try(:keys))

In Rails 4, I was using the whitelisted approach and it worked great, but haven't had any success since upgrading to the beta.

@senny
Copy link
Member

senny commented Feb 9, 2016

@andersodt what about:

params.require(:question).permit(:prompt, :question_type, :folder_id).tap { |w| 
  w[:data] = params[:question][:data].to_unsafe_h if params[:question][:data] 
}

Note the .to_unsafe_h to make sure there are no restrictions when accessing :data.

@andersodt
Copy link

That did it. The "unpermitted parameter" notice still appears on the console but the data hash is being properly saved.

Thanks!

@senny
Copy link
Member

senny commented Feb 10, 2016

@andersodt yes, the unpermitted parameter notice is printed by the permit call. That is the same pre 5.0 though.

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

Successfully merging this pull request may close these issues.