Merge pull request #108 from edx/gprice/thread-type

Add thread type feature

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/comment_threads.rb

@@ -41,8 +41,8 @@ get "#{APIPREFIX}/threads/:thread_id" do |thread_id|
 end
 
 put "#{APIPREFIX}/threads/:thread_id" do |thread_id|
+  filter_blocked_content params["body"]
   thread.update_attributes(params.slice(*%w[title body closed commentable_id group_id]))
-  filter_blocked_content thread
 
   if thread.errors.any?
     error 400, thread.errors.full_messages.to_json
@@ -53,12 +53,12 @@ put "#{APIPREFIX}/threads/:thread_id" do |thread_id|
 end
 
 post "#{APIPREFIX}/threads/:thread_id/comments" do |thread_id|
+  filter_blocked_content params["body"]
   comment = Comment.new(params.slice(*%w[body course_id]))
   comment.anonymous = bool_anonymous || false
   comment.anonymous_to_peers = bool_anonymous_to_peers || false
   comment.author = user
   comment.comment_thread = thread
-  filter_blocked_content comment
   comment.save
   if comment.errors.any?
     error 400, comment.errors.full_messages.to_json

api/commentables.rb

@@ -15,7 +15,9 @@ get "#{APIPREFIX}/:commentable_id/threads" do |commentable_id|
 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
   
@@ -24,7 +26,6 @@ post "#{APIPREFIX}/:commentable_id/threads" do |commentable_id|
   end
   
   thread.author = user
-  filter_blocked_content thread
   thread.save
   if thread.errors.any?
     error 400, thread.errors.full_messages.to_json

api/comments.rb

@@ -3,8 +3,16 @@ get "#{APIPREFIX}/comments/:comment_id" do |comment_id|
 end
 
 put "#{APIPREFIX}/comments/:comment_id" do |comment_id|
-  comment.update_attributes(params.slice(*%w[body endorsed]))
-  filter_blocked_content comment
+  filter_blocked_content params["body"]
+  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
@@ -13,12 +21,12 @@ put "#{APIPREFIX}/comments/:comment_id" do |comment_id|
 end
 
 post "#{APIPREFIX}/comments/:comment_id" do |comment_id|
+  filter_blocked_content params["body"]
   sub_comment = comment.children.new(params.slice(*%w[body course_id]))
   sub_comment.anonymous = bool_anonymous || false
   sub_comment.anonymous_to_peers = bool_anonymous_to_peers || false
   sub_comment.author = user
   sub_comment.comment_thread = comment.comment_thread
-  filter_blocked_content sub_comment
   sub_comment.save
   if sub_comment.errors.any?
     error 400, sub_comment.errors.full_messages.to_json

lib/helpers.rb

@@ -278,9 +278,9 @@ helpers do
 
   end
 
-  def filter_blocked_content c
+  def filter_blocked_content body
     begin
-      normalized_body = c.body.strip.downcase.gsub(/[^a-z ]/, '').gsub(/\s+/, ' ')
+      normalized_body = body.strip.downcase.gsub(/[^a-z ]/, '').gsub(/\s+/, ' ')
       hash = Digest::MD5.hexdigest(normalized_body)
     rescue
       # body was nil, or the hash function failed somehow - never mind

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
-      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
-          break
-        else
-          # try again with one level back in the ancestry til we find the parent
-          ancestry.pop
+  # 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 item.parent_id == ancestry.last["id"]
+            ancestry.last["children"] << item_hash
+            ancestry << item_hash
+            break
+          else
+            ancestry.pop
+            next
+          end
+        end
+        if ancestry.empty? # invalid parent; ignore item
           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]
       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,24 +55,57 @@ 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
         last_response.status.should == 400
         parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
       end
-      it "returns 503 when the post hash is blocked" do
+      it "returns 503 and does not update when the post hash is blocked" do
         comment = Comment.first
+        original_body = comment.body
         put "/api/v1/comments/#{comment.id}", body: "BLOCKED POST", endorsed: true
         last_response.status.should == 503
         parse(last_response.body).first.should == I18n.t(:blocked_content_with_body_hash, :hash => Digest::MD5.hexdigest("blocked post"))
+        comment.reload
+        comment.body.should == original_body
       end
 
       def test_unicode_data(text)
@@ -101,12 +134,13 @@ describe "app" do
         last_response.status.should == 400
         parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
       end
-      it "returns 503 when the post hash is blocked" do
+      it "returns 503 and does not create when the post hash is blocked" do
         comment = Comment.first.to_hash(recursive: true)
         user = User.first
         post "/api/v1/comments/#{comment["id"]}", body: "BLOCKED POST", course_id: "1", user_id: User.first.id
         last_response.status.should == 503
         parse(last_response.body).first.should == I18n.t(:blocked_content_with_body_hash, :hash => Digest::MD5.hexdigest("blocked post"))
+        Comment.where(body: "BLOCKED POST").to_a.should be_empty
       end
 
       def test_unicode_data(text)

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
@@ -480,12 +480,17 @@ describe "app" do
         last_response.status.should == 400
         parse(last_response.body).first.should == I18n.t(:requested_object_not_found)
       end
-      it "returns 503 if the post body has been blocked" do
+      it "returns 503 and does not update if the post body has been blocked" do
         thread = CommentThread.first
+        original_body = thread.body
         put "/api/v1/threads/#{thread.id}", body: "BLOCKED POST", title: "new title", commentable_id: "new_commentable_id"
         last_response.status.should == 503
+        thread.reload
+        thread.body.should == original_body
         put "/api/v1/threads/#{thread.id}", body: "blocked,   post...", title: "new title", commentable_id: "new_commentable_id"
         last_response.status.should == 503
+        thread.reload
+        thread.body.should == original_body
       end
 
       def test_unicode_data(text)
@@ -542,9 +547,10 @@ describe "app" do
         post "/api/v1/threads/#{CommentThread.first.id}/comments", default_params.merge(body: "    \n      \n  ")
         last_response.status.should == 400
       end
-      it "returns 503 when the post body has been blocked" do
+      it "returns 503 and does not create when the post body has been blocked" do
         post "/api/v1/threads/#{CommentThread.first.id}/comments", default_params.merge(body: "BLOCKED POST")
         last_response.status.should == 503
+        Comment.where(body: "BLOCKED POST").to_a.should be_empty
       end
 
       def test_unicode_data(text)

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)
@@ -97,10 +106,11 @@ describe "app" do
         post '/api/v1/question_1/threads', default_params.merge(body: "     \n    \n")
         last_response.status.should == 400
       end
-      it "returns 503 when the post content is blocked" do
+      it "returns 503 and does not create when the post content is blocked" do
         post '/api/v1/question_1/threads', default_params.merge(body: "BLOCKED POST")
         last_response.status.should == 503
         parse(last_response.body).first.should == I18n.t(:blocked_content_with_body_hash, :hash => Digest::MD5.hexdigest("blocked post"))
+        CommentThread.where(body: "BLOCKED POST").to_a.should be_empty
       end
 
       def test_unicode_data(text)

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/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
-    end
-    actual_comment_ids.should == expected_comment_ids
+  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
+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