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

Fix nullification of has_many :through association with `query_constraints #47721

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def construct_join_attributes(*records)

association_primary_key = source_reflection.association_primary_key(reflection.klass)

if association_primary_key == reflection.klass.primary_key && !options[:source_type]
if Array(association_primary_key) == reflection.klass.composite_query_constraints_list && !options[:source_type]
join_attributes = { source_reflection.name => records }
else
join_attributes = {
Expand Down
7 changes: 7 additions & 0 deletions activerecord/lib/active_record/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,13 @@ def query_constraints_list # :nodoc:
end
end

# Returns an array of column names to be used in queries. The source of column
# names is derived from +query_constraints_list+ or +primary_key+. This method
# is for internal use when the primary key is to be treated as an array.
def composite_query_constraints_list # :nodoc:
@composite_query_constraints_list ||= query_constraints_list || Array(primary_key)
end

# Destroy an object (or multiple objects) that has the given id. The object is instantiated first,
# therefore all callbacks and filters are fired off before the object is deleted. This method is
# less efficient than #delete but allows cleanup methods and other actions to be run.
Expand Down
13 changes: 12 additions & 1 deletion activerecord/test/cases/associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
class AssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :companies, :developers, :projects, :developers_projects,
:computers, :people, :readers, :authors, :author_addresses, :author_favorites,
:comments, :posts, :sharded_blogs, :sharded_blog_posts, :sharded_comments
:comments, :posts, :sharded_blogs, :sharded_blog_posts, :sharded_comments, :sharded_tags, :sharded_blog_posts_tags

def test_eager_loading_should_not_change_count_of_children
liquid = Liquid.create(name: "salty")
Expand Down Expand Up @@ -301,6 +301,17 @@ def test_append_composite_has_many_through_association_with_autosave
assert_includes(blog_post.reload.tags, tag)
assert_predicate Sharded::BlogPostTag.where(blog_post_id: blog_post.id, blog_id: blog_post.blog_id, tag_id: tag.id), :exists?
end

def test_nullify_composite_has_many_through_association
blog_post = sharded_blog_posts(:great_post_blog_one)
assert_not_empty(blog_post.tags)

blog_post.tags = []

assert_empty(blog_post.tags)
assert_empty(blog_post.reload.tags)
assert_not_predicate Sharded::BlogPostTag.where(blog_post_id: blog_post.id, blog_id: blog_post.blog_id), :exists?
end
end

class AssociationProxyTest < ActiveRecord::TestCase
Expand Down