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 ransack_alias issue, close #1239 #1512

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

itsalongstory
Copy link
Contributor

@itsalongstory itsalongstory commented Aug 1, 2024

Close #1239

test_ransack.rb

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'
  gem 'activerecord', '~> 7.1', '>= 7.1.3.4', require: "active_record"
  gem 'sqlite3', '~> 1.7', '>= 1.7.3'
  gem 'ransack', '~> 4.2'
  gem 'minitest', '~> 5.24', '>= 5.24.1', require: "minitest/autorun"
end

ActiveRecord::Base.establish_connection(
  adapter:  "sqlite3",
  database: "./test_ransack_alias"
)

ActiveRecord::Schema.define do
  drop_table(:fees, if_exists: true)

  create_table :fees do |t|
    t.integer :amount
  end
end


class Fee < ActiveRecord::Base
  ransack_alias :amount_a, :amount

  def self.ransackable_attributes(auth_object = nil)
    ["amount", "amount_a"]
  end
end

class MyTest < Minitest::Test
  # success
  def test_1
    query_params = { amount_not_eq: 1 }
    assert_equal "SELECT \"fees\".* FROM \"fees\" WHERE \"fees\".\"amount\" != 1", Fee.ransack(query_params).result.to_sql
  end

  # success
  def test_2
    query_params = { amount_not_eq: 1, amount_a_not_eq: 2 }
    assert_equal "SELECT \"fees\".* FROM \"fees\" WHERE (\"fees\".\"amount\" != 1 AND \"fees\".\"amount\" != 2)", Fee.ransack(query_params).result.to_sql
  end

  # failure
  def test_3
    query_params = { amount_a_not_eq: 2, amount_not_eq: 1 }
    assert_equal "SELECT \"fees\".* FROM \"fees\" WHERE (\"fees\".\"amount\" != 2 AND \"fees\".\"amount\" != 1)", Fee.ransack(query_params).result.to_sql
  end
end
@itsalongstory ➜ /workspaces $ ruby test_ransack.rb 
Fetching gem metadata from https://rubygems.org/........
Resolving dependencies...
-- drop_table(:fees, {:if_exists=>true})
   -> 0.0272s
-- create_table(:fees)
   -> 0.0005s
Run options: --seed 4070

# Running:

F..

Finished in 0.010956s, 273.8346 runs/s, 273.8346 assertions/s.

  1) Failure:
MyTest#test_3 [test_ransack.rb:49]:
--- expected
+++ actual
@@ -1 +1 @@
-"SELECT \"fees\".* FROM \"fees\" WHERE (\"fees\".\"amount\" != 2 AND \"fees\".\"amount\" != 1)"
+"SELECT \"fees\".* FROM \"fees\" WHERE \"fees\".\"amount\" != 1"


3 runs, 3 assertions, 1 failures, 0 errors, 0 skips

@le0pard
Copy link

le0pard commented Sep 13, 2024

Can #1525 also fix this issue? Because now reject! based on name and key in condition

@itsalongstory
Copy link
Contributor Author

Can #1525 also fix this issue? Because now reject! based on name and key in condition

#1525 doesn't fix this.

@le0pard
Copy link

le0pard commented Sep 14, 2024

@itsalongstory thanks for check

@itsalongstory
Copy link
Contributor Author

Can #1525 also fix this issue? Because now reject! based on name and key in condition

diff --git a/lib/ransack/nodes/grouping.rb b/lib/ransack/nodes/grouping.rb
index 95d6488..6d72097 100644
--- a/lib/ransack/nodes/grouping.rb
+++ b/lib/ransack/nodes/grouping.rb
@@ -53,7 +53,7 @@ module Ransack
       end
 
       def []=(key, value)
-        conditions.reject! { |c| c.same_name_or_key?(key) }
+        conditions.reject! { |c| c.same_name_or_key?(key) && c.value == value.value }
         self.conditions << value
       end

Maybe we need to reject! based on name and key, and also value.

@le0pard
Copy link

le0pard commented Sep 14, 2024

ok, got a problem. Maybe this commit will be enough - aa2062c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ransack_alias sometimes does not work correctly
2 participants