Merge pull request #56 from edx/bug/jsa/search-result-fields

use ThreadPresenter to render search results

api/search.rb

@@ -37,10 +37,33 @@ get "#{APIPREFIX}/search/threads" do
       results = CommentThread.perform_search(params, options.merge(page: results.total_pages))
     end
 
+    threads = CommentThread.where(id: {"$in" => results.map {|r| r.id} }).to_a
+    if threads.length == 0
+      collection = []
+    else
+      pres_threads = ThreadPresenter.new(
+        threads,
+        params[:user_id] ? user : nil,
+        params[:course_id] || threads.first.course_id
+      )
+      collection = pres_threads.to_hash_array(bool_recursive)
+    end
+
+    # merge highlighted text / highlighted body.  not sure if this is used by the client,
+    # but doing it to preserve the legacy behavior
+    # TODO: move this logic into a presenter object for search results
+    result_map = Hash[results.map { |t| [t.id, t] }]
+    collection.each do |thread_hash|
+      thread_key = thread_hash["id"].to_s
+      highlight = result_map[thread_key].highlight || {}
+      thread_hash["highlighted_body"] = (highlight[:body] || []).first || thread_hash["body"]
+      thread_hash["highlighted_title"] = (highlight[:title] || []).first || thread_hash["title"]
+    end
+
     num_pages = results.total_pages
     page = [num_pages, [1, page].max].min
     {
-      collection: results.map{|t| CommentThread.search_result_to_hash(t, recursive: bool_recursive, user_id: params[:user_id])},
+      collection: collection,
       num_pages: num_pages,
       page: page,
     }.to_json

models/comment_thread.rb

@@ -84,16 +84,6 @@ class CommentThread < Content
     c
   end
 
-  def self.search_result_to_hash(result, params={})
-
-    comment_thread = self.find(result.id)
-    highlight = result.highlight || {}
-
-    highlighted_body = (highlight[:body] || []).first || comment_thread.body
-    highlighted_title = (highlight[:title] || []).first || comment_thread.title
-    comment_thread.to_hash(params).merge(highlighted_body: highlighted_body, highlighted_title: highlighted_title)
-  end
-
   def self.perform_search(params, options={})
     
     page = [1, options[:page] || 1].max

spec/api/query_spec.rb

@@ -22,7 +22,7 @@ describe "app" do
 				get "/api/v1/search/threads", text: random_string
 				last_response.should be_ok
 				threads = parse(last_response.body)['collection']
-				threads.select{|t| t["id"].to_s == thread.id.to_s}.first.should_not be_nil
+				check_thread_result(nil, thread, threads.select{|t| t["id"] == thread.id.to_s}.first, false, true)
 			end
 
 		end
@@ -56,7 +56,7 @@ describe "app" do
 				get "/api/v1/search/threads", text: random_string
 				last_response.should be_ok
 				threads = parse(last_response.body)['collection']
-				threads.select{|t| t["id"].to_s == thread.id.to_s}.first.should_not be_nil
+				check_thread_result(nil, thread, threads.select{|t| t["id"] == thread.id.to_s}.first, false, true)
 			end
 		end
 	end

spec/api/search_spec.rb

@@ -23,27 +23,27 @@ describe "app" do
         last_response.should be_ok
         threads = parse(last_response.body)['collection']
         threads.length.should == 2
-        threads.select{|t| t["id"] == thread1.id.to_s}.first.should_not be_nil
-        threads.select{|t| t["id"] == thread2.id.to_s}.first.should_not be_nil
+        check_thread_result(nil, thread1, threads.select{|t| t["id"] == thread1.id.to_s}.first, false, true)
+        check_thread_result(nil, thread2, threads.select{|t| t["id"] == thread2.id.to_s}.first, false, true)
 
         get "/api/v1/search/threads", tags: [ai].join(",")
         last_response.should be_ok
         threads = parse(last_response.body)['collection']
         threads.length.should == 2
-        threads.select{|t| t["id"] == thread1.id.to_s}.first.should_not be_nil
-        threads.select{|t| t["id"] == thread2.id.to_s}.first.should_not be_nil
+        check_thread_result(nil, thread1, threads.select{|t| t["id"] == thread1.id.to_s}.first, false, true)
+        check_thread_result(nil, thread2, threads.select{|t| t["id"] == thread2.id.to_s}.first, false, true)
 
         get "/api/v1/search/threads", tags: [ai, random1].join(",")
         last_response.should be_ok
         threads = parse(last_response.body)['collection']
         threads.length.should == 1
-        threads.select{|t| t["id"] == thread1.id.to_s}.first.should_not be_nil
+        check_thread_result(nil, thread1, threads.select{|t| t["id"] == thread1.id.to_s}.first, false, true)
 
         get "/api/v1/search/threads", tags: [random1].join(",")
         last_response.should be_ok
         threads = parse(last_response.body)['collection']
         threads.length.should == 1
-        threads.select{|t| t["id"] == thread1.id.to_s}.first.should_not be_nil
+        check_thread_result(nil, thread1, threads.select{|t| t["id"] == thread1.id.to_s}.first, false, true)
 
         get "/api/v1/search/threads", tags: [random1, random2].join(",")
         last_response.should be_ok

spec/spec_helper.rb

@@ -169,11 +169,14 @@ end
 
 # this method is used to test results produced using the helper function handle_threads_query
 # which is used in multiple areas of the API
-def check_thread_result(user, thread, json_response, check_comments=false)
+def check_thread_result(user, thread, json_response, check_comments=false, is_search=false)
   expected_keys = %w(id title body course_id commentable_id created_at updated_at)
   expected_keys += %w(anonymous anonymous_to_peers at_position_list closed user_id)
   expected_keys += %w(username votes abuse_flaggers tags type group_id pinned)
   expected_keys += %w(comments_count unread_comments_count read endorsed)
+  if is_search
+    expected_keys += %w(highlighted_body highlighted_title)
+  end
   # the "children" key is not always present - depends on the invocation + test use case.
   # exclude it from this check - if check_comments is set, we'll assert against it later
   actual_keys = json_response.keys - ["children"]