From b945a2dc39fb462bcb001be607d9c16f2b856193 Mon Sep 17 00:00:00 2001
From: Gerhard Schlager <mail@gerhard-schlager.at>
Date: Mon, 26 Mar 2018 17:05:18 +0200
Subject: [PATCH] Call `on_drop` only when tables/columns are dropped

---
 lib/migration/table_dropper.rb                |  7 +++---
 .../migration/column_dropper_spec.rb          | 13 ++++++++++
 .../migration/table_dropper_spec.rb           | 25 +++++++++++++++++++
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/lib/migration/table_dropper.rb b/lib/migration/table_dropper.rb
index 02d383849e7..78a9c7172f5 100644
--- a/lib/migration/table_dropper.rb
+++ b/lib/migration/table_dropper.rb
@@ -44,9 +44,10 @@ module Migration
         LIMIT 1
       SQL
 
-      builder.where(new_table_exists) if @new_name.present?
+      builder.where(table_exists(":new_name")) if @new_name.present?
 
       builder.where("table_schema = 'public'")
+        .where(table_exists(":old_name"))
         .where(previous_migration_done)
         .exec(old_name: @old_name,
               new_name: @new_name,
@@ -54,13 +55,13 @@ module Migration
               after_migration: @after_migration).to_a.length > 0
     end
 
-    def new_table_exists
+    def table_exists(table_name_placeholder)
       <<~SQL
         EXISTS(
             SELECT 1
             FROM INFORMATION_SCHEMA.TABLES
             WHERE table_schema = 'public' AND
-                  table_name = :new_name
+                  table_name = #{table_name_placeholder}
         )
       SQL
     end
diff --git a/spec/components/migration/column_dropper_spec.rb b/spec/components/migration/column_dropper_spec.rb
index 6ae3f06f508..9a0bd536e2b 100644
--- a/spec/components/migration/column_dropper_spec.rb
+++ b/spec/components/migration/column_dropper_spec.rb
@@ -65,6 +65,19 @@ RSpec.describe Migration::ColumnDropper do
 
       expect(has_column?('topics', 'junk')).to eq(false)
       expect(dropped_proc_called).to eq(true)
+
+      dropped_proc_called = false
+
+      Migration::ColumnDropper.drop(
+        table: 'topics',
+        after_migration: migration_name,
+        columns: ['junk'],
+        delay: 10.minutes,
+        on_drop: ->() { dropped_proc_called = true }
+      )
+
+      # it should call "on_drop" only when there are columns to drop
+      expect(dropped_proc_called).to eq(false)
     end
 
     it "drops the columns immediately if the first migration was less than 10 minutes ago" do
diff --git a/spec/components/migration/table_dropper_spec.rb b/spec/components/migration/table_dropper_spec.rb
index e6dc4a1a063..0b798f82bcb 100644
--- a/spec/components/migration/table_dropper_spec.rb
+++ b/spec/components/migration/table_dropper_spec.rb
@@ -86,6 +86,19 @@ describe Migration::TableDropper do
 
         expect(table_exists?('table_with_old_name')).to eq(false)
         expect(dropped_proc_called).to eq(true)
+
+        dropped_proc_called = false
+
+        described_class.delayed_rename(
+          old_name: 'table_with_old_name',
+          new_name: 'table_with_new_name',
+          after_migration: migration_name,
+          delay: 10.minutes,
+          on_drop: ->() { dropped_proc_called = true }
+        )
+
+        # it should call "on_drop" only when there is a table to drop
+        expect(dropped_proc_called).to eq(false)
       end
     end
 
@@ -112,6 +125,18 @@ describe Migration::TableDropper do
 
         expect(table_exists?('table_with_old_name')).to eq(false)
         expect(dropped_proc_called).to eq(true)
+
+        dropped_proc_called = false
+
+        described_class.delayed_drop(
+          table_name: 'table_with_old_name',
+          after_migration: migration_name,
+          delay: 10.minutes,
+          on_drop: ->() { dropped_proc_called = true }
+        )
+
+        # it should call "on_drop" only when there is a table to drop
+        expect(dropped_proc_called).to eq(false)
       end
     end
   end