From be2b8a62f4d0d8bf413a5583fe644bd41a8ebf04 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 20 Jul 2008 17:26:07 +0000 Subject: [PATCH] Search engine: display total results count (#906) and count by result type. git-svn-id: http://redmine.rubyforge.org/svn/trunk@1681 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/search_controller.rb | 7 ++- app/helpers/search_helper.rb | 16 ++++++ app/models/issue.rb | 5 +- app/models/journal.rb | 6 --- app/views/search/index.rhtml | 10 ++-- public/stylesheets/application.css | 6 ++- test/functional/search_controller_test.rb | 8 +++ test/unit/search_test.rb | 49 +++++++++++-------- .../lib/acts_as_searchable.rb | 40 +++++++++------ 9 files changed, 101 insertions(+), 46 deletions(-) diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index d4ef01bf8..50e42f088 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -72,15 +72,20 @@ class SearchController < ApplicationController @tokens.slice! 5..-1 if @tokens.size > 5 # strings used in sql like statement like_tokens = @tokens.collect {|w| "%#{w.downcase}%"} + @results = [] + @results_by_type = Hash.new {|h,k| h[k] = 0} + limit = 10 @scope.each do |s| - @results += s.singularize.camelcase.constantize.search(like_tokens, projects_to_search, + r, c = s.singularize.camelcase.constantize.search(like_tokens, projects_to_search, :all_words => @all_words, :titles_only => @titles_only, :limit => (limit+1), :offset => offset, :before => params[:previous].nil?) + @results += r + @results_by_type[s] += c end @results = @results.sort {|a,b| b.event_datetime <=> a.event_datetime} if params[:previous].nil? diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index d6a2fb949..92f2da8a5 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -36,6 +36,10 @@ module SearchHelper result end + def type_label(t) + l("label_#{t.singularize}_plural") + end + def project_select_tag options = [[l(:label_project_all), 'all']] options << [l(:label_my_projects), 'my_projects'] unless User.current.memberships.empty? @@ -43,4 +47,16 @@ module SearchHelper options << [@project.name, ''] unless @project.nil? select_tag('scope', options_for_select(options, params[:scope].to_s)) if options.size > 1 end + + def render_results_by_type(results_by_type) + links = [] + # Sorts types by results count + results_by_type.keys.sort {|a, b| results_by_type[b] <=> results_by_type[a]}.each do |t| + c = results_by_type[t] + next if c == 0 + text = "#{type_label(t)} (#{c})" + links << link_to(text, :q => params[:q], :titles_only => params[:title_only], :all_words => params[:all_words], :scope => params[:scope], t => 1) + end + ('') unless links.empty? + end end diff --git a/app/models/issue.rb b/app/models/issue.rb index b19e64883..cae603dd8 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -35,7 +35,10 @@ class Issue < ActiveRecord::Base acts_as_customizable acts_as_watchable - acts_as_searchable :columns => ['subject', "#{table_name}.description"], :include => :project, :with => {:journal => :issue} + acts_as_searchable :columns => ['subject', "#{table_name}.description", "#{Journal.table_name}.notes"], + :include => [:project, :journals], + # sort by id so that limited eager loading doesn't break with postgresql + :order_column => "#{table_name}.id" acts_as_event :title => Proc.new {|o| "#{o.tracker.name} ##{o.id}: #{o.subject}"}, :url => Proc.new {|o| {:controller => 'issues', :action => 'show', :id => o.id}} diff --git a/app/models/journal.rb b/app/models/journal.rb index 8583f63de..a427f84e3 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -25,12 +25,6 @@ class Journal < ActiveRecord::Base has_many :details, :class_name => "JournalDetail", :dependent => :delete_all attr_accessor :indice - acts_as_searchable :columns => 'notes', - :include => {:issue => :project}, - :project_key => "#{Issue.table_name}.project_id", - :date_column => "#{Issue.table_name}.created_on", - :permission => :view_issues - acts_as_event :title => Proc.new {|o| status = ((s = o.new_status) ? " (#{s})" : nil); "#{o.issue.tracker} ##{o.issue.id}#{status}: #{o.issue.subject}" }, :description => :notes, :author => :user, diff --git a/app/views/search/index.rhtml b/app/views/search/index.rhtml index b5cea4645..cb5b70a4c 100644 --- a/app/views/search/index.rhtml +++ b/app/views/search/index.rhtml @@ -10,7 +10,7 @@

<% @object_types.each do |t| %> - + <% end %>

@@ -19,12 +19,16 @@ <% if @results %> -

<%= l(:label_result_plural) %>

+
+ <%= render_results_by_type(@results_by_type) unless @scope.size == 1 %> +
+ +

<%= l(:label_result_plural) %> (<%= @results_by_type.values.sum %>)

<% @results.each do |e| %>
<%= content_tag('span', h(e.project), :class => 'project') unless @project == e.project %> <%= link_to highlight_tokens(truncate(e.event_title, 255), @tokens), e.event_url %>
<%= highlight_tokens(e.event_description, @tokens) %> - <%= format_time(e.event_datetime) %>
+ <%= format_time(e.event_datetime) %>
<% end %>
<% end %> diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index f6c86b294..018c71732 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -185,9 +185,13 @@ div#activity dt.me .time { border-bottom: 1px solid #999; } div#activity dt .time { color: #777; font-size: 80%; } div#activity dd .description, #search-results dd .description { font-style: italic; } div#activity span.project:after, #search-results span.project:after { content: " -"; } -#search-results dd { margin-bottom: 1em; padding-left: 20px; margin-left:0px;} div#activity dd span.description, #search-results dd span.description { display:block; } +#search-results dd { margin-bottom: 1em; padding-left: 20px; margin-left:0px; } +div#search-results-counts {float:right;} +div#search-results-counts ul { margin-top: 0.5em; } +div#search-results-counts li { list-style-type:none; float: left; margin-left: 1em; } + dt.issue { background-image: url(../images/ticket.png); } dt.issue-edit { background-image: url(../images/ticket_edit.png); } dt.issue-closed { background-image: url(../images/ticket_checked.png); } diff --git a/test/functional/search_controller_test.rb b/test/functional/search_controller_test.rb index 1c505620b..ce06ec298 100644 --- a/test/functional/search_controller_test.rb +++ b/test/functional/search_controller_test.rb @@ -32,9 +32,17 @@ class SearchControllerTest < Test::Unit::TestCase get :index, :q => 'recipe subproject commit', :submit => 'Search' assert_response :success assert_template 'index' + assert assigns(:results).include?(Issue.find(2)) assert assigns(:results).include?(Issue.find(5)) assert assigns(:results).include?(Changeset.find(101)) + assert_tag :dt, :attributes => { :class => /issue/ }, + :child => { :tag => 'a', :content => /Add ingredients categories/ }, + :sibling => { :tag => 'dd', :content => /should be classified by categories/ } + + assert assigns(:results_by_type).is_a?(Hash) + assert_equal 4, assigns(:results_by_type)['changesets'] + assert_tag :a, :content => 'Changesets (4)' end def test_search_project_and_subprojects diff --git a/test/unit/search_test.rb b/test/unit/search_test.rb index 244b27a58..1b32df733 100644 --- a/test/unit/search_test.rb +++ b/test/unit/search_test.rb @@ -41,24 +41,24 @@ class SearchTest < Test::Unit::TestCase def test_search_by_anonymous User.current = nil - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert r.include?(@changeset) # Removes the :view_changesets permission from Anonymous role remove_permission Role.anonymous, :view_changesets - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert !r.include?(@changeset) # Make the project private @project.update_attribute :is_public, false - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert !r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert !r.include?(@changeset) end @@ -66,24 +66,24 @@ class SearchTest < Test::Unit::TestCase User.current = User.find_by_login('rhill') assert User.current.memberships.empty? - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert r.include?(@changeset) # Removes the :view_changesets permission from Non member role remove_permission Role.non_member, :view_changesets - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert !r.include?(@changeset) # Make the project private @project.update_attribute :is_public, false - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert !r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert !r.include?(@changeset) end @@ -91,16 +91,16 @@ class SearchTest < Test::Unit::TestCase User.current = User.find_by_login('jsmith') assert User.current.projects.include?(@project) - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert r.include?(@changeset) # Make the project private @project.update_attribute :is_public, false - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert r.include?(@changeset) end @@ -112,19 +112,28 @@ class SearchTest < Test::Unit::TestCase User.current = User.find_by_login('jsmith') assert User.current.projects.include?(@project) - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert !r.include?(@changeset) # Make the project private @project.update_attribute :is_public, false - r = Issue.search(@issue_keyword) + r = Issue.search(@issue_keyword).first assert r.include?(@issue) - r = Changeset.search(@changeset_keyword) + r = Changeset.search(@changeset_keyword).first assert !r.include?(@changeset) end + def test_search_issue_with_multiple_hits_in_journals + i = Issue.find(1) + assert_equal 2, i.journals.count(:all, :conditions => "notes LIKE '%notes%'") + + r = Issue.search('%notes%').first + assert_equal 1, r.size + assert_equal i, r.first + end + private def remove_permission(role, permission) diff --git a/vendor/plugins/acts_as_searchable/lib/acts_as_searchable.rb b/vendor/plugins/acts_as_searchable/lib/acts_as_searchable.rb index 2c773d70a..fec933352 100644 --- a/vendor/plugins/acts_as_searchable/lib/acts_as_searchable.rb +++ b/vendor/plugins/acts_as_searchable/lib/acts_as_searchable.rb @@ -23,6 +23,12 @@ module Redmine end module ClassMethods + # Options: + # * :columns - a column or an array of columns to search + # * :project_key - project foreign key (default to project_id) + # * :date_column - name of the datetime column (default to created_on) + # * :sort_order - name of the column used to sort results (default to :date_column or created_on) + # * :permission - permission required to search the model (default to :view_"objects") def acts_as_searchable(options = {}) return if self.included_modules.include?(Redmine::Acts::Searchable::InstanceMethods) @@ -49,6 +55,8 @@ module Redmine raise 'No date column defined defined.' end + searchable_options[:order_column] ||= searchable_options[:date_column] + # Permission needed to search this model searchable_options[:permission] = "view_#{self.name.underscore.pluralize}".to_sym unless searchable_options.has_key?(:permission) @@ -65,15 +73,22 @@ module Redmine end module ClassMethods - # Search the model for the given tokens + # Searches the model for the given tokens # projects argument can be either nil (will search all projects), a project or an array of projects + # Returns the results and the results count def search(tokens, projects=nil, options={}) tokens = [] << tokens unless tokens.is_a?(Array) projects = [] << projects unless projects.nil? || projects.is_a?(Array) find_options = {:include => searchable_options[:include]} - find_options[:limit] = options[:limit] if options[:limit] - find_options[:order] = "#{searchable_options[:date_column]} " + (options[:before] ? 'DESC' : 'ASC') + find_options[:order] = "#{searchable_options[:order_column]} " + (options[:before] ? 'DESC' : 'ASC') + + limit_options = {} + limit_options[:limit] = options[:limit] if options[:limit] + if options[:offset] + limit_options[:conditions] = "(#{searchable_options[:date_column]} " + (options[:before] ? '<' : '>') + "'#{connection.quoted_date(options[:offset])}')" + end + columns = searchable_options[:columns] columns.slice!(1..-1) if options[:titles_only] @@ -94,9 +109,6 @@ module Redmine sql = (['(' + token_clauses.join(' OR ') + ')'] * tokens.size).join(options[:all_words] ? ' AND ' : ' OR ') - if options[:offset] - sql = "(#{sql}) AND (#{searchable_options[:date_column]} " + (options[:before] ? '<' : '>') + "'#{connection.quoted_date(options[:offset])}')" - end find_options[:conditions] = [sql, * (tokens * token_clauses.size).sort] project_conditions = [] @@ -104,16 +116,16 @@ module Redmine Project.allowed_to_condition(User.current, searchable_options[:permission])) project_conditions << "#{searchable_options[:project_key]} IN (#{projects.collect(&:id).join(',')})" unless projects.nil? - results = with_scope(:find => {:conditions => project_conditions.join(' AND ')}) do - find(:all, find_options) - end - if searchable_options[:with] && !options[:titles_only] - searchable_options[:with].each do |model, assoc| - results += model.to_s.camelcase.constantize.search(tokens, projects, options).collect {|r| r.send assoc} + results = [] + results_count = 0 + + with_scope(:find => {:conditions => project_conditions.join(' AND ')}) do + with_scope(:find => find_options) do + results_count = count(:all) + results = find(:all, limit_options) end - results.uniq! end - results + [results, results_count] end end end -- 2.39.5