diff --git a/api/comment_threads.rb b/api/comment_threads.rb index ac9feaeddf..690bb7fefb 100644 --- a/api/comment_threads.rb +++ b/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 diff --git a/presenters/thread.rb b/presenters/thread.rb index 538e4cf5ef..37f8f7c9f9 100644 --- a/presenters/thread.rb +++ b/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, diff --git a/spec/api/comment_thread_spec.rb b/spec/api/comment_thread_spec.rb index 17137be5a5..daf99af631 100644 --- a/spec/api/comment_thread_spec.rb +++ b/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 } diff --git a/spec/presenters/thread_spec.rb b/spec/presenters/thread_spec.rb index 1e7d5962fe..10bc199541 100644 --- a/spec/presenters/thread_spec.rb +++ b/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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index fbf43dd582..12042a10b8 100644 --- a/spec/spec_helper.rb +++ b/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