From 07f4e56658c64c3bff7edfb42c30872ad09b265c Mon Sep 17 00:00:00 2001 From: Mark VanLandingham Date: Fri, 6 Dec 2024 09:50:53 -0600 Subject: [PATCH] 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. --- app/models/post_mover.rb | 7 ++++ spec/models/post_mover_spec.rb | 59 +++++++++++++++++++++++----------- 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/app/models/post_mover.rb b/app/models/post_mover.rb index e0ab3be3968..f09cc205410 100644 --- a/app/models/post_mover.rb +++ b/app/models/post_mover.rb @@ -278,6 +278,13 @@ class PostMover new_post.custom_fields = post.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(:post_moved, new_post, original_topic.id) diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index e832d8ba053..74fa2a9821f 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -2668,9 +2668,9 @@ RSpec.describe PostMover do 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) { Fabricate(:post, topic: original_topic, raw: "op of original topic") } 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 fab!(:first_post) { Fabricate(:post, topic: original_topic, raw: "first_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 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) + describe "moving post other than first post" do + it "Generates notification pointing to destination 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 + 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 + + 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