diff --git a/README.rdoc b/README.rdoc index 7881168..4c25e54 100644 --- a/README.rdoc +++ b/README.rdoc @@ -322,6 +322,10 @@ your controller. The other required parameter is the attribute name itself. Opti you can provide a string as a 3rd parameter to override the default link name, and then additional hashed for the +options+ and +html_options+ hashes for link_to. +By default, the link that is created will sort by the given column in ascending order when first clicked. If you'd like to reverse this (so the first click sorts the results in descending order), you can pass +:default_order => :desc+ in the options hash, like so: + + <%= sort_link @search, :ratings, "Highest Rated", :default_order => :desc %> + You can sort by more than one column as well, by creating a link like: <%= sort_link :name_and_salary %> diff --git a/Rakefile b/Rakefile index 5714ec5..18a605a 100644 --- a/Rakefile +++ b/Rakefile @@ -15,10 +15,9 @@ begin gem.homepage = "http://metautonomo.us/projects/metasearch/" gem.authors = ["Ernie Miller"] gem.add_development_dependency "shoulda" - gem.add_dependency "activerecord", "~> 3.0.2" - gem.add_dependency "activesupport", "~> 3.0.2" - gem.add_dependency "actionpack", "~> 3.0.2" - gem.add_dependency "arel", "~> 2.0.2" + gem.add_dependency "activerecord", "~> 3.1.0.alpha" + gem.add_dependency "activesupport", "~> 3.1.0.alpha" + gem.add_dependency "actionpack", "~> 3.1.0.alpha" gem.post_install_message = < STRINGS, :predicate => :matches, :formatter => '"%#{param}%"'}], ['does_not_contain', 'nlike', 'not_matches', {:types => STRINGS, :predicate => :does_not_match, :formatter => '"%#{param}%"'}], ['starts_with', 'sw', {:types => STRINGS, :predicate => :matches, :formatter => '"#{param}%"'}], - ['does_not_start_with', 'dnsw', {:types => STRINGS, :predicate => :does_not_match, :formatter => '"%#{param}%"'}], + ['does_not_start_with', 'dnsw', {:types => STRINGS, :predicate => :does_not_match, :formatter => '"#{param}%"'}], ['ends_with', 'ew', {:types => STRINGS, :predicate => :matches, :formatter => '"%#{param}"'}], ['does_not_end_with', 'dnew', {:types => STRINGS, :predicate => :does_not_match, :formatter => '"%#{param}"'}], ['greater_than', 'gt', {:types => (NUMBERS + DATES + TIMES), :predicate => :gt}], @@ -53,7 +53,7 @@ module MetaSearch I18n.load_path += Dir[File.join(File.dirname(__FILE__), 'meta_search', 'locale', '*.yml')] -ActiveRecord::Associations::ClassMethods::JoinDependency.send(:include, MetaSearch::JoinDependency) +ActiveRecord::Associations::JoinDependency.send(:include, MetaSearch::JoinDependency) ActiveRecord::Base.send(:include, MetaSearch::Searches::ActiveRecord) ActionView::Helpers::FormBuilder.send(:include, MetaSearch::Helpers::FormBuilder) ActionController::Base.helper(MetaSearch::Helpers::UrlHelper) diff --git a/lib/meta_search/builder.rb b/lib/meta_search/builder.rb index 62df551..5affa08 100644 --- a/lib/meta_search/builder.rb +++ b/lib/meta_search/builder.rb @@ -35,7 +35,7 @@ def initialize(base_or_relation, opts = {}) @options = opts # Let's just hang on to other options for use in authorization blocks @join_type = opts[:join_type] || Arel::Nodes::OuterJoin @join_type = get_join_type(@join_type) - @join_dependency = build_join_dependency + @join_dependency = build_join_dependency(@relation) @search_attributes = {} @errors = ActiveModel::Errors.new(self) end @@ -51,12 +51,7 @@ def get_association(assoc, base = @base) def get_attribute(name, parent = @join_dependency.join_base) attribute = nil if get_column(name, parent.active_record) - if parent.is_a?(ActiveRecord::Associations::ClassMethods::JoinDependency::JoinAssociation) - relation = parent.relation.is_a?(Array) ? parent.relation.last : parent.relation - attribute = relation[name] - else - attribute = @relation.arel_table[name] - end + attribute = parent.table[name] elsif (segments = name.to_s.split(/_/)).size > 1 remainder = [] found_assoc = nil @@ -252,53 +247,58 @@ def type_from_association_segments(segments, base, depth) type end - def build_or_find_association(association, parent = @join_dependency.join_base, klass = nil) + def build_or_find_association(name, parent = @join_dependency.join_base, klass = nil) found_association = @join_dependency.join_associations.detect do |assoc| - assoc.reflection.name == association.to_sym && - assoc.reflection.klass == klass && - assoc.parent == parent + assoc.reflection.name == name && + assoc.parent == parent && + (!klass || assoc.reflection.klass == klass) end unless found_association - @join_dependency.send(:build_with_metasearch, association, parent, @join_type, klass) + @join_dependency.send(:build_polymorphic, name.to_sym, parent, @join_type, klass) found_association = @join_dependency.join_associations.last + # Leverage the stashed association functionality in AR @relation = @relation.joins(found_association) end + found_association end - def build_join_dependency - joins = @relation.joins_values.map {|j| j.respond_to?(:strip) ? j.strip : j}.uniq - - association_joins = joins.select do |j| - [Hash, Array, Symbol].include?(j.class) && !array_of_strings?(j) - end - - stashed_association_joins = joins.select do |j| - j.is_a?(ActiveRecord::Associations::ClassMethods::JoinDependency::JoinAssociation) + def build_join_dependency(relation) + buckets = relation.joins_values.group_by do |join| + case join + when String + 'string_join' + when Hash, Symbol, Array + 'association_join' + when ::ActiveRecord::Associations::JoinDependency::JoinAssociation + 'stashed_join' + when Arel::Nodes::Join + 'join_node' + else + raise 'unknown class: %s' % join.class.name + end end - non_association_joins = (joins - association_joins - stashed_association_joins) - custom_joins = custom_join_sql(*non_association_joins) + association_joins = buckets['association_join'] || [] + stashed_association_joins = buckets['stashed_join'] || [] + join_nodes = buckets['join_node'] || [] + string_joins = (buckets['string_join'] || []).map { |x| + x.strip + }.uniq - ActiveRecord::Associations::ClassMethods::JoinDependency.new(@base, association_joins, custom_joins) - end + join_list = relation.send :custom_join_ast, relation.table.from(relation.table), string_joins - def custom_join_sql(*joins) - arel = @relation.table - joins.each do |join| - next if join.blank? + join_dependency = ::ActiveRecord::Associations::JoinDependency.new( + relation.klass, + association_joins, + join_list + ) - case join - when Hash, Array, Symbol - if array_of_strings?(join) - join_string = join.join(' ') - arel = arel.join(join_string) - end - else - arel = arel.join(join) - end + join_nodes.each do |join| + join_dependency.alias_tracker.aliased_name_for(join.left.name.downcase) end - arel.joins(arel) + + join_dependency.graft(*stashed_association_joins) end def get_join_type(opt_join) diff --git a/lib/meta_search/helpers/url_helper.rb b/lib/meta_search/helpers/url_helper.rb index b85cb44..1b3c818 100644 --- a/lib/meta_search/helpers/url_helper.rb +++ b/lib/meta_search/helpers/url_helper.rb @@ -16,14 +16,27 @@ module UrlHelper # <%= sort_link @search, :name, 'Company Name' %> # <%= sort_link @search, :name, :class => 'name_sort' %> # <%= sort_link @search, :name, 'Company Name', :class => 'company_name_sort' %> + # <%= sort_link @search, :name, :default_order => :desc %> + # <%= sort_link @search, :name, 'Company Name', :default_order => :desc %> + # <%= sort_link @search, :name, :class => 'name_sort', :default_order => :desc %> + # <%= sort_link @search, :name, 'Company Name', :class => 'company_name_sort', :default_order => :desc %> + def sort_link(builder, attribute, *args) raise ArgumentError, "Need a MetaSearch::Builder search object as first param!" unless builder.is_a?(MetaSearch::Builder) attr_name = attribute.to_s name = (args.size > 0 && !args.first.is_a?(Hash)) ? args.shift.to_s : builder.base.human_attribute_name(attr_name) prev_attr, prev_order = builder.search_attributes['meta_sort'].to_s.split('.') + + options = args.first.is_a?(Hash) ? args.shift.dup : {} current_order = prev_attr == attr_name ? prev_order : nil - new_order = current_order == 'asc' ? 'desc' : 'asc' - options = args.first.is_a?(Hash) ? args.shift : {} + + if options[:default_order] == :desc + new_order = current_order == 'desc' ? 'asc' : 'desc' + else + new_order = current_order == 'asc' ? 'desc' : 'asc' + end + options.delete(:default_order) + html_options = args.first.is_a?(Hash) ? args.shift : {} css = ['sort_link', current_order].compact.join(' ') html_options[:class] = [css, html_options[:class]].compact.join(' ') @@ -50,4 +63,4 @@ def order_indicator_for(order) end end end -end \ No newline at end of file +end diff --git a/lib/meta_search/join_dependency.rb b/lib/meta_search/join_dependency.rb index 72e6f82..02edfbf 100644 --- a/lib/meta_search/join_dependency.rb +++ b/lib/meta_search/join_dependency.rb @@ -2,90 +2,93 @@ module MetaSearch module JoinDependency + class JoinAssociation < ::ActiveRecord::Associations::JoinDependency::JoinAssociation + + def initialize(reflection, join_dependency, parent = nil, polymorphic_class = nil) + if polymorphic_class && ::ActiveRecord::Base > polymorphic_class + swapping_reflection_klass(reflection, polymorphic_class) do |reflection| + super(reflection, join_dependency, parent) + end + else + super(reflection, join_dependency, parent) + end + end + + def swapping_reflection_klass(reflection, klass) + reflection = reflection.clone + original_polymorphic = reflection.options.delete(:polymorphic) + reflection.instance_variable_set(:@klass, klass) + yield reflection + ensure + reflection.options[:polymorphic] = original_polymorphic + end + + def ==(other) + super && active_record == other.active_record + end + + def build_constraint(reflection, table, key, foreign_table, foreign_key) + if reflection.options[:polymorphic] + super.and( + foreign_table[reflection.foreign_type].eq(reflection.klass.name) + ) + else + super + end + end + + end + + # Yes, I'm using alias_method_chain here. No, I don't feel too + # bad about it. JoinDependency, or, to call it by its full proper + # name, ::ActiveRecord::Associations::JoinDependency, is one of the + # most "for internal use only" chunks of ActiveRecord. def self.included(base) base.class_eval do - alias_method_chain :graft, :metasearch + alias_method_chain :graft, :meta_search end end - def graft_with_metasearch(*associations) + def graft_with_meta_search(*associations) associations.each do |association| join_associations.detect {|a| association == a} || - ( - association.class == MetaSearch::PolymorphicJoinAssociation ? - build_with_metasearch(association.reflection.name, association.find_parent_in(self) || join_base, association.join_type, association.reflection.klass) : - build(association.reflection.name, association.find_parent_in(self) || join_base, association.join_type) - ) + build_polymorphic(association.reflection.name, association.find_parent_in(self) || join_base, association.join_type, association.reflection.klass) end self end - protected + # Should only be called by MetaSearch, and only with a single association name + def build_polymorphic(association, parent = nil, join_type = Arel::OuterJoin, klass = nil) + parent ||= join_parts.last + reflection = parent.reflections[association] or + raise ::ActiveRecord::ConfigurationError, "Association named '#{ association }' was not found; perhaps you misspelled it?" + unless join_association = find_join_association_respecting_polymorphism(reflection, parent, klass) + @reflections << reflection + join_association = build_join_association_respecting_polymorphism(reflection, parent, klass) + join_association.join_type = join_type + @join_parts << join_association + cache_joined_association(join_association) + end - def build_with_metasearch(associations, parent = nil, join_type = Arel::Nodes::InnerJoin, polymorphic_class = nil) - parent ||= @joins.last - case associations - when Symbol, String - reflection = parent.reflections[associations.to_s.intern] or - raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?" - unless (association = find_join_association(reflection, parent)) && (!polymorphic_class || association.active_record == polymorphic_class) - @reflections << reflection - if reflection.options[:polymorphic] - raise ArgumentError, "You can't create a polymorphic belongs_to join without specifying the polymorphic class!" unless polymorphic_class - association = PolymorphicJoinAssociation.new(reflection, self, polymorphic_class, parent) - else - association = build_join_association(reflection, parent) - end - association.join_type = join_type - @joins << association - end + join_association + end + + def find_join_association_respecting_polymorphism(reflection, parent, klass) + if association = find_join_association(reflection, parent) + unless reflection.options[:polymorphic] association else - build(associations, parent, join_type) + association if association.active_record == klass end end - end - - class PolymorphicJoinAssociation < ActiveRecord::Associations::ClassMethods::JoinDependency::JoinAssociation - - def initialize(reflection, join_dependency, polymorphic_class, parent = nil) - reflection.check_validity! - @active_record = polymorphic_class - @cached_record = {} - @join_dependency = join_dependency - @parent = parent || join_dependency.join_base - @reflection = reflection.clone - @reflection.instance_variable_set(:"@klass", polymorphic_class) - @aliased_prefix = "t#{ join_dependency.joins.size }" - @parent_table_name = @parent.active_record.table_name - @aliased_table_name = aliased_table_name_for(table_name) - @join = nil - @join_type = Arel::Nodes::InnerJoin - end - - def ==(other) - other.class == self.class && - other.reflection == reflection && - other.active_record == active_record && - other.parent == parent end - def association_join - return @join if @join - - aliased_table = Arel::Table.new(table_name, :as => @aliased_table_name, :engine => arel_engine) - parent_table = Arel::Table.new(parent.table_name, :as => parent.aliased_table_name, :engine => arel_engine) - - @join = [ - aliased_table[options[:primary_key] || reflection.klass.primary_key].eq(parent_table[options[:foreign_key] || reflection.primary_key_name]), - parent_table[options[:foreign_type]].eq(active_record.name) - ] - - if options[:conditions] - @join << interpolate_sql(sanitize_sql(options[:conditions], aliased_table_name)) + def build_join_association_respecting_polymorphism(reflection, parent, klass = nil) + if reflection.options[:polymorphic] && klass + JoinAssociation.new(reflection, self, parent, klass) + else + JoinAssociation.new(reflection, self, parent) end - - @join end end end \ No newline at end of file diff --git a/meta_search.gemspec b/meta_search.gemspec index 1b9b268..4160593 100644 --- a/meta_search.gemspec +++ b/meta_search.gemspec @@ -5,11 +5,11 @@ Gem::Specification.new do |s| s.name = %q{meta_search} - s.version = "1.0.5" + s.version = "1.1.0.pre2" - s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version= + s.required_rubygems_version = Gem::Requirement.new("> 1.3.1") if s.respond_to? :required_rubygems_version= s.authors = [%q{Ernie Miller}] - s.date = %q{2011-05-05} + s.date = %q{2011-07-07} s.description = %q{ Allows simple search forms to be created against an AR3 model and its associations, has useful view helpers for sort links @@ -71,7 +71,7 @@ you're feeling especially appreciative. It'd help me justify this } s.require_paths = [%q{lib}] - s.rubygems_version = %q{1.8.0} + s.rubygems_version = %q{1.8.5} s.summary = %q{Object-based searching (and more) for simply creating search forms.} if s.respond_to? :specification_version then @@ -79,23 +79,20 @@ you're feeling especially appreciative. It'd help me justify this if Gem::Version.new(Gem::VERSION) >= Gem::Version.new('1.2.0') then s.add_development_dependency(%q, [">= 0"]) - s.add_runtime_dependency(%q, ["~> 3.0.2"]) - s.add_runtime_dependency(%q, ["~> 3.0.2"]) - s.add_runtime_dependency(%q, ["~> 3.0.2"]) - s.add_runtime_dependency(%q, ["~> 2.0.2"]) + s.add_runtime_dependency(%q, ["~> 3.1.0.alpha"]) + s.add_runtime_dependency(%q, ["~> 3.1.0.alpha"]) + s.add_runtime_dependency(%q, ["~> 3.1.0.alpha"]) else s.add_dependency(%q, [">= 0"]) - s.add_dependency(%q, ["~> 3.0.2"]) - s.add_dependency(%q, ["~> 3.0.2"]) - s.add_dependency(%q, ["~> 3.0.2"]) - s.add_dependency(%q, ["~> 2.0.2"]) + s.add_dependency(%q, ["~> 3.1.0.alpha"]) + s.add_dependency(%q, ["~> 3.1.0.alpha"]) + s.add_dependency(%q, ["~> 3.1.0.alpha"]) end else s.add_dependency(%q, [">= 0"]) - s.add_dependency(%q, ["~> 3.0.2"]) - s.add_dependency(%q, ["~> 3.0.2"]) - s.add_dependency(%q, ["~> 3.0.2"]) - s.add_dependency(%q, ["~> 2.0.2"]) + s.add_dependency(%q, ["~> 3.1.0.alpha"]) + s.add_dependency(%q, ["~> 3.1.0.alpha"]) + s.add_dependency(%q, ["~> 3.1.0.alpha"]) end end diff --git a/test/fixtures/data_types.yml b/test/fixtures/data_types.yml index 1b8d6e1..e8f81f3 100644 --- a/test/fixtures/data_types.yml +++ b/test/fixtures/data_types.yml @@ -6,9 +6,9 @@ dt_<%= n %>: int : <%= n ** 3 %> flt : <%= n.to_f / 2.0 %> dec : <%= n.to_f ** (n + 0.1) %> - dtm : <%= (Time.utc(2009, 12, 24) + 86400 * n).to_s(:db) %> - tms : <%= (Time.utc(2009, 12, 24) + 86400 * n).to_s(:db) %> - tim : <%= Time.utc(2000, 01, 01, n+8, n).to_s(:db) %> + dtm : <%= (Time.local(2009, 12, 24) + 86400 * n).in_time_zone.to_s(:db) %> + tms : <%= (Time.local(2009, 12, 24) + 86400 * n).in_time_zone.to_s(:db) %> + tim : <%= Time.local(2000, 01, 01, n+8, n).in_time_zone.to_s(:db) %> dat : <%= (Date.new(2009, 12, 24) + n).strftime("%Y-%m-%d") %> bin : <%= "BLOB#{n}" * n %> bln : <%= n % 2 > 0 ? true : false %> diff --git a/test/helper.rb b/test/helper.rb index f53385f..4c42f10 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -1,6 +1,7 @@ require 'rubygems' require 'test/unit' require 'shoulda' +require 'active_support/time' require 'active_record' require 'active_record/fixtures' require 'action_view' @@ -23,7 +24,7 @@ load File.join(FIXTURES_PATH, 'schema.rb') end -Fixtures.create_fixtures(FIXTURES_PATH, ActiveRecord::Base.connection.tables) +ActiveRecord::Fixtures.create_fixtures(FIXTURES_PATH, ActiveRecord::Base.connection.tables) I18n.load_path += Dir[File.join(File.dirname(__FILE__), 'locales', '*.yml')] diff --git a/test/test_view_helpers.rb b/test/test_view_helpers.rb index 8596f71..b9a1d72 100644 --- a/test/test_view_helpers.rb +++ b/test/test_view_helpers.rb @@ -6,18 +6,34 @@ class TestViewHelpers < ActionView::TestCase tests MetaSearch::Helpers::FormHelper include MetaSearch::Helpers::UrlHelper - router = ActionDispatch::Routing::RouteSet.new - router.draw do - resources :developers - resources :companies - resources :projects - resources :notes - match ':controller(/:action(/:id(.:format)))' + def self.router + @router ||= begin + router = ActionDispatch::Routing::RouteSet.new + router.draw do + resources :developers + resources :companies + resources :projects + resources :notes + match ':controller(/:action(/:id(.:format)))' + end + router + end end + include router.url_helpers + # FIXME: figure out a cleaner way to get this behavior def setup + router = self.class.router @controller = ActionView::TestCase::TestController.new + @controller.instance_variable_set(:@_routes, router) + @controller.class_eval do + include router.url_helpers + end + + @controller.view_context_class.class_eval do + include router.url_helpers + end end context "A search against Company and a search against Developer" do @@ -305,6 +321,21 @@ def setup @s = Company.search end + should "generate a sort link for descending order if set as the default order" do + assert_match /name.desc/, + sort_link(@s, :name, :controller => 'companies', :default_order => :desc) + end + + should "generate a sort link for ascending order if set as the default order" do + assert_match /name.asc/, + sort_link(@s, :name, :controller => 'companies', :default_order => :asc) + end + + should "generate a sort link for ascending order if default is specified incorectly" do + assert_match /name.asc/, + sort_link(@s, :name, :controller => 'companies', :default_order => :something_else) + end + context "sorted by name ascending" do setup do @s.meta_sort = 'name.asc' @@ -320,17 +351,53 @@ def setup sort_link(@s, :created_at, :controller => 'companies') end + should "generate a sort link for descending order if ascending order is the default" do + assert_match /name.desc/, + sort_link(@s, :name, :controller => 'companies', :default_order => :asc) + end + + should "generate a sort link for descending order if descending order is the default" do + assert_match /name.desc/, + sort_link(@s, :name, :controller => 'companies', :default_order => :desc) + end + + should "generate a sort link for descending order if default is specified incorrectly" do + assert_match /name.desc/, + sort_link(@s, :name, :controller => 'companies', :default_order => :something_else) + end + context "with existing search options" do setup do @s.name_contains = 'a' end should "maintain previous search options in its sort links" do - assert_match /search\[name_contains\]=a/, + assert_match /search%5Bname_contains%5D=a/, sort_link(@s, :name, :controller => 'companies') end end end + + context "sorted by name descending" do + setup do + @s.meta_sort = 'name.desc' + end + + should "generate a sort link for ascending order if descending order is the default" do + assert_match /name.asc/, + sort_link(@s, :name, :controller => 'companies', :default_order => :desc) + end + + should "generate a sort link for ascending order if ascending order is the default" do + assert_match /name.asc/, + sort_link(@s, :name, :controller => 'companies', :default_order => :asc) + end + + should "generate a sort link for ascending order if default is specified incorrectly" do + assert_match /name.asc/, + sort_link(@s, :name, :controller => 'companies', :default_order => :desc) + end + end end context "A developer search" do @@ -359,10 +426,22 @@ def setup end should "maintain previous search options in its sort links" do - assert_match /search\[name_contains\]=a/, + assert_match /search%5Bname_contains%5D=a/, sort_link(@s, :company_name, :controller => 'companies') end end end end -end \ No newline at end of file + + context "Any search" do + setup do + @s = Company.search(:name_contains => 'foo') + end + + should "not modify passed-in parameters" do + params = { :controller => 'companies' } + sort_link(@s, :name, params) + assert_equal ({ :controller => 'companies' }), params + end + end +end diff --git a/vendor/arel b/vendor/arel index c0b1f9e..a6d5f82 160000 --- a/vendor/arel +++ b/vendor/arel @@ -1 +1 @@ -Subproject commit c0b1f9e40550f396dd69d4f094670b9c0e076149 +Subproject commit a6d5f823a5d74c6884adfc88c30ad05624c4d674 diff --git a/vendor/rails b/vendor/rails index 1081ea6..e9f2c67 160000 --- a/vendor/rails +++ b/vendor/rails @@ -1 +1 @@ -Subproject commit 1081ea66a3d864f310a1a55f22229af4e10436a7 +Subproject commit e9f2c67874a8b9a2f1fd4df068f8640bb266f139