Merge pull request #25174 from kamipo-fix_db_migrate_status

Fix `rake db:migrate:status` with subdirectories.

4 files changed, 75 insertions, 24 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 8d054a144a..1a4306c6ec 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Fix `rake db:migrate:status` with subdirectories.
+
+    *Ryuta Kamizono*
+
 *   Don't share options between reference id and type columns
 
     When using a polymorphic reference column in a migration, sharing options
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 263a8a7da3..c47e32df59 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -1056,10 +1056,6 @@ module ActiveRecord
         Array(@migrations_paths)
       end
 
-      def match_to_migration_filename?(filename) # :nodoc:
-        Migration::MigrationFilenameRegexp.match?(File.basename(filename))
-      end
-
       def parse_migration_filename(filename) # :nodoc:
         File.basename(filename).scan(Migration::MigrationFilenameRegexp).first
       end
@@ -1067,9 +1063,7 @@ module ActiveRecord
       def migrations(paths)
         paths = Array(paths)
 
-        files = Dir[*paths.map { |p| "#{p}/**/[0-9]*_*.rb" }]
-
-        migrations = files.map do |file|
+        migrations = migration_files(paths).map do |file|
           version, name, scope = parse_migration_filename(file)
           raise IllegalMigrationNameError.new(file) unless version
           version = version.to_i
@@ -1081,6 +1075,30 @@ module ActiveRecord
         migrations.sort_by(&:version)
       end
 
+      def migrations_status(paths)
+        paths = Array(paths)
+
+        db_list = ActiveRecord::SchemaMigration.normalized_versions
+
+        file_list = migration_files(paths).map do |file|
+          version, name, scope = parse_migration_filename(file)
+          raise IllegalMigrationNameError.new(file) unless version
+          version = ActiveRecord::SchemaMigration.normalize_migration_number(version)
+          status = db_list.delete(version) ? "up" : "down"
+          [status, version, (name + scope).humanize]
+        end.compact
+
+        db_list.map! do |version|
+          ["up", version, "********** NO FILE **********"]
+        end
+
+        (db_list + file_list).sort_by { |_, version, _| version }
+      end
+
+      def migration_files(paths)
+        Dir[*paths.flat_map { |path| "#{path}/**/[0-9]*_*.rb" }]
+      end
+
       private
 
       def move(direction, migrations_paths, steps)
diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake
index fabd326649..1c7206aca4 100644
--- a/activerecord/lib/active_record/railties/databases.rake
+++ b/activerecord/lib/active_record/railties/databases.rake
@@ -110,28 +110,13 @@ db_namespace = namespace :db do
       unless ActiveRecord::SchemaMigration.table_exists?
         abort "Schema migrations table does not exist yet."
       end
-      db_list = ActiveRecord::SchemaMigration.normalized_versions
-
-      file_list =
-          ActiveRecord::Tasks::DatabaseTasks.migrations_paths.flat_map do |path|
-            Dir.foreach(path).map do |file|
-              next unless ActiveRecord::Migrator.match_to_migration_filename?(file)
-
-              version, name, scope = ActiveRecord::Migrator.parse_migration_filename(file)
-              version = ActiveRecord::SchemaMigration.normalize_migration_number(version)
-              status = db_list.delete(version) ? "up" : "down"
-              [status, version, (name + scope).humanize]
-            end.compact
-          end
 
-      db_list.map! do |version|
-        ["up", version, "********** NO FILE **********"]
-      end
       # output
       puts "\ndatabase: #{ActiveRecord::Base.connection_config[:database]}\n\n"
       puts "#{'Status'.center(8)}  #{'Migration ID'.ljust(14)}  Migration Name"
       puts "-" * 50
-      (db_list + file_list).sort_by { |_, version, _| version }.each do |status, version, name|
+      paths = ActiveRecord::Tasks::DatabaseTasks.migrations_paths
+      ActiveRecord::Migrator.migrations_status(paths).each do |status, version, name|
         puts "#{status.center(8)}  #{version.ljust(14)}  #{name}"
       end
       puts
diff --git a/activerecord/test/cases/migrator_test.rb b/activerecord/test/cases/migrator_test.rb
index 20d70b75ac..81f9749063 100644
--- a/activerecord/test/cases/migrator_test.rb
+++ b/activerecord/test/cases/migrator_test.rb
@@ -124,6 +124,50 @@ class MigratorTest < ActiveRecord::TestCase
     assert_equal migration_list.last, migrations.first
   end
 
+  def test_migrations_status
+    path = MIGRATIONS_ROOT + "/valid"
+
+    ActiveRecord::SchemaMigration.create(version: 2)
+    ActiveRecord::SchemaMigration.create(version: 10)
+
+    assert_equal [
+      ["down", "001", "Valid people have last names"],
+      ["up",   "002", "We need reminders"],
+      ["down", "003", "Innocent jointable"],
+      ["up",   "010", "********** NO FILE **********"],
+    ], ActiveRecord::Migrator.migrations_status(path)
+  end
+
+  def test_migrations_status_in_subdirectories
+    path = MIGRATIONS_ROOT + "/valid_with_subdirectories"
+
+    ActiveRecord::SchemaMigration.create(version: 2)
+    ActiveRecord::SchemaMigration.create(version: 10)
+
+    assert_equal [
+      ["down", "001", "Valid people have last names"],
+      ["up",   "002", "We need reminders"],
+      ["down", "003", "Innocent jointable"],
+      ["up",   "010", "********** NO FILE **********"],
+    ], ActiveRecord::Migrator.migrations_status(path)
+  end
+
+  def test_migrations_status_from_two_directories
+    paths = [MIGRATIONS_ROOT + "/valid_with_timestamps", MIGRATIONS_ROOT + "/to_copy_with_timestamps"]
+
+    ActiveRecord::SchemaMigration.create(version: "20100101010101")
+    ActiveRecord::SchemaMigration.create(version: "20160528010101")
+
+    assert_equal [
+      ["down", "20090101010101", "People have hobbies"],
+      ["down", "20090101010202", "People have descriptions"],
+      ["up",   "20100101010101", "Valid with timestamps people have last names"],
+      ["down", "20100201010101", "Valid with timestamps we need reminders"],
+      ["down", "20100301010101", "Valid with timestamps innocent jointable"],
+      ["up",   "20160528010101", "********** NO FILE **********"],
+    ], ActiveRecord::Migrator.migrations_status(paths)
+  end
+
   def test_migrator_interleaved_migrations
     pass_one = [Sensor.new("One", 1)]