Skip to content
Draft
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ pkg/*
coverage/*
.DS_Store
.byebug_history
vendor/bundle
7 changes: 6 additions & 1 deletion lib/ransack/adapters/active_record/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,12 @@ def extract_correlated_key(join_root)
nil
end
when Arel::Nodes::And
extract_correlated_key(join_root.left) || extract_correlated_key(join_root.right)
# And may have multiple children, so we need to check all, not just left/right
join_root.children.each do |child|
key = extract_correlated_key(child)
return key if key
end
nil
else
# eg parent was Arel::Nodes::And and the evaluated side was one of
# Arel::Nodes::Grouping or MultiTenant::TenantEnforcementClause
Expand Down
74 changes: 74 additions & 0 deletions spec/ransack/adapters/active_record/context_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,80 @@ module ActiveRecord
expect(attribute.relation.name).to eq 'articles'
expect(attribute.relation.table_alias).to be_nil
end

describe '#extract_correlated_key' do
it 'handles Arel::Nodes::And with multiple children correctly' do
# This test specifically targets the bug where extract_correlated_key
# only checks left/right of And nodes instead of all children

# Create models that will trigger the issue with polymorphic associations
# and default scopes creating multiple conditions
Article.delete_all
Note.delete_all

article = Article.create!(title: 'Test Article', body: 'Test body', published: true)
note = Note.create!(note: 'Test Note', notable: article)

# This search should trigger the problematic code path where
# extract_correlated_key gets an And node with multiple children
search = Article.ransack(
g: [
{
m: "and",
c: [
{
a: ["notes_note"],
p: "not_cont_all",
v: ["test"],
}
]
}
]
)

# Let's see what SQL is actually generated first
sql = search.result.to_sql
puts "Generated SQL: #{sql}"

# The bug would cause this to raise "NoMethodError: undefined method 'eq' for nil"
# because extract_correlated_key returns nil for And nodes with more than 2 children
expect { search.result.to_sql }.not_to raise_error

# The SQL should contain proper conditions
expect(sql).to include('notes')
expect(sql).to include('NOT (')
end

it 'extract_correlated_key method directly returns the correct key for And nodes' do
# Let's test the method directly to ensure it handles And nodes properly
context = subject

# Create a mock And node with multiple children to test the specific bug
pk = context.send(:primary_key)

# Create equality nodes - first two don't match, third one matches our primary key
eq1 = Arel::Nodes::Equality.new(Arel::Attributes::Attribute.new(pk.relation, :other_id), Arel::Attributes::Attribute.new(pk.relation, :some_value))
eq2 = Arel::Nodes::Equality.new(Arel::Attributes::Attribute.new(pk.relation, :another_id), Arel::Attributes::Attribute.new(pk.relation, :another_value))
eq3 = Arel::Nodes::Equality.new(pk, Arel::Attributes::Attribute.new(pk.relation, :target_id)) # This should match and is in 3rd position

# Create an And node with multiple children (more than just left/right)
and_node = Arel::Nodes::And.new([eq1, eq2, eq3])

# Verify the setup - and_node should have 3 children but left/right only covers first 2
expect(and_node.children.length).to eq(3)
expect(and_node.left).to eq(eq1)
expect(and_node.right).to eq(eq2)
expect(and_node.children[2]).to eq(eq3) # This is the one that matches but won't be found by current code

# Call extract_correlated_key directly - this should fail with current implementation
result = context.send(:extract_correlated_key, and_node)

# With the bug, this will be nil because the matching equality is in position 3
# After the fix, it should find the correct key
expect(result).not_to be_nil, "extract_correlated_key should find the matching key even when it's not in left/right position"
expect(result.name).to eq(:target_id)
end
end
end
end
end
Expand Down
Loading