Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Creating has_many :through association with conditions #5178

Closed
heaven opened this Issue · 29 comments

10 participants

@heaven

Hi, I have troubles when trying to use such code:

class Site < ActiveRecord::Base
  has_many :site_relationships, :dependent => :destroy
  has_many :profiles, :through => :site_relationships
end

class SiteRelationship < ActiveRecord::Base
  VALID_KINDS = {:owner => "Owner", :reader => "Reader"}

  belongs_to :profile
  belongs_to :site

  validates :kind, :inclusion => VALID_KINDS.values, :allow_nil => true
end

class Profile < ActiveRecord::Base
  has_many :site_relationships, :dependent => :destroy
  has_many :owned_sites, :through => :site_relationships, :source => :site, :conditions => "site_relationships.kind = 'Owned'"
end

Actually profile.owned_sites works great, but when I try to

profile.owned_sites.create(:url => "http://google.com")

It creates the Site and creates the SiteRelationship but doesn't set the site_relationships.kind attribute to 'Owned', so: profile.owned_sites.reload => []

Is there any way to set up the join model attribute automatically? Or other way to solve this issue?

Thanks in advance,
Alex

@baroquebobcat

Have you looked at this stackoverflow question? http://stackoverflow.com/questions/3081764/rails-has-many-through-and-setting-property-on-join-model

tl;dr

if you use a hash :conditions, then ActiveRecord will do what you want.

:conditions => {"site_relationships.kind" => "Owned"}
@heaven

Weird, but it doesn't work...

has_many :owned_sites, :through => :site_relationships, :source => :site, :conditions => {"site_relationships.kind" => "Owner"}

profile.owned_sites.create(:url => "http://google.com", :service_type => "Other")

Produces:

  SQL (0.3ms)  INSERT INTO `site_relationships` (`created_at`, `follower_id`, `kind`, `profile_id`, `site_id`, `updated_at`) VALUES ('2012-02-26 10:37:41', NULL, NULL, 1, 1, '2012-02-26 10:37:41')

kind → NULL

@heaven

I have solved this in the next way:

has_many :owned_sites, :through => :site_relationships, :source => :site,
  :conditions => { :site_relationships => { :kind => SiteRelationship::VALID_KINDS[:owner] }} do

  def <<(*sites)
    sites.flatten.each do |site|
      raise ActiveRecord::AssociationTypeMismatch unless site.is_a?(Site)

      SiteRelationship.create(:kind => SiteRelationship::VALID_KINDS[:owner],
        :site => site,
        :profile => proxy_association.owner)
    end
  end
end

But still hope there is some better way to solve this...

@heaven

This issue is invalid, :conditions was added to the wrong place, it should be added to the join association, so it should look like:

has_many :owned_site_relationships, :class_name => "SiteRelationship",
  :conditions => { :kind => SiteRelationship::VALID_KINDS[:owner] }
has_many :owned_sites, :through => :owned_site_relationships, :source => :site, :autosave => true

Sorry for the confusion.

@rohit

@heaven Can you please close this issue? :)

@heaven

@rohit oops, of course :)

@heaven heaven closed this
@heaven

Found a weird behavior:

class Profile < AR::Base
  has_many :owned_site_relationships, :class_name => "SiteRelationship",
           :conditions => { :kind => SiteRelationship::VALID_KINDS[:owner] }
  has_many :owned_sites, :through => :owned_site_relationships, :source => :site
end

class Site < AR::Base
  has_many :owners_site_relationships, :class_name => "SiteRelationship",
           :conditions => { :kind => SiteRelationship::VALID_KINDS[:owner] }
  has_many :owners, :through => :owners_site_relationships, :source => :profile
end

class SiteRelationship < AR::Base
  belongs_to :profile
  belongs_to :site
end

Then this looks correct:

