`extract_correlated_key` incorrectly handles `Arel::Nodes::And` nodes with multiple children

[riseshia]

Summary

ransack fails to build query which has more than 2 conditions on join clause, that is a quite rare case such as the relation using polymorphic and default_scope both. ;)

Reproduction

This could be reproducable via all versions, with Rails >= 7.1

# test-extract-correlated-key-bug.rb

# Run it in your console with: `ruby test-extract-correlated-key-bug.rb`

unless File.exist?('Gemfile')
  File.write('Gemfile', <<-GEMFILE)
    source 'https://rubygems.org'

    # Rails master
    gem 'rails', github: 'rails/rails', branch: 'main'

    # Rails last release
    # gem 'rails'

    gem 'sqlite3'
    gem 'ransack', github: 'activerecord-hackery/ransack'
  GEMFILE

  system 'bundle install'
end

require 'bundler'
Bundler.setup(:default)

require 'active_record'
require 'minitest/autorun'
require 'logger'
require 'ransack'

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

# Display versions.
message = "Running test case with Ruby #{RUBY_VERSION}, Active Record #{
  ::ActiveRecord::VERSION::STRING}, Arel #{Arel::VERSION} and #{
  ::ActiveRecord::Base.connection.adapter_name}"
line = '=' * message.length
puts line, message, line

ActiveRecord::Schema.define do
  create_table :articles, force: true do |t|
    t.string :title
    t.text :body
    t.boolean :published, null: false, default: false
    t.timestamps null: false
  end

  create_table :notes, force: true do |t|
    t.string :note
    t.references :notable, polymorphic: true, null: false
    t.timestamps null: false
  end
end

class Article < ActiveRecord::Base
  has_many :notes, as: :notable
  has_many :recent_notes, as: :notable

  default_scope { where(published: true) }

  def self.ransackable_attributes(auth_object = nil)
    ["body", "created_at", "id", "published", "title", "updated_at"]
  end

  def self.ransackable_associations(auth_object = nil)
    ["notes", "recent_notes"]
  end
end

class Note < ActiveRecord::Base
  belongs_to :notable, polymorphic: true

  def self.ransackable_attributes(auth_object = nil)
    ["created_at", "id", "note", "notable_id", "notable_type", "updated_at"]
  end
end

class RecentNote < ActiveRecord::Base
  self.table_name = "notes"
  default_scope { where(notable_id: 1) }

  belongs_to :notable, polymorphic: true

  def self.ransackable_attributes(auth_object = nil)
    ["created_at", "id", "note", "notable_id", "notable_type", "updated_at"]
  end
end

class BugTest < Minitest::Test
  def test_extract_correlated_key_with_and_node
    # This test case demonstrates the bug where extract_correlated_key
    # returns nil for Arel::Nodes::And nodes instead of properly
    # checking all children nodes.

    search = Article.ransack(
      g: [
        {
          m: "and",
          c: [
            {
              a: ["recent_notes_note"],
              p: "not_cont_all",
              v: ["aa"],
            }
          ]
        }
      ]
    )

    expected_sql = <<~SQL.strip
      SELECT DISTINCT "articles".* FROM "articles"
      LEFT OUTER JOIN "notes" "recent_notes_articles_join"
        ON "recent_notes_articles_join"."notable_type" = 'Article'
        AND "recent_notes_articles_join"."notable_id" = "articles"."id"
        AND ("recent_notes_articles_join"."notable_id" = 1)
      WHERE "articles"."published" = 1
        AND (NOT ("recent_notes_articles_join"."note" LIKE '%aa%'))
    SQL

    # raises `NoMethodError: undefined method 'eq' for nil`
    assert_equal expected_sql, search.result.to_sql.strip
  end
end

How to fix

It seems to be enough to check all children instead of check left/right child:

--- a/lib/ransack/adapters/active_record/context.rb
+++ b/lib/ransack/adapters/active_record/context.rb
@@ -201,7 +201,10 @@ module Ransack
               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 via 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

Here is the patch with this #1572.

[riseshia]

Close via #1572