Merge pull request #122 from edx/christina/bug-tnl-543

Use group_id if present when getting user thread and comment counts.

api/users.rb

@@ -13,7 +13,9 @@ end
 
 get "#{APIPREFIX}/users/:user_id" do |user_id|
   begin
-    user.to_hash(complete: bool_complete, course_id: params["course_id"]).to_json
+    # Get any group_ids that may have been specified (will be an empty list if none specified).
+    group_ids = get_group_ids_from_params(params)
+    user.to_hash(complete: bool_complete, course_id: params["course_id"], group_ids: group_ids).to_json
   rescue Mongoid::Errors::DocumentNotFound
     error 404
   end

models/user.rb

@@ -49,9 +49,52 @@ class User
     end
     if params[:course_id]
       self.class.trace_execution_scoped(['Custom/User.to_hash/count_comments_and_threads']) do
-        hash = hash.merge("threads_count" => CommentThread.where(author_id: id,course_id: params[:course_id], anonymous: false, anonymous_to_peers: false).count,
-                          "comments_count" => Comment.where(author_id: id, course_id: params[:course_id], anonymous: false, anonymous_to_peers: false).count
-                         )
+        if not params[:group_ids].empty?
+          # Get threads in either the specified group(s) or posted to all groups (nil).
+          specified_groups_or_global = params[:group_ids] << nil
+          threads_count = CommentThread.where(
+            author_id: id,
+            course_id: params[:course_id],
+            group_id: {"$in" => specified_groups_or_global},
+            anonymous: false,
+            anonymous_to_peers: false
+          ).count
+
+          # Note that the comments may have been responses to a thread not started by author_id.
+          comment_thread_ids = Comment.where(
+            author_id: id,
+            course_id: params[:course_id],
+            anonymous: false,
+            anonymous_to_peers: false
+          ).collect{|c| c.comment_thread_id}
+
+          # Filter to the unique thread ids visible to the specified group(s).
+          group_comment_thread_ids = CommentThread.where(
+            id: {"$in" => comment_thread_ids.uniq},
+            group_id: {"$in" => specified_groups_or_global},
+          ).collect{|d| d.id}
+
+          # Now filter comment_thread_ids so it only includes things in group_comment_thread_ids
+          # (keeping duplicates so the count will be correct).
+          comments_count = comment_thread_ids.count{
+            |comment_thread_id| group_comment_thread_ids.include?(comment_thread_id)
+          }
+
+        else
+          threads_count = CommentThread.where(
+            author_id: id,
+            course_id: params[:course_id],
+            anonymous: false,
+            anonymous_to_peers: false
+          ).count
+          comments_count = Comment.where(
+            author_id: id,
+            course_id: params[:course_id],
+            anonymous: false,
+            anonymous_to_peers: false
+          ).count
+        end
+        hash = hash.merge("threads_count" => threads_count, "comments_count" => comments_count)
       end
     end
     hash

spec/api/user_spec.rb

@@ -58,6 +58,98 @@ describe "app" do
         get "/api/v1/users/3"
         last_response.status.should == 404
       end
+      describe "Returns threads_count and comments_count" do
+          before(:each) { setup_10_threads }
+
+          def create_thread_and_comment_in_specific_group(user_id, group_id, thread)
+             # Changes the specified thread and a comment within that thread to be authored by the
+             # specified user in the specified group_id.
+             @threads[thread].author = @users["u"+user_id]
+             @threads[thread].group_id = group_id
+             @threads[thread].save!
+             first_comment_in_thread = thread + " c1"
+             @comments[first_comment_in_thread].author = @users["u"+user_id]
+             @comments[first_comment_in_thread].save!
+          end
+
+          def verify_counts(expected_threads, expected_comments, user_id, group_id=nil)
+             if group_id
+                get "/api/v1/users/" + user_id, course_id: "xyz", group_id: group_id
+             else
+                get "/api/v1/users/" + user_id, course_id: "xyz"
+             end
+             parse_response_and_verify_counts(expected_threads, expected_comments)
+          end
+
+          def verify_counts_multiple_groups(expected_threads, expected_comments, user_id, group_ids)
+             get "/api/v1/users/" + user_id, course_id: "xyz", group_ids: group_ids
+             parse_response_and_verify_counts(expected_threads, expected_comments)
+          end
+
+          def parse_response_and_verify_counts(expected_threads, expected_comments)
+             res = parse(last_response.body)
+             res["threads_count"].should == expected_threads
+             res["comments_count"].should == expected_comments
+          end
+
+          it "returns threads_count and comments_count" do
+             # "setup_10_threads" creates 1 thread ("t0") and 5 comments (in "t0") authored by user 100.
+             verify_counts(1, 5, "100")
+          end
+
+          it "returns threads_count and comments_count irrespective of group_id, if group_id is not specified" do
+             # Now change thread "t1" and comment in "t1" to be authored by user 100, but in a group (43).
+             # As long as we don't ask for user info for a specific group, these will always be included.
+             create_thread_and_comment_in_specific_group("100", 43, "t1")
+             verify_counts(2, 6, "100")
+          end
+
+          it "returns threads_count and comments_count filtered by group_id, if group_id is specified" do
+             create_thread_and_comment_in_specific_group("100", 43, "t1")
+
+             # The threads and comments created by "setup_10_threads" do not have a group_id specified, so are
+             # visible to all (group_id=3000 specified).
+             verify_counts(1, 5, "100", 3000)
+
+             # There is one additional thread and comment (created by create_thread_and_comment_in_specific_group),
+             # visible to only group_id 43.
+             verify_counts(2, 6, "100", 43)
+          end
+
+          it "handles comments correctly on threads not started by the author" do
+             # "setup_10_threads" creates 1 thread ("t1") and 5 comments (in "t1") authored by user 101.
+             verify_counts(1, 5, "101")
+
+             # The next call makes user 100 the author of "t1" and "t1 c1" (within group_id 43).
+             create_thread_and_comment_in_specific_group("100", 43, "t1")
+
+             # Therefore user 101 is now just the author of 4 comments.
+             verify_counts(0, 4, "101")
+
+             # We should get the same comment count when specifically asking for comments within group_id 43.
+             verify_counts(0, 4, "101", 43)
+
+             # We should get no comments for a different group.
+             verify_counts(0, 0, "101", 3000)
+          end
+
+          it "can return comments and threads for multiple groups" do
+             create_thread_and_comment_in_specific_group("100", 43, "t1")
+             create_thread_and_comment_in_specific_group("100", 3000, "t2")
+
+             # user 100 is now the author of:
+             #    visible to all groups-- 1 thread ("t0") and 5 comments
+             #    visible to group_id 43-- 1 thread ("t1") and 1 comment
+             #    visible to group_id 3000-- 1 thread ("t2") and 1 comment
+             verify_counts(3, 7, "100")
+             verify_counts_multiple_groups(3, 7, "100", "")
+             verify_counts_multiple_groups(3, 7, "100", "43, 3000")
+             verify_counts_multiple_groups(3, 7, "100", "43, 3000, 8")
+             verify_counts_multiple_groups(2, 6, "100", "43")
+             verify_counts_multiple_groups(2, 6, "100", "3000")
+             verify_counts_multiple_groups(1, 5, "100", "8")
+          end
+      end
     end
     describe "GET /api/v1/users/:user_id/active_threads" do