Ransack subquery broken in rails 5.2.4

[stevenjonescgm]

Rails 5.2.4 fixed before_add/after_add (rails/rails#33249) by modifying association.join_constraints

This caused the expected structure for https://github.com/activerecord-hackery/ransack/blob/master/lib/ransack/adapters/active_record/context.rb#L175 extract_correlated_key to change. Now this ransack method returns the wrong attribute if the join clause has more conditions than only primary key.

I'm working on a fix for my app, but wanted to open this to let others know of the issue and make progress toward a fix (ahead of forking and making a pr)

Pseudocode from my app

class Customer < ApplicationRecord
  has_many :invoices
end

class Invoice < ApplicationRecord
  default_scope { where(disabled: false) }
  belongs_to :customer
end

puts Customer.ransack({"invoices_number_not_eq"=> 123}).result.to_sql
# SELECT "customers".* FROM "customers" WHERE "customers"."id" NOT IN (SELECT "invoices"."disabled" FROM "invoices" WHERE "invoices"."disabled" = "customers"."id" AND NOT ("invoices"."number" != '123'))

Notice "invoices"."disabled" = "customers"."id"
The bug was particularly exceptional for me because I use activerecord-multi-tenant and so the incorrect node selected is a MultiTenant::TenantEnforcementClause which does not respond to eq in subquery.where(correlated_key.eq(primary_key))

 Rails 5.2.4 changed `association.join_constraints` to put conditions earlier in the chain
   so they are available for `after_add/before_add` callbacks
   such that

 Arel::Nodes::OuterJoin
   Arel::Table
   Arel::Nodes::On
     Arel::Nodes::And
       Arel::Nodes::Equality                  # < join_root.right.expr.left
         Arel::Attributes::Attribute
         Arel::Attributes::Attribute
       Array

 became

 Arel::Nodes::OuterJoin
   Arel::Table
   Arel::Nodes::On
     Arel::Nodes::And
       Arel::Nodes::Equality                  # < join_root.right.expr.left
         Arel::Attributes::Attribute          # invoices.disabled
         Arel::Nodes::BindParam               # disabled: false
       Arel::Nodes::Equality
         Arel::Attributes::Attribute          # invoices.customer_id
         Arel::Attributes::Attribute          # customers.id aka primary_key

 Or with activerecord-multi-tenant

 Arel::Nodes::OuterJoin
   Arel::Table
   Arel::Nodes::On
     Arel::Nodes::And
       MultiTenant::TenantEnforcementClause   # < join_root.right.expr.left
       Arel::Nodes::And
         Arel::Nodes::Equality
           Arel::Attributes::Attribute
           Arel::Nodes::BindParam
         Arel::Nodes::Equality
           Arel::Attributes::Attribute
           Arel::Attributes::Attribute

The fix I'm working on looks like

def extract_correlated_key(join_root)
  # Previous implementation assumed the arel structure and presumed which node was equal `primary_key`
  #   and return it's opposite Attribute in an assumed parent Equality
  # Instead let's try to walk the arbitrary structure to find an Equality containing `primary_key`
  #
  case join_root
  when Arel::Nodes::OuterJoin
    # one of join_root.right/join_root.left is expected to be Arel::Nodes::On
    if join_root.right.is_a?(Arel::Nodes::On)
      extract_correlated_key(join_root.right.expr)
    elsif join_root.left.is_a?(Arel::Nodes::On)
      extract_correlated_key(join_root.left.expr)
    else
      raise 'Ransack encountered an unexpected arel structure'
    end
  when Arel::Nodes::Equality
    pk = primary_key
    if join_root.left == pk
      join_root.right
    elsif join_root.right == pk
      join_root.left
    else
      nil
    end
  when Arel::Nodes::And
    extract_correlated_key(join_root.left) || extract_correlated_key(join_root.right)
  else
    # eg parent was Arel::Nodes::And and the evaluated side was one of
    # Arel::Nodes::Grouping or MultiTenant::TenantEnforcementClause
    nil
  end
end

[stevenjonescgm]

fix has been merged