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.
This commit is contained in:
Mark VanLandingham 2024-12-12 11:47:14 -06:00 committed by GitHub
parent 1791abab25
commit bbb31b05ca
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 111 additions and 14 deletions

View File

@ -32,6 +32,7 @@ end
# old_topic_title :string # old_topic_title :string
# post_user_id :integer # post_user_id :integer
# user_id :integer # user_id :integer
# full_move :boolean
# #
# Indexes # Indexes
# #

View File

@ -92,14 +92,14 @@ class PostMover
Post.types[:whisper], Post.types[:whisper],
) )
.count .count
moving_all_posts = original_topic_posts_count == posts.length @full_move = original_topic_posts_count == posts.length
@first_post_number_moved = @first_post_number_moved =
posts.first.is_first_post? ? posts[1]&.post_number : posts.first.post_number 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 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) 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 @first_post_number_moved = posts.last.post_number + 1
end end
@ -114,7 +114,7 @@ class PostMover
update_upload_security_status update_upload_security_status
update_bookmarks update_bookmarks
close_topic_and_schedule_deletion if moving_all_posts close_topic_and_schedule_deletion if @full_move
destination_topic.reload destination_topic.reload
DiscourseEvent.trigger( DiscourseEvent.trigger(
@ -366,10 +366,11 @@ class PostMover
metadata[:created_new_topic] = @creating_new_topic metadata[:created_new_topic] = @creating_new_topic
metadata[:old_topic_title] = @original_topic_title metadata[:old_topic_title] = @original_topic_title
metadata[:user_id] = @user.id metadata[:user_id] = @user.id
metadata[:full_move] = @full_move
DB.exec(<<~SQL, metadata) 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) 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, :new_topic_id, :new_topic_title, :new_post_id, :new_post_number, :created_new_topic, :now, :now) 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 SQL
end end
@ -613,23 +614,38 @@ class PostMover
move_type_str = PostMover.move_types[@move_type].to_s move_type_str = PostMover.move_types[@move_type].to_s
move_type_str.sub!("topic", "message") if @move_to_pm 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 = message =
I18n.with_locale(SiteSetting.default_locale) do I18n.with_locale(SiteSetting.default_locale) do
I18n.t( I18n.t(
"move_posts.#{move_type_str}_moderator_post", "move_posts.#{move_type_str}_moderator_post",
count: posts.length, count: posts.length,
topic_link: topic_link: topic_link,
(
if posts.first.is_first_post?
"[#{destination_topic.title}](#{destination_topic.relative_url})"
else
"[#{destination_topic.title}](#{posts.first.relative_url})"
end
),
) )
end end
post_type = @move_to_pm ? Post.types[:whisper] : Post.types[:small_action]
original_topic.add_moderator_post( original_topic.add_moderator_post(
user, user,
message, message,

View File

@ -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

View File

@ -351,6 +351,42 @@ RSpec.describe PostMover do
expect(topic.closed).to eq(true) expect(topic.closed).to eq(true)
end 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 it "does not move posts that do not belong to the existing topic" do
new_topic = new_topic =
topic.move_posts(user, [p2.id, p3.id, p5.id], title: "Logan is a pretty good movie") 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") expect(moderator_post.action_code).to eq("split_topic")
end 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 it "keeps posts when moving all posts to a new topic" do
all_posts_from_original_topic = original_topic.ordered_posts.map(&:raw) all_posts_from_original_topic = original_topic.ordered_posts.map(&:raw)