FIX: Existing post timings could prevent moving posts

Post timings are created by `topic_id` and `post_number` and it's possible that the destination topic already contains post timings for non-existent posts. For example, this can happen if the destination topic was previously split and Discourse recorded post timings for moved posts in the destination topic.

This commit ensures that all timings which reference non-existent posts are deleted from the destination topic before the posts are moved.

app/models/post_mover.rb

@@ -64,6 +64,7 @@ class PostMover
     moving_all_posts = (@original_topic.posts.pluck(:id).sort == @post_ids.sort)
 
     create_temp_table
+    delete_invalid_post_timings
     move_each_post
     notify_users_that_posts_have_moved
     update_statistics
@@ -290,6 +291,20 @@ class PostMover
     SQL
   end
 
+  def delete_invalid_post_timings
+    DB.exec(<<~SQL, topid_id: destination_topic.id)
+      DELETE
+      FROM post_timings pt
+      WHERE pt.topic_id = :topid_id
+        AND NOT EXISTS(
+          SELECT 1
+          FROM posts p
+          WHERE p.topic_id = pt.topic_id
+            AND p.post_number = pt.post_number
+        )
+    SQL
+  end
+
   def move_post_timings
     DB.exec <<~SQL
       UPDATE post_timings pt

spec/models/post_mover_spec.rb

@@ -554,20 +554,38 @@ describe PostMover do
             expect(Notification.exists?(admin_notification.id)).to eq(true)
           end
 
-          it "moves post timings" do
-            some_user = Fabricate(:user)
-            create_post_timing(p1, some_user, 500)
-            create_post_timing(p2, some_user, 1000)
-            create_post_timing(p3, some_user, 1500)
-            create_post_timing(p4, some_user, 750)
+          context "post timings" do
+            fab!(:some_user) { Fabricate(:user) }
 
-            moved_to = topic.move_posts(user, [p1.id, p4.id], destination_topic_id: destination_topic.id)
+            it "successfully moves timings" do
+              create_post_timing(p1, some_user, 500)
+              create_post_timing(p2, some_user, 1000)
+              create_post_timing(p3, some_user, 1500)
+              create_post_timing(p4, some_user, 750)
 
-            expect(PostTiming.where(topic_id: topic.id, user_id: some_user.id).pluck(:post_number, :msecs))
-              .to contain_exactly([1, 500], [2, 1000], [3, 1500])
+              moved_to = topic.move_posts(user, [p1.id, p4.id], destination_topic_id: destination_topic.id)
 
-            expect(PostTiming.where(topic_id: moved_to.id, user_id: some_user.id).pluck(:post_number, :msecs))
-              .to contain_exactly([2, 500], [3, 750])
+              expect(PostTiming.where(topic_id: topic.id, user_id: some_user.id).pluck(:post_number, :msecs))
+                .to contain_exactly([1, 500], [2, 1000], [3, 1500])
+
+              expect(PostTiming.where(topic_id: moved_to.id, user_id: some_user.id).pluck(:post_number, :msecs))
+                .to contain_exactly([2, 500], [3, 750])
+            end
+
+            it "moves timings when post timing exists in destination topic" do
+              PostTiming.create!(
+                topic_id: destination_topic.id,
+                user_id: some_user.id,
+                post_number: 2,
+                msecs: 800
+              )
+              create_post_timing(p1, some_user, 500)
+
+              moved_to = topic.move_posts(user, [p1.id], destination_topic_id: destination_topic.id)
+
+              expect(PostTiming.where(topic_id: moved_to.id, user_id: some_user.id).pluck(:post_number, :msecs))
+                .to contain_exactly([2, 500])
+            end
           end
 
           context "read state and other stats per user" do