Reputation: 31660
I have a Rails app that lets a user construct a database query by filling out an extensive form. I wondered the best practice for checking form parameters in Rails. Previously, I have had my results
method (the one to which the form submits) do the following:
if params[:name] && !params[:name].blank?
@name = params[:name]
else
flash[:error] = 'You must give a name'
redirect_to :action => 'index'
return
end
But for several form fields, seeing this repeated for each one got tiresome. I couldn't just stick them all in some loop to check for each field, because the fields are set up differently:
params[:name]
params[:image][:font_size]
Etc. This was also repetitive, because I was setting flash[:error]
for each missing/invalid parameter, and redirecting to the same URL for each one. I switched to using a before_filter
that checks for all necessary form parameters and only returns true if everything's okay. Then the my results
method continues, and variables are just assigned flat-out, with no checking involved:
@name = params[:name]
In my validate_form
method, I have sections of code like the following:
if (
params[:analysis_type][:to_s] == 'development' ||
params[:results_to_generate].include?('graph')
)
{:graph_type => :to_s, :graph_width => :to_s,
:theme => :to_s}.each do |key, sub_key|
unless params[key] && params[key][sub_key]
flash[:error] = "Cannot leave '#{Inflector.humanize(key)}' blank"
redirect_to(url)
return false
end
end
end
I was just wondering if I'm going about this in the best way, or if I'm missing something obvious when it comes to parameter validation. I worry this is still not the most efficient technique, because I have several blocks where I assign a value to flash[:error]
, then redirect to the same URL, then return false.
Edit to clarify: The reason I don't have this validation in model(s) currently is for two reasons:
Edit to show improved technique: I make use of application-specific exceptions now, thanks to Jamis Buck's Raising the Right Exception article. For example:
def results
if params[:name] && !params[:name].blank?
@name = params[:name]
else
raise MyApp::MissingFieldError
end
if params[:age] && !params[:age].blank? && params[:age].numeric?
@age = params[:age].to_i
else
raise MyApp::MissingFieldError
end
rescue MyApp::MissingFieldError => err
flash[:error] = "Invalid form submission: #{err.clean_message}"
redirect_to :action => 'index'
end
Upvotes: 22
Views: 53056
Reputation: 8561
class Person
include ActiveModel::Validations
include ActiveModel::Conversion
extend ActiveModel::Naming
attr_accessor :name
attr_accessor :email
validates_presence_of :name,:message => "Please Provide User Name"
validates_presence_of :email,:message => "Please Provide Email"
end
Note that you don't necessarily need to save/persist the model to validate.
@person.name= params["name"]
@person.email= params["email"]
@person.valid?
One you called .valid? method on the model, the errors will be populated inside @person object. Hence,
<%if @person.errors.any? %>
<%@person.errors.messages.each do|msg| %>
<div class="alert alert-danger">
<%=msg[0][1]%>
</div>
<%end%>
<%end%>
Upvotes: 1
Reputation: 2057
If you were to tackle the problem again today, you could create a model for the query parameter set and use Rails' built in validations, Rails 3 makes this a lot easier with ActiveModel::Validations see this post.
Upvotes: 2
Reputation:
You could try active_form (http://github.com/cs/active_form/tree/master/lib/active_form.rb) - just ActiveRecord minus the database stuff. This way you can use all of AR's validation stuff and treat your form like you would any other model.
class MyForm < ActiveForm
validates_presence_of :name
validates_presence_of :graph_size, :if => # ...blah blah
end
form = MyForm.new(params[:form])
form.validate
form.errors
Upvotes: 25
Reputation: 954
Looks like you are doing the validation in the controller, try putting it in the model, it's better suited to that sort of thing.
Upvotes: 8