Profile.joins(:owned_sites)
=> SELECT `profiles`.* FROM `profiles` INNER JOIN `site_relationships` ON `site_relationships`.`profile_id` = `profiles`.`id` AND `site_relationships`.`kind` = 'Owner' INNER JOIN `sites` ON `sites`.`id` = `site_relationships`.`site_id`

But this not:

Site.joins(:owners)
=> SELECT `sites`.* FROM `sites` INNER JOIN `site_relationships` ON `site_relationships`.`site_id` = `sites`.`id` AND `site_relationships`.`kind` = 0 INNER JOIN `profiles` ON `profiles`.`id` = `site_relationships`.`profile_id`

It produces site_relationships.kind = 0 instead of site_relationships.kind = 'Owner'. Any suggestions?

Best,
Alex

@heaven heaven reopened this
@steveklabnik
Collaborator

Is this still an issue now that we've merged #7661?

@heaven

Hi, it's still reproducible in 3.2.9

Profile.joins(:owned_sites).first:

  Profile Load (0.6ms)  SELECT `profiles`.* FROM `profiles` INNER JOIN `site_relationships` ON `site_relationships`.`profile_id` = `profiles`.`id` AND `site_relationships`.`kind` = 'Owner' INNER JOIN `sites` ON `sites`.`id` = `site_relationships`.`site_id` LIMIT 1

Site.joins(:owners).first:

  Site Load (3.1ms)  SELECT `sites`.* FROM `sites` INNER JOIN `site_relationships` ON `site_relationships`.`site_id` = `sites`.`id` AND `site_relationships`.`kind` = 0 INNER JOIN `profiles` ON `profiles`.`id` = `site_relationships`.`profile_id` LIMIT 1

Still `site_relationships.kind = 0`

@steveklabnik
Collaborator

Thanks for confirming.

@heaven

Moreover, that commit breaks hs:t association completely. My setup:

class Profile < ActiveRecord::Base
  has_many :profile_interests, :dependent => :destroy
  has_many :interests, :through => :profile_interests, :source => :condition
end

class ProfileInterest < ActiveRecord::Base
  belongs_to :condition
  belongs_to :profile

  validates :condition, :profile, :presence => true
end

class Condition < ActiveRecord::Base
  has_many :profile_interests, :dependent => :destroy
  has_many :interested, :through => :profile_interests, :source => :profile
end

Then I do in rails console:

profile = Profile.new
condition = Condition.find_by_name("Gastrointestinal Diseases") # condition persisted
profile.interests << condition

