[search-improvements] Refactor search filters

Rodrigo Souto
1 parent ca19bf6a
Showing 12 changed files with 73 additions and 70 deletions Show diff stats
app/controllers/public/search_controller.rb
app/helpers/search_helper.rb
app/models/article.rb
app/models/enterprise.rb
app/models/organization.rb
app/models/person.rb
app/models/product.rb
app/models/profile.rb
app/views/search/_compact_profile.html.erb
app/views/search/_display_results.html.erb
app/views/search/_full_enterprise.html.erb
test/functional/search_controller_test.rb
@@ -8,7 +8,7 @@ class SearchController &lt; 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 &lt; 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 &lt; 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 &lt; 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
@@ -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)
@@ -14,13 +14,10 @@ class Article &lt; 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'
@@ -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')
@@ -3,10 +3,11 @@ class Organization &lt; 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?
@@ -3,10 +3,11 @@ class Person &lt; 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')
@@ -5,11 +5,10 @@ class Product &lt; 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
  
@@ -17,11 +17,10 @@ class Profile &lt; 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'
-<% 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' %>
 <li class="search-profile-item">
   <%= profile_image_link profile, :portrait, 'div', filter_label %>
 </li>
 <div id="search-results" class="<%= !multiple_search? ? 'only-one-result-box' : 'multiple-results-boxes' %>">
-  <% @order.each do |name| %>
+  <% @assets.each do |name| %>
     <% search = @searches[name] %>
  
     <div class="search-results-<%= name %> search-results-box <%= "search-results-empty" if search[:results].blank? %>">
@@ -2,7 +2,7 @@
   <div class="search-enterprise-item">
     <div class="search-enterprise-item-column-left">
       <%= 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') %>
     </div>
     <div class="search-enterprise-item-column-right">
       <%= link_to_homepage(enterprise.name, enterprise.identifier, :class => "search-result-title") %>
@@ -558,7 +558,7 @@ class SearchControllerTest &lt; 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 &lt; 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 &lt; 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
...	...	@@ -8,7 +8,7 @@ class SearchController < PublicController
8	8	before_filter :load_category, :except => :suggestions
9	9	before_filter :load_search_assets, :except => :suggestions
10	10	before_filter :load_query, :except => :suggestions
11		- before_filter :load_filter, :except => :suggestions
	11	+ before_filter :load_order, :except => :suggestions
12	12
13	13	# Backwards compatibility with old URLs
14	14	def redirect_asset_param
...	...	@@ -152,7 +152,7 @@ class SearchController < PublicController
152	152
153	153	def load_query
154	154	@asset = (params[:asset] \|\| params[:action]).to_sym
155		- @order \|\|= [@asset]
	155	+ @assets \|\|= [@asset]
156	156	@searches \|\|= {}
157	157
158	158	@query = params[:query] \|\| ''
...	...	@@ -189,11 +189,11 @@ class SearchController < PublicController
189	189	@names = @titles if @names.nil?
190	190	end
191	191
192		- def load_filter
193		- @filter = 'more_recent'
	192	+ def load_order
	193	+ @order = 'more_recent'
194	194	if SEARCHES.keys.include?(@asset.to_sym)
195		- available_filters = asset_class(@asset)::SEARCH_FILTERS
196		- @filter = params[:filter] if available_filters.include?(params[:filter])
	195	+ available_orders = asset_class(@asset)::SEARCH_FILTERS[:order]
	196	+ @order = params[:order] if available_orders.include?(params[:order])
197	197	end
198	198	end
199	199
...	...	@@ -217,7 +217,7 @@ class SearchController < PublicController
217	217	end
218	218
219	219	def full_text_search
220		- @searches[@asset] = find_by_contents(@asset, environment, @scope, @query, paginate_options, {:category => @category, :filter => @filter})
	220	+ @searches[@asset] = find_by_contents(@asset, environment, @scope, @query, paginate_options, {:category => @category, :filter => @order})
221	221	end
222	222
223	223	private
...	...
...	...	@@ -14,11 +14,23 @@ module SearchHelper
14	14	:events, _('Events'),
15	15	]
16	16
17		- FILTER_TRANSLATION = {
18		- 'more_popular' => _('More popular'),
19		- 'more_active' => _('More active'),
20		- 'more_recent' => _('More recent'),
21		- 'more_comments' => _('More comments')
	17	+ FILTERS_TRANSLATIONS = {
	18	+ :order => _('Order'),
	19	+ :display => _('Display')
	20	+ }
	21	+
	22	+ FILTERS_OPTIONS_TRANSLATION = {
	23	+ :order => {
	24	+ 'more_popular' => _('More popular'),
	25	+ 'more_active' => _('More active'),
	26	+ 'more_recent' => _('More recent'),
	27	+ 'more_comments' => _('More comments')
	28	+ },
	29	+ :display => {
	30	+ 'map' => _('Map'),
	31	+ 'full' => _('Full'),
	32	+ 'compact' => _('Compact')
	33	+ }
22	34	}
23	35
24	36	# FIXME remove it after search_controler refactored
...	...	@@ -50,7 +62,7 @@ module SearchHelper
50	62	end
51	63
52	64	def display?(asset, mode)
53		- defined?(asset_class(asset)::SEARCH_DISPLAYS) && asset_class(asset)::SEARCH_DISPLAYS.include?(mode.to_s)
	65	+ defined?(asset_class(asset)::SEARCH_FILTERS[:display]) && asset_class(asset)::SEARCH_FILTERS[:display].include?(mode.to_s)
54	66	end
55	67
56	68	def display_results(searches=nil, asset=nil)
...	...	@@ -87,32 +99,22 @@ module SearchHelper
87	99	end
88	100	end
89	101
90		- def display_selector(asset, display, float = 'right')
91		- display = nil if display.blank?
92		- display \|\|= asset_class(asset).default_search_display
93		- if [display?(asset, :map), display?(asset, :compact), display?(asset, :full)].select {\|option\| option}.count > 1
94		- compact_link = display?(asset, :compact) ? (display == 'compact' ? _('Compact') : link_to(_('Compact'), params.merge(:display => 'compact'))) : nil
95		- map_link = display?(asset, :map) ? (display == 'map' ? _('Map') : link_to(_('Map'), params.merge(:display => 'map'))) : nil
96		- full_link = display?(asset, :full) ? (display == 'full' ? _('Full') : link_to(_('Full'), params.merge(:display => 'full'))) : nil
97		- content_tag('div',
98		- content_tag('strong', _('Display')) + ': ' + [compact_link, map_link, full_link].compact.join(' \| ').html_safe,
99		- :class => 'search-customize-options'
100		- )
	102	+ def select_filter(name, options)
	103	+ if options.size <= 1
	104	+ return
	105	+ else
	106	+ options = options.map {\|option\| [FILTERS_OPTIONS_TRANSLATION[name][option], option]}
	107	+ options = options_for_select(options, :selected => params[name])
	108	+ select_tag(name, options)
101	109	end
102	110	end
103	111
104		- def filter_selector(asset, filter, float = 'right')
	112	+ def filters(asset)
105	113	klass = asset_class(asset)
106		- if klass::SEARCH_FILTERS.count > 1
107		- options = options_for_select(klass::SEARCH_FILTERS.map {\|f\| [FILTER_TRANSLATION[f], f]}, filter)
108		- url_params = url_for(params.merge(:filter => 'FILTER'))
109		- onchange = "document.location.href = '#{url_params}'.replace('FILTER', this.value)"
110		- select_field = select_tag(:filter, options, :onchange => onchange)
111		- content_tag('div',
112		- content_tag('strong', _('Filter')) + ': ' + select_field,
113		- :class => "search-customize-options"
114		- )
115		- end
	114	+ content_tag('div', klass::SEARCH_FILTERS.map do \|name, options\|
	115	+ select_filter(name, options)
	116	+ end.join("\n"), :id => 'search-filters')
	117	+ end
116	118	end
117	119
118	120	def filter_title(asset, filter)
...	...
...	...	@@ -14,13 +14,10 @@ class Article < ActiveRecord::Base
14	14	:filename => 1,
15	15	}
16	16
17		- SEARCH_FILTERS = %w[
18		- more_recent
19		- more_popular
20		- more_comments
21		- ]
22		-
23		- SEARCH_DISPLAYS = %w[full]
	17	+ SEARCH_FILTERS = {
	18	+ :order => %w[more_recent more_popular more_comments],
	19	+ :display => %w[full]
	20	+ }
