Fix search for create_table_migration template override #13972

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
9 participants

A developer should be able to customize the template used by the model or migration generator, by adding a file to their source tree in a predictable location.

This functionality was likely broken for create table migrations by:
20e0415

After that change, you could now generate a migration:

rails g migration CreateFoo

and it would look for a custom template at <myproject>/lib/templates/active_record/migration/create_table_migration.rb

However, if you generated the migration by generating a model:

rails g model Bar

it would look for a template at the unpredictable and awkward path <myproject>/lib/templates/migration/templates/create_table_migration.rb

This change adds the migration generator's template paths to the model generator's search paths. When you generate a model, it will now look for templates in order:

<myproject>/lib/templates/active_record/model/create_table_migration.rb
<activerecord gem>/lib/rails/generators/active_record/model/templates/create_table_migration.rb
<myproject>/lib/templates/active_record/migration/create_table_migration.rb
<activerecord gem>/lib/rails/generators/active_record/migration/templates/create_table_migration.rb

This has the side-effect of allowing you to have different templates for creating a table, depending on whether you generated a model or the migration directly. That was not the motivation, but its likely more helpful than harmful.

  • Seeking feedback on how to properly modify the source_paths for ModelGenerator. I'm not very familiar with generator or Thor Action internals, so I had to blindly stab until I found something that worked. I particularly do not like the awkwardness of the ensure_migration_source_paths method - ideally that would be handled by some sort of initializer.
  • Confirm we want to retain the functionality introduced by 20e0415. It may be preferable to revert that commit. It introduced an awkward coupling between the ModelGenerator and MigrationGenerator that is exposed by this fix.
  • Add or modify tests around this functionality so it doesn't break again. Can someone point me to existing tests?
Fix search for create_table_migration template override
A developer should be able to customize the template used by the model or migration generator, by adding a file to their source tree in a predictable location.

This functionality was likely broken by:
20e0415

After that change, you could now generate a migration:

`rails g migration CreateFoo`

and it would look for a custom template at `myproject/lib/templates/active_record/migration/create_table_migration.rb`

However, if you generated the migration by generating a model:

`rails g model Bar`

it would look for a template at the unpredictable and awkward path `myproject/lib/templates/migration/templates/create_table_migration.rb`


This Pull Request adds the migration generator's template paths to the model generator's search paths. When you generate a model, it will now look for templates in order:

```
<myproject>/lib/templates/active_record/model/create_table_migration.rb
<activerecord gem>/lib/rails/generators/active_record/model/templates/create_table_migration.rb
<myproject>/lib/templates/active_record/migration/create_table_migration.rb
<activerecord gem>/lib/rails/generators/active_record/migration/templates/create_table_migration.rb
```

This has the side-effect of allowing you to have different templates for creating a table, depending on whether you generated a model or the migration directly.

No feedback yet - is this ready to merge?

Member

schneems commented Feb 17, 2014

Is there a way to add tests so the functionality does not regress later?

I would definitely like to add tests. I was hoping to get some guidance - if nobody knows of the proper/existing location, I can make one.

mbusch commented May 13, 2014

I just wanted to support this pull request, as I wonder just now why my template at lib/templates/active_record/migration/create_table_migration.rb doesn't apply.

Contributor

etipton commented Jun 11, 2014

+1 for this

Contributor

RKushnir commented Oct 3, 2014

👍 I guess, there's no way now to add default columns to all my model migrations, right?

gfodor commented Oct 22, 2014

Any chance of this being fixed for 4.2?

I'd also love to see this fixed. My particular use case is using UUID's for primary keys:

class CreateUsers < ActiveRecord::Migration
  def change
    create_table :users, id: :uuid do |t|
      t.timestamps
      t.string :email
    end
  end
end

I constantly forget that id: :uuid

That was the exact motivation for this PR

Member

sgrif commented Nov 1, 2016

@joshuaflanagan Are you still interested in working on this? This needs tests before it can be merged.

I've asked for guidance on how/where to add tests to cover this behavior, and haven't received any. If someone can point me to existing tests that cover the create_migration_file method I changed, I can give it a try. I have not had much luck in the past trying to run the full rails test suite.

Thank you. I'll give it a try.

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