Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

There was a problem

You must be a member of this account.

This project is archived and is in readonly mode.

[PATCH] Migrations with Improper Names Generate Invalid Code

#4835

In beta 4, if you generate a migration and use a name for it that does not start with "add" or "remove", the template fails to generate valid code. The template should not create code for adding or removing attributes unless the generator knows for sure that the developer is intending to do an add or remove migration -- he may just want to create a table without generating a model (e.g. "rails g migration CreateArticleTable name:string author:string").

If you attempt that example in beta 4, you end up with bogus template code that looks like:

class CreateArticleTable < ActiveRecord::Migration
  def self.up
    _column :create_article_tables, :name
    _column :create_article_tables, :author
  end

  def self.down
    add_column :create_article_tables, :author
    add_column :create_article_tables, :name
  end
end

This should instead generate empty methods:

class CreateArticleTable < ActiveRecord::Migration
  def self.up

  end

  def self.down

  end
end

I've attached a patch that fixes the template to do this. I don't have any tests for it; it seemed awkward to try to create a test for the presence of method bodies.

Reported by sabat (at area51) · June 11th, 2010 @ 08:49 PM

State: committed
Milestone: 3.0.2
Assigned to: José Valim José Valim
Importance: Low

Activity

  1. sabat (at area51)
    sabat (at area51)
    • Tag changed from beta4 migrations template to beta4, migrations, template

    Fixed patch: I hadn't followed the rules for generating it. Sorry. :-o

    June 11th, 2010 @ 08:59 PM

  2. sabat (at area51)
    sabat (at area51)
    • Title changed from Migrations with Improper Names Generate Invalid Code to [PATCH] Migrations with Improper Names Generate Invalid Code

    June 11th, 2010 @ 09:00 PM

  3. sabat (at area51)
    sabat (at area51)

    Fixed a minor formatting problem in previous version of patch.

    June 11th, 2010 @ 09:16 PM

  4. Rohit Arondekar
    Rohit Arondekar
    • Milestone set to 3.x
    • Tag changed from beta4, migrations, template to rails 3, activerecord, migrations, template
    • State changed from new to open
    • Assigned user set to José Valim

    Confirmed issue on Rails master.

    Patch applies cleanly, and solves the problem.

    However does the change require tests?

    June 16th, 2010 @ 09:10 AM

  5. José Valim
    José Valim
    • Milestone cleared.

    Tests please!

    June 16th, 2010 @ 09:13 AM

  6. José Valim
    José Valim

    I need to apply this for the upcoming release. Could someone please work on tests? :)

    June 19th, 2010 @ 11:48 PM

  7. Rohit Arondekar
    Rohit Arondekar

    I had a go at it. Ended up adding only one test. Feedback required. :)

    Aside: I found a failing test in Railties, http://pastie.org/1011980 Might not have the time to investigate today, will do ASAP.

    June 20th, 2010 @ 03:34 AM

  8. José Valim
    José Valim

    Yes, this test is failing on purpose. There is a tip, you can always run just generator tests with: rake test TEST_DIR=generators inside the railties dir. Your test looks good Rohit, I will apply both patches soon! :)

    June 20th, 2010 @ 09:22 AM

  9. Repository
    Repository
    • State changed from open to committed

    June 20th, 2010 @ 11:43 AM

  10. Rohit Arondekar
    Rohit Arondekar

    Jose, thanks for the tip. Btw my patch didn't include the original fixing patch so that will have to be applied separately.

    June 20th, 2010 @ 02:34 PM

  11. José Valim
  12. Rohit Arondekar
    Rohit Arondekar

    Oh my bad, the patch didn't have a reference to this ticket. :)

    In case anyone is looking for the commit -> http://github.com/rails/rails/commit/451594784552bd6955687d8ff617c4...

    June 20th, 2010 @ 02:39 PM

  13. Jeremy Kemper
    Jeremy Kemper
    • Milestone set to 3.0.2
    • Importance changed from to Low

    October 15th, 2010 @ 11:01 PM