From e55e99fe89ceb518a05852783444ef91601cc665 Mon Sep 17 00:00:00 2001 From: Steve Richert Date: Fri, 26 Aug 2016 13:48:59 -0400 Subject: [PATCH] Use class attributes properly so that inheritance is respected This fixes two bugs: 1. Subclasses were not properly inheriting their parents' Ransack aliases in Mongoid because each class defined its very own set of aliases. 2. In Active Record, Ransack aliases were defined in such a way that the parent's (and grandparent's, etc.) aliases were overwritten by the child, meaning that all aliases were ultimately kept on ActiveRecord::Base. This has the unfortunate effect of enforcing uniqueness of Ransack alias names across all models rather than per model. Depending on the load order of models, earlier definitions of an alias in other models would be clobbered. --- lib/ransack/adapters/active_record/base.rb | 3 ++- lib/ransack/adapters/mongoid/base.rb | 23 +++++-------------- spec/mongoid/adapters/mongoid/base_spec.rb | 21 +++++++++++++++++ spec/mongoid/support/schema.rb | 3 +++ .../adapters/active_record/base_spec.rb | 21 +++++++++++++++++ spec/support/schema.rb | 3 +++ 6 files changed, 56 insertions(+), 18 deletions(-) diff --git a/lib/ransack/adapters/active_record/base.rb b/lib/ransack/adapters/active_record/base.rb index c4ca935..6975e80 100644 --- a/lib/ransack/adapters/active_record/base.rb +++ b/lib/ransack/adapters/active_record/base.rb @@ -23,7 +23,8 @@ module Ransack end def ransack_alias(new_name, old_name) - self._ransack_aliases.store(new_name.to_s, old_name.to_s) + self._ransack_aliases = _ransack_aliases.merge new_name.to_s => + old_name.to_s end # Ransackable_attributes, by default, returns all column names diff --git a/lib/ransack/adapters/mongoid/base.rb b/lib/ransack/adapters/mongoid/base.rb index a952a7c..50d4353 100644 --- a/lib/ransack/adapters/mongoid/base.rb +++ b/lib/ransack/adapters/mongoid/base.rb @@ -8,6 +8,10 @@ module Ransack extend ActiveSupport::Concern included do + class_attribute :_ransackers + class_attribute :_ransack_aliases + self._ransackers ||= {} + self._ransack_aliases ||= {} end class ColumnWrapper < SimpleDelegator @@ -33,22 +37,6 @@ module Ransack end module ClassMethods - def _ransack_aliases - @_ransack_aliases ||= {} - end - - def _ransack_aliases=(value) - @_ransack_aliases = value - end - - def _ransackers - @_ransackers ||= {} - end - - def _ransackers=(value) - @_ransackers = value - end - def ransack(params = {}, options = {}) params = params.presence || {} Search.new(self, params ? params.delete_if { @@ -58,7 +46,8 @@ module Ransack alias_method :search, :ransack def ransack_alias(new_name, old_name) - self._ransack_aliases.store(new_name.to_s, old_name.to_s) + self._ransack_aliases = _ransack_aliases.merge new_name.to_s => + old_name.to_s end def ransacker(name, opts = {}, &block) diff --git a/spec/mongoid/adapters/mongoid/base_spec.rb b/spec/mongoid/adapters/mongoid/base_spec.rb index 8702960..b2f7a6a 100644 --- a/spec/mongoid/adapters/mongoid/base_spec.rb +++ b/spec/mongoid/adapters/mongoid/base_spec.rb @@ -65,6 +65,27 @@ module Ransack s = Person.ransack(term_cont: 'nomatch') expect(s.result.to_a).to eq [] end + + it 'makes aliases available to subclasses' do + yngwie = Musician.create!(name: 'Yngwie Malmsteen') + + musicians = Musician.ransack(term_cont: 'ngw').result + expect(musicians).to eq([yngwie]) + end + + it 'handles naming collisions gracefully' do + frank = Person.create!(name: 'Frank Stallone') + + people = Person.ransack(term_cont: 'allon').result + expect(people).to eq([frank]) + + Class.new(Article) do + ransack_alias :term, :title + end + + people = Person.ransack(term_cont: 'allon').result + expect(people).to eq([frank]) + end end describe '#ransacker' do diff --git a/spec/mongoid/support/schema.rb b/spec/mongoid/support/schema.rb index 6c15768..5f6f0e9 100644 --- a/spec/mongoid/support/schema.rb +++ b/spec/mongoid/support/schema.rb @@ -63,6 +63,9 @@ class Person end end +class Musician < Person +end + class Article include Mongoid::Document diff --git a/spec/ransack/adapters/active_record/base_spec.rb b/spec/ransack/adapters/active_record/base_spec.rb index 9ad9175..dc6b72e 100644 --- a/spec/ransack/adapters/active_record/base_spec.rb +++ b/spec/ransack/adapters/active_record/base_spec.rb @@ -190,6 +190,27 @@ module Ransack s = Person.ransack(daddy_eq: 'Drake') expect(s.result.to_a).to eq [] end + + it 'makes aliases available to subclasses' do + yngwie = Musician.create!(name: 'Yngwie Malmsteen') + + musicians = Musician.ransack(term_cont: 'ngw').result + expect(musicians).to eq([yngwie]) + end + + it 'handles naming collisions gracefully' do + frank = Person.create!(name: 'Frank Stallone') + + people = Person.ransack(term_cont: 'allon').result + expect(people).to eq([frank]) + + Class.new(Article) do + ransack_alias :term, :title + end + + people = Person.ransack(term_cont: 'allon').result + expect(people).to eq([frank]) + end end describe '#ransacker' do diff --git a/spec/support/schema.rb b/spec/support/schema.rb index 4600303..fe5c9b8 100644 --- a/spec/support/schema.rb +++ b/spec/support/schema.rb @@ -127,6 +127,9 @@ class Person < ActiveRecord::Base end end +class Musician < Person +end + class Article < ActiveRecord::Base belongs_to :person has_many :comments