FIX: Don't swallow the original error when moving posts

FIX: Don’t swallow the original error when moving posts

Dropping the temp table in an ensure block hides the actual exception. Creating the table with ON COMMIT DROP makes the temp table disappear automatically at the end of the transaction. We only need the explicit DROP in tests, because tests already run inside a transaction, so the temp table won’t be dropped after each test which leads to spec failures.

diff --git a/app/models/post_mover.rb b/app/models/post_mover.rb
index d9d6978..6b779ed 100644
--- a/app/models/post_mover.rb
+++ b/app/models/post_mover.rb
@@ -77,11 +77,11 @@ class PostMover
 
     destination_topic.reload
     destination_topic
-  ensure
-    drop_temp_table
   end
 
   def create_temp_table
+    DB.exec("DROP TABLE IF EXISTS moved_posts") if Rails.env.test?
+
     DB.exec <<~SQL
       CREATE TEMPORARY TABLE moved_posts (
         old_topic_id INTEGER,
@@ -91,17 +91,13 @@ class PostMover
         new_topic_title VARCHAR,
         new_post_id INTEGER,
         new_post_number INTEGER
-      );
+      ) ON COMMIT DROP;
 
       CREATE INDEX moved_posts_old_post_number ON moved_posts(old_post_number);
       CREATE INDEX moved_posts_old_post_id ON moved_posts(old_post_id);
     SQL
   end
 
-  def drop_temp_table
-    DB.exec("DROP TABLE IF EXISTS moved_posts")
-  end
-
   def move_each_post
     max_post_number = destination_topic.max_post_number + 1

GitHub sha: 10e509e4

This commit has been mentioned on Discourse Meta. There might be relevant details there: