Skip to content
This repository was archived by the owner on May 5, 2020. It is now read-only.

[3.1] Fix ActiveRecord's completely broken where merge semantics #60

Open
wants to merge 3 commits into
base: 3-1-github
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 1 addition & 16 deletions activerecord/lib/active_record/relation/spawn_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,7 @@ def merge(r)

merged_wheres = @where_values + r.where_values

unless @where_values.empty?
# Remove duplicates, last one wins.
seen = Hash.new { |h,table| h[table] = {} }
merged_wheres = merged_wheres.reverse.reject { |w|
nuke = false
if w.respond_to?(:operator) && w.operator == :==
name = w.left.name
table = w.left.relation.name
nuke = seen[table][name]
seen[table][name] = true
end
nuke
}.reverse
end

merged_relation.where_values = merged_wheres
merged_relation.where_values = merged_wheres.uniq

(Relation::SINGLE_VALUE_METHODS - [:lock, :create_with]).each do |method|
value = r.send(:"#{method}_value")
Expand Down
10 changes: 8 additions & 2 deletions activerecord/test/cases/associations/eager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -981,10 +981,16 @@ def test_preload_has_many_with_association_condition_and_default_scope
LazyReader.create! :person => people(:susan), :post => post

assert_equal 1, post.lazy_readers.to_a.size
assert_equal 2, post.lazy_readers_skimmers_or_not.to_a.size

# This test relied on some completely broken and incorrect behaviour in
# ActiveRecord that we've fixed.
#
# assert_equal 2, post.lazy_readers_skimmers_or_not.to_a.size

assert_equal 1, post.lazy_readers_skimmers_or_not.to_a.size

post_with_readers = Post.includes(:lazy_readers_skimmers_or_not).find(post.id)
assert_equal 2, post_with_readers.lazy_readers_skimmers_or_not.to_a.size
assert_equal 1, post_with_readers.lazy_readers_skimmers_or_not.to_a.size
end

def test_include_has_many_using_primary_key
Expand Down
6 changes: 3 additions & 3 deletions activerecord/test/cases/named_scope_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -364,11 +364,11 @@ def test_chaining_should_use_latest_conditions_when_creating

def test_chaining_should_use_latest_conditions_when_searching
# Normal hash conditions
assert_equal Topic.where(:approved => true).to_a, Topic.rejected.approved.all
assert_equal Topic.where(:approved => false).to_a, Topic.approved.rejected.all
assert_equal [], Topic.rejected.approved.all
assert_equal [], Topic.approved.rejected.all

# Nested hash conditions with same keys
assert_equal [posts(:sti_comments)], Post.with_special_comments.with_very_special_comments.all
assert_equal [], Post.with_special_comments.with_very_special_comments.all

# Nested hash conditions with different keys
assert_equal [posts(:sti_comments)], Post.with_special_comments.with_post(4).all.uniq
Expand Down
8 changes: 7 additions & 1 deletion activerecord/test/cases/relation_scoping_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,13 @@ def test_should_maintain_default_scope_on_associations

def test_should_default_scope_on_associations_is_overriden_by_association_conditions
reference = references(:michael_unicyclist).becomes(BadReference)
assert_equal [reference], people(:michael).fixed_bad_references

# This test relied on some completely broken and incorrect behaviour in
# ActiveRecord that we've fixed.
#
# assert_equal [reference], people(:michael).fixed_bad_references

assert_equal [], people(:michael).fixed_bad_references
end

def test_should_maintain_default_scope_on_eager_loaded_associations
Expand Down