From bbb31b05ca2ba633f0ebae2c0d8e74876480b5b3 Mon Sep 17 00:00:00 2001 From: Mark VanLandingham Date: Thu, 12 Dec 2024 11:47:14 -0600 Subject: [PATCH] DEV: add full_move to MovedPost record small_action modifier (#30236) This commit adds a new column full_move to the moved_posts table. This is useful to look back at history and determine if a whole topic was moved or partial. This commit also adds an apply_modifier to skip the creation of the moved posts small action. --- app/models/moved_post.rb | 1 + app/models/post_mover.rb | 44 +++++++---- ...608_add_full_move_to_moved_post_records.rb | 7 ++ spec/models/post_mover_spec.rb | 73 +++++++++++++++++++ 4 files changed, 111 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20241211222608_add_full_move_to_moved_post_records.rb diff --git a/app/models/moved_post.rb b/app/models/moved_post.rb index 21f34e52f0b..fb621e8bd0c 100644 --- a/app/models/moved_post.rb +++ b/app/models/moved_post.rb @@ -32,6 +32,7 @@ end # old_topic_title :string # post_user_id :integer # user_id :integer +# full_move :boolean # # Indexes # diff --git a/app/models/post_mover.rb b/app/models/post_mover.rb index 555b460a4ae..461be590174 100644 --- a/app/models/post_mover.rb +++ b/app/models/post_mover.rb @@ -92,14 +92,14 @@ class PostMover Post.types[:whisper], ) .count - moving_all_posts = original_topic_posts_count == posts.length + @full_move = original_topic_posts_count == posts.length @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.ordered_posts.where("post_number > ?", posts.last.post_number) - shift_post_numbers(from_posts) if !moving_all_posts + shift_post_numbers(from_posts) if !@full_move @first_post_number_moved = posts.last.post_number + 1 end @@ -114,7 +114,7 @@ class PostMover update_upload_security_status update_bookmarks - close_topic_and_schedule_deletion if moving_all_posts + close_topic_and_schedule_deletion if @full_move destination_topic.reload DiscourseEvent.trigger( @@ -366,10 +366,11 @@ class PostMover metadata[:created_new_topic] = @creating_new_topic metadata[:old_topic_title] = @original_topic_title metadata[:user_id] = @user.id + metadata[:full_move] = @full_move DB.exec(<<~SQL, metadata) - 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) + INSERT INTO moved_posts(old_topic_id, old_topic_title, old_post_id, old_post_number, post_user_id, user_id, full_move, 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, :full_move, :new_topic_id, :new_topic_title, :new_post_id, :new_post_number, :created_new_topic, :now, :now) SQL end @@ -613,23 +614,38 @@ class PostMover move_type_str = PostMover.move_types[@move_type].to_s move_type_str.sub!("topic", "message") if @move_to_pm + topic_link = + if posts.first.is_first_post? + "[#{destination_topic.title}](#{destination_topic.relative_url})" + else + "[#{destination_topic.title}](#{posts.first.relative_url})" + end + + post_type = @move_to_pm ? Post.types[:whisper] : Post.types[:small_action] + + continue = + DiscoursePluginRegistry.apply_modifier( + :post_mover_create_moderator_post, + true, + user: user, + post_type: post_type, + post_ids: @post_ids_after_move, + full_move: @full_move, + original_topic: original_topic, + destination_topic: destination_topic, + first_post_number_moved: @first_post_number_moved, + ) + return if !continue + message = I18n.with_locale(SiteSetting.default_locale) do I18n.t( "move_posts.#{move_type_str}_moderator_post", count: posts.length, - topic_link: - ( - if posts.first.is_first_post? - "[#{destination_topic.title}](#{destination_topic.relative_url})" - else - "[#{destination_topic.title}](#{posts.first.relative_url})" - end - ), + topic_link: topic_link, ) end - post_type = @move_to_pm ? Post.types[:whisper] : Post.types[:small_action] original_topic.add_moderator_post( user, message, diff --git a/db/migrate/20241211222608_add_full_move_to_moved_post_records.rb b/db/migrate/20241211222608_add_full_move_to_moved_post_records.rb new file mode 100644 index 00000000000..1650f2b1180 --- /dev/null +++ b/db/migrate/20241211222608_add_full_move_to_moved_post_records.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddFullMoveToMovedPostRecords < ActiveRecord::Migration[7.2] + def change + add_column :moved_posts, :full_move, :boolean + end +end diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index 9002006eb2d..c98bbeb3280 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -351,6 +351,42 @@ RSpec.describe PostMover do expect(topic.closed).to eq(true) end + it "records full_move=true in MovedPost records when all posts are moved" do + post_ids = [p1.id, p2.id, p3.id, p4.id] + new_topic = + topic.move_posts( + user, + [p1.id, p2.id, p3.id, p4.id], + title: "new testing topic name", + category_id: category.id, + ) + expect( + MovedPost.where( + old_post_id: post_ids, + new_topic_id: new_topic.id, + full_move: true, + ).count, + ).to eq(4) + end + + it "records full_move=false in MovedPost records when only some posts" do + post_ids = [p3.id, p4.id] + new_topic = + topic.move_posts( + user, + [p3.id, p4.id], + title: "new testing topic name", + category_id: category.id, + ) + expect( + MovedPost.where( + old_post_id: post_ids, + new_topic_id: new_topic.id, + full_move: false, + ).count, + ).to eq(2) + end + it "does not move posts that do not belong to the existing topic" do new_topic = topic.move_posts(user, [p2.id, p3.id, p5.id], title: "Logan is a pretty good movie") @@ -2854,6 +2890,43 @@ RSpec.describe PostMover do expect(moderator_post.action_code).to eq("split_topic") end + context "with `post_mover_create_moderator_post` modifier" do + fab!(:topic_1) { Fabricate(:topic) } + fab!(:topic_2) { Fabricate(:topic) } + fab!(:post_1) { Fabricate(:post, topic: topic_1) } + fab!(:user) + + before { SiteSetting.delete_merged_stub_topics_after_days = 0 } + let(:modifier_block) { Proc.new { |continue, _| false } } + + it "does not create small action post when modifier returns false" do + plugin_instance = Plugin::Instance.new + plugin_instance.register_modifier(:post_mover_create_moderator_post, &modifier_block) + + expect { + PostMover.new( + original_topic, + Discourse.system_user, + [first_post.id, second_post.id], + options: { + freeze_original: true, + }, + ).to_topic(destination_topic.id) + }.not_to change { + original_topic + .posts + .where(post_type: Post.types[:small_action], action_code: "split_topic") + .count + } + ensure + DiscoursePluginRegistry.unregister_modifier( + plugin_instance, + :post_mover_create_moderator_post, + &modifier_block + ) + end + end + it "keeps posts when moving all posts to a new topic" do all_posts_from_original_topic = original_topic.ordered_posts.map(&:raw)