From b6ec07c3efb74e99b9c5f9c5ca74d9e3ccd6aa39 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:25:55 -0300 Subject: [PATCH] FEATURE: Add `freeze_original` option to `PostMover` (#29880) * FEATURE: Add `freeze_original` option to `PostMover` This option will allow the api user to specify if the original topic should be `frozen`(locked and posts not deleted neither moved) With this option when moving topic posts your posts will be `copied` to the new topic and the original topic will be kept there. * DEV: update tests to check raw instead of ids * DEV: Implement `freeze_original` option for `PostMover` update specs to use `*array` matcher * DEV: add tests to `MovedPost` model in post mover * DEV: Update `MovedPost` model rspec * DEV: add back empty line to `post_mover.rb` --- app/models/post_mover.rb | 35 ++++++-- spec/models/post_mover_spec.rb | 153 +++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 6 deletions(-) diff --git a/app/models/post_mover.rb b/app/models/post_mover.rb index bc9ce05e4a1..1f9031a828c 100644 --- a/app/models/post_mover.rb +++ b/app/models/post_mover.rb @@ -7,11 +7,14 @@ class PostMover @move_types ||= Enum.new(:new_topic, :existing_topic) end - def initialize(original_topic, user, post_ids, move_to_pm: false) + # options: + # 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 @user = user @post_ids = post_ids @move_to_pm = move_to_pm + @options = options end def to_topic(id, participants: nil, chronological_order: false) @@ -90,6 +93,14 @@ class PostMover @first_post_number_moved = posts.first.is_first_post? ? posts[1]&.post_number : posts.first.post_number + if @options[:freeze_original] # in this case we need to add the moderator post after the last copied post + from_posts = @original_topic.posts.where("post_number > ?", posts.last.post_number) + + shift_post_numbers(from_posts) if !moving_all_posts + + @first_post_number_moved = posts.last.post_number + 1 + end + move_each_post handle_moved_references @@ -281,15 +292,22 @@ class PostMover update[:reply_to_user_id] = nil unless @move_map[post.reply_to_post_number] - post.attributes = update - post.save(validate: false) + moved_post = + if @options[:freeze_original] + post.dup + else + post + end - DiscourseEvent.trigger(:post_moved, post, original_topic.id) + moved_post.attributes = update + moved_post.save(validate: false) + + DiscourseEvent.trigger(:post_moved, moved_post, original_topic.id) # Move any links from the post to the new topic - post.topic_links.update_all(topic_id: destination_topic.id) + moved_post.topic_links.update_all(topic_id: destination_topic.id) - post + moved_post end def move_same_topic(post) @@ -331,6 +349,10 @@ class PostMover SQL end + def shift_post_numbers(from_posts) + from_posts.each { |post| post.update_columns(post_number: post.post_number + 1) } + end + def move_incoming_emails DB.exec <<~SQL UPDATE incoming_emails ie @@ -683,6 +705,7 @@ class PostMover def close_topic_and_schedule_deletion @original_topic.update_status("closed", true, @user) + return if @options[:freeze_original] # we only close the topic when freezing it days_to_deleting = SiteSetting.delete_merged_stub_topics_after_days if days_to_deleting == 0 diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index aea2be77ffc..b0cfa809eb1 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -2637,5 +2637,158 @@ RSpec.describe PostMover do expect(topic_1.deleted_at).not_to be_nil end end + + context "with freeze_original option" do + fab!(:original_topic) { Fabricate(:topic) } + fab!(:destination_topic) { Fabricate(:topic) } + fab!(:op) { Fabricate(:post, topic: original_topic, raw: "op of this topic") } + fab!(:op_of_destination) do + Fabricate(:post, topic: destination_topic, raw: "op of this topic") + end + fab!(:first_post) { Fabricate(:post, topic: original_topic, raw: "first_post") } + fab!(:second_post) { Fabricate(:post, topic: original_topic, raw: "second_post") } + fab!(:third_post) { Fabricate(:post, topic: original_topic, raw: "third_post") } + + it "keeps a post when moving it to a new topic" do + new_topic = + PostMover.new( + original_topic, + Discourse.system_user, + [first_post.id], + options: { + freeze_original: true, + }, + ).to_new_topic("Hi I'm a new topic, with a copy of the old posts") + expect(new_topic.posts.map(&:raw)).to include(first_post.raw) + expect(original_topic.posts.map(&:raw)).to include(first_post.raw) + end + + it "keeps a post when moving to an existing topic" do + PostMover.new( + original_topic, + Discourse.system_user, + [first_post.id], + options: { + freeze_original: true, + }, + ).to_topic(destination_topic.id) + expect(destination_topic.posts.map(&:raw)).to include(first_post.raw) + expect(original_topic.posts.map(&:raw)).to include(first_post.raw) + end + + it "creates a MovedPost record when moving to an existing topic" do + PostMover.new( + original_topic, + Discourse.system_user, + [first_post.id], + options: { + freeze_original: true, + }, + ).to_topic(destination_topic.id) + expect( + MovedPost.exists?( + old_topic_id: original_topic.id, + new_topic_id: destination_topic.id, + old_post_id: first_post.id, + ), + ).to eq(true) + end + + it "creates the moderator message in the correct position" do + PostMover.new( + original_topic, + Discourse.system_user, + [first_post.id, second_post.id], + options: { + freeze_original: true, + }, + ).to_topic(destination_topic.id) + + moderator_post = + original_topic.reload.ordered_posts.find_by(post_number: second_post.post_number + 1) # the next post + expect(moderator_post).to be_present + expect(moderator_post.post_type).to eq(Post.types[:small_action]) + expect(moderator_post.action_code).to eq("split_topic") + end + + it "keeps posts when moving all posts to a new topic" do + all_posts_from_original_topic = original_topic.ordered_posts.map(&:raw) + + new_topic = + PostMover.new( + original_topic, + Discourse.system_user, + original_topic.posts.map(&:id), + options: { + freeze_original: true, + }, + ).to_new_topic("Hi I'm a new topic, with a copy of the old posts") + + expect(original_topic.deleted_at).to be_nil + expect(original_topic.closed?).to eq(true) + + expect(original_topic.posts.map(&:raw)).to include(*all_posts_from_original_topic) + expect(new_topic.posts.map(&:raw)).to include(*all_posts_from_original_topic) + end + + it "does not get deleted when moved all posts to topic" do + SiteSetting.delete_merged_stub_topics_after_days = 0 + all_posts_from_original_topic = original_topic.posts.map(&:raw) + + PostMover.new( + original_topic, + Discourse.system_user, + original_topic.posts.map(&:id), + options: { + freeze_original: true, + }, + ).to_topic(destination_topic.id) + + expect(original_topic.deleted_at).to be_nil + expect(original_topic.closed?).to eq(true) + + expect(original_topic.posts.map(&:raw)).to include(*all_posts_from_original_topic) + expect(destination_topic.posts.map(&:raw)).to include(*all_posts_from_original_topic) + end + + it "keeps all posts when moving to a new PM" do + moving_posts = [first_post, second_post] + pm = + PostMover.new( + original_topic, + Discourse.system_user, + moving_posts.map(&:id), + move_to_pm: true, + options: { + freeze_original: true, + }, + ).to_new_topic("Hi I'm a new PM, with a copy of the old posts") + + expect(original_topic.posts.map(&:raw)).to include(*moving_posts.map(&:raw)) + expect(pm.posts.map(&:raw)).to include(*moving_posts.map(&:raw)) + end + + it "keep all posts when moving to an existing PM" do + pm = Fabricate(:private_message_topic) + pm_with_posts = Fabricate(:private_message_topic) + moving_posts = [ + Fabricate(:post, topic: pm_with_posts), + Fabricate(:post, topic: pm_with_posts), + ] + + PostMover.new( + pm_with_posts, + Discourse.system_user, + moving_posts.map(&:id), + move_to_pm: true, + options: { + freeze_original: true, + }, + ).to_topic(pm.id) + + expect(pm_with_posts.posts.map(&:raw)).to include(*moving_posts.map(&:raw)) + expect(pm.posts.map(&:raw)).to include(*moving_posts.map(&:raw)) + end + end end end