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
92 changes: 92 additions & 0 deletions POLYMORPHIC_ISSUE_README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# Polymorphic Filtering Issue Reproduction

This PR reproduces the polymorphic filtering inconsistency issue described in the GitHub issue.

## Problem

When using Ransack to filter records with OR conditions between polymorphic foreign keys, the query fails with:

```
ArgumentError: Polymorphic associations do not support computing the class
```

This is inconsistent because:
1. Single polymorphic foreign key filtering works fine (`from_id_eq`)
2. OR conditions that include non-polymorphic fields work fine (`id_or_from_id_or_to_id_eq`)
3. The error occurs even though we're only filtering by ID values, not accessing the polymorphic associations

## Example Model

```ruby
class Message < ApplicationRecord
belongs_to :user, class_name: 'Person'
belongs_to :from, polymorphic: true
belongs_to :to, polymorphic: true
end
```

## Failing Cases

These should work but currently raise errors:

```ruby
# Core issue from GitHub report
Message.ransack(from_id_or_to_id_eq: '123').result.count
# => ArgumentError: Polymorphic associations do not support computing the class

# Reverse order
Message.ransack(to_id_or_from_id_eq: '123').result.count
# => ArgumentError: Polymorphic associations do not support computing the class

# Mix of regular and polymorphic FK
Message.ransack(user_id_or_from_id_eq: '123').result.count
# => ArgumentError: Polymorphic associations do not support computing the class
```

## Working Cases

These work correctly:

```ruby
# Single polymorphic FK
Message.ransack(from_id_eq: '123').result.count
# => Works fine

# OR with non-polymorphic field (workaround from issue)
Message.ransack(id_or_from_id_or_to_id_eq: '123').result.count
# => Works fine

# No polymorphic FKs involved
Message.ransack(user_id_or_id_eq: '123').result.count
# => Works fine
```

## Expected Behavior

OR conditions between polymorphic foreign keys should work just like regular foreign keys since:
1. We're only filtering by ID values, not joining to the polymorphic associations
2. No polymorphic class computation should be needed for simple ID comparisons
3. The behavior should be consistent with single polymorphic FK filtering

## Test Files

- `spec/ransack/adapters/active_record/polymorphic_spec.rb` - Comprehensive RSpec test suite
- `test_polymorphic_issue.rb` - Simple standalone test runner
- `spec/support/schema.rb` - Added Message model and database schema
- `spec/blueprints/messages.rb` - Blueprint for Message model

## Running Tests

To run the RSpec tests:
```bash
bundle exec rspec spec/ransack/adapters/active_record/polymorphic_spec.rb
```

To run the simple test runner:
```bash
ruby test_polymorphic_issue.rb
```

## Root Cause

The issue appears to be in Ransack's attribute resolution logic, which incorrectly tries to compute the polymorphic class even when only filtering by foreign key values in OR conditions.
4 changes: 4 additions & 0 deletions spec/blueprints/messages.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Message.blueprint do
user { Person.make }
content { "Test message content" }
end
144 changes: 144 additions & 0 deletions spec/ransack/adapters/active_record/polymorphic_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
require 'spec_helper'

module Ransack
module Adapters
module ActiveRecord
describe 'Polymorphic filtering' do
# This test reproduces the issue described in GitHub issue:
# "Polymorphic filtering is inconsistent"
#
# PROBLEM:
# When filtering with `from_id_or_to_id_eq` on polymorphic associations,
# Ransack throws: "Polymorphic associations do not support computing the class"
#
# However, `id_or_from_id_or_to_id_eq` works fine, which indicates
# the issue is specifically with OR conditions on polymorphic foreign keys
#
# EXPECTED BEHAVIOR:
# - `from_id_or_to_id_eq: '123'` should work and filter by foreign key values
# - The query should not need to compute polymorphic classes since we're only
# filtering by the foreign key IDs, not joining to the polymorphic associations
# - OR conditions between polymorphic foreign keys should work like regular FKs
#
# CURRENT (BROKEN) BEHAVIOR:
# - `from_id_or_to_id_eq: '123'` raises ArgumentError about polymorphic classes
# - The error occurs even though we're not accessing the polymorphic associations
# - Only workaround is to include a non-polymorphic field in the OR condition
#
# ROOT CAUSE:
# Ransack's attribute resolution logic incorrectly tries to compute the class
# for polymorphic associations even when only filtering by foreign key values
context 'with polymorphic associations' do
let!(:person1) { Person.create!(name: 'Alice', email: '[email protected]') }
let!(:person2) { Person.create!(name: 'Bob', email: '[email protected]') }
let!(:article1) { Article.create!(person: person1, title: 'Test Article', body: 'Test body') }

