DEV: Add user_id and post_user_id to MovedPost records (#30130)

Follow-up from this commit - 9b8af0ea9f

Adds helpful data into MovedPost records for later lookup. ALSO fixes notifications for freeze_original to point to the newly created post, not the moved post.
This commit is contained in:
Mark VanLandingham 2024-12-05 17:10:32 -06:00 committed by GitHub
parent 13793a3d8e
commit 71bec686a2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 84 additions and 13 deletions

View File

@ -6,6 +6,12 @@ class MovedPost < ActiveRecord::Base
belongs_to :new_topic, class_name: "Topic", foreign_key: :new_topic_id belongs_to :new_topic, class_name: "Topic", foreign_key: :new_topic_id
belongs_to :new_post, class_name: "Post", foreign_key: :new_post_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 end
# == Schema Information # == Schema Information
@ -23,11 +29,15 @@ end
# created_new_topic :boolean default(FALSE), not null # created_new_topic :boolean default(FALSE), not null
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# old_topic_title :string
# post_user_id :integer
# user_id :integer
# #
# Indexes # Indexes
# #
# index_moved_posts_on_new_post_id (new_post_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 (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_id (old_post_id)
# index_moved_posts_on_old_post_number (old_post_number) # index_moved_posts_on_old_post_number (old_post_number)
# index_moved_posts_on_old_topic_id (old_topic_id) # index_moved_posts_on_old_topic_id (old_topic_id)

View File

@ -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 # 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, options: {})
@original_topic = original_topic @original_topic = original_topic
@original_topic_title = original_topic.title
@user = user @user = user
@post_ids = post_ids @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 @move_to_pm = move_to_pm
@options = options @options = options
end end
@ -307,6 +311,11 @@ class PostMover
moved_post.disable_rate_limits! if @options[:freeze_original] moved_post.disable_rate_limits! if @options[:freeze_original]
moved_post.save(validate: false) 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) DiscourseEvent.trigger(:post_moved, moved_post, original_topic.id)
# Move any links from the post to the new topic # Move any links from the post to the new topic
@ -337,6 +346,7 @@ class PostMover
old_topic_id: post.topic_id, old_topic_id: post.topic_id,
old_post_id: post.id, old_post_id: post.id,
old_post_number: post.post_number, old_post_number: post.post_number,
post_user_id: post.user_id,
new_topic_id: destination_topic.id, new_topic_id: destination_topic.id,
new_post_number: new_post_number, new_post_number: new_post_number,
new_topic_title: destination_topic.title, new_topic_title: destination_topic.title,
@ -347,10 +357,12 @@ class PostMover
metadata[:new_post_id] = new_post.id metadata[:new_post_id] = new_post.id
metadata[:now] = Time.zone.now metadata[:now] = Time.zone.now
metadata[:created_new_topic] = @creating_new_topic metadata[:created_new_topic] = @creating_new_topic
metadata[:old_topic_title] = @original_topic_title
metadata[:user_id] = @user.id
DB.exec(<<~SQL, metadata) 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) 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_post_id, :old_post_number, :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, :new_topic_id, :new_topic_title, :new_post_id, :new_post_number, :created_new_topic, :now, :now)
SQL SQL
end end
@ -703,7 +715,7 @@ class PostMover
def enqueue_jobs(topic) def enqueue_jobs(topic)
@post_creator.enqueue_jobs if @post_creator @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) Jobs.enqueue(:delete_inaccessible_notifications, topic_id: topic.id)
end end

View File

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

View File

@ -314,8 +314,11 @@ RSpec.describe PostMover do
MovedPost.exists?( MovedPost.exists?(
new_topic: new_topic, new_topic: new_topic,
new_post_id: p2.id, new_post_id: p2.id,
old_topic: topic, old_topic_id: topic.id,
old_topic_title: topic.title,
old_post_id: p2.id, old_post_id: p2.id,
post_user_id: p2.user_id,
user_id: user.id,
created_new_topic: true, created_new_topic: true,
), ),
).to eq(true) ).to eq(true)
@ -323,8 +326,11 @@ RSpec.describe PostMover do
MovedPost.exists?( MovedPost.exists?(
new_topic: new_topic, new_topic: new_topic,
new_post_id: p4.id, new_post_id: p4.id,
old_topic: topic, old_topic_id: topic.id,
old_topic_title: topic.title,
old_post_id: p4.id, old_post_id: p4.id,
post_user_id: p4.user_id,
user_id: user.id,
created_new_topic: true, created_new_topic: true,
), ),
).to eq(true) ).to eq(true)
@ -695,8 +701,11 @@ RSpec.describe PostMover do
MovedPost.exists?( MovedPost.exists?(
new_topic: destination_topic, new_topic: destination_topic,
new_post_id: p2.id, new_post_id: p2.id,
old_topic: topic, old_topic_id: topic.id,
old_topic_title: topic.title,
old_post_id: p2.id, old_post_id: p2.id,
post_user_id: p2.user_id,
user_id: user.id,
created_new_topic: false, created_new_topic: false,
), ),
).to eq(true) ).to eq(true)
@ -704,8 +713,11 @@ RSpec.describe PostMover do
MovedPost.exists?( MovedPost.exists?(
new_topic: destination_topic, new_topic: destination_topic,
new_post_id: p4.id, new_post_id: p4.id,
old_topic: topic, old_topic_id: topic.id,
old_topic_title: topic.title,
old_post_id: p4.id, old_post_id: p4.id,
post_user_id: p4.user_id,
user_id: user.id,
created_new_topic: false, created_new_topic: false,
), ),
).to eq(true) ).to eq(true)
@ -2703,8 +2715,11 @@ RSpec.describe PostMover do
expect( expect(
MovedPost.exists?( MovedPost.exists?(
old_topic_id: original_topic.id, old_topic_id: original_topic.id,
new_topic_id: destination_topic.id, old_topic_title: original_topic.title,
old_post_id: first_post.id, 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) ).to eq(true)
end end
@ -2805,6 +2820,29 @@ RSpec.describe PostMover do
expect(pm.posts.map(&:raw)).to include(*moving_posts.map(&:raw)) expect(pm.posts.map(&:raw)).to include(*moving_posts.map(&:raw))
end 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 context "with rate limit" do
before do before do
RateLimiter.enable RateLimiter.enable