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?

Answer 1

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

Answer 2

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.