let!(:message1) { Message.create!(user: person1, from: person1, to: person2, content: 'Hello from person to person') }
let!(:message2) { Message.create!(user: person1, from: article1, to: person2, content: 'Hello from article to person') }
let!(:message3) { Message.create!(user: person2, from: person2, to: person1, content: 'Reply from person to person') }

describe 'reproducing the exact issue from GitHub' do
it 'should fail with from_id_or_to_id_eq using UUID-like string' do
# This reproduces the exact error from the GitHub issue:
# Message.ransack(from_id_or_to_id_eq: '3d4464a4-1501-4f30-a892-fa07f72f9fa1').result.count
# should raise: "Polymorphic associations do not support computing the class"
expect {
search = Message.ransack(from_id_or_to_id_eq: '3d4464a4-1501-4f30-a892-fa07f72f9fa1')
search.result.count
}.to raise_error(ArgumentError, /Polymorphic associations do not support computing the class/)
end

it 'should work with id_or_from_id_or_to_id_eq using UUID-like string (the working case)' do
# This reproduces the working case from the GitHub issue:
# Message.ransack(id_or_from_id_or_to_id_eq: '3d4464a4-1501-4f30-a892-fa07f72f9fa1').result.count
# should work and return a count
expect {
search = Message.ransack(id_or_from_id_or_to_id_eq: '3d4464a4-1501-4f30-a892-fa07f72f9fa1')
count = search.result.count
expect(count).to be >= 0 # Should return a valid count (likely 0 for non-matching UUID)
}.not_to raise_error
end
end

describe 'filtering by polymorphic foreign keys with OR condition' do
it 'should work with from_id_or_to_id_eq' do
# This should find messages where either from_id or to_id matches person1's id
search = Message.ransack(from_id_or_to_id_eq: person1.id)
result = search.result

# Should find message1 (from: person1) and message3 (to: person1)
expect(result).to include(message1, message3)
expect(result).not_to include(message2)
end

it 'should work with to_id_or_from_id_eq' do
# This should find messages where either to_id or from_id matches person2's id
search = Message.ransack(to_id_or_from_id_eq: person2.id)
result = search.result

# Should find message1 (to: person2), message2 (to: person2), and message3 (from: person2)
expect(result).to include(message1, message2, message3)
end

it 'should work with id_or_from_id_or_to_id_eq (the working case from issue)' do
# This should work as described in the issue
search = Message.ransack(id_or_from_id_or_to_id_eq: person1.id)
result = search.result

# Should find message1 (from: person1) and message3 (to: person1)
# but NOT message1 itself by id since person1.id != message1.id
expect(result).to include(message1, message3)
expect(result).not_to include(message2)
end
end

describe 'filtering by single polymorphic foreign key' do
it 'should work with from_id_eq' do
search = Message.ransack(from_id_eq: person1.id)
result = search.result

expect(result).to include(message1)
expect(result).not_to include(message2, message3)
end

it 'should work with to_id_eq' do
search = Message.ransack(to_id_eq: person2.id)
result = search.result

expect(result).to include(message1, message2)
expect(result).not_to include(message3)
end
end

describe 'additional edge cases for polymorphic OR conditions' do
it 'should fail with user_id_or_from_id_eq' do
# This combines a regular foreign key with a polymorphic foreign key
expect {
search = Message.ransack(user_id_or_from_id_eq: person1.id)
search.result.count
}.to raise_error(ArgumentError, /Polymorphic associations do not support computing the class/)
end

it 'should fail with from_id_or_to_id_or_user_id_eq' do
# This combines multiple polymorphic and regular foreign keys
expect {
search = Message.ransack(from_id_or_to_id_or_user_id_eq: person1.id)
search.result.count
}.to raise_error(ArgumentError, /Polymorphic associations do not support computing the class/)
end