profile.valid?
=> false
profile.errors.messages
=> {:profile_interests=>["is invalid"]}
profile.profile_interests
[#<ProfileInterest id: nil, condition_id: 62, profile_id: nil, created_at: nil, updated_at: nil>]

Just rolled back to 3.2.8, run same code and profile.valid? => true

@rafaelfranca

@heaven you need to set :inverse_of for profile_interests association in both models as recommended in the guides.

@bensie

I'm seeing something very similar when setting collection_ids, but I have set :inverse_of. Like @heaven this works in 3.2.8.

class Cluster < ActiveRecord::Base
  has_many :authorizations, inverse_of: :cluster, dependent: :destroy
  has_many :keypairs, through: :authorizations
end

class Keypair < ActiveRecord::Base
  has_many :authorizations, inverse_of: :keypair, dependent: :destroy
  has_many :clusters, through: :authorizations
end

class Authorization < ActiveRecord::Base
  belongs_to :cluster, inverse_of: :authorizations
  belongs_to :keypair, inverse_of: :authorizations
  validates_presence_of :cluster_id, :keypair_id
  validates_uniqueness_of :cluster_id, scope: :keypair_id
end

keypair = Keypair.create!
cluster = Cluster.new(keypair_ids: [keypair.id])

cluster.valid?
=> false

cluster.errors.messages
=> {authorizations: ["is invalid"]}
@bensie

I've posted a sample app that demonstrates this issue here:
https://github.com/bensie/rails_5178

Get it running with:

bundle install
rake db:migrate
rake db:test:prepare
bundle exec rspec

App is currently set to 3.2.8 (spec passes), change it to 3.2.9 in the Gemfile and the spec fails.

/cc @trevorturk @steveklabnik

@trevorturk

@bensie were you saying on Twitter (https://twitter.com/bensie/status/281249503489712128) that pull request #7661 with commit 610b632 is the problem? I'm not totally clear from looking at this issue. It seems like we're discussing multiple things. /cc @ernie

@pixeltrix
Owner

@bensie I think your problem is that the validates_presence_of :cluster_id in Authorization will run before the Cluster has been saved so the cluster_id is nil. If you add :validate => false to the has_many :authorizations does it work?

@ernie

@bensie Yeah, agreed with @pixeltrix -- you want to be doing something like this:

class Authorization < ActiveRecord::Base

  belongs_to :cluster
  belongs_to :keypair

  validates_presence_of :cluster, :keypair # Validate on association, not association_id
  validates_uniqueness_of :cluster_id, scope: :keypair_id

end
class Cluster < ActiveRecord::Base

  has_many :authorizations, dependent: :destroy, inverse_of: :cluster # Add inverse_of
  has_many :keypairs, through: :authorizations

  attr_accessible :name, :keypair_ids
  validates_presence_of :name

end
class Keypair < ActiveRecord::Base

  has_many :authorizations, dependent: :destroy, inverse_of: :keypair # Add inverse_of
  has_many :clusters, through: :authorizations

  attr_accessible :name
  validates_presence_of :name

end
@bensie

@trevorturk Yep, that's the PR/commit that caused the problem.

@ernie Specs are passing when validating on the association instead of the association_id and adding inverse_of, thanks! Didn't need the validate: false suggested by @pixeltrix

So, is this just me doing it wrong or is there a valid regression? I'm fine either way knowing this is the correct way, but a patch release did break the app...

@trevorturk

I don't know the answer to @bensie's question, but I think @heaven's last comment (#5178 (comment)) still demonstrates an issue?

@ernie

@trevorturk I don't think so? There are no inverse_of associations in that sample. Adding them in Profile and Condition should fix.

@trevorturk

OK, cool, thanks, @ernie. I wonder if this is worth documenting... somewhere? (I don't know where.) It seems likely to come up again, but it doesn't look like we should revert 610b632 from where I'm sitting.

@heaven can you confirm using inverse_of solves your problem and, if so, close this ticket?

@ernie

@trevorturk: @steveklabnik was kind enough to get the documentation around validations updated a couple of weeks back. Not sure where else we would document but I agree it needs more visibility.

@heaven

Weird, because the validation in my example checks :condition and :profile not the :condition_id and :profile_id. But yes, will check and post my results here.

@ernie

@heaven it's not just the validation -- you need the inverse_of so that ActiveRecord will actually set the (unsaved) record as the target of the belongs_to association proxy.

@heaven

Yup, I understand, will try a bit later since now I have no access to my work PC.

@bensie

This issue can be closed. /cc @steveklabnik

@steveklabnik
Collaborator

Seems right.

@heaven

Hi, yes, it works for me also. But issue described in comment #5178 (comment) is still a valid.

@starim starim referenced this issue in devinleonhart/bountyboard
Open

Upgrade to Rails 4 and Ruby 2 #33

@jmejia jmejia referenced this issue from a commit in jmejia/ono_burrito
@jmejia jmejia Update seed data and relevant models
  * Adjust validations in `item`, `category`, and
    `item_category` to use `inverse_of` for
    validating presence. For discussion on topic:
    rails/rails#5178 (comment)

  * NOTE: There are several comments left in
    intentionally. Please clean them up. They are
    there for comparison and to answer questions
    you may have.

  * The `seeds.rb` file is destructive. It will
    remove existing data from the database when
    run. Read the code to delete these two lines
    if this is not desired behavior.
4815b57
@plicjo

I had a similar issue, and validating the association, rather than the association's id fixed it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.