Add thread type feature

There are two thread types, discussion and question. Discussions have
the same behavior that current threads do. When a question is returned
from the API, it contains separate fields for its endorsed and non-
endorsed entries.

Co-authored-by: Jim Abramson <jsa@edx.org>

Gemfile

@@ -24,6 +24,7 @@ gem 'bson_ext'
 gem 'delayed_job'
 gem 'delayed_job_mongoid', :git => 'https://github.com/dementrock/delayed_job_mongoid.git'
 
+gem "enumerize", "~>0.8.0"
 gem 'mongoid-tree', :git => 'https://github.com/dementrock/mongoid-tree.git'
 gem 'voteable_mongo', :git => 'https://github.com/dementrock/voteable_mongo.git'
 gem 'mongoid_magic_counter_cache', :git => 'https://github.com/dementrock/mongoid-magic-counter-cache.git'

Gemfile.lock

@@ -76,6 +76,8 @@ GEM
     delayed_job (3.0.3)
       activesupport (~> 3.0)
     diff-lcs (1.1.3)
+    enumerize (0.8.0)
+      activesupport (>= 3.2)
     erubis (2.7.0)
     faker (1.0.1)
       i18n (~> 0.4)
@@ -181,6 +183,7 @@ DEPENDENCIES
   database_cleaner
   delayed_job
   delayed_job_mongoid!
+  enumerize (~> 0.8.0)
   faker
   guard
   guard-unicorn

api/commentables.rb

@@ -17,6 +17,7 @@ end
 post "#{APIPREFIX}/:commentable_id/threads" do |commentable_id|
   filter_blocked_content params["body"]
   thread = CommentThread.new(params.slice(*%w[title body course_id ]).merge(commentable_id: commentable_id))
+  thread.thread_type = params["thread_type"] || :discussion
   thread.anonymous = bool_anonymous || false
   thread.anonymous_to_peers = bool_anonymous_to_peers || false
   

api/comments.rb

@@ -4,7 +4,15 @@ end
 
 put "#{APIPREFIX}/comments/:comment_id" do |comment_id|
   filter_blocked_content params["body"]
-  comment.update_attributes(params.slice(*%w[body endorsed]))
+  updated_content = params.slice(*%w[body endorsed])
+  if params.has_key?("endorsed")
+    new_endorsed_val = Boolean.mongoize(params["endorsed"])
+    if new_endorsed_val != comment.endorsed
+      endorsement = {:user_id => params["endorsement_user_id"], :time => DateTime.now}
+      updated_content["endorsement"] = new_endorsed_val ? endorsement : nil
+    end
+  end
+  comment.update_attributes(updated_content)
   if comment.errors.any?
     error 400, comment.errors.full_messages.to_json
   else

models/comment.rb

@@ -11,6 +11,7 @@ class Comment < Content
   field :course_id, type: String
   field :body, type: String
   field :endorsed, type: Boolean, default: false
+  field :endorsement, type: Hash
   field :anonymous, type: Boolean, default: false
   field :anonymous_to_peers, type: Boolean, default: false
   field :at_position_list, type: Array, default: []
@@ -46,7 +47,7 @@ class Comment < Content
   belongs_to :comment_thread, index: true
   belongs_to :author, class_name: "User", index: true
 
-  attr_accessible :body, :course_id, :anonymous, :anonymous_to_peers, :endorsed
+  attr_accessible :body, :course_id, :anonymous, :anonymous_to_peers, :endorsed, :endorsement
 
   validates_presence_of :comment_thread, autosave: false
   validates_presence_of :body
@@ -94,7 +95,7 @@ class Comment < Content
       subtree_hash = subtree(sort: sort_by_parent_and_time)
       self.class.hash_tree(subtree_hash).first
     else
-      as_document.slice(*%w[body course_id endorsed anonymous anonymous_to_peers created_at updated_at at_position_list])
+      as_document.slice(*%w[body course_id endorsed endorsement anonymous anonymous_to_peers created_at updated_at at_position_list])
                  .merge("id" => _id)
                  .merge("user_id" => author_id)
                  .merge("username" => author_username) 

models/comment_thread.rb

@@ -5,9 +5,12 @@ require_relative 'content'
 class CommentThread < Content
 
   include Mongoid::Timestamps
