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

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
+    ('<ul>' + links.map {|link| content_tag('li', link)}.join(' ') + '</ul>') 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 @@
 </p>

 <p>

 <% @object_types.each do |t| %>

-<label><%= check_box_tag t, 1, @scope.include?(t) %> <%= l("label_#{t.singularize}_plural")%></label>

+<label><%= check_box_tag t, 1, @scope.include?(t) %> <%= type_label(t) %></label>

 <% end %>

 </p>

 

@@ -19,12 +19,16 @@
 </div>

 

 <% if @results %>

-    <h3><%= l(:label_result_plural) %></h3>

+    <div id="search-results-counts">

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

+    </div>

+    

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

     <dl id="search-results">

       <% @results.each do |e| %>

         <dt class="<%= e.event_type %>"><%= 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 %></dt>

         <dd><span class="description"><%= highlight_tokens(e.event_description, @tokens) %></span>

-        <span class="author"><%= format_time(e.event_datetime) %></span><dd>

+        <span class="author"><%= format_time(e.event_datetime) %></span></dd>

       <% end %>

     </dl>

 <% 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