Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Building a has_many :through with :conditions #7486

Closed
the8472 opened this issue Aug 30, 2012 · 15 comments
Closed

Building a has_many :through with :conditions #7486

the8472 opened this issue Aug 30, 2012 · 15 comments

Comments

@the8472
Copy link

the8472 commented Aug 30, 2012

Originally reported under activerecord-hackery/squeel#161 but it's happening with vanilla rails 3.2.8 too:

class A < ActiveRecord::Base
  has_many :a_b_mappings, :inverse_of => :a
  has_many :b, :through => :a_b_mappings, :source => :b, :conditions => {:context => "foo"}
end

class B < ActiveRecord::Base
  has_many :a_b_mappings, :inverse_of => :b
end

class ABMapping < ActiveRecord::Base
  self.table_name = :a_b_mappings
  belongs_to :a, :inverse_of => :a_b_mappings
  belongs_to :b, :inverse_of => :a_b_mappings
end


puts A.first.b.create.tap{|b| puts b.inspect}.a_b_mappings.inspect
# #<B id: 3, context: nil> 
# [#<ABMapping id: 2, a_id: 1, b_id: 2>]

It creates the through-association but does not set the conditions on table B. I.e. context is expected to be "foo", not nil.

The Associations::ClassMethods documentation states the following:

conditions
Specify the conditions that the associated objects must meet in order to be included as a WHERE SQL fragment, such as price > 5 AND name LIKE 'B%'. Record creations from the association are scoped if a hash is used. has_many :posts, :conditions => {:published => true} will create published posts with @blog.posts.create or @blog.posts.build.

@the8472
Copy link
Author

the8472 commented Aug 30, 2012

Workaround:

use the following:
:conditions => Arel::Nodes::Equality.new(Arel::Attributes::Attribute.new(AssociatedTable.arel_table,:attribute),"value")
instead of
:conditions => {:attribute => "value"}

Since arel nodes don't get mangled by the sanitizer/predicate builder and thus will be recognized by the record builder.

@rafaelfranca
Copy link
Member

Closing because it seems invalid.

Please see #5057 (comment)

If you don't agree please ping me

@the8472
Copy link
Author

the8472 commented Sep 18, 2012

@rafaelfranca

Yes, disagree. As @ernie states

This, to me, makes perfect sense. The conditions for join model creation exist on the join model's association, not on the hm:t association.

He is describing conditions being set on the join model. But in my case it's about conditions which should be set on the associated model, not the join model.

@rafaelfranca
Copy link
Member

As this never worked and it is documented behavior we can't consider it as a bug.

It is more like a feature request. If you want to make it work please open a pull request or start a discussion in the Rails core mailing list to find someone to work to you.

@the8472
Copy link
Author

the8472 commented Sep 18, 2012

and it is documented behavior we can't consider it as a bug.

Sorry, I must be blind... but where is it documented? The documentation I have quoted in my initial report seems to describe exactly the opposite of what you're saying.

@rafaelfranca
Copy link
Member

:through
Specifies an association through which to perform the query. This can be any other type of association, including other :through associations. Options for :class_name, :primary_key and :foreign_key are ignored, as the association uses the source reflection.

If the association on the join model is a belongs_to, the collection can be modified and the records on the :through model will be automatically created and removed as appropriate. Otherwise, the collection is read-only, so you should manipulate the :through association directly.

If you are going to modify the association (rather than just read from it), then it is a good idea to set the :inverse_of option on the source association on the join model. This allows associated records to be built which will automatically create the appropriate join model records when they are saved. (See the ‘Association Join Models’ section above.)

See the last paragraph. If setting the :inverse_of in the source association on the join model doesn't solve this please let me know, and I'll gladly reopen.

@the8472
Copy link
Author

the8472 commented Sep 19, 2012

Please have a look at my initial post. I did set the :inverse_of. The join model is created properly. The join model is not the issue here. Neither are the conditions on the join model.

My problem are the conditions on the nested relation.

A -> AB (join model, no conditions here) -> B (conditions)

Conditions on the has_many for AB were what ernie was talking about. Conditions on the has_many :through for B don't work when building a new object.

@rafaelfranca
Copy link
Member

@the8472 ok. Thank you for the explanation. I'm reopening and marking as bug.

Sorry for the make you loose your time.

@rafaelfranca rafaelfranca reopened this Sep 19, 2012
@kenips
Copy link

kenips commented Feb 15, 2013

Quite interesting. Just came across this and here's what I have (trying to remap the class names to the example above):

class A < ActiveRecord::Base
  has_many :a_b_mappings
  has_many :a_b_mappings_foo, -> { where context: 'foo' }, class_name: 'ABMapping'
  has_many :b_foos, through: :a_b_mappings_foo, source: :b

In this case, if I do

a = A.new
a.b_foos << B.find(1)
a.a_b_mappings_foo.count # 0
a.a_b_mappings.count # 0
a.save
a.reload
a.a_b_mappings_foo.count # 0
a.a_b_mappings.count # 1

So it's clearly failing. But, interestingly, if I continue:

a.b_foos << B.find(2)
a.a_b_mappings_foo.count # 0
a.a_b_mappings.count # 1
a.save
a.reload
a.a_b_mappings_foo.count # 1
a.a_b_mappings.count # 2

it seems like it only fails for a new record. Any thoughts?

UPDATE: this only happens when you're trying to save the association together with a new object, by replace new with create in the first case, everything works:

a = A.create
a.b_foos << B.find(1)
a.a_b_mappings_foo.count # 1 # autosaved and reloaded
a.a_b_mappings.count # 1
a.save # unnecessary, but just for consistency as above
a.reload
a.a_b_mappings_foo.count # 1
a.a_b_mappings.count # 1

@the8472
Copy link
Author

the8472 commented Feb 16, 2013

a.reload

Careful with that. If i recall correctly that only reloads attributes, not relations. Use a.a_b_mappings(true) to reload an association.

@kenips
Copy link

kenips commented Feb 16, 2013

Doesn't seem to impact in this case, switching to a.a_b_mappings(true) shows the same. Are you able to modify your original model to reproduce this (at least a step forward)?

@davidinjc
Copy link

@rafaelfranca Is this really a bug? AFAICT the documentation says that it should work for has_many, but it does not say that it will work for hm:t. Just wanted to know whether this is a bug or a feature request. This is different from #5057 because that's passing in the default attributes through create.

@the8472
Copy link
Author

the8472 commented Mar 8, 2013

but it does not say that it will work for hm:t

The documentation says

:through
If you are going to modify the association (rather than just read from it), then it is a good idea to set the :inverse_of option on the source association on the join model. This allows associated records to be built which will automatically create the appropriate join model records when they are saved. (See the ‘Association Join Models’ section above.)
(emphasis mine)

Appropriate for a has_many with conditions would be a join model with pre-filled values according to the conditions.

And there also isn't anything in the documentation saying I can't use those two together.

@neerajsingh0101
Copy link

This is no longer an issue for master. However my gist is failing for 3.2.13 .

https://gist.github.com/neerajdotname/5407350

@laurocaetano
Copy link
Contributor

I'm closing this since 3.2 no longer receives bugfixes and it works on 4.0.2.

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

No branches or pull requests

6 participants