From 58310e3542eeba2a5b4bc00a8ad5d8852c47b033 Mon Sep 17 00:00:00 2001 From: Greg Molnar Date: Sun, 14 Apr 2019 16:26:59 +0200 Subject: [PATCH 1/5] remove old polyamourous spec files --- polyamorous/spec/blueprints/articles.rb | 5 - polyamorous/spec/blueprints/comments.rb | 5 - polyamorous/spec/blueprints/notes.rb | 3 - polyamorous/spec/blueprints/people.rb | 4 - polyamorous/spec/blueprints/tags.rb | 3 - .../spec/helpers/polyamorous_helper.rb | 26 ----- .../spec/polyamorous/join_association_spec.rb | 29 ------ .../spec/polyamorous/join_dependency_spec.rb | 93 ------------------ polyamorous/spec/polyamorous/join_spec.rb | 19 ---- polyamorous/spec/spec_helper.rb | 43 -------- polyamorous/spec/support/schema.rb | 98 ------------------- 11 files changed, 328 deletions(-) delete mode 100644 polyamorous/spec/blueprints/articles.rb delete mode 100644 polyamorous/spec/blueprints/comments.rb delete mode 100644 polyamorous/spec/blueprints/notes.rb delete mode 100644 polyamorous/spec/blueprints/people.rb delete mode 100644 polyamorous/spec/blueprints/tags.rb delete mode 100644 polyamorous/spec/helpers/polyamorous_helper.rb delete mode 100644 polyamorous/spec/polyamorous/join_association_spec.rb delete mode 100644 polyamorous/spec/polyamorous/join_dependency_spec.rb delete mode 100644 polyamorous/spec/polyamorous/join_spec.rb delete mode 100644 polyamorous/spec/spec_helper.rb delete mode 100644 polyamorous/spec/support/schema.rb diff --git a/polyamorous/spec/blueprints/articles.rb b/polyamorous/spec/blueprints/articles.rb deleted file mode 100644 index 42e2f14..0000000 --- a/polyamorous/spec/blueprints/articles.rb +++ /dev/null @@ -1,5 +0,0 @@ -Article.blueprint do - person - title - body -end \ No newline at end of file diff --git a/polyamorous/spec/blueprints/comments.rb b/polyamorous/spec/blueprints/comments.rb deleted file mode 100644 index 1ecc7eb..0000000 --- a/polyamorous/spec/blueprints/comments.rb +++ /dev/null @@ -1,5 +0,0 @@ -Comment.blueprint do - article - person - body -end \ No newline at end of file diff --git a/polyamorous/spec/blueprints/notes.rb b/polyamorous/spec/blueprints/notes.rb deleted file mode 100644 index f143114..0000000 --- a/polyamorous/spec/blueprints/notes.rb +++ /dev/null @@ -1,3 +0,0 @@ -Note.blueprint do - note -end \ No newline at end of file diff --git a/polyamorous/spec/blueprints/people.rb b/polyamorous/spec/blueprints/people.rb deleted file mode 100644 index b7ba882..0000000 --- a/polyamorous/spec/blueprints/people.rb +++ /dev/null @@ -1,4 +0,0 @@ -Person.blueprint do - name - salary -end \ No newline at end of file diff --git a/polyamorous/spec/blueprints/tags.rb b/polyamorous/spec/blueprints/tags.rb deleted file mode 100644 index f0fb3d8..0000000 --- a/polyamorous/spec/blueprints/tags.rb +++ /dev/null @@ -1,3 +0,0 @@ -Tag.blueprint do - name { Sham.tag_name } -end \ No newline at end of file diff --git a/polyamorous/spec/helpers/polyamorous_helper.rb b/polyamorous/spec/helpers/polyamorous_helper.rb deleted file mode 100644 index 8dff269..0000000 --- a/polyamorous/spec/helpers/polyamorous_helper.rb +++ /dev/null @@ -1,26 +0,0 @@ -module PolyamorousHelper - if ActiveRecord::VERSION::STRING >= "4.1" - def new_join_association(reflection, children, klass) - Polyamorous::JoinAssociation.new reflection, children, klass - end - else - def new_join_association(reflection, join_dependency, parent, klass) - Polyamorous::JoinAssociation.new reflection, join_dependency, parent, klass - end - end - - if ActiveRecord::VERSION::STRING >= "5.2" - def new_join_dependency(klass, associations = {}) - alias_tracker = ::ActiveRecord::Associations::AliasTracker.create(klass.connection, klass.table_name, []) - Polyamorous::JoinDependency.new klass, klass.arel_table, associations, alias_tracker - end - else - def new_join_dependency(klass, associations = {}) - Polyamorous::JoinDependency.new klass, associations, [] - end - end - - def new_join(name, type = Polyamorous::InnerJoin, klass = nil) - Polyamorous::Join.new name, type, klass - end -end diff --git a/polyamorous/spec/polyamorous/join_association_spec.rb b/polyamorous/spec/polyamorous/join_association_spec.rb deleted file mode 100644 index bf843e3..0000000 --- a/polyamorous/spec/polyamorous/join_association_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -require 'spec_helper' - -module Polyamorous - describe JoinAssociation do - - join_base, join_association_args, polymorphic = [:join_root, 'parent.children', 'reflection.options[:polymorphic]'] - let(:join_dependency) { new_join_dependency Note, {} } - let(:reflection) { Note.reflect_on_association(:notable) } - let(:parent) { join_dependency.send(join_base) } - let(:join_association) { - eval("new_join_association(reflection, #{join_association_args}, Article)") - } - - it 'leaves the orginal reflection intact for thread safety' do - reflection.instance_variable_set(:@klass, Article) - join_association - .swapping_reflection_klass(reflection, Person) do |new_reflection| - expect(new_reflection.options).not_to equal reflection.options - expect(new_reflection.options).not_to have_key(:polymorphic) - expect(new_reflection.klass).to eq(Person) - expect(reflection.klass).to eq(Article) - end - end - - it 'sets the polymorphic option to true after initializing' do - expect(join_association.instance_eval(polymorphic)).to be true - end - end -end diff --git a/polyamorous/spec/polyamorous/join_dependency_spec.rb b/polyamorous/spec/polyamorous/join_dependency_spec.rb deleted file mode 100644 index dd64dc9..0000000 --- a/polyamorous/spec/polyamorous/join_dependency_spec.rb +++ /dev/null @@ -1,93 +0,0 @@ -require 'spec_helper' - -module Polyamorous - describe JoinDependency do - - method, join_associations, join_base = - if ActiveRecord::VERSION::STRING >= '4.1' - [:instance_eval, 'join_root.drop(1)', :join_root] - else - [:send, 'join_associations', :join_base] - end - - context 'with symbol joins' do - subject { new_join_dependency Person, articles: :comments } - - specify { expect(subject.send(method, join_associations).size) - .to eq(2) } - specify { expect(subject.send(method, join_associations).map(&:join_type)) - .to be_all { Polyamorous::InnerJoin } } - end - - context 'with has_many :through association' do - subject { new_join_dependency Person, :authored_article_comments } - - specify { expect(subject.send(method, join_associations).size) - .to eq 1 } - specify { expect(subject.send(method, join_associations).first.table_name) - .to eq 'comments' } - end - - context 'with outer join' do - subject { new_join_dependency Person, new_join(:articles, :outer) } - - specify { expect(subject.send(method, join_associations).size) - .to eq 1 } - specify { expect(subject.send(method, join_associations).first.join_type) - .to eq Polyamorous::OuterJoin } - end - - context 'with nested outer joins' do - subject { new_join_dependency Person, - new_join(:articles, :outer) => new_join(:comments, :outer) } - - specify { expect(subject.send(method, join_associations).size) - .to eq 2 } - specify { expect(subject.send(method, join_associations).map(&:join_type)) - .to eq [Polyamorous::OuterJoin, Polyamorous::OuterJoin] } - specify { expect(subject.send(method, join_associations).map(&:join_type)) - .to be_all { Polyamorous::OuterJoin } } - end - - context 'with polymorphic belongs_to join' do - subject { new_join_dependency Note, new_join(:notable, :inner, Person) } - - specify { expect(subject.send(method, join_associations).size) - .to eq 1 } - specify { expect(subject.send(method, join_associations).first.join_type) - .to eq Polyamorous::InnerJoin } - specify { expect(subject.send(method, join_associations).first.table_name) - .to eq 'people' } - end - - context 'with polymorphic belongs_to join and nested symbol join' do - subject { new_join_dependency Note, - new_join(:notable, :inner, Person) => :comments } - - specify { expect(subject.send(method, join_associations).size) - .to eq 2 } - specify { expect(subject.send(method, join_associations).map(&:join_type)) - .to be_all { Polyamorous::InnerJoin } } - specify { expect(subject.send(method, join_associations).first.table_name) - .to eq 'people' } - specify { expect(subject.send(method, join_associations)[1].table_name) - .to eq 'comments' } - end - - context '#left_outer_join in Rails 5 overrides join type specified', - if: ActiveRecord::VERSION::MAJOR >= 5 && ActiveRecord::VERSION::MINOR < 2 do - - let(:join_type_class) do - new_join_dependency( - Person, - new_join(:articles) - ).join_constraints( - [], - Arel::Nodes::OuterJoin - ).first.joins.map(&:class) - end - - specify { expect(join_type_class).to eq [Arel::Nodes::OuterJoin] } - end - end -end diff --git a/polyamorous/spec/polyamorous/join_spec.rb b/polyamorous/spec/polyamorous/join_spec.rb deleted file mode 100644 index 5e2bfc3..0000000 --- a/polyamorous/spec/polyamorous/join_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require 'spec_helper' - -module Polyamorous - describe Join do - it 'is a tree node' do - join = new_join(:articles, :outer) - expect(join).to be_kind_of(TreeNode) - end - - it 'can be added to a tree' do - join = new_join(:articles, :outer) - - tree_hash = {} - join.add_to_tree(tree_hash) - - expect(tree_hash[join]).to be {} - end - end -end diff --git a/polyamorous/spec/spec_helper.rb b/polyamorous/spec/spec_helper.rb deleted file mode 100644 index 21450e3..0000000 --- a/polyamorous/spec/spec_helper.rb +++ /dev/null @@ -1,43 +0,0 @@ -require 'machinist/active_record' -require 'sham' -require 'faker' -require 'polyamorous' - -Time.zone = 'Eastern Time (US & Canada)' - -Dir[File.expand_path('../{helpers,support,blueprints}/**/*.rb', __FILE__)] -.each do |f| - require f -end - -Sham.define do - name { Faker::Name.name } - title { Faker::Lorem.sentence } - body { Faker::Lorem.paragraph } - salary { |index| 30000 + (index * 1000) } - tag_name { Faker::Lorem.words(3).join(' ') } - note { Faker::Lorem.words(7).join(' ') } -end - -RSpec.configure do |config| - config.before(:suite) do - message = "Running Polyamorous specs with #{ - ActiveRecord::Base.connection.adapter_name - }, Active Record #{::ActiveRecord::VERSION::STRING}, Arel #{Arel::VERSION - } and Ruby #{RUBY_VERSION}" - line = '=' * message.length - puts line, message, line - Schema.create - end - config.before(:all) { Sham.reset(:before_all) } - config.before(:each) { Sham.reset(:before_each) } - - config.include PolyamorousHelper -end - -RSpec::Matchers.define :be_like do |expected| - match do |actual| - actual.gsub(/^\s+|\s+$/, '').gsub(/\s+/, ' ').strip == - expected.gsub(/^\s+|\s+$/, '').gsub(/\s+/, ' ').strip - end -end diff --git a/polyamorous/spec/support/schema.rb b/polyamorous/spec/support/schema.rb deleted file mode 100644 index 96994e2..0000000 --- a/polyamorous/spec/support/schema.rb +++ /dev/null @@ -1,98 +0,0 @@ -require 'active_record' - -ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:') - -class Person < ActiveRecord::Base - belongs_to :parent, class_name: 'Person', foreign_key: :parent_id - has_many :children, class_name: 'Person', foreign_key: :parent_id - has_many :articles - has_many :comments - has_many :authored_article_comments, through: :articles, - foreign_key: :person_id, source: :comments - has_many :notes, as: :notable -end - -class Article < ActiveRecord::Base - belongs_to :person - has_many :comments - has_and_belongs_to_many :tags - has_many :notes, as: :notable -end - -class Comment < ActiveRecord::Base - belongs_to :article - belongs_to :person -end - -class Tag < ActiveRecord::Base - has_and_belongs_to_many :articles -end - -class Note < ActiveRecord::Base - belongs_to :notable, polymorphic: true -end - -module Schema - def self.create - ActiveRecord::Migration.verbose = false - - ActiveRecord::Schema.define do - create_table :people, force: true do |t| - t.integer :parent_id - t.string :name - t.integer :salary - t.boolean :awesome, default: false - t.timestamps null: false - end - - create_table :articles, force: true do |t| - t.integer :person_id - t.string :title - t.text :body - end - - create_table :comments, force: true do |t| - t.integer :article_id - t.integer :person_id - t.text :body - end - - create_table :tags, force: true do |t| - t.string :name - end - - create_table :articles_tags, force: true, id: false do |t| - t.integer :article_id - t.integer :tag_id - end - - create_table :notes, force: true do |t| - t.integer :notable_id - t.string :notable_type - t.string :note - end - end - - 10.times do - person = Person.make - Note.make(notable: person) - 3.times do - article = Article.make(person: person) - 3.times do - article.tags = [Tag.make, Tag.make, Tag.make] - end - Note.make(notable: article) - 10.times do - Comment.make(article: article) - end - end - 2.times do - Comment.make(person: person) - end - end - - Comment.make( - body: 'First post!', article: Article.make(title: 'Hello, world!') - ) - end -end From 8141318e93a2a833cfa673ee5dd0ea1ce68e50fa Mon Sep 17 00:00:00 2001 From: Greg Molnar Date: Tue, 12 Feb 2019 20:45:48 +0100 Subject: [PATCH 2/5] fix polymorphic joins with rails > 5.2 --- lib/ransack/adapters/active_record/context.rb | 6 +----- polyamorous/lib/polyamorous.rb | 5 +++++ .../activerecord_5.1_ruby_2/join_association.rb | 3 +-- .../activerecord_5.2.0_ruby_2/join_association.rb | 5 ++--- .../activerecord_5.2.0_ruby_2/join_dependency.rb | 1 - .../activerecord_5.2.0_ruby_2/reflection.rb | 12 ++++++++++++ .../activerecord_5.2.1_ruby_2/join_association.rb | 9 --------- .../activerecord_5.2.1_ruby_2/join_dependency.rb | 10 +++++++++- .../activerecord_5.2.1_ruby_2/reflection.rb | 2 ++ ransack.gemspec | 1 + spec/spec_helper.rb | 1 + 11 files changed, 34 insertions(+), 21 deletions(-) create mode 100644 polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/reflection.rb create mode 100644 polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/reflection.rb diff --git a/lib/ransack/adapters/active_record/context.rb b/lib/ransack/adapters/active_record/context.rb index 122c981..942aebb 100644 --- a/lib/ransack/adapters/active_record/context.rb +++ b/lib/ransack/adapters/active_record/context.rb @@ -326,7 +326,6 @@ module Ransack found_association = jd.instance_variable_get(:@join_root).children.last end - @associations_pot[found_association] = parent # TODO maybe we dont need to push associations here, we could loop @@ -337,14 +336,11 @@ module Ransack if ::ActiveRecord::VERSION::STRING > Constants::RAILS_5_2_0 @join_dependency.send(:construct_tables!, jd.instance_variable_get(:@join_root)) else - @join_dependency.send( - :construct_tables!, jd.instance_variable_get(:@join_root), found_association - ) + @join_dependency.send(:construct_tables!, jd.instance_variable_get(:@join_root), found_association) end # Leverage the stashed association functionality in AR @object = @object.joins(jd) - found_association end diff --git a/polyamorous/lib/polyamorous.rb b/polyamorous/lib/polyamorous.rb index f35a696..6c97dd7 100644 --- a/polyamorous/lib/polyamorous.rb +++ b/polyamorous/lib/polyamorous.rb @@ -19,6 +19,11 @@ if defined?(::ActiveRecord) require "polyamorous/activerecord_#{ar_version}_ruby_2/#{file}" end + if ar_version >= "5.2.0" + require "polyamorous/activerecord_#{ar_version}_ruby_2/reflection.rb" + ::ActiveRecord::Reflection::AbstractReflection.send(:prepend, Polyamorous::ReflectionExtensions) + end + Polyamorous::JoinDependency.send(:prepend, Polyamorous::JoinDependencyExtensions) Polyamorous::JoinDependency.singleton_class.send(:prepend, Polyamorous::JoinDependencyExtensions::ClassMethods) Polyamorous::JoinAssociation.send(:prepend, Polyamorous::JoinAssociationExtensions) diff --git a/polyamorous/lib/polyamorous/activerecord_5.1_ruby_2/join_association.rb b/polyamorous/lib/polyamorous/activerecord_5.1_ruby_2/join_association.rb index 9a0b1f3..7e9c24e 100644 --- a/polyamorous/lib/polyamorous/activerecord_5.1_ruby_2/join_association.rb +++ b/polyamorous/lib/polyamorous/activerecord_5.1_ruby_2/join_association.rb @@ -7,8 +7,7 @@ module Polyamorous base.class_eval { attr_reader :join_type } end - def initialize(reflection, children, polymorphic_class = nil, - join_type = Arel::Nodes::InnerJoin) + def initialize(reflection, children, polymorphic_class = nil, join_type = Arel::Nodes::InnerJoin) @join_type = join_type if polymorphic_class && ::ActiveRecord::Base > polymorphic_class swapping_reflection_klass(reflection, polymorphic_class) do |reflection| diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_association.rb b/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_association.rb index b9be34c..20cf2a4 100644 --- a/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_association.rb +++ b/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_association.rb @@ -7,8 +7,7 @@ module Polyamorous base.class_eval { attr_reader :join_type } end - def initialize(reflection, children, alias_tracker, polymorphic_class = nil, - join_type = Arel::Nodes::InnerJoin) + def initialize(reflection, children, alias_tracker, polymorphic_class = nil, join_type = Arel::Nodes::InnerJoin) @join_type = join_type if polymorphic_class && ::ActiveRecord::Base > polymorphic_class swapping_reflection_klass(reflection, polymorphic_class) do |reflection| @@ -23,7 +22,7 @@ module Polyamorous def build_constraint(klass, table, key, foreign_table, foreign_key) if reflection.polymorphic? super(klass, table, key, foreign_table, foreign_key) - .and(foreign_table[reflection.foreign_type].eq(reflection.klass.name)) + .and(foreign_table[reflection.foreign_type].eq(reflection.klass.name)) else super(klass, table, key, foreign_table, foreign_key) end diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb b/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb index 0603daf..2d840db 100644 --- a/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb +++ b/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb @@ -38,7 +38,6 @@ module Polyamorous # passing an additional argument, `join_type`, to #join_constraints. # def join_constraints(outer_joins, join_type) - @alias_tracker = alias_tracker joins = join_root.children.flat_map { |child| if join_type == Arel::Nodes::OuterJoin make_polyamorous_left_outer_joins join_root, child diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/reflection.rb b/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/reflection.rb new file mode 100644 index 0000000..b9b8c32 --- /dev/null +++ b/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/reflection.rb @@ -0,0 +1,12 @@ +module Polyamorous + module ReflectionExtensions + def build_join_constraint(table, foreign_table) + if polymorphic? + super(table, foreign_table) + .and(foreign_table[foreign_type].eq(klass.name)) + else + super(table, foreign_table) + end + end + end +end diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_association.rb b/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_association.rb index 97f7068..8b39760 100644 --- a/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_association.rb +++ b/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_association.rb @@ -18,14 +18,5 @@ module Polyamorous super(reflection, children) end end - - def build_constraint(klass, table, key, foreign_table, foreign_key) - if reflection.polymorphic? - super(klass, table, key, foreign_table, foreign_key) - .and(foreign_table[reflection.foreign_type].eq(reflection.klass.name)) - else - super(klass, table, key, foreign_table, foreign_key) - end - end end end diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb b/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb index edbd257..41d3b5b 100644 --- a/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb +++ b/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb @@ -3,7 +3,6 @@ module Polyamorous module JoinDependencyExtensions # Replaces ActiveRecord::Associations::JoinDependency#build - # def build(associations, base_klass) associations.map do |name, right| if name.is_a? Join @@ -30,6 +29,15 @@ module Polyamorous end end + private + def make_constraints(parent, child, join_type = Arel::Nodes::OuterJoin) + foreign_table = parent.table + foreign_klass = parent.base_klass + join_type = child.join_type || join_type if join_type == Arel::Nodes::InnerJoin + joins = child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) + joins.concat child.children.flat_map { |c| make_constraints(child, c, join_type) } + end + module ClassMethods # Prepended before ActiveRecord::Associations::JoinDependency#walk_tree # diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/reflection.rb b/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/reflection.rb new file mode 100644 index 0000000..e66bf90 --- /dev/null +++ b/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/reflection.rb @@ -0,0 +1,2 @@ +# active_record_5.2.1_ruby_2/reflection.rb +require 'polyamorous/activerecord_5.2.0_ruby_2/reflection' diff --git a/ransack.gemspec b/ransack.gemspec index 9d2e4d6..6eb29c0 100644 --- a/ransack.gemspec +++ b/ransack.gemspec @@ -28,6 +28,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'pg', '~> 0.21' s.add_development_dependency 'mysql2', '0.3.20' s.add_development_dependency 'pry', '0.10' + s.add_development_dependency 'byebug' s.files = `git ls-files`.split("\n") diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e1f57c1..f9c5d78 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,6 +4,7 @@ require 'faker' require 'ransack' require 'pry' require 'simplecov' +require 'byebug' SimpleCov.start I18n.enforce_available_locales = false From 99cbc190ad39134f1fa77a395a821da3130d5912 Mon Sep 17 00:00:00 2001 From: Greg Molnar Date: Sun, 14 Apr 2019 16:30:07 +0200 Subject: [PATCH 3/5] use newer mysql and make 5-2-stable the default --- .gitignore | 1 + Gemfile | 12 ++---------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 4fd2280..c96f037 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ Gemfile.lock pkg/* coverage/* .DS_Store +.byebug_history diff --git a/Gemfile b/Gemfile index c48f686..85f4497 100644 --- a/Gemfile +++ b/Gemfile @@ -3,7 +3,7 @@ gemspec gem 'rake' -rails = ENV['RAILS'] || '5-0-stable' +rails = ENV['RAILS'] || '5-2-stable' gem 'pry' @@ -22,9 +22,6 @@ when /^v/ # A tagged version gem 'activerecord', require: false gem 'actionpack' end - if rails >= 'v5.2.0' - gem 'mysql2', '~> 0.4.4' - end else git 'git://github.com/rails/rails.git', :branch => rails do gem 'activesupport' @@ -32,13 +29,8 @@ else gem 'activerecord', require: false gem 'actionpack' end - if rails == '3-0-stable' - gem 'mysql2', '< 0.3' - end - if rails == '5-2-stable' - gem 'mysql2', '~> 0.4.4' - end end +gem 'mysql2', '~> 0.4.4' group :test do # TestUnit was removed from Ruby 2.2 but still needed for testing Rails 3.x. From dbd3eab06dd22315853c778e7ad1cb2810000629 Mon Sep 17 00:00:00 2001 From: Greg Molnar Date: Sun, 14 Apr 2019 17:17:56 +0200 Subject: [PATCH 4/5] fix join dependency spec --- spec/ransack/join_dependency_spec.rb | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/spec/ransack/join_dependency_spec.rb b/spec/ransack/join_dependency_spec.rb index 4c73871..3527859 100644 --- a/spec/ransack/join_dependency_spec.rb +++ b/spec/ransack/join_dependency_spec.rb @@ -8,8 +8,8 @@ module Polyamorous specify { expect(subject.send(:join_root).drop(1).size) .to eq(2) } - specify { expect(subject.send(:join_root).drop(1).map(&:join_type)) - .to be_all { Polyamorous::InnerJoin } } + specify { expect(subject.send(:join_root).drop(1).map(&:join_type).uniq) + .to eq [Polyamorous::InnerJoin] } end context 'with has_many :through association' do @@ -38,8 +38,8 @@ module Polyamorous .to eq 2 } specify { expect(subject.send(:join_root).drop(1).map(&:join_type)) .to eq [Polyamorous::OuterJoin, Polyamorous::OuterJoin] } - specify { expect(subject.send(:join_root).drop(1).map(&:join_type)) - .to be_all { Polyamorous::OuterJoin } } + specify { expect(subject.send(:join_root).drop(1).map(&:join_type).uniq) + .to eq [Polyamorous::OuterJoin] } end context 'with polymorphic belongs_to join' do @@ -59,8 +59,19 @@ module Polyamorous specify { expect(subject.send(:join_root).drop(1).size) .to eq 2 } - specify { expect(subject.send(:join_root).drop(1).map(&:join_type)) - .to be_all { Polyamorous::InnerJoin } } + specify { expect(subject.send(:join_root).drop(1).map(&:join_type).uniq) + .to eq [Polyamorous::InnerJoin] } + specify { expect(subject.send(:join_root).drop(1).first.table_name) + .to eq 'people' } + specify { expect(subject.send(:join_root).drop(1)[1].table_name) + .to eq 'comments' } + end + + context 'with polymorphic belongs_to join and nested join' do + subject { new_join_dependency Note, + new_join(:notable, :outer, Person) => :comments } + specify { expect(subject.send(:join_root).drop(1).size).to eq 2 } + specify { expect(subject.send(:join_root).drop(1).map(&:join_type)).to eq [Polyamorous::OuterJoin, Polyamorous::InnerJoin] } specify { expect(subject.send(:join_root).drop(1).first.table_name) .to eq 'people' } specify { expect(subject.send(:join_root).drop(1)[1].table_name) From 0d736a1186b9e2f34a00bc8667512ed5c6b0328d Mon Sep 17 00:00:00 2001 From: hiichan Date: Thu, 21 Feb 2019 20:24:18 +0900 Subject: [PATCH 5/5] Fix wrong table alias when using nested join --- CHANGELOG.md | 5 ++++ lib/ransack/adapters/active_record/context.rb | 4 +-- .../join_dependency.rb | 2 +- .../join_dependency.rb | 16 +++++++++++ spec/ransack/search_spec.rb | 28 +++++++++++++++++-- 5 files changed, 50 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9174df..3a16676 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +* Fix wrong table alias when using nested join. for ActiveRecord >= 5.2 + PR [374](https://github.com/activerecord-hackery/ransack/pull/374) + + *hiichan* + ## Version 2.1.1 - 2018-12-05 * Add `arabic` translation diff --git a/lib/ransack/adapters/active_record/context.rb b/lib/ransack/adapters/active_record/context.rb index 942aebb..a01d284 100644 --- a/lib/ransack/adapters/active_record/context.rb +++ b/lib/ransack/adapters/active_record/context.rb @@ -312,7 +312,7 @@ module Ransack alias_tracker = ::ActiveRecord::Associations::AliasTracker.create(self.klass.connection, parent.table.name, []) jd = Polyamorous::JoinDependency.new( parent.base_klass, - parent.base_klass.arel_table, + parent.table, Polyamorous::Join.new(name, @join_type, klass), alias_tracker ) @@ -320,7 +320,7 @@ module Ransack else jd = Polyamorous::JoinDependency.new( parent.base_klass, - parent.base_klass.arel_table, + parent.table, Polyamorous::Join.new(name, @join_type, klass), ) found_association = jd.instance_variable_get(:@join_root).children.last diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb b/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb index 2d840db..1e76107 100644 --- a/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb +++ b/polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/join_dependency.rb @@ -47,7 +47,7 @@ module Polyamorous } joins.concat outer_joins.flat_map { |oj| - if join_root.match? oj.join_root + if join_root.match?(oj.join_root) && join_root.table.name == oj.join_root.table.name walk(join_root, oj.join_root) else oj.join_root.children.flat_map { |child| diff --git a/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb b/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb index 41d3b5b..ca7de56 100644 --- a/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb +++ b/polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb @@ -29,6 +29,22 @@ module Polyamorous end end + def join_constraints(joins_to_add, join_type, alias_tracker) + @alias_tracker = alias_tracker + + construct_tables!(join_root) + joins = make_join_constraints(join_root, join_type) + + joins.concat joins_to_add.flat_map { |oj| + construct_tables!(oj.join_root) + if join_root.match?(oj.join_root) && join_root.table.name == oj.join_root.table.name + walk join_root, oj.join_root + else + make_join_constraints(oj.join_root, join_type) + end + } + end + private def make_constraints(parent, child, join_type = Arel::Nodes::OuterJoin) foreign_table = parent.table diff --git a/spec/ransack/search_spec.rb b/spec/ransack/search_spec.rb index afadcda..b4d0b3b 100644 --- a/spec/ransack/search_spec.rb +++ b/spec/ransack/search_spec.rb @@ -227,13 +227,37 @@ module Ransack children_people_name_field} = 'Ernie'/ end + it 'use appropriate table alias' do + skip "Make this spec pass for Rails <5.2" if ::ActiveRecord::VERSION::STRING < '5.2.0' + s = Search.new(Person, { + name_eq: "person_name_query", + articles_title_eq: "person_article_title_query", + parent_name_eq: "parent_name_query", + parent_articles_title_eq: 'parents_article_title_query' + }).result + real_query = remove_quotes_and_backticks(s.to_sql) + + expect(real_query) + .to include "LEFT OUTER JOIN articles ON articles.person_id = people.id" + expect(real_query) + .to include "LEFT OUTER JOIN articles articles_people ON articles_people.person_id = parents_people.id" + expect(real_query) + .to include "people.name = 'person_name_query'" + expect(real_query) + .to include "articles.title = 'person_article_title_query'" + expect(real_query) + .to include "parents_people.name = 'parent_name_query'" + expect(real_query) + .to include "articles_people.title = 'parents_article_title_query'" + end + # FIXME: Make this spec pass for Rails 4.1 / 4.2 / 5.0 and not just 4.0 by # commenting out lines 221 and 242 to run the test. Addresses issue #374. # https://github.com/activerecord-hackery/ransack/issues/374 # it 'evaluates conditions for multiple `belongs_to` associations to the same table contextually' do - skip "Make this spec pass for Rails >5.0" + skip "Make this spec pass for Rails <5.2" if ::ActiveRecord::VERSION::STRING < '5.2.0' s = Search.new( Recommendation, person_name_eq: 'Ernie', @@ -248,7 +272,7 @@ module Ransack ON target_people_recommendations.id = recommendations.target_person_id LEFT OUTER JOIN people parents_people ON parents_people.id = target_people_recommendations.parent_id - WHERE ((people.name = 'Ernie' AND parents_people.name = 'Test')) + WHERE (people.name = 'Ernie' AND parents_people.name = 'Test') SQL .squish expect(real_query).to eq expected_query