+  extend Enumerize
 
   voteable self, :up => +1, :down => -1
 
+  field :thread_type, type: String, default: :discussion
+  enumerize :thread_type, in: [:question, :discussion]
   field :comment_count, type: Integer, default: 0
   field :title, type: String
   field :body, type: String
@@ -52,6 +55,7 @@ class CommentThread < Content
 
   attr_accessible :title, :body, :course_id, :commentable_id, :anonymous, :anonymous_to_peers, :closed
 
+  validates_presence_of :thread_type
   validates_presence_of :title
   validates_presence_of :body
   validates_presence_of :course_id # do we really need this?
@@ -113,33 +117,7 @@ class CommentThread < Content
   end
 
   def to_hash(params={})
-
-    # to_hash returns the following model for each thread
-    #  title body course_id anonymous anonymous_to_peers commentable_id
-    #  created_at updated_at at_position_list closed
-    #    (all the above direct from the original document)
-    #  id
-    #    from doc._id
-    #  user_id
-    #    from doc.author_id
-    #  username
-    #    from doc.author_username
-    #  votes
-    #    from subdocument votes - {count, up_count, down_count, point}  
-    #  abuse_flaggers
-    #    from original document 
-    #  tags
-    #    deprecated - empty array
-    #  type
-    #    hardcoded "thread"
-    #  group_id
-    #    from orig doc
-    #  pinned
-    #    from orig doc
-    #  comments_count
-    #    count across all comments
-
-    as_document.slice(*%w[title body course_id anonymous anonymous_to_peers commentable_id created_at updated_at at_position_list closed])
+    as_document.slice(*%w[thread_type title body course_id anonymous anonymous_to_peers commentable_id created_at updated_at at_position_list closed])
                      .merge("id" => _id, "user_id" => author_id,
                             "username" => author_username,
                             "votes" => votes.slice(*%w[count up_count down_count point]),

presenters/thread.rb

@@ -31,62 +31,86 @@ class ThreadPresenter
     h["unread_comments_count"] = @unread_count
     h["endorsed"] = @is_endorsed || false
     if with_responses
-      unless resp_skip == 0 && resp_limit.nil?
-        # need to find responses first, set the window accordingly, then fetch the comments
-        # bypass mongoid/odm, to get just the response ids we need as directly as possible
-        responses = Content.collection.find({"comment_thread_id" => @thread._id, "parent_id" => {"$exists" => false}})
-        responses = responses.sort({"sk" => 1})
-        all_response_ids = responses.select({"_id" => 1}).to_a.map{|doc| doc["_id"] }
-        response_ids = (resp_limit.nil? ? all_response_ids[resp_skip..-1] : (all_response_ids[resp_skip,resp_limit])) || []
-        # now use the odm to fetch the desired responses and their comments
-        content = Content.where({"parent_id" => {"$in" => response_ids}}).to_a + Content.where({"_id" => {"$in" => response_ids}}).to_a
-        content.sort!{|a,b| a.sk <=> b.sk }
-        response_total = all_response_ids.length
+      if @thread.thread_type.discussion? && resp_skip == 0 && resp_limit.nil?
+        content = Comment.where(comment_thread_id: @thread._id).order_by({"sk" => 1})
+        h["children"] = merge_response_content(content)
+        h["resp_total"] = content.to_a.select{|d| d.depth == 0 }.length
       else
-        content = Content.where({"comment_thread_id" => @thread._id}).order_by({"sk"=> 1})
-        response_total = content.to_a.select{|d| d.depth == 0 }.length
+        responses = Content.where(comment_thread_id: @thread._id).exists(parent_id: false)
+        case @thread.thread_type
+        when "question"
+          endorsed_responses = responses.where(endorsed: true)
+          non_endorsed_responses = responses.where(endorsed: false)
+          endorsed_response_info = get_paged_merged_responses(@thread._id, endorsed_responses, 0, nil)
+          non_endorsed_response_info = get_paged_merged_responses(
+            @thread._id,
+            non_endorsed_responses,
+            resp_skip,
+            resp_limit
+          )
+          h["endorsed_responses"] = endorsed_response_info["responses"]
+          h["non_endorsed_responses"] = non_endorsed_response_info["responses"]
+          h["non_endorsed_resp_total"] = non_endorsed_response_info["response_count"]
+        when "discussion"
+          response_info = get_paged_merged_responses(@thread._id, responses, resp_skip, resp_limit)
+          h["children"] = response_info["responses"]
+          h["resp_total"] = response_info["response_count"]
+        end
       end
-      h = merge_comments_recursive(h, content)
       h["resp_skip"] = resp_skip
       h["resp_limit"] = resp_limit
-      h["resp_total"] = response_total
     end
     h
   end
 
-  def merge_comments_recursive thread_hash, comments
-    thread_id = thread_hash["id"]
-    root = thread_hash = thread_hash.merge("children" => [])
-    ancestry = [thread_hash]
-    # weave the fetched comments into a single hierarchical doc
-    comments.each do | comment |
-      thread_hash = comment.to_hash.merge("children" => [])
-      parent_id = comment.parent_id || thread_id
-      found_parent = false
+  # Given a Mongoid object representing responses, apply pagination and return
+  # a hash containing the following:
+  #   responses
+  #     An array of hashes representing the page of responses (including
+  #     children)
+  #   response_count
+  #     The total number of responses
+  def get_paged_merged_responses(thread_id, responses, skip, limit)
+    response_ids = responses.only(:_id).sort({"sk" => 1}).to_a.map{|doc| doc["_id"]}
+    paged_response_ids = limit.nil? ? response_ids.drop(skip) : response_ids.drop(skip).take(limit)
+    content = Comment.where(comment_thread_id: thread_id).
+      or({:parent_id => {"$in" => paged_response_ids}}, {:id => {"$in" => paged_response_ids}}).
+      sort({"sk" => 1})
+    {"responses" => merge_response_content(content), "response_count" => response_ids.length}
+  end
+
+  # Takes content output from Mongoid in a depth-first traversal order and
+  # returns an array of first-level response hashes with content represented
+  # hierarchically, with a comment's list of children in the key "children".
+  def merge_response_content(content)
+    top_level = []
+    ancestry = []
+    content.each do |item|
+      item_hash = item.to_hash.merge("children" => [])
+      if item.parent_id.nil?
+        top_level << item_hash
+        ancestry = [item_hash]
+      else
         while ancestry.length > 0 do
-        if parent_id == ancestry.last["id"] then
-          # found the children collection to which this comment belongs
-          ancestry.last["children"] << thread_hash
-          ancestry << thread_hash
-          found_parent = true
+          if item.parent_id == ancestry.last["id"]
+            ancestry.last["children"] << item_hash
+            ancestry << item_hash
             break
           else
-          # try again with one level back in the ancestry til we find the parent
             ancestry.pop
             next
           end
         end
-      if not found_parent
-        # if we arrive here, it means a parent_id somewhere in the result set
-        # is pointing to an invalid place.  reset the ancestry search path.
-        ancestry = [root]
+        if ancestry.empty? # invalid parent; ignore item
+          next
+        end
       end
     end
-    ancestry.first
+    top_level
   end
 
   include ::NewRelic::Agent::MethodTracer
   add_method_tracer :to_hash
-  add_method_tracer :merge_comments_recursive
+  add_method_tracer :merge_response_content
 
 end

scripts/db/migrate-007-thread-type.js

+print ("Adding thread_type to all comment threads where it does not yet exist\n");
+db.contents.update(
+    {_type: "CommentThread", thread_type: {$exists: false}},
+    {$set: {thread_type: "discussion"}},
+    {multi: true}
+);
+printjson (db.runCommand({ getLastError: 1, w: "majority", wtimeout: 5000 } ));

scripts/db/revert-migrate-007-thread-type.js

+print ("Removing thread_type from all comment threads\n");
+db.contents.update(
+    {_type: "CommentThread"},
+    {$unset: {thread_type: ""}},
+    {multi: true}
+);
+printjson (db.runCommand({ getLastError: 1, w: "majority", wtimeout: 5000 } ));

spec/api/comment_spec.rb

@@ -55,13 +55,43 @@ describe "app" do
       include_examples "unicode data"
     end
     describe "PUT /api/v1/comments/:comment_id" do
-      it "update information of the comment" do
+      def test_update_endorsed(true_val, false_val)
+        comment = Comment.first
+        before = DateTime.now
+        put "/api/v1/comments/#{comment.id}", endorsed: true_val, endorsement_user_id: "#{User.first.id}"
+        after = DateTime.now
+        last_response.should be_ok
+        comment.reload
+        comment.endorsed.should == true
+        comment.endorsement.should_not be_nil
+        comment.endorsement["user_id"].should == "#{User.first.id}"
+        comment.endorsement["time"].should be_between(before, after)
+        put "/api/v1/comments/#{comment.id}", endorsed: false_val
+        last_response.should be_ok
+        comment.reload
+        comment.endorsed.should == false
+        comment.endorsement.should be_nil
+      end
+      it "updates endorsed correctly" do
+        test_update_endorsed(true, false)
+      end
+      it "updates endorsed correctly with Pythonic values" do
+        test_update_endorsed("True", "False")
+      end
+      it "updates body correctly" do
+        comment = Comment.first
+        put "/api/v1/comments/#{comment.id}", body: "new body"
+        last_response.should be_ok
+        comment.reload
+        comment.body.should == "new body"
+      end
+      it "can update endorsed and body simultaneously" do
         comment = Comment.first
         put "/api/v1/comments/#{comment.id}", body: "new body", endorsed: true
         last_response.should be_ok
-        new_comment = Comment.find(comment.id)
-        new_comment.body.should == "new body"
-        new_comment.endorsed.should == true
+        comment.reload
+        comment.body.should == "new body"
+        comment.endorsed.should == true
       end
       it "returns 400 when the comment does not exist" do
         put "/api/v1/comments/does_not_exist", body: "new body", endorsed: true

spec/api/comment_thread_spec.rb

@@ -335,7 +335,7 @@ describe "app" do
         check_thread_result_json(nil, thread, response_thread)
       end
 
-      it "computes endorsed? correctly" do
+      it "computes endorsed correctly" do
         thread = CommentThread.first
         comment = thread.root_comments[1]
         comment.endorsed = true

spec/api/commentable_spec.rb

@@ -62,6 +62,15 @@ describe "app" do
         CommentThread.count.should == old_count + 1
         CommentThread.where(title: "Interesting question").first.should_not be_nil
       end
+      CommentThread.thread_type.values.each do |thread_type|
+        it "can create a #{thread_type} thread" do
+          old_count = CommentThread.where(thread_type: thread_type).count
+          post "/api/v1/question_1/threads", default_params.merge(thread_type: thread_type.to_s)
+          last_response.should be_ok
+          parse(last_response.body)["thread_type"].should == thread_type.to_s
+          CommentThread.where(thread_type: thread_type).count.should == old_count + 1
+        end
+      end
       it "allows anonymous thread" do
         old_count = CommentThread.count
         post '/api/v1/question_1/threads', default_params.merge(anonymous: true)

spec/api/notifications_spec.rb

@@ -15,6 +15,7 @@ describe "app" do
         commentable = Commentable.new("question_1")
         random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
         thread = CommentThread.new(title: "Test title", body: "elephant otter", course_id: "1", commentable_id: commentable.id, comments_text_dummy: random_string)
+        thread.thread_type = :discussion
         thread.author = user
         thread.save!
 
@@ -46,6 +47,7 @@ describe "app" do
         random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
 
         thread = CommentThread.new(title: "Test title", body: "elephant otter", course_id: "1", commentable_id: commentable.id, comments_text_dummy: random_string)
+        thread.thread_type = :discussion
         thread.author = user
         thread.save!
 

spec/api/query_spec.rb

@@ -13,6 +13,7 @@ describe "app" do
         random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
 
         thread = CommentThread.new(title: "Test title", body: random_string, course_id: "1", commentable_id: commentable.id)
+        thread.thread_type = :discussion
         thread.author = author
         thread.save!
 
@@ -35,6 +36,7 @@ describe "app" do
         random_string = (0...8).map{ ('a'..'z').to_a[rand(26)] }.join
 
         thread = CommentThread.new(title: "Test title", body: "elephant otter", course_id: "1", commentable_id: commentable.id)
+        thread.thread_type = :discussion
         thread.author = author
         thread.save!
 

spec/models/comment_thread_spec.rb

@@ -6,31 +6,6 @@ describe CommentThread do
     create_test_user(42)
   end
 
-  context "endorsed?" do
-
-    it "knows if it is #endorsed?" do
-      thread = CommentThread.new
-      criteria = build_criteria(thread, :exists? => true)
-      thread.endorsed?.should be_true
-    end
-
-    it "knows when it is not #endorsed?" do
-      thread = CommentThread.new
-      criteria = build_criteria(thread, :exists? => false)
-      thread.endorsed?.should be_false
-    end
-
-    def build_criteria(thread, options)
-      double("criteria").tap do |criteria|
-        comments = double("relation")
-        comments.stub(:where).with(endorsed: true).and_return(criteria)
-        thread.stub(:comments).and_return(comments)
-        criteria.stub(options)
-      end
-    end
-  end
-
-
   context "sorting" do
 
     before (:each) do
@@ -42,6 +17,7 @@ describe CommentThread do
       author = create_test_user('billy')
 
       thread = CommentThread.new(title: "test case", body: "testing 123", course_id: "foo", commentable_id: "bar")
+      thread.thread_type = :discussion
       thread.author = author
       thread.save!
 

spec/presenters/thread_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe ThreadPresenter do
   
   context "#to_hash" do
-
+    shared_examples "to_hash arguments" do |thread_type, endorse_responses|
       before(:each) do
         User.all.delete
         Content.all.delete
@@ -13,20 +13,23 @@ describe ThreadPresenter do
         @thread_no_responses = make_thread(
           create_test_user('author1'),
           'thread with no responses',
-        course_id, commentable_id
+          course_id, commentable_id,
+          thread_type
         )
 
         @thread_one_empty_response = make_thread(
           create_test_user('author2'),
           'thread with one response',
-        course_id, commentable_id
+          course_id, commentable_id,
+          thread_type
         )
         make_comment(create_test_user('author3'), @thread_one_empty_response, 'empty response')
 
         @thread_one_response = make_thread(
           create_test_user('author4'),
           'thread with one response and some comments',
-        course_id, commentable_id
+          course_id, commentable_id,
+          thread_type
         )
         resp = make_comment(
           create_test_user('author5'),
@@ -40,7 +43,8 @@ describe ThreadPresenter do
         @thread_ten_responses = make_thread(
           create_test_user('author9'),
           'thread with ten responses',
-        course_id, commentable_id
+          course_id, commentable_id,
+          thread_type
         )
         (1..10).each do |n|
           resp = make_comment(create_test_user("author#{n+10}"), @thread_ten_responses, "response #{n}")
@@ -49,6 +53,20 @@ describe ThreadPresenter do
           end
         end
 
+        if endorse_responses
+          [
+            @thread_one_empty_response.comments.first,
+            @thread_one_response.comments.first,
+            @thread_ten_responses.comments[0],
+            @thread_ten_responses.comments[2],
+            @thread_ten_responses.comments[6]
+          ].each do |response|
+            response.endorsed = true
+            response.endorsement = {:user_id => "1", :time => DateTime.now}
+            response.save!
+          end
+        end
+
         @threads_with_num_comments = [
           [@thread_no_responses, 0],
           [@thread_one_empty_response, 1],
@@ -61,7 +79,8 @@ describe ThreadPresenter do
 
       it "handles with_responses=false" do
         @threads_with_num_comments.each do |thread, num_comments|
-        hash = ThreadPresenter.new(thread, @reader, false, num_comments, false).to_hash
+          is_endorsed = num_comments > 0 && endorse_responses
+          hash = ThreadPresenter.new(thread, @reader, false, num_comments, is_endorsed).to_hash
           check_thread_result(@reader, thread, hash)
           ['children', 'resp_skip', 'resp_limit', 'resp_total'].each {|k| (hash.has_key? k).should be_false }
         end
@@ -69,7 +88,8 @@ describe ThreadPresenter do
 
       it "handles with_responses=true" do
         @threads_with_num_comments.each do |thread, num_comments|
-        hash = ThreadPresenter.new(thread, @reader, false, num_comments, false).to_hash true
+          is_endorsed = num_comments > 0 && endorse_responses
+          hash = ThreadPresenter.new(thread, @reader, false, num_comments, is_endorsed).to_hash true
           check_thread_result(@reader, thread, hash)
           check_thread_response_paging(thread, hash)
         end
@@ -77,8 +97,9 @@ describe ThreadPresenter do
 
       it "handles skip with no limit" do
         @threads_with_num_comments.each do |thread, num_comments|
+          is_endorsed = num_comments > 0 && endorse_responses
           [0, 1, 2, 9, 10, 11, 1000].each do |skip|
-          hash = ThreadPresenter.new(thread, @reader, false, num_comments, false).to_hash true, skip
+            hash = ThreadPresenter.new(thread, @reader, false, num_comments, is_endorsed).to_hash true, skip
             check_thread_result(@reader, thread, hash)
             check_thread_response_paging(thread, hash, skip)
           end
@@ -87,9 +108,10 @@ describe ThreadPresenter do
 
       it "handles skip and limit" do
         @threads_with_num_comments.each do |thread, num_comments|
+          is_endorsed = num_comments > 0 && endorse_responses
           [1, 2, 3, 9, 10, 11, 1000].each do |limit|
             [0, 1, 2, 9, 10, 11, 1000].each do |skip|
-            hash = ThreadPresenter.new(thread, @reader, false, num_comments, false).to_hash true, skip, limit
+              hash = ThreadPresenter.new(thread, @reader, false, num_comments, is_endorsed).to_hash true, skip, limit
               check_thread_result(@reader, thread, hash)
               check_thread_response_paging(thread, hash, skip, limit)
             end
@@ -99,41 +121,27 @@ describe ThreadPresenter do
 
       it "fails with invalid arguments" do
         @threads_with_num_comments.each do |thread, num_comments|
-        expect{ThreadPresenter.new(thread, @reader, false, num_comments, false).to_hash true, -1, nil}.to raise_error(ArgumentError)
+          is_endorsed = num_comments > 0 && endorse_responses
+          expect{ThreadPresenter.new(thread, @reader, false, num_comments, is_endorsed).to_hash(true, -1, nil)}.to raise_error(ArgumentError)
           [-1, 0].each do |limit|
-          expect{ThreadPresenter.new(thread, @reader, false, num_comments, false).to_hash true, 0, limit}.to raise_error(ArgumentError)
+            expect{ThreadPresenter.new(thread, @reader, false, num_comments, is_endorsed).to_hash(true, 0, limit)}.to raise_error(ArgumentError)
           end
         end
       end
-
     end
 
-  context "#merge_comments_recursive" do
-
-    before(:each) { @cid_seq = 10 }
-
-    def stub_each_from_array(obj, ary)
-      stub = obj.stub(:each)
-      ary.each {|v| stub = stub.and_yield(v)}
-      obj
+    [:discussion, :question].each do |thread_type|
+      [false, true].each do |endorsed_responses|
+        context "for a #{thread_type} thread #{endorsed_responses ? "with" : "without"} endorsed responses" do
+          include_examples "to_hash arguments", thread_type, endorsed_responses
+        end
+      end
+    end
   end
 
-    def set_comment_results(thread, ary)
-      # example usage:
-      # c0 = make_comment()
-      # c00 = make_comment(c0)
-      # c01 = make_comment(c0)
-      # c010 = make_comment(c01)
-      # set_comment_results(thread, [c0, c00, c01, c010])
+  context "#merge_response_content" do
 
-      # avoids an unrelated expecation error
-      thread.stub(:endorsed?).and_return(true)
-      rs = stub_each_from_array(double("rs"), ary)
-      criteria = double("criteria")
-      criteria.stub(:order_by).and_return(rs)
-      # stub Content, not Comment, because that's the model we will be querying against
-      Content.stub(:where).with({"comment_thread_id" => thread.id}).and_return(criteria)
-    end
+    before(:each) { @cid_seq = 10 }
 
     def make_comment(parent=nil)
       c = Comment.new
@@ -150,14 +158,14 @@ describe ThreadPresenter do
       c010 = make_comment(c01)
 
       pres = ThreadPresenter.new(nil, nil, nil, nil, nil)
-      h = pres.merge_comments_recursive({}, [c0, c00, c01, c010])
-      h["children"].size.should == 1 # c0
-      h["children"][0]["id"].should == c0.id
-      h["children"][0]["children"].size.should == 2 # c00, c01
-      h["children"][0]["children"][0]["id"].should == c00.id
-      h["children"][0]["children"][1]["id"].should == c01.id
-      h["children"][0]["children"][1]["children"].size.should == 1 # c010
-      h["children"][0]["children"][1]["children"][0]["id"].should == c010.id
+      responses = pres.merge_response_content([c0, c00, c01, c010])
+      responses.size.should == 1 # c0
+      responses[0]["id"].should == c0.id
+      responses[0]["children"].size.should == 2 # c00, c01
+      responses[0]["children"][0]["id"].should == c00.id
+      responses[0]["children"][1]["id"].should == c01.id
+      responses[0]["children"][1]["children"].size.should == 1 # c010
+      responses[0]["children"][1]["children"][0]["id"].should == c010.id
     end
 
     it "handles orphaned child comments gracefully" do
@@ -172,11 +180,11 @@ describe ThreadPresenter do
       # be silently skipped over.
 
       pres = ThreadPresenter.new(nil, nil, nil, nil, nil)
-      h = pres.merge_comments_recursive({}, [c00, c000, c1, c10, c111])
-      h["children"].size.should == 1 # c1
-      h["children"][0]["id"].should == c1.id
-      h["children"][0]["children"].size.should == 1 # c10
-      h["children"][0]["children"][0]["id"].should == c10.id
+      responses = pres.merge_response_content([c00, c000, c1, c10, c111])
+      responses.size.should == 1 # c1
+      responses[0]["id"].should == c1.id
+      responses[0]["children"].size.should == 1 # c10
+      responses[0]["children"][0]["id"].should == c10.id
     end
   end
 end

spec/spec_helper.rb

@@ -85,6 +85,7 @@ def init_without_subscriptions
   user = users.first
 
   thread = CommentThread.new(title: "I can't solve this problem", body: "can anyone help me?", course_id: "1", commentable_id: commentable.id)
+  thread.thread_type = :discussion
   thread.author = user
   thread.save!
   user.subscribe(thread)
@@ -110,6 +111,7 @@ def init_without_subscriptions
   comment1.save!
 
   thread = CommentThread.new(title: "This problem is wrong", body: "it is unsolvable", course_id: "2", commentable_id: commentable.id)
+  thread.thread_type = :discussion
   thread.author = user
   thread.save!
   user.subscribe(thread)
@@ -130,6 +132,7 @@ def init_without_subscriptions
   comment1.save!
 
   thread = CommentThread.new(title: "I don't know what to say", body: "lol", course_id: "2", commentable_id: "something else")
+  thread.thread_type = :discussion
   thread.author = users[1]
   thread.save!
 
@@ -202,12 +205,15 @@ 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, hash, is_json=false)
-  expected_keys = %w(id title body course_id commentable_id created_at updated_at)
+  expected_keys = %w(id thread_type 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)
   # these keys are checked separately, when desired, using check_thread_response_paging.
-  actual_keys = hash.keys - ["children", "resp_skip", "resp_limit", "resp_total"]
+  actual_keys = hash.keys - [
+    "children", "endorsed_responses", "non_endorsed_responses", "resp_skip",
+    "resp_limit", "resp_total", "non_endorsed_resp_total"
+  ]
   actual_keys.sort.should == expected_keys.sort
 
   hash["title"].should == thread.title
@@ -269,31 +275,64 @@ def check_thread_result_json(user, thread, json_response)
 end
 
 def check_thread_response_paging(thread, hash, resp_skip=0, resp_limit=nil, is_json=false)
+  case thread.thread_type
+  when "discussion"
+    check_discussion_response_paging(thread, hash, resp_skip, resp_limit, is_json)
+  when "question"
+    check_question_response_paging(thread, hash, resp_skip, resp_limit, is_json)
+  end
+end
+
+def check_comment(comment, hash, is_json)
+  hash["id"].should == (is_json ? comment.id.to_s : comment.id) # Convert from ObjectId if necessary
+  hash["body"].should == comment.body
+  hash["user_id"].should == comment.author_id
+  hash["username"].should == comment.author_username
+  hash["endorsed"].should == comment.endorsed
+  hash["endorsement"].should == comment.endorsement
+  children = Comment.where({"parent_id" => comment.id}).sort({"sk" => 1}).to_a
+  hash["children"].length.should == children.length
+  hash["children"].each_with_index do |child_hash, i|
+    check_comment(children[i], child_hash, is_json)
+  end
+end
+
+def check_discussion_response_paging(thread, hash, resp_skip=0, resp_limit=nil, is_json=false)
   all_responses = thread.root_comments.sort({"sk" => 1}).to_a
   total_responses = all_responses.length
   hash["resp_total"].should == total_responses
-  expected_response_slice = (resp_skip..(resp_limit.nil? ? total_responses : [total_responses, resp_skip + resp_limit].min)-1).to_a
-  expected_response_ids = expected_response_slice.map{|i| all_responses[i]["_id"] }
-  expected_response_ids.map!{|id| id.to_s } if is_json # otherwise they are BSON ObjectIds
-  actual_response_ids = []
-  hash["children"].each_with_index do |response, i|
-    actual_response_ids << response["id"]
-    response["body"].should == all_responses[expected_response_slice[i]].body
-    response["user_id"].should == all_responses[expected_response_slice[i]].author_id
-    response["username"].should == all_responses[expected_response_slice[i]].author_username
-    comments = Comment.where({"parent_id" => response["id"]}).sort({"sk" => 1}).to_a
-    expected_comment_ids = comments.map{|doc| doc["_id"] }
-    expected_comment_ids.map!{|id| id.to_s } if is_json # otherwise they are BSON ObjectIds
-    actual_comment_ids = []
-    response["children"].each_with_index do |comment, j|
-      actual_comment_ids << comment["id"]
-      comment["body"].should == comments[j].body
-      comment["user_id"].should == comments[j].author_id
-      comment["username"].should == comments[j].author_username
+  expected_responses = resp_limit.nil? ?
+    all_responses.drop(resp_skip) :
+    all_responses.drop(resp_skip).take(resp_limit)
+  hash["children"].length.should == expected_responses.length
+  hash["children"].each_with_index do |response_hash, i|
+    check_comment(expected_responses[i], response_hash, is_json)
+  end
+  hash["resp_skip"].to_i.should == resp_skip
+  if resp_limit.nil?
+    hash["resp_limit"].should be_nil
+  else
+    hash["resp_limit"].to_i.should == resp_limit
   end
-    actual_comment_ids.should == expected_comment_ids
+end
+
+def check_question_response_paging(thread, hash, resp_skip=0, resp_limit=nil, is_json=false)
+  all_responses = thread.root_comments.sort({"sk" => 1}).to_a
+  endorsed_responses, non_endorsed_responses = all_responses.partition { |resp| resp.endorsed }
+
+  hash["endorsed_responses"].length.should == endorsed_responses.length
+  hash["endorsed_responses"].each_with_index do |response_hash, i|
+    check_comment(endorsed_responses[i], response_hash, is_json)
+  end
+
+  hash["non_endorsed_resp_total"] == non_endorsed_responses.length
+  expected_non_endorsed_responses = resp_limit.nil? ?
+    non_endorsed_responses.drop(resp_skip) :
+    non_endorsed_responses.drop(resp_skip).take(resp_limit)
+  hash["non_endorsed_responses"].length.should == expected_non_endorsed_responses.length
+  hash["non_endorsed_responses"].each_with_index do |response_hash, i|
+    check_comment(expected_non_endorsed_responses[i], response_hash, is_json)
   end
-  actual_response_ids.should == expected_response_ids
   hash["resp_skip"].to_i.should == resp_skip
   if resp_limit.nil?
     hash["resp_limit"].should be_nil
@@ -307,8 +346,9 @@ def check_thread_response_paging_json(thread, hash, resp_skip=0, resp_limit=nil)
 end
 
 # general purpose factory helpers
-def make_thread(author, text, course_id, commentable_id)
+def make_thread(author, text, course_id, commentable_id, thread_type=:discussion)
   thread = CommentThread.new(title: text, body: text, course_id: course_id, commentable_id: commentable_id)
+  thread.thread_type = thread_type
   thread.author = author
   thread.save!
   thread