Commit 702dd93f by Ben McMorran

TNL-1943 Add context field to CommentThread

parent 8d83377b
...@@ -20,3 +20,4 @@ Sarina Canelake <sarina@edx.org> ...@@ -20,3 +20,4 @@ Sarina Canelake <sarina@edx.org>
Alexandre Dubus <alexandre.dubus@inria.fr> Alexandre Dubus <alexandre.dubus@inria.fr>
Alan Boudreault <alan@alanb.ca> Alan Boudreault <alan@alanb.ca>
Matjaz Gregoric <mtyaka@gmail.com> Matjaz Gregoric <mtyaka@gmail.com>
Ben McMorran <ben.mcmorran@gmail.com>
...@@ -20,7 +20,8 @@ get "#{APIPREFIX}/:commentable_id/threads" do |commentable_id| ...@@ -20,7 +20,8 @@ get "#{APIPREFIX}/:commentable_id/threads" do |commentable_id|
params["sort_key"], params["sort_key"],
params["sort_order"], params["sort_order"],
params["page"], params["page"],
params["per_page"] params["per_page"],
params["context"] ? params["context"] : :course
).to_json ).to_json
end end
...@@ -34,6 +35,10 @@ post "#{APIPREFIX}/:commentable_id/threads" do |commentable_id| ...@@ -34,6 +35,10 @@ post "#{APIPREFIX}/:commentable_id/threads" do |commentable_id|
if params["group_id"] if params["group_id"]
thread.group_id = params["group_id"] thread.group_id = params["group_id"]
end end
if params["context"]
thread.context = params["context"]
end
thread.author = user thread.author = user
thread.save thread.save
......
...@@ -3,6 +3,7 @@ require 'new_relic/agent/method_tracer' ...@@ -3,6 +3,7 @@ require 'new_relic/agent/method_tracer'
get "#{APIPREFIX}/search/threads" do get "#{APIPREFIX}/search/threads" do
local_params = params # Necessary for params to be available inside blocks local_params = params # Necessary for params to be available inside blocks
group_ids = get_group_ids_from_params(local_params) group_ids = get_group_ids_from_params(local_params)
context = local_params["context"] ? local_params["context"] : "course"
search_text = local_params["text"] search_text = local_params["text"]
if !search_text if !search_text
{}.to_json {}.to_json
...@@ -23,6 +24,10 @@ get "#{APIPREFIX}/search/threads" do ...@@ -23,6 +24,10 @@ get "#{APIPREFIX}/search/threads" do
filter :term, :commentable_id => local_params["commentable_id"] if local_params["commentable_id"] filter :term, :commentable_id => local_params["commentable_id"] if local_params["commentable_id"]
filter :terms, :commentable_id => local_params["commentable_ids"].split(",") if local_params["commentable_ids"] filter :terms, :commentable_id => local_params["commentable_ids"].split(",") if local_params["commentable_ids"]
filter :term, :course_id => local_params["course_id"] if local_params["course_id"] filter :term, :course_id => local_params["course_id"] if local_params["course_id"]
filter :or, [
{:not => {:exists => {:field => :context}}},
{:term => {:context => context}}
]
if not group_ids.empty? if not group_ids.empty?
if group_ids.length > 1 if group_ids.length > 1
...@@ -87,7 +92,8 @@ get "#{APIPREFIX}/search/threads" do ...@@ -87,7 +92,8 @@ get "#{APIPREFIX}/search/threads" do
local_params["sort_key"], local_params["sort_key"],
local_params["sort_order"], local_params["sort_order"],
local_params["page"], local_params["page"],
local_params["per_page"] local_params["per_page"],
context
) )
if !result_obj.empty? if !result_obj.empty?
result_obj[:corrected_text] = corrected_text result_obj[:corrected_text] = corrected_text
......
...@@ -129,11 +129,17 @@ helpers do ...@@ -129,11 +129,17 @@ helpers do
sort_key, sort_key,
sort_order, sort_order,
page, page,
per_page per_page,
context=:course
) )
context_threads = comment_threads.any_of({:context.exists => false}, {:context => context})
if not group_ids.empty? if not group_ids.empty?
comment_threads = get_group_id_criteria(comment_threads, group_ids) group_threads = get_group_id_criteria(comment_threads, group_ids)
comment_threads = comment_threads.all_of(context_threads.selector, group_threads.selector)
else
comment_threads = context_threads
end end
if filter_flagged if filter_flagged
......
...@@ -39,6 +39,7 @@ class Comment < Content ...@@ -39,6 +39,7 @@ class Comment < Content
indexes :comment_thread_id, type: :string, index: :not_analyzed, included_in_all: false, as: 'comment_thread_id' indexes :comment_thread_id, type: :string, index: :not_analyzed, included_in_all: false, as: 'comment_thread_id'
indexes :commentable_id, type: :string, index: :not_analyzed, included_in_all: false, as: 'commentable_id' indexes :commentable_id, type: :string, index: :not_analyzed, included_in_all: false, as: 'commentable_id'
indexes :group_id, type: :string, index: :not_analyzed, included_in_all: false, as: 'group_id' indexes :group_id, type: :string, index: :not_analyzed, included_in_all: false, as: 'group_id'
indexes :context, type: :string, index: :not_analyzed, included_in_all: false, as: 'context'
indexes :created_at, type: :date, included_in_all: false indexes :created_at, type: :date, included_in_all: false
indexes :updated_at, type: :date, included_in_all: false indexes :updated_at, type: :date, included_in_all: false
end end
...@@ -133,6 +134,17 @@ class Comment < Content ...@@ -133,6 +134,17 @@ class Comment < Content
nil nil
end end
def context
if self.comment_thread_id
t = CommentThread.find self.comment_thread_id
if t
t.context
end
end
rescue Mongoid::Errors::DocumentNotFound
nil
end
def self.by_date_range_and_thread_ids from_when, to_when, thread_ids def self.by_date_range_and_thread_ids from_when, to_when, thread_ids
#return all content between from_when and to_when #return all content between from_when and to_when
......
...@@ -11,6 +11,8 @@ class CommentThread < Content ...@@ -11,6 +11,8 @@ class CommentThread < Content
field :thread_type, type: String, default: :discussion field :thread_type, type: String, default: :discussion
enumerize :thread_type, in: [:question, :discussion] enumerize :thread_type, in: [:question, :discussion]
field :context, type: String, default: :course
enumerize :context, in: [:course, :standalone]
field :comment_count, type: Integer, default: 0 field :comment_count, type: Integer, default: 0
field :title, type: String field :title, type: String
field :body, type: String field :body, type: String
...@@ -41,6 +43,7 @@ class CommentThread < Content ...@@ -41,6 +43,7 @@ class CommentThread < Content
indexes :comment_count, type: :integer, included_in_all: false indexes :comment_count, type: :integer, included_in_all: false
indexes :votes_point, type: :integer, as: 'votes_point', included_in_all: false indexes :votes_point, type: :integer, as: 'votes_point', included_in_all: false
indexes :context, type: :string, index: :not_analyzed, included_in_all: false
indexes :course_id, type: :string, index: :not_analyzed, included_in_all: false indexes :course_id, type: :string, index: :not_analyzed, included_in_all: false
indexes :commentable_id, type: :string, index: :not_analyzed, included_in_all: false indexes :commentable_id, type: :string, index: :not_analyzed, included_in_all: false
indexes :author_id, type: :string, as: 'author_id', index: :not_analyzed, included_in_all: false indexes :author_id, type: :string, as: 'author_id', index: :not_analyzed, included_in_all: false
...@@ -56,6 +59,7 @@ class CommentThread < Content ...@@ -56,6 +59,7 @@ class CommentThread < Content
attr_accessible :title, :body, :course_id, :commentable_id, :anonymous, :anonymous_to_peers, :closed, :thread_type attr_accessible :title, :body, :course_id, :commentable_id, :anonymous, :anonymous_to_peers, :closed, :thread_type
validates_presence_of :thread_type validates_presence_of :thread_type
validates_presence_of :context
validates_presence_of :title validates_presence_of :title
validates_presence_of :body validates_presence_of :body
validates_presence_of :course_id # do we really need this? validates_presence_of :course_id # do we really need this?
...@@ -118,7 +122,7 @@ class CommentThread < Content ...@@ -118,7 +122,7 @@ class CommentThread < Content
end end
def to_hash(params={}) def to_hash(params={})
as_document.slice(*%w[thread_type 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 context])
.merge("id" => _id, "user_id" => author_id, .merge("id" => _id, "user_id" => author_id,
"username" => author_username, "username" => author_username,
"votes" => votes.slice(*%w[count up_count down_count point]), "votes" => votes.slice(*%w[count up_count down_count point]),
......
print ("Adding context to all comment threads where it does not yet exist\n");
db.contents.update(
{_type: "CommentThread", context: {$exists: false}},
{$set: {context: "course"}},
{multi: true}
);
printjson (db.runCommand({ getLastError: 1, w: "majority", wtimeout: 5000 } ));
print ("Removing context from all comment threads\n");
db.contents.update(
{_type: "CommentThread"},
{$unset: {context: ""}},
{multi: true}
);
printjson (db.runCommand({ getLastError: 1, w: "majority", wtimeout: 5000 } ));
...@@ -29,6 +29,18 @@ describe "app" do ...@@ -29,6 +29,18 @@ describe "app" do
res["course_id"].should == "abc" res["course_id"].should == "abc"
} }
end end
it "does not return standalone threads" do
[@threads["t1"], @threads["t2"], @threads["t3"]].each do |t|
t.course_id = "abc"
t.save!
end
@threads["t2"].context = :standalone
@threads["t2"].save!
rs = thread_result course_id: "abc", sort_order: "asc"
rs.length.should == 2
check_thread_result_json(nil, @threads["t1"], rs[0])
check_thread_result_json(nil, @threads["t3"], rs[1])
end
it "returns only threads where course id and commentable id match" do it "returns only threads where course id and commentable id match" do
@threads["t1"].course_id = "course1" @threads["t1"].course_id = "course1"
@threads["t1"].commentable_id = "commentable1" @threads["t1"].commentable_id = "commentable1"
......
...@@ -32,6 +32,11 @@ describe "app" do ...@@ -32,6 +32,11 @@ describe "app" do
threads.index{|c| c["body"] == "can anyone help me?"}.should_not be_nil threads.index{|c| c["body"] == "can anyone help me?"}.should_not be_nil
threads.index{|c| c["body"] == "it is unsolvable"}.should_not be_nil threads.index{|c| c["body"] == "it is unsolvable"}.should_not be_nil
end end
it "returns standalone threads if explicitly requested" do
threads = thread_result "question_1", context: "standalone"
threads.length.should == 1
threads[0]["body"].should == "no one can see us"
end
it "filters by course_id" do it "filters by course_id" do
course1_threads = thread_result "question_1", course_id: "1" course1_threads = thread_result "question_1", course_id: "1"
course1_threads.length.should == 1 course1_threads.length.should == 1
...@@ -98,7 +103,17 @@ describe "app" do ...@@ -98,7 +103,17 @@ describe "app" do
result["unread_comments_count"].should == 0 result["unread_comments_count"].should == 0
result["endorsed"].should == false result["endorsed"].should == false
CommentThread.count.should == old_count + 1 CommentThread.count.should == old_count + 1
CommentThread.where(title: "Interesting question").first.should_not be_nil thread = CommentThread.where(title: "Interesting question").first
thread.should_not be_nil
thread.context.should == "course"
end
it "can create a standalone thread" do
old_count = CommentThread.count
post '/api/v1/question_1/threads', default_params.merge(:context => "standalone")
CommentThread.count.should == old_count + 1
thread = CommentThread.where(title: "Interesting question").first
thread.should_not be_nil
thread.context.should == "standalone"
end end
CommentThread.thread_type.values.each do |thread_type| CommentThread.thread_type.values.each do |thread_type|
it "can create a #{thread_type} thread" do it "can create a #{thread_type} thread" do
......
...@@ -38,7 +38,7 @@ describe "app" do ...@@ -38,7 +38,7 @@ describe "app" do
describe "filtering works" do describe "filtering works" do
let!(:threads) do let!(:threads) do
threads = (0..29).map do |i| threads = (0..34).map do |i|
thread = make_thread(author, "text", course_id + (i % 2).to_s, "commentable" + (i % 3).to_s) thread = make_thread(author, "text", course_id + (i % 2).to_s, "commentable" + (i % 3).to_s)
if i < 2 if i < 2
comment = make_comment(author, thread, "objectionable") comment = make_comment(author, thread, "objectionable")
...@@ -55,6 +55,10 @@ describe "app" do ...@@ -55,6 +55,10 @@ describe "app" do
comment = make_comment(author, thread, "response") comment = make_comment(author, thread, "response")
comment.save! comment.save!
end end
if i > 29
thread.context = :standalone
thread.save!
end
thread thread
end end
refresh_es_index refresh_es_index
...@@ -74,6 +78,11 @@ describe "app" do ...@@ -74,6 +78,11 @@ describe "app" do
assert_response_contains((0..29).find_all {|i| i % 2 == 0}) assert_response_contains((0..29).find_all {|i| i % 2 == 0})
end end
it "by context" do
get "api/v1/search/threads", text: "text", context: "standalone"
assert_response_contains(30..34)
end
it "with unread filter" do it "with unread filter" do
user = create_test_user(Random.new) user = create_test_user(Random.new)
user.mark_as_read(threads[0]) user.mark_as_read(threads[0])
......
...@@ -131,6 +131,13 @@ def init_without_subscriptions ...@@ -131,6 +131,13 @@ def init_without_subscriptions
comment1.comment_thread = thread comment1.comment_thread = thread
comment1.save! comment1.save!
thread = CommentThread.new(title: "Our super secret discussion", body: "no one can see us", course_id: "2", commentable_id: commentable.id)
thread.thread_type = :discussion
thread.context = :standalone
thread.author = user
thread.save!
user.subscribe(thread)
thread = CommentThread.new(title: "I don't know what to say", body: "lol", course_id: "2", commentable_id: "something else") 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.thread_type = :discussion
thread.author = users[1] thread.author = users[1]
...@@ -205,7 +212,7 @@ end ...@@ -205,7 +212,7 @@ end
# this method is used to test results produced using the helper function handle_threads_query # this method is used to test results produced using the helper function handle_threads_query
# which is used in multiple areas of the API # which is used in multiple areas of the API
def check_thread_result(user, thread, hash, is_json=false) def check_thread_result(user, thread, hash, is_json=false)
expected_keys = %w(id thread_type 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 context)
expected_keys += %w(anonymous anonymous_to_peers at_position_list closed user_id) 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(username votes abuse_flaggers tags type group_id pinned)
expected_keys += %w(comments_count unread_comments_count read endorsed) expected_keys += %w(comments_count unread_comments_count read endorsed)
...@@ -237,6 +244,7 @@ def check_thread_result(user, thread, hash, is_json=false) ...@@ -237,6 +244,7 @@ def check_thread_result(user, thread, hash, is_json=false)
hash["pinned"].should == thread.pinned? hash["pinned"].should == thread.pinned?
hash["endorsed"].should == thread.endorsed? hash["endorsed"].should == thread.endorsed?
hash["comments_count"].should == thread.comments.length hash["comments_count"].should == thread.comments.length
hash["context"] = thread.context
if is_json if is_json
hash["id"].should == thread._id.to_s hash["id"].should == thread._id.to_s
......
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