shoulda

Comments and changes to this ticket

• 

  Tammer Saleh July 31st, 2008 @ 10:17 PM

  • → Tag changed from “active_record_helper should_have_many” to “active_record_helper bug should_have_many”

  I can verify this bug. Pre-regression:

  class User < ActiveRecord::Base
    has_many :watches
    has_many :watched_items, :through => :watches, :source => :item
  end
  
  class UserTest < Test::Unit::TestCase
    should_have_many :watches
    should_have_many :watched_items, :through => :watches
  end
  

  And the workaround:

  class User < ActiveRecord::Base
    has_many :watches
    has_many :watched_items, :through => :watches, :class_name => "Item"
  end
  

  (I had to change the :source option to :class_name)

• 

  Tammer Saleh July 31st, 2008 @ 10:39 PM

  Alain,

  I've fixed it for my case with this commit. Can you check if this works for you?

  Thanks,

  Tammer

• 

  Alain Ravet August 1st, 2008 @ 05:31 AM

  Tammer,

  I'm afraid it still doesn't work for me.

  Note: my failing test code is simpler than yours : I have no in my _has_many_, just a .

  Alain

• 

  Alain Ravet August 1st, 2008 @ 05:32 AM

  (continued)

  my failing code is simpler : I have no :source part, just a :through.

• 

  Michael Irwin August 14th, 2008 @ 10:46 AM

  I'm still seeing this regression with the following code:

  
  class Friend < ActiveRecord::Base
    belongs_to :inviter, :class_name => "Profile"
    belongs_to :invited, :class_name => "Profile"
  end
  

  
  class Profile < ActiveRecord::Base
    has_many :friend_requests_out, :class_name => "Friend", :foreign_key => "inviter_id", :dependent => :destroy
    has_many :friend_requests_in,  :class_name => "Friend", :foreign_key => "invited_id", :dependent => :destroy
    has_many :friends_out, :through => :friend_requests_out, :source => :invited, :conditions => { :friends => { :state => "confirmed" } }, :order => "profiles.first_name, profiles.last_name", :dependent => :destroy
    has_many :friends_in,  :through => :friend_requests_in,  :source => :inviter, :conditions => { :friends => { :state => "confirmed" } }, :order => "profiles.first_name, profiles.last_name", :dependent => :destroy
  

  
  class ProfileTest < ActiveSupport::TestCase
      should_have_many :friends_out, :through => :friend_requests_out, :dependent => :destroy
      should_have_many :friends_in,  :through => :friend_requests_in,  :dependent => :destroy
  end
  

  Note I can't use the :class_name work-around because I have to specify different a :source for each association.

• 

  Ryan McGeary August 31st, 2008 @ 10:23 AM

  Alain,

  I'm just expanding on your original description of this problem. To make it more clear, I think both models should be shown. I'm presuming your models look like this:

  
  class User < ActiveRecord::Base
    has_many :friendships
    has_many :friends, :through => :friendships
  end
  
  class Friendship < ActiveRecord::Base
    belongs_to :user
    belongs_to :friend, :class_name => "User"
  end
  

  If I have this right, the key here (for you) is that the Friendship defines the class for a friend. Rails automatically inspects the association in the Friendship model to determine what class a friend is (actually it looks for either a friend or friends association). Because of this, the hmt relationship doesn't require a :source or :class_name option.

  To clarify, here's the Rails docs for the :source option:

  :source - Specifies the source association name used by has_many :through queries. Only use it if the name cannot be inferred from the association. has_many :subscribers, :through => :subscriptions will look for either :subscribers or :subscriber on Subscription, unless a :source is given.

  MIchael,

  Your scenario is a little more complicated, but I think I understand what's going wrong. Shoulda is assuming the associated class name is just the classified version of :source when in fact it should instead be the class name of the association defined by :source in the through association. (Difficult to follow, I know)

  Tammer,

  Both of these issues are choking when shoulda tries to constantize this incorrectly assumed class name. Unfortunately, while related, I think they are two separate issues in the end, with some complicated consequences. I'm going to look into some possible fixes as I find time today, but no promises :-)

  All,

  If my assumptions are out of line anywhere, please speak up.

• 

  Ryan McGeary August 31st, 2008 @ 12:42 PM

  Alain, your scenario should be fixed with this commit.

  Michael, your scenario should also be fixed with that same commit, but I didn't actually test the exact scenario.

  Tammer, I removed the :source association lookup, because in the end, there weren't any assertions that were dependent on it, especially since it's only supposed to be used with the :through option.

  All, Please let me know if this fix works for everyone.

• 

  Ryan McGeary August 31st, 2008 @ 12:50 PM

  Btw, my original assessment of this problem was over-complicated. The actual fix was easier than I thought it would be. I originally assumed the assertions needed to perform associated lookups on classes referenced by the :source option, but this was not necessary.

• 

  Michael Irwin August 31st, 2008 @ 03:45 PM

  This fixes the issue for me. Thanks!

• 

  Ryan McGeary September 2nd, 2008 @ 08:27 PM

  • → State changed from “new” to “committed”

  Closing this ticket. Just chime back in if the fix doesn't suffice for your own has_many scenario.