Follow / unfollow thread actions should bump its last activity date & time + minor general fixes

.rubocop.yml

@@ -58,7 +58,7 @@ Naming/BlockForwarding:
   EnforcedStyle: explicit
 Naming/PredicateMethod:
   Mode: conservative
-  AllowedPatterns: ['verify_.*', 'check_.*', 'enforce_.*', 'setup_.*']
+  AllowedPatterns: ['verify_.*', 'check_.*', 'enforce_.*', 'setup_.*','add_.*', 'remove_.*']
   AllowedMethods:
     - post_sign_in # returns a boolean but is a necessary after-sign-in method for both normal and SAML flows
     - comment_rate_limited? # returns a tuple with the boolean as the first value

app/controllers/comments_controller.rb

@@ -188,14 +188,16 @@ def show
   end
 
   def thread
-    respond_to do |format|
-      format.html { render 'comments/thread' }
-      format.json { render json: @comment_thread }
+    if stale?(last_modified: @comment_thread.last_activity.utc)
+      respond_to do |format|
+        format.html { render 'comments/thread' }
+        format.json { render json: @comment_thread }
+      end
     end
   end
 
   def thread_content
-    if stale?(last_modified: @comment_thread.last_activity_at.utc)
+    if stale?(last_modified: @comment_thread.last_activity.utc)
       render partial: 'comment_threads/expanded',
              locals: { inline: params[:inline] == 'true',
                        show_deleted: params[:show_deleted_comments] == '1',
@@ -264,7 +266,7 @@ def delete_thread
   end
 
   def follow_thread
-    status = ThreadFollower.create(comment_thread: @comment_thread, user: current_user)
+    status = @comment_thread.add_follower(current_user)
     restrict_thread_response(@comment_thread, status)
   end
 
@@ -293,7 +295,7 @@ def undelete_thread
   end
 
   def unfollow_thread
-    status = ThreadFollower.find_by(comment_thread: @comment_thread, user: current_user)&.destroy
+    status = @comment_thread.remove_follower(current_user)
     restrict_thread_response(@comment_thread, status)
   end
 

app/models/comment.rb

@@ -17,16 +17,12 @@ class Comment < ApplicationRecord
   after_create :create_follower
   after_update :delete_thread
 
+  before_save :bump_last_activity
+
   counter_culture :comment_thread, column_name: proc { |model| model.deleted? ? nil : 'reply_count' }, touch: true
 
   validate :content_length
 
-  # Gets last activity date and time on the comment
-  # @return [DateTime] last activity date and time
-  def last_activity_at
-    [created_at, updated_at].compact.max
-  end
-
   def root
     # If parent_question is nil, the comment is already on a question, so we can just return post.
     parent_question || post
@@ -41,12 +37,28 @@ def content_length
     end
   end
 
+  # Gets last activity date and time on the comment
+  # @return [DateTime] last activity date and time
+  def last_activity
+    [created_at, updated_at, last_activity_at].compact.max
+  end
+
   def pings
     pingable = comment_thread.pingable
     matches = content.scan(USER_PING_REG_EXP)
     matches.flatten.select { |m| pingable.include?(m.to_i) }.map(&:to_i)
   end
 
+  # Directly bumps the comment's last activity date & time
+  # @param persist_changes [Boolean] if set to +true+, will persist the changes
+  def bump_last_activity(persist_changes: false)
+    self.last_activity_at = DateTime.now
+
+    if persist_changes
+      save
+    end
+  end
+
   private
 
   def create_follower

app/models/comment_thread.rb

@@ -16,6 +16,8 @@ class CommentThread < ApplicationRecord
 
   after_create :create_follower
 
+  before_save :bump_last_activity
+
   # Gets threads appropriately scoped for a given user & post
   # @param user [User, nil] user to check
   # @para post [Post] post to check
@@ -51,9 +53,9 @@ def can_access?(user)
 
   # Gets last activity date and time on the thread
   # @return [DateTime] last activity date and time
-  def last_activity_at
-    last_comment_activity_at = comments.map(&:last_activity_at).max
-    [created_at, locked_at, updated_at, last_comment_activity_at].compact.max
+  def last_activity
+    last_comment_activity = comments.map(&:last_activity).compact.max
+    [created_at, updated_at, last_activity_at, last_comment_activity].compact.max
   end
 
   # Gets a list of user IDs who should be pingable in the thread.
@@ -86,6 +88,30 @@ def maximum_title_length
     end
   end
 
+  # Registers a given user as a follower of the thread
+  # @param user [User] user to register as a follower
+  # @return [Boolean] status of the operation
+  def add_follower(user)
+    ThreadFollower.create(comment_thread: self, user: user)
+  end
+
+  # Directly bumps the thread's last activity date & time
+  # @param persist_changes [Boolean] if set to +true+, will persist the changes
+  def bump_last_activity(persist_changes: false)
+    self.last_activity_at = DateTime.now
+
+    if persist_changes
+      save
+    end
+  end
+
+  # Removes a given user from the thread's followers
+  # @param user [User] user to remove from followers
+  # @return [Boolean] status of the operation
+  def remove_follower(user)
+    ThreadFollower.where(comment_thread: self, user: user).destroy_all.any?
+  end
+
   private
 
   # Comment author and post author are automatically followed to the thread. Question author is NOT

app/models/thread_follower.rb

@@ -3,10 +3,17 @@ class ThreadFollower < ApplicationRecord
   belongs_to :post, optional: true
   belongs_to :user
 
+  after_create :bump_thread_last_activity
+  before_destroy :bump_thread_last_activity
+
   validate :thread_or_post
 
   private
 
+  def bump_thread_last_activity
+    comment_thread&.bump_last_activity(persist_changes: true)
+  end
+
   def thread_or_post
     if comment_thread.nil? && post.nil?
       errors.add(:base, 'Must refer to either a comment thread or a post.')

app/models/user.rb

@@ -483,13 +483,13 @@ def active_flags(post)
   end
 
   # Anonymizes the user (f.e., for the purpose of soft deletion)
-  # @param persist_changes [Boolean] if set to +false+, will persist the changes
+  # @param persist_changes [Boolean] if set to +true+, will persist the changes
   def anonymize(persist_changes: false)
     assign_attributes(username: "user#{id}",
                       email: "#{id}@deleted.localhost",
                       password: SecureRandom.hex(32))
 
-    unless persist_changes
+    if persist_changes
       skip_reconfirmation!
       save
     end
@@ -505,7 +505,7 @@ def soft_delete(attribute_to)
                       deleted_by_id: attribute_to.id,
                       deleted_at: DateTime.now)
 
-    anonymize(persist_changes: true)
+    anonymize(persist_changes: false)
 
     skip_reconfirmation!
     save

db/migrate/20251221140930_add_last_activity_at_to_comments.rb

@@ -0,0 +1,5 @@
+class AddLastActivityAtToComments < ActiveRecord::Migration[7.2]
+  def change
+    add_column :comments, :last_activity_at, :datetime
+  end
+end

db/migrate/20251221142105_add_last_activity_at_to_comment_threads.rb

@@ -0,0 +1,5 @@
+class AddLastActivityAtToCommentThreads < ActiveRecord::Migration[7.2]
+  def change
+    add_column :comment_threads, :last_activity_at, :datetime
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.2].define(version: 2025_10_15_121326) do
+ActiveRecord::Schema[7.2].define(version: 2025_12_21_142105) do
   create_table "abilities", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
     t.bigint "community_id"
     t.string "name"
@@ -176,6 +176,7 @@
     t.datetime "updated_at", precision: nil, null: false
     t.bigint "community_id", null: false
     t.datetime "locked_at", precision: nil
+    t.datetime "last_activity_at"
     t.index ["archived_by_id"], name: "index_comment_threads_on_archived_by_id"
     t.index ["community_id"], name: "index_comment_threads_on_community_id"
     t.index ["deleted_by_id"], name: "index_comment_threads_on_deleted_by_id"
@@ -195,6 +196,7 @@
     t.boolean "has_reference", default: false, null: false
     t.text "reference_text"
     t.bigint "references_comment_id"
+    t.datetime "last_activity_at"
     t.index ["comment_thread_id"], name: "index_comments_on_comment_thread_id"
     t.index ["community_id"], name: "index_comments_on_community_id"
     t.index ["post_id"], name: "index_comments_on_post_type_and_post_id"

test/fixtures/comment_threads.yml

@@ -74,3 +74,11 @@ with_user_pings:
   reply_count: 1
   post: question_one
   community: sample
