7

I have an (I think) relatively straightforward has_many :through relationship with a join table:

class User < ActiveRecord::Base
  has_many :user_following_thing_relationships
  has_many :things, :through => :user_following_thing_relationships
end

class Thing < ActiveRecord::Base
  has_many :user_following_thing_relationships
  has_many :followers, :through => :user_following_thing_relationships, :source => :user
end

class UserFollowingThingRelationship < ActiveRecord::Base
  belongs_to :thing
  belongs_to :user
end

And these rspec tests (I know these are not necessarily good tests, these are just to illustrate what's happening):

describe Thing do     
  before(:each) do
    @user = User.create!(:name => "Fred")
    @thing = Thing.create!(:name => "Foo")    
    @user.things << @thing
  end

  it "should have created a relationship" do
    UserFollowingThingRelationship.first.user.should == @user
    UserFollowingThingRelationship.first.thing.should == @thing
  end

  it "should have followers" do
    @thing.followers.should == [@user]
  end     
end

This works fine UNTIL I add an after_save to the Thing model that references its followers. That is, if I do

class Thing < ActiveRecord::Base
  after_save :do_stuff
  has_many :user_following_thing_relationships
  has_many :followers, :through => :user_following_thing_relationships, :source => :user

  def do_stuff
    followers.each { |f| puts "I'm followed by #{f.name}" }
  end
end

Then the second test fails - i.e., the relationship is still added to the join table, but @thing.followers returns an empty array. Furthermore, that part of the callback never gets called (as if followers is empty within the model). If I add a puts "HI" in the callback before the followers.each line, the "HI" shows up on stdout, so I know the callback is being called. If I comment out the followers.each line, then the tests pass again.

If I do this all through the console, it works fine. I.e., I can do

>> t = Thing.create!(:name => "Foo")
>> t.followers # []
>> u = User.create!(:name => "Bar")
>> u.things << t
>> t.followers  # [u]
>> t.save    # just to be super duper sure that the callback is triggered
>> t.followers  # still [u]

Why is this failing in rspec? Am I doing something horribly wrong?

Update

Everything works if I manually define Thing#followers as

def followers
  user_following_thing_relationships.all.map{ |r| r.user }
end

This leads me to believe that perhaps I am defining my has_many :through with :source incorrectly?

Update

I've created a minimal example project and put it on github: https://github.com/dantswain/RspecHasMany

Another Update

Thanks a ton to @PeterNixey and @kikuchiyo for their suggestions below. The final answer turned out to be a combination of both answers and I wish I could split credit between them. I've updated the github project with what I think is the cleanest solution and pushed the changes: https://github.com/dantswain/RspecHasMany

I would still love it if someone could give me a really solid explanation of what is going on here. The most troubling bit for me is why, in the initial problem statement, everything (except the operation of the callback itself) would work if I commented out the reference to followers.

dantswain
  • 5,427
  • 1
  • 29
  • 37

3 Answers3

9

I've had similar problems in the past that have been resolved by reloading the association (rather than the parent object).

Does it work if you reload thing.followers in the RSpec?

it "should have followers" do
  @thing.followers.reload
  @thing.followers.should == [@user]
end 

EDIT

If (as you mention) you're having problems with the callbacks not getting fired then you could do this reloading in the object itself:

class Thing < ActiveRecord::Base
  after_save { followers.reload}
  after_save :do_stuff
  ...
end

or

class Thing < ActiveRecord::Base
  ...
  def do_stuff
    followers.reload
    ...
  end
end

I don't know why RSpec has issues with not reloading associations but I've hit the same types of problems myself

Edit 2

Although @dantswain confirmed that the followers.reload helped alleviate some of the problems it still didn't fix all of them.

To do that, the solution needed a fix from @kikuchiyo which required calling save after doing the callbacks in Thing:

describe Thing do
  before :each do
    ...
    @user.things << @thing
    @thing.run_callbacks(:save)
  end 
  ...
end

Final suggestion

I believe this is happening because of the use of << on a has_many_through operation. I don't see that the << should in fact trigger your after_save event at all:

Your current code is this:

describe Thing do
  before(:each) do
    @user = User.create!(:name => "Fred")
    @thing = Thing.create!(:name => "Foo")    
    @user.things << @thing
  end
end

class Thing < ActiveRecord::Base
  after_save :do_stuff
  ...

  def do_stuff
   followers.each { |f| puts "I'm followed by #{f.name}" }
  end
end

and the problem is that the do_stuff is not getting called. I think this is the correct behaviour though.

Let's go through the RSpec:

describe Thing do
  before(:each) do
    @user = User.create!(:name => "Fred")
    # user is created and saved

    @thing = Thing.create!(:name => "Foo")    
    # thing is created and saved

    @user.things << @thing
    # user_thing_relationship is created and saved
    # no call is made to @user.save since nothing is updated on the user
  end
end

The problem is that the third step does not actually require the thing object to be resaved - its simply creating an entry in the join table.

If you'd like to make sure that the @user does call save you could probably get the effect you want like this:

describe Thing do
  before(:each) do
    @thing = Thing.create!(:name => "Foo")    
    # thing is created and saved

    @user = User.create!(:name => "Fred")
    # user is created BUT NOT SAVED

    @user.things << @thing
    # user_thing_relationship is created and saved
    # @user.save is also called as part of the addition
  end
end

You may also find that the after_save callback is in fact on the wrong object and that you'd prefer to have it on the relationship object instead. Finally, if the callback really does belong on the user and you do need it to fire after creating the relationship you could use touch to update the user when a new relationship is created.

Peter Nixey
  • 16,187
  • 14
  • 79
  • 133
  • What a pain. I've made an alternative suggestion in the updated answer – Peter Nixey Jan 22 '12 at 18:40
  • The double reload (both in the spec and the callback) turns out to be necessary, but I also needed to call @thing.save (or @thing.run_callbacks(:save)). I'm in a bit of a bind - if I've arrived at a solution to my problem it's thanks to both your suggestions and kikuchiyo's. – dantswain Jan 22 '12 at 18:45
  • 2
    I think Peter's is better. You should give him the bounty. Can I get an up-vote though? :P. Thanks for bringing up the question and Peter for your answer which I also learned from. – kikuchiyo Jan 22 '12 at 18:47
  • 1
    Peter - I'll give you answer credit if you 1) update your answer to point out that a call to `@thing.save` (or `@thing.run_callbacks(:save)`) is necessary in the spec right after `@user.things << @thing` and 2) upvote kikuchiyo's answer. – dantswain Jan 22 '12 at 19:19
  • Hi all. first, thank you both for being so generous, secondly I've upmodded every comment and answer @kikuchiyo has on the page to even things out a bit :) Finally I've updated my answer with the save but also why I think this is happening in the first place. Dantswain, what do you think to the edited suggestion? – Peter Nixey Jan 23 '12 at 09:11
  • Thanks Peter. The issue is that the callback `does` get triggered in Thing.create, before the association to followers is made, in rspec, since after_save is called on both create and updates, and followers is empty at that point and rspec. I like your original answer, using reload in the spec ( but not in the model - best not to modifiy working non-rspec code for the sake of passing spec ). Interestingly calling each on followers, when it is empty breaks the spec, unless handling this with `reload` or `unless followers.empty?`. `reload` is better, but in the spec, not in model :) – kikuchiyo Jan 23 '12 at 10:42
  • Sorry, I ran out of space :P ... and because you can `reload` from the spec, but `unless followers.empty?` must go in the model, your original answer of using `reload` in the spec is a much better solution :) Thank you again for bringing this to our attention. – kikuchiyo Jan 23 '12 at 10:59
  • @kikuchiyo - my pleasure, glad to be of help! – Peter Nixey Jan 23 '12 at 11:46