it 'should work with user_id_or_id_eq (no polymorphic keys)' do
# This should work as it doesn't involve polymorphic associations
search = Message.ransack(user_id_or_id_eq: person1.id)
result = search.result

# Should find messages where user_id matches person1.id
expect(result).to include(message1, message2)
expect(result).not_to include(message3)
end
end
end
end
end
end
end
23 changes: 23 additions & 0 deletions spec/support/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,12 @@ class Employee < ApplicationRecord
has_one :address, through: :organization
end

class Message < ApplicationRecord
belongs_to :user, class_name: 'Person'
belongs_to :from, polymorphic: true
belongs_to :to, polymorphic: true
end

module Schema
def self.create
ActiveRecord::Migration.verbose = false
Expand Down Expand Up @@ -343,6 +349,15 @@ def self.create
t.string :name
t.integer :organization_id
end

create_table :messages, force: true do |t|
t.integer :user_id
t.integer :from_id
t.string :from_type
t.integer :to_id
t.string :to_type
t.text :content
end
end

10.times do
Expand All @@ -364,6 +379,14 @@ def self.create
body: 'First post!',
article: Article.make(title: 'Hello, world!')
)

# Create some test messages with polymorphic associations
person1 = Person.create!(name: 'Alice', email: '[email protected]')
person2 = Person.create!(name: 'Bob', email: '[email protected]')
article1 = Article.create!(person: person1, title: 'Test Article', body: 'Test body')

Message.create!(user: person1, from: person1, to: person2, content: 'Hello from person to person')
Message.create!(user: person1, from: article1, to: person2, content: 'Hello from article to person')
end
end

Expand Down
73 changes: 73 additions & 0 deletions test_polymorphic_issue.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
#!/usr/bin/env ruby

# Simple test runner to demonstrate the polymorphic filtering issue
# Run with: ruby test_polymorphic_issue.rb

require 'minitest/autorun'
require_relative 'lib/ransack'
require_relative 'spec/support/schema'

# Initialize the database and schema
Schema.create

class TestPolymorphicFiltering < Minitest::Test
def setup
# Create test data
@person1 = Person.create!(name: 'Alice', email: '[email protected]')
@person2 = Person.create!(name: 'Bob', email: '[email protected]')
@article1 = Article.create!(person: @person1, title: 'Test Article', body: 'Test body')

@message1 = Message.create!(user: @person1, from: @person1, to: @person2, content: 'Hello from person to person')
@message2 = Message.create!(user: @person1, from: @article1, to: @person2, content: 'Hello from article to person')
@message3 = Message.create!(user: @person2, from: @person2, to: @person1, content: 'Reply from person to person')
end

def test_failing_case_from_id_or_to_id_eq
puts "Testing failing case: from_id_or_to_id_eq"

# This should fail with "Polymorphic associations do not support computing the class"
assert_raises(ArgumentError, /Polymorphic associations do not support computing the class/) do
search = Message.ransack(from_id_or_to_id_eq: @person1.id)
search.result.count
end

puts "✓ Correctly failed with expected error"
end

def test_working_case_id_or_from_id_or_to_id_eq
puts "Testing working case: id_or_from_id_or_to_id_eq"

# This should work as mentioned in the issue
search = Message.ransack(id_or_from_id_or_to_id_eq: @person1.id)
count = search.result.count

assert count.is_a?(Integer)
puts "✓ Correctly worked, returned count: #{count}"
end

def test_single_polymorphic_key_works
puts "Testing single polymorphic key: from_id_eq"

# This should work fine
search = Message.ransack(from_id_eq: @person1.id)
result = search.result.to_a

assert_includes result, @message1
refute_includes result, @message2
refute_includes result, @message3
puts "✓ Single polymorphic key filtering works correctly"
end
end

puts "=" * 60
puts "POLYMORPHIC FILTERING ISSUE REPRODUCTION TEST"
puts "=" * 60
puts
puts "This demonstrates the inconsistent behavior described in the GitHub issue:"
puts "- Single polymorphic foreign key filtering works (from_id_eq)"
puts "- OR with non-polymorphic field works (id_or_from_id_or_to_id_eq)"
puts "- OR between polymorphic foreign keys fails (from_id_or_to_id_eq)"
puts
puts "Expected: All should work since we're only filtering by foreign key values"
puts "Actual: OR between polymorphic FKs throws polymorphic class computation error"
puts
Loading