find_or_initialize_by and has_many :through

Question

I have an xml file the user imports data from. This creates a record in the Player model. At the same time, I want a record to be created in the Membership association.

Both of the above should only be triggered if the record does not already exist. This is a shortened version of the method:

@player = self.players.find_or_initialize_by_name_and_archetype(name, archetype)
if @player.save
  self.memberships.create(:player_id => @player.id)
end

Now find_or_initialize_by works, but a Membership is still created for each player, basically ignoring the if condition.

How can one go about doing this in a concise way?

score 2 · Accepted Answer · answered Aug 01 '11 at 18:27

2

If @player gets set from a find, or an initialize, calling save on it can still return true, even if it's not a new record—try it out in console by finding an object, not changing anything, calling save on it, and you'll still get true so long as it's a valid object.

That being said, I think you want your condition to be more along the lines of

if @player.new_record? && @player.save

answered Aug 01 '11 at 18:27

theIV

25,434
5
54
58

Thanks mate, I tried it out in console and indeed it did return true. For some reason I thought save would be false if it only found the record and did not initialize it. – amunds Aug 01 '11 at 18:43
`save` will only return false is the record is not valid (based on validations). Glad it worked out. Cheers. – theIV Aug 01 '11 at 18:52

score 1 · Answer 2 · answered Aug 01 '11 at 18:53

IMHO the cleaner way would be to add uniqueness constraint to memberships on I don't know who self is in your code (a Team?) like that

class Team < ActiveRecord::Base
has_many :memberships, :before_add => :validates_membership

I would just silently drop the database call and report success.

def validates_membership(membership)
  raise ActiveRecord::Rollback if self.memberships.include? membership
end

ActiveRecord::Rollback is internally captured but not reraised.
Then you can just call self.memberships.create(:player_id => @player.id) and there will be no duplicate memberships created.
If you wish you can add uniqueness constraint on the database level add_index :memberships, [ :team_id, :player_id ], :unique => true, :name => 'by_team_and_player.

And, the last but not least (some would say you should do that first), you can add tests to verify membership uniqueness.

find_or_initialize_by and has_many :through

2 Answers2