2

UPDATED ANSWER ** This passes rspec, without stubbing, running callbacks for save (after_save callback included ), and checks that @thing.followers is not empty before trying to access its elements. (;

describe Thing do
  before :each do
    @user  = User.create(:name => "Fred");
    @thing = Thing.new(:name => 'Foo')
    @user.things << @thing
    @thing.run_callbacks(:save)
  end 

  it "should have created a relationship" do
    @thing.followers.should == [@user]
    puts @thing.followers.inspect
  end 
end
class Thing < ActiveRecord::Base
  after_save :some_function
  has_many :user_following_thing_relationships
  has_many :followers, :through => :user_following_thing_relationships, :source => :user

  def some_function
    the_followers = followers
    unless the_followers.empty?
      puts "accessing followers here: the_followers = #{the_followers.inspect}..."
    end
  end
end

ORIGINAL ANSWER **

I was able to get things to work with the after_save callback, so long as I did not reference followers within the body / block of do_stuff. Do you have to reference followers in the real method you are calling from after_save ?

Updated code to stub out callback. Now model can remain as you need it, we show @thing.followers is indeed set as we expected, and we can investigate the functionality of do_stuff / some_function via after_save in a different spec.

I pushed a copy of the code here: https://github.com/kikuchiyo/RspecHasMany

And spec passing thing* code is below:

# thing_spec.rb
require 'spec_helper'

describe Thing do
    before :each do
        Thing.any_instance.stub(:some_function) { puts 'stubbed out...' }
        Thing.any_instance.should_receive(:some_function).once
        @thing = Thing.create(:name => "Foo");
        @user  = User.create(:name => "Fred");
        @user.things << @thing
    end

    it "should have created a relationship" do
        @thing.followers.should == [@user]
        puts @thing.followers.inspect
    end
end
# thing.rb
class Thing < ActiveRecord::Base
    after_save :some_function
    has_many :user_following_thing_relationships
    has_many :followers, :through => :user_following_thing_relationships, :source => :user

    def some_function
        # well, lets me do this, but I cannot use @x without breaking the spec...
        @x = followers 
        puts 'testing puts hear shows up in standard output'
        x ||= 1
        puts "testing variable setting and getting here: #{x} == 1\n\t also shows up in standard output"
        begin
            # If no stubbing, this causes rspec to fail...
            puts "accessing followers here: @x = #{@x.inspect}..."
        rescue
            puts "and this is but this is never seen."
        end
    end
end
kikuchiyo
  • 3,391
  • 3
  • 23
  • 29
  • Yes, I need to access `followers` in the callback. I said in the question that if I comment that line out then the test passes. Interesting about `@x`, though - that we can assign `@x = followers` and the test passes. – dantswain Jan 22 '12 at 14:05
  • Cool. We can get your test passing and keep you model code the way you need it by stubbing out :do_stuff. Adding this to the top of the before_each will pass the test for us `Thing.any_instance.stub(:some_function) { puts 'hi' }`. – kikuchiyo Jan 22 '12 at 16:33
  • I need to be able to test that the stuff in `some_function` is happening. – dantswain Jan 22 '12 at 17:00
  • - passes spec, no stubbing, callback accesses followers ( note the standard output from the callback ) – kikuchiyo Jan 22 '12 at 17:46
  • Thanks. I'm digesting this. A couple things I can change: 1) `@thing = Thing.create!` instead of `new`, and 2) instead of creating `the_followers`, just wrap whatever I want to do inside an `unless followers.empty?`. Do you know what's going on here? I'd like to *understand* why this is happening. Is it a bug in rspec-rails, or (more likely) a hole in my mental model of how this should work? – dantswain Jan 22 '12 at 18:10
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/6941/discussion-between-kikuchiyo-and-dantswain) – kikuchiyo Jan 22 '12 at 18:31
1

