From 1c03a9f078efa19068b27f6ad8dfe97dcb401720 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:47:39 -0300 Subject: [PATCH] Revert "FEATURE: Add `freeze_original` option to `PostMover` (#29880)" (#29940) This reverts commit b6ec07c3efb74e99b9c5f9c5ca74d9e3ccd6aa39. --- app/models/post_mover.rb | 35 ++------ spec/models/post_mover_spec.rb | 153 --------------------------------- 2 files changed, 6 insertions(+), 182 deletions(-) diff --git a/app/models/post_mover.rb b/app/models/post_mover.rb index 1f9031a828c..bc9ce05e4a1 100644 --- a/app/models/post_mover.rb +++ b/app/models/post_mover.rb @@ -7,14 +7,11 @@ class PostMover @move_types ||= Enum.new(:new_topic, :existing_topic) end - # 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: {}) + def initialize(original_topic, user, post_ids, move_to_pm: false) @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) @@ -93,14 +90,6 @@ 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 @@ -292,22 +281,15 @@ class PostMover update[:reply_to_user_id] = nil unless @move_map[post.reply_to_post_number] - moved_post = - if @options[:freeze_original] - post.dup - else - post - end + post.attributes = update + post.save(validate: false) - moved_post.attributes = update - moved_post.save(validate: false) - - DiscourseEvent.trigger(:post_moved, moved_post, original_topic.id) + DiscourseEvent.trigger(:post_moved, post, original_topic.id) # Move any links from the post to the new topic - moved_post.topic_links.update_all(topic_id: destination_topic.id) + post.topic_links.update_all(topic_id: destination_topic.id) - moved_post + post end def move_same_topic(post) @@ -349,10 +331,6 @@ 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 @@ -705,7 +683,6 @@ 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 b0cfa809eb1..aea2be77ffc 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -2637,158 +2637,5 @@ 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