FIX: freeze_original for moved_post notifications for OP moves, link to destination topic (#30147)

Currently when copy an OP to another topic, the link is to the topic that wasn't moved. The notification should instead be to the new topic the OP was moved to -- we have duplicate logic already for this but first post creation get special treatment, and this applies the same treatment.
This commit is contained in:
Mark VanLandingham 2024-12-06 09:50:53 -06:00 committed by GitHub
parent 2defab25b7
commit 07f4e56658
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 48 additions and 18 deletions

View File

@ -278,6 +278,13 @@ class PostMover
new_post.custom_fields = post.custom_fields new_post.custom_fields = post.custom_fields
new_post.save_custom_fields new_post.save_custom_fields
# When freezing original, ensure the notification generated points
# to the newly created post, not the old OP
if @options[:freeze_original]
@post_ids_after_move =
@post_ids_after_move.map { |post_id| post_id == post.id ? new_post.id : post_id }
end
DiscourseEvent.trigger(:first_post_moved, new_post, post) DiscourseEvent.trigger(:first_post_moved, new_post, post)
DiscourseEvent.trigger(:post_moved, new_post, original_topic.id) DiscourseEvent.trigger(:post_moved, new_post, original_topic.id)

View File

@ -2668,9 +2668,9 @@ RSpec.describe PostMover do
context "with freeze_original option" do context "with freeze_original option" do
fab!(:original_topic) { Fabricate(:topic) } fab!(:original_topic) { Fabricate(:topic) }
fab!(:destination_topic) { Fabricate(:topic) } fab!(:destination_topic) { Fabricate(:topic) }
fab!(:op) { Fabricate(:post, topic: original_topic, raw: "op of this topic") } fab!(:op) { Fabricate(:post, topic: original_topic, raw: "op of original topic") }
fab!(:op_of_destination) do fab!(:op_of_destination) do
Fabricate(:post, topic: destination_topic, raw: "op of this topic") Fabricate(:post, topic: destination_topic, raw: "op of destination topic")
end end
fab!(:first_post) { Fabricate(:post, topic: original_topic, raw: "first_post") } fab!(:first_post) { Fabricate(:post, topic: original_topic, raw: "first_post") }
fab!(:second_post) { Fabricate(:post, topic: original_topic, raw: "second_post") } fab!(:second_post) { Fabricate(:post, topic: original_topic, raw: "second_post") }
@ -2823,23 +2823,46 @@ RSpec.describe PostMover do
describe "moved_post notifications" do describe "moved_post notifications" do
before { Jobs.run_immediately! } before { Jobs.run_immediately! }
it "Generates notifications pointing to the newly created post and topic" do describe "moving post other than first post" do
PostMover.new( it "Generates notification pointing to destination topic" do
original_topic, PostMover.new(
Discourse.system_user, original_topic,
[first_post.id], Discourse.system_user,
options: { [first_post.id],
freeze_original: true, options: {
}, freeze_original: true,
).to_topic(destination_topic.id) },
).to_topic(destination_topic.id)
notification = notification =
Notification.find_by( Notification.find_by(
post_number: destination_topic.posts.find_by(raw: "first_post").post_number, post_number: destination_topic.posts.find_by(raw: "first_post").post_number,
topic_id: destination_topic.id, topic_id: destination_topic.id,
notification_type: Notification.types[:moved_post], notification_type: Notification.types[:moved_post],
) )
expect(notification).to be_present expect(notification).to be_present
end
end
describe "moving first post" do
it "Generates notification pointing to destination topic" do
PostMover.new(
original_topic,
Discourse.system_user,
[op.id],
options: {
freeze_original: true,
},
).to_topic(destination_topic.id)
notification =
Notification.find_by(
post_number: destination_topic.posts.find_by(raw: op.raw).post_number,
topic_id: destination_topic.id,
notification_type: Notification.types[:moved_post],
)
expect(notification).to be_present
end
end end
end end