24	21
25	22	def self.default_search_display
26	23	'full'
...	...
...	...	@@ -2,7 +2,11 @@
2	2	# only enterprises can offer products and services.
3	3	class Enterprise < Organization
4	4
5		- SEARCH_DISPLAYS += %w[map full]
	5	+ SEARCH_FILTERS = {
	6	+ :order => %w[more_recent more_popular more_active],
	7	+ :display => %w[compact full map]
	8	+ }
	9	+
6	10
7	11	def self.type_name
8	12	_('Enterprise')
...	...
...	...	@@ -3,10 +3,11 @@ class Organization < Profile
3	3
4	4	attr_accessible :moderated_articles, :foundation_year, :contact_person, :acronym, :legal_form, :economic_activity, :management_information, :cnpj, :display_name, :enable_contact_us
5	5
6		- SEARCH_FILTERS += %w[
7		- more_popular
8		- more_active
9		- ]
	6	+ SEARCH_FILTERS = {
	7	+ :order => %w[more_recent more_popular more_active],
	8	+ :display => %w[compact]
	9	+ }
	10	+
10	11
11	12	settings_items :closed, :type => :boolean, :default => false
12	13	def closed?
...	...
...	...	@@ -3,10 +3,11 @@ class Person < Profile
3	3
4	4	attr_accessible :organization, :contact_information, :sex, :birth_date
5	5
6		- SEARCH_FILTERS += %w[
7		- more_popular
8		- more_active
9		- ]
	6	+ SEARCH_FILTERS = {
	7	+ :order => %w[more_recent more_popular more_active],
	8	+ :display => %w[compact]
	9	+ }
	10	+
10	11
11	12	def self.type_name
12	13	_('Person')
...	...
...	...	@@ -5,11 +5,10 @@ class Product < ActiveRecord::Base
5	5	:description => 1,
6	6	}
7	7
8		- SEARCH_FILTERS = %w[
9		- more_recent
10		- ]
11		-
12		- SEARCH_DISPLAYS = %w[map full]
	8	+ SEARCH_FILTERS = {
	9	+ :order => %w[more_recent],
	10	+ :display => %w[map full]
	11	+ }
13	12
14	13	attr_accessible :name, :product_category, :highlighted, :price, :enterprise, :image_builder, :description, :available, :qualifiers
15	14
...	...
...	...	@@ -17,11 +17,10 @@ class Profile < ActiveRecord::Base
17	17	:nickname => 2,
18	18	}
19	19
20		- SEARCH_FILTERS = %w[
21		- more_recent
22		- ]
23		-
24		- SEARCH_DISPLAYS = %w[compact]
	20	+ SEARCH_FILTERS = {
	21	+ :order => %w[more_recent],
	22	+ :display => %w[compact]
	23	+ }
25	24
26	25	def self.default_search_display
27	26	'compact'
...	...
1		-<% filter_label = profile.send(@filter + '_label') %>
2		-<% filter_label += show_date(profile.created_at) if @filter == 'more_recent' %>
	1	+<% filter_label = profile.send(@order + '_label') %>
	2	+<% filter_label += show_date(profile.created_at) if @order == 'more_recent' %>
3	3	<li class="search-profile-item">
4	4	<%= profile_image_link profile, :portrait, 'div', filter_label %>
5	5	</li>
...	...
1	1	<div id="search-results" class="<%= !multiple_search? ? 'only-one-result-box' : 'multiple-results-boxes' %>">
2		- <% @order.each do \|name\| %>
	2	+ <% @assets.each do \|name\| %>
3	3	<% search = @searches[name] %>
4	4
5	5	<div class="search-results-<%= name %> search-results-box <%= "search-results-empty" if search[:results].blank? %>">
...	...