From 82fbfb86637f337674231209da0d4792c3d19014 Mon Sep 17 00:00:00 2001 From: Rodrigo Souto Date: Fri, 23 May 2014 14:28:37 -0300 Subject: [PATCH] [search-improvements] Refactor search filters --- app/controllers/public/search_controller.rb | 14 +++++++------- app/helpers/search_helper.rb | 58 ++++++++++++++++++++++++++++++---------------------------- app/models/article.rb | 11 ++++------- app/models/enterprise.rb | 6 +++++- app/models/organization.rb | 9 +++++---- app/models/person.rb | 9 +++++---- app/models/product.rb | 9 ++++----- app/models/profile.rb | 9 ++++----- app/views/search/_compact_profile.html.erb | 4 ++-- app/views/search/_display_results.html.erb | 2 +- app/views/search/_full_enterprise.html.erb | 2 +- test/functional/search_controller_test.rb | 10 +++++----- 12 files changed, 73 insertions(+), 70 deletions(-) diff --git a/app/controllers/public/search_controller.rb b/app/controllers/public/search_controller.rb index 8ddec6f..a75111c 100644 --- a/app/controllers/public/search_controller.rb +++ b/app/controllers/public/search_controller.rb @@ -8,7 +8,7 @@ class SearchController < PublicController before_filter :load_category, :except => :suggestions before_filter :load_search_assets, :except => :suggestions before_filter :load_query, :except => :suggestions - before_filter :load_filter, :except => :suggestions + before_filter :load_order, :except => :suggestions # Backwards compatibility with old URLs def redirect_asset_param @@ -152,7 +152,7 @@ class SearchController < PublicController def load_query @asset = (params[:asset] || params[:action]).to_sym - @order ||= [@asset] + @assets ||= [@asset] @searches ||= {} @query = params[:query] || '' @@ -189,11 +189,11 @@ class SearchController < PublicController @names = @titles if @names.nil? end - def load_filter - @filter = 'more_recent' + def load_order + @order = 'more_recent' if SEARCHES.keys.include?(@asset.to_sym) - available_filters = asset_class(@asset)::SEARCH_FILTERS - @filter = params[:filter] if available_filters.include?(params[:filter]) + available_orders = asset_class(@asset)::SEARCH_FILTERS[:order] + @order = params[:order] if available_orders.include?(params[:order]) end end @@ -217,7 +217,7 @@ class SearchController < PublicController end def full_text_search - @searches[@asset] = find_by_contents(@asset, environment, @scope, @query, paginate_options, {:category => @category, :filter => @filter}) + @searches[@asset] = find_by_contents(@asset, environment, @scope, @query, paginate_options, {:category => @category, :filter => @order}) end private diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 1f1114e..c76b15e 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -14,11 +14,23 @@ module SearchHelper :events, _('Events'), ] - FILTER_TRANSLATION = { - 'more_popular' => _('More popular'), - 'more_active' => _('More active'), - 'more_recent' => _('More recent'), - 'more_comments' => _('More comments') + FILTERS_TRANSLATIONS = { + :order => _('Order'), + :display => _('Display') + } + + FILTERS_OPTIONS_TRANSLATION = { + :order => { + 'more_popular' => _('More popular'), + 'more_active' => _('More active'), + 'more_recent' => _('More recent'), + 'more_comments' => _('More comments') + }, + :display => { + 'map' => _('Map'), + 'full' => _('Full'), + 'compact' => _('Compact') + } } # FIXME remove it after search_controler refactored @@ -50,7 +62,7 @@ module SearchHelper end def display?(asset, mode) - defined?(asset_class(asset)::SEARCH_DISPLAYS) && asset_class(asset)::SEARCH_DISPLAYS.include?(mode.to_s) + defined?(asset_class(asset)::SEARCH_FILTERS[:display]) && asset_class(asset)::SEARCH_FILTERS[:display].include?(mode.to_s) end def display_results(searches=nil, asset=nil) @@ -87,32 +99,22 @@ module SearchHelper end end - def display_selector(asset, display, float = 'right') - display = nil if display.blank? - display ||= asset_class(asset).default_search_display - if [display?(asset, :map), display?(asset, :compact), display?(asset, :full)].select {|option| option}.count > 1 - compact_link = display?(asset, :compact) ? (display == 'compact' ? _('Compact') : link_to(_('Compact'), params.merge(:display => 'compact'))) : nil - map_link = display?(asset, :map) ? (display == 'map' ? _('Map') : link_to(_('Map'), params.merge(:display => 'map'))) : nil - full_link = display?(asset, :full) ? (display == 'full' ? _('Full') : link_to(_('Full'), params.merge(:display => 'full'))) : nil - content_tag('div', - content_tag('strong', _('Display')) + ': ' + [compact_link, map_link, full_link].compact.join(' | ').html_safe, - :class => 'search-customize-options' - ) + def select_filter(name, options) + if options.size <= 1 + return + else + options = options.map {|option| [FILTERS_OPTIONS_TRANSLATION[name][option], option]} + options = options_for_select(options, :selected => params[name]) + select_tag(name, options) end end - def filter_selector(asset, filter, float = 'right') + def filters(asset) klass = asset_class(asset) - if klass::SEARCH_FILTERS.count > 1 - options = options_for_select(klass::SEARCH_FILTERS.map {|f| [FILTER_TRANSLATION[f], f]}, filter) - url_params = url_for(params.merge(:filter => 'FILTER')) - onchange = "document.location.href = '#{url_params}'.replace('FILTER', this.value)" - select_field = select_tag(:filter, options, :onchange => onchange) - content_tag('div', - content_tag('strong', _('Filter')) + ': ' + select_field, - :class => "search-customize-options" - ) - end + content_tag('div', klass::SEARCH_FILTERS.map do |name, options| + select_filter(name, options) + end.join("\n"), :id => 'search-filters') + end end def filter_title(asset, filter) diff --git a/app/models/article.rb b/app/models/article.rb index d667bce..7023406 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -14,13 +14,10 @@ class Article < ActiveRecord::Base :filename => 1, } - SEARCH_FILTERS = %w[ - more_recent - more_popular - more_comments - ] - - SEARCH_DISPLAYS = %w[full] + SEARCH_FILTERS = { + :order => %w[more_recent more_popular more_comments], + :display => %w[full] + } def self.default_search_display 'full' diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index f9e0b32..2e86603 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -2,7 +2,11 @@ # only enterprises can offer products and services. class Enterprise < Organization - SEARCH_DISPLAYS += %w[map full] + SEARCH_FILTERS = { + :order => %w[more_recent more_popular more_active], + :display => %w[compact full map] + } + def self.type_name _('Enterprise') diff --git a/app/models/organization.rb b/app/models/organization.rb index 69c9333..d13c457 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -3,10 +3,11 @@ class Organization < Profile attr_accessible :moderated_articles, :foundation_year, :contact_person, :acronym, :legal_form, :economic_activity, :management_information, :cnpj, :display_name, :enable_contact_us - SEARCH_FILTERS += %w[ - more_popular - more_active - ] + SEARCH_FILTERS = { + :order => %w[more_recent more_popular more_active], + :display => %w[compact] + } + settings_items :closed, :type => :boolean, :default => false def closed? diff --git a/app/models/person.rb b/app/models/person.rb index 3dc93c5..aebe59d 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -3,10 +3,11 @@ class Person < Profile attr_accessible :organization, :contact_information, :sex, :birth_date - SEARCH_FILTERS += %w[ - more_popular - more_active - ] + SEARCH_FILTERS = { + :order => %w[more_recent more_popular more_active], + :display => %w[compact] + } + def self.type_name _('Person') diff --git a/app/models/product.rb b/app/models/product.rb index 7a8070e..2cfebc4 100644 --- a/app/models/product.rb +++ b/app/models/product.rb @@ -5,11 +5,10 @@ class Product < ActiveRecord::Base :description => 1, } - SEARCH_FILTERS = %w[ - more_recent - ] - - SEARCH_DISPLAYS = %w[map full] + SEARCH_FILTERS = { + :order => %w[more_recent], + :display => %w[map full] + } attr_accessible :name, :product_category, :highlighted, :price, :enterprise, :image_builder, :description, :available, :qualifiers diff --git a/app/models/profile.rb b/app/models/profile.rb index 7054341..2c31c0e 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -17,11 +17,10 @@ class Profile < ActiveRecord::Base :nickname => 2, } - SEARCH_FILTERS = %w[ - more_recent - ] - - SEARCH_DISPLAYS = %w[compact] + SEARCH_FILTERS = { + :order => %w[more_recent], + :display => %w[compact] + } def self.default_search_display 'compact' diff --git a/app/views/search/_compact_profile.html.erb b/app/views/search/_compact_profile.html.erb index d0ed3eb..5fd62e1 100644 --- a/app/views/search/_compact_profile.html.erb +++ b/app/views/search/_compact_profile.html.erb @@ -1,5 +1,5 @@ -<% filter_label = profile.send(@filter + '_label') %> -<% filter_label += show_date(profile.created_at) if @filter == 'more_recent' %> +<% filter_label = profile.send(@order + '_label') %> +<% filter_label += show_date(profile.created_at) if @order == 'more_recent' %>
  • <%= profile_image_link profile, :portrait, 'div', filter_label %>
    • diff --git a/app/views/search/_display_results.html.erb b/app/views/search/_display_results.html.erb index c95be59..55bda41 100644 --- a/app/views/search/_display_results.html.erb +++ b/app/views/search/_display_results.html.erb @@ -1,5 +1,5 @@
    - <% @order.each do |name| %> + <% @assets.each do |name| %> <% search = @searches[name] %>
    "> diff --git a/app/views/search/_full_enterprise.html.erb b/app/views/search/_full_enterprise.html.erb index 4e1ef23..5b4fafd 100644 --- a/app/views/search/_full_enterprise.html.erb +++ b/app/views/search/_full_enterprise.html.erb @@ -2,7 +2,7 @@
    <%= profile_image_link enterprise, :portrait, 'div', - @filter == 'more_recent' ? enterprise.send(@filter + '_label') + show_date(enterprise.created_at) : enterprise.send(@filter + '_label') %> + @order == 'more_recent' ? enterprise.send(@order + '_label') + show_date(enterprise.created_at) : enterprise.send(@order + '_label') %>
    <%= link_to_homepage(enterprise.name, enterprise.identifier, :class => "search-result-title") %> diff --git a/test/functional/search_controller_test.rb b/test/functional/search_controller_test.rb index 83a407a..54babda 100644 --- a/test/functional/search_controller_test.rb +++ b/test/functional/search_controller_test.rb @@ -558,7 +558,7 @@ class SearchControllerTest < ActionController::TestCase assert_tag :a, '', :attributes => {:class => 'next_page'} end - should 'list all communities filter by more active' do + should 'list all communities sort by more active' do person = fast_create(Person) c1 = create(Community, :name => 'Testing community 1') c2 = create(Community, :name => 'Testing community 2') @@ -567,21 +567,21 @@ class SearchControllerTest < ActionController::TestCase create(ActionTracker::Record, :target => c1, :user => person, :created_at => Time.now, :verb => 'leave_scrap') create(ActionTracker::Record, :target => c2, :user => person, :created_at => Time.now, :verb => 'leave_scrap') create(ActionTracker::Record, :target => c2, :user => person, :created_at => Time.now, :verb => 'leave_scrap') - get :communities, :filter => 'more_active' + get :communities, :order => 'more_active' assert_equal [c2,c1,c3] , assigns(:searches)[:communities][:results] end should "only include visible people in more_recent filter" do # assuming that all filters behave the same! p1 = fast_create(Person, :visible => false) - get :people, :filter => 'more_recent' + get :people, :order => 'more_recent' assert_not_includes assigns(:searches)[:people][:results], p1 end should "only include visible communities in more_recent filter" do # assuming that all filters behave the same! p1 = fast_create(Community, :visible => false) - get :communities, :filter => 'more_recent' + get :communities, :order=> 'more_recent' assert_not_includes assigns(:searches)[:communities][:results], p1 end @@ -648,7 +648,7 @@ class SearchControllerTest < ActionController::TestCase art2 = create(Article, :name => 'review A', :profile_id => fast_create(Person).id, :created_at => Time.now) art3 = create(Article, :name => 'review B', :profile_id => fast_create(Person).id, :created_at => Time.now-2.days) - get :articles, :filter => :more_recent + get :articles, :order=> :more_recent assert_equal [art2, art1, art3], assigns(:searches)[:articles][:results] end -- libgit2 0.21.2