ransack icon indicating copy to clipboard operation
ransack copied to clipboard

Ransack not_eq not working

Open msyesyan opened this issue 6 years ago • 9 comments

undefined method eq for #<Arel::Nodes::Equality:0x007fb12685a2d0>

I found there is a closed issue about this issue. And I have found the repro steps. The issues occurs when ransack a association attributes and the child model extend a parent model.

for example:

class Patient
    has_many :visits
    has_many :follow_visits
end

class Visit
    belongs_to :patient
end

class FollowVisit < Visit
    belongs_to :patient
end
Patient.ransack({visits_name_not_eql: 'test'}) #=> this is successful
Patient.ransack({follow_visits_name_not_eql: 'test'}) #=> raise this error

I have followed the source code, and I found the error code is in adapters/active_record/context.rb:193

def build_correlated_subquery(association)
  join_constraints = extract_joins(association)
  join_root = join_constraints.shift
  correlated_key = join_root.right.expr.left
  subquery = Arel::SelectManager.new(association.base_klass)
  subquery.from(join_root.left)
  subquery.project(correlated_key)
  join_constraints.each do |j|
    subquery.join_sources << Arel::Nodes::InnerJoin.new(j.left, j.right)
  end
  binding.pry
  subquery.where(correlated_key.eq(primary_key))
end

the expect(correlated_key.is_a? Ransack::Nodes::Attribute).to be_true is false, because correlated_key is a Arel::Nodes::Equality I don't know how to fix this, can you help ? @seanfcarroll thanks

msyesyan avatar Jun 14 '18 02:06 msyesyan

I haven't find a good way to fix this, @jonatack could you please have a look? ^_^

msyesyan avatar Jun 21 '18 07:06 msyesyan

Try declaring the table name in the STI model. @msyesyan

alexisraca avatar Nov 01 '18 22:11 alexisraca

I think I found the issue, not sure, but if I change in the Context#extract_correlated_key to detect children its working..

when Arel::Nodes::And if join_root.children.any? join_root.children.each do |child| eck = extract_correlated_key(child) return eck unless eck.nil? end else extract_correlated_key(join_root.left) || extract_correlated_key(join_root.right) end

ciihla avatar Dec 01 '21 10:12 ciihla

@ciihla Can you create a PR with your solution?

deivid-rodriguez avatar Mar 07 '22 15:03 deivid-rodriguez

I don't have time for writing proper tests, etc, but basically, this is working for us:

module RansackPatch
  def self.included(base)
    base.class_eval do
      alias_method :extract_correlated_key, :extract_correlated_key_with_children_support
    end
  end

  def extract_correlated_key_with_children_support(join_root)
    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
      end
    when Arel::Nodes::And
      # This is new to support nested children
      if join_root.children.any?
        join_root.children.each do |child|
          eck = extract_correlated_key(child)
          return eck unless eck.nil?
        end
      else
        extract_correlated_key(join_root.left) || extract_correlated_key(join_root.right)
      end
    else # rubocop:disable Style/EmptyElse
      # eg parent was Arel::Nodes::And and the evaluated side was one of
      # Arel::Nodes::Grouping or MultiTenant::TenantEnforcementClause
      nil
    end
  end
end

::Ransack::Adapters::ActiveRecord::Context.include RansackPatch

ciihla avatar Mar 08 '22 08:03 ciihla

It'd be ok to just create a PR with the patch, I can handle tests later.

deivid-rodriguez avatar Mar 08 '22 09:03 deivid-rodriguez

https://github.com/activerecord-hackery/ransack/pull/1279/files

ciihla avatar Mar 08 '22 09:03 ciihla

Thanks!

deivid-rodriguez avatar Mar 08 '22 09:03 deivid-rodriguez

I tried to reproduce this with a spec using the models in the original post, but it all worked fine (I had to modify the models to actually inherit from ActiveRecord::Base). So still struggling to find a repro here.

deivid-rodriguez avatar Mar 08 '22 11:03 deivid-rodriguez