Survey/Question/Answer validation with fields_for?

rubyonrails-talk
1

Imagine we had a basic survey application (Rails 4.2 with Devise) similar to the following:

class User < ActiveRecord::Base

Assume devise user here

has_many :user_surveys

has_many :surveys, through: :user_surveys

has_many :responses

end

class UserSurvey < ActiveRecord::Base

belongs_to :user

belongs_to :survey

end

class Survey < ActiveRecord::Base

has_many :questions

has_many :user_surveys

has_many :users, through: :user_surveys

accepts_nested_attributes_for :questions

name: string

end

class Question < ActiveRecord::Base

belongs_to :survey

has_many :responses

accepts_nested_attributes_for :responses

response_text: string

end

Class Response < ActiveRecord::Base

belongs_to :question

belongs_to :user

response_text: string

end

The idea being, an admin can create a survey with questions, and then assign that survey to users through the join model UserSurvey.

Now imagine a basic form that we give the user for their survey that looks like this:

<%= @survey.name %> Answers

<%= form_for(@survey) do |f| %>

<%= current_user.name %>

<% @questions.each do |question| -%>

<% end -%>

Questions Response
<%= question.question_text %>

<%= f.fields_for :questions, question do |q| -%>

<%= q.fields_for :responses, question.responses.find_or_initialize_by(user: current_user.id) do |r| -%>

<%= r.text_area :response_text %>

<%= r.hidden_field :user_id, current_user.id %>

<% end -%>

<% end -%>

<%= f.submit %>

<% end -%>

My main question revolves around the following line (under fields_for responses):

<%= r.hidden_field :user_id, current_user.id %>

I see it suggested other places (eg: here on stackoverflow, or this tutorial here ) to put a hidden user_id field, but this feels incredibly wrong to me – if a user is malicious, they could edit the form and modify the hidden user_id field to modify another participants answer.

Now, the one solution I did think was that on the controller side I could mess with the params, IE assume the submitted parameters look as follows

“survey”=>{“questions_attributes”=>{“0”=>{“responses_attributes”=>{“0”=>{“response_text”=>“one”}}, “id”=>“7”}, “1”=>{“responses_attributes”=>{“0”=>{“response_text”=>“two”}}, “id”=>“8”}}}

I could do something like:

def update

@survey = current_user.surveys.find(params[:id])

@questions = @survey.questions

params[:survey][:question_attributes].each do |key, attribs|

question = @questions.find(attribs[:id])

response = question.responses.where(:user_id => current_user.id).first_or_initialize

response.response_text = attribs["responses_attributes"]["0"]["response_text"]

response.save!

end

end

This way, I not only check to make sure the survey is attached to the user, but also make sure the questions they are submitting are attached to that specific survey, and the responses are not only tied to the correct question, but also back to the correct user. This feels messy though, and I feel I’m majorly overthinking this.

Can anyone give me a sanity check as to what I’m doing wrong?

2

In the controller check that the id from the form matches that for the current logged in user. In fact not sure why you need it in the form at all, as it should just be that for the current user. Possibly I am missing something in your question.

Colin

3

At the end of my email, I do just that – you notice that in the params I suggest, there is no user_id present, and instead I break apart the params and inject the user_id more manually, but breaking out the params seems messy to validate against, so I’m wondering if there is a more “cleaner” method of doing it.

4

That is not quite what I was suggesting, your question indicated you were happy with the hidden field concept, apart from the fact that someone might inject a false id. My suggestion was to use the hidden field but then to verify it against current_user in the controller. So just one test to insert in the controller.

Colin

5

Does it make sense to not have this hidden_field in the form but instead add this info inside the controller while saving the object? For instance:

Response.new(response_params)

def response_params

params.require(:response).permit(:response_text).merge(user_id: current_user.id)

end

6

That definitely makes more sense in my mind fernando, but if its a deeply nested object, can you do that? As the params would probably look something like:

params.require(:survey).permit(:question_attributes => [:response_attributes => [:response_text].merge(user_id: current_user.id)])

7

If the logic to create your object is more complex than a few lines of code, you could extract it to a separate class. I used to create adapter classes which build model objects from the request parameters. That worked well for me.