diff --git a/app/models/post_mover.rb b/app/models/post_mover.rb index 5ac0a416a5..6c494810dc 100644 --- a/app/models/post_mover.rb +++ b/app/models/post_mover.rb @@ -76,6 +76,7 @@ class PostMover update_user_actions update_last_post_stats update_upload_security_status + update_bookmarks if moving_all_posts @original_topic.update_status('closed', true, @user) @@ -491,6 +492,10 @@ class PostMover end end + def update_bookmarks + Bookmark.where(post_id: post_ids).update_all(topic_id: @destination_topic.id) + end + def watch_new_topic if @destination_topic.archetype == Archetype.private_message if @original_topic.archetype == Archetype.private_message diff --git a/db/post_migrate/20210328233843_fix_bookmarks_with_incorrect_topic_id.rb b/db/post_migrate/20210328233843_fix_bookmarks_with_incorrect_topic_id.rb new file mode 100644 index 0000000000..0c735dce62 --- /dev/null +++ b/db/post_migrate/20210328233843_fix_bookmarks_with_incorrect_topic_id.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class FixBookmarksWithIncorrectTopicId < ActiveRecord::Migration[6.0] + def up + result = DB.exec(<<~SQL) + UPDATE bookmarks bm + SET topic_id = subquery.correct_topic_id, updated_at = NOW() + FROM ( + SELECT bookmarks.id AS bookmark_id, bookmarks.post_id, bookmarks.topic_id, + posts.topic_id AS correct_topic_id + FROM bookmarks + INNER JOIN posts ON posts.id = bookmarks.post_id + WHERE posts.topic_id != bookmarks.topic_id + ) AS subquery + WHERE bm.id = subquery.bookmark_id; + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index 018c850a3e..0cf705f7b2 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -385,6 +385,20 @@ describe PostMover do .to contain_exactly([1, 500], [2, 750]) end + it "updates bookmark topic_ids to the new topic id and does not affect bookmarks for posts not moving" do + some_user = Fabricate(:user) + new_post = Fabricate(:post, topic: p1.topic) + bookmark1 = Fabricate(:bookmark, topic: p1.topic, post: p1, user: some_user) + bookmark2 = Fabricate(:bookmark, topic: p4.topic, post: p4) + bookmark3 = Fabricate(:bookmark, topic: p4.topic, post: p4) + bookmark4 = Fabricate(:bookmark, topic: new_post.topic, post: new_post) + new_topic = topic.move_posts(user, [p1.id, p4.id], title: "new testing topic name") + expect(bookmark1.reload.topic_id).to eq(new_topic.id) + expect(bookmark2.reload.topic_id).to eq(new_topic.id) + expect(bookmark3.reload.topic_id).to eq(new_topic.id) + expect(bookmark4.reload.topic_id).to eq(new_post.topic_id) + end + context "read state and other stats per user" do def create_topic_user(user, opts = {}) notification_level = opts.delete(:notification_level) || :regular @@ -600,6 +614,20 @@ describe PostMover do expect(Notification.exists?(admin_notification.id)).to eq(true) end + it "updates bookmark topic_ids to the new topic id and does not affect bookmarks for posts not moving" do + some_user = Fabricate(:user) + new_post = Fabricate(:post, topic: p3.topic) + bookmark1 = Fabricate(:bookmark, topic: p3.topic, post: p3, user: some_user) + bookmark2 = Fabricate(:bookmark, topic: p3.topic, post: p3) + bookmark3 = Fabricate(:bookmark, topic: p3.topic, post: p3) + bookmark4 = Fabricate(:bookmark, topic: new_post.topic, post: new_post) + topic.move_posts(user, [p3.id], destination_topic_id: destination_topic.id) + expect(bookmark1.reload.topic_id).to eq(destination_topic.id) + expect(bookmark2.reload.topic_id).to eq(destination_topic.id) + expect(bookmark3.reload.topic_id).to eq(destination_topic.id) + expect(bookmark4.reload.topic_id).to eq(new_post.topic_id) + end + context "post timings" do fab!(:some_user) { Fabricate(:user) }