feat: updated api to allow ordering on endorsed status (#425)

* feat: updated api to allow ordering on endorsed status

* test: fixed test cases

* test: fixed failing test case

api/comment_threads.rb

@@ -35,6 +35,8 @@
     error 404, [t(:requested_object_not_found)].to_json
   end
 
+  merge_question_type_responses = value_to_boolean(params["merge_question_type_responses"])
+
   # user is required to return user-specific fields, such as "read" (even if bool_mark_as_read is False)
   if params["user_id"]
     user = User.only([:id, :username, :read_states]).find_by(external_id: params["user_id"])
@@ -63,7 +65,8 @@
     error 400, [t(:param_exceeds_limit, :param => resp_limit, :limit => size_limit)].to_json
   end
   presenter.to_hash(
-    bool_with_responses, resp_skip, resp_limit, bool_recursive, bool_flagged_comments, bool_reverse_order
+    bool_with_responses, resp_skip, resp_limit, bool_recursive, bool_flagged_comments, bool_reverse_order,
+    merge_question_type_responses
   ).to_json
 end
 

presenters/thread.rb

@@ -35,7 +35,8 @@ def to_hash(
     resp_limit=nil,
     recursive=true,
     flagged_comments=false,
-    reverse_order=false
+    reverse_order=false,
+    merge_question_type_responses=false
   )
     raise ArgumentError unless resp_skip >= 0
     raise ArgumentError unless resp_limit.nil? or resp_limit >= 1
@@ -48,7 +49,8 @@ def to_hash(
     end
     sorting_key_order = reverse_order ? -1 : 1
     if with_responses
-      if @thread.thread_type.discussion? && resp_skip == 0 && resp_limit.nil?
+      if (@thread.thread_type.discussion? || (@thread.thread_type.question? && merge_question_type_responses)) &&
+         resp_skip == 0 && resp_limit.nil?
         if recursive
           content = Comment.where(comment_thread_id: @thread._id).order_by({"sk" => sorting_key_order})
         else
@@ -66,28 +68,41 @@ def to_hash(
         end
         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(
+          if merge_question_type_responses
+            response_info = get_paged_merged_responses(
             @thread._id,
-            endorsed_responses,
-            0,
-            nil,
-            recursive,
-            sorting_key_order
-          )
-          non_endorsed_response_info = get_paged_merged_responses(
-            @thread._id,
-            non_endorsed_responses,
+            responses,
             resp_skip,
             resp_limit,
             recursive,
             sorting_key_order
-          )
-          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"]
-          h["resp_total"] = non_endorsed_response_info["response_count"] + endorsed_response_info["response_count"]
+            )
+            h["children"] = response_info["responses"]
+            h["resp_total"] = response_info["response_count"]
+          else
+            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,
+              recursive,
+              sorting_key_order
+            )
+            non_endorsed_response_info = get_paged_merged_responses(
+              @thread._id,
+              non_endorsed_responses,
+              resp_skip,
+              resp_limit,
+              recursive,
+              sorting_key_order
+            )
+            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"]
+            h["resp_total"] = non_endorsed_response_info["response_count"] + endorsed_response_info["response_count"]
+          end
         when "discussion"
           response_info = get_paged_merged_responses(
             @thread._id,

spec/api/comment_thread_spec.rb

@@ -669,7 +669,7 @@ def thread_result(id, params)
         it "limits responses when no skip/limit params given" do
           @threads.each do |n, thread|
             res = thread_result thread.id, {}
-            check_thread_response_paging_json thread, res, 0, nil, false
+            check_thread_response_paging_json thread, res, 0, nil, false, false
           end
         end
 
@@ -697,6 +697,41 @@ def thread_result(id, params)
       end
     end
 
+    context "response pagination" do
+        def thread_result(id, params)
+          get "/api/v1/threads/#{id}", params
+          expect(last_response).to be_ok
+          parse(last_response.body)
+        end
+
+        it "handles responses when merge_question_type_responses=true" do
+          User.all.delete
+          Content.all.delete
+          @user = create_test_user(999)
+          @threads = {}
+          @comments = {}
+          [201, 10, 3, 2, 1, 0].each do |n|
+            thread_key = "t#{n}"
+            thread = make_thread(@user, thread_key, DFLT_COURSE_ID, "pdq", :question)
+            @threads[n] = thread
+            n.times do |i|
+              # generate n responses in this thread
+              comment_key = "#{thread_key} r#{i}"
+              comment = make_comment(@user, thread, comment_key)
+              2.times do |j|
+                subcomment_key = "#{comment_key} c#{j}"
+                subcomment = make_comment(@user, comment, subcomment_key)
+              end
+              @comments[comment_key] = comment
+            end
+          end
+          @threads.each do |n, thread|
+            res = thread_result thread.id, {:merge_question_type_responses => true}
+            check_thread_response_paging_json thread, res, 0, nil, false, false, true
+          end
+      end
+    end
+
     describe "PUT /api/v1/threads/:thread_id" do
 
       before(:each) { init_without_subscriptions }

spec/presenters/thread_spec.rb

@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe ThreadPresenter do
-  
+
   context "#to_hash" do
     let(:default_resp_limit) { CommentService.config["thread_response_default_size"] }
 
@@ -167,6 +167,15 @@ def random_flag_abuses!(comment)
         end
       end
 
+      it "handles merge_question_type_responses=true" do
+        @threads_with_num_comments.each do |thread, num_comments|
+          is_endorsed = num_comments > 0 && endorse_responses
+          hash = ThreadPresenter.new(thread, @reader, false, num_comments, is_endorsed, nil).to_hash(true, 0, default_resp_limit, true, false, true, true)
+          check_thread_result(@reader, thread, hash)
+          check_thread_response_paging(thread, hash, 0, default_resp_limit, false, false, true, true)
+        end
+      end
+
       it "handles reversed_order and recursive with skip and limit" do
         @threads_with_num_comments.each do |thread, num_comments|
           is_endorsed = num_comments > 0 && endorse_responses

spec/spec_helper.rb

@@ -254,12 +254,12 @@ def check_unread_thread_result_json(thread, json_response)
   check_thread_result(nil, thread, json_response, true)
 end
 
-def check_thread_response_paging(thread, hash, resp_skip=0, resp_limit=nil, is_json=false, recursive=false, reverse_order=false)
-  case thread.thread_type
-    when "discussion"
-      check_discussion_response_paging(thread, hash, resp_skip, resp_limit, is_json, recursive, reverse_order)
-    when "question"
-      check_question_response_paging(thread, hash, resp_skip, resp_limit, is_json, recursive, reverse_order)
+def check_thread_response_paging(thread, hash, resp_skip=0, resp_limit=nil, is_json=false, recursive=false, reverse_order=false, merge_question_type_responses=false)
+
+  if (thread.thread_type == "discussion" || (thread.thread_type == "question" && merge_question_type_responses))
+     check_discussion_response_paging(thread, hash, resp_skip, resp_limit, is_json, recursive, reverse_order)
+  else
+    check_question_response_paging(thread, hash, resp_skip, resp_limit, is_json, recursive, reverse_order)
   end
 end
 
@@ -333,8 +333,8 @@ def check_question_response_paging(thread, hash, resp_skip=0, resp_limit=nil, is
   end
 end
 
-def check_thread_response_paging_json(thread, hash, resp_skip=0, resp_limit=nil, recursive=false, reverse_order=false)
-  check_thread_response_paging(thread, hash, resp_skip, resp_limit, true, recursive, reverse_order)
+def check_thread_response_paging_json(thread, hash, resp_skip=0, resp_limit=nil, recursive=false, reverse_order=false, merge_question_type_responses=false)
+  check_thread_response_paging(thread, hash, resp_skip, resp_limit, true, recursive, reverse_order, merge_question_type_responses)
 end
 
 # general purpose factory helpers