My guess is that you need to reload your Thing instance by doing @thing.reload (I'm sure there's a way to avoid this, but that might get your test passing at first and then you can figure out where you've gone wrong).

Few questions:

I don't see you calling @thing.save in your spec. Are you doing that, just like in your console example?

Why are you calling t.save and not u.save in your console test, considering you're pushing t onto u? Saving u should trigger a save to t, getting the end result you want, and I think it would "make more sense" considering you are really working on u, not t.

theIV
  • 25,434
  • 5
  • 54
  • 58
  • Adding a reload to @thing doesn't make the test pass. I'm under the impression that the `<<` operator automatically saves (which I think is evidenced by the fact that the first test passes). Anyways, I can add `@thing.save` and `@user.save` to the test setup and that also doesn't make the tests pass. I'm only calling `t.save` in console to convince myself that the `after_save` is being called. The `after_save` needs to be on the `Thing` because that's what I'm working towards (e.g., after a thing is saved, notify all of its followers). – dantswain Jan 18 '12 at 17:52
  • I didn't realize `<<` automatically saved both records. You're right that it wouldn't make much sense for the first test to pass if it wasn't being saved... I'll think a little more about it. – theIV Jan 18 '12 at 17:55
  • Check out my update. It works if I manually define `Thing#followers` - maybe my `has_many :through` is wrong? – dantswain Jan 18 '12 at 21:50