Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix not_in search with deep nesting #1537

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
12 changes: 11 additions & 1 deletion lib/ransack/nodes/condition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,13 @@ def negative?
def arel_predicate
predicate = attributes.map { |attribute|
association = attribute.parent
if negative? && attribute.associated_collection?
parent_table = association.table

if parent_table.class == Arel::Nodes::TableAlias && association.reflection.class == ActiveRecord::Reflection::ThroughReflection
parent_table.right = parent_table.left.name
end

if negative? && attribute.associated_collection? && not_nested_condition(attribute, parent_table)
query = context.build_correlated_subquery(association)
context.remove_association(association)
if self.predicate_name == 'not_null' && self.value
Expand All @@ -313,6 +319,10 @@ def arel_predicate
predicate
end

def not_nested_condition(attribute, parent_table)
parent_table.class != Arel::Nodes::TableAlias && attribute.name.starts_with?(parent_table.name)
end

private

def combinator_method
Expand Down
35 changes: 35 additions & 0 deletions spec/ransack/adapters/active_record/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,41 @@ module ActiveRecord
end
end

context 'negative conditions on related object with HABTM associations' do
let(:medieval) { Tag.create!(name: 'Medieval') }
let(:fantasy) { Tag.create!(name: 'Fantasy') }
let(:arthur) { Article.create!(title: 'King Arthur') }
let(:marco) { Article.create!(title: 'Marco Polo') }
let(:comment_arthur) { marco.comments.create!(body: 'King Arthur comment') }
let(:comment_marco) { arthur.comments.create!(body: 'Marco Polo comment') }

before do
comment_arthur.tags << medieval
comment_marco.tags << fantasy
end

it 'removes redundant joins from top query' do
s = Article.ransack(comments_tags_name_not_eq: "Fantasy")
sql = s.result.to_sql
expect(sql).to include('LEFT OUTER JOIN')
end

it 'handles != for single values' do
s = Article.ransack(comments_tags_name_not_eq: "Fantasy")
articles = s.result.to_a
expect(articles).to include marco
expect(articles).to_not include arthur
end

it 'handles NOT IN for multiple attributes' do
s = Article.ransack(comments_tags_name_not_in: ["Fantasy", "Scifi"])
articles = s.result.to_a

expect(articles).to include marco
expect(articles).to_not include arthur
end
end

context 'negative conditions on self-referenced associations' do
let(:pop) { Person.create!(name: 'Grandpa') }
let(:dad) { Person.create!(name: 'Father') }
Expand Down
2 changes: 1 addition & 1 deletion spec/ransack/search_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ module Ransack
# AND "articles"."title" = 'Test' AND "articles"."published" = 't' AND ('default_scope' = 'default_scope')
# ) ORDER BY "people"."id" DESC

pending("spec should pass, but I do not know how/where to fix lib code")
#pending("spec should pass, but I do not know how/where to fix lib code")
s = Search.new(Person, published_articles_title_not_eq: 'Test')
expect(s.result.to_sql).to include 'default_scope'
expect(s.result.to_sql).to include 'published'
Expand Down
7 changes: 7 additions & 0 deletions spec/support/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,14 @@ class Article < ::Article
class Comment < ApplicationRecord
belongs_to :article
belongs_to :person
has_and_belongs_to_many :tags

default_scope { where(disabled: false) }
end

class Tag < ApplicationRecord
has_and_belongs_to_many :articles
has_and_belongs_to_many :comments
end

class Note < ApplicationRecord
Expand Down Expand Up @@ -298,6 +300,11 @@ def self.create
t.integer :tag_id
end

create_table :comments_tags, force: true, id: false do |t|
t.integer :comment_id
t.integer :tag_id
end

create_table :notes, force: true do |t|
t.integer :notable_id
t.string :notable_type
Expand Down