should_* macros not working for validations that depend on the state of the object?

[Christos Zisopoulos]

Hi,

Let me quickly explain what I have:

class User << AR::Base
attr_accessor :signup_step
validates_acceptance_of :terms_and_conditions, :if => Proc.new {|u|
u.signup_step == 2}
end

If I were to try and do a
'should_require_acceptance_of :terms_and_conditions' I would not be
able to target a User object that is in its second part of the signup
because the should_* macros code always creates a new object and
cherry picks errors on specific attributes.

Am I correct, or am I missing something obvious? And if so, can
someone suggest a better way of doing this?

Cheers!

-christos

[Ryan McGeary]

Hi. I too have had a similar need to tests validations that are
dependent on object state. Unless there's already a good idiomatic
way of handling this I was thinking that maybe adding an
optional :attributes or :object option to the Active Record shoulda
helpers. Here are some examples:

A) should_require_acceptance_of :terms_and_conditions, :attributes =>
{ :signup_step => 2 }
B) should_require_attributes :password_confirmation, :object =>
Person.new(:password => "something")

I can see A becoming ugly fast and B isn't much better. Perhaps
something like this:

C) should_require_attributes :password_confirmation, :object =>
"@person"

...where @person is constructed in the setup.

D) Maybe a combination of B and C where the logic is based on whether
a string (to be eval'd) is passed.

None of these really work well when you have a set of validations for
a particular object state.

E) Another alternative I can think of is to standardize on a specially
named instance variable (e.g. @shoulda_object) that is specified in
the setup. Use it to test the validation instead of 'klass.new' if it
exists.

After talking through this, D or E seem reasonable. D doesn't feel
optimal. E is simple, but I'm unsure about the special variable.
I'd be willing to work on this if any of it seems worthwhile to
others.

What does everyone else think?
-Ryan

[Tammer Saleh]

I've been thinking about this as well, and I think a variation on E
would be best. Each active_record macro should find an object to work
on like such:

1) use "@#{model_class.to_s.downcase}" if that instance variable
exists (eg @user or @post)
2) use model_class.find(:first) if there's one in the dB
3) use model_class.new

This should allow you to use the macros inside a context where you've
created a model, as long as you name the instance variable after the
class. For example:

context "An admin user" do
setup { @user = Factory(:user, :admin => true) }
should_require_attributes :name
end

[Ryan McGeary]

Tammer, Nice. I like that convention very much.

Probably just an oversight, but I think the instance variable should be "@#{model_class.to_s.underscore}"  (instead of downcase) for multi-word class names.

I'm not sure I understand the purpose of step 2 (using the first db row).  This actually conflicts with a patch I submitted last night :-)

http://rubyurl.com/c5Y8

Is there a scenario I'm not thinking of that the database access is required or preferred?  I would argue that if you want to use the first db row, you should have the setup take care of it.  E.g.:

setup { @user = User.find(:first) }

Again, I'm happy to work on this since some colleagues and I have a direct need, but I'll wait for a response as to whether my patch that avoids the db is acceptable or not.

-Ryan

[Chad Pytel]

Yeah, I agree with Ryan.  The first DB row is too hidden, and could lead to confusion, and if thats what someone wants, they can just assign it in the setup.  The instance variable is straightforward, and the macros could provide a nice error message if it isn't found.

-Chad

---

Chad Pytel, President

thoughtbot, inc.

organic brains. digital solutions.

-------------------------------------------

tel: 617.482.1300 x113

fax: 866.217.5992

http://www.thoughtbot.com

[Eric Mill]

I get why we've arrived at a solution involving an instance variable
convention, but I'd love it if we can think of any better convention.

What if the context method took an optional object for a second
parameter?

context "Admins", User.create(:admin => true) do
should_require_attributes :name
end

In practice maybe you'd set an instance variable in a setup block with
an @admin user or something, but at least you wouldn't have to do
that. You could take it one step further and have the context method
look to see if it's first arg isn't a string:

setup do
@admin = User.create :admin => true
end

context @admin do
should_require_attributes :name
end

But maybe that's getting too crazy.

-- Eric

[Tammer Saleh]

I think that's tying contexts to rails too much. Having the instance
variable convention works just as well, but only requires the AR
macros to be aware of the convention (as opposed to the fundamental
building blocks of shoulda).

Tammer

[Eric Mill]

I see what you mean, but does it really have to be Rails-specific?
You could just have the logic to be to do internally what you were
already suggesting we do, assign the result to an instance variable
named after the model. This could be nice sugar even outside of
ActiveRecord.

context "Trees", Tree.new(:sap => 20) do
should "survive for at least 2 days" do
assert @tree.alive_after(2)
end
end

Basically, it'd eliminate the need for tiny setup blocks, as well as
make it easier to write contexts that only use simple helpers, such as
your ActiveRecord helpers.

-- Eric

> >>ChadPytel, President

[Ryan McGeary]

Eric,

I understand what you're getting at, but I don't think this necessarily applies to the original premise of this thread.  Besides, you can already simulate this behavior yourself by writing your own context extension macro.  E.g.:

  def self.context_for(object, &block)

    klass = object.class

    context "A #{klass}" do

      setup do

        instance_variable_set("@#{klass.name.underscore}", object)

      end

      block.bind(self).call

    end

  end

And with usage like this:

  context_for Document.new do

    should "have a paperclip" do

      assert @document.paperclip?

    end

  end

Now, maybe a context_for macro might be useful in the core shoulda library, but again, let's break this discussion into a different thread.

-Ryan

[Ryan McGeary]

Support for stateful Active Record validations is now available in shoulda.  Here is an example:

  context "A tangible product" do

    setup { @product = Product.new(:tangible => true) }

    should_require_attributes :price

  end

If an instance variable exists with the same name as the model class that is being tested, the value of that instance variable is used when running the underlying assertions.

For a more concrete example, please look at some of the core shoulda tests:

http://github.com/thoughtbot/shoulda/tree/master/test/rails_root/app/models/product.rb

http://github.com/thoughtbot/shoulda/tree/master/test/unit/product_test.rb

Feedback welcome,

Ryan

On Sat, Jun 28, 2008 at 8:32 AM, Tammer Saleh <tsa...@gmail.com> wrote: