diff --git a/app/models/moved_post.rb b/app/models/moved_post.rb index 60cda5a44b2..21f34e52f0b 100644 --- a/app/models/moved_post.rb +++ b/app/models/moved_post.rb @@ -6,6 +6,12 @@ class MovedPost < ActiveRecord::Base belongs_to :new_topic, class_name: "Topic", foreign_key: :new_topic_id belongs_to :new_post, class_name: "Post", foreign_key: :new_post_id + + # The author of the moved post + belongs_to :posting_user, class_name: "User", foreign_key: :post_user_id + + # The user who moved the post + belongs_to :user end # == Schema Information @@ -23,12 +29,16 @@ end # created_new_topic :boolean default(FALSE), not null # created_at :datetime not null # updated_at :datetime not null +# old_topic_title :string +# post_user_id :integer +# user_id :integer # # Indexes # -# index_moved_posts_on_new_post_id (new_post_id) -# index_moved_posts_on_new_topic_id (new_topic_id) -# index_moved_posts_on_old_post_id (old_post_id) -# index_moved_posts_on_old_post_number (old_post_number) -# index_moved_posts_on_old_topic_id (old_topic_id) +# index_moved_posts_on_new_post_id (new_post_id) +# index_moved_posts_on_new_topic_id (new_topic_id) +# index_moved_posts_on_new_topic_id_and_post_user_id (new_topic_id,post_user_id) +# index_moved_posts_on_old_post_id (old_post_id) +# index_moved_posts_on_old_post_number (old_post_number) +# index_moved_posts_on_old_topic_id (old_topic_id) # diff --git a/app/models/post_mover.rb b/app/models/post_mover.rb index 8033bdebc0a..e0ab3be3968 100644 --- a/app/models/post_mover.rb +++ b/app/models/post_mover.rb @@ -11,8 +11,12 @@ class PostMover # freeze_original: :boolean - if true, the original topic will be frozen but not deleted and posts will be "copied" to topic def initialize(original_topic, user, post_ids, move_to_pm: false, options: {}) @original_topic = original_topic + @original_topic_title = original_topic.title @user = user @post_ids = post_ids + # For now we store a copy of post_ids. If `freeze_original` is present, we will have new post_ids. + # When we create the new posts, we will pluck out post_ids out of this and replace with updated ids. + @post_ids_after_move = post_ids @move_to_pm = move_to_pm @options = options end @@ -307,6 +311,11 @@ class PostMover moved_post.disable_rate_limits! if @options[:freeze_original] moved_post.save(validate: false) + if moved_post.id != post.id + @post_ids_after_move = + @post_ids_after_move.map { |post_id| post_id == post.id ? moved_post.id : post_id } + end + DiscourseEvent.trigger(:post_moved, moved_post, original_topic.id) # Move any links from the post to the new topic @@ -337,6 +346,7 @@ class PostMover old_topic_id: post.topic_id, old_post_id: post.id, old_post_number: post.post_number, + post_user_id: post.user_id, new_topic_id: destination_topic.id, new_post_number: new_post_number, new_topic_title: destination_topic.title, @@ -347,10 +357,12 @@ class PostMover metadata[:new_post_id] = new_post.id metadata[:now] = Time.zone.now metadata[:created_new_topic] = @creating_new_topic + metadata[:old_topic_title] = @original_topic_title + metadata[:user_id] = @user.id DB.exec(<<~SQL, metadata) - INSERT INTO moved_posts(old_topic_id, old_post_id, old_post_number, new_topic_id, new_topic_title, new_post_id, new_post_number, created_new_topic, created_at, updated_at) - VALUES (:old_topic_id, :old_post_id, :old_post_number, :new_topic_id, :new_topic_title, :new_post_id, :new_post_number, :created_new_topic, :now, :now) + INSERT INTO moved_posts(old_topic_id, old_topic_title, old_post_id, old_post_number, post_user_id, user_id, new_topic_id, new_topic_title, new_post_id, new_post_number, created_new_topic, created_at, updated_at) + VALUES (:old_topic_id, :old_topic_title, :old_post_id, :old_post_number, :post_user_id, :user_id, :new_topic_id, :new_topic_title, :new_post_id, :new_post_number, :created_new_topic, :now, :now) SQL end @@ -703,7 +715,7 @@ class PostMover def enqueue_jobs(topic) @post_creator.enqueue_jobs if @post_creator - Jobs.enqueue(:notify_moved_posts, post_ids: post_ids, moved_by_id: user.id) + Jobs.enqueue(:notify_moved_posts, post_ids: @post_ids_after_move, moved_by_id: user.id) Jobs.enqueue(:delete_inaccessible_notifications, topic_id: topic.id) end diff --git a/db/migrate/20241205162117_add_columns_to_moved_posts.rb b/db/migrate/20241205162117_add_columns_to_moved_posts.rb new file mode 100644 index 00000000000..a52d1480137 --- /dev/null +++ b/db/migrate/20241205162117_add_columns_to_moved_posts.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true +class AddColumnsToMovedPosts < ActiveRecord::Migration[7.2] + def change + add_column :moved_posts, :old_topic_title, :string + add_column :moved_posts, :post_user_id, :integer + add_column :moved_posts, :user_id, :integer + + # Index for querying moved_posts for post author given a new topic ID + add_index :moved_posts, %i[new_topic_id post_user_id] + end +end diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index d397e8f18f0..e832d8ba053 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -314,8 +314,11 @@ RSpec.describe PostMover do MovedPost.exists?( new_topic: new_topic, new_post_id: p2.id, - old_topic: topic, + old_topic_id: topic.id, + old_topic_title: topic.title, old_post_id: p2.id, + post_user_id: p2.user_id, + user_id: user.id, created_new_topic: true, ), ).to eq(true) @@ -323,8 +326,11 @@ RSpec.describe PostMover do MovedPost.exists?( new_topic: new_topic, new_post_id: p4.id, - old_topic: topic, + old_topic_id: topic.id, + old_topic_title: topic.title, old_post_id: p4.id, + post_user_id: p4.user_id, + user_id: user.id, created_new_topic: true, ), ).to eq(true) @@ -695,8 +701,11 @@ RSpec.describe PostMover do MovedPost.exists?( new_topic: destination_topic, new_post_id: p2.id, - old_topic: topic, + old_topic_id: topic.id, + old_topic_title: topic.title, old_post_id: p2.id, + post_user_id: p2.user_id, + user_id: user.id, created_new_topic: false, ), ).to eq(true) @@ -704,8 +713,11 @@ RSpec.describe PostMover do MovedPost.exists?( new_topic: destination_topic, new_post_id: p4.id, - old_topic: topic, + old_topic_id: topic.id, + old_topic_title: topic.title, old_post_id: p4.id, + post_user_id: p4.user_id, + user_id: user.id, created_new_topic: false, ), ).to eq(true) @@ -2703,8 +2715,11 @@ RSpec.describe PostMover do expect( MovedPost.exists?( old_topic_id: original_topic.id, - new_topic_id: destination_topic.id, + old_topic_title: original_topic.title, old_post_id: first_post.id, + post_user_id: first_post.user_id, + user_id: Discourse.system_user.id, + new_topic_id: destination_topic.id, ), ).to eq(true) end @@ -2805,6 +2820,29 @@ RSpec.describe PostMover do expect(pm.posts.map(&:raw)).to include(*moving_posts.map(&:raw)) end + describe "moved_post notifications" do + before { Jobs.run_immediately! } + + it "Generates notifications pointing to the newly created post and topic" do + PostMover.new( + original_topic, + Discourse.system_user, + [first_post.id], + options: { + freeze_original: true, + }, + ).to_topic(destination_topic.id) + + notification = + Notification.find_by( + post_number: destination_topic.posts.find_by(raw: "first_post").post_number, + topic_id: destination_topic.id, + notification_type: Notification.types[:moved_post], + ) + expect(notification).to be_present + end + end + context "with rate limit" do before do RateLimiter.enable