Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Ensure valid migration filename on generating migration #7419

Merged
merged 1 commit into from

6 participants

Ian Bernactki Jeroen van Ingen Dmitry Vorotilin Steve Klabnik Egor Homakov José Valim
Ian Bernactki
rails g migration add_statistic_updated_at:datetime

create    db/migrate/20120822103649_add_statistic_updated_at:datetime.rb

rake db:migrate

llegal name for migration file: ...db/migrate/20120822103649_add_statistic_updated_at:datetime.rb
    (only lower case letters, numbers, and '_' allowed)
railties/lib/rails/generators/migration.rb
@@ -49,7 +49,11 @@ def migration_template(source, destination=nil, config={})
@migration_number = self.class.next_migration_number(migration_dir)
@migration_file_name = File.basename(destination).sub(/\.rb$/, '')
@migration_class_name = @migration_file_name.camelize
-
+
+ unless @migration_file_name =~ /^[_a-z0-9]+$/
+ raise ActiveRecord::IllegalMigrationNameError.new(@migration_file_name)
José Valim Owner

We should not reference ActiveRecord from the Rails directory. Or we should move this validation to the Active Record migration generator or provide a similar error in Rails. Given that this validation is specific to Active Record, I would do the former.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Ian Bernactki

Thanks.

I have moved validation to AR. Is it ok to use AR::IllegalMigrationNameError in tests?

Jeroen van Ingen

It is nice to raise an error if the name of the migration is incorrect.
But on the other side I think it's a little overkill...

Ian Bernactki

So.. Merge or Close?

Dmitry Vorotilin

I'm :+1:, just squash your commits into one and rebase, otherwise it will be the next request ;)

Steve Klabnik
Collaborator

Yep, both those will need done.

Ian Bernactki

All done

Egor Homakov

:+1: Also I just want rails putthingsback command that will revert just created crap

Steve Klabnik
Collaborator

Somehow, it's out of date again.

Ian Bernactki

I don't understand what is wrong. What should I do?

Steve Klabnik
Collaborator

Rebase it again. This happens often when CHANGELOGS get merged, they tend to cause a conflict.

Ian Bernactki

Rebased

Egor Homakov

@steveklabnik you don't see a green button right? Btw why github doesn't show other people that this PR needs to be rebased?

Ian Bernactki

Another one rebase :)

Steve Klabnik
Collaborator

Btw why github doesn't show other people that this PR needs to be rebased?

No idea. You should tell them. ;)

Ian Bernactki

Is it unavailable again?

Steve Klabnik
Collaborator

Oh, no, you're good. I'm not allowed to merge, or I would. You'll have to wait for someone on core.

Ian Bernactki

bump!

José Valim josevalim merged commit 75bde02 into from
Prem Sichanugrist sikachu referenced this pull request from a commit in sikachu/rails
Prem Sichanugrist sikachu Update Active Record CHANGELOG for #7419 db8460c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 6, 2012
  1. Ian Bernactki

    add mini-validator on creating migration

    releu authored
    move validation to AR
This page is out of date. Refresh to see the latest.
9 activerecord/lib/rails/generators/active_record/migration/migration_generator.rb
View
@@ -7,6 +7,7 @@ class MigrationGenerator < Base
def create_migration_file
set_local_assigns!
+ validate_file_name!
migration_template "migration.rb", "db/migrate/#{file_name}.rb"
end
@@ -41,6 +42,14 @@ def index_name_for(attribute)
attribute.name.singularize.foreign_key
end.to_sym
end
+
+ private
+
+ def validate_file_name!
+ unless file_name =~ /^[_a-z0-9]+$/
+ raise IllegalMigrationNameError.new(file_name)
+ end
+ end
end
end
end
7 railties/test/generators/migration_generator_test.rb
View
@@ -28,6 +28,13 @@ def test_migration_with_class_name
run_generator [migration]
assert_migration "db/migrate/change_title_body_from_posts.rb", /class #{migration} < ActiveRecord::Migration/
end
+
+ def test_migration_with_invalid_file_name
+ migration = "add_something:datetime"
+ assert_raise ActiveRecord::IllegalMigrationNameError do
+ run_generator [migration]
+ end
+ end
def test_add_migration_with_attributes
migration = "add_title_body_to_posts"
Something went wrong with that request. Please try again.