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

Breaking change in 2.0.0 re: joins+order #947

Closed
jaredbeck opened this issue Aug 10, 2018 · 10 comments · Fixed by #952
Closed

Breaking change in 2.0.0 re: joins+order #947

jaredbeck opened this issue Aug 10, 2018 · 10 comments · Fixed by #952

Comments

@jaredbeck
Copy link
Contributor

This test passes in rails 5.2.0 + ransack 1.8.8.
This test fails in rails 5.2.1 + ransack 2.0.0.
Is this an intentional breaking change, or a bug?

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue ::LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "ransack", "2.0.0", require: false
  gem "rails", "5.2.1", require: false
  gem "sqlite3"
end

require "active_record"
require "ransack"
require "minitest/autorun"
require "logger"

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

::ActiveRecord::Schema.define do
  create_table :books, force: true do |t|
  end

  create_table :chapters, force: true do |t|
    t.string :name
    t.belongs_to :book
  end

  create_table :pages, force: true do |t|
    t.belongs_to :chapter
  end
end

class Book < ::ActiveRecord::Base
end

class Chapter < ::ActiveRecord::Base
  belongs_to :book
end

class Page < ::ActiveRecord::Base
  belongs_to :chapter
end

class BugTest < ::ActiveSupport::TestCase
  # Test passes in rails 5.2.0 + ransack 1.8.8
  # Test fails in rails 5.2.1 + ransack 2.0.0
  def test_association_stuff
    b = Book.create!
    c = Chapter.create!(book: b)
    Page.create!(chapter: c)
    search = { chapter_book_id_eq: b.id }
    assert_nothing_raised {
      ::Page.joins(:chapter).order('chapter.name').ransack(search).result(distinct: true)
    }
    # NoMethodError: undefined method `[]' for nil:NilClass
    # /Users/jared/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/ransack-2.0.0/lib/ransack/nodes/bindable.rb:42:in `get_attribute'
    # /Users/jared/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/ransack-2.0.0/lib/ransack/nodes/bindable.rb:34:in `get_arel_attribute'
  end
end
@kennyadsl
Copy link

This also happens with:

::Page.joins(:chapter).ransack(search)

so I think it's just related to the join.

For some reason, at a certain point table_for(parent) is called. parent is:

#<ActiveRecord::Associations::JoinDependency::JoinAssociation:0x007ff7afa59200 @join_type=Arel::Nodes::InnerJoin, @base_klass=Chapter(id: integer, name: string, book_id: integer), @children=[], @reflection=#<ActiveRecord::Reflection::BelongsToReflection:0x007ff7b0069370 @name=:chapter, @scope=nil, @options={}, @active_record=Page(id: integer, chapter_id: integer), @klass=Chapter(id: integer, name: string, book_id: integer), @plural_name="chapters", @type=nil, @foreign_type=nil, @constructable=true, @association_scope_cache=#<Concurrent::Map:0x007ff7b0069118 entries=0 default_proc=nil>, @class_name="Chapter", @inverse_name=nil, @foreign_key="chapter_id", @join_keys=#<struct ActiveRecord::Reflection::AbstractReflection::JoinKeys key="id", foreign_key="chapter_id">>, @tables=nil>

and calling .table on it returns nil which I think is not the expected behavior.

@jaredbeck
Copy link
Contributor Author

jaredbeck commented Aug 10, 2018

We need to know whether this is a bug or an intentional breaking change. However, in the meantime, we can work around this issue by doing our joins after ransack. With that workaround, I'll be upgrading to 2.0.0.

@gregmolnar
Copy link
Member

@jaredbeck It isn't an intentional change. I will fix later this week but PRs welcomed.

@jaredbeck
Copy link
Contributor Author

Thanks Greg. I used the workaround I mentioned above and upgraded to 2.0.0 in production on Friday. We'll see if anything blows up when business hours begin on the East coast today. (knock on wood)

@VygovskySergey
Copy link

VygovskySergey commented Aug 13, 2018

Hey, guys, I have tried different approaches with this, and find out that this error raised for
has_one and belongs_to, for has_many it looks good

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue ::LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "ransack", "2.0.0", require: false
  gem "rails", "5.2.1", require: false
  gem "sqlite3"
end

require "active_record"
require "ransack"
require "minitest/autorun"
require "logger"

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

::ActiveRecord::Schema.define do
  create_table :authors, force: true do |t|
  end

  create_table :books, force: true do |t|
    t.belongs_to :author
  end

  create_table :chapters, force: true do |t|
    t.string :name
    t.belongs_to :book
  end

  create_table :pages, force: true do |t|
    t.belongs_to :chapter
  end
end

class Book < ::ActiveRecord::Base
  has_many :chapters
  belongs_to :author
end

class Author < ::ActiveRecord::Base
  has_one :book
end

class Chapter < ::ActiveRecord::Base
  has_many :chapters
  belongs_to :book
end

class Page < ::ActiveRecord::Base
  belongs_to :chapter
end

class BugTest < ::ActiveSupport::TestCase
  # Test passes in rails 5.2.0 + ransack 1.8.8
  # Test fails in rails 5.2.1 + ransack 2.0.0
  def test_association_stuff
    a  = Author.create!
    b = Book.create!(author: a)
    c = Chapter.create!(book: b)
    Page.create!(chapter: c)
    search_author   = { book_id_eq: b.id }
    search_book = { chapter_id_eq: c.id }
    search = { chapter_book_id_eq: b.id }

    # Raises error for has_one association
    # assert_nothing_raised {
    #   ::Author.joins(:book).ransack(search_author).result(distinct: true)
    # }

    # Doesn't raise error for has_many association
    assert_nothing_raised {
      ::Book.joins(:chapters).ransack(search_book).result(distinct: true)
    }

    # Raises error for belongs_to association
    # assert_nothing_raised {
    #   ::Page.joins(:chapter).ransack(search).result(distinct: true)
    # }
    # NoMethodError: undefined method `[]' for nil:NilClass
    # /Users/jared/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/ransack-2.0.0/lib/ransack/nodes/bindable.rb:42:in `get_attribute'
    # /Users/jared/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/ransack-2.0.0/lib/ransack/nodes/bindable.rb:34:in `get_arel_attribute'
  end
end

@CGA1123
Copy link
Contributor

CGA1123 commented Aug 16, 2018

Can people check whether #951 fixes this for you?

@kennyadsl
Copy link

@CGA1123 the script above works for me using your branch. Thanks!

@jsantos
Copy link

jsantos commented Aug 17, 2018

@CGA1123 Seems to work here. Thanks a lot!

@mensfeld
Copy link

@jaredbeck any chance for 2.0.1 release with this fix?

@gregmolnar
Copy link
Member

2.0.1 is released with a fix for this issue.

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 a pull request may close this issue.

7 participants