diff --git a/POLYMORPHIC_ISSUE_README.md b/POLYMORPHIC_ISSUE_README.md new file mode 100644 index 000000000..a6fd1993a --- /dev/null +++ b/POLYMORPHIC_ISSUE_README.md @@ -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. \ No newline at end of file diff --git a/spec/blueprints/messages.rb b/spec/blueprints/messages.rb new file mode 100644 index 000000000..2e5ea14a1 --- /dev/null +++ b/spec/blueprints/messages.rb @@ -0,0 +1,4 @@ +Message.blueprint do + user { Person.make } + content { "Test message content" } +end \ No newline at end of file diff --git a/spec/ransack/adapters/active_record/polymorphic_spec.rb b/spec/ransack/adapters/active_record/polymorphic_spec.rb new file mode 100644 index 000000000..899109766 --- /dev/null +++ b/spec/ransack/adapters/active_record/polymorphic_spec.rb @@ -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: 'alice@example.com') } + let!(:person2) { Person.create!(name: 'Bob', email: 'bob@example.com') } + 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 \ No newline at end of file diff --git a/spec/support/schema.rb b/spec/support/schema.rb index 02e5b4a1f..246f55504 100644 --- a/spec/support/schema.rb +++ b/spec/support/schema.rb @@ -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 @@ -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 @@ -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: 'alice@example.com') + person2 = Person.create!(name: 'Bob', email: 'bob@example.com') + 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 diff --git a/test_polymorphic_issue.rb b/test_polymorphic_issue.rb new file mode 100644 index 000000000..46dde16cb --- /dev/null +++ b/test_polymorphic_issue.rb @@ -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: 'alice@example.com') + @person2 = Person.create!(name: 'Bob', email: 'bob@example.com') + @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 \ No newline at end of file