Commit 965efede by Toby Lawrence Committed by Robert Raposa

Add the ability to request a thread without its responses.

For cases where we don't need or want the responses on a thread at all,
like getting some metadata about a thread, we should be able to request
it without the responses.  This exists in the ThreadPresenter but didn't
exist as an option that could be passed in the API call itself.

We've added this as an option now -- with_responses -- that defaults to
true, which preserves the existing behavior but does in fact allow
toggling.

This gives us a backwards-compatible upgrade path to allow changes to be
made the LMS so that requests which don't need the responses, or do need
the responses, can be explicit about what they want.
parent f051f495
......@@ -49,7 +49,7 @@ get "#{APIPREFIX}/threads/:thread_id" do |thread_id|
else
resp_limit = nil
end
presenter.to_hash(true, resp_skip, resp_limit, bool_recursive).to_json
presenter.to_hash(bool_with_responses, resp_skip, resp_limit, bool_recursive).to_json
end
put "#{APIPREFIX}/threads/:thread_id" do |thread_id|
......
require 'new_relic/agent/method_tracer'
helpers do
def commentable
@commentable ||= Commentable.find(params[:commentable_id])
end
......@@ -10,7 +9,7 @@ helpers do
raise ArgumentError, t(:user_id_is_required) unless @user || params[:user_id]
@user ||= User.find_by(external_id: params[:user_id])
end
def thread
@thread ||= CommentThread.find(params[:thread_id])
end
......@@ -56,7 +55,7 @@ helpers do
obj.save
obj.reload.to_hash.to_json
end
def un_flag_as_abuse(obj)
raise ArgumentError, t(:user_id_is_required) unless user
if params["all"]
......@@ -66,7 +65,7 @@ helpers do
else
obj.abuse_flaggers.delete user.id
end
obj.save
obj.reload.to_hash.to_json
end
......@@ -78,7 +77,6 @@ helpers do
end
obj.reload.to_hash.to_json
end
def pin(obj)
raise ArgumentError, t(:user_id_is_required) unless user
......@@ -86,16 +84,14 @@ helpers do
obj.save
obj.reload.to_hash.to_json
end
def unpin(obj)
raise ArgumentError, t(:user_id_is_required) unless user
obj.pinned = nil
obj.save
obj.reload.to_hash.to_json
end
end
def value_to_boolean(value)
!!(value.to_s =~ /^true$/i)
end
......@@ -104,6 +100,10 @@ helpers do
value_to_boolean params["recursive"]
end
def bool_with_responses
value_to_boolean params["with_responses"] || "true"
end
def bool_mark_as_read
value_to_boolean params["mark_as_read"]
end
......@@ -142,7 +142,6 @@ helpers do
per_page,
context=:course
)
context_threads = comment_threads.where({:context => context})
if not group_ids.empty?
......@@ -158,7 +157,7 @@ helpers do
comment_ids = Comment.where(:course_id => course_id).
where(:abuse_flaggers.ne => [], :abuse_flaggers.exists => true).
collect{|c| c.comment_thread_id}.uniq
thread_ids = comment_threads.where(:abuse_flaggers.ne => [], :abuse_flaggers.exists => true).
collect{|c| c.id}
......@@ -171,7 +170,7 @@ helpers do
endorsed_thread_ids = Comment.where(:course_id => course_id).
where(:parent_id.exists => false, :endorsed => true).
collect{|c| c.comment_thread_id}.uniq
comment_threads = comment_threads.where({"thread_type" => :question}).nin({"_id" => endorsed_thread_ids})
end
end
......@@ -228,7 +227,7 @@ helpers do
page = [1, page].max
threads = comment_threads.paginate(:page => page, :per_page => per_page).to_a
end
if threads.length == 0
collection = []
else
......@@ -328,11 +327,11 @@ helpers do
current_thread = thread_map[c.comment_thread_id]
#do not include threads or comments who have current or historical abuse flags
if current_thread.abuse_flaggers.to_a.empty? and
current_thread.historical_abuse_flaggers.to_a.empty? and
c.abuse_flaggers.to_a.empty? and
if current_thread.abuse_flaggers.to_a.empty? and
current_thread.historical_abuse_flaggers.to_a.empty? and
c.abuse_flaggers.to_a.empty? and
c.historical_abuse_flaggers.to_a.empty?
user_ids = subscriptions_map[c.comment_thread_id.to_s]
user_ids.each do |u|
if not notification_map.keys.include? u
......@@ -365,7 +364,6 @@ helpers do
end
notification_map.to_json
end
def filter_blocked_content body
......@@ -382,7 +380,7 @@ helpers do
error 503, [msg].to_json
end
end
include ::NewRelic::Agent::MethodTracer
add_method_tracer :user
add_method_tracer :thread
......@@ -390,5 +388,4 @@ helpers do
add_method_tracer :flag_as_abuse
add_method_tracer :un_flag_as_abuse
add_method_tracer :handle_threads_query
end
......@@ -23,7 +23,7 @@ class ThreadPresenter
@is_endorsed = is_endorsed
end
def to_hash with_responses=false, resp_skip=0, resp_limit=nil, recursive=true
def to_hash(with_responses=false, resp_skip=0, resp_limit=nil, recursive=true)
raise ArgumentError unless resp_skip >= 0
raise ArgumentError unless resp_limit.nil? or resp_limit >= 1
h = @thread.to_hash
......
......@@ -513,6 +513,18 @@ describe "app" do
check_thread_result_json(nil, thread, parsed)
end
context 'when requesting the thread for informational purposes' do
subject do
get "/api/v1/threads/#{thread.id}", with_responses: false # we're asking for no responses here.
end
it 'should have no children' do
expect(subject).to be_ok
parsed = parse(subject.body)
expect(parsed).not_to include('children')
end
end
context 'when marking as read' do
subject do
get "/api/v1/threads/#{thread.id}", {:user_id => thread.author.id, :mark_as_read => true}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment