views:

36

answers:

1

I have a problem where an :if clause within an ActiveRecord validation is not being honoured.

My model has an ip_port attribute which I am validating as being present, numeric and within a certain range. I am trying to ensure that each condition only generates one error. I do not want the situation where an empty attribute results in three messages displayed to the user stating that it is not present, required and not numeric.

This is my model as it stands

class Arc < ActiveRecord::Base
  attr_accessible :ip_port

  validates_presence_of :ip_port
  validates_numericality_of :ip_port, :allow_blank => true
  validates_inclusion_of :ip_port, :in => 1025..65535, :allow_blank => true, 
     :if => Proc.new {|arc| arc.ip_port.to_s.match(/^\d+$/) }
end

And this is my model spec and its results.

describe Arc do
  it "should be valid with valid attributes" do
    Arc.new(:ip_port => 1200).should be_valid
  end
  it "should be invalid with a non-numberic port" do
    Arc.new(:ip_port => "test").should be_invalid
  end
  it "should be invalid with a missing port" do
    Arc.new(:ip_port => nil).should be_invalid
  end
  it "should have one error with a missing port" do
    a = Arc.new(:ip_port => nil)
    a.should be_invalid
    a.should have(1).errors_on(:ip_port)
  end
  it "should have one error with a non-numeric port" do
    a = Arc.new(:ip_port => "test")
    a.should be_invalid
    a.should have(1).errors_on(:ip_port)
  end
  it "should have one error with a numeric port outside the range" do
    a = Arc.new(:ip_port => 999)
    a.should be_invalid
    a.should have(1).errors_on(:ip_port)
  end
end
Arc
- should be valid with valid attributes
- should be invalid with a non-numberic port
- should be invalid with a missing port
- should have one error with a missing port
- should have one error with a non-numeric port (FAILED - 1)
- should have one error with a numeric port outside the range

1)
'Arc should have one error with a non-numeric port' FAILED
expected 1 errors on :ip_port, got 2
./spec/models/arc_spec.rb:21:

Finished in 0.108245 seconds

My question is why I am getting two errors for a non-numeric ip_port when the :if clause should prevent the validates_inclusion of from being called.

This is Rails 2.3.5 with Ruby 1.8.7 on OS/X 10.6.3

+1  A: 

Whilst having a contemplative stroll I have solved my own problem.

The issue is that in order to validate the inclusion within the range it converts the provided value to an int and then checks for inclusion. So for a non-numeric value I will get both a :not_a_number and an :inclusion error.

The answer is to modify the :if clause to use the value before it was typecast so my validates_inclusion_of method becomes

validates_inclusion_of :ip_port, :in => 1025..65535, :allow_blank => true, 
  :if => Proc.new {|arc| arc.ip_port_before_type_cast.to_s.match(/^\d+$/) }

This then gives me one error for each of three conditions.

Steve Weet
If you're using the value before type cast, do you still need the to_s? (Not saying you're definitely wrong - just asking for my own ignorance)
Chris
@Chris. Yes you do. If the ip_port attribute is assigned an integer then the validation will fail as the match will fail on Fixnum.
Steve Weet
Ah, of course, because this is *writing to* the database. Continuing to explore the bounds of my knowledge - *why* does the numericality validation fire when you use `arc.ip_port`?
Chris
@chris. I think I didn't explain myself very well. Without the to_s if I instantiate a new model and then call `model.ip_port = 3000`. When I attempt to validate the model the validates_inclusion macro will call the Proc which will then do 3000.match(...) this will fail as 3000 is an Instance of Fixnum which has no match method.
Steve Weet
I think it may be me who isn't being clear... Basically, I don't fully understand the cause of your original problem. Why, exactly, do you get two errors in the original case?
Chris
Ah Sorry. I got two errors because setting ip_port to "test" would fire the validates_numericality clause. Then to test whether it should run the validates_inclusion clause it would cast the value to an int (value 0) to compare against the range so the :if clause got a value of 0 causing the regex to match and fire the validates_inclusion_of which would then fail.
Steve Weet
Right, yes, I understand. Thanks for explaining!
Chris