Rails before_validation strip whitespace best practices
I would like my User model to sanitize some input before before save. For now some simple whitespace stripping will do. So to avoid people registering with "Harry " and pretend to be "Harry", for example.
I assume it is a good idea to do this stripping before validation, so that the validates_uniqueness_of can avoid accidental duplicates.
class User < ActiveRecord::Base
has_many :open_ids
validates_presence_of :name
validates_presence_of :email
validates_uniqueness_of :name
validates_uniqueness_of :email
validates_format_of :email, :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i
before_validation :strip_whitespace, :only => [:name, :email, :nick]
private
def strip_whitespace(value)
value.responds_to?('strip') ? value.strip : value
end
end
However, this code comes with an error ArgumentError: wrong number of arguments (0 for 1). I assumed the callback would be passed the values.
Also: is this stripping actually a good idea? Or should I rather validate on space and tell the user that "Harry " contains invalid spacess (I want to allow "Harry Potter" but not "Harry\s\sPotter").
Edit: As pointed out in a comment, my code is wrong (which is why I was asking the question a.o.). Please make sure you read the accepted answer in addition to my question for the correct code and to avoid the same mistakes I made.
I don't believe before_validation
works like that. You probably want to write your method like this instead:
def strip_whitespace
self.name = self.name.strip unless self.name.nil?
self.email = self.email.strip unless self.email.nil?
self.nick = self.nick.strip unless self.nick.nil?
end
You could make it more dynamic if you want using something like self.columns
, but that's the gist of it.
There are several gems to do this automatically. Those gems work in the similar way of creating callback in before_validation. One good gem is at https://github.com/holli/auto_strip_attributes
gem "auto_strip_attributes", "~> 2.2"
class User < ActiveRecord::Base
auto_strip_attributes :name, :nick, nullify: false, squish: true
auto_strip_attributes :email
end
Stripping is often a good idea. Especially for leading and trailing whitespaces. User often creates trailing spaces when copy/pasting value to a form. With names and other identifying strings you also might want squish the string. So that "Harry Potter" will become "Harry Potter" (squish option in the gem).
Charlie's answer is good, but there's a little verbosity. Here's a tighter version:
def clean_data
# trim whitespace from beginning and end of string attributes
attribute_names.each do |name|
if send(name).respond_to?(:strip)
send("#{name}=", send(name).strip)
end
end
end
The reason we use
self.foo = "bar"
instead of
foo = "bar"
in the context of ActiveRecord objects is that Ruby interprets the latter as a local variable assignment. It will just set the foo variable in your method scope, instead of calling the "foo=" method of your object.
But if you are calling a method, there is no ambiguity. The interpreter knows you're not referring to a local variable called foo because there is none. So for example with:
self.foo = foo + 1
you need to use "self" for the assignment, but not to read the current value.
I'd like to add one pitfall that you might experience with the "before_validations" solutions above. Take this example:
u = User.new(name: " lala")
u.name # => " lala"
u.save
u.name # => "lala"
This means you have an inconsistent behavior based on whether your object was saved or not. If you want to address this, I suggest another solution to your problem: overwriting the corresponding setter methods.
class User < ActiveRecord::Base
def name=(name)
write_attribute(:name, name.try(:strip))
end
end
I also like this approach because it does not force you to enable stripping for all attributes that support it - unlike the attribute_names.each
mentioned earlier. Also, no callbacks required.
Instead we can write a better method more generic regardless whatever may be the type of attributes with the object(might have 3 string type fields, few booleans, few numeric)
before_validation :strip_input_fields
def strip_input_fields
self.attributes.each do |key, value|
self[key] = value.strip if value.respond_to?("strip")
end
end
Hope that will helps someone!