Commit 38da9b9ed0bf2ea087f1ed67984851d2320dbb33
1 parent
43a5b255
Exists in
master
and in
28 other branches
ActionItem410: refactoring search controller
To unify interface _and_ add the possibility of having specific result pages for each type of content git-svn-id: https://svn.colivre.coop.br/svn/noosfero/trunk@1881 3f533792-8f58-4932-b0fe-aaf55b0a4547
Showing
5 changed files
with
52 additions
and
21 deletions
Show diff stats
app/controllers/public/search_controller.rb
| @@ -69,6 +69,8 @@ class SearchController < ApplicationController | @@ -69,6 +69,8 @@ class SearchController < ApplicationController | ||
| 69 | def index | 69 | def index |
| 70 | @query = params[:query] || '' | 70 | @query = params[:query] || '' |
| 71 | @filtered_query = remove_stop_words(@query) | 71 | @filtered_query = remove_stop_words(@query) |
| 72 | + | ||
| 73 | + # FIXME name is not unique | ||
| 72 | @region = Region.find_by_name(params[:region][:name]) if params[:region] | 74 | @region = Region.find_by_name(params[:region][:name]) if params[:region] |
| 73 | 75 | ||
| 74 | @results = {} | 76 | @results = {} |
| @@ -81,6 +83,12 @@ class SearchController < ApplicationController | @@ -81,6 +83,12 @@ class SearchController < ApplicationController | ||
| 81 | end | 83 | end |
| 82 | @names[key] = gettext(description) | 84 | @names[key] = gettext(description) |
| 83 | end | 85 | end |
| 86 | + | ||
| 87 | + if @results.keys.size == 1 | ||
| 88 | + specific_action = @results.keys.first | ||
| 89 | + send(specific_action) | ||
| 90 | + render :action => specific_action | ||
| 91 | + end | ||
| 84 | end | 92 | end |
| 85 | 93 | ||
| 86 | ####################################################### | 94 | ####################################################### |
| @@ -90,12 +98,13 @@ class SearchController < ApplicationController | @@ -90,12 +98,13 @@ class SearchController < ApplicationController | ||
| 90 | @results = {} | 98 | @results = {} |
| 91 | @names = {} | 99 | @names = {} |
| 92 | [ | 100 | [ |
| 93 | - [ :recent_people, _('Recently registered people'), @finder.recent('people') ], | ||
| 94 | - [ :recent_communities, _('Recently created communities'), @finder.recent('communities') ], | ||
| 95 | - [ :recent_articles, _('Recent articles'), @finder.recent('articles') ], | 101 | + [ :people, _('Recently registered people'), @finder.recent('people') ], |
| 102 | + [ :communities, _('Recently created communities'), @finder.recent('communities') ], | ||
| 103 | + [ :articles, _('Recent articles'), @finder.recent('articles') ], | ||
| 96 | [ :comments, _('Recent comments'), @finder.recent('comments') ], | 104 | [ :comments, _('Recent comments'), @finder.recent('comments') ], |
| 97 | [ :most_commented_articles, _('Most commented articles'), @finder.most_commented_articles ], | 105 | [ :most_commented_articles, _('Most commented articles'), @finder.most_commented_articles ], |
| 98 | - [ :recent_enterptises, _('Recently created enterprises'), @finder.recent('enterprises') ] | 106 | + [ :enterprises, _('Recently created enterprises'), @finder.recent('enterprises') ], |
| 107 | + [ :events, _('Recently added events'), @finder.recent('events') ] | ||
| 99 | ].each do |key, name, list| | 108 | ].each do |key, name, list| |
| 100 | @results[key] = list | 109 | @results[key] = list |
| 101 | @names[key] = name | 110 | @names[key] = name |
app/models/category_finder.rb
| @@ -7,12 +7,16 @@ class CategoryFinder | @@ -7,12 +7,16 @@ class CategoryFinder | ||
| 7 | 7 | ||
| 8 | attr_reader :category_ids | 8 | attr_reader :category_ids |
| 9 | 9 | ||
| 10 | - def find(asset, query, options={}) | ||
| 11 | - find_in_categorized(asset.to_s.singularize.camelize.constantize, query, options) | 10 | + def find(asset, query = nil, options={}, limit = nil) |
| 11 | + if query.blank? | ||
| 12 | + asset_class(asset).find(:all, options_for_find(asset_class(asset), {:limit => limit, :order => "created_at desc, #{asset_table(asset)}.id desc"})) | ||
| 13 | + else | ||
| 14 | + find_in_categorized(asset.to_s.singularize.camelize.constantize, query, options) | ||
| 15 | + end | ||
| 12 | end | 16 | end |
| 13 | 17 | ||
| 14 | - def recent(asset, limit = 10) | ||
| 15 | - asset_class(asset).find(:all, options_for_find(asset_class(asset), {:limit => limit, :order => "created_at desc, #{asset_table(asset)}.id desc"})) | 18 | + def recent(asset, limit = nil) |
| 19 | + find(asset, nil, {}, limit) | ||
| 16 | end | 20 | end |
| 17 | 21 | ||
| 18 | def find_by_initial(asset, initial) | 22 | def find_by_initial(asset, initial) |
app/models/environment_finder.rb
| @@ -4,22 +4,27 @@ class EnvironmentFinder | @@ -4,22 +4,27 @@ class EnvironmentFinder | ||
| 4 | @environment = env | 4 | @environment = env |
| 5 | end | 5 | end |
| 6 | 6 | ||
| 7 | - def find(asset, query, options={}) | ||
| 8 | - @region = Region.find_by_id(options.delete(:region)) if options.has_key?(:region) | 7 | + def find(asset, query = nil, options={}, limit = nil) |
| 8 | + @region = Region.find_by_id(options.delete(:region)) if options.has_key?(:region) | ||
| 9 | if @region && options[:within] | 9 | if @region && options[:within] |
| 10 | options[:origin] = [@region.lat, @region.lng] | 10 | options[:origin] = [@region.lat, @region.lng] |
| 11 | else | 11 | else |
| 12 | options.delete(:within) | 12 | options.delete(:within) |
| 13 | end | 13 | end |
| 14 | - @environment.send(asset).find_by_contents(query, {}, options) | ||
| 15 | - end | ||
| 16 | 14 | ||
| 17 | - def recent(asset, limit = 10) | ||
| 18 | - with_options :limit => limit, :order => 'created_at desc, id desc' do |finder| | ||
| 19 | - @environment.send(asset).recent(limit) | 15 | + if query.blank? |
| 16 | + with_options :limit => limit, :order => 'created_at desc, id desc' do |finder| | ||
| 17 | + @environment.send(asset).recent(limit) | ||
| 18 | + end | ||
| 19 | + else | ||
| 20 | + @environment.send(asset).find_by_contents(query, {}, options) | ||
| 20 | end | 21 | end |
| 21 | end | 22 | end |
| 22 | 23 | ||
| 24 | + def recent(asset, limit = nil) | ||
| 25 | + find(asset, nil, {}, limit) | ||
| 26 | + end | ||
| 27 | + | ||
| 23 | def find_by_initial(asset, initial) | 28 | def find_by_initial(asset, initial) |
| 24 | @environment.send(asset).find_by_initial(initial) | 29 | @environment.send(asset).find_by_initial(initial) |
| 25 | end | 30 | end |
app/views/search/_display_results.rhtml
| @@ -12,7 +12,10 @@ | @@ -12,7 +12,10 @@ | ||
| 12 | <% if !results.nil? and !results.empty? %> | 12 | <% if !results.nil? and !results.empty? %> |
| 13 | <div class="search-results-<%= name %> search-results-box <%= pos2 %> <%= 'col%s_of3' % pos3.to_s %>"> | 13 | <div class="search-results-<%= name %> search-results-box <%= pos2 %> <%= 'col%s_of3' % pos3.to_s %>"> |
| 14 | <% if @controller.action_name != 'assets' %> | 14 | <% if @controller.action_name != 'assets' %> |
| 15 | - <h3><%= @names[name] %></h3> | 15 | + <h3> |
| 16 | + <%= @names[name] %> | ||
| 17 | + <%= link_to _('(see more ...)'), params.merge(:action => 'index', :find_in => [ name ]) %> | ||
| 18 | + </h3> | ||
| 16 | <% end %> | 19 | <% end %> |
| 17 | <% partial = partial_for_class results.first.class %> | 20 | <% partial = partial_for_class results.first.class %> |
| 18 | <div class="search-results-innerbox search-results-type-<%= partial %> <%= 'common-profile-list-block' if partial == 'profile' %>"> | 21 | <div class="search-results-innerbox search-results-type-<%= partial %> <%= 'common-profile-list-block' if partial == 'profile' %>"> |
| @@ -22,6 +25,7 @@ | @@ -22,6 +25,7 @@ | ||
| 22 | <% end %> | 25 | <% end %> |
| 23 | </ul> | 26 | </ul> |
| 24 | </div><!-- end class="search-results-innerbox" --> | 27 | </div><!-- end class="search-results-innerbox" --> |
| 28 | + | ||
| 25 | </div><!-- end class="search-results-<%= name %>" --> | 29 | </div><!-- end class="search-results-<%= name %>" --> |
| 26 | <% else %> | 30 | <% else %> |
| 27 | <div class="search-results-<%= name %> search-results-empty search-results-box <%= pos2 %> <%= 'col%s_of3' % pos3.to_s %>"> | 31 | <div class="search-results-<%= name %> search-results-empty search-results-box <%= pos2 %> <%= 'col%s_of3' % pos3.to_s %>"> |
test/functional/search_controller_test.rb
| @@ -362,7 +362,7 @@ class SearchControllerTest < Test::Unit::TestCase | @@ -362,7 +362,7 @@ class SearchControllerTest < Test::Unit::TestCase | ||
| 362 | :products => 'Products', | 362 | :products => 'Products', |
| 363 | } | 363 | } |
| 364 | names.each do |thing, description| | 364 | names.each do |thing, description| |
| 365 | - assert_tag :tag => 'div', :attributes => { :class => /search-results-#{thing}/ }, :descendant => { :tag => 'h3', :content => description } | 365 | + assert_tag :tag => 'div', :attributes => { :class => /search-results-#{thing}/ }, :descendant => { :tag => 'h3', :content => Regexp.new(description) } |
| 366 | assert_tag :tag => 'a', :content => "display #{thing.to_s.singularize}" | 366 | assert_tag :tag => 'a', :content => "display #{thing.to_s.singularize}" |
| 367 | end | 367 | end |
| 368 | end | 368 | end |
| @@ -447,7 +447,7 @@ class SearchControllerTest < Test::Unit::TestCase | @@ -447,7 +447,7 @@ class SearchControllerTest < Test::Unit::TestCase | ||
| 447 | CategoryFinder.expects(:new).with(@category).returns(finger) | 447 | CategoryFinder.expects(:new).with(@category).returns(finger) |
| 448 | 448 | ||
| 449 | get :category_index, :category_path => [ 'my-category' ] | 449 | get :category_index, :category_path => [ 'my-category' ] |
| 450 | - assert_same recent, assigns(:results)[:recent_articles] | 450 | + assert_same recent, assigns(:results)[:articles] |
| 451 | end | 451 | end |
| 452 | 452 | ||
| 453 | should 'list recent comments in the category' do | 453 | should 'list recent comments in the category' do |
| @@ -479,7 +479,7 @@ class SearchControllerTest < Test::Unit::TestCase | @@ -479,7 +479,7 @@ class SearchControllerTest < Test::Unit::TestCase | ||
| 479 | CategoryFinder.expects(:new).with(@category).returns(finger) | 479 | CategoryFinder.expects(:new).with(@category).returns(finger) |
| 480 | 480 | ||
| 481 | get :category_index, :category_path => [ 'my-category' ] | 481 | get :category_index, :category_path => [ 'my-category' ] |
| 482 | - assert_same recent_people, assigns(:results)[:recent_people] | 482 | + assert_same recent_people, assigns(:results)[:people] |
| 483 | end | 483 | end |
| 484 | 484 | ||
| 485 | should 'list recently registered communities in the category' do | 485 | should 'list recently registered communities in the category' do |
| @@ -490,7 +490,7 @@ class SearchControllerTest < Test::Unit::TestCase | @@ -490,7 +490,7 @@ class SearchControllerTest < Test::Unit::TestCase | ||
| 490 | CategoryFinder.expects(:new).with(@category).returns(finger) | 490 | CategoryFinder.expects(:new).with(@category).returns(finger) |
| 491 | 491 | ||
| 492 | get :category_index, :category_path => [ 'my-category' ] | 492 | get :category_index, :category_path => [ 'my-category' ] |
| 493 | - assert_same recent_communities, assigns(:results)[:recent_communities] | 493 | + assert_same recent_communities, assigns(:results)[:communities] |
| 494 | end | 494 | end |
| 495 | 495 | ||
| 496 | should 'list recently registered enterprises in the category' do | 496 | should 'list recently registered enterprises in the category' do |
| @@ -501,7 +501,7 @@ class SearchControllerTest < Test::Unit::TestCase | @@ -501,7 +501,7 @@ class SearchControllerTest < Test::Unit::TestCase | ||
| 501 | CategoryFinder.expects(:new).with(@category).returns(finger) | 501 | CategoryFinder.expects(:new).with(@category).returns(finger) |
| 502 | 502 | ||
| 503 | get :category_index, :category_path => [ 'my-category' ] | 503 | get :category_index, :category_path => [ 'my-category' ] |
| 504 | - assert_same recent_enterptises, assigns(:results)[:recent_enterptises] | 504 | + assert_same recent_enterptises, assigns(:results)[:enterprises] |
| 505 | end | 505 | end |
| 506 | 506 | ||
| 507 | should 'not list "Search for ..." in category_index' do | 507 | should 'not list "Search for ..." in category_index' do |
| @@ -900,6 +900,15 @@ class SearchControllerTest < Test::Unit::TestCase | @@ -900,6 +900,15 @@ class SearchControllerTest < Test::Unit::TestCase | ||
| 900 | assert_not_includes assigns(:results)[:events], ev3 | 900 | assert_not_includes assigns(:results)[:events], ev3 |
| 901 | end | 901 | end |
| 902 | 902 | ||
| 903 | + %w[ people enterprises articles events communities products comments ].each do |asset| | ||
| 904 | + | ||
| 905 | + should "render asset-specific template when searching for #{asset}" do | ||
| 906 | + get :index, :find_in => [ asset ] | ||
| 907 | + assert_template asset | ||
| 908 | + end | ||
| 909 | + | ||
| 910 | + end | ||
| 911 | + | ||
| 903 | ################################################################## | 912 | ################################################################## |
| 904 | ################################################################## | 913 | ################################################################## |
| 905 | 914 |