1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Guard against using VERSION with db:rollback (#41430)

* Guard against using VERSION with db:rollback

I recently ran a migration that I needed to rollback, and admittedly, I often forget the proper incantation for this 😅
so the first thing I tried was to run `bin/rake db:rollback VERSION=123454679`. I had hoped that this reverted my
migration back and at first glance I thought it worked. However on closer inspection I realized that it was a different
migration, which initially confused me.

So I looked over the docs and saw that I was using the rake task incorrectly, and promptly corrected my mistake.

Proposal

Looking at the how the `:down` task is defined we see

8dc7439058/activerecord/lib/active_record/railties/databases.rake (L206-L211)

This got me thinking that maybe it would be helpful to have the opposite of this guard defined in `rollback` so that if
`VERSION` is passed it will raise an exception instead of just negecting the extra argument. This could help the user
realize that something went wrong instead of just seeing output and assuming that the rollback happened.

Change

We now raise an execption if `VERSION` is passed when attempting to rollback a migration

* update test name and fix failing test

* remove byebug

[Nick Borromeo + Kate Travers + Rafael Mendonça França]
This commit is contained in:
Nick Borromeo 2021-02-15 16:30:05 -08:00 committed by GitHub
parent a642b7c02b
commit 0f09dfca36
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 1 deletions

View file

@ -275,6 +275,7 @@ db_namespace = namespace :db do
desc "Rolls the schema back to the previous version (specify steps w/ STEP=n)."
task rollback: :load_config do
ActiveRecord::Tasks::DatabaseTasks.raise_for_multi_db(command: "db:rollback")
raise "VERSION is not supported - To rollback a specific version, use db:migrate:down" if ENV["VERSION"]
step = ENV["STEP"] ? ENV["STEP"].to_i : 1

View file

@ -136,6 +136,23 @@ module ApplicationTests
assert_match(/up\s+002\s+Two migration/, output)
end
test "rollback raises when VERSION is passed" do
app_file "db/migrate/01_one_migration.rb", <<-MIGRATION
class OneMigration < ActiveRecord::Migration::Current
end
MIGRATION
app_file "db/migrate/02_two_migration.rb", <<-MIGRATION
class TwoMigration < ActiveRecord::Migration::Current
end
MIGRATION
rails "db:migrate"
output = rails("db:rollback", "VERSION=01_one_migration.rb", allow_failure: true)
assert_match(/VERSION is not supported - To rollback a specific version, use db:migrate:down/, output)
end
test "migration with 0 version" do
app_file "db/migrate/01_one_migration.rb", <<-MIGRATION
class OneMigration < ActiveRecord::Migration::Current
@ -416,7 +433,8 @@ module ApplicationTests
output = rails("generate", "model", "author", "name:string")
version = output =~ %r{[^/]+db/migrate/(\d+)_create_authors\.rb} && $1
rails "db:migrate", "db:rollback", "db:forward", "db:migrate:up", "db:migrate:down", "VERSION=#{version}"
rails "db:migrate", "db:rollback", "db:forward"
rails "db:migrate:up", "db:migrate:down", "VERSION=#{version}"
assert_not File.exist?("db/schema.rb"), "should not dump schema when configured not to"
end