Rails before_validation strip whitespace best practices

Question

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.

Answer 1

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.

Answer 2

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).

Answer 3

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.

Answer 4

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.