Rails append not adding object to has_many :through #7364

Closed
workmaster2n opened this Issue Aug 16, 2012 · 6 comments

Comments

Projects
None yet
6 participants

Posted this on SO: http://stackoverflow.com/questions/11889922/rails-append-not-adding-object-to-has-many-through

I'm getting weird behavior when trying to add objects via a has_many :through relationship.

My models:

Class Player < ActiveRecord::Base
  has_many :player_to_team_histories
  has_many :team_histories, through: :player_to_team_histories
end
Class TeamHistory < ActiveRecord::Base
  has_many :player_to_team_histories
  has_many :players, through: :player_to_team_histories
end

The code:

>>p = Player.first
>>p.team_histories.count
0
>>p.team_histories.append TeamHistory.create
>>p.team_histories.count
0
>>p.team_histories.push TeamHistory.create
>>p.team_histories.count
1
>>p.team_histories << TeamHistory.create
>>p.team_histories.count
2

Why does append not add the newly created TeamHistory to the team_histories array?

I'm using Ruby 1.9.2.

There is no documentation about the append method for ActiveRecord. Was append intentionally left out?

Member

steveklabnik commented Aug 16, 2012

I've always used <<, so I'm not sure what's right here. /cc @tenderlove @jonleighton

append and prepend are Array aliases do << and unshift, respectively, see 9482554.

>> [].append 2
NoMethodError: undefined method 'append' for []:Array
>> require 'active_support/all'
=> true
>> [].append 2
=> [2]

This means that, when they're called on the association like that, they're just being delegated to Array, so they do not work with the association.

Perhaps - but just perhaps - we should add aliases to the proxy as well, since we added them to the Array, but I'd like to see other thoughts on that first.

Contributor

al2o3cr commented Sep 26, 2012

@carlosantoniodasilva - there's already code to do this for Array#push:

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/collection_proxy.rb#L952

On the other hand, I'm not at all sure what we should do about prepend, also added by ActiveSupport. Is there an equivalent to Array#unshift that makes sense?

Owner

tenderlove commented Sep 26, 2012

I suppose if we're going to monkey patch array to have this behavior, we should make it consistent everywhere. Changing the alias method to a delegate method would probably fix this as well:

class Array
  def append(*args); self.<<(*args); end
end
Member

senny commented Feb 10, 2013

When adding new records to an association we currently don't differentiate between append/prepend. I see the following options:

  1. make append/prepend work the same as <<. This could be confusing though because you can't really prepend records.
  2. make only append work and warn when prepend or unshift is used. Since append and prepend are counterparts this feels inconsistent.
  3. warn when append, prepend or unshift is used and refer to <<.

I'm for option 3 to prevent further confusion. Let me know what you think and I'll submit a PR.

@tenderlove the delegation won't fix the problem since the proxy will delegate append to the target (the Array) and then << is called on the Array not on the proxy.

Member

senny commented Mar 1, 2013

I submitted a PR with an implementation for 3.)

senny closed this in b9399c4 Mar 1, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment