From 17986cd0544aa86269d461533e8ef0e9d8c8af30 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Wed, 3 Dec 2025 21:03:19 +0000 Subject: [PATCH 01/47] Add NewThreadFollower model --- app/models/new_thread_follower.rb | 4 ++++ .../20251203164131_create_new_thread_followers.rb | 12 ++++++++++++ db/schema.rb | 11 ++++++++++- test/fixtures/new_thread_followers.yml | 9 +++++++++ test/models/new_thread_follower_test.rb | 7 +++++++ 5 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 app/models/new_thread_follower.rb create mode 100644 db/migrate/20251203164131_create_new_thread_followers.rb create mode 100644 test/fixtures/new_thread_followers.yml create mode 100644 test/models/new_thread_follower_test.rb diff --git a/app/models/new_thread_follower.rb b/app/models/new_thread_follower.rb new file mode 100644 index 000000000..90042d30d --- /dev/null +++ b/app/models/new_thread_follower.rb @@ -0,0 +1,4 @@ +class NewThreadFollower < ApplicationRecord + belongs_to :user + belongs_to :post +end diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb new file mode 100644 index 000000000..0caff968b --- /dev/null +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -0,0 +1,12 @@ +class CreateNewThreadFollowers < ActiveRecord::Migration[7.2] + def change + create_table :new_thread_followers do |t| + t.bigint :user_id + t.bigint :post_id + + t.timestamps + end + add_index :new_thread_followers, :user_id + add_index :new_thread_followers, :post_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 8d92b11e7..0946affd7 100644 --- a/db/schema.rb +++ b/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_03_164131) do create_table "abilities", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "community_id" t.string "name" @@ -401,6 +401,15 @@ t.index ["user_id"], name: "index_micro_auth_tokens_on_user_id" end + create_table "new_thread_followers", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| + t.bigint "user_id" + t.bigint "post_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["post_id"], name: "index_new_thread_followers_on_post_id" + t.index ["user_id"], name: "index_new_thread_followers_on_user_id" + end + create_table "notifications", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.text "content" t.string "link" diff --git a/test/fixtures/new_thread_followers.yml b/test/fixtures/new_thread_followers.yml new file mode 100644 index 000000000..8cac10012 --- /dev/null +++ b/test/fixtures/new_thread_followers.yml @@ -0,0 +1,9 @@ +# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html + +one: + user_id: + post_id: + +two: + user_id: + post_id: diff --git a/test/models/new_thread_follower_test.rb b/test/models/new_thread_follower_test.rb new file mode 100644 index 000000000..c14dcfe2f --- /dev/null +++ b/test/models/new_thread_follower_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class NewThreadFollowerTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end From ffd77914c34b50fb7c99473bd22dadbb204ebb6d Mon Sep 17 00:00:00 2001 From: trichoplax Date: Wed, 3 Dec 2025 21:18:22 +0000 Subject: [PATCH 02/47] Amend code to use NewThreadFollower model instead --- app/controllers/comments_controller.rb | 14 +++++++------- app/models/concerns/user_merge.rb | 1 + app/models/post.rb | 2 +- test/controllers/comments/post_follow_test.rb | 12 ++++++------ test/test_helper.rb | 1 + 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 6c37822a4..0d9433d5a 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -59,11 +59,11 @@ def create_thread @comment.post.user.create_notification(notification, helpers.comment_link(@comment)) end - ThreadFollower.where(post: @post).each do |tf| - unless tf.user == current_user || tf.user == @comment.post.user - tf.user.create_notification(notification, helpers.comment_link(@comment)) + NewThreadFollower.where(post: @post).each do |ntf| + unless ntf.user == current_user || ntf.user == @comment.post.user + ntf.user.create_notification(notification, helpers.comment_link(@comment)) end - ThreadFollower.create(user: tf.user, comment_thread: @comment_thread) + ThreadFollower.create(user: ntf.user, comment_thread: @comment_thread) end apply_pings(pings) @@ -330,8 +330,8 @@ def post end def post_follow - if ThreadFollower.where(post: @post, user: current_user).none? - ThreadFollower.create(post: @post, user: current_user) + if NewThreadFollower.where(post: @post, user: current_user).none? + NewThreadFollower.create(post: @post, user: current_user) end respond_to do |format| @@ -341,7 +341,7 @@ def post_follow end def post_unfollow - ThreadFollower.where(post: @post, user: current_user).destroy_all + NewThreadFollower.where(post: @post, user: current_user).destroy_all respond_to do |format| format.html { redirect_to post_path(@post) } diff --git a/app/models/concerns/user_merge.rb b/app/models/concerns/user_merge.rb index c72c7763b..a7b821ea3 100644 --- a/app/models/concerns/user_merge.rb +++ b/app/models/concerns/user_merge.rb @@ -71,6 +71,7 @@ def update_thread_references(target_user) CommentThread.where(archived_by_id: id).update_all(archived_by_id: target_user.id) CommentThread.where(deleted_by_id: id).update_all(deleted_by_id: target_user.id) ThreadFollower.where(user_id: id).update_all(user_id: target_user.id) + NewThreadFollower.where(user_id: id).update_all(user_id: target_user.id) end def update_post_action_references(target_user) diff --git a/app/models/post.rb b/app/models/post.rb index 3d99035da..bcbfad256 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -243,7 +243,7 @@ def deleted_by_owner? # @param user [User] user to check # @return [Boolean] check result def followed_by?(user) - ThreadFollower.where(post: self, user: user).any? + NewThreadFollower.where(post: self, user: user).any? end # Is the post an imported post? diff --git a/test/controllers/comments/post_follow_test.rb b/test/controllers/comments/post_follow_test.rb index 2547d6114..bd2932b16 100644 --- a/test/controllers/comments/post_follow_test.rb +++ b/test/controllers/comments/post_follow_test.rb @@ -11,13 +11,13 @@ class CommentsControllerTest < ActionController::TestCase question = posts(:question_one) # Assert user follows post - assert_equal 1, ThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count + assert_equal 1, NewThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count try_post_unfollow(question) assert_response(:found) # Assert user does not follow post - assert_equal 0, ThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count + assert_equal 0, NewThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count end test 'non-follower can follow post' do @@ -26,13 +26,13 @@ class CommentsControllerTest < ActionController::TestCase question = posts(:question_one) # Assert user does not follow post - assert_equal 0, ThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count + assert_equal 0, NewThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count try_post_follow(question) assert_response(:found) # Assert user follows post - assert_equal 1, ThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count + assert_equal 1, NewThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count end test 'follower cannot duplicate the following of a post' do @@ -41,12 +41,12 @@ class CommentsControllerTest < ActionController::TestCase question = posts(:question_one) # Assert user follows post - assert_equal 1, ThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count + assert_equal 1, NewThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count try_post_follow(question) assert_response(:found) # Assert user still only follows post once - assert_equal 1, ThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count + assert_equal 1, NewThreadFollower.where(['post_id = ? AND user_id = ?', question, user]).count end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 9eb4ffc98..7befbb484 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -20,6 +20,7 @@ WarningTemplate, ModWarning, ThreadFollower, + NewThreadFollower, Comment, CommentThread, Reaction, From 88827852eab7a6a6fa70315ba7b01aa6d64a594f Mon Sep 17 00:00:00 2001 From: trichoplax Date: Wed, 3 Dec 2025 22:09:22 +0000 Subject: [PATCH 03/47] Use single quotes for rubocop --- test/models/new_thread_follower_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/models/new_thread_follower_test.rb b/test/models/new_thread_follower_test.rb index c14dcfe2f..56754b3d5 100644 --- a/test/models/new_thread_follower_test.rb +++ b/test/models/new_thread_follower_test.rb @@ -1,7 +1,7 @@ -require "test_helper" +require 'test_helper' class NewThreadFollowerTest < ActiveSupport::TestCase - # test "the truth" do + # test 'the truth' do # assert true # end end From 9a3e5c08f96cb817649bcf49cad8e4035012ac7a Mon Sep 17 00:00:00 2001 From: trichoplax Date: Thu, 4 Dec 2025 00:22:48 +0000 Subject: [PATCH 04/47] Include data migration and column removal in migration --- ...251203164131_create_new_thread_followers.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index 0caff968b..9d289128c 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -1,5 +1,11 @@ class CreateNewThreadFollowers < ActiveRecord::Migration[7.2] def change + create_table_new_thread_followers + move_rows_with_non_nil_post_id + remove_post_id_column_from_thread_followers + end + + def create_table_new_thread_followers create_table :new_thread_followers do |t| t.bigint :user_id t.bigint :post_id @@ -9,4 +15,16 @@ def change add_index :new_thread_followers, :user_id add_index :new_thread_followers, :post_id end + + def move_rows_with_non_nil_post_id + NewThreadFollower.insert_all( + ThreadFollower.select(:user_id, :post_id, :created_at, :updated_at).where.not(post_id:nil) + ) + end + + def remove_post_id_column_from_thread_followers + change_table :thread_followers do |t| + t.remove :post_id + end + end end From 97c264923460b6b696f71305d36ba3c24a39c5c7 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Thu, 4 Dec 2025 00:38:44 +0000 Subject: [PATCH 05/47] Fix tests by rearranging fixtures for new table --- test/fixtures/new_thread_followers.yml | 11 +++-------- test/fixtures/thread_followers.yml | 4 ---- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/test/fixtures/new_thread_followers.yml b/test/fixtures/new_thread_followers.yml index 8cac10012..0d75b44ff 100644 --- a/test/fixtures/new_thread_followers.yml +++ b/test/fixtures/new_thread_followers.yml @@ -1,9 +1,4 @@ # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html - -one: - user_id: - post_id: - -two: - user_id: - post_id: +standard_author_question_one: + user: standard_user + post: question_one diff --git a/test/fixtures/thread_followers.yml b/test/fixtures/thread_followers.yml index b51e03b64..893c177d7 100644 --- a/test/fixtures/thread_followers.yml +++ b/test/fixtures/thread_followers.yml @@ -5,7 +5,3 @@ deleter_normal: standard_author_normal: user: standard_user comment_thread: normal - -standard_author_question_one: - user: standard_user - post: question_one From 5da697b7866a43a2eaf14052e8e53e413068f008 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Thu, 4 Dec 2025 01:50:47 +0000 Subject: [PATCH 06/47] Fix migration to remove moved rows and post reference --- app/models/thread_follower.rb | 13 +------------ .../20251203164131_create_new_thread_followers.rb | 8 +++----- db/schema.rb | 6 +----- 3 files changed, 5 insertions(+), 22 deletions(-) diff --git a/app/models/thread_follower.rb b/app/models/thread_follower.rb index 426e1f394..2f66a4842 100644 --- a/app/models/thread_follower.rb +++ b/app/models/thread_follower.rb @@ -1,15 +1,4 @@ class ThreadFollower < ApplicationRecord - belongs_to :comment_thread, optional: true - belongs_to :post, optional: true + belongs_to :comment_thread belongs_to :user - - validate :thread_or_post - - private - - def thread_or_post - if comment_thread.nil? && post.nil? - errors.add(:base, 'Must refer to either a comment thread or a post.') - end - end end diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index 9d289128c..77bf2dcdd 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -12,19 +12,17 @@ def create_table_new_thread_followers t.timestamps end - add_index :new_thread_followers, :user_id - add_index :new_thread_followers, :post_id + add_index :new_thread_followers, [:user_id, :post_id] end def move_rows_with_non_nil_post_id NewThreadFollower.insert_all( ThreadFollower.select(:user_id, :post_id, :created_at, :updated_at).where.not(post_id:nil) ) + ThreadFollower.where.not(post_id:nil).delete_all end def remove_post_id_column_from_thread_followers - change_table :thread_followers do |t| - t.remove :post_id - end + remove_reference :thread_followers, :post, index: true, foreign_key: true end end diff --git a/db/schema.rb b/db/schema.rb index 0946affd7..978480338 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -406,8 +406,7 @@ t.bigint "post_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["post_id"], name: "index_new_thread_followers_on_post_id" - t.index ["user_id"], name: "index_new_thread_followers_on_user_id" + t.index ["user_id", "post_id"], name: "index_new_thread_followers_on_user_id_and_post_id" end create_table "notifications", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| @@ -732,9 +731,7 @@ t.bigint "user_id" t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false - t.bigint "post_id" t.index ["comment_thread_id"], name: "index_thread_followers_on_comment_thread_id" - t.index ["post_id"], name: "index_thread_followers_on_post_id" t.index ["user_id"], name: "index_thread_followers_on_user_id" end @@ -902,7 +899,6 @@ add_foreign_key "tag_synonyms", "tags" add_foreign_key "tags", "communities" add_foreign_key "tags", "tags", column: "parent_id" - add_foreign_key "thread_followers", "posts" add_foreign_key "user_abilities", "abilities" add_foreign_key "user_abilities", "community_users" add_foreign_key "user_websites", "users" From f7c79f02aa4bf1efaa6d3cc4b3ca3bbc1638fd75 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Sat, 6 Dec 2025 01:46:55 +0000 Subject: [PATCH 07/47] Add model tests for ThreadFollower and NewThreadFollower --- test/models/new_thread_follower_test.rb | 21 ++++++++++++++++++--- test/models/thread_follower_test.rb | 21 ++++++++++++++++++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/test/models/new_thread_follower_test.rb b/test/models/new_thread_follower_test.rb index 56754b3d5..fa411935d 100644 --- a/test/models/new_thread_follower_test.rb +++ b/test/models/new_thread_follower_test.rb @@ -1,7 +1,22 @@ require 'test_helper' class NewThreadFollowerTest < ActiveSupport::TestCase - # test 'the truth' do - # assert true - # end + test "save succeeds with user and post" do + new_thread_follower=NewThreadFollower.new + new_thread_follower.user=users(:basic_user) + new_thread_follower.post=posts(:question_one) + assert new_thread_follower.save + end + + test "save fails without user" do + new_thread_follower=NewThreadFollower.new + new_thread_follower.post=posts(:question_one) + assert_not new_thread_follower.save + end + + test "save fails without post" do + new_thread_follower=NewThreadFollower.new + new_thread_follower.user=users(:basic_user) + assert_not new_thread_follower.save + end end diff --git a/test/models/thread_follower_test.rb b/test/models/thread_follower_test.rb index c4658bbb0..fc9e3e4a2 100644 --- a/test/models/thread_follower_test.rb +++ b/test/models/thread_follower_test.rb @@ -1,7 +1,22 @@ require 'test_helper' class ThreadFollowerTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end + test "save succeeds with user and thread" do + new_thread_follower=ThreadFollower.new + new_thread_follower.user=users(:basic_user) + new_thread_follower.comment_thread=comment_threads(:normal) + assert new_thread_follower.save + end + + test "save fails without user" do + new_thread_follower=ThreadFollower.new + new_thread_follower.comment_thread=comment_threads(:normal) + assert_not new_thread_follower.save + end + + test "save fails without thread" do + new_thread_follower=ThreadFollower.new + new_thread_follower.user=users(:basic_user) + assert_not new_thread_follower.save + end end From ac4f738c6caaae1f78265e280077a23831125154 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Sat, 6 Dec 2025 01:52:10 +0000 Subject: [PATCH 08/47] Tidying thanks to rubocop --- test/models/new_thread_follower_test.rb | 20 ++++++++++---------- test/models/thread_follower_test.rb | 20 ++++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/test/models/new_thread_follower_test.rb b/test/models/new_thread_follower_test.rb index fa411935d..5a6d8bed2 100644 --- a/test/models/new_thread_follower_test.rb +++ b/test/models/new_thread_follower_test.rb @@ -1,22 +1,22 @@ require 'test_helper' class NewThreadFollowerTest < ActiveSupport::TestCase - test "save succeeds with user and post" do - new_thread_follower=NewThreadFollower.new - new_thread_follower.user=users(:basic_user) - new_thread_follower.post=posts(:question_one) + test 'save succeeds with user and post' do + new_thread_follower = NewThreadFollower.new + new_thread_follower.user = users(:basic_user) + new_thread_follower.post = posts(:question_one) assert new_thread_follower.save end - test "save fails without user" do - new_thread_follower=NewThreadFollower.new - new_thread_follower.post=posts(:question_one) + test 'save fails without user' do + new_thread_follower = NewThreadFollower.new + new_thread_follower.post = posts(:question_one) assert_not new_thread_follower.save end - test "save fails without post" do - new_thread_follower=NewThreadFollower.new - new_thread_follower.user=users(:basic_user) + test 'save fails without post' do + new_thread_follower = NewThreadFollower.new + new_thread_follower.user = users(:basic_user) assert_not new_thread_follower.save end end diff --git a/test/models/thread_follower_test.rb b/test/models/thread_follower_test.rb index fc9e3e4a2..b50fa8043 100644 --- a/test/models/thread_follower_test.rb +++ b/test/models/thread_follower_test.rb @@ -1,22 +1,22 @@ require 'test_helper' class ThreadFollowerTest < ActiveSupport::TestCase - test "save succeeds with user and thread" do - new_thread_follower=ThreadFollower.new - new_thread_follower.user=users(:basic_user) - new_thread_follower.comment_thread=comment_threads(:normal) + test 'save succeeds with user and thread' do + new_thread_follower = ThreadFollower.new + new_thread_follower.user = users(:basic_user) + new_thread_follower.comment_thread = comment_threads(:normal) assert new_thread_follower.save end - test "save fails without user" do - new_thread_follower=ThreadFollower.new - new_thread_follower.comment_thread=comment_threads(:normal) + test 'save fails without user' do + new_thread_follower = ThreadFollower.new + new_thread_follower.comment_thread = comment_threads(:normal) assert_not new_thread_follower.save end - test "save fails without thread" do - new_thread_follower=ThreadFollower.new - new_thread_follower.user=users(:basic_user) + test 'save fails without thread' do + new_thread_follower = ThreadFollower.new + new_thread_follower.user = users(:basic_user) assert_not new_thread_follower.save end end From c82fd1fbd54f151706d4c7586317fd550df387bb Mon Sep 17 00:00:00 2001 From: trichoplax Date: Sun, 7 Dec 2025 14:01:25 +0000 Subject: [PATCH 09/47] Fix data insertion in NewThreadFollower migration --- db/migrate/20251203164131_create_new_thread_followers.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index 77bf2dcdd..676d1b6bf 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -17,7 +17,10 @@ def create_table_new_thread_followers def move_rows_with_non_nil_post_id NewThreadFollower.insert_all( - ThreadFollower.select(:user_id, :post_id, :created_at, :updated_at).where.not(post_id:nil) + ThreadFollower.select(:user_id, :post_id, :created_at, :updated_at) + .where.not(post_id:nil) + .to_a + .map(&:attributes) ) ThreadFollower.where.not(post_id:nil).delete_all end From a2af20399554ab1b5108ac88c25d276e613d6908 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Sun, 7 Dec 2025 14:06:16 +0000 Subject: [PATCH 10/47] Defend migration against new_thread_followers table already existing --- db/migrate/20251203164131_create_new_thread_followers.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index 676d1b6bf..c1c9ab7da 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -6,13 +6,13 @@ def change end def create_table_new_thread_followers - create_table :new_thread_followers do |t| + create_table :new_thread_followers, if_not_exists: true do |t| t.bigint :user_id t.bigint :post_id t.timestamps end - add_index :new_thread_followers, [:user_id, :post_id] + add_index :new_thread_followers, [:user_id, :post_id], if_not_exists: true end def move_rows_with_non_nil_post_id From 5b77c12969c6d8f897f1f4e1bb9f197d57f05a1c Mon Sep 17 00:00:00 2001 From: trichoplax Date: Sun, 7 Dec 2025 14:31:39 +0000 Subject: [PATCH 11/47] Check if post_id column exists during NewThreadFollower migration --- db/migrate/20251203164131_create_new_thread_followers.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index c1c9ab7da..062c81a66 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -1,8 +1,10 @@ class CreateNewThreadFollowers < ActiveRecord::Migration[7.2] def change create_table_new_thread_followers - move_rows_with_non_nil_post_id - remove_post_id_column_from_thread_followers + if column_exists?(:thread_followers, :post_id) + move_rows_with_non_nil_post_id + remove_post_id_column_from_thread_followers + end end def create_table_new_thread_followers From 5eb10d14c4c335c58c91e73d5170e50289139e63 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Sun, 7 Dec 2025 19:52:32 +0000 Subject: [PATCH 12/47] Add up and down to make NewThreadFollower migration reversible --- ...51203164131_create_new_thread_followers.rb | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index 062c81a66..68a69d59e 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -1,5 +1,5 @@ class CreateNewThreadFollowers < ActiveRecord::Migration[7.2] - def change + def up create_table_new_thread_followers if column_exists?(:thread_followers, :post_id) move_rows_with_non_nil_post_id @@ -7,6 +7,14 @@ def change end end + def down + if !column_exists?(:thread_followers, :post_id) + add_post_id_column_to_thread_followers + end + move_rows_back_from_new_thread_followers + delete_table_new_thread_followers + end + def create_table_new_thread_followers create_table :new_thread_followers, if_not_exists: true do |t| t.bigint :user_id @@ -30,4 +38,24 @@ def move_rows_with_non_nil_post_id def remove_post_id_column_from_thread_followers remove_reference :thread_followers, :post, index: true, foreign_key: true end + + def add_post_id_column_to_thread_followers + add_reference :thread_followers, :post, :index: true, foreign_key: true + end + + def move_rows_back_from_new_thread_followers + ThreadFollower.insert_all( + NewThreadFollower.select(:user_id, :post_id, :created_at, :updated_at) + .to_a + .map(&:attributes) + ) + NewThreadFollower.delete_all + end + + def delete_table_new_thread_followers + remove_index :new_thread_followers, [:user_id, :post_id], if_exists: true + remove_reference :new_thread_followers, :user_id, foreign_key: true + remove_reference :new_thread_followers, :post_id, foreign_key: true + drop_table :new_thread_followers + end end From 59ca21c2b2bd01524eb5669fd70c98fa3fd1e82c Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 8 Dec 2025 06:43:50 +0300 Subject: [PATCH 13/47] fixed typo in the create_new_thread_followers migration --- db/migrate/20251203164131_create_new_thread_followers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index 68a69d59e..efd6ddbc6 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -40,7 +40,7 @@ def remove_post_id_column_from_thread_followers end def add_post_id_column_to_thread_followers - add_reference :thread_followers, :post, :index: true, foreign_key: true + add_reference :thread_followers, :post, index: true, foreign_key: true end def move_rows_back_from_new_thread_followers From 8d2631409c940341d648d494c4dfb92f2327f113 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 8 Dec 2025 07:04:55 +0300 Subject: [PATCH 14/47] made create_new_thread_followers more resilient to partial state --- db/migrate/20251203164131_create_new_thread_followers.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index efd6ddbc6..8a5b9f1d7 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -36,11 +36,11 @@ def move_rows_with_non_nil_post_id end def remove_post_id_column_from_thread_followers - remove_reference :thread_followers, :post, index: true, foreign_key: true + remove_reference :thread_followers, :post, index: true, foreign_key: true, if_exists: true end def add_post_id_column_to_thread_followers - add_reference :thread_followers, :post, index: true, foreign_key: true + add_reference :thread_followers, :post, index: true, foreign_key: true, if_not_exists: true end def move_rows_back_from_new_thread_followers @@ -54,8 +54,8 @@ def move_rows_back_from_new_thread_followers def delete_table_new_thread_followers remove_index :new_thread_followers, [:user_id, :post_id], if_exists: true - remove_reference :new_thread_followers, :user_id, foreign_key: true - remove_reference :new_thread_followers, :post_id, foreign_key: true + remove_reference :new_thread_followers, :user_id, foreign_key: true, if_exists: true + remove_reference :new_thread_followers, :post_id, foreign_key: true, if_exists: true drop_table :new_thread_followers end end From c0acdf065b37d8eff94781984050e29f988bd0b5 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 9 Dec 2025 02:47:24 +0300 Subject: [PATCH 15/47] drop new_thread_followers table only if it exists --- db/migrate/20251203164131_create_new_thread_followers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index 8a5b9f1d7..b71a4aac5 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -56,6 +56,6 @@ def delete_table_new_thread_followers remove_index :new_thread_followers, [:user_id, :post_id], if_exists: true remove_reference :new_thread_followers, :user_id, foreign_key: true, if_exists: true remove_reference :new_thread_followers, :post_id, foreign_key: true, if_exists: true - drop_table :new_thread_followers + drop_table :new_thread_followers, if_exists: true end end From b890d3863165bc79f2b73a12f1ee9cfe49929ed8 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 9 Dec 2025 06:15:45 +0300 Subject: [PATCH 16/47] added thread follower tests for the user merge concern --- test/fixtures/community_users.yml | 14 ++++++++ test/fixtures/new_thread_followers.yml | 4 +++ test/fixtures/thread_followers.yml | 4 +++ test/fixtures/user_abilities.yml | 16 +++++++++ test/fixtures/users.yml | 14 ++++++++ test/models/concerns/user_merge_test.rb | 46 +++++++++++++++++++++++++ 6 files changed, 98 insertions(+) create mode 100644 test/models/concerns/user_merge_test.rb diff --git a/test/fixtures/community_users.yml b/test/fixtures/community_users.yml index f86d72a60..a333d475a 100644 --- a/test/fixtures/community_users.yml +++ b/test/fixtures/community_users.yml @@ -140,3 +140,17 @@ sample_post_scorer: is_admin: false is_moderator: false reputation: 1 + +sample_merge_source: + user: merge_source + community: sample + is_admin: false + is_moderator: false + reputation: 42 + +sample_merge_target: + user: merge_target + community: sample + is_admin: false + is_moderator: false + reputation: 24 diff --git a/test/fixtures/new_thread_followers.yml b/test/fixtures/new_thread_followers.yml index 0d75b44ff..04a565328 100644 --- a/test/fixtures/new_thread_followers.yml +++ b/test/fixtures/new_thread_followers.yml @@ -2,3 +2,7 @@ standard_author_question_one: user: standard_user post: question_one + +merge_source_question_one: + user: merge_source + post: question_one diff --git a/test/fixtures/thread_followers.yml b/test/fixtures/thread_followers.yml index 893c177d7..d2ff03753 100644 --- a/test/fixtures/thread_followers.yml +++ b/test/fixtures/thread_followers.yml @@ -5,3 +5,7 @@ deleter_normal: standard_author_normal: user: standard_user comment_thread: normal + +merge_source_normal: + user: merge_source + comment_thread: normal diff --git a/test/fixtures/user_abilities.yml b/test/fixtures/user_abilities.yml index 7a348256a..945e33ce7 100644 --- a/test/fixtures/user_abilities.yml +++ b/test/fixtures/user_abilities.yml @@ -30,6 +30,14 @@ pp_eo: community_user: sample_post_scorer ability: everyone +ms_eo: + community_user: sample_merge_source + ability: everyone + +mt_eo: + community_user: sample_merge_target + ability: everyone + stu_ur: community_user: sample_standard_user ability: unrestricted @@ -62,6 +70,14 @@ pp_ur: community_user: sample_post_scorer ability: unrestricted +ms_ur: + community_user: sample_merge_source + ability: unrestricted + +mt_ur: + community_user: sample_merge_target + ability: unrestricted + c_fc: community_user: sample_closer ability: flag_close diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 8ecf6f0b5..90fd06e39 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -208,3 +208,17 @@ post_scorer: sign_in_count: 1337 username: post_scorer confirmed_at: 2020-01-01T00:00:00.000000Z + +merge_source: + email: merge-source@example.com + encrypted_password: '$2a$11$roUHXKxecjyQ72Qn7DWs3.9eRCCoRn176kX/UNb/xiue3aGqf7xEW' + sign_in_count: 1337 + username: merge_source + confirmed_at: 2020-01-01T00:00:00.000000Z + +merge_target: + email: merge-target@example.com + encrypted_password: '$2a$11$roUHXKxecjyQ72Qn7DWs3.9eRCCoRn176kX/UNb/xiue3aGqf7xEW' + sign_in_count: 1337 + username: merge_target + confirmed_at: 2020-01-01T00:00:00.000000Z diff --git a/test/models/concerns/user_merge_test.rb b/test/models/concerns/user_merge_test.rb new file mode 100644 index 000000000..a63c1b875 --- /dev/null +++ b/test/models/concerns/user_merge_test.rb @@ -0,0 +1,46 @@ +require 'test_helper' + +class UserMergeTest < ActiveSupport::TestCase + test 'merge_into should destroy the old user upon success' do + merger = users(:global_admin) + src_usr = users(:merge_source) + tgt_usr = users(:merge_target) + + src_usr.merge_into(tgt_usr, merger) + + assert_raises ActiveRecord::RecordNotFound do + src_usr.reload + end + end + + test 'merge_info should move followed threads / posts to the target user' do + merger = users(:global_admin) + src_usr = users(:merge_source) + tgt_usr = users(:merge_target) + + src_new_threads_followed = NewThreadFollower.where(user: src_usr) + src_new_threads_followed_ids = src_new_threads_followed.map(&:id) + assert src_new_threads_followed_ids.any? + + src_threads_followed = ThreadFollower.where(user: src_usr) + src_threads_followed_ids = src_threads_followed.map(&:id) + assert src_threads_followed_ids.any? + + src_usr.merge_into(tgt_usr, merger) + + src_new_threads_followed.reload + src_threads_followed.reload + + assert src_new_threads_followed.none? + assert src_threads_followed.none? + + tgt_new_threads_followed = NewThreadFollower.where(id: src_new_threads_followed_ids) + tgt_threads_followed = ThreadFollower.where(id: src_threads_followed_ids) + + assert tgt_new_threads_followed.any? + assert(tgt_new_threads_followed.all? { |a| a.user.same_as?(tgt_usr) }) + + assert tgt_threads_followed.any? + assert(tgt_threads_followed.all? { |a| a.user.same_as?(tgt_usr) }) + end +end From c1b8251051084b87606e9d0e4a306acf18a2045f Mon Sep 17 00:00:00 2001 From: trichoplax Date: Fri, 12 Dec 2025 13:58:13 +0000 Subject: [PATCH 17/47] Include specific post_id index in migration --- db/migrate/20251203164131_create_new_thread_followers.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251203164131_create_new_thread_followers.rb index b71a4aac5..f2bd631be 100644 --- a/db/migrate/20251203164131_create_new_thread_followers.rb +++ b/db/migrate/20251203164131_create_new_thread_followers.rb @@ -23,6 +23,7 @@ def create_table_new_thread_followers t.timestamps end add_index :new_thread_followers, [:user_id, :post_id], if_not_exists: true + add_index :new_thread_followers, :post_id, if_not_exists: true end def move_rows_with_non_nil_post_id @@ -53,6 +54,7 @@ def move_rows_back_from_new_thread_followers end def delete_table_new_thread_followers + remove_index :new_thread_followers, :post_id, if_exists: true remove_index :new_thread_followers, [:user_id, :post_id], if_exists: true remove_reference :new_thread_followers, :user_id, foreign_key: true, if_exists: true remove_reference :new_thread_followers, :post_id, foreign_key: true, if_exists: true From a994cf8a71db5c490755346095905bfb39dd2354 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 21 Dec 2025 17:20:16 +0300 Subject: [PATCH 18/47] switched comments last_activity_at to a column --- app/models/comment.rb | 12 ++++++------ app/models/comment_thread.rb | 2 +- ...0251221140930_add_last_activity_at_to_comments.rb | 5 +++++ db/schema.rb | 3 ++- 4 files changed, 14 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20251221140930_add_last_activity_at_to_comments.rb diff --git a/app/models/comment.rb b/app/models/comment.rb index 66ac1af5b..f41186132 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -17,16 +17,12 @@ class Comment < ApplicationRecord after_create :create_follower after_update :delete_thread + before_save :bump_last_activity_at + 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 @@ -49,6 +45,10 @@ def pings private + def bump_last_activity_at + self.last_activity_at = DateTime.now + end + def create_follower if user.preference('auto_follow_comment_threads') == 'true' ThreadFollower.find_or_create_by(comment_thread: comment_thread, user: user) diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index f33af6735..6a30828cd 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -52,7 +52,7 @@ 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 + last_comment_activity_at = comments.map(&:last_activity_at).compact.max [created_at, locked_at, updated_at, last_comment_activity_at].compact.max end diff --git a/db/migrate/20251221140930_add_last_activity_at_to_comments.rb b/db/migrate/20251221140930_add_last_activity_at_to_comments.rb new file mode 100644 index 000000000..1aff88dd8 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 8d92b11e7..560478f47 100644 --- a/db/schema.rb +++ b/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_140930) do create_table "abilities", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "community_id" t.string "name" @@ -195,6 +195,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" From 940aaa1ec51c29e17b8dd25dc99e392793ee932f Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 21 Dec 2025 17:25:52 +0300 Subject: [PATCH 19/47] switched comment threads last_activity_at to a column --- app/models/comment_thread.rb | 6 ++++++ ...0251221142105_add_last_activity_at_to_comment_threads.rb | 5 +++++ db/schema.rb | 3 ++- 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20251221142105_add_last_activity_at_to_comment_threads.rb diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index 6a30828cd..be730a8e9 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -16,6 +16,8 @@ class CommentThread < ApplicationRecord after_create :create_follower + before_save :bump_last_activity_at + # Gets threads appropriately scoped for a given user & post # @param user [User, nil] user to check # @para post [Post] post to check @@ -88,6 +90,10 @@ def maximum_title_length private + def bump_last_activity_at + self.last_activity_at = DateTime.now + end + # Comment author and post author are automatically followed to the thread. Question author is NOT # automatically followed on new answer comment threads. Comment author follower creation is done # on the Comment model. diff --git a/db/migrate/20251221142105_add_last_activity_at_to_comment_threads.rb b/db/migrate/20251221142105_add_last_activity_at_to_comment_threads.rb new file mode 100644 index 000000000..231dd5bf8 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 560478f47..07e7a08a1 100644 --- a/db/schema.rb +++ b/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_12_21_140930) 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" From 0594fdd2c96de83271c00122d0fa0d02903eb120 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 03:26:55 +0300 Subject: [PATCH 20/47] added last_activity wrapper method to Comment (max of created, updated, or last activity) --- app/models/comment.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/models/comment.rb b/app/models/comment.rb index f41186132..ce6ef9d79 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -37,6 +37,12 @@ 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) From 253a35b6294a472aa97b17cbbfaae33e4f7f9df8 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 03:28:50 +0300 Subject: [PATCH 21/47] renamed last_activity_at CommentThread method to last_activity & ensured it accounts for the last_activity_at column --- app/models/comment_thread.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index be730a8e9..d49fd2298 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -53,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).compact.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. From 178e300a45aecf18daa32e7222d39c5199bcc039 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 03:30:24 +0300 Subject: [PATCH 22/47] made bump_last_activity_at methods public to be able to bump by unrelated actions --- app/models/comment.rb | 4 ++-- app/models/comment_thread.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index ce6ef9d79..2687509fb 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -49,12 +49,12 @@ def pings matches.flatten.select { |m| pingable.include?(m.to_i) }.map(&:to_i) end - private - def bump_last_activity_at self.last_activity_at = DateTime.now end + private + def create_follower if user.preference('auto_follow_comment_threads') == 'true' ThreadFollower.find_or_create_by(comment_thread: comment_thread, user: user) diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index d49fd2298..46fd014f0 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -88,12 +88,12 @@ def maximum_title_length end end - private - def bump_last_activity_at self.last_activity_at = DateTime.now end + private + # Comment author and post author are automatically followed to the thread. Question author is NOT # automatically followed on new answer comment threads. Comment author follower creation is done # on the Comment model. From cbde601ce3bfef6b5a6a3b12eaca3ab93bacb068 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 03:36:30 +0300 Subject: [PATCH 23/47] switched 'thread_content' freshness check to last_activity & added the same to 'thread' to match --- app/controllers/comments_controller.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 6c37822a4..1ece3b164 100644 --- a/app/controllers/comments_controller.rb +++ b/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', From 3f1b69e26855165ddc673cb8547b6c49488c7885 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 03:42:56 +0300 Subject: [PATCH 24/47] creating/destroying ThreadFollowers should bump their threads last activity timestamp --- app/models/thread_follower.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/models/thread_follower.rb b/app/models/thread_follower.rb index 426e1f394..91a4000ab 100644 --- a/app/models/thread_follower.rb +++ b/app/models/thread_follower.rb @@ -3,10 +3,18 @@ class ThreadFollower < ApplicationRecord belongs_to :post, optional: true belongs_to :user + after_create :bump_thread_last_activity_at + before_destroy :bump_thread_last_activity_at + validate :thread_or_post private + def bump_thread_last_activity_at + comment_thread&.bump_last_activity_at + comment_thread&.save + end + def thread_or_post if comment_thread.nil? && post.nil? errors.add(:base, 'Must refer to either a comment thread or a post.') From dff7c9f6c78534a3efb878a6b03429dd2b5881ce Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 03:48:22 +0300 Subject: [PATCH 25/47] renamed all *_last_activity_at methods to *_last_activity --- app/models/comment.rb | 4 ++-- app/models/comment_thread.rb | 4 ++-- app/models/thread_follower.rb | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 2687509fb..aa929d5b9 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -17,7 +17,7 @@ class Comment < ApplicationRecord after_create :create_follower after_update :delete_thread - before_save :bump_last_activity_at + before_save :bump_last_activity counter_culture :comment_thread, column_name: proc { |model| model.deleted? ? nil : 'reply_count' }, touch: true @@ -49,7 +49,7 @@ def pings matches.flatten.select { |m| pingable.include?(m.to_i) }.map(&:to_i) end - def bump_last_activity_at + def bump_last_activity self.last_activity_at = DateTime.now end diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index 46fd014f0..35db0e2f0 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -16,7 +16,7 @@ class CommentThread < ApplicationRecord after_create :create_follower - before_save :bump_last_activity_at + before_save :bump_last_activity # Gets threads appropriately scoped for a given user & post # @param user [User, nil] user to check @@ -88,7 +88,7 @@ def maximum_title_length end end - def bump_last_activity_at + def bump_last_activity self.last_activity_at = DateTime.now end diff --git a/app/models/thread_follower.rb b/app/models/thread_follower.rb index 91a4000ab..ae30a79b7 100644 --- a/app/models/thread_follower.rb +++ b/app/models/thread_follower.rb @@ -3,15 +3,15 @@ class ThreadFollower < ApplicationRecord belongs_to :post, optional: true belongs_to :user - after_create :bump_thread_last_activity_at - before_destroy :bump_thread_last_activity_at + after_create :bump_thread_last_activity + before_destroy :bump_thread_last_activity validate :thread_or_post private - def bump_thread_last_activity_at - comment_thread&.bump_last_activity_at + def bump_thread_last_activity + comment_thread&.bump_last_activity comment_thread&.save end From 66e035eb9831d8bba351406d2f85353b8251b436 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 04:17:23 +0300 Subject: [PATCH 26/47] [unrelated] fixed test description for the pings method --- test/models/comment_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb index 72a28d95f..50022d775 100644 --- a/test/models/comment_test.rb +++ b/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) From c73eb26a63b8bdb6180d968f82ded1e1935892f0 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 04:28:01 +0300 Subject: [PATCH 27/47] [unrelated] fixed User#anonymize confusing inverted persist_changes param behavior due to rename from 'dirty' --- app/models/user.rb | 6 +++--- test/models/user_test.rb | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 5ad2aeb2c..5e1607028 100644 --- a/app/models/user.rb +++ b/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 diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 8ea34b2d9..c92638738 100644 --- a/test/models/user_test.rb +++ b/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 From c12ef0c76d4a23c164f51c26c6c087fa6f878533 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 04:59:59 +0300 Subject: [PATCH 28/47] added tests for Comment#last_activity method --- test/fixtures/comments.yml | 9 +++++++++ test/models/comment_test.rb | 14 ++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/test/fixtures/comments.yml b/test/fixtures/comments.yml index 5f1ecbd47..709d6f9ca 100644 --- a/test/fixtures/comments.yml +++ b/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 diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb index 50022d775..ca6a8d52d 100644 --- a/test/models/comment_test.rb +++ b/test/models/comment_test.rb @@ -43,4 +43,18 @@ 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 + assert_operator last_activity_after_update, '<=', comment.last_activity + end end From bbf383a2953faf2e5893318455a6781296694a22 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 05:07:17 +0300 Subject: [PATCH 29/47] reworked CommentThread#last_activity_at test into the new CommentThread#last_activity test --- test/fixtures/comment_threads.yml | 8 ++++++++ test/models/comment_thread_test.rb | 19 ++++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/test/fixtures/comment_threads.yml b/test/fixtures/comment_threads.yml index b4bb74dfb..1d9bea300 100644 --- a/test/fixtures/comment_threads.yml +++ b/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 diff --git a/test/models/comment_thread_test.rb b/test/models/comment_thread_test.rb index aafddaf42..0d49fefb0 100644 --- a/test/models/comment_thread_test.rb +++ b/test/models/comment_thread_test.rb @@ -16,16 +16,17 @@ 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 + assert_operator last_activity_after_update, '<=', thread.last_activity end end From aa48e589fd00287d93d4f32269c2c04bf58b824a Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 05:15:08 +0300 Subject: [PATCH 30/47] added CommentThread#add_follower QoL method & expanded last_activity test accordingly --- app/controllers/comments_controller.rb | 2 +- app/models/comment_thread.rb | 7 +++++++ test/models/comment_thread_test.rb | 7 ++++++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 1ece3b164..a46084d1d 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -266,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 diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index 35db0e2f0..65814a2c1 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -88,6 +88,13 @@ 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 + def bump_last_activity self.last_activity_at = DateTime.now end diff --git a/test/models/comment_thread_test.rb b/test/models/comment_thread_test.rb index 0d49fefb0..e48cf4f2c 100644 --- a/test/models/comment_thread_test.rb +++ b/test/models/comment_thread_test.rb @@ -27,6 +27,11 @@ class CommentThreadTest < ActiveSupport::TestCase assert_operator thread.updated_at, '<=', last_activity_after_update thread.bump_last_activity - assert_operator last_activity_after_update, '<=', thread.last_activity + last_activity_after_bump = thread.last_activity + assert_operator last_activity_after_update, '<=', last_activity_after_bump + + thread.add_follower(users(:editor)) + thread.reload + assert_operator last_activity_after_bump, '<=', thread.last_activity end end From 0f32bf4a54f66758a0259aaa310851817794dee1 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 05:20:53 +0300 Subject: [PATCH 31/47] added CommentThread#remove_follower QoL method & expanded last_activity test accordingly --- app/controllers/comments_controller.rb | 2 +- app/models/comment_thread.rb | 7 +++++++ test/models/comment_thread_test.rb | 9 +++++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index a46084d1d..cca1263b0 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -295,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 diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index 65814a2c1..cd2a6aaaf 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -99,6 +99,13 @@ def bump_last_activity self.last_activity_at = DateTime.now 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.find_by(comment_thread: self, user: user)&.destroy || false + end + private # Comment author and post author are automatically followed to the thread. Question author is NOT diff --git a/test/models/comment_thread_test.rb b/test/models/comment_thread_test.rb index e48cf4f2c..6d6147002 100644 --- a/test/models/comment_thread_test.rb +++ b/test/models/comment_thread_test.rb @@ -28,10 +28,15 @@ class CommentThreadTest < ActiveSupport::TestCase thread.bump_last_activity last_activity_after_bump = thread.last_activity - assert_operator last_activity_after_update, '<=', last_activity_after_bump + assert_operator last_activity_after_update, '<', last_activity_after_bump thread.add_follower(users(:editor)) thread.reload - assert_operator last_activity_after_bump, '<=', thread.last_activity + 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 From be1f95256e145040b709db71d280a41f29da7472 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 05:24:22 +0300 Subject: [PATCH 32/47] CommentThread#bump_last_activity should have the option to persist changes --- app/models/comment_thread.rb | 8 +++++++- app/models/thread_follower.rb | 3 +-- test/models/comment_thread_test.rb | 3 ++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index cd2a6aaaf..df8c389e3 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -95,8 +95,14 @@ def add_follower(user) ThreadFollower.create(comment_thread: self, user: user) end - def bump_last_activity + # 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 diff --git a/app/models/thread_follower.rb b/app/models/thread_follower.rb index ae30a79b7..562dbc53d 100644 --- a/app/models/thread_follower.rb +++ b/app/models/thread_follower.rb @@ -11,8 +11,7 @@ class ThreadFollower < ApplicationRecord private def bump_thread_last_activity - comment_thread&.bump_last_activity - comment_thread&.save + comment_thread&.bump_last_activity(persist_changes: true) end def thread_or_post diff --git a/test/models/comment_thread_test.rb b/test/models/comment_thread_test.rb index 6d6147002..6101e0609 100644 --- a/test/models/comment_thread_test.rb +++ b/test/models/comment_thread_test.rb @@ -26,7 +26,8 @@ class CommentThreadTest < ActiveSupport::TestCase last_activity_after_update = thread.last_activity assert_operator thread.updated_at, '<=', last_activity_after_update - thread.bump_last_activity + 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 From 18142a421e05817cef1cda8b9f19d88e5499fdf6 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 22 Dec 2025 05:27:28 +0300 Subject: [PATCH 33/47] Comment#bump_last_activity should have the option to persist changes --- app/models/comment.rb | 8 +++++++- test/models/comment_test.rb | 7 ++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index aa929d5b9..2ecb237b3 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -49,8 +49,14 @@ def pings matches.flatten.select { |m| pingable.include?(m.to_i) }.map(&:to_i) end - def bump_last_activity + # 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 diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb index ca6a8d52d..bab89bf8d 100644 --- a/test/models/comment_test.rb +++ b/test/models/comment_test.rb @@ -52,9 +52,10 @@ class CommentTest < ActiveSupport::TestCase 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 + assert_operator comment.updated_at, '<', last_activity_after_update - comment.bump_last_activity - assert_operator last_activity_after_update, '<=', comment.last_activity + comment.bump_last_activity(persist_changes: true) + comment.reload + assert_operator last_activity_after_update, '<', comment.last_activity end end From 74eaa4cc80a41d418be7b3b1d01b29a4276cc366 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 23 Dec 2025 01:52:36 +0300 Subject: [PATCH 34/47] CommentThread#remove_follower should remove all followers --- .rubocop.yml | 2 +- app/models/comment_thread.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 7c85865b5..86ff8fdc5 100644 --- a/.rubocop.yml +++ b/.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 diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index df8c389e3..d61590498 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -109,7 +109,7 @@ def bump_last_activity(persist_changes: false) # @param user [User] user to remove from followers # @return [Boolean] status of the operation def remove_follower(user) - ThreadFollower.find_by(comment_thread: self, user: user)&.destroy || false + ThreadFollower.where(comment_thread: self, user: user).destroy_all.any? end private From d06c750d658e584e1fc052e61a7990ac0736b7f2 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 23 Dec 2025 03:29:36 +0300 Subject: [PATCH 35/47] add_follower should prevent duplicate thread follower records --- app/models/comment_thread.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index d61590498..8ede13a58 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -92,7 +92,9 @@ def maximum_title_length # @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) + if ThreadFollower.where(comment_thread: self, user: user).none? + ThreadFollower.create(comment_thread: self, user: user) + end end # Directly bumps the thread's last activity date & time From a4d6b95ce442be29ad07dd2aaec66b6adb44ee24 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 23 Dec 2025 03:43:29 +0300 Subject: [PATCH 36/47] moved unfollowing coment threads from unrestrict_thread to its own unfollow_thread action --- app/assets/javascripts/comments.js | 15 +++++++++++++++ app/assets/javascripts/qpixel_api.js | 8 ++++++++ app/controllers/comments_controller.rb | 3 +-- app/views/comments/_thread_follow_link.html.erb | 3 +-- config/routes.rb | 1 + global.d.ts | 7 +++++++ test/comments_test_helpers.rb | 2 +- 7 files changed, 34 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/comments.js b/app/assets/javascripts/comments.js index e56a4b428..ff6f9023b 100644 --- a/app/assets/javascripts/comments.js +++ b/app/assets/javascripts/comments.js @@ -255,6 +255,21 @@ $(() => { }); }); + $(document).on('click', '.js--unfollow-thread', async (ev) => { + ev.preventDefault(); + + const $tgt = $(ev.target); + const threadID = $tgt.data('thread'); + + const data = await QPixel.unfollowThread(threadID); + + QPixel.handleJSONResponse(data, () => { + const wrapper = getCommentThreadWrapper($tgt); + const inline = isInlineCommentThread(wrapper); + openThread(wrapper, threadID, { inline }); + }); + }) + /** * @param {Element} target */ diff --git a/app/assets/javascripts/qpixel_api.js b/app/assets/javascripts/qpixel_api.js index 51a9336e6..b9d0d4a08 100644 --- a/app/assets/javascripts/qpixel_api.js +++ b/app/assets/javascripts/qpixel_api.js @@ -506,6 +506,14 @@ window.QPixel = { return QPixel.parseJSONResponse(resp, 'Failed to follow thread'); }, + unfollowThread: async (id) => { + const resp = await QPixel.fetchJSON(`/comments/thread/${id}/unfollow`, {}, { + headers: { 'Accept': 'application/json' }, + }); + + return QPixel.parseJSONResponse(resp, 'Failed to unfollow thread'); + }, + lockThread: async (id, duration) => { const resp = await QPixel.fetchJSON(`/comments/thread/${id}/lock`, { duration, diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index cca1263b0..6cc1078b7 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -12,6 +12,7 @@ class CommentsController < ApplicationController :archive_thread, :delete_thread, :follow_thread, + :unfollow_thread, :lock_thread, :thread_unrestrict, :thread_followers] @@ -313,8 +314,6 @@ def thread_unrestrict unarchive_thread when 'delete' undelete_thread - when 'follow' - unfollow_thread else not_found! end diff --git a/app/views/comments/_thread_follow_link.html.erb b/app/views/comments/_thread_follow_link.html.erb index 1a2f46f4e..e99ac05b3 100644 --- a/app/views/comments/_thread_follow_link.html.erb +++ b/app/views/comments/_thread_follow_link.html.erb @@ -8,8 +8,7 @@ <% if thread.followed_by?(user) %> Promise + /** + * Attempts to unfollow a comment thread + * @param id id of the thread to unfollow + * @returns result of the operation + */ + unfollowThread?: (id: string) => Promise + /** * Attempts to start following comments on a given post * @param postId id of the post to follow comments on diff --git a/test/comments_test_helpers.rb b/test/comments_test_helpers.rb index 8f7a14ae5..4ca0e1da1 100644 --- a/test/comments_test_helpers.rb +++ b/test/comments_test_helpers.rb @@ -68,7 +68,7 @@ def try_follow_thread(thread) # Attempts to unfollow a given comment thread # @param thread [CommentThread] thread to unfollow def try_unfollow_thread(thread) - post :thread_unrestrict, params: { id: thread.id, type: 'follow' } + post :unfollow_thread, params: { id: thread.id } end # Attempts to follow new threads on a given post From 8f5b89479b1eb2d72c7f202a6cf106c17944c780 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 23 Dec 2025 03:51:37 +0300 Subject: [PATCH 37/47] let's not fail if add_follower / remove_follower is a noop --- app/models/comment_thread.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index 8ede13a58..4403bda2d 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -92,9 +92,9 @@ def maximum_title_length # @param user [User] user to register as a follower # @return [Boolean] status of the operation def add_follower(user) - if ThreadFollower.where(comment_thread: self, user: user).none? - ThreadFollower.create(comment_thread: self, user: user) - end + return true if ThreadFollower.where(comment_thread: self, user: user).any? + + ThreadFollower.create(comment_thread: self, user: user) end # Directly bumps the thread's last activity date & time @@ -111,6 +111,8 @@ def bump_last_activity(persist_changes: false) # @param user [User] user to remove from followers # @return [Boolean] status of the operation def remove_follower(user) + return true if ThreadFollower.where(comment_thread: self, user: user).none? + ThreadFollower.where(comment_thread: self, user: user).destroy_all.any? end From 76f56551d7c0f8ac3df37867519d18e294d84d48 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 23 Dec 2025 04:44:51 +0300 Subject: [PATCH 38/47] added job for cleaning up duplicate thread followers --- app/jobs/clean_up_thread_followers_job.rb | 24 +++++++++++++++++++ config/schedule.rb | 4 ++++ .../threads_with_duplicate_followers.sql | 13 ++++++++++ scripts/run_thread_followers_cleanup.rb | 1 + test/fixtures/comment_threads.yml | 7 ++++++ .../clean_up_thread_followers_job_test.rb | 19 +++++++++++++++ 6 files changed, 68 insertions(+) create mode 100644 app/jobs/clean_up_thread_followers_job.rb create mode 100644 db/scripts/threads_with_duplicate_followers.sql create mode 100644 scripts/run_thread_followers_cleanup.rb create mode 100644 test/jobs/clean_up_thread_followers_job_test.rb diff --git a/app/jobs/clean_up_thread_followers_job.rb b/app/jobs/clean_up_thread_followers_job.rb new file mode 100644 index 000000000..1f3c23945 --- /dev/null +++ b/app/jobs/clean_up_thread_followers_job.rb @@ -0,0 +1,24 @@ +class CleanUpThreadFollowersJob < ApplicationJob + queue_as :default + + def perform + sql = File.read(Rails.root.join('db/scripts/threads_with_duplicate_followers.sql')) + threads = ActiveRecord::Base.connection.execute(sql).to_a + + threads.each do |thread| + user_id, thread_id = thread + + followers = ThreadFollower.where(comment_thread_id: thread_id, user_id: user_id) + + next unless followers.many? + + duplicate = followers.first + result = duplicate.destroy + + unless result + puts "failed to destroy thread follower duplicate \"#{duplicate.id}\"" + duplicate.errors.each { |e| puts e.full_message } + end + end + end +end diff --git a/config/schedule.rb b/config/schedule.rb index 5dec460ac..07092bdbb 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -26,6 +26,10 @@ runner 'scripts/run_complaints_closure.rb' end +every 7.days, at: '02:35' do + runner 'scripts/run_thread_followers_cleanup.rb' +end + every 6.hours do runner 'scripts/recalc_abilities.rb' end diff --git a/db/scripts/threads_with_duplicate_followers.sql b/db/scripts/threads_with_duplicate_followers.sql new file mode 100644 index 000000000..7a87fa9a8 --- /dev/null +++ b/db/scripts/threads_with_duplicate_followers.sql @@ -0,0 +1,13 @@ +select + user_id, + comment_thread_id, + count(*) as count +from + thread_followers +where + post_id is null +group by + user_id, + comment_thread_id +having + count > 1; diff --git a/scripts/run_thread_followers_cleanup.rb b/scripts/run_thread_followers_cleanup.rb new file mode 100644 index 000000000..06bbee308 --- /dev/null +++ b/scripts/run_thread_followers_cleanup.rb @@ -0,0 +1 @@ +CleanUpThreadFollowersJob.perform_later diff --git a/test/fixtures/comment_threads.yml b/test/fixtures/comment_threads.yml index 1d9bea300..6a150cbf3 100644 --- a/test/fixtures/comment_threads.yml +++ b/test/fixtures/comment_threads.yml @@ -82,3 +82,10 @@ without_activity: community: sample created_at: 2020-01-01T00:00:00.000000Z updated_at: 2020-01-01T00:00:00.000000Z + +without_followers: + title: Comment thread without any followers + title: Comment thread without activity + reply_count: 0 + post: question_one + community: sample diff --git a/test/jobs/clean_up_thread_followers_job_test.rb b/test/jobs/clean_up_thread_followers_job_test.rb new file mode 100644 index 000000000..79d1a7993 --- /dev/null +++ b/test/jobs/clean_up_thread_followers_job_test.rb @@ -0,0 +1,19 @@ +require 'test_helper' + +class CleanUpThreadFollowersJobTest < ActiveJob::TestCase + test 'should correctly clean up thread followers' do + thread = comment_threads(:without_followers) + std_usr = users(:standard_user) + + ThreadFollower.create(comment_thread: thread, user: std_usr) + ThreadFollower.create(comment_thread: thread, user: std_usr) + assert_equal 2, ThreadFollower.where(comment_thread: thread, user: std_usr).size + + perform_enqueued_jobs do + CleanUpThreadFollowersJob.perform_later + end + + assert_performed_jobs 1 + assert_equal 1, ThreadFollower.where(comment_thread: thread, user: std_usr).size + end +end From b6d0a527ebb3820f94a05a5cd8aafabb20aa3977 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 23 Dec 2025 09:45:58 +0300 Subject: [PATCH 39/47] even if there's nothing to do, bump thread's last activity to improve user experience --- app/models/comment_thread.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/models/comment_thread.rb b/app/models/comment_thread.rb index 4403bda2d..68d40af67 100644 --- a/app/models/comment_thread.rb +++ b/app/models/comment_thread.rb @@ -92,7 +92,10 @@ def maximum_title_length # @param user [User] user to register as a follower # @return [Boolean] status of the operation def add_follower(user) - return true if ThreadFollower.where(comment_thread: self, user: user).any? + if ThreadFollower.where(comment_thread: self, user: user).any? + bump_last_activity(persist_changes: true) + return true + end ThreadFollower.create(comment_thread: self, user: user) end @@ -111,7 +114,10 @@ def bump_last_activity(persist_changes: false) # @param user [User] user to remove from followers # @return [Boolean] status of the operation def remove_follower(user) - return true if ThreadFollower.where(comment_thread: self, user: user).none? + if ThreadFollower.where(comment_thread: self, user: user).none? + bump_last_activity(persist_changes: true) + return true + end ThreadFollower.where(comment_thread: self, user: user).destroy_all.any? end From d2f2a81f551ed26cb8583f40d6d1fc66e60fb973 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 23 Dec 2025 10:17:29 +0300 Subject: [PATCH 40/47] added tests covering CommentThread#remove_follower last activity bumping --- test/models/comment_thread_test.rb | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/test/models/comment_thread_test.rb b/test/models/comment_thread_test.rb index 6101e0609..721543def 100644 --- a/test/models/comment_thread_test.rb +++ b/test/models/comment_thread_test.rb @@ -16,9 +16,28 @@ class CommentThreadTest < ActiveSupport::TestCase end end - test 'last_activity should correctly get the thread\'s last activity date & time' do + test 'remove_follower should correctly bump last_activity' do + std_usr = users(:standard_user) thread = comment_threads(:without_activity) + assert_equal thread.created_at, thread.last_activity + + thread.add_follower(std_usr) + thread.reload + last_activity_after_follow = thread.last_activity + + thread.remove_follower(std_usr) + thread.reload + last_activity_after_unfollow = thread.last_activity + assert_operator last_activity_after_follow, '<', last_activity_after_unfollow + + thread.remove_follower(std_usr) + thread.reload + last_activity_after_noop = thread.last_activity + assert_operator last_activity_after_unfollow, '<', last_activity_after_noop + end + test 'last_activity should correctly get the thread\'s last activity date & time' do + thread = comment_threads(:without_activity) assert_equal thread.created_at, thread.last_activity thread.update!(title: 'this should bump last_activity') From 8a6e63b64857113d809f99c0e498513c757afb8c Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Tue, 23 Dec 2025 10:36:18 +0300 Subject: [PATCH 41/47] thread wrapper can be absent at the time of handling follow/unfollow success when clicking too fast --- app/assets/javascripts/comments.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/comments.js b/app/assets/javascripts/comments.js index ff6f9023b..e520674c7 100644 --- a/app/assets/javascripts/comments.js +++ b/app/assets/javascripts/comments.js @@ -250,8 +250,11 @@ $(() => { QPixel.handleJSONResponse(data, () => { const wrapper = getCommentThreadWrapper($tgt); - const inline = isInlineCommentThread(wrapper); - openThread(wrapper, threadID, { inline }); + + if (wrapper) { + const inline = isInlineCommentThread(wrapper); + openThread(wrapper, threadID, { inline }); + } }); }); @@ -265,8 +268,11 @@ $(() => { QPixel.handleJSONResponse(data, () => { const wrapper = getCommentThreadWrapper($tgt); - const inline = isInlineCommentThread(wrapper); - openThread(wrapper, threadID, { inline }); + + if (wrapper) { + const inline = isInlineCommentThread(wrapper); + openThread(wrapper, threadID, { inline }); + } }); }) From 74b2c2ea1c6c4b533170f216c558c770f462bc4b Mon Sep 17 00:00:00 2001 From: trichoplax Date: Fri, 26 Dec 2025 18:59:39 +0000 Subject: [PATCH 42/47] Rename migration to later before merging an earlier one --- ...> 20251226185531_create_new_thread_followers.rb} | 0 db/schema.rb | 13 ++++--------- 2 files changed, 4 insertions(+), 9 deletions(-) rename db/migrate/{20251203164131_create_new_thread_followers.rb => 20251226185531_create_new_thread_followers.rb} (100%) diff --git a/db/migrate/20251203164131_create_new_thread_followers.rb b/db/migrate/20251226185531_create_new_thread_followers.rb similarity index 100% rename from db/migrate/20251203164131_create_new_thread_followers.rb rename to db/migrate/20251226185531_create_new_thread_followers.rb diff --git a/db/schema.rb b/db/schema.rb index 978480338..8d92b11e7 100644 --- a/db/schema.rb +++ b/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_12_03_164131) do +ActiveRecord::Schema[7.2].define(version: 2025_10_15_121326) do create_table "abilities", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "community_id" t.string "name" @@ -401,14 +401,6 @@ t.index ["user_id"], name: "index_micro_auth_tokens_on_user_id" end - create_table "new_thread_followers", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| - t.bigint "user_id" - t.bigint "post_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["user_id", "post_id"], name: "index_new_thread_followers_on_user_id_and_post_id" - end - create_table "notifications", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.text "content" t.string "link" @@ -731,7 +723,9 @@ t.bigint "user_id" t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false + t.bigint "post_id" t.index ["comment_thread_id"], name: "index_thread_followers_on_comment_thread_id" + t.index ["post_id"], name: "index_thread_followers_on_post_id" t.index ["user_id"], name: "index_thread_followers_on_user_id" end @@ -899,6 +893,7 @@ add_foreign_key "tag_synonyms", "tags" add_foreign_key "tags", "communities" add_foreign_key "tags", "tags", column: "parent_id" + add_foreign_key "thread_followers", "posts" add_foreign_key "user_abilities", "abilities" add_foreign_key "user_abilities", "community_users" add_foreign_key "user_websites", "users" From 0304650a87fba45c0b219cc2edc83734e8f19a02 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Fri, 26 Dec 2025 19:35:33 +0000 Subject: [PATCH 43/47] Remove post_id condition since column has been removed --- db/scripts/threads_with_duplicate_followers.sql | 2 -- 1 file changed, 2 deletions(-) diff --git a/db/scripts/threads_with_duplicate_followers.sql b/db/scripts/threads_with_duplicate_followers.sql index 7a87fa9a8..ad370956f 100644 --- a/db/scripts/threads_with_duplicate_followers.sql +++ b/db/scripts/threads_with_duplicate_followers.sql @@ -4,8 +4,6 @@ select count(*) as count from thread_followers -where - post_id is null group by user_id, comment_thread_id From b030287d98d5f42e407237ec16f1864e96b9b79f Mon Sep 17 00:00:00 2001 From: trichoplax Date: Fri, 26 Dec 2025 19:36:42 +0000 Subject: [PATCH 44/47] Add job for new thread followers duplicates following split --- app/jobs/clean_up_new_thread_followers_job.rb | 24 +++++++++++++++++++ ...ts_with_duplicate_new_thread_followers.sql | 11 +++++++++ 2 files changed, 35 insertions(+) create mode 100644 app/jobs/clean_up_new_thread_followers_job.rb create mode 100644 db/scripts/posts_with_duplicate_new_thread_followers.sql diff --git a/app/jobs/clean_up_new_thread_followers_job.rb b/app/jobs/clean_up_new_thread_followers_job.rb new file mode 100644 index 000000000..a72ff2f57 --- /dev/null +++ b/app/jobs/clean_up_new_thread_followers_job.rb @@ -0,0 +1,24 @@ +class CleanUpNewThreadFollowersJob < ApplicationJob + queue_as :default + + def perform + sql = File.read(Rails.root.join('db/scripts/posts_with_duplicate_new_thread_followers.sql')) + posts = ActiveRecord::Base.connection.execute(sql).to_a + + posts.each do |post| + user_id, post_id = post + + followers = NewThreadFollower.where(post_id: post_id, user_id: user_id) + + next unless followers.many? + + duplicate = followers.first + result = duplicate.destroy + + unless result + puts "failed to destroy new thread follower duplicate \"#{duplicate.id}\"" + duplicate.errors.each { |e| puts e.full_message } + end + end + end +end diff --git a/db/scripts/posts_with_duplicate_new_thread_followers.sql b/db/scripts/posts_with_duplicate_new_thread_followers.sql new file mode 100644 index 000000000..26c58ccf0 --- /dev/null +++ b/db/scripts/posts_with_duplicate_new_thread_followers.sql @@ -0,0 +1,11 @@ +select + user_id, + post_id, + count(*) as count +from + new_thread_followers +group by + user_id, + post_id +having + count > 1; From c19a3a7506d3c5eb562eb274ce3916a21e31d87b Mon Sep 17 00:00:00 2001 From: trichoplax Date: Fri, 26 Dec 2025 19:41:04 +0000 Subject: [PATCH 45/47] Remove validation now that post_id column has been removed --- app/models/thread_follower.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/models/thread_follower.rb b/app/models/thread_follower.rb index c4c65222b..efad9415f 100644 --- a/app/models/thread_follower.rb +++ b/app/models/thread_follower.rb @@ -5,17 +5,9 @@ class ThreadFollower < ApplicationRecord 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.') - end - end end From f94f99f6537c6b28ee6effac5e0dc887773d7735 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Fri, 26 Dec 2025 19:45:59 +0000 Subject: [PATCH 46/47] Add new runner script for new thread followers duplicate job --- config/schedule.rb | 4 ++++ scripts/run_new_thread_followers_cleanup.rb | 1 + 2 files changed, 5 insertions(+) create mode 100644 scripts/run_new_thread_followers_cleanup.rb diff --git a/config/schedule.rb b/config/schedule.rb index 07092bdbb..d8472d7b7 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -30,6 +30,10 @@ runner 'scripts/run_thread_followers_cleanup.rb' end +every 7.days, at: '02:40' do + runner 'scripts/run_new_thread_followers_cleanup.rb' +end + every 6.hours do runner 'scripts/recalc_abilities.rb' end diff --git a/scripts/run_new_thread_followers_cleanup.rb b/scripts/run_new_thread_followers_cleanup.rb new file mode 100644 index 000000000..73305dbde --- /dev/null +++ b/scripts/run_new_thread_followers_cleanup.rb @@ -0,0 +1 @@ +CleanUpNewThreadFollowersJob.perform_later From 393e1c10da5ff3296b9bcbb2766b85e9b9724c12 Mon Sep 17 00:00:00 2001 From: trichoplax Date: Fri, 26 Dec 2025 20:09:32 +0000 Subject: [PATCH 47/47] Update schema following migration --- db/schema.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 07e7a08a1..8752d5420 100644 --- a/db/schema.rb +++ b/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_12_21_142105) do +ActiveRecord::Schema[7.2].define(version: 2025_12_26_185531) do create_table "abilities", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "community_id" t.string "name" @@ -403,6 +403,15 @@ t.index ["user_id"], name: "index_micro_auth_tokens_on_user_id" end + create_table "new_thread_followers", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| + t.bigint "user_id" + t.bigint "post_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["post_id"], name: "index_new_thread_followers_on_post_id" + t.index ["user_id", "post_id"], name: "index_new_thread_followers_on_user_id_and_post_id" + end + create_table "notifications", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.text "content" t.string "link" @@ -725,9 +734,7 @@ t.bigint "user_id" t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false - t.bigint "post_id" t.index ["comment_thread_id"], name: "index_thread_followers_on_comment_thread_id" - t.index ["post_id"], name: "index_thread_followers_on_post_id" t.index ["user_id"], name: "index_thread_followers_on_user_id" end @@ -895,7 +902,6 @@ add_foreign_key "tag_synonyms", "tags" add_foreign_key "tags", "communities" add_foreign_key "tags", "tags", column: "parent_id" - add_foreign_key "thread_followers", "posts" add_foreign_key "user_abilities", "abilities" add_foreign_key "user_abilities", "community_users" add_foreign_key "user_websites", "users"