[PATCH] Remove warning in validates_presence_of documentation

+1 I actually didn't know that work to now. Very nice.

Save it the other way and it fails. But if you put validates_presence_of and put :foo_id, it will fail both ways.

By "save it the other way" I mean this:

One way:

foo = Foo.new bar = Bar.new foo.bars << bar foo.save

The other way is:

bar = Bar.new foo = Foo.new bar.foo = foo bar.save

If you put the association, it will save one way but not the other. If you put the field (as the documentation suggest), it will fail both ways.

10 models and tests. See my comments below.

I attached a bzip2 tar file of a rails project. The project has ten models and unit tests for each model. The models are in pairs. There is a hmN and a btN model in each pair. The hm is for has_many and the bt is for belongs_to.

The first pair (hm1 and bt1) has a constraint that hm1_id is not null. All of the btN's have this constraint. But the pair has no other validates_... .

The second pair has a validates_presence_of on the association. The third pair has a validates_presence_of on the id. The first has one failure that I would expect to pass. The two failures that I would expect to pass.

The fourth and fifty pairs each have validates_associated -- with the fifth pair adding an :allow_nil => false which appears to not provide any additional functionality since the migration adds the constraint already.

As I mentioned, I've marked three tests that I believe should pass but do not. I found all of this very confusing. There is a comment in the validates_associated that leads you to believe that you also need a validates_presence_of. But my testing shows this to be not only not true but not possible.

It may be that you want to just say "do not use validates_presence_of for foreign keys".

I hope that a unified statement and approach is created because, right now, it seems that this is a very confused subject with a lot of misinformation out there (and in the documentation).

I hope this helps in that cause.

Oh... the other thing I found not crystal clear is that the validates_ ... can or can not be used along with a custom validate method.

Needs two more points...

So from that tar.bz2, what are the cases that currently break, and do we think they justify the warning in the documentation?

A quick recap:

class Hm2 < ActiveRecord::Base
  has_many :bt2s
end
class Bt2 < ActiveRecord::Base
  belongs_to :hm2
  
  validates_presence_of :hm2
end

  def test_hm2_save_w_bt2
    hm2 = Hm2.new
    bt2 = Bt2.new
    hm2.bt2s << bt2
    assert_nothing_raised ActiveRecord::StatementInvalid,
                          "Should be able to save hm2 when bt2 is new" do
      # This currently fails
      assert hm2.save, "Save of hm2 with bt2 should return true"
    end
    assert_not_nil bt2.hm2, "hm2 should not be nil at this point"
    assert_equal hm2.id, bt2.hm2_id, "hm2 ids should match"
  end

---

class Hm3 < ActiveRecord::Base
  has_many :bt3s
end
class Bt3 < ActiveRecord::Base
  belongs_to :hm3
  
  validates_presence_of :hm3_id
end

  def test_bt3_save_w_hm3
    bt3 = Bt3.new
    hm3 = Hm3.new
    bt3.hm3 = hm3
    assert_nothing_raised ActiveRecord::StatementInvalid,
                          "Should be able to save bt3 when hm3 is new" do
      # This currently fails
      assert bt3.save, "Save of bt3 with hm3 should return true"
    end
    assert_not_nil bt3.hm3, "hm3 should not be nil at this point"
    assert_equal hm3.id, bt3.hm3_id, "hm3 ids should match"
  end

  def test_hm3_save_w_bt3
    hm3 = Hm3.new
    bt3 = Bt3.new
    hm3.bt3s << bt3
    assert_nothing_raised ActiveRecord::StatementInvalid,
                          "Should be able to save hm3 when bt3 is new" do
      # This currently fails
      assert hm3.save, "Save of hm3 with bt3 should return true"
    end
    assert_not_nil bt3.hm3, "hm3 should not be nil at this point"
    assert_equal hm3.id, bt3.hm3_id, "hm3 ids should match"
  end

To me, the warnings are more than justified. I even consider the code broken since you can not actually do sensible validation in Rails. You must have db constraints or roll your own validate methods.

Well, ok. Upon further review, the warning is backwards as well as incomplete.

+1. The warning's actively wrong, regardless of whether validates_presence_of :foos works properly or not.

+1, current docs are wrong.

This issue was closed in [8825].