Merge pull request #192 from edx/jia/MA-2678

MA-2678: replace use of 'updated_at' for 'read' state

models/comment.rb

@@ -25,6 +25,7 @@ class Comment < Content
 
   index({author_id: 1, course_id: 1})
   index({_type: 1, comment_thread_id: 1, author_id: 1, updated_at: 1})
+  index({comment_thread_id: 1, author_id: 1, created_at: 1})
 
   index_name Content::ES_INDEX_NAME
 
@@ -53,7 +54,6 @@ class Comment < Content
 
   before_destroy :destroy_children
   before_create :set_thread_last_activity_at
-  before_update :set_thread_last_activity_at
   before_save :set_sk
 
   def self.hash_tree(nodes)

models/comment_thread.rb

@@ -68,7 +68,6 @@ class CommentThread < Content
   validates_presence_of :author, autosave: false
 
   before_create :set_last_activity_at
-  before_update :set_last_activity_at, :unless => lambda { closed_changed? }
   after_update :clear_endorsements
   before_destroy :destroy_subscriptions
 

presenters/thread_utils.rb

@@ -27,11 +27,11 @@ module ThreadUtils
         threads.each do |t|
           thread_key = t._id.to_s
           if read_dates.has_key? thread_key
-            is_read = read_dates[thread_key] >= t.updated_at
+            is_read = read_dates[thread_key] >= t.last_activity_at
             unread_comment_count = Comment.collection.find(
               :comment_thread_id => t._id,
               :author_id => {"$ne" => user.id},
-              :updated_at => {"$gte" => read_dates[thread_key]},
+              :created_at => {"$gte" => read_dates[thread_key]}
               ).count
             read_states[thread_key] = [is_read, unread_comment_count]
           end

scripts/db/migrate-009-comment_thread-author-created_at-indexes.js

@@ -0,0 +1 @@
+db.contents.ensureIndex({ comment_thread_id: 1, author_id: 1, created_at: 1 }, { background: true })

spec/api/comment_thread_spec.rb

@@ -211,7 +211,7 @@ describe "app" do
           rs.each_with_index { |result, i|
             check_thread_result_json(user, @threads["t#{i+1}"], result)
           }
-          rs[0]["read"].should == false # no unread comments, but the thread itself was updated
+          rs[0]["read"].should == true
           rs[0]["unread_comments_count"].should == 0
           rs[1]["read"].should == false
           rs[1]["unread_comments_count"].should == 5
@@ -258,18 +258,54 @@ describe "app" do
             expected_order = @default_order
             actual_order.should == expected_order
           end
-          it "sorts using last activity / descending" do
+          it "sort unchanged using last activity / descending when thread is updated" do
+            t5 = @threads["t5"]
+            t5.update(body: "changed!")
+            t5.save!
+            actual_order = thread_result_order("activity", "desc")
+            expected_order = @default_order
+            actual_order.should == expected_order
+          end
+          it "sort unchanged using last activity / ascending when thread is updated" do
+            t5 = @threads["t5"]
+            t5.update(body: "changed!")
+            t5.save!
+            actual_order = thread_result_order("activity", "asc")
+            expected_order = @default_order.reverse
+            actual_order.should == expected_order
+          end
+          it "sort unchanged using last activity / descending when comment is updated" do
             t5c = @threads["t5"].comments.first
             t5c.update(body: "changed!")
             t5c.save!
             actual_order = thread_result_order("activity", "desc")
-            expected_order = move_to_front(@default_order, "t5")
+            expected_order = @default_order
             actual_order.should == expected_order
           end
-          it "sorts using last activity / ascending" do
+          it "sort unchanged using last activity / ascending when comment is updated" do
             t5c = @threads["t5"].comments.first
             t5c.update(body: "changed!")
             t5c.save!
+            actual_order = thread_result_order("activity", "asc")
+            expected_order = @default_order.reverse
+            actual_order.should == expected_order
+          end
+          it "sorts using last activity / descending when response is created" do
+            t5 = @threads["t5"]
+            comment = t5.comments.new(body: "this problem is so easy", course_id: "1")
+            comment.author = User.first
+            comment.save!
+
+            actual_order = thread_result_order("activity", "desc")
+            expected_order = move_to_front(@default_order, "t5")
+            actual_order.should == expected_order
+          end
+          it "sorts using last activity / ascending when response is created" do
+            t5 = @threads["t5"]
+            comment = t5.comments.new(body: "this problem is so easy", course_id: "1")
+            comment.author = User.first
+            comment.save!
+
             actual_order = thread_result_order("activity", "asc")
             expected_order = move_to_end(@default_order.reverse, "t5")
             actual_order.should == expected_order

spec/api/search_spec.rb

@@ -175,7 +175,7 @@ describe "app" do
         end
 
         it "by activity" do
-          asc_order = [0, 2, 5, 1, 3, 4]
+          asc_order = [0, 1, 2, 3, 4, 5]
           check_sort("activity", "asc", asc_order)
           check_sort("activity", "desc", asc_order.reverse)
         end

spec/spec_helper.rb

@@ -214,11 +214,11 @@ def check_thread_result(user, thread, hash, is_json=false)
       read_date = read_states.first.last_read_times[thread.id.to_s]
       if read_date
         thread.comments.each do |c|
-          if c.updated_at < read_date
+          if c.created_at < read_date
             expected_unread_cnt -= 1
           end
         end
-        hash["read"].should == (read_date >= thread.updated_at)
+        hash["read"].should == (read_date >= thread.last_activity_at)
       else
         hash["read"].should == false
       end