diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 6cc1078b7..f30af7ce1 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -60,11 +60,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) @@ -331,8 +331,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| @@ -342,7 +342,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/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/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/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/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/app/models/thread_follower.rb b/app/models/thread_follower.rb index 562dbc53d..efad9415f 100644 --- a/app/models/thread_follower.rb +++ b/app/models/thread_follower.rb @@ -1,22 +1,13 @@ class ThreadFollower < ApplicationRecord - belongs_to :comment_thread, optional: true - belongs_to :post, optional: true + belongs_to :comment_thread 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.') - end - end end 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/db/migrate/20251226185531_create_new_thread_followers.rb b/db/migrate/20251226185531_create_new_thread_followers.rb new file mode 100644 index 000000000..f2bd631be --- /dev/null +++ b/db/migrate/20251226185531_create_new_thread_followers.rb @@ -0,0 +1,63 @@ +class CreateNewThreadFollowers < ActiveRecord::Migration[7.2] + def up + create_table_new_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 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 + t.bigint :post_id + + 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 + NewThreadFollower.insert_all( + 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 + + def remove_post_id_column_from_thread_followers + 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, if_not_exists: 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, :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 + drop_table :new_thread_followers, if_exists: true + end +end 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" 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; 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 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 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/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 new file mode 100644 index 000000000..04a565328 --- /dev/null +++ b/test/fixtures/new_thread_followers.yml @@ -0,0 +1,8 @@ +# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html +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/posts.yml b/test/fixtures/posts.yml index e1afa8a62..bb001e98e 100644 --- a/test/fixtures/posts.yml +++ b/test/fixtures/posts.yml @@ -547,3 +547,17 @@ deleted_spam_question: deleted: true deleted_at: 2019-01-01T00:00:00.000000Z deleted_by: moderator + +without_new_thread_followers: + post_type: question + title: This post does not have any new thread followers + body: This is the body of the post used to test new thread followers + body_markdown:
This is the body of the post used to test new thread followers
+ tags_cache: + - support + tags: + - test + user: standard_user + community: sample + category: main + license: cc_by_sa diff --git a/test/fixtures/thread_followers.yml b/test/fixtures/thread_followers.yml index b51e03b64..d2ff03753 100644 --- a/test/fixtures/thread_followers.yml +++ b/test/fixtures/thread_followers.yml @@ -6,6 +6,6 @@ standard_author_normal: user: standard_user comment_thread: normal -standard_author_question_one: - user: standard_user - post: question_one +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/jobs/clean_up_new_thread_followers_job_test.rb b/test/jobs/clean_up_new_thread_followers_job_test.rb new file mode 100644 index 000000000..dba7abe57 --- /dev/null +++ b/test/jobs/clean_up_new_thread_followers_job_test.rb @@ -0,0 +1,19 @@ +require 'test_helper' + +class CleanUpNewThreadFollowersJobTest < ActiveJob::TestCase + test 'should correctly clean up new thread followers' do + post = posts(:without_new_thread_followers) + std_usr = users(:standard_user) + + NewThreadFollower.create(post: post, user: std_usr) + NewThreadFollower.create(post: post, user: std_usr) + assert_equal 2, NewThreadFollower.where(post: post, user: std_usr).size + + perform_enqueued_jobs do + CleanUpNewThreadFollowersJob.perform_later + end + + assert_performed_jobs 1 + assert_equal 1, NewThreadFollower.where(post: post, user: std_usr).size + end +end 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 diff --git a/test/models/new_thread_follower_test.rb b/test/models/new_thread_follower_test.rb new file mode 100644 index 000000000..5a6d8bed2 --- /dev/null +++ b/test/models/new_thread_follower_test.rb @@ -0,0 +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) + 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..b50fa8043 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 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,