Commit 200af7bf by Bill DeRusha

Do not return standalone context threads for users

TNL-2971
parent 255c176b
...@@ -39,7 +39,7 @@ get "#{APIPREFIX}/users/:user_id/active_threads" do |user_id| ...@@ -39,7 +39,7 @@ get "#{APIPREFIX}/users/:user_id/active_threads" do |user_id|
thread_ids thread_ids
end end
threads = CommentThread.in({"_id" => active_thread_ids}) threads = CommentThread.course_context.in({"_id" => active_thread_ids})
group_ids = get_group_ids_from_params(params) group_ids = get_group_ids_from_params(params)
if not group_ids.empty? if not group_ids.empty?
......
...@@ -135,14 +135,15 @@ class Comment < Content ...@@ -135,14 +135,15 @@ class Comment < Content
end end
def context def context
if self.comment_thread_id self.comment_thread_id ? self.comment_thread.context : nil
t = CommentThread.find self.comment_thread_id end
if t
t.context def course_context?
end self.context == 'course'
end end
rescue Mongoid::Errors::DocumentNotFound
nil def standalone_context?
self.context == 'standalone'
end 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
......
...@@ -73,6 +73,8 @@ class CommentThread < Content ...@@ -73,6 +73,8 @@ class CommentThread < Content
before_destroy :destroy_subscriptions before_destroy :destroy_subscriptions
scope :active_since, ->(from_time) { where(:last_activity_at => {:$gte => from_time}) } scope :active_since, ->(from_time) { where(:last_activity_at => {:$gte => from_time}) }
scope :standalone_context, ->() { where(:context => :standalone) }
scope :course_context, ->() { where(:context => :course) }
def self.new_dumb_thread(options={}) def self.new_dumb_thread(options={})
c = self.new c = self.new
......
...@@ -52,7 +52,7 @@ class User ...@@ -52,7 +52,7 @@ class User
if not params[:group_ids].empty? if not params[:group_ids].empty?
# Get threads in either the specified group(s) or posted to all groups (nil). # Get threads in either the specified group(s) or posted to all groups (nil).
specified_groups_or_global = params[:group_ids] << nil specified_groups_or_global = params[:group_ids] << nil
threads_count = CommentThread.where( threads_count = CommentThread.course_context.where(
author_id: id, author_id: id,
course_id: params[:course_id], course_id: params[:course_id],
group_id: {"$in" => specified_groups_or_global}, group_id: {"$in" => specified_groups_or_global},
...@@ -61,12 +61,17 @@ class User ...@@ -61,12 +61,17 @@ class User
).count ).count
# Note that the comments may have been responses to a thread not started by author_id. # Note that the comments may have been responses to a thread not started by author_id.
comment_thread_ids = Comment.where(
# comment.standalone_context? gets the context from the parent comment_thread
# we need to eager load the comment_thread to prevent an N+1 when we iterate through the results
comment_thread_ids = Comment.includes(:comment_thread).where(
author_id: id, author_id: id,
course_id: params[:course_id], course_id: params[:course_id],
anonymous: false, anonymous: false,
anonymous_to_peers: false anonymous_to_peers: false
).collect{|c| c.comment_thread_id} ).
reject{ |comment| comment.standalone_context? }.
collect{ |comment| comment.comment_thread_id }
# Filter to the unique thread ids visible to the specified group(s). # Filter to the unique thread ids visible to the specified group(s).
group_comment_thread_ids = CommentThread.where( group_comment_thread_ids = CommentThread.where(
...@@ -81,18 +86,20 @@ class User ...@@ -81,18 +86,20 @@ class User
} }
else else
threads_count = CommentThread.where( threads_count = CommentThread.course_context.where(
author_id: id, author_id: id,
course_id: params[:course_id], course_id: params[:course_id],
anonymous: false, anonymous: false,
anonymous_to_peers: false anonymous_to_peers: false
).count ).count
comments_count = Comment.where( # comment.standalone_context? gets the context from the parent comment_thread
# we need to eager load the comment_thread to prevent an N+1 when we iterate through the results
comments_count = Comment.includes(:comment_thread).where(
author_id: id, author_id: id,
course_id: params[:course_id], course_id: params[:course_id],
anonymous: false, anonymous: false,
anonymous_to_peers: false anonymous_to_peers: false
).count ).reject{ |comment| comment.standalone_context? }.count
end end
hash = hash.merge("threads_count" => threads_count, "comments_count" => comments_count) hash = hash.merge("threads_count" => threads_count, "comments_count" => comments_count)
end end
......
...@@ -149,6 +149,19 @@ describe "app" do ...@@ -149,6 +149,19 @@ describe "app" do
verify_counts_multiple_groups(2, 6, "100", "3000") verify_counts_multiple_groups(2, 6, "100", "3000")
verify_counts_multiple_groups(1, 5, "100", "8") verify_counts_multiple_groups(1, 5, "100", "8")
end end
context "standalone threads" do
before(:each) do
# creates a standalone thread with 3 comments by user 100
make_standalone_thread_with_comments(@users['u100'])
end
it 'does not return standalone thread or comments in counts' do
# user 100 already has 1 thread and 5 comments created by `setup_10_threads`
# verify that the new standalone thread is not added to the counts
verify_counts(1, 5, "100")
end
end
end end
end end
describe "GET /api/v1/users/:user_id/active_threads" do describe "GET /api/v1/users/:user_id/active_threads" do
...@@ -168,13 +181,36 @@ describe "app" do ...@@ -168,13 +181,36 @@ describe "app" do
last_response.body.should == "{}" last_response.body.should == "{}"
end end
it "only returns threads with activity from the specified user" do context 'with standalone thread' do
@comments["t3 c4"].author = @users["u100"] before(:each) do
@comments["t3 c4"].save! # creates a standalone thread with 3 comments by user 100, stored as "standalone t0 c{i}"
rs = thread_result 100, course_id: "xyz" make_standalone_thread_with_comments(@users['u100'], 0)
rs.length.should == 2 # creates a standalone thread with 3 comments by user 101, with stored as "standalone t0 c{i}"
check_thread_result_json(@users["u100"], @threads["t3"], rs[0]) make_standalone_thread_with_comments(@users['u101'], 1)
check_thread_result_json(@users["u100"], @threads["t0"], rs[1]) end
it "only returns threads with non-standalone activity from the specified user" do
# `setup_10_threads` creates a thread "t3" and 5 comments all owned by user 103
# we are hijacking a course thread comment owned by user 103 and making it owned
# by user 100 instead, so this user has a comment on someone else's thread
@comments["t3 c4"].author = @users["u100"]
@comments["t3 c4"].save!
# do the same as above but with standalone
# hijack a standalone thread comment and make it owned by user 100
@comments["standalone t1 c1"].author = @users["u100"]
@comments["standalone t1 c1"].save!
results = thread_result 100, course_id: "xyz"
# it should not include the standalone thread we created for user 100
# not the standalone thread user 100 is a commenter on
results.length.should == 2
# it should include the course thread owned by user 100 (t0)
# and the course thread user 100 has a comment on (t3)
check_thread_result_json(@users["u100"], @threads["t3"], results[0])
check_thread_result_json(@users["u100"], @threads["t0"], results[1])
end
end end
it "filters by group_id" do it "filters by group_id" do
......
...@@ -6,15 +6,67 @@ describe Comment do ...@@ -6,15 +6,67 @@ describe Comment do
create_test_user(42) create_test_user(42)
end end
let(:thread) do let(:course_thread) do
make_thread(author, "Test thread", "test_course", "test_commentable") make_thread(author, "Test course thread", "test_course", "test_commentable", :discussion, :course)
end
let(:standalone_thread) do
make_thread(author, "Test standalone thread", "test_course", "test_commentable", :discussion, :standalone)
end end
def test_unicode_data(text) def test_unicode_data(text)
comment = make_comment(author, thread, text) comment = make_comment(author, course_thread, text)
retrieved = Comment.find(comment._id) retrieved = Comment.find(comment._id)
retrieved.body.should == text retrieved.body.should == text
end end
include_examples "unicode data" include_examples "unicode data"
describe '#context' do
context 'with standalone_thread' do
it 'returns "standalone"' do
comment = make_comment(author, standalone_thread, "comment")
expect(comment.context).to eq("standalone")
end
end
context 'with course_thread' do
it 'returns "course"' do
comment = make_comment(author, course_thread, "comment")
expect(comment.context).to eq("course")
end
end
end
describe '#course_context?' do
context 'with standalone_thread' do
it 'returns false' do
comment = make_comment(author, standalone_thread, "comment")
expect(comment.course_context?).to be_false
end
end
context 'with course_thread' do
it 'returns true' do
comment = make_comment(author, course_thread, "comment")
expect(comment.course_context?).to be_true
end
end
end
describe '#standalone_context?' do
context 'with standalone_thread' do
it 'returns true' do
comment = make_comment(author, standalone_thread, "comment")
expect(comment.standalone_context?).to be_true
end
end
context 'with course_thread' do
it 'returns false' do
comment = make_comment(author, course_thread, "comment")
expect(comment.standalone_context?).to be_false
end
end
end
end end
...@@ -6,12 +6,11 @@ describe CommentThread do ...@@ -6,12 +6,11 @@ describe CommentThread do
create_test_user(42) create_test_user(42)
end end
context "sorting" do before (:each) do
[Comment, CommentThread, User].each(&:delete_all).each(&:remove_indexes).each(&:create_indexes)
before (:each) do end
[Comment, CommentThread, User].each(&:delete_all).each(&:remove_indexes).each(&:create_indexes)
end
context "sorting" do
it "indexes comments in hierarchical order" do it "indexes comments in hierarchical order" do
author = create_test_user('billy') author = create_test_user('billy')
...@@ -57,6 +56,54 @@ describe CommentThread do ...@@ -57,6 +56,54 @@ describe CommentThread do
end end
end end
context "scoping" do
before(:each) do
author = create_test_user('billy')
# create a course thread
course_thread = CommentThread.new(title: "course thread", body: "testing 123", course_id: "foo", commentable_id: "bar")
course_thread.thread_type = :discussion
course_thread.author = author
course_thread.context = :course
course_thread.save!
# create a course thread (using the default context rather than setting it explicitly)
course_thread = CommentThread.new(title: "course thread", body: "testing 123", course_id: "foo", commentable_id: "bar")
course_thread.thread_type = :discussion
course_thread.author = author
course_thread.save!
# create a standalone thread
standalone_thread = CommentThread.new(title: "standalone_thread thread", body: "testing 123", course_id: "foo", commentable_id: "bear")
standalone_thread.thread_type = :discussion
standalone_thread.author = author
standalone_thread.context = :standalone
standalone_thread.save!
end
context '#unscoped' do
it 'returns all' do
expect(CommentThread.count).to eq(3)
end
end
context "#course_context" do
it 'returns all' do
threads = CommentThread.course_context
expect(threads.count).to eq(2)
expect(threads.first.title).to eq("course thread")
end
end
context "#standalone_context" do
it 'returns all' do
threads = CommentThread.standalone_context
expect(threads.count).to eq(1)
expect(threads.first.title).to eq("standalone_thread thread")
end
end
end
def test_unicode_data(text) def test_unicode_data(text)
thread = make_thread(author, text, "unicode_course", commentable_id: "unicode_commentable") thread = make_thread(author, text, "unicode_course", commentable_id: "unicode_commentable")
retrieved = CommentThread.find(thread._id) retrieved = CommentThread.find(thread._id)
......
...@@ -354,10 +354,11 @@ def check_thread_response_paging_json(thread, hash, resp_skip=0, resp_limit=nil) ...@@ -354,10 +354,11 @@ def check_thread_response_paging_json(thread, hash, resp_skip=0, resp_limit=nil)
end end
# general purpose factory helpers # general purpose factory helpers
def make_thread(author, text, course_id, commentable_id, thread_type=:discussion) def make_thread(author, text, course_id, commentable_id, thread_type=:discussion, context=:course)
thread = CommentThread.new(title: text, body: text, course_id: course_id, commentable_id: commentable_id) thread = CommentThread.new(title: text, body: text, course_id: course_id, commentable_id: commentable_id)
thread.thread_type = thread_type thread.thread_type = thread_type
thread.author = author thread.author = author
thread.context = context
thread.save! thread.save!
thread thread
end end
...@@ -377,6 +378,27 @@ def make_comment(author, parent, text) ...@@ -377,6 +378,27 @@ def make_comment(author, parent, text)
comment comment
end end
# add standalone threads and comments to the @threads and @comments hashes
# using the namespace "standalone t#{index}" for threads and "standalone t#{index} c#{i}" for comments
# takes an index param if used within an iterator, otherwise will namespace using 0 for thread index
# AKA this will overwrite "standalone t0" each time it is called.
def make_standalone_thread_with_comments(author, index=0)
thread = make_thread(
author,
"standalone thread #{index}",
DFLT_COURSE_ID,
"pdq",
:discussion,
:standalone
)
3.times do |i|
@comments["standalone t#{index} c#{i}"] = make_comment(author, thread, "stand alone comment #{i}")
end
@threads["standalone t#{index}"] = thread
end
DFLT_COURSE_ID = "xyz" DFLT_COURSE_ID = "xyz"
def setup_10_threads def setup_10_threads
......
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