+
+without_activity:
+  title: Comment thread without activity
+  reply_count: 0
+  post: question_one
+  community: sample
+  created_at: 2020-01-01T00:00:00.000000Z
+  updated_at: 2020-01-01T00:00:00.000000Z

test/fixtures/comments.yml

@@ -81,3 +81,12 @@ with_user_pings:
   comment_thread: with_user_pings
   created_at: 2020-01-01T00:00:00.000000Z
   updated_at: 2021-01-01T00:00:00.000000Z
+
+without_activity:
+  user: standard_user
+  post: question_one
+  content: This comment must be without any initial activity
+  community: sample
+  comment_thread: normal
+  created_at: 2020-01-01T00:00:00.000000Z
+  updated_at: 2020-01-01T00:00:00.000000Z

test/models/comment_test.rb

@@ -34,7 +34,7 @@ class CommentTest < ActiveSupport::TestCase
     end
   end
 
-  test 'pings should correctly' do
+  test 'pings should correctly get pinged user ids' do
     std_user = users(:standard_user)
     with_pings = comments(:with_user_pings)
 
@@ -43,4 +43,19 @@ class CommentTest < ActiveSupport::TestCase
     assert pings.include?(std_user.id)
     assert_not pings.include?(User.system.id)
   end
+
+  test 'last_activity should correctly get the comment\'s last activity date & time' do
+    comment = comments(:without_activity)
+
+    assert_equal comment.created_at, comment.last_activity
+
+    comment.update!(content: 'this should bump last_activity')
+    comment.reload
+    last_activity_after_update = comment.last_activity
+    assert_operator comment.updated_at, '<', last_activity_after_update
+
+    comment.bump_last_activity(persist_changes: true)
+    comment.reload
+    assert_operator last_activity_after_update, '<', comment.last_activity
+  end
 end

test/models/comment_thread_test.rb

@@ -16,16 +16,28 @@ class CommentThreadTest < ActiveSupport::TestCase
     end
   end
 
-  test 'last_activity_at should correctly get last activity' do
-    normal = comment_threads(:normal)
-    normal_two = comment_threads(:normal_two)
-    locked = comment_threads(:locked)
+  test 'last_activity should correctly get the thread\'s last activity date & time' do
+    thread = comment_threads(:without_activity)
 
-    assert_not_equal locked.last_activity_at, locked.created_at
-    assert_not_equal normal_two.last_activity_at, normal_two.created_at
+    assert_equal thread.created_at, thread.last_activity
 
-    assert_equal locked.last_activity_at, locked.locked_at
-    assert_equal normal.last_activity_at, normal.created_at
-    assert_equal normal_two.last_activity_at, normal_two.updated_at
+    thread.update!(title: 'this should bump last_activity')
+    thread.reload
+    last_activity_after_update = thread.last_activity
+    assert_operator thread.updated_at, '<=', last_activity_after_update
+
+    thread.bump_last_activity(persist_changes: true)
+    thread.reload
+    last_activity_after_bump = thread.last_activity
+    assert_operator last_activity_after_update, '<', last_activity_after_bump
+
+    thread.add_follower(users(:editor))
+    thread.reload
+    last_activity_after_follow = thread.last_activity
+    assert_operator last_activity_after_bump, '<', last_activity_after_follow
+
+    thread.remove_follower(users(:editor))
+    thread.reload
+    assert_operator last_activity_after_follow, '<', thread.last_activity
   end
 end

test/models/user_test.rb

@@ -321,7 +321,7 @@ class UserTest < ActiveSupport::TestCase
     anonymized_name = "user#{std.id}"
     anonymized_email = "#{std.id}@deleted.localhost"
 
-    [true, false].each do |persist|
+    [false, true].each do |persist|
       std.anonymize(persist_changes: persist)
 
       assert_equal std.username, anonymized_name
@@ -330,11 +330,11 @@ class UserTest < ActiveSupport::TestCase
       std.reload
 
       if persist
-        assert_not_equal std.username, anonymized_name
-        assert_not_equal std.email, anonymized_email
-      else
         assert_equal std.username, anonymized_name
         assert_equal std.email, anonymized_email
+      else
+        assert_not_equal std.username, anonymized_name
+        assert_not_equal std.email, anonymized_email
       end
     end
   end