ActiveRecord custom validation problem

Question

Hi,

I'm having a problem with validation in my RoR Model:

def save
  self.accessed = Time.now.to_s
  self.modified = accessed
  validate_username
  super
end

def validate_username
  if User.find(:first, :select => :id, :conditions => ["userid = '#{self.userid}'"])
    self.errors.add(:userid, "already exists")
  end
end

As you can see, I've replaced the Model's save method with my own, calling validate_username before I call the parent .save method. My Problem is, that, even though the error is being added, Rails still tries to insert the new row into the database, even if the user name is a duplicate. What am I doing wrong here?

PS: I'm not using validate_uniqueness_of because of the following issue with case sensitivity: https://rails.lighthouseapp.com/projects/8994/tickets/2503-validates%5Funiqueness%5Fof-is-horribly-inefficient-in-mysql

Update: I tried weppos solution, and it works, but not quite as I'd like it to. Now, the field gets marked as incorrect, but only if all other fields are correct. What I mean is, if I enter a wrong E-Mail address for example, the email field is marked es faulty, the userid field is not. When I submit a correct email address then, the userid fields gets marked as incorrect. Hope you guys understand what I mean :D

Update2: The data should be validated in a way, that it should not be possible to insert duplicate user ids into the database, case insensitive. The user ids have the format "user-domain", eg. "test-something.net". Unfortunately, validates_uniqueness_of :userid does not work, it tries to insert "test-something.net" into the database even though there already is an "Test-something.net". validate_username was supposed to be my (quick) workaround for this problem, but it didn't work. weppos solution did work, but not quite as I want it to (as explained in my first update).

Haven't figured this out yet... anyone?

Best regards, x3ro

Answer 1

How about calling super only if validate_username returns true or something similar?

def save
  self.accessed = Time.now.to_s
  self.modified = accessed
  super if validate_username
end

def validate_username
  if User.find(:first, :select => :id, :conditions => ["userid = '#{self.userid}'"])
    self.errors.add(:userid, "already exists")
    return false
  end
end

... I think that you could also remove totally the super call. Not sure, but you could test it out.

Answer 2

Why don't you use a callback and leave the save method untouched? Also, avoid direct SQL value interpolation.

class ... < ActiveRecord::Base

  before_save :set_defaults
  before_create :validate_username

  protected

  def set_defaults
    self.accessed = Time.now.to_s
    self.modified = accessed
  end

  def validate_username
    errors.add(:userid, "already exists") if User.exists?(:userid => self.userid)
    errors.empty?
  end

end